diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt index c1ca2c9f55c4..f40430d61303 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt @@ -31,6 +31,8 @@ import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.core.content.edit import anki.scheduler.CustomStudyDefaultsResponse +import anki.scheduler.CustomStudyRequestKt +import anki.scheduler.customStudyRequest import com.ichi2.anki.CollectionManager.TR import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.CrashReportService @@ -41,7 +43,6 @@ import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.ST import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_FORGOT import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_NEW import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_PREVIEW -import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_RANDOM import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_REV import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_TAGS import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyDefaults.Companion.toDomainModel @@ -53,6 +54,7 @@ import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.showThemedToast import com.ichi2.anki.ui.internationalization.toSentenceCase import com.ichi2.anki.utils.ext.dismissAllDialogFragments +import com.ichi2.anki.utils.ext.sharedPrefs import com.ichi2.anki.utils.ext.showDialogFragment import com.ichi2.anki.withProgress import com.ichi2.annotations.NeedsTest @@ -61,6 +63,7 @@ import com.ichi2.libanki.Consts import com.ichi2.libanki.Consts.DynPriority import com.ichi2.libanki.Deck import com.ichi2.libanki.DeckId +import com.ichi2.libanki.undoableOp import com.ichi2.utils.BundleUtils.getNullableInt import com.ichi2.utils.KotlinCleanup import com.ichi2.utils.cancelable @@ -70,10 +73,8 @@ import com.ichi2.utils.negativeButton import com.ichi2.utils.positiveButton import com.ichi2.utils.title import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException -import org.json.JSONArray import org.json.JSONObject import timber.log.Timber -import java.util.Locale /** * Implements custom studying either by: @@ -196,7 +197,6 @@ class CustomStudyDialog( STUDY_REV, STUDY_FORGOT, STUDY_AHEAD, - STUDY_RANDOM, STUDY_PREVIEW, -> { // User asked for a standard custom study option @@ -250,71 +250,7 @@ class CustomStudyDialog( // This should never happen because we disable positive button for non-parsable inputs return@positiveButton } - when (contextMenuOption) { - STUDY_NEW -> { - requireActivity().sharedPrefs().edit { putInt("extendNew", n) } - val deck = collection.decks.get(dialogDeckId)!! - deck.put("extendNew", n) - collection.decks.save(deck) - collection.sched.extendLimits(n, 0) - onLimitsExtended() - } - STUDY_REV -> { - requireActivity().sharedPrefs().edit { putInt("extendRev", n) } - val deck = collection.decks.get(dialogDeckId)!! - deck.put("extendRev", n) - collection.decks.save(deck) - collection.sched.extendLimits(0, n) - onLimitsExtended() - } - STUDY_FORGOT -> { - val ar = JSONArray() - ar.put(0, 1) - createCustomStudySession( - ar, - arrayOf( - String.format( - Locale.US, - "rated:%d:1", - n, - ), - Consts.DYN_MAX_SIZE, - Consts.DYN_RANDOM, - ), - false, - ) - } - STUDY_AHEAD -> { - createCustomStudySession( - JSONArray(), - arrayOf( - String.format( - Locale.US, - "prop:due<=%d", - n, - ), - Consts.DYN_MAX_SIZE, - Consts.DYN_DUE, - ), - true, - ) - } - STUDY_RANDOM -> { - createCustomStudySession(JSONArray(), arrayOf("", n, Consts.DYN_RANDOM), true) - } - STUDY_PREVIEW -> { - createCustomStudySession( - JSONArray(), - arrayOf( - "is:new added:$n", - Consts.DYN_MAX_SIZE, - Consts.DYN_OLDEST, - ), - false, - ) - } - STUDY_TAGS -> TODO("This branch has not been covered before") - } + requireActivity().launchCatchingTask { customStudy(contextMenuOption, n) } }.negativeButton(R.string.dialog_cancel) { requireActivity().dismissAllDialogFragments() }.create() // Added .create() because we wanted to access alertDialog positive button enable state @@ -351,6 +287,59 @@ class CustomStudyDialog( return dialog } + private suspend fun customStudy( + contextMenuOption: ContextMenuOption, + userEntry: Int, + ) { + Timber.i("Custom study: $contextMenuOption; input = $userEntry") + + suspend fun customStudy(block: CustomStudyRequestKt.Dsl.() -> Unit) { + undoableOp { + collection.sched.customStudy( + customStudyRequest { + deckId = dialogDeckId + block(this) + }, + ) + } + } + + suspend fun extendLimits(block: CustomStudyRequestKt.Dsl.() -> Unit) { + try { + customStudy { block(this) } + customStudyListener?.onExtendStudyLimits() + } finally { + requireActivity().dismissAllDialogFragments() + } + } + + suspend fun createCustomStudy(block: CustomStudyRequestKt.Dsl.() -> Unit) { + try { + customStudy { block(this) } + customStudyListener?.onCreateCustomStudySession() + } finally { + requireActivity().dismissAllDialogFragments() + } + } + + // save the default values (not in upstream) + when (contextMenuOption) { + STUDY_FORGOT -> sharedPrefs().edit { putInt("forgottenDays", userEntry) } + STUDY_AHEAD -> sharedPrefs().edit { putInt("aheadDays", userEntry) } + STUDY_PREVIEW -> sharedPrefs().edit { putInt("previewDays", userEntry) } + else -> {} + } + + when (contextMenuOption) { + STUDY_NEW -> extendLimits { newLimitDelta = userEntry } + STUDY_REV -> extendLimits { reviewLimitDelta = userEntry } + STUDY_FORGOT -> createCustomStudy { forgotDays = userEntry } + STUDY_AHEAD -> createCustomStudy { reviewAheadDays = userEntry } + STUDY_PREVIEW -> createCustomStudy { previewDays = userEntry } + STUDY_TAGS -> TODO("This branch has not been covered before") + } + } + /** * Gathers the final selection of tags and type of cards, * Generates the search screen for the custom study deck. @@ -369,14 +358,12 @@ class CustomStudyDialog( } sb.append("(").append(arr.joinToString(" or ")).append(")") } - createCustomStudySession( - JSONArray(), + createTagsCustomStudySession( arrayOf( sb.toString(), Consts.DYN_MAX_SIZE, Consts.DYN_RANDOM, ), - true, ) } @@ -386,7 +373,7 @@ class CustomStudyDialog( */ private fun getListIds(): List { // Standard context menu - return mutableListOf(STUDY_FORGOT, STUDY_AHEAD, STUDY_RANDOM, STUDY_PREVIEW, STUDY_TAGS).apply { + return mutableListOf(STUDY_FORGOT, STUDY_AHEAD, STUDY_PREVIEW, STUDY_TAGS).apply { if (defaults.extendReview.isUsable) { this.add(0, STUDY_REV) } @@ -405,7 +392,6 @@ class CustomStudyDialog( STUDY_REV -> defaults.labelForReviewQueueAvailable() STUDY_FORGOT, STUDY_AHEAD, - STUDY_RANDOM, STUDY_PREVIEW, STUDY_TAGS, null, @@ -420,7 +406,6 @@ class CustomStudyDialog( STUDY_REV -> res.getString(R.string.custom_study_rev_extend) STUDY_FORGOT -> res.getString(R.string.custom_study_forgotten) STUDY_AHEAD -> res.getString(R.string.custom_study_ahead) - STUDY_RANDOM -> res.getString(R.string.custom_study_random) STUDY_PREVIEW -> res.getString(R.string.custom_study_preview) STUDY_TAGS, null, @@ -435,7 +420,6 @@ class CustomStudyDialog( STUDY_REV -> defaults.extendReview.initialValue.toString() STUDY_FORGOT -> prefs.getInt("forgottenDays", 1).toString() STUDY_AHEAD -> prefs.getInt("aheadDays", 1).toString() - STUDY_RANDOM -> prefs.getInt("randomCards", 100).toString() STUDY_PREVIEW -> prefs.getInt("previewDays", 1).toString() STUDY_TAGS, null, @@ -445,15 +429,9 @@ class CustomStudyDialog( /** * Create a custom study session - * @param delays delay options for scheduling algorithm * @param terms search terms - * @param resched whether to reschedule the cards based on the answers given (or ignore them if false) */ - private fun createCustomStudySession( - delays: JSONArray, - terms: Array, - resched: Boolean, - ) { + private fun createTagsCustomStudySession(terms: Array) { val dyn: Deck val decks = collection.decks @@ -494,17 +472,13 @@ class CustomStudyDialog( return } // and then set various options - if (delays.length() > 0) { - dyn.put("delays", delays) - } else { - dyn.put("delays", JSONObject.NULL) - } + dyn.put("delays", JSONObject.NULL) val ar = dyn.getJSONArray("terms") ar.getJSONArray(0).put(0, """deck:"$deckToStudyName" terms[0]""") ar.getJSONArray(0).put(1, terms[1]) @DynPriority val priority = terms[2] as Int ar.getJSONArray(0).put(2, priority) - dyn.put("resched", resched) + dyn.put("resched", true) // Rebuild the filtered deck Timber.i("Rebuilding Custom Study Deck") // PERF: Should be in background @@ -523,11 +497,6 @@ class CustomStudyDialog( } } - private fun onLimitsExtended() { - customStudyListener?.onExtendStudyLimits() - requireActivity().dismissAllDialogFragments() - } - /** * Possible context menu options that could be shown in the custom study dialog. */ @@ -547,8 +516,6 @@ class CustomStudyDialog( /** Review ahead */ STUDY_AHEAD({ TR.customStudyReviewAhead() }), - STUDY_RANDOM({ getString(R.string.custom_study_random_selection) }), - /** Preview new cards */ STUDY_PREVIEW({ TR.customStudyPreviewNewCards() }), diff --git a/AnkiDroid/src/main/res/values/02-strings.xml b/AnkiDroid/src/main/res/values/02-strings.xml index 0c74ab961d94..fba8a26862f4 100644 --- a/AnkiDroid/src/main/res/values/02-strings.xml +++ b/AnkiDroid/src/main/res/values/02-strings.xml @@ -62,7 +62,6 @@ Flags - Study a random selection of cards Limit to particular tags +%d buried diff --git a/AnkiDroid/src/main/res/values/03-dialogs.xml b/AnkiDroid/src/main/res/values/03-dialogs.xml index 9f8695916722..31645b2d95db 100644 --- a/AnkiDroid/src/main/res/values/03-dialogs.xml +++ b/AnkiDroid/src/main/res/values/03-dialogs.xml @@ -56,7 +56,6 @@ Increase/decrease (“-3”) today’s review limit by Review cards forgotten in last x days: Review ahead by x days: - Select x cards randomly from the deck: Preview new cards added in the last x days: Could not rebuild custom study deck diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt index 6da92ef434cb..05d621518ad8 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt @@ -17,7 +17,6 @@ package com.ichi2.anki.dialogs import android.os.Bundle import androidx.appcompat.app.AlertDialog -import androidx.lifecycle.Lifecycle import androidx.test.espresso.Espresso.onView import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import androidx.test.espresso.assertion.ViewAssertions.matches @@ -30,6 +29,7 @@ import anki.scheduler.customStudyDefaultsResponse import com.ichi2.anki.CollectionManager.TR import com.ichi2.anki.RobolectricTest import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog +import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyListener import com.ichi2.anki.dialogs.customstudy.CustomStudyDialogFactory import com.ichi2.anki.dialogs.utils.performPositiveClick @@ -41,8 +41,7 @@ import com.ichi2.testutils.ParametersUtils import com.ichi2.testutils.isJsonEqual import io.mockk.every import io.mockk.mockk -import org.hamcrest.CoreMatchers.notNullValue -import org.hamcrest.MatcherAssert +import org.hamcrest.MatcherAssert.assertThat import org.intellij.lang.annotations.Language import org.json.JSONObject import org.junit.After @@ -69,50 +68,55 @@ class CustomStudyDialogTest : RobolectricTest() { } @Test - fun learnAheadCardsRegressionTest() { - // #6289 - Regression Test - val args = - CustomStudyDialog(mock(), ParametersUtils.whatever()) - .withArguments( - 1, - contextMenuAttribute = CustomStudyDialog.ContextMenuOption.STUDY_AHEAD, - ).arguments - val factory = CustomStudyDialogFactory({ this.col }, mockListener) - AnkiFragmentScenario.launch(CustomStudyDialog::class.java, args, factory).use { scenario -> - scenario.moveToState(Lifecycle.State.RESUMED) - scenario.onFragment { f: CustomStudyDialog -> - val dialog = assertNotNull(f.dialog as AlertDialog?) + fun `new custom study decks have expected structure - issue 6289`() = + runTest { + val studyType = ContextMenuOption.STUDY_PREVIEW + // we need a non-empty deck to custom study + addNoteUsingBasicModel() + + withCustomStudyFragment( + args = argumentsDisplayingSubscreen(studyType), + ) { dialogFragment: CustomStudyDialog -> + val dialog = assertNotNull(dialogFragment.dialog as? AlertDialog?, "dialog") dialog.performPositiveClick() } + val customStudy = col.decks.current() - MatcherAssert.assertThat("Custom Study should be dynamic", customStudy.isFiltered) - MatcherAssert.assertThat("could not find deck: Custom study session", customStudy, notNullValue()) + assertThat("Custom Study should be filtered", customStudy.isFiltered) + + // remove timestamps to allow us to compare JSON customStudy.remove("id") customStudy.remove("mod") customStudy.remove("name") - @Language("JSON") + + // compare JSON + @Language("json") val expected = - """{ - "browserCollapsed":false, - "collapsed":false, - "delays":null, - "desc":"", - "dyn":1, - "lrnToday":[0,0], - "newToday":[0,0], - "previewDelay":0, - "previewAgainSecs":60,"previewHardSecs":600,"previewGoodSecs":0, - "resched":true, - "revToday":[0,0], - "separate":true, - "terms":[["deck:\"Default\" prop:due<=1",99999,6]], - "timeToday":[0,0], - "usn":-1 - } """ - MatcherAssert.assertThat(customStudy, isJsonEqual(JSONObject(expected))) + { + "browserCollapsed": false, + "collapsed": false, + "delays": null, + "desc": "", + "dyn": 1, + "lrnToday": [0, 0], + "newToday": [0, 0], + "previewDelay": 0, + "previewAgainSecs": 60, + "previewHardSecs": 600, + "previewGoodSecs": 0, + "resched": true, + "revToday": [0, 0], + "separate": true, + "terms": [ + ["deck:\"Default\" prop:due<=1", 99999, 6] + ], + "timeToday": [0, 0], + "usn": -1 + } + """.trimIndent() + assertThat(customStudy, isJsonEqual(JSONObject(expected))) } - } @Test @Config(qualifiers = "en") @@ -181,7 +185,7 @@ class CustomStudyDialogTest : RobolectricTest() { */ private fun withCustomStudyFragment( args: Bundle, - factory: CustomStudyDialogFactory, + factory: CustomStudyDialogFactory = dialogFactory(), block: (CustomStudyDialog) -> Unit, ) { AnkiFragmentScenario.launch(CustomStudyDialog::class.java, args, factory).use { scenario -> @@ -199,13 +203,25 @@ class CustomStudyDialogTest : RobolectricTest() { } } - private fun dialogFactory(col: Collection) = CustomStudyDialogFactory({ col }, mockListener) + private fun dialogFactory(col: Collection? = null) = CustomStudyDialogFactory({ col ?: this.col }, mockListener) private fun argumentsDisplayingMainScreen() = CustomStudyDialog(mock(), ParametersUtils.whatever()) .displayingMainScreen() .requireArguments() + @Suppress("SameParameterValue") + private fun argumentsDisplayingSubscreen(subscreen: ContextMenuOption) = + CustomStudyDialog(mock(), ParametersUtils.whatever()) + .displayingSubscreen(subscreen) + .requireArguments() + + private fun CustomStudyDialog.displayingSubscreen(subscreen: ContextMenuOption) = + withArguments( + did = Consts.DEFAULT_DECK_ID, + contextMenuAttribute = subscreen, + ) + private fun CustomStudyDialog.displayingMainScreen() = withArguments( did = Consts.DEFAULT_DECK_ID,