From 9a4e885dd5e4f3c9381cb3ff6d76d377380f8840 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Wed, 13 May 2026 15:04:49 +0200 Subject: [PATCH 01/25] Add Fire-all action to Duck.ai chat history screen Wires the toolbar fire icon to a confirmation dialog reusing SingleTabFireDialog via a new ChatHistory origin. Confirming clears all Duck.ai chats and closes any open Duck.ai tabs without touching browser tabs or stored fire preferences. --- .../com/duckduckgo/app/fire/DataClearing.kt | 29 +++- .../duckduckgo/app/fire/ManualDataClearing.kt | 9 +- .../app/global/view/SingleTabFireDialog.kt | 19 ++- .../view/SingleTabFireDialogViewModel.kt | 58 +++++-- app/src/main/res/values/donottranslate.xml | 5 + .../duckduckgo/app/fire/DataClearingTest.kt | 37 ++++ .../api/fire/FireDialogProvider.kt | 9 + .../impl/history/ChatHistoryFragment.kt | 44 ++++- .../impl/history/ChatHistoryUiState.kt | 8 +- .../impl/history/ChatHistoryViewModel.kt | 54 +++++- .../res/menu/menu_chat_history_default.xml | 2 +- .../src/main/res/values/donottranslate.xml | 1 + .../impl/history/ChatHistoryViewModelTest.kt | 158 +++++++++++++++++- 13 files changed, 398 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt index c4bcd82b680f..79cae114408c 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt @@ -140,18 +140,22 @@ class DataClearing @Inject constructor( dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.Selected(setOf(tabUrl)))) } - override suspend fun clearDataUsingManualFireOptions(shouldRestartIfRequired: Boolean, wasAppUsedSinceLastClear: Boolean) { - val options = fireDataStore.getManualClearOptions() + override suspend fun clearDataUsingManualFireOptions( + shouldRestartIfRequired: Boolean, + wasAppUsedSinceLastClear: Boolean, + options: Set?, + ) { + val effectiveOptions = options ?: fireDataStore.getManualClearOptions() performGranularClear( - options = options, + options = effectiveOptions, shouldFireDataClearPixel = true, ) clearDataAction.setAppUsedSinceLastClearFlag(wasAppUsedSinceLastClear) - val wasDuckAiChatsCleared = options.contains(FireClearOption.DUCKAI_CHATS) && + val wasDuckAiChatsCleared = effectiveOptions.contains(FireClearOption.DUCKAI_CHATS) && duckAiFeatureState.showClearDuckAIChatHistory.value - val wasDataCleared = options.contains(FireClearOption.DATA) || wasDuckAiChatsCleared + val wasDataCleared = effectiveOptions.contains(FireClearOption.DATA) || wasDuckAiChatsCleared if (shouldRestartIfRequired && wasDataCleared) { dataClearingWideEvent.finishSuccess() // If there is an open wide event, complete it before killing the process. clearDataAction.killAndRestartProcess(notifyDataCleared = false) @@ -243,12 +247,12 @@ class DataClearing @Inject constructor( ) { logcat { "Performing granular clear with options: $options" } - val shouldClearTabs = FireClearOption.TABS in options + val shouldClearAllTabs = FireClearOption.TABS in options val shouldClearData = FireClearOption.DATA in options val shouldClearDuckAiChats = FireClearOption.DUCKAI_CHATS in options && duckAiFeatureState.showClearDuckAIChatHistory.value - if (shouldClearTabs) { + if (shouldClearAllTabs) { clearDataAction.clearTabsOnly() } @@ -258,9 +262,20 @@ class DataClearing @Inject constructor( if (shouldClearDuckAiChats) { clearDataAction.clearDuckAiChatsOnly() + if (!shouldClearAllTabs) closeOnlyDuckAiTabs() dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.All)) } logcat { "Granular clear completed" } } + + private suspend fun closeOnlyDuckAiTabs() { + val duckAiTabIds = tabRepository.getTabs() + .filter { tab -> tab.url?.toUri()?.let(duckChat::isDuckChatUrl) == true } + .map { it.tabId } + if (duckAiTabIds.isNotEmpty()) { + logcat { "Closing ${duckAiTabIds.size} open Duck.ai tab(s) after chat clear" } + tabRepository.deleteTabs(duckAiTabIds) + } + } } diff --git a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt index d893bb5c83f7..a9416a5c0609 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt @@ -17,6 +17,7 @@ package com.duckduckgo.app.fire import com.duckduckgo.app.global.view.ClearDataResult +import com.duckduckgo.app.settings.clear.FireClearOption /** * Interface for manual data clearing operations triggered by user actions (e.g., Fire button). @@ -26,8 +27,14 @@ interface ManualDataClearing { * Clears data when user requests data clearing using the FireDialog. * @param shouldRestartIfRequired whether to restart the app process after clearing data, if required (when data or chats cleared) * @param wasAppUsedSinceLastClear whether the app was used since the last data clear + * @param options when non-null, drives the clear instead of the user's stored fire-data-store + * options; the store is not mutated. */ - suspend fun clearDataUsingManualFireOptions(shouldRestartIfRequired: Boolean = false, wasAppUsedSinceLastClear: Boolean = false) + suspend fun clearDataUsingManualFireOptions( + shouldRestartIfRequired: Boolean = false, + wasAppUsedSinceLastClear: Boolean = false, + options: Set? = null, + ) /** * Clears all data associated with tab: diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt index 03dd48a255e1..4a43da7bfb3c 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt @@ -72,11 +72,13 @@ private const val BOTTOM_SHEET_MAX_WIDTH_DP = 640 private const val NO_MAX_WIDTH = -1 private const val ARG_ORIGIN = "origin" private const val ARG_TAB_ID = "tabId" +private const val ARG_CHAT_HISTORY_COUNT = "chatHistoryCount" internal const val ORIGIN_BROWSER = "Browser" internal const val ORIGIN_SETTINGS = "Settings" internal const val ORIGIN_TAB_SWITCHER = "TabSwitcher" internal const val ORIGIN_DUCK_AI_CONTEXTUAL_CHAT = "DuckAiContextualChat" internal const val ORIGIN_HATCH = "Hatch" +internal const val ORIGIN_CHAT_HISTORY = "ChatHistory" @InjectWith(FragmentScope::class) class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { @@ -271,14 +273,11 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { binding.fireIcon.gone() } - val titleRes = if (state.stateData.isDuckAiTab && state.isDeleteThisTabButtonVisible) { - R.string.singleTabFireDialogTitleDuckAi - } else if (state.stateData.isDuckAiChatsSelected) { - R.string.singleTabFireDialogTitleWithChats - } else { - R.string.singleTabFireDialogTitle + binding.dialogTitle.text = when (val source = state.stateData.titleSource) { + is SingleTabFireDialogViewModel.TitleSource.Static -> requireContext().getString(source.resId) + is SingleTabFireDialogViewModel.TitleSource.Plural -> + resources.getQuantityString(source.pluralsId, source.count, source.count) } - binding.dialogTitle.text = requireContext().getString(titleRes) if (state.isDeleteThisTabButtonVisible) { binding.deleteThisTabButton.show() @@ -439,6 +438,10 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { FireDialogProvider.FireDialogOrigin.Browser } } + ORIGIN_CHAT_HISTORY -> { + val count = arguments?.getInt(ARG_CHAT_HISTORY_COUNT) ?: 0 + FireDialogProvider.FireDialogOrigin.ChatHistory(count) + } else -> FireDialogProvider.FireDialogOrigin.Browser } } @@ -449,6 +452,7 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { arguments = bundleOf( ARG_ORIGIN to origin.tag(), ARG_TAB_ID to (origin as? FireDialogProvider.FireDialogOrigin.Hatch)?.tabId, + ARG_CHAT_HISTORY_COUNT to (origin as? FireDialogProvider.FireDialogOrigin.ChatHistory)?.count, ) } } @@ -459,6 +463,7 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { FireDialogProvider.FireDialogOrigin.TabSwitcher -> ORIGIN_TAB_SWITCHER FireDialogProvider.FireDialogOrigin.DuckAiContextualChat -> ORIGIN_DUCK_AI_CONTEXTUAL_CHAT is FireDialogProvider.FireDialogOrigin.Hatch -> ORIGIN_HATCH + is FireDialogProvider.FireDialogOrigin.ChatHistory -> ORIGIN_CHAT_HISTORY } } } diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt index d09ea4af019c..2fbeb8b1265a 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt @@ -20,6 +20,7 @@ import androidx.core.net.toUri import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesViewModel +import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.api.WebViewCapabilityChecker import com.duckduckgo.app.browser.api.WebViewCapabilityChecker.WebViewCapability.DeleteBrowsingData import com.duckduckgo.app.fire.ManualDataClearing @@ -145,13 +146,14 @@ class SingleTabFireDialogViewModel @Inject constructor( withContext(dispatcherProvider.io()) { fireButtonStore.incrementFireButtonUseCount() userEventsStore.registerUserEvent(UserEventKey.FIRE_BUTTON_EXECUTED) - val clearOptions = fireDataStore.getManualClearOptions() + val fireOptionsOverride = (viewState.value as? ViewState.Loaded)?.stateData?.fireOptionsOverride + val clearOptions = fireOptionsOverride ?: fireDataStore.getManualClearOptions() dataClearingWideEvent.start( entryPoint = DataClearingWideEvent.EntryPoint.SINGLE_TAB_FIRE_DIALOG, clearOptions = clearOptions, ) try { - dataClearing.clearDataUsingManualFireOptions() + dataClearing.clearDataUsingManualFireOptions(options = fireOptionsOverride) dataClearingWideEvent.finishSuccess() } catch (e: Exception) { dataClearingWideEvent.finishFailure(e) @@ -230,28 +232,57 @@ class SingleTabFireDialogViewModel @Inject constructor( } private suspend fun mapToViewState(dialogOrigin: FireDialogOrigin): ViewState.Loaded { + // Non-tab origins skip tab/download/WebView probes and show a simplified dialog with only the "Clear all" option + val isTabAware = dialogOrigin !is FireDialogOrigin.ChatHistory + val isDuckAiChatsSelected = - fireDataStore.isManualClearOptionSelected(FireClearOption.DUCKAI_CHATS) - val isDeleteBrowsingDataSupported = webViewCapabilityChecker.isSupported(DeleteBrowsingData) - val shownCount = settingsDataStore.singleTabFireDialogShownCount - val downloads = downloadsRepository.getDownloads() - val targetTabUrl = resolveTargetTabUrl(dialogOrigin) + isTabAware && fireDataStore.isManualClearOptionSelected(FireClearOption.DUCKAI_CHATS) + val isDeleteBrowsingDataSupported = isTabAware && webViewCapabilityChecker.isSupported(DeleteBrowsingData) + val downloads = if (isTabAware) downloadsRepository.getDownloads() else emptyList() + val targetTabUrl = if (isTabAware) resolveTargetTabUrl(dialogOrigin) else null + val tabCount = if (isTabAware) tabRepository.getOpenTabCount() else 0 val isDuckAiTab = dialogOrigin == DuckAiContextualChat || - targetTabUrl?.let { duckChat.isDuckChatUrl(it.toUri()) } ?: false - val tabCount = tabRepository.getOpenTabCount() + targetTabUrl?.let { duckChat.isDuckChatUrl(it.toUri()) } == true val isFireAnimationUpdateEnabled = withContext(dispatcherProvider.io()) { brandDesignUpdateToggles.fireAnimationUpdate().isEnabled() } + val isDeleteThisTabAvailable = (isDeleteBrowsingDataSupported && dialogOrigin == Browser) || + dialogOrigin == DuckAiContextualChat || + dialogOrigin is Hatch + val shownCount = settingsDataStore.singleTabFireDialogShownCount + + val titleSource: TitleSource = when (dialogOrigin) { + is FireDialogOrigin.ChatHistory -> TitleSource.Plural( + pluralsId = R.plurals.fireDialogDeleteCountTitle, + count = dialogOrigin.count, + ) + else -> { + val titleResId = when { + isDuckAiTab && isDeleteThisTabAvailable -> R.string.singleTabFireDialogTitleDuckAi + isDuckAiChatsSelected -> R.string.singleTabFireDialogTitleWithChats + else -> R.string.singleTabFireDialogTitle + } + TitleSource.Static(titleResId) + } + } + + val fireOptionsOverride: Set? = when (dialogOrigin) { + is FireDialogOrigin.ChatHistory -> setOf(FireClearOption.DUCKAI_CHATS) + else -> null + } + return ViewState.Loaded( stateData = ViewState.Loaded.StateData( isDuckAiChatsSelected = isDuckAiChatsSelected, isSingleTabEnabled = isDeleteBrowsingDataSupported, isDuckAiTab = isDuckAiTab, tabCount = tabCount, - isSiteDataSubtitleEligible = shownCount < DIALOG_WARNING_MESSAGE_SHOWN_LIMIT, + isSiteDataSubtitleEligible = isTabAware && shownCount < DIALOG_WARNING_MESSAGE_SHOWN_LIMIT, isDownloadsSubtitleEligible = downloads.any { download -> download.downloadStatus == DownloadStatus.STARTED }, isFirePictogramVisible = settingsDataStore.fireAnimationEnabled, isFireAnimationUpdateEnabled = isFireAnimationUpdateEnabled, + titleSource = titleSource, + fireOptionsOverride = fireOptionsOverride, ), origin = dialogOrigin, ) @@ -322,10 +353,17 @@ class SingleTabFireDialogViewModel @Inject constructor( val isDownloadsSubtitleEligible: Boolean = false, val isFirePictogramVisible: Boolean = true, val isFireAnimationUpdateEnabled: Boolean = false, + val titleSource: TitleSource = TitleSource.Static(R.string.singleTabFireDialogTitle), + val fireOptionsOverride: Set? = null, ) } } + sealed class TitleSource { + data class Static(val resId: Int) : TitleSource() + data class Plural(val pluralsId: Int, val count: Int) : TitleSource() + } + sealed class Command { data object PlayAnimation : Command() data object ClearingComplete : Command() diff --git a/app/src/main/res/values/donottranslate.xml b/app/src/main/res/values/donottranslate.xml index 31c1310c8d68..1e87ecfbadf5 100644 --- a/app/src/main/res/values/donottranslate.xml +++ b/app/src/main/res/values/donottranslate.xml @@ -129,4 +129,9 @@ Inferno Classic Inferno + + + Delete %1$d chat? + Delete %1$d chats? + diff --git a/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt index 5dd31627ea2a..e79117dc09d8 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt @@ -19,6 +19,7 @@ package com.duckduckgo.app.fire import android.annotation.SuppressLint +import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 import com.duckduckgo.app.fire.store.FireDataStore import com.duckduckgo.app.fire.store.TabVisitedSitesRepository @@ -123,6 +124,7 @@ class DataClearingTest { runBlocking { whenever(mockClearDataAction.clearDataForSpecificDomains(any())).thenReturn(ClearDataResult.Success) whenever(mockFireDataStore.getManualClearOptions()).thenReturn(emptySet()) + whenever(mockTabRepository.getTabs()).thenReturn(emptyList()) } testee = DataClearing( fireDataStore = mockFireDataStore, @@ -194,6 +196,41 @@ class DataClearingTest { verify(mockClearDataAction, never()).killAndRestartProcess(any(), any(), any()) } + @Test + fun whenDuckAiChatsOnlyAndOpenDuckAiTabExists_thenCloseOnlyTheDuckAiTab() = runTest { + configureManualOptions(setOf(FireClearOption.DUCKAI_CHATS)) + val duckAiTab = TabEntity(tabId = "duck-ai", url = "https://duck.ai") + val browserTab = TabEntity(tabId = "browser", url = "https://example.com") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(duckAiTab, browserTab)) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) + + testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) + + verify(mockTabRepository).deleteTabs(listOf("duck-ai")) + } + + @Test + fun whenDuckAiChatsOnlyAndNoDuckAiTabs_thenDoNotCallDeleteTabs() = runTest { + configureManualOptions(setOf(FireClearOption.DUCKAI_CHATS)) + whenever(mockTabRepository.getTabs()).thenReturn(listOf(TabEntity(tabId = "browser", url = "https://example.com"))) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(false) + + testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) + + verify(mockTabRepository, never()).deleteTabs(any()) + } + + @Test + fun whenTabsAndDuckAiChats_thenDoNotCloseDuckAiTabsSeparately() = runTest { + configureManualOptions(setOf(FireClearOption.TABS, FireClearOption.DUCKAI_CHATS)) + + testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) + + verify(mockClearDataAction).clearTabsOnly() + verify(mockTabRepository, never()).deleteTabs(any()) + } + @Test fun whenManualClearWithAllOptions_thenClearAllAndSetFlag() = runTest { configureManualOptions(setOf(FireClearOption.TABS, FireClearOption.DATA, FireClearOption.DUCKAI_CHATS)) diff --git a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt index 1e550db3e4b4..298d541b097b 100644 --- a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt +++ b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt @@ -59,5 +59,14 @@ interface FireDialogProvider { * @property tabId The id of the tab the Hatch is offering to burn. */ data class Hatch(val tabId: String) : FireDialogOrigin() + + /** + * Chat history screen — bulk-delete confirmation. Overrides the user's stored + * fire-options with `DUCKAI_CHATS` only; browser tabs are kept, but any open + * Duck.ai chat tabs are closed alongside the chat data. + * + * @property count Number of chats shown in the title ("Delete N chats?"). + */ + data class ChatHistory(val count: Int) : FireDialogOrigin() } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index aedc36025139..4c7051930104 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -35,6 +35,8 @@ import com.duckduckgo.common.ui.view.show import com.duckduckgo.common.ui.viewbinding.viewBinding import com.duckduckgo.common.utils.FragmentViewModelFactory import com.duckduckgo.common.utils.extensions.hideKeyboard +import com.duckduckgo.dataclearing.api.fire.FireDialog +import com.duckduckgo.dataclearing.api.fire.FireDialogProvider import com.duckduckgo.di.scopes.FragmentScope import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.R @@ -42,6 +44,7 @@ import com.duckduckgo.duckchat.impl.databinding.FragmentChatHistoryBinding import com.google.android.material.snackbar.Snackbar import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import logcat.logcat import javax.inject.Inject @@ -57,6 +60,9 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { @Inject lateinit var pixel: Pixel + @Inject + lateinit var fireDialogProvider: FireDialogProvider + private val binding: FragmentChatHistoryBinding by viewBinding() private val viewModel: ChatHistoryViewModel by lazy { ViewModelProvider(this, viewModelFactory)[ChatHistoryViewModel::class.java] @@ -96,6 +102,13 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { requireActivity().onBackPressedDispatcher.addCallback(viewLifecycleOwner, onBackPressedCallback) + childFragmentManager.setFragmentResultListener(FireDialog.REQUEST_KEY, viewLifecycleOwner) { _, bundle -> + when (bundle.getString(FireDialog.RESULT_KEY_EVENT)) { + FireDialog.EVENT_ON_CLEAR_STARTED -> viewModel.onFireAllConfirmed() + FireDialog.EVENT_ON_CANCEL -> viewModel.onConfirmationCancelled() + } + } + viewModel.uiState .flowWithLifecycle(viewLifecycleOwner.lifecycle, Lifecycle.State.STARTED) .onEach(::render) @@ -108,16 +121,22 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { ChatHistoryUiState.Loading -> { binding.chatHistoryList.visibility = View.GONE binding.chatHistoryEmptyState.visibility = View.GONE + setFireActionVisible(false) } ChatHistoryUiState.Empty -> { binding.chatHistoryList.visibility = View.GONE binding.chatHistoryEmptyState.visibility = View.VISIBLE adapter.submitList(emptyList()) + setFireActionVisible(false) } is ChatHistoryUiState.Loaded -> { binding.chatHistoryList.visibility = View.VISIBLE binding.chatHistoryEmptyState.visibility = View.GONE adapter.submitList(buildEntries(state)) + // Hide when Recent is empty — the title counts Recent chats, so a Pinned-only + // state would render "Delete 0 chats?". + setFireActionVisible(state.recent.isNotEmpty()) + renderConfirmation(state.confirmation) } } } @@ -133,6 +152,27 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { } } + private fun renderConfirmation(confirmation: ChatHistoryUiState.PendingConfirmation?) { + if (confirmation == null) return + // Idempotent: skip if a dialog is already attached. + if (childFragmentManager.findFragmentByTag(FIRE_DIALOG_TAG) != null) return + + val count = when (confirmation) { + is ChatHistoryUiState.PendingConfirmation.FireAll -> confirmation.count + is ChatHistoryUiState.PendingConfirmation.DeleteSelected -> confirmation.count + } + viewLifecycleOwner.lifecycleScope.launch { + val dialog = fireDialogProvider.createFireDialog( + FireDialogProvider.FireDialogOrigin.ChatHistory(count), + ) + dialog.show(childFragmentManager, FIRE_DIALOG_TAG) + } + } + + private fun setFireActionVisible(visible: Boolean) { + binding.toolbar.menu.findItem(R.id.chat_history_action_fire)?.isVisible = visible + } + private fun onMenuItemClicked(item: MenuItem): Boolean = when (item.itemId) { R.id.chat_history_action_overflow -> { showToolbarOverflowPopup() @@ -143,7 +183,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { true } R.id.chat_history_action_fire -> { - showComingSoonSnackbar() + viewModel.onFireAllRequested() true } else -> false @@ -187,6 +227,8 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { } companion object { + private const val FIRE_DIALOG_TAG = "chat_history_fire_dialog" + fun newInstance(): ChatHistoryFragment = ChatHistoryFragment() } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt index 67cce811becf..dd0c2fc3285c 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt @@ -34,7 +34,13 @@ sealed interface ChatHistoryUiState { } sealed interface PendingConfirmation { - data object FireAll : PendingConfirmation + /** Fire-all confirmation; set when count ≥ 2 (count == 1 deletes directly). */ + data class FireAll(val count: Int) : PendingConfirmation + + /** + * Delete-selected confirmation — placeholder for future selection-mode work. No caller + * sets it yet; the current dialog path clears every Duck.ai chat and cannot scope to a subset. + */ data class DeleteSelected(val count: Int) : PendingConfirmation } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index 5fde24abd7e0..481e44156b98 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -19,28 +19,39 @@ package com.duckduckgo.duckchat.impl.history import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesViewModel +import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.di.scopes.FragmentScope import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Mode +import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.PendingConfirmation +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import logcat.logcat import javax.inject.Inject @ContributesViewModel(FragmentScope::class) class ChatHistoryViewModel @Inject constructor( private val chatHistoryRepository: ChatHistoryRepository, + @AppCoroutineScope private val appScope: CoroutineScope, ) : ViewModel() { private val searchState = MutableStateFlow(SearchState()) + private val confirmationState = MutableStateFlow(null) + + /** Cached snapshot so non-suspend action methods can read Recent without re-subscribing. */ + private var latestItems: List = emptyList() val uiState: StateFlow = combine( - chatHistoryRepository.observeChats(), + chatHistoryRepository.observeChats().onEach { latestItems = it }, searchState, + confirmationState, ::reduce, ).stateIn( scope = viewModelScope, @@ -60,8 +71,44 @@ class ChatHistoryViewModel @Inject constructor( searchState.value = SearchState() } - private fun reduce(items: List, search: SearchState): ChatHistoryUiState { - logcat { "ChatHistory: reduce ${items.size} item(s), searchActive=${search.active}" } + /** + * 0 Recent → no-op; 1 → delete directly (spares Pinned); ≥2 → confirmation dialog, + * which clears every Duck.ai chat (Pinned included) via the options-driven path. + */ + fun onFireAllRequested() { + val recent = latestItems.filter { !it.pinned } + when { + recent.isEmpty() -> { + logcat { "ChatHistory: Fire-all no-op (Recent empty)" } + } + recent.size == 1 -> { + logcat { "ChatHistory: Fire-all single-chat fast-path" } + appScope.launch { chatHistoryRepository.deleteChat(recent.single().chatId) } + } + else -> { + logcat { "ChatHistory: Fire-all confirmation requested (count=${recent.size})" } + confirmationState.value = PendingConfirmation.FireAll(count = recent.size) + } + } + } + + /** Clears the confirmation state. The dialog performs the actual deletion via the options-driven path. */ + fun onFireAllConfirmed() { + logcat { "ChatHistory: Fire-all confirmed (options-driven path handles deletion)" } + confirmationState.value = null + } + + fun onConfirmationCancelled() { + logcat { "ChatHistory: confirmation cancelled" } + confirmationState.value = null + } + + private fun reduce( + items: List, + search: SearchState, + confirmation: PendingConfirmation?, + ): ChatHistoryUiState { + logcat { "ChatHistory: reduce ${items.size} item(s), searchActive=${search.active}, confirmation=$confirmation" } if (items.isEmpty()) return ChatHistoryUiState.Empty val (pinned, recent) = items.partition { it.pinned } return Loaded( @@ -70,6 +117,7 @@ class ChatHistoryViewModel @Inject constructor( searchQuery = search.query, searchActive = search.active, mode = Mode.Default, + confirmation = confirmation, ) } diff --git a/duckchat/duckchat-impl/src/main/res/menu/menu_chat_history_default.xml b/duckchat/duckchat-impl/src/main/res/menu/menu_chat_history_default.xml index dd6f6bc55a0d..df15a2e62590 100644 --- a/duckchat/duckchat-impl/src/main/res/menu/menu_chat_history_default.xml +++ b/duckchat/duckchat-impl/src/main/res/menu/menu_chat_history_default.xml @@ -20,7 +20,7 @@ Search chats Close search Clear search + Delete chats diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index e1b9d076b091..515659fdff02 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -35,7 +35,7 @@ class ChatHistoryViewModelTest { private val source = MutableStateFlow>(emptyList()) private val repository = FakeChatHistoryRepository(source) - private val viewModel = ChatHistoryViewModel(repository) + private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope) @Test fun `initial state is Loading`() = coroutineRule.testScope.runTest { @@ -209,6 +209,144 @@ class ChatHistoryViewModelTest { } } + // --- Fire-all --- + + @Test + fun `onFireAllRequested with two or more Recent chats sets FireAll confirmation with the Recent count`() = runTest { + source.value = listOf( + item("p", pinned = true), + item("r1"), + item("r2"), + item("r3"), + ) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireAllRequested() + + val confirming = awaitItem() as Loaded + assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(count = 3), confirming.confirmation) + assertTrue(repository.deletedChatIds.isEmpty()) + } + } + + @Test + fun `onFireAllRequested with exactly one Recent chat deletes immediately without setting confirmation`() = runTest { + source.value = listOf( + item("p", pinned = true), + item("r1"), + ) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireAllRequested() + + val afterDelete = awaitItem() as Loaded + assertEquals(null, afterDelete.confirmation) + assertEquals(listOf("r1"), repository.deletedChatIds) + assertEquals(listOf("p"), afterDelete.pinned.map { it.chatId }) + assertEquals(emptyList(), afterDelete.recent.map { it.chatId }) + } + } + + @Test + fun `onFireAllRequested with no Recent chats is a no-op`() = runTest { + source.value = listOf( + item("p1", pinned = true), + item("p2", pinned = true), + ) + + viewModel.uiState.test { + awaitItem() // Loading + val initial = awaitItem() as Loaded + assertEquals(null, initial.confirmation) + + viewModel.onFireAllRequested() + + expectNoEvents() + assertTrue(repository.deletedChatIds.isEmpty()) + } + } + + @Test + fun `onFireAllConfirmed only clears the confirmation state — dialog options path handles deletion`() = runTest { + source.value = listOf( + item("p1", pinned = true), + item("p2", pinned = true), + item("r1"), + item("r2"), + item("r3"), + ) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireAllRequested() + awaitItem() // confirmation = FireAll(3) + + viewModel.onFireAllConfirmed() + + // ViewModel doesn't touch the repository — the dialog drives the deletion. + val cleared = awaitItem() as Loaded + assertEquals(null, cleared.confirmation) + assertTrue(repository.deletedChatIds.isEmpty()) + assertEquals(listOf("p1", "p2"), cleared.pinned.map { it.chatId }) + assertEquals(listOf("r1", "r2", "r3"), cleared.recent.map { it.chatId }) + } + } + + @Test + fun `onConfirmationCancelled clears the FireAll confirmation without deleting`() = runTest { + source.value = listOf( + item("r1"), + item("r2"), + ) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireAllRequested() + val confirming = awaitItem() as Loaded + assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(count = 2), confirming.confirmation) + + viewModel.onConfirmationCancelled() + + val cancelled = awaitItem() as Loaded + assertEquals(null, cancelled.confirmation) + assertTrue(repository.deletedChatIds.isEmpty()) + assertEquals(listOf("r1", "r2"), cancelled.recent.map { it.chatId }) + } + } + + @Test + fun `onFireAllConfirmed does not call the repository directly — dialog drives the deletion`() = runTest { + source.value = listOf( + item("p", pinned = true), + item("r1"), + item("r2"), + ) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireAllRequested() + awaitItem() // confirmation = FireAll(2) + viewModel.onFireAllConfirmed() + awaitItem() // confirmation cleared + } + + // ViewModel never touches the repository on the dialog path — production wipes Pinned too. + assertEquals(false, repository.deleteAllChatsCalled) + assertTrue(repository.deletedChatIds.isEmpty()) + } + /** * `stateIn(WhileSubscribed)` does not guarantee subscribers observe the `Loading` initial * value — the upstream may emit before the StateFlow can replay it. Tolerate both orderings. @@ -238,9 +376,21 @@ class ChatHistoryViewModelTest { } private class FakeChatHistoryRepository( - private val source: Flow>, + private val source: MutableStateFlow>, ) : ChatHistoryRepository { + val deletedChatIds: MutableList = mutableListOf() + var deleteAllChatsCalled: Boolean = false + private set + override fun observeChats(): Flow> = source - override suspend fun deleteChat(chatId: String) = Unit - override suspend fun deleteAllChats() = Unit + + override suspend fun deleteChat(chatId: String) { + deletedChatIds += chatId + source.value = source.value.filterNot { it.chatId == chatId } + } + + override suspend fun deleteAllChats() { + deleteAllChatsCalled = true + source.value = emptyList() + } } From 1f63b2d59334d481d26d5557a6bdc454c55381c7 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Wed, 13 May 2026 17:18:23 +0200 Subject: [PATCH 02/25] Add select mode to Duck.ai chat history screen Entered from the toolbar overflow Select entry or a row long-press, with a Select-all header below the toolbar and a brand-blue check swap on each selected row. The toolbar fire icon doubles as Delete-selected when the selection is non-empty. Chat-resume, Duck.ai open, and fire-icon routing move to the ViewModel so the Fragment no longer holds a DuckChatInternal reference. Multi-chat deletes still flow through the existing repository path with a TODO for the upcoming `ClearableData.DuckChats.Selected` pipeline. --- .../duckduckgo/duckchat/impl/RealDuckChat.kt | 11 + .../impl/history/ChatHistoryAdapter.kt | 10 + .../impl/history/ChatHistoryFragment.kt | 89 +++-- .../impl/history/ChatHistoryListEntry.kt | 3 +- .../history/ChatHistorySelectAllViewHolder.kt | 52 +++ .../impl/history/ChatHistoryUiState.kt | 9 +- .../impl/history/ChatHistoryViewHolder.kt | 15 +- .../impl/history/ChatHistoryViewModel.kt | 159 ++++++++- .../bg_chat_history_circle_accent.xml | 5 + .../drawable/ic_chat_history_unchecked_24.xml | 10 + .../layout/view_chat_history_select_all.xml | 36 ++ .../src/main/res/values/donottranslate.xml | 5 + .../impl/history/ChatHistoryViewModelTest.kt | 322 +++++++++++++++++- .../messaging/fakes/FakeDuckChatInternal.kt | 14 +- 14 files changed, 710 insertions(+), 30 deletions(-) create mode 100644 duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt create mode 100644 duckchat/duckchat-impl/src/main/res/drawable/bg_chat_history_circle_accent.xml create mode 100644 duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml create mode 100644 duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt index 0f1850573bfe..9b074f58b892 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt @@ -183,6 +183,13 @@ interface DuckChatInternal : DuckChat { */ fun openWithChatId(chatId: String) + /** + * Returns the Duck.ai URL that addresses the chat with [chatId]. Single source of truth for + * the `chatID=` URL shape — callers that need a chat-URL representation (e.g. dispatching + * `ClearableData.DuckChats.Selected(urls)`) go through this helper. + */ + fun buildChatUrl(chatId: String): String + /** * Calls onClose when a close event is emitted. */ @@ -829,6 +836,10 @@ class RealDuckChat @Inject constructor( openDuckChat(parameters = mapOf(CHAT_ID_QUERY_NAME to chatId), forceNewSession = true) } + override fun buildChatUrl(chatId: String): String { + return appendParameters(mapOf(CHAT_ID_QUERY_NAME to chatId), getDuckChatLink()) + } + override suspend fun setDefaultTogglePosition(position: DefaultTogglePosition) { duckChatFeatureRepository.setDefaultTogglePosition(position.name) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt index 8e35479b2660..928609205dd6 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt @@ -25,16 +25,19 @@ class ChatHistoryAdapter( private val onChatClicked: (ChatHistoryItem) -> Unit, private val onChatMoreClicked: (ChatHistoryItem, android.view.View) -> Unit, private val onChatLongClicked: (ChatHistoryItem) -> Boolean = { false }, + private val onSelectAllClicked: () -> Unit = {}, ) : ListAdapter(Diff) { override fun getItemViewType(position: Int): Int = when (getItem(position)) { is ChatHistoryListEntry.Header -> VIEW_TYPE_HEADER is ChatHistoryListEntry.Row -> VIEW_TYPE_ROW + is ChatHistoryListEntry.SelectAllHeader -> VIEW_TYPE_SELECT_ALL } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder = when (viewType) { VIEW_TYPE_HEADER -> ChatHistorySectionHeaderViewHolder.create(parent) VIEW_TYPE_ROW -> ChatHistoryViewHolder.create(parent) + VIEW_TYPE_SELECT_ALL -> ChatHistorySelectAllViewHolder.create(parent) else -> error("Unknown viewType=$viewType") } @@ -43,10 +46,15 @@ class ChatHistoryAdapter( is ChatHistoryListEntry.Header -> (holder as ChatHistorySectionHeaderViewHolder).bind(entry.labelRes) is ChatHistoryListEntry.Row -> (holder as ChatHistoryViewHolder).bind( item = entry.item, + selected = entry.selected, onClick = onChatClicked, onMoreClick = onChatMoreClicked, onLongClick = onChatLongClicked, ) + is ChatHistoryListEntry.SelectAllHeader -> (holder as ChatHistorySelectAllViewHolder).bind( + allSelected = entry.allSelected, + onClick = onSelectAllClicked, + ) } } @@ -56,6 +64,7 @@ class ChatHistoryAdapter( oldItem.labelRes == newItem.labelRes oldItem is ChatHistoryListEntry.Row && newItem is ChatHistoryListEntry.Row -> oldItem.item.chatId == newItem.item.chatId + oldItem is ChatHistoryListEntry.SelectAllHeader && newItem is ChatHistoryListEntry.SelectAllHeader -> true else -> false } @@ -66,5 +75,6 @@ class ChatHistoryAdapter( private companion object { const val VIEW_TYPE_HEADER = 0 const val VIEW_TYPE_ROW = 1 + const val VIEW_TYPE_SELECT_ALL = 2 } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index 4c7051930104..ff109c7d9340 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -20,6 +20,7 @@ import android.os.Bundle import android.view.MenuItem import android.view.View import androidx.activity.OnBackPressedCallback +import androidx.core.view.isVisible import androidx.lifecycle.Lifecycle import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.flowWithLifecycle @@ -38,7 +39,6 @@ import com.duckduckgo.common.utils.extensions.hideKeyboard import com.duckduckgo.dataclearing.api.fire.FireDialog import com.duckduckgo.dataclearing.api.fire.FireDialogProvider import com.duckduckgo.di.scopes.FragmentScope -import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.R import com.duckduckgo.duckchat.impl.databinding.FragmentChatHistoryBinding import com.google.android.material.snackbar.Snackbar @@ -54,9 +54,6 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { @Inject lateinit var viewModelFactory: FragmentViewModelFactory - @Inject - lateinit var duckChat: DuckChatInternal - @Inject lateinit var pixel: Pixel @@ -69,13 +66,18 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { } private val adapter = ChatHistoryAdapter( - onChatClicked = { item -> duckChat.openWithChatId(item.chatId) }, + onChatClicked = { item -> viewModel.onChatRowClicked(item.chatId) }, onChatMoreClicked = { _, anchor -> showRowPopup(anchor) }, + onChatLongClicked = { item -> viewModel.onChatRowLongClicked(item.chatId) }, + onSelectAllClicked = { viewModel.onSelectAllToggled() }, ) private val onBackPressedCallback = object : OnBackPressedCallback(enabled = false) { override fun handleOnBackPressed() { - hideSearchBar() + when { + binding.searchBar.isVisible -> hideSearchBar() + viewModel.isSelectMode() -> viewModel.onSelectModeCancelled() + } } } @@ -91,7 +93,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { binding.chatHistoryList.layoutManager = LinearLayoutManager(requireContext()) binding.chatHistoryList.adapter = adapter - binding.chatHistoryEmptyState.setOnPrimaryCtaClickListener { duckChat.openDuckChat() } + binding.chatHistoryEmptyState.setOnPrimaryCtaClickListener { viewModel.onOpenDuckAiClicked() } binding.searchBar.onAction { action -> when (action) { @@ -103,8 +105,14 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { requireActivity().onBackPressedDispatcher.addCallback(viewLifecycleOwner, onBackPressedCallback) childFragmentManager.setFragmentResultListener(FireDialog.REQUEST_KEY, viewLifecycleOwner) { _, bundle -> - when (bundle.getString(FireDialog.RESULT_KEY_EVENT)) { - FireDialog.EVENT_ON_CLEAR_STARTED -> viewModel.onFireAllConfirmed() + val event = bundle.getString(FireDialog.RESULT_KEY_EVENT) + val confirmation = (viewModel.uiState.value as? ChatHistoryUiState.Loaded)?.confirmation + when (event) { + FireDialog.EVENT_ON_CLEAR_STARTED -> when (confirmation) { + is ChatHistoryUiState.PendingConfirmation.FireAll -> viewModel.onFireAllConfirmed() + is ChatHistoryUiState.PendingConfirmation.DeleteSelected -> viewModel.onDeleteSelectedConfirmed() + null -> Unit + } FireDialog.EVENT_ON_CANCEL -> viewModel.onConfirmationCancelled() } } @@ -121,34 +129,77 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { ChatHistoryUiState.Loading -> { binding.chatHistoryList.visibility = View.GONE binding.chatHistoryEmptyState.visibility = View.GONE + applyDefaultToolbar() setFireActionVisible(false) } ChatHistoryUiState.Empty -> { binding.chatHistoryList.visibility = View.GONE binding.chatHistoryEmptyState.visibility = View.VISIBLE adapter.submitList(emptyList()) + applyDefaultToolbar() setFireActionVisible(false) } is ChatHistoryUiState.Loaded -> { binding.chatHistoryList.visibility = View.VISIBLE binding.chatHistoryEmptyState.visibility = View.GONE - adapter.submitList(buildEntries(state)) - // Hide when Recent is empty — the title counts Recent chats, so a Pinned-only - // state would render "Delete 0 chats?". - setFireActionVisible(state.recent.isNotEmpty()) + val selectMode = state.mode as? ChatHistoryUiState.Mode.Selecting + adapter.submitList(buildEntries(state, selectMode)) + if (selectMode != null) { + applySelectModeToolbar(selectMode.selectedChatIds.size) + setFireActionVisible(selectMode.selectedChatIds.isNotEmpty()) + onBackPressedCallback.isEnabled = true + } else { + applyDefaultToolbar() + // In default mode the fire icon drives Fire-all; hide when Recent is empty — + // the title counts Recent chats, so a Pinned-only state would render "Delete 0 chats?". + setFireActionVisible(state.recent.isNotEmpty()) + onBackPressedCallback.isEnabled = binding.searchBar.isVisible + } renderConfirmation(state.confirmation) } } } - private fun buildEntries(state: ChatHistoryUiState.Loaded): List = buildList { + private fun applyDefaultToolbar() { + binding.toolbar.setNavigationIcon(com.duckduckgo.mobile.android.R.drawable.ic_arrow_left_24) + binding.toolbar.setNavigationOnClickListener { requireActivity().onBackPressedDispatcher.onBackPressed() } + binding.toolbar.setTitle(R.string.duck_ai_chat_history_title) + binding.toolbar.menu.findItem(R.id.chat_history_action_search)?.isVisible = true + binding.toolbar.menu.findItem(R.id.chat_history_action_overflow)?.isVisible = true + } + + private fun applySelectModeToolbar(count: Int) { + binding.toolbar.setNavigationIcon(com.duckduckgo.mobile.android.R.drawable.ic_arrow_left_24) + binding.toolbar.navigationContentDescription = + getString(R.string.duck_ai_chat_history_exit_select_mode_content_description) + binding.toolbar.setNavigationOnClickListener { viewModel.onSelectModeCancelled() } + binding.toolbar.title = count.toString() + binding.toolbar.menu.findItem(R.id.chat_history_action_search)?.isVisible = false + binding.toolbar.menu.findItem(R.id.chat_history_action_overflow)?.isVisible = false + } + + private fun buildEntries( + state: ChatHistoryUiState.Loaded, + selectMode: ChatHistoryUiState.Mode.Selecting?, + ): List = buildList { + if (selectMode != null) { + val visibleIds = (state.pinned + state.recent).map { it.chatId }.toSet() + val allSelected = visibleIds.isNotEmpty() && selectMode.selectedChatIds == visibleIds + add(ChatHistoryListEntry.SelectAllHeader(allSelected = allSelected)) + } if (state.pinned.isNotEmpty()) { if (!state.searchActive) add(ChatHistoryListEntry.Header(R.string.duck_ai_chat_history_section_pinned)) - state.pinned.forEach { add(ChatHistoryListEntry.Row(it)) } + state.pinned.forEach { item -> + val selected = selectMode != null && item.chatId in selectMode.selectedChatIds + add(ChatHistoryListEntry.Row(item = item, selected = selected)) + } } if (state.recent.isNotEmpty()) { if (!state.searchActive) add(ChatHistoryListEntry.Header(R.string.duck_ai_chat_history_section_recent)) - state.recent.forEach { add(ChatHistoryListEntry.Row(it)) } + state.recent.forEach { item -> + val selected = selectMode != null && item.chatId in selectMode.selectedChatIds + add(ChatHistoryListEntry.Row(item = item, selected = selected)) + } } } @@ -183,7 +234,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { true } R.id.chat_history_action_fire -> { - viewModel.onFireAllRequested() + viewModel.onFireIconClicked() true } else -> false @@ -197,18 +248,18 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { } private fun hideSearchBar() { - onBackPressedCallback.isEnabled = false binding.searchBar.handle(SearchBar.Event.DismissSearchBar) requireActivity().hideKeyboard() binding.toolbar.show() viewModel.onSearchClosed() + // Leave onBackPressedCallback.isEnabled to render() — select mode may still be active. } private fun showToolbarOverflowPopup() { val anchor = binding.toolbar.findViewById(R.id.chat_history_action_overflow) ?: return val popup = PopupMenu(layoutInflater, R.layout.popup_chat_history_overflow) val view = popup.contentView - popup.onMenuItemClicked(view.findViewById(R.id.select)) { showComingSoonSnackbar() } + popup.onMenuItemClicked(view.findViewById(R.id.select)) { viewModel.onEnterSelectMode() } popup.show(binding.root, anchor) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryListEntry.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryListEntry.kt index abecd7dfd9e9..a3e72e5d56c3 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryListEntry.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryListEntry.kt @@ -20,5 +20,6 @@ import androidx.annotation.StringRes sealed interface ChatHistoryListEntry { data class Header(@StringRes val labelRes: Int) : ChatHistoryListEntry - data class Row(val item: ChatHistoryItem) : ChatHistoryListEntry + data class Row(val item: ChatHistoryItem, val selected: Boolean = false) : ChatHistoryListEntry + data class SelectAllHeader(val allSelected: Boolean) : ChatHistoryListEntry } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt new file mode 100644 index 000000000000..3b5b8ba510d3 --- /dev/null +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2026 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.duckchat.impl.history + +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import android.widget.ImageView +import androidx.recyclerview.widget.RecyclerView +import com.duckduckgo.common.ui.view.text.DaxTextView +import com.duckduckgo.duckchat.impl.R + +class ChatHistorySelectAllViewHolder( + itemView: View, +) : RecyclerView.ViewHolder(itemView) { + + private val icon: ImageView = itemView.findViewById(R.id.chatHistorySelectAllIcon) + private val label: DaxTextView = itemView.findViewById(R.id.chatHistorySelectAllLabel) + + fun bind(allSelected: Boolean, onClick: () -> Unit) { + if (allSelected) { + icon.setImageResource(com.duckduckgo.mobile.android.R.drawable.ic_check_blue_round_24) + label.setText(R.string.duck_ai_chat_history_unselect_all) + } else { + icon.setImageResource(R.drawable.ic_chat_history_unchecked_24) + label.setText(R.string.duck_ai_chat_history_select_all) + } + itemView.setOnClickListener { onClick() } + } + + companion object { + fun create(parent: ViewGroup): ChatHistorySelectAllViewHolder { + val view = LayoutInflater.from(parent.context) + .inflate(R.layout.view_chat_history_select_all, parent, false) + return ChatHistorySelectAllViewHolder(view) + } + } +} diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt index dd0c2fc3285c..fd6979163a02 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt @@ -38,9 +38,12 @@ sealed interface ChatHistoryUiState { data class FireAll(val count: Int) : PendingConfirmation /** - * Delete-selected confirmation — placeholder for future selection-mode work. No caller - * sets it yet; the current dialog path clears every Duck.ai chat and cannot scope to a subset. + * Delete-selected confirmation. `chatIds` is the captured selection snapshot at the + * moment Delete-selected was tapped — a concurrent repository emission between + * "tapped" and "confirmed" can't change what gets deleted. */ - data class DeleteSelected(val count: Int) : PendingConfirmation + data class DeleteSelected(val chatIds: Set) : PendingConfirmation { + val count: Int get() = chatIds.size + } } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt index 27e4483c3c5a..b501005da9bc 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt @@ -19,6 +19,7 @@ package com.duckduckgo.duckchat.impl.history import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import android.widget.FrameLayout import android.widget.ImageView import androidx.recyclerview.widget.RecyclerView import com.duckduckgo.common.ui.view.text.DaxTextView @@ -28,18 +29,30 @@ class ChatHistoryViewHolder( itemView: View, ) : RecyclerView.ViewHolder(itemView) { + private val iconContainer: FrameLayout = itemView.findViewById(R.id.chatHistoryIconContainer) private val typeIcon: ImageView = itemView.findViewById(R.id.chatHistoryTypeIcon) private val title: DaxTextView = itemView.findViewById(R.id.chatHistoryTitle) private val moreButton: ImageView = itemView.findViewById(R.id.chatHistoryMore) fun bind( item: ChatHistoryItem, + selected: Boolean, onClick: (ChatHistoryItem) -> Unit, onMoreClick: (ChatHistoryItem, View) -> Unit, onLongClick: (ChatHistoryItem) -> Boolean = { false }, ) { title.text = item.displayTitle - typeIcon.setImageResource(iconFor(item.type, item.pinned)) + if (selected) { + iconContainer.setBackgroundResource(R.drawable.bg_chat_history_circle_accent) + typeIcon.setImageResource(com.duckduckgo.mobile.android.R.drawable.ic_check_24) + typeIcon.setColorFilter(itemView.context.getColor(com.duckduckgo.mobile.android.R.color.white)) + iconContainer.contentDescription = itemView.context.getString(R.string.duck_ai_chat_history_row_selected_content_description) + } else { + iconContainer.setBackgroundResource(R.drawable.bg_chat_history_circle_solid) + typeIcon.setImageResource(iconFor(item.type, item.pinned)) + typeIcon.colorFilter = null + iconContainer.contentDescription = null + } itemView.setOnClickListener { onClick(item) } itemView.setOnLongClickListener { onLongClick(item) } moreButton.setOnClickListener { anchor -> onMoreClick(item, anchor) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index 481e44156b98..580de3f9ef7d 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -21,6 +21,7 @@ import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesViewModel import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.di.scopes.FragmentScope +import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Mode import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.PendingConfirmation @@ -40,18 +41,24 @@ import javax.inject.Inject class ChatHistoryViewModel @Inject constructor( private val chatHistoryRepository: ChatHistoryRepository, @AppCoroutineScope private val appScope: CoroutineScope, + private val duckChat: DuckChatInternal, ) : ViewModel() { private val searchState = MutableStateFlow(SearchState()) private val confirmationState = MutableStateFlow(null) + private val modeState = MutableStateFlow(Mode.Default) /** Cached snapshot so non-suspend action methods can read Recent without re-subscribing. */ private var latestItems: List = emptyList() val uiState: StateFlow = combine( - chatHistoryRepository.observeChats().onEach { latestItems = it }, + chatHistoryRepository.observeChats().onEach { items -> + latestItems = items + reconcileSelection(items) + }, searchState, confirmationState, + modeState, ::reduce, ).stateIn( scope = viewModelScope, @@ -59,6 +66,66 @@ class ChatHistoryViewModel @Inject constructor( initialValue = ChatHistoryUiState.Loading, ) + /** + * On every Repository emission, intersect any selection with the current item IDs so a + * concurrent omnibar Delete or single-row overflow Delete doesn't desync the selection. + */ + private fun reconcileSelection(items: List) { + val current = modeState.value + if (current is Mode.Selecting && current.selectedChatIds.isNotEmpty()) { + val knownIds = items.mapTo(mutableSetOf()) { it.chatId } + val intersected = current.selectedChatIds.intersect(knownIds) + if (intersected.size != current.selectedChatIds.size) { + modeState.value = Mode.Selecting(intersected) + } + } + } + + /** True when the screen is currently in select mode — read synchronously by the Fragment. */ + fun isSelectMode(): Boolean = modeState.value is Mode.Selecting + + /** + * Row tap. In default mode resumes the chat in Duck.ai; in select mode toggles the row in + * the current selection. Centralised here so the Fragment doesn't need to read mode state + * or hold a `DuckChatInternal` reference. + */ + fun onChatRowClicked(chatId: String) { + if (modeState.value is Mode.Selecting) { + onSelectionToggled(chatId) + } else { + duckChat.openWithChatId(chatId) + } + } + + /** + * Row long-press — power-user entry to select mode alongside the toolbar overflow Select + * action. In default mode enters select mode with the row pre-selected. In select mode + * toggles the row, matching tap behaviour. Returns true to consume the long-press event. + */ + fun onChatRowLongClicked(chatId: String): Boolean { + if (modeState.value !is Mode.Selecting) { + logcat { "ChatHistory: enter select mode via long-press (chatId=$chatId)" } + modeState.value = Mode.Selecting(setOf(chatId)) + } else { + onSelectionToggled(chatId) + } + return true + } + + /** Empty-state CTA — opens Duck.ai for a fresh chat. */ + fun onOpenDuckAiClicked() { + duckChat.openDuckChat() + } + + /** Toolbar fire icon. Routes to Delete-selected in select mode, Fire-all otherwise. */ + fun onFireIconClicked() { + if (modeState.value is Mode.Selecting) { + onDeleteSelectedRequested() + } else { + onFireAllRequested() + } + } + fun onSearchActivated() { searchState.update { it.copy(active = true) } } @@ -103,12 +170,98 @@ class ChatHistoryViewModel @Inject constructor( confirmationState.value = null } + /** Enters select mode with an empty selection. Triggered by the toolbar overflow "Select" entry. */ + fun onEnterSelectMode() { + logcat { "ChatHistory: enter select mode" } + modeState.value = Mode.Selecting(emptySet()) + } + + /** Toggles row membership inside the current selection. Empty selection stays in select mode. */ + fun onSelectionToggled(chatId: String) { + val current = modeState.value as? Mode.Selecting ?: return + val next = if (chatId in current.selectedChatIds) { + current.selectedChatIds - chatId + } else { + current.selectedChatIds + chatId + } + modeState.value = Mode.Selecting(next) + } + + /** Select-all / Unselect-all toggle in the sticky header row. */ + fun onSelectAllToggled() { + val current = modeState.value as? Mode.Selecting ?: return + val visibleIds = visibleChatIds() + val next = if (current.selectedChatIds == visibleIds) emptySet() else visibleIds + modeState.value = Mode.Selecting(next) + } + + /** Exits select mode without deleting. Back-arrow / system back. */ + fun onSelectModeCancelled() { + logcat { "ChatHistory: select mode cancelled" } + modeState.value = Mode.Default + } + + /** + * Toolbar fire icon in select mode. Branches on selection size: + * - 0 → no-op (the icon should be disabled). + * - 1 → delete the single chat directly (TODO: route through `ClearableData.DuckChats.Selected` + * once T059–T061 land; for now uses `chatHistoryRepository.deleteChat(chatId)`). + * - ≥ 2 → confirmation dialog with the captured `chatIds` snapshot. + */ + fun onDeleteSelectedRequested() { + val current = modeState.value as? Mode.Selecting ?: return + val ids = current.selectedChatIds + when { + ids.isEmpty() -> logcat { "ChatHistory: Delete-selected no-op (empty selection)" } + ids.size == 1 -> { + logcat { "ChatHistory: Delete-selected single-chat fast-path" } + // Exit select mode before launching so the reconciler doesn't race the delete. + modeState.value = Mode.Default + appScope.launch { chatHistoryRepository.deleteChat(ids.single()) } + } + else -> { + logcat { "ChatHistory: Delete-selected confirmation requested (count=${ids.size})" } + confirmationState.value = PendingConfirmation.DeleteSelected(chatIds = ids) + } + } + } + + /** + * Clears the confirmation state and exits select mode. The dialog drives the actual deletion + * via its `selectedChatUrls` plumbing (T057 — TODO: wire when the dialog change lands). + */ + fun onDeleteSelectedConfirmed() { + logcat { "ChatHistory: Delete-selected confirmed" } + // TODO(T058/T064): once the dialog dispatches via `ClearableData.DuckChats.Selected`, + // remove this fallback. For now (UI-only landing), the ViewModel iterates the captured + // snapshot directly so the feature is end-to-end functional on internal builds. + val ids = (confirmationState.value as? PendingConfirmation.DeleteSelected)?.chatIds.orEmpty() + // Exit select mode FIRST so subsequent repository emissions don't race the reconciler — + // otherwise a delete could observe `Mode.Selecting(ids)` and rewrite `mode` to + // `Selecting(empty)` before this method finishes. + confirmationState.value = null + modeState.value = Mode.Default + if (ids.isNotEmpty()) { + appScope.launch { ids.forEach { chatHistoryRepository.deleteChat(it) } } + } + } + + /** Snapshot of the IDs currently visible (respects the active search filter). */ + private fun visibleChatIds(): Set { + val search = searchState.value + return latestItems + .asSequence() + .filter { item -> !search.active || search.query.isEmpty() || item.displayTitle.contains(search.query, ignoreCase = true) } + .mapTo(mutableSetOf()) { it.chatId } + } + private fun reduce( items: List, search: SearchState, confirmation: PendingConfirmation?, + mode: Mode, ): ChatHistoryUiState { - logcat { "ChatHistory: reduce ${items.size} item(s), searchActive=${search.active}, confirmation=$confirmation" } + logcat { "ChatHistory: reduce ${items.size} item(s), searchActive=${search.active}, confirmation=$confirmation, mode=$mode" } if (items.isEmpty()) return ChatHistoryUiState.Empty val (pinned, recent) = items.partition { it.pinned } return Loaded( @@ -116,7 +269,7 @@ class ChatHistoryViewModel @Inject constructor( recent = recent.sortedByDate().filterBy(search), searchQuery = search.query, searchActive = search.active, - mode = Mode.Default, + mode = mode, confirmation = confirmation, ) } diff --git a/duckchat/duckchat-impl/src/main/res/drawable/bg_chat_history_circle_accent.xml b/duckchat/duckchat-impl/src/main/res/drawable/bg_chat_history_circle_accent.xml new file mode 100644 index 000000000000..154beec21816 --- /dev/null +++ b/duckchat/duckchat-impl/src/main/res/drawable/bg_chat_history_circle_accent.xml @@ -0,0 +1,5 @@ + + + + diff --git a/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml b/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml new file mode 100644 index 000000000000..82bbfedaa16e --- /dev/null +++ b/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml @@ -0,0 +1,10 @@ + + + diff --git a/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml b/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml new file mode 100644 index 000000000000..ffa1059b14d6 --- /dev/null +++ b/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml @@ -0,0 +1,36 @@ + + + + + + + + diff --git a/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml b/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml index a8f992497285..e0adc3c6b1cf 100644 --- a/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml +++ b/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml @@ -97,4 +97,9 @@ Close search Clear search Delete chats + Select all + Unselect all + Selected + Not selected + Exit selection mode diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 515659fdff02..df5950a1f47e 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -20,6 +20,7 @@ import app.cash.turbine.TurbineTestContext import app.cash.turbine.test import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded +import com.duckduckgo.duckchat.impl.messaging.fakes.FakeDuckChatInternal import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest @@ -35,7 +36,8 @@ class ChatHistoryViewModelTest { private val source = MutableStateFlow>(emptyList()) private val repository = FakeChatHistoryRepository(source) - private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope) + private val duckChat = FakeDuckChatInternal() + private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope, duckChat) @Test fun `initial state is Loading`() = coroutineRule.testScope.runTest { @@ -347,6 +349,324 @@ class ChatHistoryViewModelTest { assertTrue(repository.deletedChatIds.isEmpty()) } + // --- Chat resume / Duck.ai open (centralised on the ViewModel) --- + + @Test + fun `onChatRowClicked in default mode resumes the chat in DuckAi`() = runTest { + viewModel.onChatRowClicked("abc") + + assertEquals(listOf("abc"), duckChat.openWithChatIdCalls) + } + + @Test + fun `onChatRowLongClicked in default mode enters select mode with the row pre-selected`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded (Default) + + val consumed = viewModel.onChatRowLongClicked("a") + + assertTrue(consumed) + val loaded = awaitItem() as Loaded + val mode = loaded.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("a"), mode.selectedChatIds) + assertTrue(duckChat.openWithChatIdCalls.isEmpty()) + } + } + + @Test + fun `onChatRowLongClicked in select mode toggles the row like a tap`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onChatRowLongClicked("a") + awaitItem() // Selecting({a}) + + val consumed = viewModel.onChatRowLongClicked("b") + + assertTrue(consumed) + val loaded = awaitItem() as Loaded + val mode = loaded.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("a", "b"), mode.selectedChatIds) + } + } + + @Test + fun `onChatRowClicked in select mode toggles selection instead of opening DuckAi`() = runTest { + source.value = listOf(item("a")) + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + + viewModel.onChatRowClicked("a") + + val loaded = awaitItem() as Loaded + val mode = loaded.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("a"), mode.selectedChatIds) + assertTrue(duckChat.openWithChatIdCalls.isEmpty()) + } + } + + @Test + fun `onOpenDuckAiClicked delegates to DuckChat`() = runTest { + viewModel.onOpenDuckAiClicked() + + assertEquals(1, duckChat.openDuckChatCalls) + } + + @Test + fun `onFireIconClicked in default mode triggers Fire-all`() = runTest { + source.value = listOf(item("r1"), item("r2")) + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireIconClicked() + + val confirming = awaitItem() as Loaded + assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(count = 2), confirming.confirmation) + } + } + + @Test + fun `onFireIconClicked in select mode triggers Delete-selected`() = runTest { + source.value = listOf(item("a"), item("b"), item("c")) + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectionToggled("a") + awaitItem() + viewModel.onSelectionToggled("b") + awaitItem() + + viewModel.onFireIconClicked() + + val confirming = awaitItem() as Loaded + val confirmation = confirming.confirmation as ChatHistoryUiState.PendingConfirmation.DeleteSelected + assertEquals(setOf("a", "b"), confirmation.chatIds) + } + } + + // --- Select-mode (FR-013, FR-013a) --- + + @Test + fun `onEnterSelectMode transitions to Selecting with empty selection`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded (Default) + + viewModel.onEnterSelectMode() + + val loaded = awaitItem() as Loaded + val mode = loaded.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(emptySet(), mode.selectedChatIds) + } + } + + @Test + fun `onSelectionToggled adds and removes ids and empty selection stays in select mode`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + + viewModel.onSelectionToggled("a") + val afterAdd = awaitItem() as Loaded + assertEquals(setOf("a"), (afterAdd.mode as ChatHistoryUiState.Mode.Selecting).selectedChatIds) + + viewModel.onSelectionToggled("a") + val afterRemove = awaitItem() as Loaded + val mode = afterRemove.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(emptySet(), mode.selectedChatIds) + } + } + + @Test + fun `onSelectAllToggled fills the selection with every visible chat id`() = runTest { + source.value = listOf(item("p", pinned = true), item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + + viewModel.onSelectAllToggled() + + val loaded = awaitItem() as Loaded + val mode = loaded.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("p", "a", "b"), mode.selectedChatIds) + } + } + + @Test + fun `onSelectAllToggled with everything selected clears the selection but stays in select mode`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + viewModel.onSelectAllToggled() + awaitItem() // Selecting({a, b}) + + viewModel.onSelectAllToggled() + + val cleared = awaitItem() as Loaded + val mode = cleared.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(emptySet(), mode.selectedChatIds) + } + } + + @Test + fun `onSelectModeCancelled returns to Default mode with no deletion`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectionToggled("a") + awaitItem() + + viewModel.onSelectModeCancelled() + + val cancelled = awaitItem() as Loaded + assertEquals(ChatHistoryUiState.Mode.Default, cancelled.mode) + assertTrue(repository.deletedChatIds.isEmpty()) + } + } + + @Test + fun `onDeleteSelectedRequested with empty selection is a no-op`() = runTest { + source.value = listOf(item("a")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + + viewModel.onDeleteSelectedRequested() + + expectNoEvents() + assertTrue(repository.deletedChatIds.isEmpty()) + } + } + + @Test + fun `onDeleteSelectedRequested with one selected deletes directly and exits select mode`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectionToggled("a") + awaitItem() + + viewModel.onDeleteSelectedRequested() + cancelAndConsumeRemainingEvents() + } + // After the runTest scheduler drains: mode reset to Default + delete propagated. + assertEquals(listOf("a"), repository.deletedChatIds) + val finalState = viewModel.uiState.value as Loaded + assertEquals(ChatHistoryUiState.Mode.Default, finalState.mode) + assertEquals(listOf("b"), finalState.recent.map { it.chatId }) + } + + @Test + fun `onDeleteSelectedRequested with two or more sets DeleteSelected with the captured ids`() = runTest { + source.value = listOf(item("a"), item("b"), item("c")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectionToggled("a") + awaitItem() + viewModel.onSelectionToggled("b") + awaitItem() + + viewModel.onDeleteSelectedRequested() + + val confirming = awaitItem() as Loaded + val confirmation = confirming.confirmation as ChatHistoryUiState.PendingConfirmation.DeleteSelected + assertEquals(setOf("a", "b"), confirmation.chatIds) + assertEquals(2, confirmation.count) + assertTrue(repository.deletedChatIds.isEmpty()) + } + } + + @Test + fun `onDeleteSelectedConfirmed deletes the captured ids and returns to Default mode`() = runTest { + source.value = listOf(item("p", pinned = true), item("a"), item("b"), item("c")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectionToggled("a") + awaitItem() + viewModel.onSelectionToggled("c") + awaitItem() + viewModel.onDeleteSelectedRequested() + awaitItem() // DeleteSelected({a, c}) + + viewModel.onDeleteSelectedConfirmed() + + // Drain all subsequent emissions until the list reflects both deletes — state-flow + // batching can fold the mode + confirmation + per-delete emissions in any order. + cancelAndConsumeRemainingEvents() + } + // After the runTest scheduler drains all coroutines: + assertEquals(setOf("a", "c"), repository.deletedChatIds.toSet()) + val finalState = viewModel.uiState.value as Loaded + assertEquals(ChatHistoryUiState.Mode.Default, finalState.mode) + assertEquals(null, finalState.confirmation) + assertEquals(listOf("p"), finalState.pinned.map { it.chatId }) + assertEquals(listOf("b"), finalState.recent.map { it.chatId }) + } + + @Test + fun `concurrent repository emission removing a selected id reconciles the selection`() = runTest { + source.value = listOf(item("a"), item("b"), item("c")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectAllToggled() + awaitItem() // Selecting({a, b, c}) + + // Simulate an external delete (e.g. omnibar Fire-icon) removing chat "b" + source.value = listOf(item("a"), item("c")) + + val reconciled = awaitItem() as Loaded + val mode = reconciled.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("a", "c"), mode.selectedChatIds) + } + } + /** * `stateIn(WhileSubscribed)` does not guarantee subscribers observe the `Loading` initial * value — the upstream may emit before the StateFlow can replay it. Tolerate both orderings. diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt index 7c0e5fbea8a6..0b720d588cb3 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt @@ -56,7 +56,13 @@ class FakeDuckChatInternal( // DuckChat interface methods override fun isEnabled(): Boolean = enabled - override fun openDuckChat() { } + val openWithChatIdCalls: MutableList = mutableListOf() + var openDuckChatCalls: Int = 0 + private set + + override fun openDuckChat() { + openDuckChatCalls += 1 + } override fun openDuckChatWithAutoPrompt(query: String) { } @@ -189,7 +195,11 @@ class FakeDuckChatInternal( override suspend fun isChatHistoryAvailable(): Boolean = false - override fun openWithChatId(chatId: String) { } + override fun openWithChatId(chatId: String) { + openWithChatIdCalls += chatId + } + + override fun buildChatUrl(chatId: String): String = "https://duck.ai?chatID=$chatId" private val _defaultTogglePosition = MutableStateFlow(null) From 0e3027d8391bbb35b64bc1e35a6b0bb8559d3fd7 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Thu, 14 May 2026 08:09:41 +0200 Subject: [PATCH 03/25] Wire chat-history Delete-selected through the data-clearing plugin chain Adds a `ClearableData.DuckChats.Selected(chatUrls)` variant and a dedicated `ManualDataClearing.clearSelectedDuckAiChats` entry point so chat-history Delete-selected (and the N=1 Fire-all fast-path) clear only the targeted chats and their open tabs, with sync recording batched per user action. The chat-history origin carries the URL subset on `FireDialogOrigin.ChatHistory` so the dialog picks the surgical path on its own. Tab cleanup that used to live inline in `performGranularClear` moves to a new `DuckAiTabsCleanupPlugin` that handles both All and Selected dispatches. --- .../com/duckduckgo/app/fire/DataClearing.kt | 22 +-- .../app/fire/DuckAiTabsCleanupPlugin.kt | 66 +++++++++ .../duckduckgo/app/fire/ManualDataClearing.kt | 8 +- .../app/global/view/SingleTabFireDialog.kt | 8 +- .../view/SingleTabFireDialogViewModel.kt | 9 +- .../duckduckgo/app/fire/DataClearingTest.kt | 82 +++++++---- .../app/fire/DuckAiTabsCleanupPluginTest.kt | 137 ++++++++++++++++++ .../api/fire/FireDialogProvider.kt | 15 +- .../duckduckgo/duckchat/impl/RealDuckChat.kt | 6 +- .../impl/history/ChatHistoryFragment.kt | 13 +- .../impl/history/ChatHistoryUiState.kt | 6 +- .../impl/history/ChatHistoryViewModel.kt | 104 ++++--------- .../DuckChatDataClearingPluginTest.kt | 53 +++++++ .../impl/history/ChatHistoryViewModelTest.kt | 100 +++++++++---- 14 files changed, 454 insertions(+), 175 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt create mode 100644 app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt index 79cae114408c..b2090ac29ea7 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt @@ -237,10 +237,12 @@ class DataClearing @Inject constructor( return fireDataStore.getAutomaticClearOptions().isNotEmpty() } - /** - * Performs granular data clearing based on the provided options - * @return true if process needs to be restarted - */ + override suspend fun clearSelectedDuckAiChats(chatUrls: Set) { + if (chatUrls.isEmpty()) return + if (!duckAiFeatureState.showClearDuckAIChatHistory.value) return + dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.Selected(chatUrls))) + } + private suspend fun performGranularClear( options: Set, shouldFireDataClearPixel: Boolean, @@ -262,20 +264,10 @@ class DataClearing @Inject constructor( if (shouldClearDuckAiChats) { clearDataAction.clearDuckAiChatsOnly() - if (!shouldClearAllTabs) closeOnlyDuckAiTabs() + // DuckAiTabsCleanupPlugin closes any open Duck.ai tabs via the All dispatch. dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.All)) } logcat { "Granular clear completed" } } - - private suspend fun closeOnlyDuckAiTabs() { - val duckAiTabIds = tabRepository.getTabs() - .filter { tab -> tab.url?.toUri()?.let(duckChat::isDuckChatUrl) == true } - .map { it.tabId } - if (duckAiTabIds.isNotEmpty()) { - logcat { "Closing ${duckAiTabIds.size} open Duck.ai tab(s) after chat clear" } - tabRepository.deleteTabs(duckAiTabIds) - } - } } diff --git a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt new file mode 100644 index 000000000000..4fd6a7a901d0 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2026 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.fire + +import androidx.core.net.toUri +import com.duckduckgo.app.tabs.model.TabRepository +import com.duckduckgo.dataclearing.api.plugin.ClearableData +import com.duckduckgo.dataclearing.api.plugin.DataClearingPlugin +import com.duckduckgo.di.scopes.AppScope +import com.duckduckgo.duckchat.api.DuckChat +import com.squareup.anvil.annotations.ContributesMultibinding +import logcat.logcat +import javax.inject.Inject + +/** Closes Duck.ai tabs left pointing at cleared chat URLs. Never touches non-Duck.ai tabs. */ +@ContributesMultibinding(AppScope::class) +class DuckAiTabsCleanupPlugin @Inject constructor( + private val tabRepository: TabRepository, + private val duckChat: DuckChat, +) : DataClearingPlugin { + + override suspend fun onClearData(types: Set) { + types.forEach { type -> + when (type) { + is ClearableData.DuckChats.All -> closeAllDuckAiTabs() + is ClearableData.DuckChats.Selected -> closeTabsMatching(type.chatUrls) + else -> { /* not handled by this plugin */ } + } + } + } + + private suspend fun closeAllDuckAiTabs() { + val ids = tabRepository.getTabs() + .filter { tab -> tab.url?.toUri()?.let(duckChat::isDuckChatUrl) == true } + .map { it.tabId } + if (ids.isNotEmpty()) { + logcat { "Closing ${ids.size} open Duck.ai tab(s) after chat clear" } + tabRepository.deleteTabs(ids) + } + } + + private suspend fun closeTabsMatching(chatUrls: Set) { + if (chatUrls.isEmpty()) return + val ids = tabRepository.getTabs() + .filter { it.url in chatUrls } + .map { it.tabId } + if (ids.isNotEmpty()) { + logcat { "Closing ${ids.size} Duck.ai tab(s) matching the cleared subset" } + tabRepository.deleteTabs(ids) + } + } +} diff --git a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt index a9416a5c0609..65080aa807be 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt @@ -25,10 +25,7 @@ import com.duckduckgo.app.settings.clear.FireClearOption interface ManualDataClearing { /** * Clears data when user requests data clearing using the FireDialog. - * @param shouldRestartIfRequired whether to restart the app process after clearing data, if required (when data or chats cleared) - * @param wasAppUsedSinceLastClear whether the app was used since the last data clear - * @param options when non-null, drives the clear instead of the user's stored fire-data-store - * options; the store is not mutated. + * @param options when non-null, drives the clear instead of the user's stored options. */ suspend fun clearDataUsingManualFireOptions( shouldRestartIfRequired: Boolean = false, @@ -36,6 +33,9 @@ interface ManualDataClearing { options: Set? = null, ) + /** Deletes only the chats addressed by [chatUrls] and closes any browser tabs pointing at them. */ + suspend fun clearSelectedDuckAiChats(chatUrls: Set) + /** * Clears all data associated with tab: * site browsing data (via WebStorageCompat), tab-specific history, diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt index 4a43da7bfb3c..93eb69b39769 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt @@ -73,6 +73,7 @@ private const val NO_MAX_WIDTH = -1 private const val ARG_ORIGIN = "origin" private const val ARG_TAB_ID = "tabId" private const val ARG_CHAT_HISTORY_COUNT = "chatHistoryCount" +private const val ARG_SELECTED_CHAT_URLS = "selectedChatUrls" internal const val ORIGIN_BROWSER = "Browser" internal const val ORIGIN_SETTINGS = "Settings" internal const val ORIGIN_TAB_SWITCHER = "TabSwitcher" @@ -440,7 +441,10 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { } ORIGIN_CHAT_HISTORY -> { val count = arguments?.getInt(ARG_CHAT_HISTORY_COUNT) ?: 0 - FireDialogProvider.FireDialogOrigin.ChatHistory(count) + val urls = arguments?.getStringArrayList(ARG_SELECTED_CHAT_URLS) + ?.toSet() + ?.takeIf { it.isNotEmpty() } + FireDialogProvider.FireDialogOrigin.ChatHistory(count = count, selectedChatUrls = urls) } else -> FireDialogProvider.FireDialogOrigin.Browser } @@ -453,6 +457,8 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { ARG_ORIGIN to origin.tag(), ARG_TAB_ID to (origin as? FireDialogProvider.FireDialogOrigin.Hatch)?.tabId, ARG_CHAT_HISTORY_COUNT to (origin as? FireDialogProvider.FireDialogOrigin.ChatHistory)?.count, + ARG_SELECTED_CHAT_URLS to (origin as? FireDialogProvider.FireDialogOrigin.ChatHistory) + ?.selectedChatUrls?.let { ArrayList(it) }, ) } } diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt index 2fbeb8b1265a..51b0226aadaa 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt @@ -146,6 +146,7 @@ class SingleTabFireDialogViewModel @Inject constructor( withContext(dispatcherProvider.io()) { fireButtonStore.incrementFireButtonUseCount() userEventsStore.registerUserEvent(UserEventKey.FIRE_BUTTON_EXECUTED) + val selectedChatUrls = (origin.value as? FireDialogOrigin.ChatHistory)?.selectedChatUrls val fireOptionsOverride = (viewState.value as? ViewState.Loaded)?.stateData?.fireOptionsOverride val clearOptions = fireOptionsOverride ?: fireDataStore.getManualClearOptions() dataClearingWideEvent.start( @@ -153,7 +154,11 @@ class SingleTabFireDialogViewModel @Inject constructor( clearOptions = clearOptions, ) try { - dataClearing.clearDataUsingManualFireOptions(options = fireOptionsOverride) + if (selectedChatUrls != null) { + dataClearing.clearSelectedDuckAiChats(selectedChatUrls) + } else { + dataClearing.clearDataUsingManualFireOptions(options = fireOptionsOverride) + } dataClearingWideEvent.finishSuccess() } catch (e: Exception) { dataClearingWideEvent.finishFailure(e) @@ -232,7 +237,7 @@ class SingleTabFireDialogViewModel @Inject constructor( } private suspend fun mapToViewState(dialogOrigin: FireDialogOrigin): ViewState.Loaded { - // Non-tab origins skip tab/download/WebView probes and show a simplified dialog with only the "Clear all" option + // Non-tab origins skip the tab/download/WebView probes — there's no tab to reason about. val isTabAware = dialogOrigin !is FireDialogOrigin.ChatHistory val isDuckAiChatsSelected = diff --git a/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt index e79117dc09d8..6935239dee87 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt @@ -19,7 +19,6 @@ package com.duckduckgo.app.fire import android.annotation.SuppressLint -import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 import com.duckduckgo.app.fire.store.FireDataStore import com.duckduckgo.app.fire.store.TabVisitedSitesRepository @@ -197,37 +196,13 @@ class DataClearingTest { } @Test - fun whenDuckAiChatsOnlyAndOpenDuckAiTabExists_thenCloseOnlyTheDuckAiTab() = runTest { + fun whenManualClearWithDuckAiChats_thenDispatchDuckChatsAllViaTrigger() = runTest { configureManualOptions(setOf(FireClearOption.DUCKAI_CHATS)) - val duckAiTab = TabEntity(tabId = "duck-ai", url = "https://duck.ai") - val browserTab = TabEntity(tabId = "browser", url = "https://example.com") - whenever(mockTabRepository.getTabs()).thenReturn(listOf(duckAiTab, browserTab)) - whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai".toUri()))).thenReturn(true) - whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) - verify(mockTabRepository).deleteTabs(listOf("duck-ai")) - } - - @Test - fun whenDuckAiChatsOnlyAndNoDuckAiTabs_thenDoNotCallDeleteTabs() = runTest { - configureManualOptions(setOf(FireClearOption.DUCKAI_CHATS)) - whenever(mockTabRepository.getTabs()).thenReturn(listOf(TabEntity(tabId = "browser", url = "https://example.com"))) - whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(false) - - testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) - - verify(mockTabRepository, never()).deleteTabs(any()) - } - - @Test - fun whenTabsAndDuckAiChats_thenDoNotCloseDuckAiTabsSeparately() = runTest { - configureManualOptions(setOf(FireClearOption.TABS, FireClearOption.DUCKAI_CHATS)) - - testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) - - verify(mockClearDataAction).clearTabsOnly() + verify(mockClearDataAction).clearDuckAiChatsOnly() + verify(mockDataClearingTrigger).clearData(eq(setOf(ClearableData.DuckChats.All))) verify(mockTabRepository, never()).deleteTabs(any()) } @@ -1046,6 +1021,57 @@ class DataClearingTest { verify(mockNavigationHistory).removeHistoryForTab("tab1") } + // --- clearSelectedDuckAiChats --- + + @Test + fun whenClearSelectedDuckAiChatsWithUrls_thenDispatchSelectedViaTrigger() = runTest { + val urls = setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b") + + testee.clearSelectedDuckAiChats(urls) + + verify(mockDataClearingTrigger).clearData(eq(setOf(ClearableData.DuckChats.Selected(urls)))) + } + + @Test + fun whenClearSelectedDuckAiChats_thenDoNotWipeDuckAiWebStorage() = runTest { + testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) + + verify(mockClearDataAction, never()).clearDuckAiChatsOnly() + } + + @Test + fun whenClearSelectedDuckAiChats_thenDoNotTouchTabsOrBrowserData() = runTest { + testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) + + verify(mockClearDataAction, never()).clearTabsOnly() + verify(mockClearDataAction, never()).clearBrowserDataOnly(any()) + verify(mockTabRepository, never()).deleteTabs(any()) + } + + @Test + fun whenClearSelectedDuckAiChats_thenDoNotTouchFireButtonOrchestrationFlags() = runTest { + testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) + + verify(mockClearDataAction, never()).setAppUsedSinceLastClearFlag(any()) + verify(mockClearDataAction, never()).killAndRestartProcess(any(), any(), any()) + } + + @Test + fun whenClearSelectedDuckAiChatsWithEmptySet_thenNoOp() = runTest { + testee.clearSelectedDuckAiChats(emptySet()) + + verify(mockDataClearingTrigger, never()).clearData(any()) + } + + @Test + fun whenClearSelectedDuckAiChatsAndFeatureFlagOff_thenNoOp() = runTest { + showClearDuckAIChatHistoryFlow.value = false + + testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) + + verify(mockDataClearingTrigger, never()).clearData(any()) + } + private suspend fun configureManualOptions(options: Set) { whenever(mockFireDataStore.getManualClearOptions()).thenReturn(options) } diff --git a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt new file mode 100644 index 000000000000..6a8e39b4af33 --- /dev/null +++ b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2026 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.fire + +import androidx.core.net.toUri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.duckduckgo.app.tabs.model.TabEntity +import com.duckduckgo.app.tabs.model.TabRepository +import com.duckduckgo.dataclearing.api.plugin.ClearableData +import com.duckduckgo.duckchat.api.DuckChat +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +@RunWith(AndroidJUnit4::class) +class DuckAiTabsCleanupPluginTest { + + @Mock + private lateinit var mockTabRepository: TabRepository + + @Mock + private lateinit var mockDuckChat: DuckChat + + private lateinit var testee: DuckAiTabsCleanupPlugin + + @Before + fun setup() { + MockitoAnnotations.openMocks(this) + runBlocking { + whenever(mockTabRepository.getTabs()).thenReturn(emptyList()) + } + testee = DuckAiTabsCleanupPlugin(mockTabRepository, mockDuckChat) + } + + @Test + fun whenDuckChatsAllAndDuckAiTabOpen_thenCloseOnlyDuckAiTab() = runTest { + val duckAiTab = TabEntity(tabId = "duck-ai", url = "https://duck.ai") + val browserTab = TabEntity(tabId = "browser", url = "https://example.com") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(duckAiTab, browserTab)) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) + + testee.onClearData(setOf(ClearableData.DuckChats.All)) + + verify(mockTabRepository).deleteTabs(listOf("duck-ai")) + } + + @Test + fun whenDuckChatsAllAndNoDuckAiTabs_thenDoNotCallDeleteTabs() = runTest { + whenever(mockTabRepository.getTabs()).thenReturn(listOf(TabEntity(tabId = "browser", url = "https://example.com"))) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(false) + + testee.onClearData(setOf(ClearableData.DuckChats.All)) + + verify(mockTabRepository, never()).deleteTabs(any()) + } + + @Test + fun whenDuckChatsAllAndNoOpenTabs_thenNoOp() = runTest { + testee.onClearData(setOf(ClearableData.DuckChats.All)) + + verify(mockTabRepository, never()).deleteTabs(any()) + } + + @Test + fun whenDuckChatsSelectedAndTabUrlMatches_thenCloseMatchingTabOnly() = runTest { + val matchingTab = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") + val unrelatedDuckAiTab = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=zzz") + val browserTab = TabEntity(tabId = "tab3", url = "https://example.com") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(matchingTab, unrelatedDuckAiTab, browserTab)) + + testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) + + verify(mockTabRepository).deleteTabs(listOf("tab1")) + } + + @Test + fun whenDuckChatsSelectedAndMultipleUrlsMatch_thenCloseAllMatching() = runTest { + val t1 = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=a") + val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=b") + val t3 = TabEntity(tabId = "tab3", url = "https://example.com") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) + + testee.onClearData( + setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b"))), + ) + + verify(mockTabRepository).deleteTabs(listOf("tab1", "tab2")) + } + + @Test + fun whenDuckChatsSelectedAndNoTabMatches_thenDoNotCallDeleteTabs() = runTest { + val unrelated = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=other") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(unrelated)) + + testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) + + verify(mockTabRepository, never()).deleteTabs(any()) + } + + @Test + fun whenDuckChatsSelectedWithEmptyUrlSet_thenNoOp() = runTest { + testee.onClearData(setOf(ClearableData.DuckChats.Selected(emptySet()))) + + verify(mockTabRepository, never()).deleteTabs(any()) + } + + @Test + fun whenUnrelatedClearableData_thenNoOp() = runTest { + testee.onClearData(setOf(ClearableData.BrowserData.All, ClearableData.Tabs.All)) + + verify(mockTabRepository, never()).deleteTabs(any()) + } +} diff --git a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt index 298d541b097b..4a4dc4a62de5 100644 --- a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt +++ b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt @@ -28,9 +28,7 @@ interface FireDialogProvider { /** * Creates a Fire dialog instance. * - * @param origin Where the dialog is being launched from. Determines which buttons - * are shown and which tab(s) the dialog operates on. - * @return A FireDialog (Granular or NonGranular variant, selected by remote feature flag). + * @param origin Where the dialog is being launched from; also carries any per-call destructive scope. */ suspend fun createFireDialog(origin: FireDialogOrigin): FireDialog @@ -61,12 +59,15 @@ interface FireDialogProvider { data class Hatch(val tabId: String) : FireDialogOrigin() /** - * Chat history screen — bulk-delete confirmation. Overrides the user's stored - * fire-options with `DUCKAI_CHATS` only; browser tabs are kept, but any open - * Duck.ai chat tabs are closed alongside the chat data. + * Chat history screen — bulk-delete confirmation. * * @property count Number of chats shown in the title ("Delete N chats?"). + * @property selectedChatUrls Non-null scopes the clear to this subset (Delete-selected); + * `null` clears every Duck.ai chat (Fire-all). */ - data class ChatHistory(val count: Int) : FireDialogOrigin() + data class ChatHistory( + val count: Int, + val selectedChatUrls: Set? = null, + ) : FireDialogOrigin() } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt index 9b074f58b892..f62dfa388b0c 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt @@ -183,11 +183,7 @@ interface DuckChatInternal : DuckChat { */ fun openWithChatId(chatId: String) - /** - * Returns the Duck.ai URL that addresses the chat with [chatId]. Single source of truth for - * the `chatID=` URL shape — callers that need a chat-URL representation (e.g. dispatching - * `ClearableData.DuckChats.Selected(urls)`) go through this helper. - */ + /** Single source of truth for the Duck.ai chat URL shape. */ fun buildChatUrl(chatId: String): String /** diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index ff109c7d9340..dcfa24e3cdca 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -150,8 +150,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { onBackPressedCallback.isEnabled = true } else { applyDefaultToolbar() - // In default mode the fire icon drives Fire-all; hide when Recent is empty — - // the title counts Recent chats, so a Pinned-only state would render "Delete 0 chats?". + // Hide fire when Recent is empty — title is "Delete N chats?" of the Recent count. setFireActionVisible(state.recent.isNotEmpty()) onBackPressedCallback.isEnabled = binding.searchBar.isVisible } @@ -205,16 +204,20 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { private fun renderConfirmation(confirmation: ChatHistoryUiState.PendingConfirmation?) { if (confirmation == null) return - // Idempotent: skip if a dialog is already attached. if (childFragmentManager.findFragmentByTag(FIRE_DIALOG_TAG) != null) return val count = when (confirmation) { is ChatHistoryUiState.PendingConfirmation.FireAll -> confirmation.count is ChatHistoryUiState.PendingConfirmation.DeleteSelected -> confirmation.count } + val selectedChatUrls = if (confirmation is ChatHistoryUiState.PendingConfirmation.DeleteSelected) { + viewModel.chatUrlsForDialog() + } else { + null + } viewLifecycleOwner.lifecycleScope.launch { val dialog = fireDialogProvider.createFireDialog( - FireDialogProvider.FireDialogOrigin.ChatHistory(count), + FireDialogProvider.FireDialogOrigin.ChatHistory(count = count, selectedChatUrls = selectedChatUrls), ) dialog.show(childFragmentManager, FIRE_DIALOG_TAG) } @@ -252,7 +255,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { requireActivity().hideKeyboard() binding.toolbar.show() viewModel.onSearchClosed() - // Leave onBackPressedCallback.isEnabled to render() — select mode may still be active. + // onBackPressedCallback.isEnabled is reset by render() — select mode may still be active. } private fun showToolbarOverflowPopup() { diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt index fd6979163a02..1a7964b15a40 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt @@ -37,11 +37,7 @@ sealed interface ChatHistoryUiState { /** Fire-all confirmation; set when count ≥ 2 (count == 1 deletes directly). */ data class FireAll(val count: Int) : PendingConfirmation - /** - * Delete-selected confirmation. `chatIds` is the captured selection snapshot at the - * moment Delete-selected was tapped — a concurrent repository emission between - * "tapped" and "confirmed" can't change what gets deleted. - */ + /** Captured selection snapshot — concurrent emissions can't change the deletion target. */ data class DeleteSelected(val chatIds: Set) : PendingConfirmation { val count: Int get() = chatIds.size } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index 580de3f9ef7d..c35a7e1a0a10 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -20,6 +20,8 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesViewModel import com.duckduckgo.app.di.AppCoroutineScope +import com.duckduckgo.dataclearing.api.plugin.ClearableData +import com.duckduckgo.dataclearing.api.plugin.DataClearingTrigger import com.duckduckgo.di.scopes.FragmentScope import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded @@ -34,7 +36,6 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import logcat.logcat import javax.inject.Inject @ContributesViewModel(FragmentScope::class) @@ -42,6 +43,7 @@ class ChatHistoryViewModel @Inject constructor( private val chatHistoryRepository: ChatHistoryRepository, @AppCoroutineScope private val appScope: CoroutineScope, private val duckChat: DuckChatInternal, + private val dataClearingTrigger: DataClearingTrigger, ) : ViewModel() { private val searchState = MutableStateFlow(SearchState()) @@ -66,10 +68,7 @@ class ChatHistoryViewModel @Inject constructor( initialValue = ChatHistoryUiState.Loading, ) - /** - * On every Repository emission, intersect any selection with the current item IDs so a - * concurrent omnibar Delete or single-row overflow Delete doesn't desync the selection. - */ + /** Intersect any selection with the current item IDs so concurrent deletes don't desync it. */ private fun reconcileSelection(items: List) { val current = modeState.value if (current is Mode.Selecting && current.selectedChatIds.isNotEmpty()) { @@ -81,14 +80,8 @@ class ChatHistoryViewModel @Inject constructor( } } - /** True when the screen is currently in select mode — read synchronously by the Fragment. */ fun isSelectMode(): Boolean = modeState.value is Mode.Selecting - /** - * Row tap. In default mode resumes the chat in Duck.ai; in select mode toggles the row in - * the current selection. Centralised here so the Fragment doesn't need to read mode state - * or hold a `DuckChatInternal` reference. - */ fun onChatRowClicked(chatId: String) { if (modeState.value is Mode.Selecting) { onSelectionToggled(chatId) @@ -97,14 +90,9 @@ class ChatHistoryViewModel @Inject constructor( } } - /** - * Row long-press — power-user entry to select mode alongside the toolbar overflow Select - * action. In default mode enters select mode with the row pre-selected. In select mode - * toggles the row, matching tap behaviour. Returns true to consume the long-press event. - */ + /** Long-press enters select mode with the row pre-selected; returns true to consume the event. */ fun onChatRowLongClicked(chatId: String): Boolean { if (modeState.value !is Mode.Selecting) { - logcat { "ChatHistory: enter select mode via long-press (chatId=$chatId)" } modeState.value = Mode.Selecting(setOf(chatId)) } else { onSelectionToggled(chatId) @@ -112,12 +100,10 @@ class ChatHistoryViewModel @Inject constructor( return true } - /** Empty-state CTA — opens Duck.ai for a fresh chat. */ fun onOpenDuckAiClicked() { duckChat.openDuckChat() } - /** Toolbar fire icon. Routes to Delete-selected in select mode, Fire-all otherwise. */ fun onFireIconClicked() { if (modeState.value is Mode.Selecting) { onDeleteSelectedRequested() @@ -138,45 +124,37 @@ class ChatHistoryViewModel @Inject constructor( searchState.value = SearchState() } - /** - * 0 Recent → no-op; 1 → delete directly (spares Pinned); ≥2 → confirmation dialog, - * which clears every Duck.ai chat (Pinned included) via the options-driven path. - */ + /** N=1 spares Pinned; N≥2 routes through the dialog, which wipes every Duck.ai chat. */ fun onFireAllRequested() { val recent = latestItems.filter { !it.pinned } when { - recent.isEmpty() -> { - logcat { "ChatHistory: Fire-all no-op (Recent empty)" } - } - recent.size == 1 -> { - logcat { "ChatHistory: Fire-all single-chat fast-path" } - appScope.launch { chatHistoryRepository.deleteChat(recent.single().chatId) } - } - else -> { - logcat { "ChatHistory: Fire-all confirmation requested (count=${recent.size})" } - confirmationState.value = PendingConfirmation.FireAll(count = recent.size) - } + recent.isEmpty() -> Unit + recent.size == 1 -> dispatchSelectedClear(setOf(recent.single().chatId)) + else -> confirmationState.value = PendingConfirmation.FireAll(count = recent.size) + } + } + + private fun dispatchSelectedClear(chatIds: Set) { + if (chatIds.isEmpty()) return + val urls = chatIds.mapTo(mutableSetOf()) { duckChat.buildChatUrl(it) } + appScope.launch { + dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.Selected(urls))) } } - /** Clears the confirmation state. The dialog performs the actual deletion via the options-driven path. */ + /** The dialog drives the actual deletion via its options-driven path. */ fun onFireAllConfirmed() { - logcat { "ChatHistory: Fire-all confirmed (options-driven path handles deletion)" } confirmationState.value = null } fun onConfirmationCancelled() { - logcat { "ChatHistory: confirmation cancelled" } confirmationState.value = null } - /** Enters select mode with an empty selection. Triggered by the toolbar overflow "Select" entry. */ fun onEnterSelectMode() { - logcat { "ChatHistory: enter select mode" } modeState.value = Mode.Selecting(emptySet()) } - /** Toggles row membership inside the current selection. Empty selection stays in select mode. */ fun onSelectionToggled(chatId: String) { val current = modeState.value as? Mode.Selecting ?: return val next = if (chatId in current.selectedChatIds) { @@ -187,7 +165,6 @@ class ChatHistoryViewModel @Inject constructor( modeState.value = Mode.Selecting(next) } - /** Select-all / Unselect-all toggle in the sticky header row. */ fun onSelectAllToggled() { val current = modeState.value as? Mode.Selecting ?: return val visibleIds = visibleChatIds() @@ -195,58 +172,36 @@ class ChatHistoryViewModel @Inject constructor( modeState.value = Mode.Selecting(next) } - /** Exits select mode without deleting. Back-arrow / system back. */ fun onSelectModeCancelled() { - logcat { "ChatHistory: select mode cancelled" } modeState.value = Mode.Default } - /** - * Toolbar fire icon in select mode. Branches on selection size: - * - 0 → no-op (the icon should be disabled). - * - 1 → delete the single chat directly (TODO: route through `ClearableData.DuckChats.Selected` - * once T059–T061 land; for now uses `chatHistoryRepository.deleteChat(chatId)`). - * - ≥ 2 → confirmation dialog with the captured `chatIds` snapshot. - */ fun onDeleteSelectedRequested() { val current = modeState.value as? Mode.Selecting ?: return val ids = current.selectedChatIds when { - ids.isEmpty() -> logcat { "ChatHistory: Delete-selected no-op (empty selection)" } + ids.isEmpty() -> Unit ids.size == 1 -> { - logcat { "ChatHistory: Delete-selected single-chat fast-path" } - // Exit select mode before launching so the reconciler doesn't race the delete. modeState.value = Mode.Default - appScope.launch { chatHistoryRepository.deleteChat(ids.single()) } - } - else -> { - logcat { "ChatHistory: Delete-selected confirmation requested (count=${ids.size})" } - confirmationState.value = PendingConfirmation.DeleteSelected(chatIds = ids) + dispatchSelectedClear(ids) } + else -> confirmationState.value = PendingConfirmation.DeleteSelected(chatIds = ids) } } - /** - * Clears the confirmation state and exits select mode. The dialog drives the actual deletion - * via its `selectedChatUrls` plumbing (T057 — TODO: wire when the dialog change lands). - */ + /** The dialog drives the actual deletion via the URL set surfaced by [chatUrlsForDialog]. */ fun onDeleteSelectedConfirmed() { - logcat { "ChatHistory: Delete-selected confirmed" } - // TODO(T058/T064): once the dialog dispatches via `ClearableData.DuckChats.Selected`, - // remove this fallback. For now (UI-only landing), the ViewModel iterates the captured - // snapshot directly so the feature is end-to-end functional on internal builds. - val ids = (confirmationState.value as? PendingConfirmation.DeleteSelected)?.chatIds.orEmpty() - // Exit select mode FIRST so subsequent repository emissions don't race the reconciler — - // otherwise a delete could observe `Mode.Selecting(ids)` and rewrite `mode` to - // `Selecting(empty)` before this method finishes. confirmationState.value = null modeState.value = Mode.Default - if (ids.isNotEmpty()) { - appScope.launch { ids.forEach { chatHistoryRepository.deleteChat(it) } } - } } - /** Snapshot of the IDs currently visible (respects the active search filter). */ + /** Snapshot of the captured chat IDs (resolved to URLs) for the pending DeleteSelected confirmation. */ + fun chatUrlsForDialog(): Set? { + val ids = (confirmationState.value as? PendingConfirmation.DeleteSelected)?.chatIds ?: return null + if (ids.isEmpty()) return null + return ids.mapTo(mutableSetOf()) { duckChat.buildChatUrl(it) } + } + private fun visibleChatIds(): Set { val search = searchState.value return latestItems @@ -261,7 +216,6 @@ class ChatHistoryViewModel @Inject constructor( confirmation: PendingConfirmation?, mode: Mode, ): ChatHistoryUiState { - logcat { "ChatHistory: reduce ${items.size} item(s), searchActive=${search.active}, confirmation=$confirmation, mode=$mode" } if (items.isEmpty()) return ChatHistoryUiState.Empty val (pinned, recent) = items.partition { it.pinned } return Loaded( diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt index 0c08e6c4c393..b3bbce318054 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt @@ -198,4 +198,57 @@ class DuckChatDataClearingPluginTest { verify(duckChatDeleter, never()).deleteAllChats() verify(duckChatDeleter, never()).deleteChat(any()) } + + @Test + fun `DuckChats Selected deletes each chat in the set and records per-chat sync deletions`() = runTest { + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn(true) + whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) + whenever(duckChatDeleter.deleteChat("beta")).thenReturn(true) + + plugin.onClearData( + setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=alpha", "https://duck.ai?chatID=beta"))), + ) + + verify(duckChatDeleter).deleteChat("alpha") + verify(duckChatDeleter).deleteChat("beta") + verify(duckChatSyncRepository).recordSingleChatDeletion("alpha") + verify(duckChatSyncRepository).recordSingleChatDeletion("beta") + // Batched sync trigger — one event for the whole subset, not one per chat. + verify(syncEngine, org.mockito.kotlin.times(1)).triggerSync(any()) + } + + @Test + fun `DuckChats Selected does not call deleteAllChats`() = runTest { + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChatDeleter.deleteChat(any())).thenReturn(true) + + plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=x")))) + + verify(duckChatDeleter, never()).deleteAllChats() + } + + @Test + fun `DuckChats Selected with empty url set is a no-op`() = runTest { + plugin.onClearData(setOf(ClearableData.DuckChats.Selected(emptySet()))) + + verify(duckChatDeleter, never()).deleteChat(any()) + verify(duckChatSyncRepository, never()).recordSingleChatDeletion(any()) + verify(syncEngine, never()).triggerSync(any()) + } + + @Test + fun `DuckChats Selected skips urls that are not duck chat urls`() = runTest { + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://example.com")))).thenReturn(false) + whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) + + plugin.onClearData( + setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=alpha", "https://example.com"))), + ) + + verify(duckChatDeleter).deleteChat("alpha") + verify(duckChatDeleter, never()).deleteChat(eq("")) + } } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index df5950a1f47e..1fe16200647d 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -19,6 +19,8 @@ package com.duckduckgo.duckchat.impl.history import app.cash.turbine.TurbineTestContext import app.cash.turbine.test import com.duckduckgo.common.test.CoroutineTestRule +import com.duckduckgo.dataclearing.api.plugin.ClearableData +import com.duckduckgo.dataclearing.api.plugin.DataClearingTrigger import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded import com.duckduckgo.duckchat.impl.messaging.fakes.FakeDuckChatInternal import kotlinx.coroutines.flow.Flow @@ -37,7 +39,8 @@ class ChatHistoryViewModelTest { private val source = MutableStateFlow>(emptyList()) private val repository = FakeChatHistoryRepository(source) private val duckChat = FakeDuckChatInternal() - private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope, duckChat) + private val dataClearingTrigger = RecordingDataClearingTrigger() + private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope, duckChat, dataClearingTrigger) @Test fun `initial state is Loading`() = coroutineRule.testScope.runTest { @@ -235,7 +238,7 @@ class ChatHistoryViewModelTest { } @Test - fun `onFireAllRequested with exactly one Recent chat deletes immediately without setting confirmation`() = runTest { + fun `onFireAllRequested with exactly one Recent chat dispatches DuckChats Selected with that chat url`() = runTest { source.value = listOf( item("p", pinned = true), item("r1"), @@ -247,12 +250,13 @@ class ChatHistoryViewModelTest { viewModel.onFireAllRequested() - val afterDelete = awaitItem() as Loaded - assertEquals(null, afterDelete.confirmation) - assertEquals(listOf("r1"), repository.deletedChatIds) - assertEquals(listOf("p"), afterDelete.pinned.map { it.chatId }) - assertEquals(emptyList(), afterDelete.recent.map { it.chatId }) + expectNoEvents() } + assertEquals( + listOf(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=r1")))), + dataClearingTrigger.calls, + ) + assertTrue(repository.deletedChatIds.isEmpty()) } @Test @@ -349,7 +353,7 @@ class ChatHistoryViewModelTest { assertTrue(repository.deletedChatIds.isEmpty()) } - // --- Chat resume / Duck.ai open (centralised on the ViewModel) --- + // --- Chat resume / Duck.ai open --- @Test fun `onChatRowClicked in default mode resumes the chat in DuckAi`() = runTest { @@ -455,7 +459,7 @@ class ChatHistoryViewModelTest { } } - // --- Select-mode (FR-013, FR-013a) --- + // --- Select-mode --- @Test fun `onEnterSelectMode transitions to Selecting with empty selection`() = runTest { @@ -570,7 +574,7 @@ class ChatHistoryViewModelTest { } @Test - fun `onDeleteSelectedRequested with one selected deletes directly and exits select mode`() = runTest { + fun `onDeleteSelectedRequested with one selected dispatches DuckChats Selected and exits select mode`() = runTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { @@ -582,13 +586,15 @@ class ChatHistoryViewModelTest { awaitItem() viewModel.onDeleteSelectedRequested() - cancelAndConsumeRemainingEvents() + + val final = awaitItem() as Loaded + assertEquals(ChatHistoryUiState.Mode.Default, final.mode) } - // After the runTest scheduler drains: mode reset to Default + delete propagated. - assertEquals(listOf("a"), repository.deletedChatIds) - val finalState = viewModel.uiState.value as Loaded - assertEquals(ChatHistoryUiState.Mode.Default, finalState.mode) - assertEquals(listOf("b"), finalState.recent.map { it.chatId }) + assertEquals( + listOf(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=a")))), + dataClearingTrigger.calls, + ) + assertTrue(repository.deletedChatIds.isEmpty()) } @Test @@ -616,7 +622,7 @@ class ChatHistoryViewModelTest { } @Test - fun `onDeleteSelectedConfirmed deletes the captured ids and returns to Default mode`() = runTest { + fun `onDeleteSelectedConfirmed clears confirmation and exits select mode without dispatching`() = runTest { source.value = listOf(item("p", pinned = true), item("a"), item("b"), item("c")) viewModel.uiState.test { @@ -632,18 +638,49 @@ class ChatHistoryViewModelTest { awaitItem() // DeleteSelected({a, c}) viewModel.onDeleteSelectedConfirmed() - - // Drain all subsequent emissions until the list reflects both deletes — state-flow - // batching can fold the mode + confirmation + per-delete emissions in any order. cancelAndConsumeRemainingEvents() } - // After the runTest scheduler drains all coroutines: - assertEquals(setOf("a", "c"), repository.deletedChatIds.toSet()) val finalState = viewModel.uiState.value as Loaded assertEquals(ChatHistoryUiState.Mode.Default, finalState.mode) assertEquals(null, finalState.confirmation) - assertEquals(listOf("p"), finalState.pinned.map { it.chatId }) - assertEquals(listOf("b"), finalState.recent.map { it.chatId }) + // ViewModel must not dispatch — the dialog drives the clear via selectedChatUrls. + assertTrue(dataClearingTrigger.calls.isEmpty()) + assertTrue(repository.deletedChatIds.isEmpty()) + } + + @Test + fun `chatUrlsForDialog returns the captured chatIds mapped through DuckChat buildChatUrl`() = runTest { + source.value = listOf(item("a"), item("b"), item("c")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + viewModel.onEnterSelectMode() + awaitItem() + viewModel.onSelectionToggled("a") + awaitItem() + viewModel.onSelectionToggled("c") + awaitItem() + viewModel.onDeleteSelectedRequested() + awaitItem() // DeleteSelected({a, c}) + + assertEquals( + setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=c"), + viewModel.chatUrlsForDialog(), + ) + } + } + + @Test + fun `chatUrlsForDialog returns null when no DeleteSelected confirmation is pending`() = runTest { + source.value = listOf(item("a")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + assertEquals(null, viewModel.chatUrlsForDialog()) + } } @Test @@ -660,11 +697,10 @@ class ChatHistoryViewModelTest { // Simulate an external delete (e.g. omnibar Fire-icon) removing chat "b" source.value = listOf(item("a"), item("c")) - - val reconciled = awaitItem() as Loaded - val mode = reconciled.mode as ChatHistoryUiState.Mode.Selecting - assertEquals(setOf("a", "c"), mode.selectedChatIds) + cancelAndConsumeRemainingEvents() } + val mode = (viewModel.uiState.value as Loaded).mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("a", "c"), mode.selectedChatIds) } /** @@ -714,3 +750,11 @@ private class FakeChatHistoryRepository( source.value = emptyList() } } + +private class RecordingDataClearingTrigger : DataClearingTrigger { + val calls: MutableList> = mutableListOf() + + override suspend fun clearData(types: Set) { + calls += types + } +} From 5b7b751d09655e5fe15bb0c3ea5824fd61b39748 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 12:16:37 +0200 Subject: [PATCH 04/25] =?UTF-8?q?Spare=20Pinned=20chats=20on=20Fire-all=20?= =?UTF-8?q?by=20routing=20N=E2=89=A52=20through=20Selected?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The N≥2 Fire-all path used to wipe every Duck.ai chat (Pinned included) because it dispatched `DuckChats.All`. Capture the Recent chat IDs into `PendingConfirmation.FireAll(chatIds)` (mirroring `DeleteSelected`) and let the dialog clear only those URLs via `ClearableData.DuckChats.Selected`. Drops the now-unused `fireOptionsOverride` field and the `options` parameter on `clearDataUsingManualFireOptions` — both were only there to support the buggy options-driven path. --- .../com/duckduckgo/app/fire/DataClearing.kt | 9 +++---- .../duckduckgo/app/fire/ManualDataClearing.kt | 7 +---- .../view/SingleTabFireDialogViewModel.kt | 11 ++------ .../impl/history/ChatHistoryFragment.kt | 15 ++++------- .../impl/history/ChatHistoryUiState.kt | 13 +++++----- .../impl/history/ChatHistoryViewModel.kt | 10 ++++--- .../impl/history/ChatHistoryViewModelTest.kt | 26 ++++++++++++++++--- 7 files changed, 47 insertions(+), 44 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt index b2090ac29ea7..c08162fc7b0d 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt @@ -143,19 +143,18 @@ class DataClearing @Inject constructor( override suspend fun clearDataUsingManualFireOptions( shouldRestartIfRequired: Boolean, wasAppUsedSinceLastClear: Boolean, - options: Set?, ) { - val effectiveOptions = options ?: fireDataStore.getManualClearOptions() + val options = fireDataStore.getManualClearOptions() performGranularClear( - options = effectiveOptions, + options = options, shouldFireDataClearPixel = true, ) clearDataAction.setAppUsedSinceLastClearFlag(wasAppUsedSinceLastClear) - val wasDuckAiChatsCleared = effectiveOptions.contains(FireClearOption.DUCKAI_CHATS) && + val wasDuckAiChatsCleared = options.contains(FireClearOption.DUCKAI_CHATS) && duckAiFeatureState.showClearDuckAIChatHistory.value - val wasDataCleared = effectiveOptions.contains(FireClearOption.DATA) || wasDuckAiChatsCleared + val wasDataCleared = options.contains(FireClearOption.DATA) || wasDuckAiChatsCleared if (shouldRestartIfRequired && wasDataCleared) { dataClearingWideEvent.finishSuccess() // If there is an open wide event, complete it before killing the process. clearDataAction.killAndRestartProcess(notifyDataCleared = false) diff --git a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt index 65080aa807be..52209c33e5ee 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt @@ -17,20 +17,15 @@ package com.duckduckgo.app.fire import com.duckduckgo.app.global.view.ClearDataResult -import com.duckduckgo.app.settings.clear.FireClearOption /** * Interface for manual data clearing operations triggered by user actions (e.g., Fire button). */ interface ManualDataClearing { - /** - * Clears data when user requests data clearing using the FireDialog. - * @param options when non-null, drives the clear instead of the user's stored options. - */ + /** Clears data when user requests data clearing using the FireDialog. */ suspend fun clearDataUsingManualFireOptions( shouldRestartIfRequired: Boolean = false, wasAppUsedSinceLastClear: Boolean = false, - options: Set? = null, ) /** Deletes only the chats addressed by [chatUrls] and closes any browser tabs pointing at them. */ diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt index 51b0226aadaa..70c0a5fea657 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt @@ -147,8 +147,7 @@ class SingleTabFireDialogViewModel @Inject constructor( fireButtonStore.incrementFireButtonUseCount() userEventsStore.registerUserEvent(UserEventKey.FIRE_BUTTON_EXECUTED) val selectedChatUrls = (origin.value as? FireDialogOrigin.ChatHistory)?.selectedChatUrls - val fireOptionsOverride = (viewState.value as? ViewState.Loaded)?.stateData?.fireOptionsOverride - val clearOptions = fireOptionsOverride ?: fireDataStore.getManualClearOptions() + val clearOptions = fireDataStore.getManualClearOptions() dataClearingWideEvent.start( entryPoint = DataClearingWideEvent.EntryPoint.SINGLE_TAB_FIRE_DIALOG, clearOptions = clearOptions, @@ -157,7 +156,7 @@ class SingleTabFireDialogViewModel @Inject constructor( if (selectedChatUrls != null) { dataClearing.clearSelectedDuckAiChats(selectedChatUrls) } else { - dataClearing.clearDataUsingManualFireOptions(options = fireOptionsOverride) + dataClearing.clearDataUsingManualFireOptions() } dataClearingWideEvent.finishSuccess() } catch (e: Exception) { @@ -271,11 +270,6 @@ class SingleTabFireDialogViewModel @Inject constructor( } } - val fireOptionsOverride: Set? = when (dialogOrigin) { - is FireDialogOrigin.ChatHistory -> setOf(FireClearOption.DUCKAI_CHATS) - else -> null - } - return ViewState.Loaded( stateData = ViewState.Loaded.StateData( isDuckAiChatsSelected = isDuckAiChatsSelected, @@ -287,7 +281,6 @@ class SingleTabFireDialogViewModel @Inject constructor( isFirePictogramVisible = settingsDataStore.fireAnimationEnabled, isFireAnimationUpdateEnabled = isFireAnimationUpdateEnabled, titleSource = titleSource, - fireOptionsOverride = fireOptionsOverride, ), origin = dialogOrigin, ) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index dcfa24e3cdca..e575af858baf 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -206,18 +206,13 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { if (confirmation == null) return if (childFragmentManager.findFragmentByTag(FIRE_DIALOG_TAG) != null) return - val count = when (confirmation) { - is ChatHistoryUiState.PendingConfirmation.FireAll -> confirmation.count - is ChatHistoryUiState.PendingConfirmation.DeleteSelected -> confirmation.count - } - val selectedChatUrls = if (confirmation is ChatHistoryUiState.PendingConfirmation.DeleteSelected) { - viewModel.chatUrlsForDialog() - } else { - null - } + val selectedChatUrls = viewModel.chatUrlsForDialog() viewLifecycleOwner.lifecycleScope.launch { val dialog = fireDialogProvider.createFireDialog( - FireDialogProvider.FireDialogOrigin.ChatHistory(count = count, selectedChatUrls = selectedChatUrls), + FireDialogProvider.FireDialogOrigin.ChatHistory( + count = confirmation.count, + selectedChatUrls = selectedChatUrls, + ), ) dialog.show(childFragmentManager, FIRE_DIALOG_TAG) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt index 1a7964b15a40..31ee594f8575 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryUiState.kt @@ -34,12 +34,13 @@ sealed interface ChatHistoryUiState { } sealed interface PendingConfirmation { - /** Fire-all confirmation; set when count ≥ 2 (count == 1 deletes directly). */ - data class FireAll(val count: Int) : PendingConfirmation + val chatIds: Set + val count: Int get() = chatIds.size - /** Captured selection snapshot — concurrent emissions can't change the deletion target. */ - data class DeleteSelected(val chatIds: Set) : PendingConfirmation { - val count: Int get() = chatIds.size - } + /** Fire-all confirmation — captured Recent chat IDs at the moment Fire-all was tapped. */ + data class FireAll(override val chatIds: Set) : PendingConfirmation + + /** Delete-selected confirmation — captured selection snapshot. */ + data class DeleteSelected(override val chatIds: Set) : PendingConfirmation } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index c35a7e1a0a10..68bd0c37fcbf 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -130,7 +130,9 @@ class ChatHistoryViewModel @Inject constructor( when { recent.isEmpty() -> Unit recent.size == 1 -> dispatchSelectedClear(setOf(recent.single().chatId)) - else -> confirmationState.value = PendingConfirmation.FireAll(count = recent.size) + else -> confirmationState.value = PendingConfirmation.FireAll( + chatIds = recent.mapTo(mutableSetOf()) { it.chatId }, + ) } } @@ -142,7 +144,7 @@ class ChatHistoryViewModel @Inject constructor( } } - /** The dialog drives the actual deletion via its options-driven path. */ + /** The dialog drives the actual deletion via the URL set surfaced by [chatUrlsForDialog]. */ fun onFireAllConfirmed() { confirmationState.value = null } @@ -195,9 +197,9 @@ class ChatHistoryViewModel @Inject constructor( modeState.value = Mode.Default } - /** Snapshot of the captured chat IDs (resolved to URLs) for the pending DeleteSelected confirmation. */ + /** Snapshot of the captured chat IDs (resolved to URLs) for the pending confirmation. */ fun chatUrlsForDialog(): Set? { - val ids = (confirmationState.value as? PendingConfirmation.DeleteSelected)?.chatIds ?: return null + val ids = confirmationState.value?.chatIds ?: return null if (ids.isEmpty()) return null return ids.mapTo(mutableSetOf()) { duckChat.buildChatUrl(it) } } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 1fe16200647d..0e36626aff7e 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -232,7 +232,7 @@ class ChatHistoryViewModelTest { viewModel.onFireAllRequested() val confirming = awaitItem() as Loaded - assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(count = 3), confirming.confirmation) + assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("r1", "r2", "r3")), confirming.confirmation) assertTrue(repository.deletedChatIds.isEmpty()) } } @@ -319,7 +319,7 @@ class ChatHistoryViewModelTest { viewModel.onFireAllRequested() val confirming = awaitItem() as Loaded - assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(count = 2), confirming.confirmation) + assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("r1", "r2")), confirming.confirmation) viewModel.onConfirmationCancelled() @@ -434,7 +434,7 @@ class ChatHistoryViewModelTest { viewModel.onFireIconClicked() val confirming = awaitItem() as Loaded - assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(count = 2), confirming.confirmation) + assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("r1", "r2")), confirming.confirmation) } } @@ -672,7 +672,7 @@ class ChatHistoryViewModelTest { } @Test - fun `chatUrlsForDialog returns null when no DeleteSelected confirmation is pending`() = runTest { + fun `chatUrlsForDialog returns null when no confirmation is pending`() = runTest { source.value = listOf(item("a")) viewModel.uiState.test { @@ -683,6 +683,24 @@ class ChatHistoryViewModelTest { } } + @Test + fun `chatUrlsForDialog returns Recent URLs while a FireAll confirmation is pending`() = runTest { + source.value = listOf(item("p", pinned = true), item("r1"), item("r2")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onFireAllRequested() + awaitItem() // FireAll({r1, r2}) + + assertEquals( + setOf("https://duck.ai?chatID=r1", "https://duck.ai?chatID=r2"), + viewModel.chatUrlsForDialog(), + ) + } + } + @Test fun `concurrent repository emission removing a selected id reconciles the selection`() = runTest { source.value = listOf(item("a"), item("b"), item("c")) From 5689b52bb214b0aa14f3b19cb610ef84e551224e Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 13:28:02 +0200 Subject: [PATCH 05/25] Match Duck.ai tabs by chatID query param, not full URL Tab URLs drift over a session (server redirects, accumulated params, fragments after replies), so exact-string set membership only matched the freshly-opened tab and missed older tabs of the same chat. Extract chatID from both sides of the comparison so every tab pointing at a cleared chat gets closed, regardless of URL drift. --- .../app/fire/DuckAiTabsCleanupPlugin.kt | 16 ++++++- .../app/fire/DuckAiTabsCleanupPluginTest.kt | 47 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt index 4fd6a7a901d0..6114561647f8 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt @@ -16,6 +16,8 @@ package com.duckduckgo.app.fire +import android.annotation.SuppressLint +import android.net.Uri import androidx.core.net.toUri import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.dataclearing.api.plugin.ClearableData @@ -53,14 +55,26 @@ class DuckAiTabsCleanupPlugin @Inject constructor( } } + /** + * Match by `chatID` query param rather than full URL equality — tabs drift (server redirects, + * extra query params accumulated during the session) so multiple tabs of the same chat would + * otherwise miss the match. + */ private suspend fun closeTabsMatching(chatUrls: Set) { if (chatUrls.isEmpty()) return + val targetChatIds = chatUrls.mapNotNullTo(mutableSetOf()) { it.toUri().chatIdOrNull() } + if (targetChatIds.isEmpty()) return val ids = tabRepository.getTabs() - .filter { it.url in chatUrls } + .filter { tab -> tab.url?.toUri()?.chatIdOrNull() in targetChatIds } .map { it.tabId } if (ids.isNotEmpty()) { logcat { "Closing ${ids.size} Duck.ai tab(s) matching the cleared subset" } tabRepository.deleteTabs(ids) } } + + private fun Uri.chatIdOrNull(): String? { + if (!duckChat.isDuckChatUrl(this)) return null + return getQueryParameter("chatID")?.takeIf { it.isNotBlank() } + } } diff --git a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt index 6a8e39b4af33..282a2e898cf4 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt @@ -91,6 +91,9 @@ class DuckAiTabsCleanupPluginTest { val unrelatedDuckAiTab = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=zzz") val browserTab = TabEntity(tabId = "tab3", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(matchingTab, unrelatedDuckAiTab, browserTab)) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=zzz".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -103,6 +106,9 @@ class DuckAiTabsCleanupPluginTest { val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=b") val t3 = TabEntity(tabId = "tab3", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=a".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=b".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) testee.onClearData( setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b"))), @@ -115,12 +121,53 @@ class DuckAiTabsCleanupPluginTest { fun whenDuckChatsSelectedAndNoTabMatches_thenDoNotCallDeleteTabs() = runTest { val unrelated = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=other") whenever(mockTabRepository.getTabs()).thenReturn(listOf(unrelated)) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) verify(mockTabRepository, never()).deleteTabs(any()) } + @Test + fun whenDuckChatsSelectedAndMultipleTabsShareTheSameChatId_thenCloseAllOfThem() = runTest { + val t1 = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") + val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=abc") + val t3 = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + + testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) + + verify(mockTabRepository).deleteTabs(listOf("tab1", "tab2", "tab3")) + } + + @Test + fun whenTabUrlDriftedFromCanonicalChatUrl_thenStillCloseTheTabByChatId() = runTest { + // Tab URLs that share the same chatID but differ in path, extra params, fragments — + // the kinds of drift that happen as the user interacts with the chat over a session. + val original = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") + val withPath = TabEntity(tabId = "tab2", url = "https://duck.ai/chat?chatID=abc") + val withExtraParams = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc&model=gpt&session=xyz") + val withFragment = TabEntity(tabId = "tab4", url = "https://duck.ai?chatID=abc#message-5") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(original, withPath, withExtraParams, withFragment)) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + + testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) + + verify(mockTabRepository).deleteTabs(listOf("tab1", "tab2", "tab3", "tab4")) + } + + @Test + fun whenSelectedUrlHasNoChatIdQueryParam_thenNoOp() = runTest { + val anyTab = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") + whenever(mockTabRepository.getTabs()).thenReturn(listOf(anyTab)) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + + testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai")))) + + verify(mockTabRepository, never()).deleteTabs(any()) + } + @Test fun whenDuckChatsSelectedWithEmptyUrlSet_thenNoOp() = runTest { testee.onClearData(setOf(ClearableData.DuckChats.Selected(emptySet()))) From 15bec23cac3da5c98e080824e58532bff9541ceb Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 14:02:35 +0200 Subject: [PATCH 06/25] Cleanup fire implementation --- .../java/com/duckduckgo/app/fire/DataClearing.kt | 14 +++++++------- .../com/duckduckgo/app/fire/ManualDataClearing.kt | 11 ++++++----- .../dataclearing/api/fire/FireDialogProvider.kt | 4 +++- .../duckchat/impl/history/ChatHistoryFragment.kt | 4 ---- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt index c08162fc7b0d..734a4b462c36 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt @@ -140,10 +140,7 @@ class DataClearing @Inject constructor( dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.Selected(setOf(tabUrl)))) } - override suspend fun clearDataUsingManualFireOptions( - shouldRestartIfRequired: Boolean, - wasAppUsedSinceLastClear: Boolean, - ) { + override suspend fun clearDataUsingManualFireOptions(shouldRestartIfRequired: Boolean, wasAppUsedSinceLastClear: Boolean) { val options = fireDataStore.getManualClearOptions() performGranularClear( options = options, @@ -242,18 +239,22 @@ class DataClearing @Inject constructor( dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.Selected(chatUrls))) } + /** + * Performs granular data clearing based on the provided options + * @return true if process needs to be restarted + */ private suspend fun performGranularClear( options: Set, shouldFireDataClearPixel: Boolean, ) { logcat { "Performing granular clear with options: $options" } - val shouldClearAllTabs = FireClearOption.TABS in options + val shouldClearTabs = FireClearOption.TABS in options val shouldClearData = FireClearOption.DATA in options val shouldClearDuckAiChats = FireClearOption.DUCKAI_CHATS in options && duckAiFeatureState.showClearDuckAIChatHistory.value - if (shouldClearAllTabs) { + if (shouldClearTabs) { clearDataAction.clearTabsOnly() } @@ -263,7 +264,6 @@ class DataClearing @Inject constructor( if (shouldClearDuckAiChats) { clearDataAction.clearDuckAiChatsOnly() - // DuckAiTabsCleanupPlugin closes any open Duck.ai tabs via the All dispatch. dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.All)) } diff --git a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt index 52209c33e5ee..046a64ab76c9 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/ManualDataClearing.kt @@ -22,11 +22,12 @@ import com.duckduckgo.app.global.view.ClearDataResult * Interface for manual data clearing operations triggered by user actions (e.g., Fire button). */ interface ManualDataClearing { - /** Clears data when user requests data clearing using the FireDialog. */ - suspend fun clearDataUsingManualFireOptions( - shouldRestartIfRequired: Boolean = false, - wasAppUsedSinceLastClear: Boolean = false, - ) + /** + * Clears data when user requests data clearing using the FireDialog. + * @param shouldRestartIfRequired whether to restart the app process after clearing data, if required (when data or chats cleared) + * @param wasAppUsedSinceLastClear whether the app was used since the last data clear + */ + suspend fun clearDataUsingManualFireOptions(shouldRestartIfRequired: Boolean = false, wasAppUsedSinceLastClear: Boolean = false) /** Deletes only the chats addressed by [chatUrls] and closes any browser tabs pointing at them. */ suspend fun clearSelectedDuckAiChats(chatUrls: Set) diff --git a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt index 4a4dc4a62de5..5117e4f30456 100644 --- a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt +++ b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt @@ -28,7 +28,9 @@ interface FireDialogProvider { /** * Creates a Fire dialog instance. * - * @param origin Where the dialog is being launched from; also carries any per-call destructive scope. + * @param origin Where the dialog is being launched from. Determines which buttons + * are shown and which tab(s) the dialog operates on. + * @return A FireDialog (Granular or NonGranular variant, selected by remote feature flag). */ suspend fun createFireDialog(origin: FireDialogOrigin): FireDialog diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index e575af858baf..53aac378ba98 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -27,7 +27,6 @@ import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.LinearLayoutManager import com.duckduckgo.anvil.annotations.InjectWith -import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.common.ui.DuckDuckGoFragment import com.duckduckgo.common.ui.menu.PopupMenu import com.duckduckgo.common.ui.view.SearchBar @@ -54,9 +53,6 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { @Inject lateinit var viewModelFactory: FragmentViewModelFactory - @Inject - lateinit var pixel: Pixel - @Inject lateinit var fireDialogProvider: FireDialogProvider From f5a8b72472bcd393b3bfae05ee545cdb31bc5d84 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 15:09:16 +0200 Subject: [PATCH 07/25] Wire per-row Delete in chat history overflow menu The 3-dot overflow on each chat row now fires the chat-history-screen single-delete path (via the existing `dispatchSelectedClear` helper) instead of showing a Coming-soon snackbar. Pin / Rename / Download keep the placeholder snackbar. --- .../impl/history/ChatHistoryFragment.kt | 6 +++--- .../impl/history/ChatHistoryViewModel.kt | 5 +++++ .../impl/history/ChatHistoryViewModelTest.kt | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index 53aac378ba98..90f9cafff32f 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -63,7 +63,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { private val adapter = ChatHistoryAdapter( onChatClicked = { item -> viewModel.onChatRowClicked(item.chatId) }, - onChatMoreClicked = { _, anchor -> showRowPopup(anchor) }, + onChatMoreClicked = { item, anchor -> showRowPopup(item, anchor) }, onChatLongClicked = { item -> viewModel.onChatRowLongClicked(item.chatId) }, onSelectAllClicked = { viewModel.onSelectAllToggled() }, ) @@ -257,13 +257,13 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { popup.show(binding.root, anchor) } - private fun showRowPopup(anchor: View) { + private fun showRowPopup(item: ChatHistoryItem, anchor: View) { val popup = PopupMenu(layoutInflater, R.layout.popup_chat_history_row) val view = popup.contentView popup.onMenuItemClicked(view.findViewById(R.id.pin)) { showComingSoonSnackbar() } popup.onMenuItemClicked(view.findViewById(R.id.rename)) { showComingSoonSnackbar() } popup.onMenuItemClicked(view.findViewById(R.id.download)) { showComingSoonSnackbar() } - popup.onMenuItemClicked(view.findViewById(R.id.delete)) { showComingSoonSnackbar() } + popup.onMenuItemClicked(view.findViewById(R.id.delete)) { viewModel.onDeleteSingleChat(item.chatId) } popup.show(binding.root, anchor) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index 68bd0c37fcbf..f2ef7ca2c79f 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -136,6 +136,11 @@ class ChatHistoryViewModel @Inject constructor( } } + /** Per-row overflow Delete — fires immediately, no confirmation. */ + fun onDeleteSingleChat(chatId: String) { + dispatchSelectedClear(setOf(chatId)) + } + private fun dispatchSelectedClear(chatIds: Set) { if (chatIds.isEmpty()) return val urls = chatIds.mapTo(mutableSetOf()) { duckChat.buildChatUrl(it) } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 0e36626aff7e..5bb1d9699829 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -701,6 +701,27 @@ class ChatHistoryViewModelTest { } } + // --- Per-row overflow Delete --- + + @Test + fun `onDeleteSingleChat dispatches DuckChats Selected with that one chat url and no confirmation`() = runTest { + source.value = listOf(item("a"), item("b")) + + viewModel.uiState.test { + awaitItem() // Loading + awaitItem() // initial Loaded + + viewModel.onDeleteSingleChat("a") + + expectNoEvents() // no confirmation state set + } + assertEquals( + listOf(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=a")))), + dataClearingTrigger.calls, + ) + assertTrue(repository.deletedChatIds.isEmpty()) + } + @Test fun `concurrent repository emission removing a selected id reconciles the selection`() = runTest { source.value = listOf(item("a"), item("b"), item("c")) From 92627f3b285d4c592352d3187cba06e54f8a312f Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 15:09:38 +0200 Subject: [PATCH 08/25] Tighten FireDialogOrigin.ChatHistory shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make `selectedChatUrls` non-nullable and derive `count` from its size so the two fields can't drift. An empty set is a valid no-op shape — the dialog still renders but the destructive dispatch becomes a no-op via the `Selected(emptySet())` plugin contract. Bundle plumbing drops `ARG_CHAT_HISTORY_COUNT` and deserialization no longer downgrades a malformed ChatHistory bundle to the Browser origin (which would have surfaced the wrong destructive UI). DuckAiTabsCleanup drops the lint-suppressed `:duckchat-impl` import in favour of the stable `"chatID"` literal. --- .../app/fire/DuckAiTabsCleanupPlugin.kt | 1 - .../app/global/view/SingleTabFireDialog.kt | 9 ++------- .../dataclearing/api/fire/FireDialogProvider.kt | 16 ++++++++-------- .../duckchat/impl/history/ChatHistoryFragment.kt | 7 ++----- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt index 6114561647f8..8413d232e3d8 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt @@ -16,7 +16,6 @@ package com.duckduckgo.app.fire -import android.annotation.SuppressLint import android.net.Uri import androidx.core.net.toUri import com.duckduckgo.app.tabs.model.TabRepository diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt index 93eb69b39769..0a1ab5795b6b 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt @@ -72,7 +72,6 @@ private const val BOTTOM_SHEET_MAX_WIDTH_DP = 640 private const val NO_MAX_WIDTH = -1 private const val ARG_ORIGIN = "origin" private const val ARG_TAB_ID = "tabId" -private const val ARG_CHAT_HISTORY_COUNT = "chatHistoryCount" private const val ARG_SELECTED_CHAT_URLS = "selectedChatUrls" internal const val ORIGIN_BROWSER = "Browser" internal const val ORIGIN_SETTINGS = "Settings" @@ -440,11 +439,8 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { } } ORIGIN_CHAT_HISTORY -> { - val count = arguments?.getInt(ARG_CHAT_HISTORY_COUNT) ?: 0 - val urls = arguments?.getStringArrayList(ARG_SELECTED_CHAT_URLS) - ?.toSet() - ?.takeIf { it.isNotEmpty() } - FireDialogProvider.FireDialogOrigin.ChatHistory(count = count, selectedChatUrls = urls) + val urls = arguments?.getStringArrayList(ARG_SELECTED_CHAT_URLS)?.toSet().orEmpty() + FireDialogProvider.FireDialogOrigin.ChatHistory(selectedChatUrls = urls) } else -> FireDialogProvider.FireDialogOrigin.Browser } @@ -456,7 +452,6 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { arguments = bundleOf( ARG_ORIGIN to origin.tag(), ARG_TAB_ID to (origin as? FireDialogProvider.FireDialogOrigin.Hatch)?.tabId, - ARG_CHAT_HISTORY_COUNT to (origin as? FireDialogProvider.FireDialogOrigin.ChatHistory)?.count, ARG_SELECTED_CHAT_URLS to (origin as? FireDialogProvider.FireDialogOrigin.ChatHistory) ?.selectedChatUrls?.let { ArrayList(it) }, ) diff --git a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt index 5117e4f30456..d84a87407d63 100644 --- a/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt +++ b/data-clearing/data-clearing-api/src/main/java/com/duckduckgo/dataclearing/api/fire/FireDialogProvider.kt @@ -61,15 +61,15 @@ interface FireDialogProvider { data class Hatch(val tabId: String) : FireDialogOrigin() /** - * Chat history screen — bulk-delete confirmation. + * Chat history screen — bulk-delete confirmation. Scopes the clear to [selectedChatUrls]. + * An empty set is a valid no-op shape; the dialog still renders but the destructive + * action does nothing. * - * @property count Number of chats shown in the title ("Delete N chats?"). - * @property selectedChatUrls Non-null scopes the clear to this subset (Delete-selected); - * `null` clears every Duck.ai chat (Fire-all). + * @property selectedChatUrls The chat URLs to clear on confirm. Title count + * ("Delete N chats?") is derived from this set's size. */ - data class ChatHistory( - val count: Int, - val selectedChatUrls: Set? = null, - ) : FireDialogOrigin() + data class ChatHistory(val selectedChatUrls: Set) : FireDialogOrigin() { + val count: Int get() = selectedChatUrls.size + } } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index 90f9cafff32f..fe6af1fc93c0 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -202,13 +202,10 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { if (confirmation == null) return if (childFragmentManager.findFragmentByTag(FIRE_DIALOG_TAG) != null) return - val selectedChatUrls = viewModel.chatUrlsForDialog() + val selectedChatUrls = viewModel.chatUrlsForDialog().orEmpty() viewLifecycleOwner.lifecycleScope.launch { val dialog = fireDialogProvider.createFireDialog( - FireDialogProvider.FireDialogOrigin.ChatHistory( - count = confirmation.count, - selectedChatUrls = selectedChatUrls, - ), + FireDialogProvider.FireDialogOrigin.ChatHistory(selectedChatUrls = selectedChatUrls), ) dialog.show(childFragmentManager, FIRE_DIALOG_TAG) } From 4bebf91d068745997e27ac8c12b3a72a298e80ff Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 16:47:51 +0200 Subject: [PATCH 09/25] Merge DuckChats.Single into DuckChats.Selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single(url) is just Selected(setOf(url)) on the data side; collapse the two variants so callers describe the shape in one place. The in-chat fire flow preserves the old "don't touch this tab" behaviour by replacing the tab URL before dispatching the clear, so the tabs-cleanup plugin's chatID match no longer finds this tab. Duplicate tabs at the same chatID now also get closed on the in-chat fire path — same multi-tab fix we already shipped for the chat-history path. --- .../main/java/com/duckduckgo/app/fire/DataClearing.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt index 734a4b462c36..d4704ee4e52b 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearing.kt @@ -95,10 +95,9 @@ class DataClearing @Inject constructor( val clearDataResult = clearDataAction.clearDataForSpecificDomains(visitedSites) val tabUrl = tabRepository.getTab(tabId)?.url - clearDuckAiChatIfNeeded(tabUrl) - clearContextualChatDataIfNeeded(tabId) - navigationHistory.removeHistoryForTab(tabId) - + // Reset this tab's URL before dispatching the chat clear: the tabs-cleanup plugin matches + // tabs by chatID, and we don't want this tab caught by that match — it stays open with a + // new chat URL. Other tabs at the same chatID (duplicates) do get closed, which is desired. if (replaceCurrentTab) { val url = getNewTabUrl(tabUrl) tabOperations.replaceTabWithNewTab(tabId, url) @@ -106,6 +105,10 @@ class DataClearing @Inject constructor( tabRepository.deleteTabs(listOf(tabId)) } + clearDuckAiChatIfNeeded(tabUrl) + clearContextualChatDataIfNeeded(tabId) + navigationHistory.removeHistoryForTab(tabId) + logcat { "Single tab clear completed for tab: $tabId" } return clearDataResult } From 211778243eedac57d30f5c98af7b40e857075708 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 18:24:33 +0200 Subject: [PATCH 10/25] Fix flaky ChatHistoryViewModelTest tests by using awaitInitialLoaded stateIn(WhileSubscribed) doesn't guarantee subscribers observe the Loading initial value, so manually awaiting Loading then initial Loaded races against the StateFlow replay. Route every test in the file through the existing awaitInitialLoaded helper that already tolerates both orderings. --- .../impl/history/ChatHistoryViewModelTest.kt | 75 +++++++------------ 1 file changed, 25 insertions(+), 50 deletions(-) diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 5bb1d9699829..693fd16dd4a3 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -226,8 +226,7 @@ class ChatHistoryViewModelTest { ) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireAllRequested() @@ -245,8 +244,7 @@ class ChatHistoryViewModelTest { ) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireAllRequested() @@ -267,8 +265,7 @@ class ChatHistoryViewModelTest { ) viewModel.uiState.test { - awaitItem() // Loading - val initial = awaitItem() as Loaded + val initial = awaitInitialLoaded() assertEquals(null, initial.confirmation) viewModel.onFireAllRequested() @@ -289,8 +286,7 @@ class ChatHistoryViewModelTest { ) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireAllRequested() awaitItem() // confirmation = FireAll(3) @@ -314,8 +310,7 @@ class ChatHistoryViewModelTest { ) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireAllRequested() val confirming = awaitItem() as Loaded @@ -339,8 +334,7 @@ class ChatHistoryViewModelTest { ) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireAllRequested() awaitItem() // confirmation = FireAll(2) @@ -367,8 +361,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded (Default) + awaitInitialLoaded() val consumed = viewModel.onChatRowLongClicked("a") @@ -385,8 +378,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onChatRowLongClicked("a") awaitItem() // Selecting({a}) @@ -403,8 +395,7 @@ class ChatHistoryViewModelTest { fun `onChatRowClicked in select mode toggles selection instead of opening DuckAi`() = runTest { source.value = listOf(item("a")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() @@ -428,8 +419,7 @@ class ChatHistoryViewModelTest { fun `onFireIconClicked in default mode triggers Fire-all`() = runTest { source.value = listOf(item("r1"), item("r2")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireIconClicked() @@ -442,8 +432,7 @@ class ChatHistoryViewModelTest { fun `onFireIconClicked in select mode triggers Delete-selected`() = runTest { source.value = listOf(item("a"), item("b"), item("c")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectionToggled("a") @@ -466,8 +455,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded (Default) + awaitInitialLoaded() viewModel.onEnterSelectMode() @@ -482,8 +470,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() // Selecting({}) @@ -503,8 +490,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("p", pinned = true), item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() // Selecting({}) @@ -521,8 +507,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() // Selecting({}) viewModel.onSelectAllToggled() @@ -541,8 +526,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectionToggled("a") @@ -561,8 +545,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() // Selecting({}) @@ -578,8 +561,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectionToggled("a") @@ -602,8 +584,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b"), item("c")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectionToggled("a") @@ -626,8 +607,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("p", pinned = true), item("a"), item("b"), item("c")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectionToggled("a") @@ -653,8 +633,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b"), item("c")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectionToggled("a") @@ -676,8 +655,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() assertEquals(null, viewModel.chatUrlsForDialog()) } @@ -688,8 +666,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("p", pinned = true), item("r1"), item("r2")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onFireAllRequested() awaitItem() // FireAll({r1, r2}) @@ -708,8 +685,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onDeleteSingleChat("a") @@ -727,8 +703,7 @@ class ChatHistoryViewModelTest { source.value = listOf(item("a"), item("b"), item("c")) viewModel.uiState.test { - awaitItem() // Loading - awaitItem() // initial Loaded + awaitInitialLoaded() viewModel.onEnterSelectMode() awaitItem() viewModel.onSelectAllToggled() From 4ebab494c893a9c3afceb9f5880308cbed5d6432 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Sat, 16 May 2026 07:54:16 +0200 Subject: [PATCH 11/25] Collapse ChatHistoryViewModel UI flows into a single state holder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit searchState, confirmationState and modeState were three independent MutableStateFlows fed into a 4-arg combine. Any action that needed to mutate more than one of them (only onDeleteSelectedConfirmed today) produced two upstream signals to combine, which could surface as one or two downstream frames depending on scheduler timing — the test for that action was flaky in suite runs as a result. Consolidate into a single UiControls(search, confirmation, mode) behind one MutableStateFlow. Every action is now one .update { it.copy(...) } → exactly one combine frame. Restores the deterministic awaitItem contract the test relies on. --- .../impl/history/ChatHistoryViewModel.kt | 116 +++++++++--------- .../impl/history/ChatHistoryViewModelTest.kt | 7 +- 2 files changed, 64 insertions(+), 59 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index f2ef7ca2c79f..fcf2f7b0f0ca 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -46,9 +46,7 @@ class ChatHistoryViewModel @Inject constructor( private val dataClearingTrigger: DataClearingTrigger, ) : ViewModel() { - private val searchState = MutableStateFlow(SearchState()) - private val confirmationState = MutableStateFlow(null) - private val modeState = MutableStateFlow(Mode.Default) + private val controls = MutableStateFlow(UiControls()) /** Cached snapshot so non-suspend action methods can read Recent without re-subscribing. */ private var latestItems: List = emptyList() @@ -58,9 +56,7 @@ class ChatHistoryViewModel @Inject constructor( latestItems = items reconcileSelection(items) }, - searchState, - confirmationState, - modeState, + controls, ::reduce, ).stateIn( scope = viewModelScope, @@ -70,20 +66,20 @@ class ChatHistoryViewModel @Inject constructor( /** Intersect any selection with the current item IDs so concurrent deletes don't desync it. */ private fun reconcileSelection(items: List) { - val current = modeState.value + val current = controls.value.mode if (current is Mode.Selecting && current.selectedChatIds.isNotEmpty()) { val knownIds = items.mapTo(mutableSetOf()) { it.chatId } val intersected = current.selectedChatIds.intersect(knownIds) if (intersected.size != current.selectedChatIds.size) { - modeState.value = Mode.Selecting(intersected) + controls.update { it.copy(mode = Mode.Selecting(intersected)) } } } } - fun isSelectMode(): Boolean = modeState.value is Mode.Selecting + fun isSelectMode(): Boolean = controls.value.mode is Mode.Selecting fun onChatRowClicked(chatId: String) { - if (modeState.value is Mode.Selecting) { + if (controls.value.mode is Mode.Selecting) { onSelectionToggled(chatId) } else { duckChat.openWithChatId(chatId) @@ -92,10 +88,12 @@ class ChatHistoryViewModel @Inject constructor( /** Long-press enters select mode with the row pre-selected; returns true to consume the event. */ fun onChatRowLongClicked(chatId: String): Boolean { - if (modeState.value !is Mode.Selecting) { - modeState.value = Mode.Selecting(setOf(chatId)) - } else { - onSelectionToggled(chatId) + controls.update { c -> + val nextMode = when (val mode = c.mode) { + is Mode.Selecting -> Mode.Selecting(toggle(mode.selectedChatIds, chatId)) + Mode.Default -> Mode.Selecting(setOf(chatId)) + } + c.copy(mode = nextMode) } return true } @@ -105,7 +103,7 @@ class ChatHistoryViewModel @Inject constructor( } fun onFireIconClicked() { - if (modeState.value is Mode.Selecting) { + if (controls.value.mode is Mode.Selecting) { onDeleteSelectedRequested() } else { onFireAllRequested() @@ -113,15 +111,15 @@ class ChatHistoryViewModel @Inject constructor( } fun onSearchActivated() { - searchState.update { it.copy(active = true) } + controls.update { it.copy(search = it.search.copy(active = true)) } } fun onSearchQueryChanged(query: String) { - searchState.update { it.copy(query = query) } + controls.update { it.copy(search = it.search.copy(query = query)) } } fun onSearchClosed() { - searchState.value = SearchState() + controls.update { it.copy(search = SearchState()) } } /** N=1 spares Pinned; N≥2 routes through the dialog, which wipes every Duck.ai chat. */ @@ -130,9 +128,9 @@ class ChatHistoryViewModel @Inject constructor( when { recent.isEmpty() -> Unit recent.size == 1 -> dispatchSelectedClear(setOf(recent.single().chatId)) - else -> confirmationState.value = PendingConfirmation.FireAll( - chatIds = recent.mapTo(mutableSetOf()) { it.chatId }, - ) + else -> controls.update { + it.copy(confirmation = PendingConfirmation.FireAll(chatIds = recent.mapTo(mutableSetOf()) { i -> i.chatId })) + } } } @@ -151,90 +149,92 @@ class ChatHistoryViewModel @Inject constructor( /** The dialog drives the actual deletion via the URL set surfaced by [chatUrlsForDialog]. */ fun onFireAllConfirmed() { - confirmationState.value = null + controls.update { it.copy(confirmation = null) } } fun onConfirmationCancelled() { - confirmationState.value = null + controls.update { it.copy(confirmation = null) } } fun onEnterSelectMode() { - modeState.value = Mode.Selecting(emptySet()) + controls.update { it.copy(mode = Mode.Selecting(emptySet())) } } fun onSelectionToggled(chatId: String) { - val current = modeState.value as? Mode.Selecting ?: return - val next = if (chatId in current.selectedChatIds) { - current.selectedChatIds - chatId - } else { - current.selectedChatIds + chatId + controls.update { c -> + val mode = c.mode as? Mode.Selecting ?: return@update c + c.copy(mode = Mode.Selecting(toggle(mode.selectedChatIds, chatId))) } - modeState.value = Mode.Selecting(next) } fun onSelectAllToggled() { - val current = modeState.value as? Mode.Selecting ?: return - val visibleIds = visibleChatIds() - val next = if (current.selectedChatIds == visibleIds) emptySet() else visibleIds - modeState.value = Mode.Selecting(next) + controls.update { c -> + val mode = c.mode as? Mode.Selecting ?: return@update c + val visibleIds = visibleChatIds(c.search) + val next = if (mode.selectedChatIds == visibleIds) emptySet() else visibleIds + c.copy(mode = Mode.Selecting(next)) + } } fun onSelectModeCancelled() { - modeState.value = Mode.Default + controls.update { it.copy(mode = Mode.Default) } } fun onDeleteSelectedRequested() { - val current = modeState.value as? Mode.Selecting ?: return + val current = controls.value.mode as? Mode.Selecting ?: return val ids = current.selectedChatIds when { ids.isEmpty() -> Unit ids.size == 1 -> { - modeState.value = Mode.Default + controls.update { it.copy(mode = Mode.Default) } dispatchSelectedClear(ids) } - else -> confirmationState.value = PendingConfirmation.DeleteSelected(chatIds = ids) + else -> controls.update { + it.copy(confirmation = PendingConfirmation.DeleteSelected(chatIds = ids)) + } } } - /** The dialog drives the actual deletion via the URL set surfaced by [chatUrlsForDialog]. */ + /** + * The dialog drives the actual deletion via the URL set surfaced by [chatUrlsForDialog]. + * Both fields update atomically (one frame, not two) — keeps the test contract simple. + */ fun onDeleteSelectedConfirmed() { - confirmationState.value = null - modeState.value = Mode.Default + controls.update { it.copy(confirmation = null, mode = Mode.Default) } } /** Snapshot of the captured chat IDs (resolved to URLs) for the pending confirmation. */ fun chatUrlsForDialog(): Set? { - val ids = confirmationState.value?.chatIds ?: return null + val ids = controls.value.confirmation?.chatIds ?: return null if (ids.isEmpty()) return null return ids.mapTo(mutableSetOf()) { duckChat.buildChatUrl(it) } } - private fun visibleChatIds(): Set { - val search = searchState.value - return latestItems + private fun visibleChatIds(search: SearchState): Set = + latestItems .asSequence() .filter { item -> !search.active || search.query.isEmpty() || item.displayTitle.contains(search.query, ignoreCase = true) } .mapTo(mutableSetOf()) { it.chatId } - } private fun reduce( items: List, - search: SearchState, - confirmation: PendingConfirmation?, - mode: Mode, + controls: UiControls, ): ChatHistoryUiState { if (items.isEmpty()) return ChatHistoryUiState.Empty val (pinned, recent) = items.partition { it.pinned } return Loaded( - pinned = pinned.sortedByDate().filterBy(search), - recent = recent.sortedByDate().filterBy(search), - searchQuery = search.query, - searchActive = search.active, - mode = mode, - confirmation = confirmation, + pinned = pinned.sortedByDate().filterBy(controls.search), + recent = recent.sortedByDate().filterBy(controls.search), + searchQuery = controls.search.query, + searchActive = controls.search.active, + mode = controls.mode, + confirmation = controls.confirmation, ) } + private fun toggle(current: Set, chatId: String): Set = + if (chatId in current) current - chatId else current + chatId + private fun List.filterBy(search: SearchState): List = if (!search.active || search.query.isEmpty()) { this @@ -245,6 +245,12 @@ class ChatHistoryViewModel @Inject constructor( private fun List.sortedByDate(): List = sortedByDescending { it.lastEditMillis } + private data class UiControls( + val search: SearchState = SearchState(), + val confirmation: PendingConfirmation? = null, + val mode: Mode = Mode.Default, + ) + private data class SearchState( val active: Boolean = false, val query: String = "", diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 693fd16dd4a3..bf9d03758d29 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -618,11 +618,10 @@ class ChatHistoryViewModelTest { awaitItem() // DeleteSelected({a, c}) viewModel.onDeleteSelectedConfirmed() - cancelAndConsumeRemainingEvents() + val final = awaitItem() as Loaded + assertEquals(ChatHistoryUiState.Mode.Default, final.mode) + assertEquals(null, final.confirmation) } - val finalState = viewModel.uiState.value as Loaded - assertEquals(ChatHistoryUiState.Mode.Default, finalState.mode) - assertEquals(null, finalState.confirmation) // ViewModel must not dispatch — the dialog drives the clear via selectedChatUrls. assertTrue(dataClearingTrigger.calls.isEmpty()) assertTrue(repository.deletedChatIds.isEmpty()) From 5b1c539ca1d16f77d0ab1e648bbcadb63c58c7fb Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 10:23:11 +0200 Subject: [PATCH 12/25] Rename _then test cases added on this branch to backtick natural English MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switches the 18 tests we authored on this branch (7 in DataClearingTest, 11 in the new DuckAiTabsCleanupPluginTest) from the camelCase whenX_thenY style to the backtick natural-English style already used elsewhere in duckchat-impl tests. The pre-existing 68 _then tests in DataClearingTest are left alone — only the tests this branch introduces are renamed. --- .../duckduckgo/app/fire/DataClearingTest.kt | 14 ++++++------ .../app/fire/DuckAiTabsCleanupPluginTest.kt | 22 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt index 6935239dee87..148935179622 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DataClearingTest.kt @@ -196,7 +196,7 @@ class DataClearingTest { } @Test - fun whenManualClearWithDuckAiChats_thenDispatchDuckChatsAllViaTrigger() = runTest { + fun `manual clear with DuckAi chats dispatches DuckChats All via trigger`() = runTest { configureManualOptions(setOf(FireClearOption.DUCKAI_CHATS)) testee.clearDataUsingManualFireOptions(shouldRestartIfRequired = false, wasAppUsedSinceLastClear = true) @@ -1024,7 +1024,7 @@ class DataClearingTest { // --- clearSelectedDuckAiChats --- @Test - fun whenClearSelectedDuckAiChatsWithUrls_thenDispatchSelectedViaTrigger() = runTest { + fun `clearSelectedDuckAiChats with urls dispatches Selected via trigger`() = runTest { val urls = setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b") testee.clearSelectedDuckAiChats(urls) @@ -1033,14 +1033,14 @@ class DataClearingTest { } @Test - fun whenClearSelectedDuckAiChats_thenDoNotWipeDuckAiWebStorage() = runTest { + fun `clearSelectedDuckAiChats does not wipe DuckAi web storage`() = runTest { testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) verify(mockClearDataAction, never()).clearDuckAiChatsOnly() } @Test - fun whenClearSelectedDuckAiChats_thenDoNotTouchTabsOrBrowserData() = runTest { + fun `clearSelectedDuckAiChats does not touch tabs or browser data`() = runTest { testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) verify(mockClearDataAction, never()).clearTabsOnly() @@ -1049,7 +1049,7 @@ class DataClearingTest { } @Test - fun whenClearSelectedDuckAiChats_thenDoNotTouchFireButtonOrchestrationFlags() = runTest { + fun `clearSelectedDuckAiChats does not touch fire button orchestration flags`() = runTest { testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) verify(mockClearDataAction, never()).setAppUsedSinceLastClearFlag(any()) @@ -1057,14 +1057,14 @@ class DataClearingTest { } @Test - fun whenClearSelectedDuckAiChatsWithEmptySet_thenNoOp() = runTest { + fun `clearSelectedDuckAiChats with empty set is a no-op`() = runTest { testee.clearSelectedDuckAiChats(emptySet()) verify(mockDataClearingTrigger, never()).clearData(any()) } @Test - fun whenClearSelectedDuckAiChatsAndFeatureFlagOff_thenNoOp() = runTest { + fun `clearSelectedDuckAiChats with feature flag off is a no-op`() = runTest { showClearDuckAIChatHistoryFlow.value = false testee.clearSelectedDuckAiChats(setOf("https://duck.ai?chatID=a")) diff --git a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt index 282a2e898cf4..408c2f67060d 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt @@ -56,7 +56,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsAllAndDuckAiTabOpen_thenCloseOnlyDuckAiTab() = runTest { + fun `DuckChats All closes only DuckAi tabs`() = runTest { val duckAiTab = TabEntity(tabId = "duck-ai", url = "https://duck.ai") val browserTab = TabEntity(tabId = "browser", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(duckAiTab, browserTab)) @@ -69,7 +69,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsAllAndNoDuckAiTabs_thenDoNotCallDeleteTabs() = runTest { + fun `DuckChats All with no DuckAi tabs does not call deleteTabs`() = runTest { whenever(mockTabRepository.getTabs()).thenReturn(listOf(TabEntity(tabId = "browser", url = "https://example.com"))) whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(false) @@ -79,14 +79,14 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsAllAndNoOpenTabs_thenNoOp() = runTest { + fun `DuckChats All with no open tabs is a no-op`() = runTest { testee.onClearData(setOf(ClearableData.DuckChats.All)) verify(mockTabRepository, never()).deleteTabs(any()) } @Test - fun whenDuckChatsSelectedAndTabUrlMatches_thenCloseMatchingTabOnly() = runTest { + fun `DuckChats Selected closes only the matching tab`() = runTest { val matchingTab = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") val unrelatedDuckAiTab = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=zzz") val browserTab = TabEntity(tabId = "tab3", url = "https://example.com") @@ -101,7 +101,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsSelectedAndMultipleUrlsMatch_thenCloseAllMatching() = runTest { + fun `DuckChats Selected with multiple matching urls closes all of them`() = runTest { val t1 = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=a") val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=b") val t3 = TabEntity(tabId = "tab3", url = "https://example.com") @@ -118,7 +118,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsSelectedAndNoTabMatches_thenDoNotCallDeleteTabs() = runTest { + fun `DuckChats Selected with no matching tab does not call deleteTabs`() = runTest { val unrelated = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=other") whenever(mockTabRepository.getTabs()).thenReturn(listOf(unrelated)) whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) @@ -129,7 +129,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsSelectedAndMultipleTabsShareTheSameChatId_thenCloseAllOfThem() = runTest { + fun `DuckChats Selected closes every tab sharing the same chatID`() = runTest { val t1 = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=abc") val t3 = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc") @@ -142,7 +142,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenTabUrlDriftedFromCanonicalChatUrl_thenStillCloseTheTabByChatId() = runTest { + fun `tab url drifted from canonical chat url still closes the tab by chatID`() = runTest { // Tab URLs that share the same chatID but differ in path, extra params, fragments — // the kinds of drift that happen as the user interacts with the chat over a session. val original = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") @@ -158,7 +158,7 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenSelectedUrlHasNoChatIdQueryParam_thenNoOp() = runTest { + fun `Selected url with no chatID query param is a no-op`() = runTest { val anyTab = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") whenever(mockTabRepository.getTabs()).thenReturn(listOf(anyTab)) whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) @@ -169,14 +169,14 @@ class DuckAiTabsCleanupPluginTest { } @Test - fun whenDuckChatsSelectedWithEmptyUrlSet_thenNoOp() = runTest { + fun `DuckChats Selected with empty url set is a no-op`() = runTest { testee.onClearData(setOf(ClearableData.DuckChats.Selected(emptySet()))) verify(mockTabRepository, never()).deleteTabs(any()) } @Test - fun whenUnrelatedClearableData_thenNoOp() = runTest { + fun `unrelated ClearableData is a no-op`() = runTest { testee.onClearData(setOf(ClearableData.BrowserData.All, ClearableData.Tabs.All)) verify(mockTabRepository, never()).deleteTabs(any()) From d1e1dd70ebd66eb0c078b3796fa2b550b3dfd1a2 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 10:39:33 +0200 Subject: [PATCH 13/25] Stop ChatHistory fire dialog from restarting the app process onDeleteAllClicked routed ChatHistory clears to clearSelectedDuckAiChats but left shouldRestartAfterClearing at its default true, so after a bounded chat-subset clear the dialog still called killAndRestartProcess(). Set the flag to false on that branch, mirroring onDeleteThisTabClicked. --- .../global/view/SingleTabFireDialogViewModel.kt | 1 + .../view/SingleTabFireDialogViewModelTest.kt | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt index 70c0a5fea657..a0fc0a4b4c93 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt @@ -154,6 +154,7 @@ class SingleTabFireDialogViewModel @Inject constructor( ) try { if (selectedChatUrls != null) { + shouldRestartAfterClearing = false dataClearing.clearSelectedDuckAiChats(selectedChatUrls) } else { dataClearing.clearDataUsingManualFireOptions() diff --git a/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt index dda239ff2e9f..4d1e2306feb5 100644 --- a/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt +++ b/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt @@ -1099,6 +1099,21 @@ class SingleTabFireDialogViewModelTest { } } + @Test + fun `when delete all clicked from ChatHistory origin then clearSelectedDuckAiChats is dispatched and process is not restarted`() = runTest { + val urls = setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b") + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = urls)) + + testee.onDeleteAllClicked() + + coroutineTestRule.testScope.testScheduler.advanceUntilIdle() + + verify(mockDataClearing).clearSelectedDuckAiChats(urls) + verify(mockDataClearing, never()).clearDataUsingManualFireOptions(any(), any()) + assertFalse(testee.shouldRestartAfterClearing) + } + // endregion // region onDeleteThisTabClicked From c04c334c812ce4153c4680598e7d8040c3c75077 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 10:56:17 +0200 Subject: [PATCH 14/25] Reset back-press callback on Loading and Empty in ChatHistoryFragment The callback's isEnabled flag was only assigned inside the Loaded branch of render(); a transition out of Loaded (e.g. last chat deleted externally while in select mode) left it enabled but with no matching case in handleOnBackPressed, silently consuming every back press and the toolbar nav arrow. Re-derive isEnabled from the rendered state at the end of render() via a pure helper, so every transition recomputes it and any future state defaults to off. --- .../duckchat/impl/history/ChatHistoryFragment.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index fe6af1fc93c0..dfcd4220ba6a 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -143,16 +143,23 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { if (selectMode != null) { applySelectModeToolbar(selectMode.selectedChatIds.size) setFireActionVisible(selectMode.selectedChatIds.isNotEmpty()) - onBackPressedCallback.isEnabled = true } else { applyDefaultToolbar() // Hide fire when Recent is empty — title is "Delete N chats?" of the Recent count. setFireActionVisible(state.recent.isNotEmpty()) - onBackPressedCallback.isEnabled = binding.searchBar.isVisible } renderConfirmation(state.confirmation) } } + // Re-derive every render so a transition out of Loaded (e.g. last chat deleted externally) + // can't leave us intercepting back presses with no overlay to dismiss. + onBackPressedCallback.isEnabled = shouldInterceptBack(state) + } + + private fun shouldInterceptBack(state: ChatHistoryUiState): Boolean { + if (binding.searchBar.isVisible) return true + val loaded = state as? ChatHistoryUiState.Loaded ?: return false + return loaded.mode is ChatHistoryUiState.Mode.Selecting } private fun applyDefaultToolbar() { From 8528037a8b103bc5e0f1aa1ceb853784900a1ea3 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 11:27:07 +0200 Subject: [PATCH 15/25] Reset toolbar nav CD on default toolbar and re-check fire dialog tag after suspend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit applyDefaultToolbar now clears navigationContentDescription so the back arrow doesn't keep announcing "Exit selection mode" to screen readers after leaving select mode — that CD is only set by applySelectModeToolbar on the same toolbar instance. renderConfirmation re-checks the fragment tag after createFireDialog returns. The outer tag check is synchronous, but createFireDialog is suspending, so two near-simultaneous emissions could both pass the outer guard and race to show the dialog under the same tag. The post-suspend check serialises on show(). --- .../com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index dfcd4220ba6a..04ae69786c56 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -164,6 +164,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { private fun applyDefaultToolbar() { binding.toolbar.setNavigationIcon(com.duckduckgo.mobile.android.R.drawable.ic_arrow_left_24) + binding.toolbar.navigationContentDescription = null binding.toolbar.setNavigationOnClickListener { requireActivity().onBackPressedDispatcher.onBackPressed() } binding.toolbar.setTitle(R.string.duck_ai_chat_history_title) binding.toolbar.menu.findItem(R.id.chat_history_action_search)?.isVisible = true @@ -214,6 +215,7 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { val dialog = fireDialogProvider.createFireDialog( FireDialogProvider.FireDialogOrigin.ChatHistory(selectedChatUrls = selectedChatUrls), ) + if (childFragmentManager.findFragmentByTag(FIRE_DIALOG_TAG) != null) return@launch dialog.show(childFragmentManager, FIRE_DIALOG_TAG) } } From e86a98b1ac5a39a2a104cf16f1a5196f17e081ed Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 15:55:13 +0200 Subject: [PATCH 16/25] Drop chat-history confirmation on EVENT_CLEAR_WITHOUT_RESTART_STARTED too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChatHistory always takes the without-restart path now (shouldRestartAfterClearing is set to false on that branch), so the dialog emits EVENT_CLEAR_WITHOUT_RESTART_STARTED instead of EVENT_ON_CLEAR_STARTED. The fragment listener only handled the latter, leaving controls.confirmation non-null after the user confirmed — which let any subsequent render re-open the dialog. Handle both events under the same arm: from the fragment's perspective they mean the same thing (the dialog is now driving the clear; drop the pending confirmation state). --- .../duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index 04ae69786c56..9745750d45fd 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -104,7 +104,8 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { val event = bundle.getString(FireDialog.RESULT_KEY_EVENT) val confirmation = (viewModel.uiState.value as? ChatHistoryUiState.Loaded)?.confirmation when (event) { - FireDialog.EVENT_ON_CLEAR_STARTED -> when (confirmation) { + FireDialog.EVENT_ON_CLEAR_STARTED, + FireDialog.EVENT_CLEAR_WITHOUT_RESTART_STARTED -> when (confirmation) { is ChatHistoryUiState.PendingConfirmation.FireAll -> viewModel.onFireAllConfirmed() is ChatHistoryUiState.PendingConfirmation.DeleteSelected -> viewModel.onDeleteSelectedConfirmed() null -> Unit From c45e6a0a0b48adaf502c52ca8ece1b47b7a44989 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 16:17:11 +0200 Subject: [PATCH 17/25] Align select-all unchecked indicator size with checked state The unchecked drawable was r=9 inside a 24x24 viewport (18dp visible) while the checked drawable is a filled r=12 disc (24dp visible), matching the Figma spec of a 24px outlined circle. Resize the unchecked vector to a 2dp ring at full 24dp and drive the swap via a state-list drawable + duplicateParentState, replacing the imperative setImageResource toggle in the view holder. --- .../history/ChatHistorySelectAllViewHolder.kt | 13 ++++------- .../drawable/ic_chat_history_unchecked_24.xml | 2 +- ...ctor_chat_history_select_all_indicator.xml | 22 +++++++++++++++++++ .../layout/view_chat_history_select_all.xml | 5 +++-- 4 files changed, 30 insertions(+), 12 deletions(-) create mode 100644 duckchat/duckchat-impl/src/main/res/drawable/selector_chat_history_select_all_indicator.xml diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt index 3b5b8ba510d3..87cf8cd44051 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistorySelectAllViewHolder.kt @@ -19,7 +19,6 @@ package com.duckduckgo.duckchat.impl.history import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.ImageView import androidx.recyclerview.widget.RecyclerView import com.duckduckgo.common.ui.view.text.DaxTextView import com.duckduckgo.duckchat.impl.R @@ -28,17 +27,13 @@ class ChatHistorySelectAllViewHolder( itemView: View, ) : RecyclerView.ViewHolder(itemView) { - private val icon: ImageView = itemView.findViewById(R.id.chatHistorySelectAllIcon) private val label: DaxTextView = itemView.findViewById(R.id.chatHistorySelectAllLabel) fun bind(allSelected: Boolean, onClick: () -> Unit) { - if (allSelected) { - icon.setImageResource(com.duckduckgo.mobile.android.R.drawable.ic_check_blue_round_24) - label.setText(R.string.duck_ai_chat_history_unselect_all) - } else { - icon.setImageResource(R.drawable.ic_chat_history_unchecked_24) - label.setText(R.string.duck_ai_chat_history_select_all) - } + itemView.isSelected = allSelected + label.setText( + if (allSelected) R.string.duck_ai_chat_history_unselect_all else R.string.duck_ai_chat_history_select_all, + ) itemView.setOnClickListener { onClick() } } diff --git a/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml b/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml index 82bbfedaa16e..d41dc4509d0f 100644 --- a/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml +++ b/duckchat/duckchat-impl/src/main/res/drawable/ic_chat_history_unchecked_24.xml @@ -4,7 +4,7 @@ android:viewportWidth="24" android:viewportHeight="24"> diff --git a/duckchat/duckchat-impl/src/main/res/drawable/selector_chat_history_select_all_indicator.xml b/duckchat/duckchat-impl/src/main/res/drawable/selector_chat_history_select_all_indicator.xml new file mode 100644 index 000000000000..60738c892459 --- /dev/null +++ b/duckchat/duckchat-impl/src/main/res/drawable/selector_chat_history_select_all_indicator.xml @@ -0,0 +1,22 @@ + + + + + + diff --git a/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml b/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml index ffa1059b14d6..382c0056565a 100644 --- a/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml +++ b/duckchat/duckchat-impl/src/main/res/layout/view_chat_history_select_all.xml @@ -15,11 +15,12 @@ android:id="@+id/chatHistorySelectAllIcon" android:layout_width="24dp" android:layout_height="24dp" + android:duplicateParentState="true" android:importantForAccessibility="no" + android:src="@drawable/selector_chat_history_select_all_indicator" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintStart_toStartOf="parent" - app:layout_constraintTop_toTopOf="parent" - tools:src="@drawable/ic_chat_history_unchecked_24" /> + app:layout_constraintTop_toTopOf="parent" /> Date: Mon, 18 May 2026 16:59:08 +0200 Subject: [PATCH 18/25] Flip chat-history row icon when selection toggles Animate a 300ms Y-axis flip on the row's icon container when the user toggles a row in or out of selection, swapping the chat-type drawable and accent background at the apex. The flip runs only on selection changes detected via DiffUtil.getChangePayload and routed through the 3-arg onBindViewHolder, so scrolling and full rebinds stay snappy. --- .../impl/history/ChatHistoryAdapter.kt | 23 ++++++++++++ .../impl/history/ChatHistoryViewHolder.kt | 35 +++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt index 928609205dd6..d55c31a3fbfe 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryAdapter.kt @@ -58,6 +58,20 @@ class ChatHistoryAdapter( } } + override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int, payloads: MutableList) { + val selectionChange = payloads.firstOrNull { it is RowChange.SelectionChanged } as? RowChange.SelectionChanged + val entry = getItem(position) + if (selectionChange != null && holder is ChatHistoryViewHolder && entry is ChatHistoryListEntry.Row) { + holder.animateSelectionChange(entry.item, selectionChange.selected) + } else { + onBindViewHolder(holder, position) + } + } + + private sealed interface RowChange { + data class SelectionChanged(val selected: Boolean) : RowChange + } + private object Diff : DiffUtil.ItemCallback() { override fun areItemsTheSame(oldItem: ChatHistoryListEntry, newItem: ChatHistoryListEntry): Boolean = when { oldItem is ChatHistoryListEntry.Header && newItem is ChatHistoryListEntry.Header -> @@ -70,6 +84,15 @@ class ChatHistoryAdapter( override fun areContentsTheSame(oldItem: ChatHistoryListEntry, newItem: ChatHistoryListEntry): Boolean = oldItem == newItem + + override fun getChangePayload(oldItem: ChatHistoryListEntry, newItem: ChatHistoryListEntry): Any? { + if (oldItem is ChatHistoryListEntry.Row && newItem is ChatHistoryListEntry.Row && + oldItem.item == newItem.item && oldItem.selected != newItem.selected + ) { + return RowChange.SelectionChanged(newItem.selected) + } + return null + } } private companion object { diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt index b501005da9bc..0d6235865175 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewHolder.kt @@ -19,6 +19,8 @@ package com.duckduckgo.duckchat.impl.history import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import android.view.animation.AccelerateInterpolator +import android.view.animation.DecelerateInterpolator import android.widget.FrameLayout import android.widget.ImageView import androidx.recyclerview.widget.RecyclerView @@ -41,7 +43,34 @@ class ChatHistoryViewHolder( onMoreClick: (ChatHistoryItem, View) -> Unit, onLongClick: (ChatHistoryItem) -> Boolean = { false }, ) { + iconContainer.animate().cancel() + iconContainer.rotationY = 0f title.text = item.displayTitle + applySelectionState(item, selected) + itemView.setOnClickListener { onClick(item) } + itemView.setOnLongClickListener { onLongClick(item) } + moreButton.setOnClickListener { anchor -> onMoreClick(item, anchor) } + } + + fun animateSelectionChange(item: ChatHistoryItem, selected: Boolean) { + iconContainer.animate().cancel() + iconContainer.animate() + .rotationY(HALF_FLIP_DEGREES) + .setDuration(HALF_FLIP_DURATION_MS) + .setInterpolator(AccelerateInterpolator()) + .withEndAction { + applySelectionState(item, selected) + iconContainer.rotationY = -HALF_FLIP_DEGREES + iconContainer.animate() + .rotationY(0f) + .setDuration(HALF_FLIP_DURATION_MS) + .setInterpolator(DecelerateInterpolator()) + .start() + } + .start() + } + + private fun applySelectionState(item: ChatHistoryItem, selected: Boolean) { if (selected) { iconContainer.setBackgroundResource(R.drawable.bg_chat_history_circle_accent) typeIcon.setImageResource(com.duckduckgo.mobile.android.R.drawable.ic_check_24) @@ -53,9 +82,6 @@ class ChatHistoryViewHolder( typeIcon.colorFilter = null iconContainer.contentDescription = null } - itemView.setOnClickListener { onClick(item) } - itemView.setOnLongClickListener { onLongClick(item) } - moreButton.setOnClickListener { anchor -> onMoreClick(item, anchor) } } private fun iconFor(type: ChatType, pinned: Boolean): Int = when (type) { @@ -65,6 +91,9 @@ class ChatHistoryViewHolder( } companion object { + private const val HALF_FLIP_DEGREES = 90f + private const val HALF_FLIP_DURATION_MS = 150L + fun create(parent: ViewGroup): ChatHistoryViewHolder { val view = LayoutInflater.from(parent.context) .inflate(R.layout.view_chat_history_item, parent, false) From 8c012a7c6896854175198173f3304d02467660b4 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 17:09:16 +0200 Subject: [PATCH 19/25] Apply spotless formatting to multi-line when arm --- .../duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index 9745750d45fd..004ab33a1ad9 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -105,7 +105,8 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { val confirmation = (viewModel.uiState.value as? ChatHistoryUiState.Loaded)?.confirmation when (event) { FireDialog.EVENT_ON_CLEAR_STARTED, - FireDialog.EVENT_CLEAR_WITHOUT_RESTART_STARTED -> when (confirmation) { + FireDialog.EVENT_CLEAR_WITHOUT_RESTART_STARTED, + -> when (confirmation) { is ChatHistoryUiState.PendingConfirmation.FireAll -> viewModel.onFireAllConfirmed() is ChatHistoryUiState.PendingConfirmation.DeleteSelected -> viewModel.onDeleteSelectedConfirmed() null -> Unit From 29027b46d9db8242d2ec1f85b349d494366dec50 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 11:19:01 +0200 Subject: [PATCH 20/25] Cleanup after a mistake with latest rebase --- .../view/SingleTabFireDialogViewModel.kt | 1 - .../impl/history/ChatHistoryViewModelTest.kt | 19 ------------------- 2 files changed, 20 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt index a0fc0a4b4c93..15a4f745e3e5 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt @@ -353,7 +353,6 @@ class SingleTabFireDialogViewModel @Inject constructor( val isFirePictogramVisible: Boolean = true, val isFireAnimationUpdateEnabled: Boolean = false, val titleSource: TitleSource = TitleSource.Static(R.string.singleTabFireDialogTitle), - val fireOptionsOverride: Set? = null, ) } } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index bf9d03758d29..1804c52a4fa0 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -697,25 +697,6 @@ class ChatHistoryViewModelTest { assertTrue(repository.deletedChatIds.isEmpty()) } - @Test - fun `concurrent repository emission removing a selected id reconciles the selection`() = runTest { - source.value = listOf(item("a"), item("b"), item("c")) - - viewModel.uiState.test { - awaitInitialLoaded() - viewModel.onEnterSelectMode() - awaitItem() - viewModel.onSelectAllToggled() - awaitItem() // Selecting({a, b, c}) - - // Simulate an external delete (e.g. omnibar Fire-icon) removing chat "b" - source.value = listOf(item("a"), item("c")) - cancelAndConsumeRemainingEvents() - } - val mode = (viewModel.uiState.value as Loaded).mode as ChatHistoryUiState.Mode.Selecting - assertEquals(setOf("a", "c"), mode.selectedChatIds) - } - /** * `stateIn(WhileSubscribed)` does not guarantee subscribers observe the `Loading` initial * value — the upstream may emit before the StateFlow can replay it. Tolerate both orderings. From d1adba91ea176ffdaee1b35bd275b2eaadabd58e Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 14:37:37 +0200 Subject: [PATCH 21/25] Include Pinned chats in Fire-all from chat history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Internal discussion landed on: Fire-all should wipe every Duck.ai chat, Pinned included. onFireAllRequested now operates on the full item list rather than the non-pinned subset; N=1 fast-path covers a single pinned chat too, and N≥2 builds the confirmation set from every chatId. The fragment shows the fire icon whenever any chat is present, not just when Recent is non-empty. --- .../impl/history/ChatHistoryFragment.kt | 4 +- .../impl/history/ChatHistoryViewModel.kt | 10 ++-- .../impl/history/ChatHistoryViewModelTest.kt | 56 +++++++++++++------ 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt index 004ab33a1ad9..4dd3808f47fa 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryFragment.kt @@ -147,8 +147,8 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { setFireActionVisible(selectMode.selectedChatIds.isNotEmpty()) } else { applyDefaultToolbar() - // Hide fire when Recent is empty — title is "Delete N chats?" of the Recent count. - setFireActionVisible(state.recent.isNotEmpty()) + // Fire-all wipes every chat including Pinned — show whenever any chat is present. + setFireActionVisible(state.pinned.isNotEmpty() || state.recent.isNotEmpty()) } renderConfirmation(state.confirmation) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index fcf2f7b0f0ca..d78a899d42b0 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -122,14 +122,14 @@ class ChatHistoryViewModel @Inject constructor( controls.update { it.copy(search = SearchState()) } } - /** N=1 spares Pinned; N≥2 routes through the dialog, which wipes every Duck.ai chat. */ + /** Fire-all wipes every Duck.ai chat including Pinned. N=1 fires directly; N≥2 confirms via dialog. */ fun onFireAllRequested() { - val recent = latestItems.filter { !it.pinned } + val all = latestItems when { - recent.isEmpty() -> Unit - recent.size == 1 -> dispatchSelectedClear(setOf(recent.single().chatId)) + all.isEmpty() -> Unit + all.size == 1 -> dispatchSelectedClear(setOf(all.single().chatId)) else -> controls.update { - it.copy(confirmation = PendingConfirmation.FireAll(chatIds = recent.mapTo(mutableSetOf()) { i -> i.chatId })) + it.copy(confirmation = PendingConfirmation.FireAll(chatIds = all.mapTo(mutableSetOf()) { i -> i.chatId })) } } } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 1804c52a4fa0..ca01c44b80d7 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -217,7 +217,7 @@ class ChatHistoryViewModelTest { // --- Fire-all --- @Test - fun `onFireAllRequested with two or more Recent chats sets FireAll confirmation with the Recent count`() = runTest { + fun `onFireAllRequested with two or more chats sets FireAll confirmation with every chatId including pinned`() = runTest { source.value = listOf( item("p", pinned = true), item("r1"), @@ -231,17 +231,17 @@ class ChatHistoryViewModelTest { viewModel.onFireAllRequested() val confirming = awaitItem() as Loaded - assertEquals(ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("r1", "r2", "r3")), confirming.confirmation) + assertEquals( + ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("p", "r1", "r2", "r3")), + confirming.confirmation, + ) assertTrue(repository.deletedChatIds.isEmpty()) } } @Test - fun `onFireAllRequested with exactly one Recent chat dispatches DuckChats Selected with that chat url`() = runTest { - source.value = listOf( - item("p", pinned = true), - item("r1"), - ) + fun `onFireAllRequested with exactly one recent chat dispatches DuckChats Selected with that chat url`() = runTest { + source.value = listOf(item("r1")) viewModel.uiState.test { awaitInitialLoaded() @@ -258,19 +258,40 @@ class ChatHistoryViewModelTest { } @Test - fun `onFireAllRequested with no Recent chats is a no-op`() = runTest { + fun `onFireAllRequested with exactly one pinned chat dispatches DuckChats Selected with that chat url`() = runTest { + source.value = listOf(item("p", pinned = true)) + + viewModel.uiState.test { + awaitInitialLoaded() + + viewModel.onFireAllRequested() + + expectNoEvents() + } + assertEquals( + listOf(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=p")))), + dataClearingTrigger.calls, + ) + assertTrue(repository.deletedChatIds.isEmpty()) + } + + @Test + fun `onFireAllRequested with only pinned chats and no recent sets FireAll confirmation with the pinned ids`() = runTest { source.value = listOf( item("p1", pinned = true), item("p2", pinned = true), ) viewModel.uiState.test { - val initial = awaitInitialLoaded() - assertEquals(null, initial.confirmation) + awaitInitialLoaded() viewModel.onFireAllRequested() - expectNoEvents() + val confirming = awaitItem() as Loaded + assertEquals( + ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("p1", "p2")), + confirming.confirmation, + ) assertTrue(repository.deletedChatIds.isEmpty()) } } @@ -289,7 +310,7 @@ class ChatHistoryViewModelTest { awaitInitialLoaded() viewModel.onFireAllRequested() - awaitItem() // confirmation = FireAll(3) + awaitItem() // confirmation = FireAll(5) — p1, p2, r1, r2, r3 viewModel.onFireAllConfirmed() @@ -337,12 +358,13 @@ class ChatHistoryViewModelTest { awaitInitialLoaded() viewModel.onFireAllRequested() - awaitItem() // confirmation = FireAll(2) + awaitItem() // confirmation = FireAll(3) — p, r1, r2 viewModel.onFireAllConfirmed() awaitItem() // confirmation cleared } - // ViewModel never touches the repository on the dialog path — production wipes Pinned too. + // ViewModel never touches the repository on the dialog path — production wipes every chat + // including Pinned via the dialog-driven Selected dispatch. assertEquals(false, repository.deleteAllChatsCalled) assertTrue(repository.deletedChatIds.isEmpty()) } @@ -661,17 +683,17 @@ class ChatHistoryViewModelTest { } @Test - fun `chatUrlsForDialog returns Recent URLs while a FireAll confirmation is pending`() = runTest { + fun `chatUrlsForDialog returns every chat URL including pinned while a FireAll confirmation is pending`() = runTest { source.value = listOf(item("p", pinned = true), item("r1"), item("r2")) viewModel.uiState.test { awaitInitialLoaded() viewModel.onFireAllRequested() - awaitItem() // FireAll({r1, r2}) + awaitItem() // FireAll({p, r1, r2}) assertEquals( - setOf("https://duck.ai?chatID=r1", "https://duck.ai?chatID=r2"), + setOf("https://duck.ai?chatID=p", "https://duck.ai?chatID=r1", "https://duck.ai?chatID=r2"), viewModel.chatUrlsForDialog(), ) } From 3aa237fcff951cedb7e9c2fe743c41704dc59bed Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 22:36:16 +0200 Subject: [PATCH 22/25] Split chat-history bulk delete off the fire-button path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add onDeleteSelectedChatsClicked() for the ChatHistory dialog origin so chat-only deletes no longer ride the fire-button pixels, daily pixel, fire-button use count, FIRE_BUTTON_EXECUTED user event, or DataClearingWideEvent — those describe the fire button, not a chat subset wipe. The fragment routes the delete-all buttons to the new function when ARG_ORIGIN is CHAT_HISTORY; everything else still calls onDeleteAllClicked unchanged. --- .../app/global/view/SingleTabFireDialog.kt | 12 +- .../view/SingleTabFireDialogViewModel.kt | 29 +++-- .../view/SingleTabFireDialogViewModelTest.kt | 109 +++++++++++++++++- 3 files changed, 139 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt index 0a1ab5795b6b..eaca44d74e34 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialog.kt @@ -176,11 +176,11 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { private fun setupLayout() { binding.deleteAllPrimaryButton.setOnClickListener { hideDialog() - viewModel.onDeleteAllClicked() + dispatchDeleteAll() } binding.deleteAllSecondaryButton.setOnClickListener { hideDialog() - viewModel.onDeleteAllClicked() + dispatchDeleteAll() } binding.deleteThisTabButton.setOnClickListener { hideDialog() @@ -188,6 +188,14 @@ class SingleTabFireDialog : BottomSheetDialogFragment(), FireDialog { } } + private fun dispatchDeleteAll() { + if (arguments?.getString(ARG_ORIGIN) == ORIGIN_CHAT_HISTORY) { + viewModel.onDeleteSelectedChatsClicked() + } else { + viewModel.onDeleteAllClicked() + } + } + private fun configureBottomSheet() { (dialog as? BottomSheetDialog)?.behavior?.apply { state = BottomSheetBehavior.STATE_EXPANDED diff --git a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt index 15a4f745e3e5..be473a9b5f73 100644 --- a/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModel.kt @@ -146,19 +146,13 @@ class SingleTabFireDialogViewModel @Inject constructor( withContext(dispatcherProvider.io()) { fireButtonStore.incrementFireButtonUseCount() userEventsStore.registerUserEvent(UserEventKey.FIRE_BUTTON_EXECUTED) - val selectedChatUrls = (origin.value as? FireDialogOrigin.ChatHistory)?.selectedChatUrls val clearOptions = fireDataStore.getManualClearOptions() dataClearingWideEvent.start( entryPoint = DataClearingWideEvent.EntryPoint.SINGLE_TAB_FIRE_DIALOG, clearOptions = clearOptions, ) try { - if (selectedChatUrls != null) { - shouldRestartAfterClearing = false - dataClearing.clearSelectedDuckAiChats(selectedChatUrls) - } else { - dataClearing.clearDataUsingManualFireOptions() - } + dataClearing.clearDataUsingManualFireOptions() dataClearingWideEvent.finishSuccess() } catch (e: Exception) { dataClearingWideEvent.finishFailure(e) @@ -170,6 +164,27 @@ class SingleTabFireDialogViewModel @Inject constructor( } } + fun onDeleteSelectedChatsClicked() { + val selectedChatUrls = (origin.value as? FireDialogOrigin.ChatHistory)?.selectedChatUrls ?: return + viewModelScope.launch { + shouldRestartAfterClearing = false + command.send(Command.OnClearStarted) + + val fireAnimationEnabled = withContext(dispatcherProvider.io()) { + settingsDataStore.fireAnimationEnabled + } + if (fireAnimationEnabled) { + command.send(Command.PlayAnimation) + } + + withContext(dispatcherProvider.io()) { + dataClearing.clearSelectedDuckAiChats(selectedChatUrls) + } + + command.send(Command.ClearingComplete) + } + } + fun onDeleteThisTabClicked() { viewModelScope.launch { shouldRestartAfterClearing = false diff --git a/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt index 4d1e2306feb5..df73f206733f 100644 --- a/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt +++ b/app/src/test/java/com/duckduckgo/app/global/view/SingleTabFireDialogViewModelTest.kt @@ -1099,21 +1099,126 @@ class SingleTabFireDialogViewModelTest { } } + // endregion + + // region onDeleteSelectedChatsClicked + @Test - fun `when delete all clicked from ChatHistory origin then clearSelectedDuckAiChats is dispatched and process is not restarted`() = runTest { + fun `when delete selected chats clicked then clearSelectedDuckAiChats is dispatched with the origin urls`() = runTest { val urls = setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b") testee = createViewModel() testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = urls)) - testee.onDeleteAllClicked() + testee.onDeleteSelectedChatsClicked() coroutineTestRule.testScope.testScheduler.advanceUntilIdle() verify(mockDataClearing).clearSelectedDuckAiChats(urls) verify(mockDataClearing, never()).clearDataUsingManualFireOptions(any(), any()) + } + + @Test + fun `when delete selected chats clicked then process is not restarted`() = runTest { + val urls = setOf("https://duck.ai?chatID=a") + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = urls)) + + testee.onDeleteSelectedChatsClicked() + + coroutineTestRule.testScope.testScheduler.advanceUntilIdle() + assertFalse(testee.shouldRestartAfterClearing) } + @Test + fun `when delete selected chats clicked then no fire-dialog pixels are fired`() = runTest { + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = setOf("https://duck.ai?chatID=a"))) + + testee.onDeleteSelectedChatsClicked() + + coroutineTestRule.testScope.testScheduler.advanceUntilIdle() + + verify(mockPixel, never()).enqueueFire(eq(FIRE_DIALOG_CLEAR_PRESSED), any(), any(), any()) + verify(mockPixel, never()).enqueueFire(eq(FIRE_DIALOG_CLEAR_PRESSED_DAILY), any(), any(), any()) + verify(mockPixel, never()).enqueueFire(eq(PRODUCT_TELEMETRY_SURFACE_DATA_CLEARING), any(), any(), any()) + verify(mockPixel, never()).enqueueFire(eq(FIRE_DIALOG_ANIMATION), any(), any(), any()) + } + + @Test + fun `when delete selected chats clicked then fire button counters are not touched`() = runTest { + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = setOf("https://duck.ai?chatID=a"))) + + testee.onDeleteSelectedChatsClicked() + + coroutineTestRule.testScope.testScheduler.advanceUntilIdle() + + verify(mockFireButtonStore, never()).incrementFireButtonUseCount() + verify(mockUserEventsStore, never()).registerUserEvent(UserEventKey.FIRE_BUTTON_EXECUTED) + } + + @Test + fun `when delete selected chats clicked then data clearing wide event is not started`() = runTest { + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = setOf("https://duck.ai?chatID=a"))) + + testee.onDeleteSelectedChatsClicked() + + coroutineTestRule.testScope.testScheduler.advanceUntilIdle() + + verify(mockDataClearingWideEvent, never()).start(any(), any()) + verify(mockDataClearingWideEvent, never()).finishSuccess() + } + + @Test + fun `when delete selected chats clicked with animation enabled then play animation command is sent`() = runTest { + whenever(mockSettingsDataStore.fireAnimationEnabled).thenReturn(true) + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = setOf("https://duck.ai?chatID=a"))) + + testee.commands().test { + testee.onDeleteSelectedChatsClicked() + + awaitItem() // OnShow from init + assertEquals(Command.OnClearStarted, awaitItem()) + assertEquals(Command.PlayAnimation, awaitItem()) + assertEquals(Command.ClearingComplete, awaitItem()) + + cancelAndConsumeRemainingEvents() + } + } + + @Test + fun `when delete selected chats clicked with animation disabled then play animation command is not sent`() = runTest { + whenever(mockSettingsDataStore.fireAnimationEnabled).thenReturn(false) + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.ChatHistory(selectedChatUrls = setOf("https://duck.ai?chatID=a"))) + + testee.commands().test { + testee.onDeleteSelectedChatsClicked() + + awaitItem() // OnShow from init + assertEquals(Command.OnClearStarted, awaitItem()) + assertEquals(Command.ClearingComplete, awaitItem()) + + cancelAndConsumeRemainingEvents() + } + } + + @Test + fun `when delete selected chats clicked without ChatHistory origin then nothing happens`() = runTest { + testee = createViewModel() + testee.setOrigin(FireDialogOrigin.Browser) + + testee.onDeleteSelectedChatsClicked() + + coroutineTestRule.testScope.testScheduler.advanceUntilIdle() + + verify(mockDataClearing, never()).clearSelectedDuckAiChats(any()) + verify(mockDataClearing, never()).clearDataUsingManualFireOptions(any(), any()) + } + // endregion // region onDeleteThisTabClicked From b3569d9ff2446b04e836cd43dcda79ec49ab4abc Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 22:41:50 +0200 Subject: [PATCH 23/25] Always confirm Fire-all from chat history Drop the N=1 short-circuit so tapping the Fire icon outside select mode always opens the "Delete N chats?" dialog, even when only one chat exists. Brings the Fire-all entry point in line with the destructive- action affordance the icon implies. Per-row Delete and N=1 selected- delete keep their immediate-execution behaviour. --- .../impl/history/ChatHistoryViewModel.kt | 11 +++----- .../impl/history/ChatHistoryViewModelTest.kt | 26 ++++++++++--------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index d78a899d42b0..7db8747e5b7a 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -122,15 +122,12 @@ class ChatHistoryViewModel @Inject constructor( controls.update { it.copy(search = SearchState()) } } - /** Fire-all wipes every Duck.ai chat including Pinned. N=1 fires directly; N≥2 confirms via dialog. */ + /** Fire-all wipes every Duck.ai chat including Pinned — always confirms via dialog before deleting. */ fun onFireAllRequested() { val all = latestItems - when { - all.isEmpty() -> Unit - all.size == 1 -> dispatchSelectedClear(setOf(all.single().chatId)) - else -> controls.update { - it.copy(confirmation = PendingConfirmation.FireAll(chatIds = all.mapTo(mutableSetOf()) { i -> i.chatId })) - } + if (all.isEmpty()) return + controls.update { + it.copy(confirmation = PendingConfirmation.FireAll(chatIds = all.mapTo(mutableSetOf()) { i -> i.chatId })) } } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index ca01c44b80d7..750a3c983b38 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -240,7 +240,7 @@ class ChatHistoryViewModelTest { } @Test - fun `onFireAllRequested with exactly one recent chat dispatches DuckChats Selected with that chat url`() = runTest { + fun `onFireAllRequested with exactly one recent chat sets FireAll confirmation`() = runTest { source.value = listOf(item("r1")) viewModel.uiState.test { @@ -248,17 +248,18 @@ class ChatHistoryViewModelTest { viewModel.onFireAllRequested() - expectNoEvents() + val confirming = awaitItem() as Loaded + assertEquals( + ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("r1")), + confirming.confirmation, + ) } - assertEquals( - listOf(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=r1")))), - dataClearingTrigger.calls, - ) + assertTrue(dataClearingTrigger.calls.isEmpty()) assertTrue(repository.deletedChatIds.isEmpty()) } @Test - fun `onFireAllRequested with exactly one pinned chat dispatches DuckChats Selected with that chat url`() = runTest { + fun `onFireAllRequested with exactly one pinned chat sets FireAll confirmation`() = runTest { source.value = listOf(item("p", pinned = true)) viewModel.uiState.test { @@ -266,12 +267,13 @@ class ChatHistoryViewModelTest { viewModel.onFireAllRequested() - expectNoEvents() + val confirming = awaitItem() as Loaded + assertEquals( + ChatHistoryUiState.PendingConfirmation.FireAll(chatIds = setOf("p")), + confirming.confirmation, + ) } - assertEquals( - listOf(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=p")))), - dataClearingTrigger.calls, - ) + assertTrue(dataClearingTrigger.calls.isEmpty()) assertTrue(repository.deletedChatIds.isEmpty()) } From d458161282b3cddc52ef0bce9918e79a140aabc2 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 22:54:04 +0200 Subject: [PATCH 24/25] Reconcile chat-history selection inside reduce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fold the stale-id intersection into reduce so the uiState derivation is pure. The upstream observeChats no longer writes back into controls through reconcileSelection — the previous shape round-tripped through combine and made the dataflow harder to follow. onSelectAllToggled now filters its comparison through latestItems so a selection that lags a delete cannot skew the "all visible already selected?" check. --- .../impl/history/ChatHistoryViewModel.kt | 27 ++++-------- .../impl/history/ChatHistoryViewModelTest.kt | 43 +++++++++++++++++++ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index 7db8747e5b7a..bfceaafaa71c 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -52,10 +52,7 @@ class ChatHistoryViewModel @Inject constructor( private var latestItems: List = emptyList() val uiState: StateFlow = combine( - chatHistoryRepository.observeChats().onEach { items -> - latestItems = items - reconcileSelection(items) - }, + chatHistoryRepository.observeChats().onEach { latestItems = it }, controls, ::reduce, ).stateIn( @@ -64,18 +61,6 @@ class ChatHistoryViewModel @Inject constructor( initialValue = ChatHistoryUiState.Loading, ) - /** Intersect any selection with the current item IDs so concurrent deletes don't desync it. */ - private fun reconcileSelection(items: List) { - val current = controls.value.mode - if (current is Mode.Selecting && current.selectedChatIds.isNotEmpty()) { - val knownIds = items.mapTo(mutableSetOf()) { it.chatId } - val intersected = current.selectedChatIds.intersect(knownIds) - if (intersected.size != current.selectedChatIds.size) { - controls.update { it.copy(mode = Mode.Selecting(intersected)) } - } - } - } - fun isSelectMode(): Boolean = controls.value.mode is Mode.Selecting fun onChatRowClicked(chatId: String) { @@ -168,7 +153,9 @@ class ChatHistoryViewModel @Inject constructor( controls.update { c -> val mode = c.mode as? Mode.Selecting ?: return@update c val visibleIds = visibleChatIds(c.search) - val next = if (mode.selectedChatIds == visibleIds) emptySet() else visibleIds + // Filter to live ids — selection can lag deletes and skew the comparison. + val effectiveSelected = mode.selectedChatIds intersect latestItems.mapTo(mutableSetOf()) { it.chatId } + val next = if (effectiveSelected == visibleIds) emptySet() else visibleIds c.copy(mode = Mode.Selecting(next)) } } @@ -219,12 +206,16 @@ class ChatHistoryViewModel @Inject constructor( ): ChatHistoryUiState { if (items.isEmpty()) return ChatHistoryUiState.Empty val (pinned, recent) = items.partition { it.pinned } + val effectiveMode = when (val mode = controls.mode) { + is Mode.Selecting -> Mode.Selecting(mode.selectedChatIds intersect items.mapTo(mutableSetOf()) { it.chatId }) + Mode.Default -> Mode.Default + } return Loaded( pinned = pinned.sortedByDate().filterBy(controls.search), recent = recent.sortedByDate().filterBy(controls.search), searchQuery = controls.search.query, searchActive = controls.search.active, - mode = controls.mode, + mode = effectiveMode, confirmation = controls.confirmation, ) } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 750a3c983b38..298ab93438bf 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -545,6 +545,49 @@ class ChatHistoryViewModelTest { } } + @Test + fun `when an item disappears from the source the selection drops the stale id in the next state`() = runTest { + source.value = listOf(item("a"), item("b"), item("c")) + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + viewModel.onSelectionToggled("a") + awaitItem() // Selecting({a}) + viewModel.onSelectionToggled("b") + awaitItem() // Selecting({a, b}) + + source.value = listOf(item("a"), item("c")) + + val updated = awaitItem() as Loaded + val mode = updated.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(setOf("a"), mode.selectedChatIds) + } + } + + @Test + fun `onSelectAllToggled with a stale selection equal to visible still toggles off`() = runTest { + source.value = listOf(item("a"), item("b"), item("c")) + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + viewModel.onSelectAllToggled() + awaitItem() // Selecting({a, b, c}) + + source.value = listOf(item("a"), item("b")) + awaitItem() // mode reconciled to Selecting({a, b}) by reduce + + viewModel.onSelectAllToggled() + + val cleared = awaitItem() as Loaded + val mode = cleared.mode as ChatHistoryUiState.Mode.Selecting + assertEquals(emptySet(), mode.selectedChatIds) + } + } + @Test fun `onSelectModeCancelled returns to Default mode with no deletion`() = runTest { source.value = listOf(item("a"), item("b")) From 3f58bc311df1a5a579696ee6f6e7a50924c4c5ac Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 23:04:20 +0200 Subject: [PATCH 25/25] Gate chat-history dispatch on showClearDuckAIChatHistory The dialog path went through DataClearing.clearSelectedDuckAiChats, which already drops the request when the showClearDuckAIChatHistory remote-config flag is off. The ViewModel paths (per-row Delete, N=1 fire-all, N=1 selected-delete) dispatched DuckChats.Selected through DataClearingTrigger directly and bypassed that check, so chats could still be deleted one at a time with the flag off. Apply the same gate inline so both paths agree. --- .../impl/history/ChatHistoryViewModel.kt | 3 ++ .../impl/history/ChatHistoryViewModelTest.kt | 43 ++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt index bfceaafaa71c..fc9f9f9b75ff 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModel.kt @@ -23,6 +23,7 @@ import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.dataclearing.api.plugin.ClearableData import com.duckduckgo.dataclearing.api.plugin.DataClearingTrigger import com.duckduckgo.di.scopes.FragmentScope +import com.duckduckgo.duckchat.api.DuckAiFeatureState import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Mode @@ -44,6 +45,7 @@ class ChatHistoryViewModel @Inject constructor( @AppCoroutineScope private val appScope: CoroutineScope, private val duckChat: DuckChatInternal, private val dataClearingTrigger: DataClearingTrigger, + private val duckAiFeatureState: DuckAiFeatureState, ) : ViewModel() { private val controls = MutableStateFlow(UiControls()) @@ -123,6 +125,7 @@ class ChatHistoryViewModel @Inject constructor( private fun dispatchSelectedClear(chatIds: Set) { if (chatIds.isEmpty()) return + if (!duckAiFeatureState.showClearDuckAIChatHistory.value) return val urls = chatIds.mapTo(mutableSetOf()) { duckChat.buildChatUrl(it) } appScope.launch { dataClearingTrigger.clearData(setOf(ClearableData.DuckChats.Selected(urls))) diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt index 298ab93438bf..bde85aebf831 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt @@ -21,6 +21,7 @@ import app.cash.turbine.test import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.dataclearing.api.plugin.ClearableData import com.duckduckgo.dataclearing.api.plugin.DataClearingTrigger +import com.duckduckgo.duckchat.api.DuckAiFeatureState import com.duckduckgo.duckchat.impl.history.ChatHistoryUiState.Loaded import com.duckduckgo.duckchat.impl.messaging.fakes.FakeDuckChatInternal import kotlinx.coroutines.flow.Flow @@ -30,6 +31,8 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever class ChatHistoryViewModelTest { @@ -40,7 +43,11 @@ class ChatHistoryViewModelTest { private val repository = FakeChatHistoryRepository(source) private val duckChat = FakeDuckChatInternal() private val dataClearingTrigger = RecordingDataClearingTrigger() - private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope, duckChat, dataClearingTrigger) + private val showClearDuckAIChatHistoryFlow = MutableStateFlow(true) + private val duckAiFeatureState: DuckAiFeatureState = mock { + whenever(it.showClearDuckAIChatHistory).thenReturn(showClearDuckAIChatHistoryFlow) + } + private val viewModel = ChatHistoryViewModel(repository, coroutineRule.testScope, duckChat, dataClearingTrigger, duckAiFeatureState) @Test fun `initial state is Loading`() = coroutineRule.testScope.runTest { @@ -764,6 +771,40 @@ class ChatHistoryViewModelTest { assertTrue(repository.deletedChatIds.isEmpty()) } + @Test + fun `onDeleteSingleChat is a no-op when showClearDuckAIChatHistory is off`() = runTest { + source.value = listOf(item("a")) + showClearDuckAIChatHistoryFlow.value = false + + viewModel.uiState.test { + awaitInitialLoaded() + + viewModel.onDeleteSingleChat("a") + + expectNoEvents() + } + assertTrue(dataClearingTrigger.calls.isEmpty()) + } + + @Test + fun `onDeleteSelectedRequested with one selection is a no-op when showClearDuckAIChatHistory is off`() = runTest { + source.value = listOf(item("a")) + showClearDuckAIChatHistoryFlow.value = false + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onEnterSelectMode() + awaitItem() // Selecting({}) + viewModel.onSelectionToggled("a") + awaitItem() // Selecting({a}) + + viewModel.onDeleteSelectedRequested() + + awaitItem() // mode reset to Default before dispatch (which is gated off) + } + assertTrue(dataClearingTrigger.calls.isEmpty()) + } + /** * `stateIn(WhileSubscribed)` does not guarantee subscribers observe the `Loading` initial * value — the upstream may emit before the StateFlow can replay it. Tolerate both orderings.