Use store as primary data source for preferences #11173
Conversation
|
Failing tests have uncovered another patron settings API that needs to be wrangled. |
|
RSLGTM -- it looks like there's a test to fix and also @jimchamp mentions there may be another settings call that needs to be updated to use the store. |
b558f90 to
713636f
Compare
|
Test was fixed. This requires a rebase and an update to the script added by #10943 to fix the failing workflows. |
| {"author": self.key, "limit": limit, "offset": offset} | ||
| ) | ||
|
|
||
| def get_users_settings(self): |
There was a problem hiding this comment.
This method, and get_safe_mode (another User method, several lines below), are called in various places throughout the codebase. get_safe_mode calls this method (get_users_settings) for preferences, so I think we need only to replace references to get_users_settings() with preferences() to establish a single source of truth for patron preferences.
I figure that this can handled in a separate PR, and is likely also a good first issue.
Updates `save_preferences` method to write only to the store tables. The `preferences` method has been updated to fetch preferences from the store, then fall-back to the thing database. If no preferences were found in either data source, the default preference object is returned.
0cbf266 to
ed35221
Compare
for more information, see https://pre-commit.ci
|
We added this PR to testing, we logged in as mek+test3Archive.org, we checked https://testing.openlibrary.org/people/xxxtests3/preferences.json, we set the preference to private reading log, verified the page was private (and that the preferences.json did not reflect this, i.e. are not being used; the store is) |
Supports #11009
Updates
save_preferences()to write only to the store.The
preferences()method has been updated to first query the store for preferences, then the legacy thing tables if nothing was found in the store. If no preferences were found in either data source, the default preference object is returned.It should be safe to deploy this before the preference migration script from #10943 is executed. However, we will need to update the query that populates the admin
pdtable to include the store tables (marking this as a draft until this is addressed).Also noting here that querying the store for this data takes a significantly longer time (minutes v. seconds).Edit: Query times for either data source are comparable.Technical
Testing
Screenshot
Stakeholders