feat(reminders): threshold filters

[ericli3690]

Purpose / Description

Add a group of new advanced review reminder options: count new cards, count cards in learning, and count cards in review. When the review reminder is about to send a notification and checks to see if the amount of cards in the deck is greater than the card trigger threshold, it examines these options to check if it should count and consider new cards, cards in learning, and cards in review.

Adds three new checkboxes to the AddEditReminderDialog to toggle these booleans on or off. Edits some logic in NotificationService to add up cards only from selected card type when determining whether the card trigger threshold is met.

Adds three new boolean fields to store the states of these settings to ReviewReminder. Adds unit tests.

UI

Fixes

• GSoC 2025: Review Reminders

Approach

One of the two planned review reminder advanced options! Just needs some extra checks when handling the Count objects.

How Has This Been Tested?

• Unit tests pass.
• Builds and runs on a physical Samsung S23, API 34.
• Based on my own personal testing, it seems the feature works! I created a few review reminders, reviewed a few cards, etc.

Checklist

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[ericli3690]

• Rebased.

[ericli3690]

• Rebased.

[ericli3690]

• Rebased.

[ericli3690]

Forgot to stage some code. Fixed.

[ericli3690]

Rebased, fully unblocked, and ready for review!

[ericli3690]

I've decided to block this behind #19157, as some of this PR's code will need to be updated once that guy's merged.

[ericli3690]

