Skip to content

Commit

Permalink
refactor(custom-study): remove direct col access
Browse files Browse the repository at this point in the history
  • Loading branch information
david-allison committed Jan 17, 2025
1 parent 9c8e6d8 commit 70d22d0
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 176 deletions.
10 changes: 3 additions & 7 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ import com.ichi2.anki.dialogs.SyncErrorDialog.Companion.newInstance
import com.ichi2.anki.dialogs.SyncErrorDialog.SyncErrorDialogListener
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyAction
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialogFactory
import com.ichi2.anki.export.ActivityExportingDelegate
import com.ichi2.anki.export.ExportDialogFragment
import com.ichi2.anki.export.ExportDialogsFactory
Expand Down Expand Up @@ -177,7 +176,6 @@ import com.ichi2.ui.AccessibleSearchView
import com.ichi2.ui.BadgeDrawableBuilder
import com.ichi2.utils.AdaptionUtil
import com.ichi2.utils.ClipboardUtil.IMPORT_MIME_TYPES
import com.ichi2.utils.FragmentFactoryUtils
import com.ichi2.utils.HandlerUtils
import com.ichi2.utils.ImportUtils
import com.ichi2.utils.ImportUtils.ImportResult
Expand Down Expand Up @@ -329,7 +327,6 @@ open class DeckPicker :

private var toolbarSearchItem: MenuItem? = null
private var toolbarSearchView: AccessibleSearchView? = null
private lateinit var customStudyDialogFactory: CustomStudyDialogFactory

override val permissionScreenLauncher = recreateActivityResultLauncher()

Expand Down Expand Up @@ -476,7 +473,6 @@ open class DeckPicker :
return
}
exportingDelegate = ActivityExportingDelegate(this) { getColUnsafe }
customStudyDialogFactory = CustomStudyDialogFactory { getColUnsafe }.attachToActivity(this)

