Created ReviewRemindersDatabase and ReviewReminder
Purpose / Description
Review Reminders database system.
- Created
ReviewRemindersDatabase, which contains methods for reading and writing to a Preferences Datastore instance for storing review reminder data - Created
ReviewReminder, which defines the review reminder data class - Created Robolectric unit tests for
ReviewRemindersDatabaseandReviewReminder
Fixes
- For GSoC 2025: Review Reminders
Approach
Using Preferences Datastore was the original method I proposed for storing review reminder data in my original project proposal: see [1]
Why did I initially choose Preferences Datastore for storing review reminder data?
- Google recommends using it over Shared Preferences: see [2]
- It has been used before. Past attempts at creating a review reminders system (see [3]) used Preferences Datastore.
However, we've now chosen to use SharedPreferences instead. It's consistent with what the rest of AnkiDroid uses and doesn't necessitate a whole new import. It's also a lot simpler and straightforward.
How Has This Been Tested?
ReviewRemindersDatabaseTestdirectly or indirectly exercises all methods withinReviewRemindersDatabase.ReviewReminderTestexercises the ID allocation method ofReviewReminder- On a physical Samsung S23, API 34., buttons to add, edit, and delete reminders (coming soon in a later PR) cause expected changes to the database and the database changes persist between app sessions.
Learning
I've tried my best to make my code clean and safe. In particular, allocating and deallocating IDs is handled internally and any developer creating a new ReviewReminder should be unable to manually forge an ID.
Originally, I planned to create different types of review reminders. I've now abandoned that in favour of a simpler approach involving snooze functionality.
Checklist
- [x] You have a descriptive commit message with a short title (first line, max 50 chars).
- [x] You have commented your code, particularly in hard-to-understand areas
- [x] You have performed a self-review of your own code
- [x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
- [x] UI Changes: You have tested your change using the Google Accessibility Scanner
Hi, here's a first draft of what I'd like the review reminder storage local database storage system to be like. Looking forward to constructive criticism. For now, I’ve put these files in anki/notifications since that seemed a reasonable place to put them. Let me know if I should move them elsewhere, such as by creating a new anki/reviewreminders folder.
Had to do a second force push because I forgot to move the files to a new package as outlined at #18318.
Repeated force push because I forgot to rebase, my bad.
Hi, I've refactored to use SharedPreferences instead of Preferences Datastore. In retrospect, thanks for telling me to use it, it really is a lot more simple and I should have started with this method. Using Datastore was like trying to crack a nut with a sledgehammer. The only negative consequence is that my branch I'm trying to merge these changes in from is still called review-reminders-datastore, so that's a bit confusing-looking but shouldn't be an issue.
Regarding other overly-complex points I've simplified: the fancy ID-allocation system is now gone, and I've decided to move the addReminderForDeck, editReminderForDeck, and deleteReminderForDeck methods to the UI onClick methods since that should be the only place they're called from, and so they don't need to be in ReviewRemindersDatabase.
I've also done my best to clean up the docstrings, as per feedback. Previously they were more ad-hoc, now I've tried my best to make them genuine pieces of documentation.
I realized the SharedPreferences keys are too short. Since there's tons of other things being stored in SharedPreferences, it's probably a good idea to append review_reminders_ to the beginning of the keys I use in these files so they don't collide with any other keys.
I realized while I was building out the ScheduleReminders RecyclerView more that:
- There are typos in this commit, whoops.
- I need to store a review reminder's corresponding deck ID within ReviewReminder for the RecyclerView to work properly in GLOBAL_DECK_SPECIFIC scope mode.
- I need to add a method to retrieve the reminders in all decks to ReviewRemindersDatastore, to be used in GLOBAL_DECK_SPECIFIC scope mode.
Rebased and fixed a List that should have been a MutableList.
- Added a ReviewReminderId typealias.
- Tweaked the contracts of the database's "edit" methods to now require a lambda which returns a List rather than strictly a MutableList.
- Added an editAllDeckSpecificReminders method. I realized I needed it for the deck-specific scope mode's editing while creating the bulk deletion feature.
- Added a new test to ReviewRemindersDatabaseTest to test editAllDeckSpecificReminders.
- Cleaned up some syntax in the two new test files.
Please do not review this PR. I'm actively refactoring it as I've realized I can streamline the UX. Sorry for asking for reviews and then going back and changing things again, I really do appreciate the constructive criticism of my code and am sorry for pulling it out of the merge pipeline when it's so close to the end.
Refactored. Removed ReviewReminderTypes and replaced it with snoozing. Looking forward to reviews!
Thanks Ashish! I've added some try-catch blocks to handle corrupt JSON strings and added a few unit tests to test that behaviour.
- Added getAllDeckSpecificRemindersGrouped
- Refactored getAllDeckSpecificReminders and editAllDeckSpecificReminders to use it
- Replaced
ListwithHashMapto make editing individual review reminders more efficient - Added two utility methods to
ReviewRemindersDatabaseto flatten and group maps of review reminders - Updated
ReviewRemindersDatabaseTest, which no longer needs to inefficiently sort lists of review reminders when asserting
- Added the
enabledproperty toReviewReminder
@ericli3690 I wanted to check—when we delete a deck, do its associated reminders still remain stored in SharedPreferences?
Thanks for the reviews.
@SanjaySargam Regarding whether review reminders should remain in SharedPreferences after the corresponding deck is deleted: no, they should not, and don't worry, it's on my to-do list. Thanks for reminding me!
- Added try-catch for JSON encoding
- Created a
APP_WIDE_REMINDER_DECK_IDconstant - Added
ReviewReminderTimeandSnoozeAmountclasses - Cleaned up references to
context.sharedPrefs()to instead usePrefsorAnkiDroidApp.sharedPrefs() - Added a new preferences resource to allow for
Prefs.getString()
- Used an inline value class for
ReviewReminderId, modifiedReviewReminderTestaccordingly - Modified
ReviewReminderTime's methods to better use the Duration API - Modified
ReviewReminderCardTriggerThresholdto be an inline value class - Moved
ReviewReminderfield types out ofReviewReminderso that they are no longer nested classes; this allows for less verbose code in other files ReviewReminderScopefromScheduleRemindersis now used inReviewReminderto indicate deck ID, it will be used in both places, as fundamentally indicating what scope is being edited byScheduleRemindersand which scope aReviewReminderis attached to can be done by the same sealed data class; I personally think this is an elegant abstraction- Used
by intPreffor the next free review reminder ID - Shortened the docstring of
getAndIncrementNextFreeReminderId - Implemented the backend of the
ReviewRemindermigration process, includingattemptSchemaMigrationandOldReviewReminderSchema; the use of these backend methods fromScheduleRemindersis also complete and has unit tests, but will be delivered in the upcoming RecyclerView PR - Added an explanation of the
ReviewRemindermigration process to the docstring ofReviewReminder - Added
getAllReviewReminderSharedPrefsAsMapanddeleteAllReviewReminderSharedPrefshelper methods toReviewRemindersDatabaseand added tests for them toReviewRemindersDatabaseTest - Cleaned up tests in
ReviewRemindersDatabaseTestto minimize code duplication - Made
ReviewRemindersDatabaseentirely Context-free; error-checking and dialog-throwing is complete in the form ofScheduleReminders.catchDatabaseExceptions, but will be delivered in the upcoming RecyclerView PR - Added lines to
ReviewRemindersDatabasedocstrings indicating that methods may throwSerializationExceptions/IllegalArgumentExceptionsand should be caught within the upcomingScheduleReminders.catchDatabaseExceptionsmethod
- Rebased on main
- Fixed a bug where migrated review reminder HashMaps would have incorrect, unmigrated keys; unit tests to check for this have been developed but will be lumped together with the RecyclerView PR
- Rebased on main
- Fixed typo: Added the missing
enabledparam to the docstring ofReviewReminder - Tweaked a few tests to validate manually passing the optional
enabledfield tocreateReviewReminder
- Moved
getAndIncrementNewFreeReminderIdto ReviewReminderId - Modified
requireclauses of ReviewReminderSnoozeAmount - Renamed
timeIntervalproperty of ReviewReminderSnoozeAmount tointerval - Added more documentation to the docstrings of ReviewReminderSnoozeAmount and
ReviewReminderScope.DeckSpecific.getDeckName - Rebased on main