Skip to content

Commit bffdf9f

Browse files
committed
feat(reminders): handle deck deletion
GSoC 2025: Review Reminders - Added logic for checking if a deck exists before returning deck-specific reminders to ReviewRemindersDatabase. This code uses the `decks.have` method, since it seemed like the most straightforward way to accomplish deck-existence-checking. - `decks.have` was previously marked as "unused". I've removed this annotation. - Added a deleteAllRemindersForDeck helper method. It's public because it will also be used in NotificationService. - Had to mark a few database methods as suspending since they use the collection to check if a deck exists. This in turn meant wrapping some of the tests in the test file with `runTest` and marking the database access wrapper in ScheduleReminders with `suspend`. Also had to explicitly create decks using `addDeck` in the test file. Moved the dummy review reminder declarations to setUp to accommodate this. - Added new tests for deck deletion functionality.
1 parent da73efe commit bffdf9f

File tree

4 files changed

+389
-249
lines changed

4 files changed

+389
-249
lines changed

AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabase.kt

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import android.content.SharedPreferences
2121
import androidx.annotation.VisibleForTesting
2222
import androidx.core.content.edit
2323
import com.ichi2.anki.AnkiDroidApp
24+
import com.ichi2.anki.CollectionManager.withCol
2425
import com.ichi2.anki.libanki.DeckId
2526
import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase.StoredReviewRemindersMap
2627
import kotlinx.serialization.InternalSerializationApi
@@ -239,11 +240,19 @@ object ReviewRemindersDatabase {
239240
}
240241

241242
/**
242-
* Get the [ReviewReminder]s for a specific deck.
243+
* Get the [ReviewReminder]s for a specific deck. Deletes the review reminders for this deck if the deck does not exist.
243244
* @throws SerializationException If the reminders map has not been stored in SharedPreferences as a valid JSON string.
244245
* @throws IllegalArgumentException If the decoded reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>.
245246
*/
246-
fun getRemindersForDeck(did: DeckId): HashMap<ReviewReminderId, ReviewReminder> = getRemindersForKey(DECK_SPECIFIC_KEY + did)
247+
suspend fun getRemindersForDeck(did: DeckId): HashMap<ReviewReminderId, ReviewReminder> {
248+
val doesDeckExist = withCol { decks.have(did) }
249+
return if (doesDeckExist) {
250+
getRemindersForKey(DECK_SPECIFIC_KEY + did)
251+
} else {
252+
deleteAllRemindersForDeck(did)
253+
hashMapOf()
254+
}
255+
}
247256

248257
/**
249258
* Get the app-wide [ReviewReminder]s.
@@ -254,15 +263,43 @@ object ReviewRemindersDatabase {
254263

255264
/**
256265
* Get all [ReviewReminder]s that are associated with a specific deck, all in a single flattened map.
266+
* For each deck, deletes the deck's review reminders if the deck does not exist.
257267
* @throws SerializationException If the reminders maps have not been stored in SharedPreferences as valid JSON strings.
258268
* @throws IllegalArgumentException If the decoded reminders maps are not instances of HashMap<[ReviewReminderId], [ReviewReminder]>.
259269
*/
260-
fun getAllDeckSpecificReminders(): HashMap<ReviewReminderId, ReviewReminder> =
261-
remindersSharedPrefs
262-
.all
263-
.filterKeys { it.startsWith(DECK_SPECIFIC_KEY) }
264-
.flatMap { (key, value) -> decodeJson(value.toString(), deckKeyForMigrationPurposes = key).entries }
265-
.associateTo(hashMapOf()) { it.toPair() }
270+
suspend fun getAllDeckSpecificReminders(): HashMap<ReviewReminderId, ReviewReminder> {
271+
// Get all deck-specific reminders
272+
val deckSpecificRemindersMap =
273+
remindersSharedPrefs
274+
.all
275+
.filterKeys { it.startsWith(DECK_SPECIFIC_KEY) }
276+
.toMutableMap()
277+
// Delete deck-specific reminders for decks that do not exist
278+
// Opens a SharedPreferences transaction and the collection only once
279+
remindersSharedPrefs.edit {
280+
withCol {
281+
deckSpecificRemindersMap.entries.removeIf { (key, _) ->
282+
val did = key.removePrefix(DECK_SPECIFIC_KEY).toLong()
283+
val doesDeckExist = decks.have(did)
284+
if (doesDeckExist) {
285+
false // Keep this group of review reminders
286+
} else {
287+
Timber.d("Deleting review reminders for deck $did")
288+
remove(key) // Remove from SharedPreferences
289+
true // Remove from deckSpecificRemindersMap
290+
}
291+
}
292+
}
293+
}
294+
// Decode the remaining deck-specific reminders and return
295+
return deckSpecificRemindersMap
296+
.flatMap { (key, value) ->
297+
decodeJson(
298+
value.toString(),
299+
deckKeyForMigrationPurposes = key,
300+
).entries
301+
}.associateTo(hashMapOf()) { it.toPair() }
302+
}
266303

