diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt index 7c7768418bb0..953e86a3f450 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt @@ -683,10 +683,11 @@ open class Reviewer : } } - private fun showDueDateDialog() { - val dialog = SetDueDateDialog.newInstance(listOf(currentCardId!!)) - showDialogFragment(dialog) - } + private fun showDueDateDialog() = + launchCatchingTask { + val dialog = SetDueDateDialog.newInstance(listOf(currentCardId!!)) + showDialogFragment(dialog) + } private fun showResetCardDialog() { Timber.i("showResetCardDialog() Reset progress button pressed") diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateDialog.kt index fcd997545d63..6b336b8bd4de 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateDialog.kt @@ -28,6 +28,7 @@ import android.widget.TextView import androidx.annotation.CheckResult import androidx.core.content.ContextCompat import androidx.core.os.bundleOf +import androidx.core.view.isVisible import androidx.core.widget.doOnTextChanged import androidx.fragment.app.DialogFragment import androidx.fragment.app.Fragment @@ -48,6 +49,7 @@ import com.ichi2.anki.R import com.ichi2.anki.launchCatchingTask import com.ichi2.anki.requireAnkiActivity import com.ichi2.anki.scheduling.SetDueDateViewModel.Tab +import com.ichi2.anki.servicelayer.getFSRSStatus import com.ichi2.anki.showThemedToast import com.ichi2.anki.snackbar.showSnackbar import com.ichi2.anki.ui.internationalization.toSentenceCase @@ -59,6 +61,7 @@ import com.ichi2.utils.create import com.ichi2.utils.negativeButton import com.ichi2.utils.neutralButton import com.ichi2.utils.positiveButton +import com.ichi2.utils.requireBoolean import com.ichi2.utils.title import kotlinx.coroutines.launch import timber.log.Timber @@ -84,10 +87,15 @@ class SetDueDateDialog : DialogFragment() { // used to determine if a rotation has taken place private var initialRotation: Int = 0 + val cardIds: LongArray + get() = requireNotNull(requireArguments().getLongArray(ARG_CARD_IDS)) { ARG_CARD_IDS } + + val fsrsEnabled: Boolean + get() = requireArguments().requireBoolean(ARG_FSRS) + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - val cardIds = requireNotNull(requireArguments().getLongArray(ARG_CARD_IDS)) { ARG_CARD_IDS } - viewModel.init(cardIds) + viewModel.init(cardIds, fsrsEnabled) Timber.d("Set due date dialog: %d card(s)", cardIds.size) this.initialRotation = getScreenRotation() } @@ -151,9 +159,12 @@ class SetDueDateDialog : DialogFragment() { ) // setup 'set interval to same value' checkbox - findViewById(R.id.change_interval)!!.apply { - isChecked = viewModel.updateIntervalToMatchDueDate - setOnCheckedChangeListener { _, isChecked -> + findViewById(R.id.change_interval)!!.also { cb -> + // `.also` is used as .isVisible is an extension, so Kotlin prefers + // incorrectly setting Fragment.isVisible + cb.isVisible = viewModel.canSetUpdateIntervalToMatchDueDate + cb.isChecked = viewModel.updateIntervalToMatchDueDate + cb.setOnCheckedChangeListener { _, isChecked -> viewModel.updateIntervalToMatchDueDate = isChecked } } @@ -184,12 +195,17 @@ class SetDueDateDialog : DialogFragment() { companion object { const val ARG_CARD_IDS = "ARGS_CARD_IDS" + const val ARG_FSRS = "ARGS_FSRS" const val MAX_WIDTH_DP = 450f @CheckResult - fun newInstance(cardIds: List) = + suspend fun newInstance(cardIds: List) = SetDueDateDialog().apply { - arguments = bundleOf(ARG_CARD_IDS to cardIds.toLongArray()) + arguments = + bundleOf( + ARG_CARD_IDS to cardIds.toLongArray(), + ARG_FSRS to (getFSRSStatus() ?: false.also { Timber.w("FSRS Status error") }), + ) Timber.i("Showing 'set due date' dialog for %d cards", cardIds.size) } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateViewModel.kt index 7c64635d845a..4be52487320d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateViewModel.kt @@ -44,6 +44,25 @@ class SetDueDateViewModel : ViewModel() { /** The cards to change the due date of */ lateinit var cardIds: List + /** Whether the Free Spaced Repetition Scheduler is enabled */ + // initialized in init() + private var fsrsEnabled: Boolean = false + set(value) { + field = value + Timber.d("fsrsEnabled : %b", value) + if (value) { + Timber.d("updateIntervalToMatchDueDate forced to true: FSRS is enabled") + this.updateIntervalToMatchDueDate = true + } + } + + /** Whether the user can set [updateIntervalToMatchDueDate] */ + val canSetUpdateIntervalToMatchDueDate + // this only makes sense in SM-2, where the due date does not directly impact the next + // interval calculation. In FSRS, the current date is taken into account + // so ivl should match due date for simplicity + get() = !fsrsEnabled + /** * The number of cards which will be affected * @@ -75,15 +94,30 @@ class SetDueDateViewModel : ViewModel() { refreshIsValid() } - /** If `true`, the interval of the card is updated to match the calculated due date */ + /** + * If `true`, the interval of the card is updated to match the calculated due date + * + * This is only supported when using SM-2. In FSRS, there's no reason for ivl != due date, + * as the date when a card is seen affects the scheduling. + * + * @throws UnsupportedOperationException if unset when FSRS is enabled + */ var updateIntervalToMatchDueDate: Boolean = false + get() = if (fsrsEnabled) true else field set(value) { Timber.d("updateIntervalToMatchDueDate: %b", value) + if (fsrsEnabled && !value) { + throw UnsupportedOperationException("due date must match interval if using FSRS") + } field = value } - fun init(cardIds: LongArray) { + fun init( + cardIds: LongArray, + fsrsEnabled: Boolean, + ) { this.cardIds = cardIds.toList() + this.fsrsEnabled = fsrsEnabled } fun setNextDateRangeStart(value: Int?) { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/DebugInfoService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/DebugInfoService.kt index a838d9eb96dc..b4ddc630da67 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/DebugInfoService.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/DebugInfoService.kt @@ -53,13 +53,18 @@ object DebugInfoService { } private fun isSendingCrashReports(context: Context): Boolean = CrashReportService.isAcraEnabled(context, false) - - private suspend fun getFSRSStatus(): Boolean? = - try { - CollectionManager.withOpenColOrNull { config.get("fsrs", false) } - } catch (e: Throwable) { - // Error and Exception paths are the same, so catch Throwable - Timber.w(e) - null - } } + +/** + * Whether the Free Spaced Repetition Scheduler is enabled + * + * Note: this can return `null` if the collection is not openable + */ +suspend fun getFSRSStatus(): Boolean? = + try { + CollectionManager.withOpenColOrNull { config.get("fsrs", false) } + } catch (e: Throwable) { + // Error and Exception paths are the same, so catch Throwable + Timber.w(e) + null + } diff --git a/AnkiDroid/src/main/java/com/ichi2/utils/BundleUtils.kt b/AnkiDroid/src/main/java/com/ichi2/utils/BundleUtils.kt index 57e099db43c1..2edcfbf72886 100644 --- a/AnkiDroid/src/main/java/com/ichi2/utils/BundleUtils.kt +++ b/AnkiDroid/src/main/java/com/ichi2/utils/BundleUtils.kt @@ -63,3 +63,15 @@ object BundleUtils { getInt(key) } } + +/** + * Retrieves a [Boolean] value from a [Bundle] using a key, throws if not found + * + * @param key A string key + * @return the value associated with [key] + * @throws IllegalStateException If [key] does not exist in the bundle + */ +fun Bundle.requireBoolean(key: String): Boolean { + check(containsKey(key)) { "key: '$key' not found" } + return getBoolean(key) +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateDialogTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateDialogTest.kt index 7b0db1d50a2d..0e7d5247c12d 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateDialogTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateDialogTest.kt @@ -39,6 +39,7 @@ import org.junit.runner.RunWith @Ignore("selectTab(1) does not attach Ids") @NeedsTest("get the tests working") +@NeedsTest("set interval to same value visibility with FSRS") @RunWith(AndroidJUnit4::class) class SetDueDateDialogTest : RobolectricTest() { @Test diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateViewModelTest.kt index fc0e595ac7b7..067505e99479 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/scheduling/SetDueDateViewModelTest.kt @@ -22,10 +22,13 @@ import com.ichi2.anki.scheduling.SetDueDateViewModel.Tab import com.ichi2.libanki.CardId import com.ichi2.libanki.sched.SetDueDateDays import com.ichi2.testutils.JvmTest +import com.ichi2.testutils.common.assertThrows import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test import org.junit.runner.RunWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) class SetDueDateViewModelTest : JvmTest() { @@ -111,12 +114,58 @@ class SetDueDateViewModelTest : JvmTest() { assertThat(calculateDaysParameter(), eq("0-1!")) } + @Test + fun `if FSRS is enabled updateIntervalToMatchDueDate may not be set to false`() { + runViewModelTest(fsrsEnabled = true) { + // the value can be set to true + updateIntervalToMatchDueDate = true + + assertFalse( + canSetUpdateIntervalToMatchDueDate, + "canSetUpdateIntervalToMatchDueDate when FSRS enabled", + ) + + val ex = + assertThrows("due date mismatch & FSRS enabled") { + updateIntervalToMatchDueDate = false + } + assertThat(ex.message, equalTo("due date must match interval if using FSRS")) + } + + runViewModelTest(fsrsEnabled = false) { + assertTrue( + canSetUpdateIntervalToMatchDueDate, + "canSetUpdateIntervalToMatchDueDate and no FSRS", + ) + } + } + + @Test + fun `updateIntervalToMatchDueDate default value, given scheduler`() { + runViewModelTest(fsrsEnabled = true) { + assertThat( + "updateIntervalToMatchDueDate default if FSRS", + updateIntervalToMatchDueDate, + equalTo(true), + ) + } + + runViewModelTest(fsrsEnabled = false) { + assertThat( + "updateIntervalToMatchDueDate default if no FSRS", + updateIntervalToMatchDueDate, + equalTo(false), + ) + } + } + private fun runViewModelTest( cardIds: List = listOf(1, 2, 3), + fsrsEnabled: Boolean = false, testBody: suspend SetDueDateViewModel.() -> Unit, ) = runTest { val viewModel = SetDueDateViewModel() - viewModel.init(cardIds.toLongArray()) + viewModel.init(cardIds.toLongArray(), fsrsEnabled = fsrsEnabled) testBody(viewModel) } } diff --git a/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt b/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt index 472b2efbe974..b738c570b55b 100644 --- a/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/utils/BundleUtilsTest.kt @@ -18,6 +18,8 @@ package com.ichi2.utils import android.os.Bundle import com.ichi2.utils.BundleUtils.getNullableLong import com.ichi2.utils.BundleUtils.requireLong +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.MatcherAssert.assertThat import org.junit.Assert.assertEquals import org.junit.Test import org.mockito.Mockito.anyString @@ -87,6 +89,34 @@ class BundleUtilsTest { assertEquals(expected, value) } + @Test + fun test_RequireBoolean_NotFound_ThrowsException() { + val mockedBundle = mock(Bundle::class.java) + + whenever(mockedBundle.containsKey(anyString())).thenReturn(false) + + val exception = assertFailsWith { mockedBundle.requireBoolean(KEY) } + + assertThat(exception.message, equalTo("key: 'KEY' not found")) + verify(mockedBundle).containsKey(eq(KEY)) + } + + @Test + fun test_RequireBoolean_Found_ReturnIt() { + val expected = true + val mockedBundle = mock(Bundle::class.java) + + whenever(mockedBundle.containsKey(anyString())).thenReturn(true) + whenever(mockedBundle.getBoolean(anyString())).thenReturn(expected) + + val value = mockedBundle.requireBoolean(KEY) + + verify(mockedBundle).containsKey(eq(KEY)) + verify(mockedBundle).getBoolean(eq(KEY)) + + assertEquals(expected, value) + } + companion object { const val KEY = "KEY" } diff --git a/testlib/src/main/java/com/ichi2/testutils/common/AnkiAssert.kt b/testlib/src/main/java/com/ichi2/testutils/common/AnkiAssert.kt index 246f8dcaaeee..2edaf5ff9b3d 100644 --- a/testlib/src/main/java/com/ichi2/testutils/common/AnkiAssert.kt +++ b/testlib/src/main/java/com/ichi2/testutils/common/AnkiAssert.kt @@ -33,6 +33,4 @@ import org.junit.function.ThrowingRunnable inline fun assertThrows( message: String? = null, runnable: ThrowingRunnable, -) { - org.junit.Assert.assertThrows(message, T::class.java, runnable) -} +): T = org.junit.Assert.assertThrows(message, T::class.java, runnable)