diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialog.kt index e427093c179d..f1413bd76236 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialog.kt @@ -35,7 +35,9 @@ import androidx.fragment.app.DialogFragment import androidx.fragment.app.setFragmentResult import androidx.fragment.app.setFragmentResultListener import androidx.fragment.app.viewModels +import androidx.lifecycle.LiveData import com.google.android.material.button.MaterialButton +import com.google.android.material.checkbox.MaterialCheckBox import com.google.android.material.textfield.TextInputLayout import com.google.android.material.timepicker.MaterialTimePicker import com.google.android.material.timepicker.TimeFormat @@ -134,6 +136,7 @@ class AddEditReminderDialog : DialogFragment() { setInitialDeckSelection() setUpAdvancedDropdown() setUpCardThresholdInput() + setUpCountCheckboxes() // For getting the result of the deck selection sub-dialog from ScheduleReminders // See ScheduleReminders.onDeckSelected for more information @@ -264,6 +267,54 @@ class AddEditReminderDialog : DialogFragment() { } } + /** + * Convenience data class for setting up the checkboxes for whether to count new, learning, and review cards + * when considering the card trigger threshold. + * @see setUpCountCheckboxes + */ + private data class CountViewsAndActions( + val section: LinearLayout, + val checkbox: MaterialCheckBox, + val actionOnClick: () -> Unit, + val state: LiveData, + ) + + /** + * Sets up the checkboxes for whether to count new, learning, and review cards when considering the card trigger threshold. + * @see CountViewsAndActions + */ + private fun setUpCountCheckboxes() { + val countViewsAndActionsItems = + listOf( + CountViewsAndActions( + section = contentView.findViewById(R.id.add_edit_reminder_count_new_section), + checkbox = contentView.findViewById(R.id.add_edit_reminder_count_new_checkbox), + actionOnClick = viewModel::toggleCountNew, + state = viewModel.countNew, + ), + CountViewsAndActions( + section = contentView.findViewById(R.id.add_edit_reminder_count_lrn_section), + checkbox = contentView.findViewById(R.id.add_edit_reminder_count_lrn_checkbox), + actionOnClick = viewModel::toggleCountLrn, + state = viewModel.countLrn, + ), + CountViewsAndActions( + section = contentView.findViewById(R.id.add_edit_reminder_count_rev_section), + checkbox = contentView.findViewById(R.id.add_edit_reminder_count_rev_checkbox), + actionOnClick = viewModel::toggleCountRev, + state = viewModel.countRev, + ), + ) + + countViewsAndActionsItems.forEach { item -> + item.section.setOnClickListener { item.actionOnClick() } + item.checkbox.setOnClickListener { item.actionOnClick() } + item.state.observe(this) { value -> + item.checkbox.isChecked = value + } + } + } + /** * Show the time picker dialog for selecting a time with a given hour and minute. * Does not automatically dismiss the old dialog. diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialogViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialogViewModel.kt index c562a7e53759..3036cee01587 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialogViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/AddEditReminderDialogViewModel.kt @@ -89,6 +89,33 @@ class AddEditReminderDialogViewModel( ) val cardTriggerThreshold: LiveData = _cardTriggerThreshold + private val _countNew = + MutableLiveData( + when (dialogMode) { + is AddEditReminderDialog.DialogMode.Add -> INITIAL_COUNT_NEW + is AddEditReminderDialog.DialogMode.Edit -> dialogMode.reminderToBeEdited.countNew + }, + ) + val countNew: LiveData = _countNew + + private val _countLrn = + MutableLiveData( + when (dialogMode) { + is AddEditReminderDialog.DialogMode.Add -> INITIAL_COUNT_LRN + is AddEditReminderDialog.DialogMode.Edit -> dialogMode.reminderToBeEdited.countLrn + }, + ) + val countLrn: LiveData = _countLrn + + private val _countRev = + MutableLiveData( + when (dialogMode) { + is AddEditReminderDialog.DialogMode.Add -> INITIAL_COUNT_REV + is AddEditReminderDialog.DialogMode.Edit -> dialogMode.reminderToBeEdited.countRev + }, + ) + val countRev: LiveData = _countRev + private val _advancedSettingsOpen = MutableLiveData(INITIAL_ADVANCED_SETTINGS_OPEN) val advancedSettingsOpen: LiveData = _advancedSettingsOpen @@ -107,6 +134,21 @@ class AddEditReminderDialogViewModel( _cardTriggerThreshold.value = threshold } + fun toggleCountNew() { + Timber.d("Toggled count new from %s", _countNew.value) + _countNew.value = !(_countNew.value ?: false) + } + + fun toggleCountLrn() { + Timber.d("Toggled count lrn from %s", _countLrn.value) + _countLrn.value = !(_countLrn.value ?: false) + } + + fun toggleCountRev() { + Timber.d("Toggled count rev from %s", _countRev.value) + _countRev.value = !(_countRev.value ?: false) + } + fun toggleAdvancedSettingsOpen() { Timber.d("Toggled advanced settings open from %s", _advancedSettingsOpen.value) _advancedSettingsOpen.value = !(_advancedSettingsOpen.value ?: false) @@ -136,6 +178,9 @@ class AddEditReminderDialogViewModel( is AddEditReminderDialog.DialogMode.Add -> true is AddEditReminderDialog.DialogMode.Edit -> dialogMode.reminderToBeEdited.enabled }, + countNew = countNew.value ?: INITIAL_COUNT_NEW, + countLrn = countLrn.value ?: INITIAL_COUNT_LRN, + countRev = countRev.value ?: INITIAL_COUNT_REV, ) companion object { @@ -153,5 +198,25 @@ class AddEditReminderDialogViewModel( * We start with it closed to avoid overwhelming the user. */ private const val INITIAL_ADVANCED_SETTINGS_OPEN = false + + /** + * The default setting for whether new cards are counted when checking the card trigger threshold. + * This value, and the other default settings for whether certain kinds of cards are counted + * when checking the card trigger threshold, are all set to true, as removing some card types + * from card trigger threshold consideration is a form of advanced review reminder customization. + */ + private const val INITIAL_COUNT_NEW = true + + /** + * The default setting for whether cards in learning are counted when checking the card trigger threshold. + * @see INITIAL_COUNT_NEW + */ + private const val INITIAL_COUNT_LRN = true + + /** + * The default setting for whether cards in review are counted when checking the card trigger threshold. + * @see INITIAL_COUNT_NEW + */ + private const val INITIAL_COUNT_REV = true } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt index c3b13f30c4ae..e147c7ae4c2f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt @@ -178,8 +178,6 @@ sealed class ReviewReminderScope : Parcelable { * Preferably, also add some unit tests to ensure your migration works properly on all user devices once your update is rolled out. * See ReviewRemindersDatabaseTest for examples on how to do this. * - * TODO: add remaining fields planned for GSoC 2025. - * * @param id Unique, auto-incremented ID of the review reminder. * @param time See [ReviewReminderTime]. * @param cardTriggerThreshold See [ReviewReminderCardTriggerThreshold]. @@ -187,6 +185,9 @@ sealed class ReviewReminderScope : Parcelable { * @param enabled Whether the review reminder's notifications are active or disabled. * @param profileID ID representing the profile which created this review reminder, as review reminders for * multiple profiles might be active simultaneously. + * @param countNew Whether new cards are counted when checking the [cardTriggerThreshold]. + * @param countLrn Whether learning cards are counted when checking the [cardTriggerThreshold]. + * @param countRev Whether review cards are counted when checking the [cardTriggerThreshold]. */ @Serializable @Parcelize @@ -198,6 +199,9 @@ data class ReviewReminder private constructor( val scope: ReviewReminderScope, var enabled: Boolean, val profileID: String, + val countNew: Boolean, + val countLrn: Boolean, + val countRev: Boolean, ) : Parcelable, ReviewReminderSchema { companion object { @@ -212,6 +216,9 @@ data class ReviewReminder private constructor( scope: ReviewReminderScope = ReviewReminderScope.Global, enabled: Boolean = true, profileID: String = "", + countNew: Boolean = true, + countLrn: Boolean = true, + countRev: Boolean = true, ) = ReviewReminder( id = ReviewReminderId.getAndIncrementNextFreeReminderId(), time, @@ -219,6 +226,9 @@ data class ReviewReminder private constructor( scope, enabled, profileID, + countNew, + countLrn, + countRev, ) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt index b536fa169e11..7426a38c866d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt @@ -32,6 +32,7 @@ import com.ichi2.anki.R import com.ichi2.anki.canUserAccessDeck import com.ichi2.anki.common.annotations.LegacyNotifications import com.ichi2.anki.libanki.Decks +import com.ichi2.anki.libanki.sched.Counts import com.ichi2.anki.preferences.PENDING_NOTIFICATIONS_ONLY import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.reviewreminders.ReviewReminder @@ -104,8 +105,16 @@ class NotificationService : BroadcastReceiver() { } } val dueCardsTotal = dueCardsCount.count() - if (dueCardsTotal < reviewReminder.cardTriggerThreshold.threshold) { - Timber.d("Aborting notification due to threshold: $dueCardsTotal < ${reviewReminder.cardTriggerThreshold.threshold}") + val consideredCardsCount = + Counts().apply { + if (reviewReminder.countNew) addNew(dueCardsCount.new) + if (reviewReminder.countLrn) addLrn(dueCardsCount.lrn) + if (reviewReminder.countRev) addRev(dueCardsCount.rev) + } + val consideredCardsTotal = consideredCardsCount.count() + Timber.d("Due cards count: $dueCardsCount, Considered cards count: $consideredCardsCount") + if (consideredCardsTotal < reviewReminder.cardTriggerThreshold.threshold) { + Timber.d("Aborting notification due to threshold: $consideredCardsTotal < ${reviewReminder.cardTriggerThreshold.threshold}") return } diff --git a/AnkiDroid/src/main/res/layout/add_edit_reminder_dialog.xml b/AnkiDroid/src/main/res/layout/add_edit_reminder_dialog.xml index d17dff33e870..d48af4a4c7bd 100644 --- a/AnkiDroid/src/main/res/layout/add_edit_reminder_dialog.xml +++ b/AnkiDroid/src/main/res/layout/add_edit_reminder_dialog.xml @@ -189,6 +189,69 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt index 487317aeb797..27ff9c107de2 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt @@ -24,6 +24,8 @@ import androidx.test.core.app.ApplicationProvider.getApplicationContext import androidx.test.ext.junit.runners.AndroidJUnit4 import com.ichi2.anki.CollectionManager import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.libanki.DeckId +import com.ichi2.anki.libanki.QueueType import com.ichi2.anki.reviewreminders.ReviewReminder import com.ichi2.anki.reviewreminders.ReviewReminderCardTriggerThreshold import com.ichi2.anki.reviewreminders.ReviewReminderId @@ -69,6 +71,27 @@ class NotificationServiceTest : RobolectricTest() { ReviewRemindersDatabase.remindersSharedPrefs.edit { clear() } } + private fun createAndSaveDummyReminder(did: DeckId): ReviewReminder { + val reviewReminder = + ReviewReminder.createReviewReminder( + ReviewReminderTime(9, 0), + ReviewReminderCardTriggerThreshold(1), + ReviewReminderScope.DeckSpecific(did), + ) + ReviewRemindersDatabase.editRemindersForDeck(did) { mapOf(ReviewReminderId(0) to reviewReminder) } + return reviewReminder + } + + private fun triggerDummyReminderNotification(reviewReminder: ReviewReminder) { + val intent = + NotificationService.getIntent( + context, + reviewReminder, + NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, + ) + NotificationService().onReceive(context, intent) + } + @Test fun `onReceive with less cards than card threshold should not fire notification but schedule next`() = runTest { @@ -84,13 +107,7 @@ class NotificationServiceTest : RobolectricTest() { ) ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } - val intent = - NotificationService.getIntent( - context, - reviewReminder, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, - ) - NotificationService().onReceive(context, intent) + triggerDummyReminderNotification(reviewReminder) verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } verify( @@ -105,21 +122,9 @@ class NotificationServiceTest : RobolectricTest() { addNotes(2).forEach { it.firstCard().update { did = did1 } } - val reviewReminder = - ReviewReminder.createReviewReminder( - ReviewReminderTime(9, 0), - ReviewReminderCardTriggerThreshold(1), - ReviewReminderScope.DeckSpecific(did1), - ) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } + val reviewReminder = createAndSaveDummyReminder(did1) - val intent = - NotificationService.getIntent( - context, - reviewReminder, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, - ) - NotificationService().onReceive(context, intent) + triggerDummyReminderNotification(reviewReminder) verify( exactly = 1, @@ -147,13 +152,8 @@ class NotificationServiceTest : RobolectricTest() { ReviewReminderScope.Global, ) - val intent = - NotificationService.getIntent( - context, - reviewReminder, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, - ) - NotificationService().onReceive(context, intent) + triggerDummyReminderNotification(reviewReminder) + verify( exactly = 1, ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminder.id.value, any()) } @@ -172,77 +172,225 @@ class NotificationServiceTest : RobolectricTest() { ReviewReminderScope.DeckSpecific(9999), ) + triggerDummyReminderNotification(reviewReminder) + + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + verify( + exactly = 1, + ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + } + + @Test + fun `onReceive with blocked collection should not fire notification but schedule next`() = + runTest { + val did1 = addDeck("Deck") + addNotes(2).forEach { + it.firstCard().update { did = did1 } + } + val reviewReminder = createAndSaveDummyReminder(did1) + + CollectionManager.emulatedOpenFailure = CollectionManager.CollectionOpenFailure.LOCKED + triggerDummyReminderNotification(reviewReminder) + + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + verify( + exactly = 1, + ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + } + + @Test + fun `onReceive with snoozed notification should fire notification but not schedule next`() = + runTest { + val did1 = addDeck("Deck") + addNotes(2).forEach { + it.firstCard().update { did = did1 } + } + val reviewReminder = createAndSaveDummyReminder(did1) + val intent = NotificationService.getIntent( context, reviewReminder, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, + NotificationService.NotificationServiceAction.SnoozeNotification, ) NotificationService().onReceive(context, intent) - verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } verify( exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminder.id.value, any()) } + verify(exactly = 0) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } } @Test - fun `onReceive with blocked collection should not fire notification but schedule next`() = + fun `onReceive with rev cards not counted and only rev cards present should not fire notification`() = runTest { val did1 = addDeck("Deck") addNotes(2).forEach { - it.firstCard().update { did = did1 } + it.firstCard().update { + did = did1 + queue = QueueType.Rev + } } val reviewReminder = ReviewReminder.createReviewReminder( ReviewReminderTime(9, 0), ReviewReminderCardTriggerThreshold(1), ReviewReminderScope.DeckSpecific(did1), + countRev = false, ) ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } - CollectionManager.emulatedOpenFailure = CollectionManager.CollectionOpenFailure.LOCKED - val intent = - NotificationService.getIntent( - context, - reviewReminder, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, + triggerDummyReminderNotification(reviewReminder) + + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + } + + @Test + fun `onReceive with new cards not counted and only new cards present should not fire notification`() = + runTest { + val did1 = addDeck("Deck") + addNotes(2).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.New + } + } + val reviewReminder = + ReviewReminder.createReviewReminder( + ReviewReminderTime(9, 0), + ReviewReminderCardTriggerThreshold(1), + ReviewReminderScope.DeckSpecific(did1), + countNew = false, ) - NotificationService().onReceive(context, intent) + ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } + + triggerDummyReminderNotification(reviewReminder) verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } } @Test - fun `onReceive with snoozed notification should fire notification but not schedule next`() = + fun `onReceive with lrn cards not counted and only lrn cards present should not fire notification`() = runTest { val did1 = addDeck("Deck") addNotes(2).forEach { - it.firstCard().update { did = did1 } + it.firstCard().update { + did = did1 + queue = QueueType.Lrn + } } val reviewReminder = ReviewReminder.createReviewReminder( ReviewReminderTime(9, 0), ReviewReminderCardTriggerThreshold(1), ReviewReminderScope.DeckSpecific(did1), + countLrn = false, ) ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } - val intent = - NotificationService.getIntent( - context, - reviewReminder, - NotificationService.NotificationServiceAction.SnoozeNotification, + triggerDummyReminderNotification(reviewReminder) + + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + } + + @Test + fun `onReceive with all cards not counted and many cards present should not fire notification`() = + runTest { + val did1 = addDeck("Deck") + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.New + } + } + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.Lrn + } + } + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.Rev + } + } + val reviewReminder = + ReviewReminder.createReviewReminder( + ReviewReminderTime(9, 0), + ReviewReminderCardTriggerThreshold(1), + ReviewReminderScope.DeckSpecific(did1), + countNew = false, + countLrn = false, + countRev = false, ) - NotificationService().onReceive(context, intent) + ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } + + triggerDummyReminderNotification(reviewReminder) + + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + } + + @Test + fun `onReceive with new cards not counted but other kinds present should fire notification`() = + runTest { + val did1 = addDeck("Deck") + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.New + } + } + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.Lrn + } + } + val reviewReminder = + ReviewReminder.createReviewReminder( + ReviewReminderTime(9, 0), + ReviewReminderCardTriggerThreshold(1), + ReviewReminderScope.DeckSpecific(did1), + countNew = false, + ) + ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } + + triggerDummyReminderNotification(reviewReminder) verify( exactly = 1, ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminder.id.value, any()) } - verify(exactly = 0) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + } + + @Test + fun `onReceive with new cards not counted and not enough non new cards to trigger threshold should not fire notification`() = + runTest { + val did1 = addDeck("Deck") + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.Lrn + } + } + addNotes(1).forEach { + it.firstCard().update { + did = did1 + queue = QueueType.New + } + } + val reviewReminder = + ReviewReminder.createReviewReminder( + ReviewReminderTime(9, 0), + ReviewReminderCardTriggerThreshold(2), + ReviewReminderScope.DeckSpecific(did1), + countNew = false, + ) + ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminder) } + + triggerDummyReminderNotification(reviewReminder) + + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } } @Test