From 9b82edb5c8b049e005e1810c273740abc65b76a3 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Mon, 11 May 2026 19:39:19 +0100 Subject: [PATCH 1/2] refactor(card-browser): initial selected deck This should be a responsibility of the browser, not the destination Assisted-by: Claude Opus 4.7 --- .../ichi2/anki/browser/BrowserDestination.kt | 16 ++-- .../anki/browser/CardBrowserViewModel.kt | 18 ++++- .../anki/browser/CardBrowserViewModelTest.kt | 81 +++++++++++++++++-- 3 files changed, 100 insertions(+), 15 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt b/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt index 9e098df2798a..dc5be1162c62 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt @@ -18,7 +18,6 @@ package com.ichi2.anki.browser import android.content.Context import android.content.Intent -import com.ichi2.anki.AnkiDroidApp import com.ichi2.anki.CardBrowser import com.ichi2.anki.libanki.CardId import com.ichi2.anki.libanki.DeckId @@ -31,10 +30,10 @@ sealed interface BrowserDestination : Destination { data class ToDeck( val deckId: DeckId, ) : BrowserDestination { - override fun toIntent(context: Context): Intent { - AnkiDroidApp.instance.sharedPrefsLastDeckIdRepository.lastDeckId = deckId - return Intent(context, CardBrowser::class.java) - } + override fun toIntent(context: Context): Intent = + Intent(context, CardBrowser::class.java).apply { + putExtra(CardBrowserViewModel.EXTRA_DECK_ID, deckId) + } } /** @@ -45,12 +44,11 @@ sealed interface BrowserDestination : Destination { val deckId: DeckId, val cardId: CardId, ) : BrowserDestination { - override fun toIntent(context: Context): Intent { - AnkiDroidApp.instance.sharedPrefsLastDeckIdRepository.lastDeckId = deckId - return Intent(context, CardBrowser::class.java).apply { + override fun toIntent(context: Context): Intent = + Intent(context, CardBrowser::class.java).apply { + putExtra(CardBrowserViewModel.EXTRA_DECK_ID, deckId) putExtra(EXTRA_CARD_ID_KEY, cardId) } - } companion object { const val EXTRA_CARD_ID_KEY = "cardId" diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt index 9dada14e7d94..d40dacd045cb 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt @@ -396,7 +396,17 @@ class CardBrowserViewModel( suspend fun queryDataForCardEdit(id: CardOrNoteId): CardId? = id.toCardId(cardsOrNotes) private suspend fun getInitialDeck(): SelectableDeck { - // TODO: Handle the launch intent + suspend fun consumeIntentDeck(): SelectableDeck.Deck? { + if (savedStateHandle.get(STATE_LAUNCH_INTENT_CONSUMED) == true) return null + savedStateHandle[STATE_LAUNCH_INTENT_CONSUMED] = true + val deckId = savedStateHandle.get(EXTRA_DECK_ID) ?: return null + val name = withCol { decks.nameIfExists(deckId) } ?: return null + return SelectableDeck.Deck(deckId = deckId, name = name) + } + + // Intent-supplied deck takes precedence, but only on the first launch + consumeIntentDeck()?.let { deck -> return deck } + val lastDeckId = lastDeckId if (lastDeckId == ALL_DECKS_ID) { return SelectableDeck.AllDecks @@ -1424,6 +1434,12 @@ class CardBrowserViewModel( suspend fun getAvailableDecks(): List = SelectableDeck.fromCollection(includeFiltered = false) companion object { + /** Intent extra carrying the [DeckId] the browser should open scoped to. */ + const val EXTRA_DECK_ID = "deckId" + + /** Prevents one-shot extras from being re-applied after process death. */ + private const val STATE_LAUNCH_INTENT_CONSUMED = "launchIntentConsumed" + const val STATE_MULTISELECT = "multiselect" const val STATE_MULTISELECT_VALUES = "multiselect_values" diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt index ef4621680760..8628f255462a 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt @@ -403,6 +403,77 @@ class CardBrowserViewModelTest : JvmTest() { } } + @Test + fun `EXTRA_DECK_ID intent opens the specified deck`() = + runTest { + val deckId = addDeck("New") + val savedState = SavedStateHandle(mapOf(CardBrowserViewModel.EXTRA_DECK_ID to deckId)) + viewModel(savedStateHandle = savedState).apply { + assertThat("intent deck is selected", deckId, equalTo(this.deckId)) + } + } + + @Test + fun `EXTRA_DECK_ID intent persists deck to lastDeckIdRepository`() = + runTest { + val deckId = addDeck("New") + val savedState = SavedStateHandle(mapOf(CardBrowserViewModel.EXTRA_DECK_ID to deckId)) + viewModel(savedStateHandle = savedState).apply { + assertThat("deck persisted for next launch", lastDeckId, equalTo(deckId)) + } + } + + @Test + fun `no EXTRA_DECK_ID falls back to lastDeckIdRepository`() = + runTest { + val deckId = addDeck("Persisted") + viewModel(lastDeckId = deckId).apply { + assertThat("repository value is used", deckId, equalTo(this.deckId)) + } + } + + @Test + fun `EXTRA_DECK_ID for unknown deck falls back to lastDeckIdRepository`() = + runTest { + val persisted = addDeck("Persisted") + val unknownDeckId: DeckId = 9_999_999_999L + val savedState = SavedStateHandle(mapOf(CardBrowserViewModel.EXTRA_DECK_ID to unknownDeckId)) + viewModel(lastDeckId = persisted, savedStateHandle = savedState).apply { + assertThat("unknown intent deck is ignored", persisted, equalTo(this.deckId)) + } + } + + @Test + fun `user deck change survives process death`() = + runTest { + val intentDeckId = addDeck("From intent") + val userDeckId = addDeck("User selection") + val savedState = SavedStateHandle(mapOf(CardBrowserViewModel.EXTRA_DECK_ID to intentDeckId)) + + val persistentRepo = + object : LastDeckIdRepository { + override var lastDeckId: DeckId? = null + } + + // setup: initial launch + select new deck + viewModel( + savedStateHandle = savedState, + lastDeckIdRepository = persistentRepo, + ).apply { + assertThat("intent honored on first launch", deckId, equalTo(intentDeckId)) + setSelectedDeck(SelectableDeck.Deck(deckId = userDeckId, name = "User selection")) + assertThat("user choice persisted to repository", persistentRepo.lastDeckId, equalTo(userDeckId)) + } + + // intent does not override user selection + viewModel( + savedStateHandle = savedState, + lastDeckIdRepository = persistentRepo, + ).apply { + assertThat("user choice survives recreation", deckId, equalTo(userDeckId)) + } + } + @Test fun `sort order from notes is selected - 16514`() { col.config.set("sortType", "noteCrt") @@ -1710,12 +1781,12 @@ class CardBrowserViewModelTest : JvmTest() { lastDeckId: DeckId? = null, intent: CardBrowserLaunchOptions? = null, mode: CardsOrNotes = CardsOrNotes.CARDS, - ): CardBrowserViewModel { - val lastDeckIdRepository = + savedStateHandle: SavedStateHandle = SavedStateHandle(), + lastDeckIdRepository: LastDeckIdRepository = object : LastDeckIdRepository { override var lastDeckId: DeckId? = lastDeckId - } - + }, + ): CardBrowserViewModel { // default is CARDS, do nothing in this case if (mode == CardsOrNotes.NOTES) { CollectionManager.withCol { mode.saveToCollection(this@withCol) } @@ -1728,7 +1799,7 @@ class CardBrowserViewModelTest : JvmTest() { options = intent, isFragmented = false, preferences = AnkiDroidApp.sharedPreferencesProvider, - savedStateHandle = SavedStateHandle(), + savedStateHandle = savedStateHandle, ).apply { invokeInitialSearch() } From b8ec92668f613d1cd9ad496e0d0ec1c0d7aed716 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Mon, 11 May 2026 19:47:08 +0100 Subject: [PATCH 2/2] refactor(card-browser): browser destination * ToCard => ScrollToCard * Move EXTRA_CARD_ID_KEY to ViewModel This should be a responsibility of the browser, not the destination Assisted-by: Claude Opus 4.7 - all --- .../com/ichi2/anki/browser/BrowserDestination.kt | 12 ++++-------- .../ichi2/anki/browser/CardBrowserLaunchOptions.kt | 2 +- .../com/ichi2/anki/browser/CardBrowserViewModel.kt | 3 +++ .../anki/ui/windows/reviewer/ReviewerViewModel.kt | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt b/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt index dc5be1162c62..a5dc6b2e0108 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/browser/BrowserDestination.kt @@ -37,21 +37,17 @@ sealed interface BrowserDestination : Destination { } /** - * Opens the [CardBrowser] with the specified deck selected, - * and automatically scrolls to [cardId] if the card is present on the deck. + * Opens the [CardBrowser] scoped to [deckId], auto-scrolling to [cardId] + * if the card is present on the deck. */ - data class ToCard( + data class ScrollToCard( val deckId: DeckId, val cardId: CardId, ) : BrowserDestination { override fun toIntent(context: Context): Intent = Intent(context, CardBrowser::class.java).apply { putExtra(CardBrowserViewModel.EXTRA_DECK_ID, deckId) - putExtra(EXTRA_CARD_ID_KEY, cardId) + putExtra(CardBrowserViewModel.EXTRA_CARD_ID_KEY, cardId) } - - companion object { - const val EXTRA_CARD_ID_KEY = "cardId" - } } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserLaunchOptions.kt b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserLaunchOptions.kt index cb1020a0ea88..f92b1c3be9cb 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserLaunchOptions.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserLaunchOptions.kt @@ -54,7 +54,7 @@ fun Intent.toCardBrowserLaunchOptions(): CardBrowserLaunchOptions? { return CardBrowserLaunchOptions.SearchQueryJs(it, getBooleanExtra("all_decks", false)) } - getLongExtra(BrowserDestination.ToCard.EXTRA_CARD_ID_KEY)?.let { cardId -> + getLongExtra(CardBrowserViewModel.EXTRA_CARD_ID_KEY)?.let { cardId -> return CardBrowserLaunchOptions.ScrollToCard(cardId) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt index d40dacd045cb..cefb6dc3b4d2 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt @@ -1437,6 +1437,9 @@ class CardBrowserViewModel( /** Intent extra carrying the [DeckId] the browser should open scoped to. */ const val EXTRA_DECK_ID = "deckId" + /** Intent extra carrying a [CardId] to auto-scroll to once the browser opens. */ + const val EXTRA_CARD_ID_KEY = "cardId" + /** Prevents one-shot extras from being re-applied after process death. */ private const val STATE_LAUNCH_INTENT_CONSUMED = "launchIntentConsumed" diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt index 69c677162d0e..abbd6260fd49 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt @@ -346,7 +346,7 @@ class ReviewerViewModel( private suspend fun emitBrowseDestination() { val deckId = withCol { decks.getCurrentId() } val cardId = currentCard.await().id - val destination = BrowserDestination.ToCard(deckId, cardId) + val destination = BrowserDestination.ScrollToCard(deckId, cardId) Timber.i("Launching 'browse options' for deck %d", deckId) destinationFlow.emit(destination) }