267304
/**
268305
* Edit the [ReviewReminder]s for a specific key.
@@ -283,17 +320,24 @@ object ReviewRemindersDatabase {
283320
}
284321

285322
/**
286-
* Edit the [ReviewReminder]s for a specific deck.
323+
* Edit the [ReviewReminder]s for a specific deck. Deletes the review reminders for this deck if the deck does not exist.
287324
* This assumes the resulting map contains only reminders of scope [ReviewReminderScope.DeckSpecific].
288325
* @param did
289326
* @param reminderEditor A lambda that takes the current map and returns the updated map.
290327
* @throws SerializationException If the current reminders map has not been stored in SharedPreferences as a valid JSON string.
291328
* @throws IllegalArgumentException If the decoded current reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>.
292329
*/
293-
fun editRemindersForDeck(
330+
suspend fun editRemindersForDeck(
294331
did: DeckId,
295332
reminderEditor: (HashMap<ReviewReminderId, ReviewReminder>) -> Map<ReviewReminderId, ReviewReminder>,
296-
) = editRemindersForKey(DECK_SPECIFIC_KEY + did, reminderEditor)
333+
) {
334+
val doesDeckExist = withCol { decks.have(did) }
335+
if (doesDeckExist) {
336+
editRemindersForKey(DECK_SPECIFIC_KEY + did, reminderEditor)
337+
} else {
338+
deleteAllRemindersForDeck(did)
339+
}
340+
}
297341

298342
/**
299343
* Edit the app-wide [ReviewReminder]s.
@@ -304,6 +348,23 @@ object ReviewRemindersDatabase {
304348
*/
305349
fun editAllAppWideReminders(reminderEditor: (HashMap<ReviewReminderId, ReviewReminder>) -> Map<ReviewReminderId, ReviewReminder>) =
306350
editRemindersForKey(APP_WIDE_KEY, reminderEditor)
351+
352+
/**
353+
* Delete all [ReviewReminder]s for a specific deck.
354+
* Fully removes the stored JSON string representing the stored review reminders from SharedPreferences.
355+
* Does nothing if no review reminders for this deck have been stored.
356+
*
357+
* Public so that if a notification is being fired for a deck that has been deleted, the notification can be
358+
* cancelled and the review reminders deleted. In general, deleting review reminders when a deck has been deleted
359+
* is handled lazily: i.e., we do not immediately delete reminders for a deck when it is deleted but rather
360+
* wait until the reminders are requested for display or for notification to check if a deletion should be performed.
361+
*/
362+
fun deleteAllRemindersForDeck(did: DeckId) {
363+
Timber.d("Deleting review reminders for deck $did")
364+
remindersSharedPrefs.edit {
365+
remove(DECK_SPECIFIC_KEY + did)
366+
}
367+
}
307368
}
308369

309370
/**

AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ScheduleReminders.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class ScheduleReminders :
275275
* Shows an error dialog if [SerializationException]s or [IllegalArgumentException]s are thrown.
276276
* Shows a progress dialog if database access takes a long time.
277277
*/
278-
private suspend fun <T> Fragment.catchDatabaseExceptions(block: () -> T): T? =
278+
private suspend fun <T> Fragment.catchDatabaseExceptions(block: suspend () -> T): T? =
279279
try {
280280
Timber.d("Attempting ReviewRemindersDatabase operation")
281281
withProgress { block() }

0 commit comments

Comments
 (0)