// Then set theme and content view
super.onCreate(savedInstanceState)
Expand Down Expand Up @@ -585,10 +581,12 @@ open class DeckPicker :
supportFragmentManager.setFragmentResultListener(CustomStudyAction.REQUEST_KEY, this) { requestKey, bundle ->
when (CustomStudyAction.fromBundle(bundle)) {
CustomStudyAction.CUSTOM_STUDY_SESSION -> {
Timber.d("Custom study created")
updateDeckList()
openStudyOptions(false)
}
CustomStudyAction.EXTEND_STUDY_LIMITS -> {
Timber.d("Study limits updated")
if (fragmented) {
fragment!!.refreshInterface()
}
Expand Down Expand Up @@ -687,9 +685,7 @@ open class DeckPicker :
}
DeckPickerContextMenuOption.CUSTOM_STUDY -> {
Timber.i("ContextMenu: Custom study option selected")
val d = FragmentFactoryUtils.instantiate(this, CustomStudyDialog::class.java)
d.withArguments(deckId)
showDialogFragment(d)
showDialogFragment(CustomStudyDialog.createInstance(deckId))
}
DeckPickerContextMenuOption.CREATE_SHORTCUT -> {
Timber.i("ContextMenu: Create icon for a deck")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import androidx.fragment.app.commit
import com.ichi2.anki.android.input.ShortcutGroup
import com.ichi2.anki.android.input.ShortcutGroupProvider
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyAction
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialogFactory
import com.ichi2.utils.ExtendedFragmentFactory
import com.ichi2.utils.FragmentFactoryUtils
import timber.log.Timber
import kotlin.reflect.KClass
Expand All @@ -47,12 +45,6 @@ open class SingleFragmentActivity : AnkiActivity() {
return
}

// This page *may* host the CustomStudyDialog (CongratsPage)
// CustomStudyDialog requires a custom factory install during lifecycle or it can
// crash during lifecycle resume after background kill
val customStudyDialogFactory = CustomStudyDialogFactory { this.getColUnsafe }
customStudyDialogFactory.attachToActivity<ExtendedFragmentFactory>(this)

super.onCreate(savedInstanceState)
if (!ensureStoragePermissions()) {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ import anki.collection.OpChanges
import com.ichi2.anki.CollectionManager.withCol
import com.ichi2.anki.StudyOptionsFragment.StudyOptionsListener
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyAction
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialogFactory
import com.ichi2.libanki.ChangeManager
import com.ichi2.ui.RtlCompliantActionProvider
import com.ichi2.utils.ExtendedFragmentFactory
import com.ichi2.widget.WidgetStatus
import kotlinx.coroutines.launch

Expand All @@ -43,8 +41,6 @@ class StudyOptionsActivity :
if (showedActivityFailedScreen(savedInstanceState)) {
return
}
val customStudyDialogFactory = CustomStudyDialogFactory { this.getColUnsafe }
customStudyDialogFactory.attachToActivity<ExtendedFragmentFactory>(this)
super.onCreate(savedInstanceState)
setContentView(R.layout.studyoptions)
enableToolbar().apply { title = "" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import com.ichi2.libanki.ChangeManager
import com.ichi2.libanki.Collection
import com.ichi2.libanki.Decks
import com.ichi2.libanki.Utils
import com.ichi2.utils.FragmentFactoryUtils.instantiate
import com.ichi2.utils.HtmlUtils.convertNewlinesToHtml
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
Expand Down Expand Up @@ -256,10 +255,8 @@ class StudyOptionsFragment :
* Show the context menu for the custom study options
*/
private fun showCustomStudyContextMenu() {
val activity = requireActivity()
val contextMenu = instantiate(activity, CustomStudyDialog::class.java)
contextMenu.withArguments(col!!.decks.selected())
activity.showDialogFragment(contextMenu)
val dialog = CustomStudyDialog.createInstance(deckId = col!!.decks.selected())
requireActivity().showDialogFragment(dialog)
}

override fun onMenuItemSelected(item: MenuItem): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ import com.ichi2.anki.utils.ext.sharedPrefs
import com.ichi2.anki.utils.ext.showDialogFragment
import com.ichi2.anki.withProgress
import com.ichi2.annotations.NeedsTest
import com.ichi2.libanki.Collection
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.bundleOfNotNull
import com.ichi2.utils.cancelable
import com.ichi2.utils.customView
import com.ichi2.utils.dp
Expand All @@ -77,6 +77,7 @@ import com.ichi2.utils.positiveButton
import com.ichi2.utils.setPaddingRelative
import com.ichi2.utils.textAsIntOrNull
import com.ichi2.utils.title
import kotlinx.coroutines.runBlocking
import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException
import org.json.JSONObject
import timber.log.Timber
Expand Down Expand Up @@ -112,10 +113,9 @@ import timber.log.Timber
* * [sched.proto: CustomStudyRequest](https://github.com/search?q=repo%3Aankitects%2Fanki+CustomStudyRequest+language%3A%22Protocol+Buffer%22&type=code&l=Protocol+Buffer)
* * [https://github.com/ankitects/anki/blob/main/qt/aqt/customstudy.py](https://github.com/ankitects/anki/blob/main/qt/aqt/customstudy.py)
*/
@KotlinCleanup("remove 'collection' parameter and use withCol { }")
class CustomStudyDialog(
private val collection: Collection,
) : AnalyticsDialogFragment(),
@KotlinCleanup("remove 'runBlocking' calls'")
class CustomStudyDialog :
AnalyticsDialogFragment(),
TagsDialogListener {
/** ID of the [Deck] which this dialog was created for */
private val dialogDeckId: DeckId
Expand All @@ -131,21 +131,6 @@ class CustomStudyDialog(
/** @see CustomStudyDefaults */
private lateinit var defaults: CustomStudyDefaults

fun withArguments(
did: DeckId,
contextMenuAttribute: ContextMenuOption? = null,
): CustomStudyDialog {
val args = this.arguments ?: Bundle()
args.apply {
if (contextMenuAttribute != null) {
putInt(ARG_SUB_DIALOG_ID, contextMenuAttribute.ordinal)
}
putLong(ARG_DID, did)
}
this.arguments = args
return this
}

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
registerFragmentResultReceiver()
Expand All @@ -154,11 +139,11 @@ class CustomStudyDialog(
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
super.onCreate(savedInstanceState)
val option = selectedSubDialog
this.defaults = collection.sched.customStudyDefaults(dialogDeckId).toDomainModel()
this.defaults = runBlocking { withCol { sched.customStudyDefaults(dialogDeckId).toDomainModel() } }
return if (option == null) {
Timber.i("Showing Custom Study main menu")
// Select the specified deck
collection.decks.select(dialogDeckId)
runBlocking { withCol { decks.select(dialogDeckId) } }
buildContextMenu()
} else {
Timber.i("Showing Custom Study dialog: $option")
Expand All @@ -170,33 +155,33 @@ class CustomStudyDialog(
* Handles selecting an item from the main menu of the dialog
*/
private fun onMenuItemSelected(item: ContextMenuOption) =
when (item) {
STUDY_TAGS -> {
launchCatchingTask {
when (item) {
STUDY_TAGS -> {
/*
* This is a special Dialog for CUSTOM STUDY, where instead of only collecting a
* number, it is necessary to collect a list of tags. This case handles the creation
* of that Dialog.
*/
val dialogFragment =
TagsDialog().withArguments(
context = requireContext(),
type = TagsDialog.DialogType.CUSTOM_STUDY_TAGS,
checkedTags = ArrayList(),
allTags = ArrayList(collection.tags.byDeck(dialogDeckId)),
)
requireActivity().showDialogFragment(dialogFragment)
}
EXTEND_NEW,
EXTEND_REV,
STUDY_FORGOT,
STUDY_AHEAD,
STUDY_PREVIEW,
-> {
// User asked for a standard custom study option
val d =
CustomStudyDialog(collection)
.withArguments(dialogDeckId, item)
requireActivity().showDialogFragment(d)
val dialogFragment =
TagsDialog().withArguments(
context = requireContext(),
type = TagsDialog.DialogType.CUSTOM_STUDY_TAGS,
checkedTags = ArrayList(),
allTags = ArrayList(withCol { tags.byDeck(dialogDeckId) }),
)
requireActivity().showDialogFragment(dialogFragment)
}
EXTEND_NEW,
EXTEND_REV,
STUDY_FORGOT,
STUDY_AHEAD,
STUDY_PREVIEW,
-> {
// User asked for a standard custom study option
val dialog: CustomStudyDialog = createInstance(dialogDeckId, item)
requireActivity().showDialogFragment(dialog)
}
}
}

Expand Down Expand Up @@ -319,9 +304,7 @@ class CustomStudyDialog(
}

try {
undoableOp {
collection.sched.customStudy(request)
}
undoableOp { sched.customStudy(request) }
val action =
when (contextMenuOption) {
EXTEND_NEW, EXTEND_REV -> CustomStudyAction.EXTEND_STUDY_LIMITS
Expand Down Expand Up @@ -364,13 +347,15 @@ class CustomStudyDialog(
}
sb.append("(").append(arr.joinToString(" or ")).append(")")
}
createTagsCustomStudySession(
arrayOf(
sb.toString(),
Consts.DYN_MAX_SIZE,
Consts.DYN_RANDOM,
),
)
launchCatchingTask {
createTagsCustomStudySession(
arrayOf(
sb.toString(),
Consts.DYN_MAX_SIZE,
Consts.DYN_RANDOM,
),
)
}
}

/** Line 1 of the number entry dialog */
Expand Down Expand Up @@ -423,13 +408,12 @@ class CustomStudyDialog(
* Create a custom study session
* @param terms search terms
*/
private fun createTagsCustomStudySession(terms: Array<Any>) {
private suspend fun createTagsCustomStudySession(terms: Array<Any>) {
val dyn: Deck

val decks = collection.decks
val deckToStudyName = decks.name(dialogDeckId)
val deckToStudyName = withCol { decks.name(dialogDeckId) }
val customStudyDeck = resources.getString(R.string.custom_study_deck_name)
val cur = decks.byName(customStudyDeck)
val cur = withCol { decks.byName(customStudyDeck) }
if (cur != null) {
Timber.i("Found deck: '%s'", customStudyDeck)
if (cur.isNormal) {
Expand All @@ -439,16 +423,16 @@ class CustomStudyDialog(
} else {
Timber.i("Emptying dynamic deck '%s' for custom study", customStudyDeck)
// safe to empty
collection.sched.emptyDyn(cur.getLong("id"))
withCol { sched.emptyDyn(cur.getLong("id")) }
// reuse; don't delete as it may have children
dyn = cur
decks.select(cur.getLong("id"))
withCol { decks.select(cur.getLong("id")) }
}
} else {
Timber.i("Creating Dynamic Deck '%s' for custom study", customStudyDeck)
dyn =
try {
decks.get(decks.newFiltered(customStudyDeck))!!
withCol { decks.get(decks.newFiltered(customStudyDeck))!! }
} catch (ex: BackendDeckIsFilteredException) {
showThemedToast(requireActivity(), ex.localizedMessage ?: ex.message ?: "", true)
return
Expand All @@ -465,7 +449,7 @@ class CustomStudyDialog(
// Rebuild the filtered deck
Timber.i("Rebuilding Custom Study Deck")
// PERF: Should be in background
collection.decks.save(dyn)
withCol { decks.save(dyn) }
// launch this in the activity scope, rather than the fragment scope
requireActivity().launchCatchingTask { rebuildDynamicDeck() }
// Hide the dialogs (required due to a DeckPicker issue)
Expand Down Expand Up @@ -638,6 +622,18 @@ class CustomStudyDialog(
}

companion object {
fun createInstance(
deckId: DeckId,
contextMenuAttribute: ContextMenuOption? = null,
): CustomStudyDialog =
CustomStudyDialog().apply {
arguments =
bundleOfNotNull(
ARG_DID to deckId,
contextMenuAttribute?.let { ARG_SUB_DIALOG_ID to it.ordinal },
)
}

/**
* (required) Key for the [DeckId] this dialog deals with.
* @see CustomStudyDialog.dialogDeckId
Expand Down

This file was deleted.

Loading

0 comments on commit 70d22d0

Please sign in to comment.