Conversation
* Provides better compatibility and merging of defaults
Author
|
Sorry, tried to follow the contributions guide for which branch but because they're out-of-sync it's not going to be correct for develop. If required, I can start with the develop branch and push the changes there. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Potentially fixes #1056.
We discovered an issue where some defaults were saved to the database but not the complete set of fields. This meant that unless fields such as
general_records_ttlwere set, it wouldn't load the defaults and therefore never purge the records. The issue has the same description as #1056 but unsure of the exact cause from the bug reporter.When reviewing the code, I could see that the Settings class already had the correct syntax for merging defaults so it was easiest to therefore adjust the Admin class to simply use this (instead of duplicating). By ensuring the full defaults are loaded (if not set in the database), this also ensures any new options added won't be dropped in the future.
This may also resolve #1236.