Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options defaults fix #1365

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Options defaults fix #1365

wants to merge 2 commits into from

Conversation

timbutler
Copy link

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_ttl were 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.

@timbutler timbutler changed the title Optionsfix Options Defaults Fix Sep 13, 2022
@timbutler timbutler changed the title Options Defaults Fix Options defaults fix Sep 13, 2022
@timbutler timbutler changed the base branch from master to develop September 13, 2022 06:57
@timbutler timbutler changed the base branch from develop to master September 13, 2022 07:05
@timbutler
Copy link
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant