-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Created ReviewRemindersDatabase and ReviewReminder #18364
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,172 @@ | ||||
/* | ||||
* Copyright (c) 2025 Eric Li <[email protected]> | ||||
* | ||||
* This program is free software; you can redistribute it and/or modify it under | ||||
* the terms of the GNU General Public License as published by the Free Software | ||||
* Foundation; either version 3 of the License, or (at your option) any later | ||||
* version. | ||||
* | ||||
* This program is distributed in the hope that it will be useful, but WITHOUT ANY | ||||
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A | ||||
* PARTICULAR PURPOSE. See the GNU General Public License for more details. | ||||
* | ||||
* You should have received a copy of the GNU General Public License along with | ||||
* this program. If not, see <http://www.gnu.org/licenses/>. | ||||
*/ | ||||
|
||||
package com.ichi2.anki.reviewreminders | ||||
|
||||
import com.ichi2.anki.R | ||||
import com.ichi2.anki.settings.Prefs | ||||
import com.ichi2.libanki.DeckId | ||||
import kotlinx.serialization.Serializable | ||||
import timber.log.Timber | ||||
import java.time.LocalTime | ||||
import java.time.format.DateTimeFormatter | ||||
import java.time.format.FormatStyle | ||||
import java.util.Locale | ||||
import kotlin.time.Duration | ||||
import kotlin.time.Duration.Companion.minutes | ||||
|
||||
internal typealias ReviewReminderId = Int | ||||
|
||||
/** | ||||
* A "review reminder" is a recurring scheduled notification that reminds the user | ||||
* to review their Anki cards. Individual instances of a review reminder firing and showing up | ||||
* on the user's phone are called "notifications". | ||||
* | ||||
* Below, a public way of creating review reminders is exposed via a companion object so that | ||||
* reminders with invalid IDs are never created. This class is annotated | ||||
* with @ConsistentCopyVisibility to ensure copy() is private too and does not leak the constructor. | ||||
* | ||||
* TODO: add remaining fields planned for GSoC 2025. | ||||
* | ||||
* @param id Unique, auto-incremented ID of the review reminder. | ||||
* @param time See [ReviewReminderTime]. | ||||
* @param snoozeAmount See [SnoozeAmount]. | ||||
* @param cardTriggerThreshold If, at the time of the reminder, less than this many cards are due, the notification is not triggered. | ||||
* @param did The deck this reminder is associated with, or [APP_WIDE_REMINDER_DECK_ID] if it is an app-wide reminder. | ||||
*/ | ||||
@Serializable | ||||
@ConsistentCopyVisibility | ||||
data class ReviewReminder private constructor( | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be called "ReviewReminderSettings" or something like that. Because it does not represents a reminedr. It represents the setting that is used for a reminder by day. This name seems to indicate that this corresponds to a very specific reminder. That is, one specific message shown at a specific point in time to the user There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If possible, I'd actually like to keep it as If anything, "ReviewReminderSettings" seems too global. It feels like it's referring to the settings for the entire review reminder system, rather than the settings for a single reminder. Additionally, if we were to rename ReviewReminder to ReviewReminderSettings, why should a group of "settings" have an ID associated with it? Perhaps "ReviewReminderConfiguration" is a better name? But that's a bit long and unwieldy, and it means I have to rename my test file to the even longer "ReviewReminderConfigurationsDatabaseTest". "ReviewReminder" just seems simpler and more succinct. I believe we should standardize the following terminology:
In other words, a review reminder triggers notifications and appears on a concrete level to the user as notifications, but review reminders != notifications. Thus, the ReviewReminder class carries the data associated with each ReviewReminder. |
||||
val id: ReviewReminderId, | ||||
val time: ReviewReminderTime, | ||||
val snoozeAmount: SnoozeAmount, | ||||
val cardTriggerThreshold: Int, | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'd really need to document that, it's not very clear unless one have read your proposal and remember it, or know the feature well |
||||
val did: DeckId, | ||||
var enabled: Boolean, | ||||
) { | ||||
init { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a single There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I think I'd actually like to keep the separate
This comment was marked as outdated.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't that make it so that if the |
||||
require(cardTriggerThreshold >= 0) { "Card trigger threshold must be >= 0" } | ||||
} | ||||
|
||||
/** | ||||
* The time of day at which reminders will send a notification. | ||||
*/ | ||||
@Serializable | ||||
data class ReviewReminderTime( | ||||
val hour: Int, | ||||
val minute: Int, | ||||
) { | ||||
init { | ||||
require(hour in 0..23) { "Hour must be between 0 and 23" } | ||||
require(minute in 0..59) { "Minute must be between 0 and 59" } | ||||
} | ||||
|
||||
override fun toString(): String = | ||||
LocalTime | ||||
.of(hour, minute) | ||||
.format( | ||||
DateTimeFormatter | ||||
.ofLocalizedTime(FormatStyle.SHORT) | ||||
.withLocale(Locale.getDefault()), | ||||
) | ||||
|
||||
fun toMinutesFromMidnight(): Int = hour * 60 + minute | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may be able to do something with the fun elapsedSinceMidnight(): Duration = (hour.hours + minute.minutes)
val seconds = time.elapsedSinceMidnight().inWholeSeconds |
||||
} | ||||
|
||||
/** | ||||
* Types of snooze behaviour that can be present on notifications sent by review reminders. | ||||
*/ | ||||
@Serializable | ||||
sealed class SnoozeAmount { | ||||
/** | ||||
* The snooze button will never appear on notifications set by this review reminder. | ||||
*/ | ||||
@Serializable | ||||
data object Disabled : SnoozeAmount() | ||||
|
||||
/** | ||||
* The snooze button will always be available on notifications sent by this review reminder. | ||||
*/ | ||||
@Serializable | ||||
data class Infinite( | ||||
val timeInterval: Duration, | ||||
) : SnoozeAmount() { | ||||
init { | ||||
require(timeInterval >= 0.minutes) { "Snooze time interval must be >= 0 minutes" } | ||||
} | ||||
} | ||||
|
||||
/** | ||||
* The snooze button can be pressed a maximum amount of times on notifications sent by this review reminder. | ||||
* After it has been pressed that many times, the button will no longer appear. | ||||
*/ | ||||
@Serializable | ||||
data class SetAmount( | ||||
val timeInterval: Duration, | ||||
val maxSnoozes: Int, | ||||
) : SnoozeAmount() { | ||||
init { | ||||
require(timeInterval >= 0.minutes) { "Snooze time interval must be >= 0 minutes" } | ||||
require(maxSnoozes >= 0) { "Max snoozes must be >= 0" } | ||||
} | ||||
} | ||||
} | ||||
|
||||
companion object { | ||||
/** | ||||
* The "deck ID" field for review reminders that are app-wide rather than deck-specific. | ||||
*/ | ||||
const val APP_WIDE_REMINDER_DECK_ID = -1L | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may or may not want this as a sealed class, depending on how low-level this API will be |
||||
|
||||
/** | ||||
* IDs start at this value and climb upwards by one each time. | ||||
*/ | ||||
private const val FIRST_REMINDER_ID = 0 | ||||
|
||||
/** | ||||
* Create a new review reminder. This will allocate a new ID for the reminder. | ||||
* @return A new [ReviewReminder] object. | ||||
* @see [ReviewReminder] | ||||
*/ | ||||
fun createReviewReminder( | ||||
time: ReviewReminderTime, | ||||
snoozeAmount: SnoozeAmount, | ||||
cardTriggerThreshold: Int, | ||||
did: DeckId = APP_WIDE_REMINDER_DECK_ID, | ||||
enabled: Boolean = true, | ||||
) = ReviewReminder( | ||||
id = getAndIncrementNextFreeReminderId(), | ||||
time, | ||||
snoozeAmount, | ||||
cardTriggerThreshold, | ||||
did, | ||||
enabled, | ||||
) | ||||
|
||||
/** | ||||
* Get and return the next free reminder ID which can be associated with a new review reminder. | ||||
* Also increment the next free reminder ID stored in SharedPreferences. | ||||
* Since there are 4 billion IDs available, this should not overflow in practice. | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
* @return The next free reminder ID. | ||||
*/ | ||||
private fun getAndIncrementNextFreeReminderId(): ReviewReminderId { | ||||
val nextFreeId = Prefs.getInt(R.string.review_reminders_next_free_id, FIRST_REMINDER_ID) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Look into Index: AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt (revision 9d0e81e75ce77a0d7ce8949b5344af2318389b23)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt (date 1750647615025)
@@ -16,7 +16,7 @@
package com.ichi2.anki.reviewreminders
-import com.ichi2.anki.R
+import com.ichi2.anki.reviewreminders.ReviewReminder.Companion.APP_WIDE_REMINDER_DECK_ID
import com.ichi2.anki.settings.Prefs
import com.ichi2.libanki.DeckId
import kotlinx.serialization.Serializable
@@ -131,11 +131,6 @@
*/
const val APP_WIDE_REMINDER_DECK_ID = -1L
- /**
- * IDs start at this value and climb upwards by one each time.
- */
- private const val FIRST_REMINDER_ID = 0
-
/**
* Create a new review reminder. This will allocate a new ID for the reminder.
* @return A new [ReviewReminder] object.
@@ -163,8 +158,8 @@
* @return The next free reminder ID.
*/
private fun getAndIncrementNextFreeReminderId(): ReviewReminderId {
- val nextFreeId = Prefs.getInt(R.string.review_reminders_next_free_id, FIRST_REMINDER_ID)
- Prefs.putInt(R.string.review_reminders_next_free_id, nextFreeId + 1)
+ val nextFreeId = Prefs.reviewReminderNextFreeId
+ Prefs.reviewReminderNextFreeId = nextFreeId + 1
Timber.d("Generated next free review reminder ID: $nextFreeId")
return nextFreeId
}
Index: AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt b/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt (revision 9d0e81e75ce77a0d7ce8949b5344af2318389b23)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt (date 1750647582518)
@@ -204,4 +204,6 @@
var isDevOptionsEnabled: Boolean
get() = getBoolean(R.string.dev_options_enabled_by_user_key, false) || BuildConfig.DEBUG
set(value) = putBoolean(R.string.dev_options_enabled_by_user_key, value)
+
+ var reviewReminderNextFreeId by intPref(R.string.review_reminders_next_free_id, defaultValue = 0)
}
|
||||
Prefs.putInt(R.string.review_reminders_next_free_id, nextFreeId + 1) | ||||
Timber.d("Generated next free review reminder ID: $nextFreeId") | ||||
return nextFreeId | ||||
} | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,212 @@ | ||
/* | ||
* Copyright (c) 2025 Eric Li <[email protected]> | ||
* | ||
* This program is free software; you can redistribute it and/or modify it under | ||
* the terms of the GNU General Public License as published by the Free Software | ||
* Foundation; either version 3 of the License, or (at your option) any later | ||
* version. | ||
* | ||
* This program is distributed in the hope that it will be useful, but WITHOUT ANY | ||
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A | ||
* PARTICULAR PURPOSE. See the GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License along with | ||
* this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package com.ichi2.anki.reviewreminders | ||
|
||
import android.content.Context | ||
import androidx.annotation.VisibleForTesting | ||
import androidx.core.content.edit | ||
import com.ichi2.anki.AnkiDroidApp | ||
import com.ichi2.anki.showError | ||
import com.ichi2.libanki.DeckId | ||
import kotlinx.serialization.SerializationException | ||
import kotlinx.serialization.json.Json | ||
|
||
/** | ||
* Manages the storage and retrieval of [ReviewReminder]s in SharedPreferences. | ||
* | ||
* [ReviewReminder]s can either be tied to a specific deck and trigger based on the number of cards | ||
* due in that deck, or they can be app-wide reminders that trigger based on the total number | ||
* of cards due across all decks. | ||
*/ | ||
class ReviewRemindersDatabase( | ||
val context: Context, | ||
) { | ||
companion object { | ||
/** | ||
* Key in SharedPreferences for retrieving deck-specific reminders. | ||
* Should have deck ID appended to its end, ex. "review_reminders_deck_12345". | ||
* Its value is a HashMap<[ReviewReminderId], [ReviewReminder]> serialized as a JSON String. | ||
*/ | ||
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) | ||
const val DECK_SPECIFIC_KEY = "review_reminders_deck_" | ||
|
||
/** | ||
* Key in SharedPreferences for retrieving app-wide reminders. | ||
* Its value is a HashMap<[ReviewReminderId], [ReviewReminder]> serialized as a JSON String. | ||
*/ | ||
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) | ||
const val APP_WIDE_KEY = "review_reminders_app_wide" | ||
} | ||
|
||
/** | ||
* Decode an encoded HashMap<[ReviewReminderId], [ReviewReminder]> JSON string. | ||
* It is possible for Json.decodeFromString to throw [SerializationException]s if a serialization | ||
* error is encountered, or [IllegalArgumentException]s if the decoded object is not a HashMap<[ReviewReminderId], [ReviewReminder]>. | ||
* @see Json.decodeFromString | ||
*/ | ||
private fun decodeJson(jsonString: String): HashMap<ReviewReminderId, ReviewReminder> = | ||
try { | ||
// If, during development, the review reminders storage schema (i.e. ReviewReminder) is modified, | ||
// this class will attempt to read review reminders stored in the old schema via the new schema, causing a SerializationException. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll want to plan how to update the schema |
||
// To delete all review reminder keys in SharedPreferences, uncomment the following line. | ||
// AnkiDroidApp.sharedPrefs().edit { AnkiDroidApp.sharedPrefs().all.keys.filter { it.startsWith("review_reminders_") }.forEach { remove(it) } } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this discussed and confirmed with mentors? We may want to use a separate file for these
|
||
|
||
Json.decodeFromString<HashMap<ReviewReminderId, ReviewReminder>>(jsonString) | ||
} catch (e: SerializationException) { | ||
showError( | ||
context, | ||
"Something went wrong. A serialization error was encountered while retrieving review reminders.", | ||
) | ||
hashMapOf() | ||
} catch (e: IllegalArgumentException) { | ||
showError( | ||
context, | ||
"Something went wrong. An unexpected data type was read while retrieving review reminders.", | ||
) | ||
hashMapOf() | ||
} | ||
|
||
/** | ||
* Encode a Map<[ReviewReminderId], [ReviewReminder]> as a JSON string. | ||
* it is possible for Json.encodeToString to throw [SerializationException]s if a serialization | ||
* error is encountered. | ||
* @see Json.encodeToString | ||
*/ | ||
private fun encodeJson(reminders: Map<ReviewReminderId, ReviewReminder>): String = | ||
try { | ||
Json.encodeToString(reminders) | ||
} catch (e: SerializationException) { | ||
showError( | ||
context, | ||
"Something went wrong. A serialization error was encountered while saving review reminders.", | ||
) | ||
"{}" | ||
} | ||
|
||
/** | ||
* Get the [ReviewReminder]s for a specific key. | ||
*/ | ||
private fun getRemindersForKey(key: String): HashMap<ReviewReminderId, ReviewReminder> { | ||
val jsonString = AnkiDroidApp.sharedPrefs().getString(key, null) ?: return hashMapOf() | ||
return decodeJson(jsonString) | ||
} | ||
|
||
/** | ||
* Get the [ReviewReminder]s for a specific deck. | ||
*/ | ||
fun getRemindersForDeck(did: DeckId): HashMap<ReviewReminderId, ReviewReminder> = getRemindersForKey(DECK_SPECIFIC_KEY + did) | ||
|
||
/** | ||
* Get the app-wide [ReviewReminder]s. | ||
*/ | ||
fun getAllAppWideReminders(): HashMap<ReviewReminderId, ReviewReminder> = getRemindersForKey(APP_WIDE_KEY) | ||
|
||
/** | ||
* Get all [ReviewReminder]s that are associated with a specific deck, grouped by deck ID. | ||
*/ | ||
private fun getAllDeckSpecificRemindersGrouped(): Map<DeckId, HashMap<ReviewReminderId, ReviewReminder>> { | ||
return AnkiDroidApp | ||
.sharedPrefs() | ||
.all | ||
.filterKeys { it.startsWith(DECK_SPECIFIC_KEY) } | ||
.mapNotNull { (key, value) -> | ||
val did = key.removePrefix(DECK_SPECIFIC_KEY).toLongOrNull() ?: return@mapNotNull null | ||
val reminders = decodeJson(value.toString()) | ||
did to reminders | ||
}.toMap() | ||
} | ||
|
||
/** | ||
* Get all [ReviewReminder]s that are associated with a specific deck, all in a single flattened map. | ||
*/ | ||
fun getAllDeckSpecificReminders(): HashMap<ReviewReminderId, ReviewReminder> = getAllDeckSpecificRemindersGrouped().flatten() | ||
|
||
/** | ||
* Edit the [ReviewReminder]s for a specific key. | ||
* @param key | ||
* @param reminderEditor A lambda that takes the current map and returns the updated map. | ||
*/ | ||
private fun editRemindersForKey( | ||
key: String, | ||
reminderEditor: (HashMap<ReviewReminderId, ReviewReminder>) -> Map<ReviewReminderId, ReviewReminder>, | ||
) { | ||
val existingReminders = getRemindersForKey(key) | ||
val updatedReminders = reminderEditor(existingReminders) | ||
AnkiDroidApp.sharedPrefs().edit { | ||
putString(key, encodeJson(updatedReminders)) | ||
} | ||
} | ||
|
||
/** | ||
* Edit the [ReviewReminder]s for a specific deck. | ||
* @param did | ||
* @param reminderEditor A lambda that takes the current map and returns the updated map. | ||
*/ | ||
fun editRemindersForDeck( | ||
did: DeckId, | ||
reminderEditor: (HashMap<ReviewReminderId, ReviewReminder>) -> Map<ReviewReminderId, ReviewReminder>, | ||
) = editRemindersForKey(DECK_SPECIFIC_KEY + did, reminderEditor) | ||
|
||
/** | ||
* Edit the app-wide [ReviewReminder]s. | ||
* @param reminderEditor A lambda that takes the current map and returns the updated map. | ||
*/ | ||
fun editAllAppWideReminders(reminderEditor: (HashMap<ReviewReminderId, ReviewReminder>) -> Map<ReviewReminderId, ReviewReminder>) = | ||
editRemindersForKey(APP_WIDE_KEY, reminderEditor) | ||
|
||
/** | ||
* Edit all [ReviewReminder]s that are associated with a specific deck by operating on a single mutable map. | ||
*/ | ||
fun editAllDeckSpecificReminders(reminderEditor: (HashMap<ReviewReminderId, ReviewReminder>) -> Map<ReviewReminderId, ReviewReminder>) { | ||
val existingRemindersGrouped = getAllDeckSpecificRemindersGrouped() | ||
val existingRemindersFlattened = existingRemindersGrouped.flatten() | ||
|
||
val updatedRemindersFlattened = reminderEditor(existingRemindersFlattened) | ||
val updatedRemindersGrouped = updatedRemindersFlattened.groupByDeckId() | ||
|
||
val existingKeys = existingRemindersGrouped.keys.map { DECK_SPECIFIC_KEY + it } | ||
|
||
AnkiDroidApp.sharedPrefs().edit { | ||
// Clear existing review reminder keys in SharedPreferences | ||
existingKeys.forEach { remove(it) } | ||
// Add the updated ones back in | ||
updatedRemindersGrouped.forEach { (did, reminders) -> | ||
putString(DECK_SPECIFIC_KEY + did, encodeJson(reminders)) | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Utility function for flattening maps of [ReviewReminder]s grouped by deck ID into a single map. | ||
*/ | ||
private fun Map<DeckId, HashMap<ReviewReminderId, ReviewReminder>>.flatten(): HashMap<ReviewReminderId, ReviewReminder> = | ||
hashMapOf<ReviewReminderId, ReviewReminder>().apply { | ||
this@flatten.forEach { (_, reminders) -> | ||
putAll(reminders) | ||
} | ||
} | ||
|
||
/** | ||
* Utility function for grouping maps of [ReviewReminder]s by deck ID. | ||
*/ | ||
private fun Map<ReviewReminderId, ReviewReminder>.groupByDeckId(): Map<DeckId, HashMap<ReviewReminderId, ReviewReminder>> = | ||
hashMapOf<DeckId, HashMap<ReviewReminderId, ReviewReminder>>().apply { | ||
this@groupByDeckId.forEach { (id, reminder) -> | ||
getOrPut(reminder.did) { hashMapOf() }[id] = reminder | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: look into
value class
for a low/0-cost abstraction which stops anInt
being passed in where aReviewReminderId
is required