• Rebased! Resolved merge conflicts, cleaned up the test file.
• Switched AddEditReminderDialog over to viewbinding (related to Add View Binding #11116)

Unblocked and ready to review!

[ericli3690]

Thanks for the test patch! I've decided to go the full mile and add a few more helper functions on top, hopefully the file is more readable now.

As for the UI coloring, I love the idea. It was a bit tricky to implement in a way that would work with the translation system, but this is what I came up with:

// Manually split the string resource so that we can color just the review state part
val (reviewState, colorAttr) = REVIEW_STATE_STRINGS_AND_COLORS.entries.elementAt(i)
val splitString = getString(R.string.review_reminders_include_review_state_for_threshold_do_not_translate).split("%s")
item.textView.text =
    buildSpannedString {
        append(splitString[0])
        color(MaterialColors.getColor(requireContext(), colorAttr, 0)) {
            append(getString(reviewState))
        }
        append(splitString[1])
    }

Hopefully that works. I opted for no underline because it made it look clickable and the English text I've used is taken from upstream's wording; attached below is an image of the new UI. Should I move the new strings to a different file? Thoughts on the styling? Is it more readable now?

[david-allison]

I'd strongly prefer lowercase: new, learn, review

Check with the other mentors on wording, but I'd prefer shorter text:

• Include **review** cards is obvious [to me].

[david-allison]

Excellent, thanks so much!!

[david-allison]

This is awesome.

I'd consider defining partition (pseudocode below, needs docs, maybe tests).

• Minor bug that if %s is missing, things blow up - probably fine to ignore
• Minor bug that the string is truncated if there are too many %s - fixed below
• Ensure that a change: %s to %1$s would fail a unit test

    fun String.partition(delimiter: String): Pair<String, String> = this.split(delimiter, limit = 2)
    val (before, after) = "a %s b".partition("%s")

[ericli3690]

Ah, elegant! Implemented, I've added unit tests too.

[david-allison]

give naming a second consideration, maybe even define withNote to mean 1, and withNotes to mean more than 1.

Subject: [PATCH] 
---
Index: AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt
--- a/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt	(revision f4af44f18bcf1ad2a3348d3ec51a9e8d3b314072)
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt	(date 1768720363077)
@@ -87,7 +87,7 @@
     @Test
     fun `onReceive with less cards than card threshold should not fire notification but schedule next`() =
         runTest {
-            val did1 = addDeck("Deck", setAsSelected = true).withNote(count = 2)
+            val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2)
             val reviewReminderDeckSpecific = createTestReminder(deckId = did1, thresholdInt = 3)
             val reviewReminderAppWide = createTestReminder(thresholdInt = 3)
             ReviewRemindersDatabase.storeReminders(reviewReminderDeckSpecific, reviewReminderAppWide)
@@ -103,7 +103,7 @@
     @Test
     fun `onReceive with happy path for single deck should fire notification and schedule next`() =
         runTest {
-            val did1 = addDeck("Deck", setAsSelected = true).withNote(count = 2)
+            val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2)
             val reviewReminder = createAndSaveDummyDeckSpecificReminder(did1)
 
             triggerDummyReminderNotification(reviewReminder)
@@ -115,8 +115,8 @@
     @Test
     fun `onReceive with happy path for global reminder should fire notification and schedule next`() =
         runTest {
-            addDeck("Deck1").withNote(count = 2)
-            addDeck("Deck2").withNote(count = 2)
+            addDeck("Deck1").withNotes(count = 2)
+            addDeck("Deck2").withNotes(count = 2)
             val reviewReminder = createTestReminder(thresholdInt = 4)
 
             triggerDummyReminderNotification(reviewReminder)
@@ -210,7 +210,7 @@
     @Test
     fun `onReceive with blocked collection should not fire notification but schedule next`() =
         runTest {
-            val did1 = addDeck("Deck", setAsSelected = true).withNote(count = 2)
+            val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2)
             val reviewReminderDeckSpecific = createAndSaveDummyDeckSpecificReminder(did1)
             val reviewReminderAppWide = createAndSaveDummyAppWideReminder()
 
@@ -226,7 +226,7 @@
     @Test
     fun `onReceive with snoozed notification should fire notification but not schedule next`() =
         runTest {
-            val did1 = addDeck("Deck", setAsSelected = true).withNote(count = 2)
+            val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2)
             val reviewReminderDeckSpecific = createAndSaveDummyDeckSpecificReminder(did1)
             val reviewReminderAppWide = createAndSaveDummyAppWideReminder()
 
@@ -243,7 +243,7 @@
     @Test
     fun `onReceive with rev cards not counted and only rev cards present should not fire notification`() =
         runTest {
-            val did1 = addDeck("Deck", setAsSelected = true).withNote(count = 2, queueType = QueueType.Rev)
+            val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2, queueType = QueueType.Rev)
             val reviewReminder = createTestReminder(deckId = did1, countRev = false)
             ReviewRemindersDatabase.storeReminders(reviewReminder)
 
@@ -255,7 +255,7 @@
     @Test
     fun `onReceive with new cards not counted and only new cards present should not fire notification`() =
         runTest {
-            val did1 = addDeck("Deck").withNote(count = 2)
+            val did1 = addDeck("Deck").withNotes(count = 2)
             val reviewReminder = createTestReminder(deckId = did1, countNew = false)
             ReviewRemindersDatabase.storeReminders(reviewReminder)
 
@@ -267,7 +267,7 @@
     @Test
     fun `onReceive with lrn cards not counted and only lrn cards present should not fire notification`() =
         runTest {
-            val did1 = addDeck("Deck").withNote(count = 2, queueType = QueueType.Lrn)
+            val did1 = addDeck("Deck").withNotes(count = 2, queueType = QueueType.Lrn)
             val reviewReminder = createTestReminder(deckId = did1, countLrn = false)
             ReviewRemindersDatabase.storeReminders(reviewReminder)
 
@@ -319,8 +319,8 @@
     @Test
     fun `snooze actions of different notifications and different intervals should be different`() =
         runTest {
-            val did1 = addDeck("Deck1").withNote(count = 2)
-            val did2 = addDeck("Deck2").withNote(count = 2)
+            val did1 = addDeck("Deck1").withNotes(count = 2)
+            val did2 = addDeck("Deck2").withNotes(count = 2)
             val reviewReminderOne = createTestReminder(deckId = did1, thresholdInt = 1)
             val reviewReminderTwo = createTestReminder(deckId = did2, thresholdInt = 1)
             ReviewRemindersDatabase.storeReminders(reviewReminderOne, reviewReminderTwo)
Index: libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt b/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt
--- a/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt	(revision f4af44f18bcf1ad2a3348d3ec51a9e8d3b314072)
+++ b/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt	(date 1768720430579)
@@ -226,10 +226,18 @@
     /**
      * Convenience method for chaining [addDeck] and [addNoteToDeck].
      *
-     * Usage: `val deckId = addDeck("My Deck").withNote(count = 5, queueType = QueueType.New)`
+     * Usage: `val deckId = addDeck("My Deck").withNote(queueType = QueueType.New)`
      */
-    fun DeckId.withNote(
-        count: Int = 1,
+    fun DeckId.withNote(queueType: QueueType = QueueType.New): DeckId
+        = withNotes(count = 1, queueType = queueType)
+
+    /**
+     * Convenience method for chaining [addDeck] and [addNoteToDeck].
+     *
+     * Usage: `val deckId = addDeck("My Deck").withNotes(count = 5, queueType = QueueType.New)`
+     */
+    fun DeckId.withNotes(
+        count: Int,
         queueType: QueueType = QueueType.New,
     ): DeckId =
         this.apply {

[ericli3690]

I've decided to keep the existing text ("Include [type] cards for card threshold") for now and will create an issue later to address possibly changing it to be more precise. I think that we should probably err on the side of verbosity to ensure users and translators understand what the string is supposed to mean. I've implemented making the text lowercase, too.

Due to #20163, we're putting this PR on hold until migrations on the main branch are stable. I'll also change this code to have the proper migration.