improvement(browser-columns): allow cards/notes mismatch

If a user is editing options and changes 'cards/notes'
then they should be editing columns for their provided selection

If a user saves columns, then reverts the switch, columns
should not be updated

Issue 17780

AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt

@@ -409,13 +409,15 @@ open class CardBrowser :
                 showFilteredDecks = true,
             )
 
-        this.browserColumnHeadings = findViewById<ViewGroup>(R.id.browser_column_headings).apply {
-            setOnLongClickListener {
-                Timber.d("long press on headings: opening column selection options")
-                showDialogFragment(BrowserColumnSelectionFragment())
-                true
+        this.browserColumnHeadings =
+            findViewById<ViewGroup>(R.id.browser_column_headings).apply {
+                setOnLongClickListener {
+                    Timber.d("long press on headings: opening column selection options")
+                    val dialog = BrowserColumnSelectionFragment.createInstance(viewModel.cardsOrNotes)
+                    showDialogFragment(dialog)
+                    true
+                }
             }
-        }
 
         startLoadingCollection()
 

AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserColumnSelectionFragment.kt

@@ -22,6 +22,8 @@ import android.view.KeyEvent
 import android.view.View
 import androidx.activity.OnBackPressedCallback
 import androidx.annotation.StringRes
+import androidx.core.os.BundleCompat
+import androidx.core.os.bundleOf
 import androidx.fragment.app.DialogFragment
 import androidx.fragment.app.activityViewModels
 import androidx.recyclerview.widget.ItemTouchHelper
@@ -63,6 +65,12 @@ class BrowserColumnSelectionFragment : DialogFragment(R.layout.preferences_brows
     /** The columns which were selected when this dialog was opened */
     private lateinit var initiallySelectedColumns: List<CardBrowserColumn>
 
+    private val cardsOrNotes: CardsOrNotes
+        get() =
+            requireNotNull(
+                BundleCompat.getParcelable(requireArguments(), ARG_MODE, CardsOrNotes::class.java),
+            )
+
     // this is needed as requireActivity().onBackPressedDispatcher.onBackPressed()
     // closes the activity, but an actual back press closes the dialog
     private val onCloseDialogCallback =
@@ -105,7 +113,7 @@ class BrowserColumnSelectionFragment : DialogFragment(R.layout.preferences_brows
                 when (menuItem.itemId) {
                     R.id.action_save_columns -> {
                         Timber.d("save columns and close")
-                        if (viewModel.updateColumns(columnAdapter.selected)) {
+                        if (viewModel.updateColumns(columnAdapter.selected, cardsOrNotes)) {
                             dismiss()
                             true
                         } else {
@@ -153,7 +161,7 @@ class BrowserColumnSelectionFragment : DialogFragment(R.layout.preferences_brows
     private fun setupRecyclerView(view: View) {
         // Create a RecyclerView with 2 types of elements: [ColumnWithSample] and [ColumnUsage]
         // Columns are draggable elements, the usage elements act as headings
-        val (displayed, available) = runBlocking { viewModel.previewColumns() }
+        val (displayed, available) = runBlocking { viewModel.previewColumns(cardsOrNotes) }
         this.initiallySelectedColumns = displayed.map { it.columnType }
 
         val recyclerViewItems =
@@ -208,6 +216,19 @@ class BrowserColumnSelectionFragment : DialogFragment(R.layout.preferences_brows
     // Although a user can reorder the non-displayed columns, this order is not persisted
     private val hasUnsavedChanges: Boolean
         get() = initiallySelectedColumns != columnAdapter.selected
+
+    companion object {
+        const val ARG_MODE = "mode"
+
+        fun createInstance(cardsOrNotes: CardsOrNotes): BrowserColumnSelectionFragment =
+            BrowserColumnSelectionFragment().apply {
+                Timber.d("Building 'Manage columns' dialog for %s mode", cardsOrNotes)
+                arguments =
+                    bundleOf(
+                        ARG_MODE to cardsOrNotes,
+                    )
+            }
+    }
 }
 
 /**

AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt

@@ -511,7 +511,10 @@ class CardBrowserViewModel(
     }
 
     @CheckResult
-    fun updateColumns(columns: List<CardBrowserColumn>): Boolean {
+    fun updateColumns(
+        columns: List<CardBrowserColumn>,
+        cardsOrNotes: CardsOrNotes,
+    ): Boolean {
         if (columns.isEmpty()) return false.also { Timber.d("updateColumns: no columns") }
         if (availableColumns == columns) return false.also { Timber.d("updateColumns: no changes") }
 
@@ -856,8 +859,13 @@ class CardBrowserViewModel(
 
     suspend fun querySelectedCardIdAtPosition(index: Int): CardId = selectedRows.toList()[index].toCardId(cardsOrNotes)
 
-    suspend fun previewColumns(): Pair<List<ColumnWithSample>, List<ColumnWithSample>> {
-        val currentColumns = availableColumns
+    suspend fun previewColumns(cardsOrNotes: CardsOrNotes): Pair<List<ColumnWithSample>, List<ColumnWithSample>> {
+        val currentColumns =
+            when {
+                // if we match, use the loaded the columns
+                cardsOrNotes == this.cardsOrNotes -> availableColumns
+                else -> BrowserColumnCollection.load(sharedPrefs(), cardsOrNotes).columns
+            }
 
         var columnsWithSample = ColumnWithSample.loadSample(cards.firstOrNull(), cardsOrNotes)
 

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/BrowserOptionsDialog.kt

@@ -43,13 +43,20 @@ class BrowserOptionsDialog : AppCompatDialogFragment() {
 
     private val viewModel: CardBrowserViewModel by activityViewModels()
 
+    /** The unsaved value of [CardsOrNotes] */
+    private val dialogCardsOrNotes: CardsOrNotes
+        get() {
+            @IdRes val selectedButtonId =
+                dialogView.findViewById<RadioGroup>(R.id.select_browser_mode).checkedRadioButtonId
+            return when (selectedButtonId) {
+                R.id.select_cards_mode -> CardsOrNotes.CARDS
+                else -> CardsOrNotes.NOTES
+            }
+        }
+
     private val positiveButtonClick = { _: DialogInterface, _: Int ->
-        @IdRes val selectedButtonId =
-            dialogView.findViewById<RadioGroup>(R.id.select_browser_mode).checkedRadioButtonId
-        val newCardsOrNotes =
-            if (selectedButtonId == R.id.select_cards_mode) CardsOrNotes.CARDS else CardsOrNotes.NOTES
-        if (cardsOrNotes != newCardsOrNotes) {
-            viewModel.setCardsOrNotes(newCardsOrNotes)
+        if (cardsOrNotes != dialogCardsOrNotes) {
+            viewModel.setCardsOrNotes(dialogCardsOrNotes)
         }
         val newTruncate = dialogView.findViewById<CheckBox>(R.id.truncate_checkbox).isChecked
 
@@ -114,9 +121,10 @@ class BrowserOptionsDialog : AppCompatDialogFragment() {
         }
     }
 
-    fun openColumnManager() {
-        Timber.d("opening column manager")
-        requireActivity().showDialogFragment(BrowserColumnSelectionFragment())
+    /** Opens [BrowserColumnSelectionFragment] for the current selection of [CardsOrNotes] */
+    private fun openColumnManager() {
+        val dialog = BrowserColumnSelectionFragment.createInstance(dialogCardsOrNotes)
+        requireActivity().showDialogFragment(dialog)
     }
 
     companion object {

AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt

@@ -424,6 +424,29 @@ class CardBrowserViewModelTest : JvmTest() {
             }
         }
 
+    @Test
+    fun `mode mismatch - changing columns`() {
+        // a user can use the options to update the inactive column set
+        // this should cause an update to the backend, but no frontend events should be obtained
+        runViewModelTest {
+            flowOfAvailableColumns.test {
+                ignoreEventsDuringViewModelInit()
+                assertThat("cardsOrNotes", cardsOrNotes, equalTo(CardsOrNotes.CARDS))
+
+                // we're in cards so editing NOTES should not update visible columns
+                setColumn(1, FSRS_STABILITY, CardsOrNotes.NOTES)
+                expectNoEvents()
+
+                BrowserColumnCollection.load(sharedPrefs(), CardsOrNotes.NOTES).apply {
+                    assertThat("notes is changed", this.columns[1], equalTo(FSRS_STABILITY))
+                }
+                BrowserColumnCollection.load(sharedPrefs(), CardsOrNotes.CARDS).apply {
+                    assertThat("cards is unchanged", this.columns[1], not(equalTo(FSRS_STABILITY)))
+                }
+            }
+        }
+    }
+
     @Test
     fun `change card order to NO_SORTING is a no-op if done twice`() =
         runViewModelTest {
@@ -815,7 +838,7 @@ class CardBrowserViewModelTest : JvmTest() {
         }
         runViewModelTest(notes = 0) {
             assertThat("no rows", rowCount, equalTo(0))
-            this.previewColumns().apply {
+            this.previewColumns(CardsOrNotes.CARDS).apply {
                 assertTrue("no sample values") { allColumns.all { it.sampleValue == null } }
                 val (displayed, _) = this
                 assertThat(
@@ -828,7 +851,7 @@ class CardBrowserViewModelTest : JvmTest() {
 
         runViewModelNotesTest(notes = 0) {
             assertThat("no rows", rowCount, equalTo(0))
-            this.previewColumns().apply {
+            this.previewColumns(CardsOrNotes.NOTES).apply {
                 assertTrue("no sample values") { allColumns.all { it.sampleValue == null } }
                 val (displayed, _) = this
                 assertThat(
@@ -849,7 +872,7 @@ class CardBrowserViewModelTest : JvmTest() {
     @Test
     fun `preview - cards`() {
         runViewModelTest(notes = 1) {
-            for (preview in previewColumns().allColumns) {
+            for (preview in previewColumns(CardsOrNotes.CARDS).allColumns) {
                 val (expectedLabel, expectedValue) =
                     when (preview.columnType) {
                         SFLD -> Pair("Sort Field", "Front")
@@ -882,7 +905,7 @@ class CardBrowserViewModelTest : JvmTest() {
     @Test
     fun `preview - notes`() {
         runViewModelNotesTest(notes = 1) {
-            for (preview in previewColumns().allColumns) {
+            for (preview in previewColumns(CardsOrNotes.NOTES).allColumns) {
                 val (expectedLabel, expectedValue) =
                     when (preview.columnType) {
                         SFLD -> Pair("Sort Field", "Front")
@@ -1088,10 +1111,11 @@ val CardBrowserViewModel.column2
 fun CardBrowserViewModel.setColumn(
     index: Int,
     column: CardBrowserColumn,
+    cardsOrNotes: CardsOrNotes = this.cardsOrNotes,
 ): Boolean {
     val newColumns = availableColumns.toMutableList()
     newColumns[index] = column
-    return updateColumns(newColumns)
+    return updateColumns(newColumns, cardsOrNotes)
 }
 
 val Pair<List<ColumnWithSample>, List<ColumnWithSample>>.allColumns