Refactor DeckUtils and CardAnalysisWidget for Improved Deck Handling

xenonnn4w · david-allison · commit 6d3d0ca72828 · 2024-09-08T18:47:20.000Z
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckUtils.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckUtils.kt
@@ -17,57 +17,43 @@
 package com.ichi2.anki
 
 import com.ichi2.anki.CollectionManager.withCol
-import com.ichi2.libanki.Decks
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.withContext
+import com.ichi2.libanki.Collection
+import com.ichi2.libanki.Consts
 
-object DeckUtils {
-
-    /**
-     * Checks if a given deck, including its subdecks if specified, is empty.
-     *
-     * @param decks The [Decks] instance containing the decks to check.
-     * @param deckId The ID of the deck to check.
-     * @param includeSubdecks If true, includes subdecks in the check. Default is true.
-     * @return `true` if the deck (and subdecks if specified) is empty, otherwise `false`.
-     */
-    private fun isDeckEmpty(decks: Decks, deckId: Long, includeSubdecks: Boolean = true): Boolean {
-        val deckIds = decks.deckAndChildIds(deckId)
-        val totalCardCount = decks.cardCount(*deckIds.toLongArray(), includeSubdecks = includeSubdecks)
-        return totalCardCount == 0
-    }
+/**
+ * Checks if a given deck, including its subdecks if specified, is empty.
+ *
+ * @param deckId The ID of the deck to check.
+ * @param includeSubdecks If true, includes subdecks in the check. Default is true.
+ * @return `true` if the deck (and subdecks if specified) is empty, otherwise `false`.
+ */
+private fun Collection.isDeckEmpty(deckId: Long, includeSubdecks: Boolean = true): Boolean {
+    val deckIds = decks.deckAndChildIds(deckId)
+    val totalCardCount = decks.cardCount(*deckIds.toLongArray(), includeSubdecks = includeSubdecks)
+    return totalCardCount == 0
+}
 
-    /**
-     * Checks if the default deck is empty.
-     *
-     * This method runs on an IO thread and accesses the collection to determine if the default deck (with ID 1) is empty.
-     *
-     * @return `true` if the default deck is empty, otherwise `false`.
-     */
-    suspend fun isDefaultDeckEmpty(): Boolean {
-        val defaultDeckId = 1L
-        return withContext(Dispatchers.IO) {
-            withCol {
-                isDeckEmpty(decks, defaultDeckId)
-            }
-        }
-    }
+/**
+ * Checks if the default deck is empty.
+ *
+ * This method runs on an IO thread and accesses the collection to determine if the default deck (with ID 1) is empty.
+ *
+ * @return `true` if the default deck is empty, otherwise `false`.
+ */
+suspend fun isDefaultDeckEmpty(): Boolean = withCol { isDeckEmpty(Consts.DEFAULT_DECK_ID) }
 
-    /**
-     * Returns whether the deck picker displays any deck.
-     * Technically, it means that there is a non default deck, or that the default deck is non-empty.
-     *
-     * This function is specifically implemented to address an issue where the default deck
-     * isn't handled correctly when a second deck is added to the
-     * collection. In this case, the deck tree may incorrectly appear as non-empty when it contains
-     * only the default deck and no other cards.
-     *
-     */
-    suspend fun isCollectionEmpty(): Boolean {
-        val tree = withCol { sched.deckDueTree() }
-        if (tree.children.size == 1 && tree.children[0].did == 1L) {
-            return isDefaultDeckEmpty()
-        }
-        return false
-    }
+/**
+ * Returns whether the deck picker displays any deck.
+ * Technically, it means that there is a non-default deck, or that the default deck is non-empty.
+ *
+ * This function is specifically implemented to address an issue where the default deck
+ * isn't handled correctly when a second deck is added to the
+ * collection. In this case, the deck tree may incorrectly appear as non-empty when it contains
+ * only the default deck and no other cards.
+ *
+ */
+suspend fun isCollectionEmpty(): Boolean {
+    val tree = withCol { sched.deckDueTree() }
+    val onlyDefaultDeckAvailable = tree.children.singleOrNull()?.did == Consts.DEFAULT_DECK_ID
+    return onlyDefaultDeckAvailable && isDefaultDeckEmpty()
 }
diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt b/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt
@@ -26,12 +26,13 @@ import android.view.View
 import android.widget.RemoteViews
 import com.ichi2.anki.AnkiDroidApp
 import com.ichi2.anki.CrashReportService
-import com.ichi2.anki.DeckUtils
 import com.ichi2.anki.R
 import com.ichi2.anki.Reviewer
 import com.ichi2.anki.analytics.UsageAnalytics
+import com.ichi2.anki.isCollectionEmpty
 import com.ichi2.anki.pages.DeckOptions
 import com.ichi2.libanki.DeckId
+import com.ichi2.libanki.Decks.Companion.NOT_FOUND_DECK_ID
 import com.ichi2.widget.ACTION_UPDATE_WIDGET
 import com.ichi2.widget.AnalyticsWidgetProvider
 import com.ichi2.widget.cancelRecurringAlarm
@@ -60,43 +61,53 @@ class CardAnalysisWidget : AnalyticsWidgetProvider() {
          * Updates the widget with the deck data.
          *
          * This method updates the widget view content with the deck data corresponding
-         * to the provided deck ID. If the deck is deleted, the widget will be cleared.
+         * to the provided deck ID. If the deck is deleted, the widget will be show a message "Missing deck. Please reconfigure".
          *
          * @param context the context of the application
          * @param appWidgetManager the AppWidgetManager instance
          * @param appWidgetId the ID of the app widget
-         * @param deckId the ID of the deck to be displayed in the widget.
          */
         fun updateWidget(
             context: Context,
             appWidgetManager: AppWidgetManager,
-            appWidgetId: Int,
-            deckId: DeckId?
+            appWidgetId: Int
         ) {
+            val deckId = getDeckIdForWidget(context, appWidgetId)
             val remoteViews = RemoteViews(context.packageName, R.layout.widget_card_analysis)
-            if (deckId == null) {
+
+            if (deckId == NOT_FOUND_DECK_ID) {
+                // If deckId is null, it means no deck was selected or the selected deck was deleted.
+                // In this case, we don't save the null value to preferences because we want to
+                // keep the previous deck ID if the user reconfigures the widget later.
+                // Instead, we show a message prompting the user to reconfigure the widget.
                 showMissingDeck(context, appWidgetManager, appWidgetId, remoteViews)
                 return
             }
+
             AnkiDroidApp.applicationScope.launch {
-                val isCollectionEmpty = DeckUtils.isCollectionEmpty()
+                val isCollectionEmpty = isCollectionEmpty()
                 if (isCollectionEmpty) {
                     showCollectionDeck(context, appWidgetManager, appWidgetId, remoteViews)
                     return@launch
                 }
 
                 val deckData = getDeckNameAndStats(deckId)
-
                 if (deckData == null) {
-                    // If the deck was deleted, clear the stored deck ID
-                    CardAnalysisWidgetPreferences(context).saveSelectedDeck(appWidgetId, null)
+                    // The deck was found but no data could be fetched, so update the preferences to remove the deck.
+                    // This ensures that the widget does not retain a reference to a non-existent or invalid deck.
+                    CardAnalysisWidgetPreferences(context).saveSelectedDeck(appWidgetId, NOT_FOUND_DECK_ID)
                     showMissingDeck(context, appWidgetManager, appWidgetId, remoteViews)
                     return@launch
                 }
                 showDeck(context, appWidgetManager, appWidgetId, remoteViews, deckData)
             }
         }
 
+        private fun getDeckIdForWidget(context: Context, appWidgetId: Int): DeckId {
+            val widgetPreferences = CardAnalysisWidgetPreferences(context)
+            return widgetPreferences.getSelectedDeckIdFromPreferences(appWidgetId) ?: NOT_FOUND_DECK_ID
+        }
+
         private fun showCollectionDeck(
             context: Context,
             appWidgetManager: AppWidgetManager,
@@ -160,6 +171,7 @@ class CardAnalysisWidget : AnalyticsWidgetProvider() {
                 Intent(context, Reviewer::class.java).apply {
                     action = Intent.ACTION_VIEW
                     putExtra("deckId", deckData.deckId)
+                    addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
                 }
             } else {
                 DeckOptions.getIntent(context, deckData.deckId)
@@ -189,9 +201,8 @@ class CardAnalysisWidget : AnalyticsWidgetProvider() {
             Timber.d("AppWidgetIds to update: ${appWidgetIds.joinToString(", ")}")
 
             for (appWidgetId in appWidgetIds) {
-                val widgetPreferences = CardAnalysisWidgetPreferences(context)
-                val deckId = widgetPreferences.getSelectedDeckIdFromPreferences(appWidgetId)
-                updateWidget(context, appWidgetManager, appWidgetId, deckId)
+                getDeckIdForWidget(context, appWidgetId)
+                updateWidget(context, appWidgetManager, appWidgetId)
             }
         }
     }
@@ -204,21 +215,25 @@ class CardAnalysisWidget : AnalyticsWidgetProvider() {
     ) {
         Timber.d("Performing widget update for appWidgetIds: %s", appWidgetIds)
 
-        val widgetPreferences = CardAnalysisWidgetPreferences(context)
-
         for (widgetId in appWidgetIds) {
             Timber.d("Updating widget with ID: $widgetId")
-            val selectedDeckId = widgetPreferences.getSelectedDeckIdFromPreferences(widgetId)
 
-            /**Explanation of behavior when selectedDeckId is empty
+            // Get the selected deck ID internally
+            val selectedDeckId = getDeckIdForWidget(context, widgetId)
+
+            /**
+             * Explanation of behavior when selectedDeckId is empty
              * If selectedDeckId is empty, the widget will retain the previous deck.
              * This behavior ensures that the widget does not display an empty view, which could be
              * confusing to the user. Instead, it maintains the last known state until a new valid
              * deck ID is provided. This approach prioritizes providing a consistent
              * user experience over showing an empty or default state.
              */
             Timber.d("Selected deck ID: $selectedDeckId for widget ID: $widgetId")
-            updateWidget(context, appWidgetManager, widgetId, selectedDeckId)
+
+            // Update the widget with the selected deck ID
+            updateWidget(context, appWidgetManager, widgetId)
+            // Set the recurring alarm for the widget
             setRecurringAlarm(context, widgetId, CardAnalysisWidget::class.java)
         }
 
@@ -244,19 +259,23 @@ class CardAnalysisWidget : AnalyticsWidgetProvider() {
 
                 Timber.d("Received ACTION_APPWIDGET_UPDATE with widget ID: $appWidgetId and selectedDeckId: $selectedDeckId")
 
-                if (appWidgetId != AppWidgetManager.INVALID_APPWIDGET_ID && selectedDeckId != -1L) {
+                if (appWidgetId != AppWidgetManager.INVALID_APPWIDGET_ID) {
                     Timber.d("Updating widget with ID: $appWidgetId")
-                    // Wrap selectedDeckId into a LongArray
-                    updateWidget(context, appWidgetManager, appWidgetId, selectedDeckId)
+
+                    // Update the widget using the internally fetched deck ID
+                    updateWidget(context, appWidgetManager, appWidgetId)
+
                     Timber.d("Widget update process completed for widget ID: $appWidgetId")
                 }
             }
-            // This custom action is received to update a specific widget.
-            // It is triggered by the setRecurringAlarm method to refresh the widget's data periodically.
+            // Custom action to update a specific widget, triggered by the setRecurringAlarm method
             ACTION_UPDATE_WIDGET -> {
                 val appWidgetId = intent.getIntExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, AppWidgetManager.INVALID_APPWIDGET_ID)
                 if (appWidgetId != AppWidgetManager.INVALID_APPWIDGET_ID) {
                     Timber.d("Received ACTION_UPDATE_WIDGET for widget ID: $appWidgetId")
+
+                    // Update the widget using the internally fetched deck ID
+                    updateWidget(context, AppWidgetManager.getInstance(context), appWidgetId)
                 }
             }
             AppWidgetManager.ACTION_APPWIDGET_DELETED -> {
diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidgetConfig.kt b/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidgetConfig.kt
@@ -26,19 +26,20 @@ import android.os.Bundle
 import android.view.View
 import android.widget.Button
 import androidx.activity.OnBackPressedCallback
+import androidx.annotation.StringRes
 import androidx.core.view.isVisible
 import androidx.lifecycle.lifecycleScope
 import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.RecyclerView
 import com.google.android.material.floatingactionbutton.FloatingActionButton
 import com.google.android.material.snackbar.Snackbar
 import com.ichi2.anki.AnkiActivity
-import com.ichi2.anki.DeckUtils.isCollectionEmpty
 import com.ichi2.anki.R
 import com.ichi2.anki.dialogs.DeckSelectionDialog
 import com.ichi2.anki.dialogs.DeckSelectionDialog.DeckSelectionListener
 import com.ichi2.anki.dialogs.DeckSelectionDialog.SelectableDeck
 import com.ichi2.anki.dialogs.DiscardChangesDialog
+import com.ichi2.anki.isCollectionEmpty
 import com.ichi2.anki.showThemedToast
 import com.ichi2.anki.snackbar.BaseSnackbarBuilderProvider
 import com.ichi2.anki.snackbar.SnackbarBuilder
@@ -49,6 +50,8 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 
+// TODO: Ensure that the Deck Selection Dialog does not close automatically while the user is interacting with it.
+
 class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnackbarBuilderProvider {
 
     private var appWidgetId = AppWidgetManager.INVALID_APPWIDGET_ID
@@ -62,6 +65,7 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
     private var hasUnsavedChanges = false
     private var isAdapterObserverRegistered = false
     private lateinit var onBackPressedCallback: OnBackPressedCallback
+    private val EXTRA_SELECTED_DECK_IDS = "card_analysis_widget_selected_deck_ids"
 
     override fun onCreate(savedInstanceState: Bundle?) {
         if (showedActivityFailedScreen(savedInstanceState)) {
@@ -117,7 +121,7 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
         )
     }
 
-    fun showSnackbar(messageResId: Int) {
+    fun showSnackbar(@StringRes messageResId: Int) {
         showSnackbar(getString(messageResId))
     }
 
@@ -128,7 +132,6 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
             updateViewVisibility()
             updateFabVisibility()
             updateSubmitButtonText()
-            hasUnsavedChanges = true
             setUnsavedChanges(true)
         }
 
@@ -156,9 +159,9 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
 
         registerReceiver(widgetRemovedReceiver, IntentFilter(AppWidgetManager.ACTION_APPWIDGET_DELETED))
 
-        onBackPressedCallback = object : OnBackPressedCallback(false) {
+        onBackPressedCallback = object : OnBackPressedCallback(hasUnsavedChanges) {
             override fun handleOnBackPressed() {
-                if (hasUnsavedChanges) {
+                if (isEnabled) {
                     showDiscardChangesDialog()
                 }
             }
@@ -291,17 +294,16 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
             deckAdapter.addDeck(deck)
             updateViewVisibility()
             updateFabVisibility()
-            hasUnsavedChanges = true
             setUnsavedChanges(true)
 
             // Save the selected deck immediately
             saveSelectedDecksToPreferencesCardAnalysisWidget()
-            hasUnsavedChanges = false
             setUnsavedChanges(false)
 
-            val selectedDeckId = cardAnalysisWidgetPreferences.getSelectedDeckIdFromPreferences(appWidgetId)
+            // Update the widget with the new selected deck ID
+            cardAnalysisWidgetPreferences.getSelectedDeckIdFromPreferences(appWidgetId)
             val appWidgetManager = AppWidgetManager.getInstance(this)
-            CardAnalysisWidget.updateWidget(this, appWidgetManager, appWidgetId, selectedDeckId)
+            CardAnalysisWidget.updateWidget(this, appWidgetManager, appWidgetId)
 
             val resultValue = Intent().putExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, appWidgetId)
             setResult(RESULT_OK, resultValue)
@@ -328,8 +330,7 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
         val updateIntent = Intent(this, CardAnalysisWidget::class.java).apply {
             action = AppWidgetManager.ACTION_APPWIDGET_UPDATE
             putExtra(AppWidgetManager.EXTRA_APPWIDGET_IDS, intArrayOf(appWidgetId))
-
-            putExtra("card_analysis_widget_selected_deck_ids", selectedDeck)
+            putExtra(EXTRA_SELECTED_DECK_IDS, selectedDeck)
         }
 
         sendBroadcast(updateIntent)
@@ -347,7 +348,7 @@ class CardAnalysisWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnac
                 return
             }
 
-            context?.let { cardAnalysisWidgetPreferences.deleteDeckData(appWidgetId) }
+            cardAnalysisWidgetPreferences.deleteDeckData(appWidgetId)
         }
     }
 }
diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidgetConfig.kt b/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidgetConfig.kt
@@ -34,13 +34,13 @@ import androidx.recyclerview.widget.RecyclerView
 import com.google.android.material.floatingactionbutton.FloatingActionButton
 import com.google.android.material.snackbar.Snackbar
 import com.ichi2.anki.AnkiActivity
-import com.ichi2.anki.DeckUtils
-import com.ichi2.anki.DeckUtils.isCollectionEmpty
 import com.ichi2.anki.R
 import com.ichi2.anki.dialogs.DeckSelectionDialog
 import com.ichi2.anki.dialogs.DeckSelectionDialog.DeckSelectionListener
 import com.ichi2.anki.dialogs.DeckSelectionDialog.SelectableDeck
 import com.ichi2.anki.dialogs.DiscardChangesDialog
+import com.ichi2.anki.isCollectionEmpty
+import com.ichi2.anki.isDefaultDeckEmpty
 import com.ichi2.anki.showThemedToast
 import com.ichi2.anki.snackbar.BaseSnackbarBuilderProvider
 import com.ichi2.anki.snackbar.SnackbarBuilder
@@ -280,10 +280,6 @@ class DeckPickerWidgetConfig : AnkiActivity(), DeckSelectionListener, BaseSnackb
         }
     }
 
-    private suspend fun isDefaultDeckEmpty(): Boolean {
-        return DeckUtils.isDefaultDeckEmpty()
-    }
-
     /** Updates the view according to the saved preference for appWidgetId.*/
     fun updateViewWithSavedPreferences() {
         val selectedDeckIds = deckPickerWidgetPreferences.getSelectedDeckIdsFromPreferences(appWidgetId)
diff --git a/AnkiDroid/src/main/res/values/08-widget.xml b/AnkiDroid/src/main/res/values/08-widget.xml
@@ -43,7 +43,7 @@
 
     <!-- Strings to explain usage in Deck Picker and Card Analysis Widget Configuration screen -->
     <string name="select_decks_title" comment="Title for Deck Selection Dialog">Select decks</string>
-    <string name="select_deck_title" comment="Title for Deck Selection Dialog">Select deck</string>
+    <string name="select_deck_title" comment="Title for Deck Selection Dialog">Select a deck</string>
     <string name="no_selected_deck_placeholder_title" comment="Placeholder title when no decks are selected">Select decks to display in the widget. Select decks with the + icon.</string>
     <string name="deck_removed_from_widget" comment="Snackbar when deck is removed from widget">Deck removed</string>
     <string name="deck_already_selected_message" comment="Snackbar when user try to select the same deck again">This deck is already selected</string>
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/widget/cardanalysis/CardAnalysisWidgetConfigTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/widget/cardanalysis/CardAnalysisWidgetConfigTest.kt
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/widget/deckpicker/DeckPickerWidgetConfigTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/widget/deckpicker/DeckPickerWidgetConfigTest.kt
