From 3fd7b551b66fe6d1278fa9a10c7a58376e4ad7da Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 15 May 2026 23:02:13 +0200 Subject: [PATCH 1/4] Wire per-row Pin/Unpin in chat history overflow menu --- .../impl/history/ChatHistoryFragment.kt | 6 ++- .../impl/history/ChatHistoryRepository.kt | 5 ++ .../impl/history/ChatHistoryViewModel.kt | 5 ++ .../impl/history/ChatHistoryViewModelTest.kt | 46 +++++++++++++++++++ .../store/impl/RealDuckAiChatStore.kt | 14 ++++++ .../store/impl/RealDuckAiChatStoreTest.kt | 39 ++++++++++++++++ 6 files changed, 114 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 403367e8a52b..78cc7aeede7a 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 @@ -29,6 +29,7 @@ import androidx.recyclerview.widget.LinearLayoutManager import com.duckduckgo.anvil.annotations.InjectWith import com.duckduckgo.common.ui.DuckDuckGoFragment import com.duckduckgo.common.ui.menu.PopupMenu +import com.duckduckgo.common.ui.view.PopupMenuItemView import com.duckduckgo.common.ui.view.SearchBar import com.duckduckgo.common.ui.view.gone import com.duckduckgo.common.ui.view.show @@ -286,7 +287,10 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { 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() } + val pinAction = view.findViewById(R.id.pin) + val pinLabel = if (item.pinned) R.string.duck_ai_chat_history_action_unpin else R.string.duck_ai_chat_history_action_pin + pinAction.setPrimaryText(getString(pinLabel)) + popup.onMenuItemClicked(pinAction) { viewModel.onTogglePin(item.chatId) } popup.onMenuItemClicked(view.findViewById(R.id.rename)) { viewModel.onRenameRequested(item.chatId, item.displayTitle) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt index 4226abdbfc9d..971d4aaa7d8e 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt @@ -36,6 +36,7 @@ interface ChatHistoryRepository { suspend fun deleteChat(chatId: String) suspend fun deleteAllChats() suspend fun renameChat(chatId: String, newTitle: String): Boolean + suspend fun setPinned(chatId: String, pinned: Boolean) } @ContributesBinding(AppScope::class) @@ -63,6 +64,10 @@ class RealChatHistoryRepository @Inject constructor( override suspend fun renameChat(chatId: String, newTitle: String): Boolean = withContext(dispatchers.io()) { chatStore.renameChat(chatId, newTitle) } + override suspend fun setPinned(chatId: String, pinned: Boolean) { + withContext(dispatchers.io()) { chatStore.setPinned(chatId, pinned) } + } + private fun toChatHistoryItem(chat: DuckAiChat): ChatHistoryItem = ChatHistoryItem( chatId = chat.chatId, displayTitle = chat.title.takeIf { it.isNotBlank() && it != UPSTREAM_UNTITLED } ?: fallbackTitle, 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 62fbd178cf03..929a48589870 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 @@ -134,6 +134,11 @@ class ChatHistoryViewModel @Inject constructor( navigationChannel.trySend(NavigationEvent.OpenRename(chatId = chatId, currentTitle = currentTitle)) } + fun onTogglePin(chatId: String) { + val current = latestItems.firstOrNull { it.chatId == chatId } ?: return + appScope.launch { chatHistoryRepository.setPinned(chatId, !current.pinned) } + } + private fun dispatchSelectedClear(chatIds: Set) { if (chatIds.isEmpty()) return if (!duckAiFeatureState.showClearDuckAIChatHistory.value) return 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 ae28ffaa1198..ea084cfe0ea7 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 @@ -818,6 +818,46 @@ class ChatHistoryViewModelTest { } } + @Test + fun `onTogglePin flips pinned to true when row was unpinned`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = false)) + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onTogglePin("a") + val updated = awaitItem() as Loaded + assertEquals(listOf("a"), updated.pinned.map { it.chatId }) + assertEquals(emptyList(), updated.recent.map { it.chatId }) + } + assertEquals(listOf("a" to true), repository.pinnedChats) + } + + @Test + fun `onTogglePin flips pinned to false when row was pinned`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = true)) + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onTogglePin("a") + val updated = awaitItem() as Loaded + assertEquals(emptyList(), updated.pinned.map { it.chatId }) + assertEquals(listOf("a"), updated.recent.map { it.chatId }) + } + assertEquals(listOf("a" to false), repository.pinnedChats) + } + + @Test + fun `onTogglePin is a no-op when chatId is unknown`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = false)) + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onTogglePin("missing") + expectNoEvents() + } + assertTrue(repository.pinnedChats.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. @@ -851,6 +891,7 @@ private class FakeChatHistoryRepository( ) : ChatHistoryRepository { val deletedChatIds: MutableList = mutableListOf() val renamedChats: MutableList> = mutableListOf() + val pinnedChats: MutableList> = mutableListOf() var deleteAllChatsCalled: Boolean = false private set @@ -870,6 +911,11 @@ private class FakeChatHistoryRepository( renamedChats += chatId to newTitle return true } + + override suspend fun setPinned(chatId: String, pinned: Boolean) { + pinnedChats += chatId to pinned + source.value = source.value.map { if (it.chatId == chatId) it.copy(pinned = pinned) else it } + } } private class RecordingDataClearingTrigger : DataClearingTrigger { diff --git a/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt b/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt index c5436ab5f268..5b7391dbaf16 100644 --- a/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt +++ b/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt @@ -66,6 +66,9 @@ interface DuckAiChatStore { /** Renames [chatId] in the FE-owned JSON blob. Returns true if the chat existed and was updated, false otherwise. */ suspend fun renameChat(chatId: String, newTitle: String): Boolean + + /** Sets the pinned flag on [chatId]. Returns true if the chat existed and was updated, false otherwise. */ + suspend fun setPinned(chatId: String, pinned: Boolean): Boolean } @SingleInstanceIn(AppScope::class) @@ -143,6 +146,17 @@ class RealDuckAiChatStore @Inject constructor( true } + override suspend fun setPinned(chatId: String, pinned: Boolean): Boolean = + withContext(dispatchers.io()) { + logcat { "DuckAI: RealDuckAiChatStore.setPinned($chatId, $pinned)" } + val entity = chatsDao.getById(chatId) ?: return@withContext false + val updatedJson = runCatching { + JSONObject(entity.data).put("pinned", pinned).toString() + }.getOrNull() ?: return@withContext false + chatsDao.upsert(entity.copy(data = updatedJson)) + true + } + private fun DuckAiBridgeChatEntity.toDuckAiChat(): DuckAiChat? = runCatching { val json = JSONObject(data) val chatId = json.optString("chatId").takeIf { it.isNotEmpty() } ?: return@runCatching null diff --git a/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt b/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt index 3e4db9f819dc..907be0ffaf9a 100644 --- a/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt +++ b/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt @@ -336,4 +336,43 @@ class RealDuckAiChatStoreTest { // traversal file should not be deleted — it's outside filesDir verify(chatsDao).deleteAll() } + + // --- setPinned --- + + @Test + fun `setPinned returns false when chat not found`() = runTest { + whenever(chatsDao.getById("missing")).thenReturn(null) + + assertFalse(store.setPinned("missing", true)) + verify(chatsDao, never()).upsert(any()) + } + + @Test + fun `setPinned updates only the pinned flag and preserves other JSON fields`() = runTest { + val originalJson = """ + {"chatId":"abc","title":"Old","model":"gpt-5-mini","lastEdit":"2026-04-01T21:31:54.260Z","pinned":false,"fileRefs":["uuid1"],"messages":[{"role":"user","text":"hi"}]} + """.trimIndent() + whenever(chatsDao.getById("abc")).thenReturn(DuckAiBridgeChatEntity("abc", originalJson)) + + assertTrue(store.setPinned("abc", true)) + + val entityCaptor = argumentCaptor() + verify(chatsDao).upsert(entityCaptor.capture()) + val json = JSONObject(entityCaptor.firstValue.data) + assertTrue(json.getBoolean("pinned")) + assertEquals("Old", json.getString("title")) + assertEquals("abc", json.getString("chatId")) + assertEquals("gpt-5-mini", json.getString("model")) + assertEquals("2026-04-01T21:31:54.260Z", json.getString("lastEdit")) + assertEquals("uuid1", json.getJSONArray("fileRefs").getString(0)) + assertEquals("hi", json.getJSONArray("messages").getJSONObject(0).getString("text")) + } + + @Test + fun `setPinned returns false when stored JSON is malformed`() = runTest { + whenever(chatsDao.getById("abc")).thenReturn(DuckAiBridgeChatEntity("abc", "not a json")) + + assertFalse(store.setPinned("abc", true)) + verify(chatsDao, never()).upsert(any()) + } } From 2df6f977e81a4f49e5267e25bfdde956bf192bec Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Mon, 18 May 2026 18:00:50 +0200 Subject: [PATCH 2/4] Show Pin/Unpin snackbar with Undo action Surfaces a snackbar after the per-row Pin/Unpin overflow action so the user can recover from a mistap. The toggle still fires immediately; the snackbar is purely informational + an Undo affordance that re-issues setPinned with the previous value. Also fixes a pre-existing build break in RecordingRenameRepository, which was missing the setPinned override added in b3157b93c6. --- .../impl/history/ChatHistoryFragment.kt | 24 ++++++++ .../impl/history/ChatHistoryViewModel.kt | 15 ++++- .../src/main/res/values/donottranslate.xml | 3 + .../impl/history/ChatHistoryViewModelTest.kt | 60 +++++++++++++++++++ 4 files changed, 101 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 78cc7aeede7a..8cba71f26ea5 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 @@ -125,6 +125,11 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { .flowWithLifecycle(viewLifecycleOwner.lifecycle, Lifecycle.State.STARTED) .onEach(::onNavigationEvent) .launchIn(viewLifecycleOwner.lifecycleScope) + + viewModel.messageEvents + .flowWithLifecycle(viewLifecycleOwner.lifecycle, Lifecycle.State.STARTED) + .onEach(::onMessageEvent) + .launchIn(viewLifecycleOwner.lifecycleScope) } private fun onNavigationEvent(event: ChatHistoryViewModel.NavigationEvent) { @@ -133,6 +138,12 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { } } + private fun onMessageEvent(event: ChatHistoryViewModel.MessageEvent) { + when (event) { + is ChatHistoryViewModel.MessageEvent.PinToggled -> showPinToggledSnackbar(event) + } + } + private fun openRenameScreen(chatId: String, currentTitle: String) { parentFragmentManager.beginTransaction() .replace(R.id.chatHistoryFragmentContainer, RenameChatFragment.newInstance(chatId, currentTitle)) @@ -140,6 +151,19 @@ class ChatHistoryFragment : DuckDuckGoFragment(R.layout.fragment_chat_history) { .commit() } + private fun showPinToggledSnackbar(event: ChatHistoryViewModel.MessageEvent.PinToggled) { + val messageRes = if (event.wasPinned) { + R.string.duck_ai_chat_history_unpin_snackbar + } else { + R.string.duck_ai_chat_history_pin_snackbar + } + Snackbar.make(binding.root, messageRes, Snackbar.LENGTH_LONG) + .setAction(R.string.duck_ai_chat_history_undo) { + viewModel.onUndoTogglePin(event.chatId, restorePinned = event.wasPinned) + } + .show() + } + private fun render(state: ChatHistoryUiState) { logcat { "ChatHistory: render ${state::class.simpleName}" } when (state) { 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 929a48589870..98adc04162ae 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 @@ -57,6 +57,9 @@ class ChatHistoryViewModel @Inject constructor( private val navigationChannel = Channel(capacity = Channel.BUFFERED, onBufferOverflow = BufferOverflow.DROP_OLDEST) val navigationEvents: Flow = navigationChannel.receiveAsFlow() + private val messageChannel = Channel(capacity = Channel.BUFFERED, onBufferOverflow = BufferOverflow.DROP_OLDEST) + val messageEvents: Flow = messageChannel.receiveAsFlow() + /** Cached snapshot so non-suspend action methods can read Recent without re-subscribing. */ private var latestItems: List = emptyList() @@ -136,7 +139,13 @@ class ChatHistoryViewModel @Inject constructor( fun onTogglePin(chatId: String) { val current = latestItems.firstOrNull { it.chatId == chatId } ?: return - appScope.launch { chatHistoryRepository.setPinned(chatId, !current.pinned) } + val wasPinned = current.pinned + appScope.launch { chatHistoryRepository.setPinned(chatId, !wasPinned) } + messageChannel.trySend(MessageEvent.PinToggled(chatId = chatId, wasPinned = wasPinned)) + } + + fun onUndoTogglePin(chatId: String, restorePinned: Boolean) { + appScope.launch { chatHistoryRepository.setPinned(chatId, restorePinned) } } private fun dispatchSelectedClear(chatIds: Set) { @@ -267,6 +276,10 @@ class ChatHistoryViewModel @Inject constructor( data class OpenRename(val chatId: String, val currentTitle: String) : NavigationEvent } + sealed interface MessageEvent { + data class PinToggled(val chatId: String, val wasPinned: Boolean) : MessageEvent + } + private companion object { const val STOP_TIMEOUT_MILLIS = 5_000L } diff --git a/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml b/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml index c13d85162ec7..a6610bcd4262 100644 --- a/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml +++ b/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml @@ -107,4 +107,7 @@ Save Save chat title Couldn\'t rename chat + Chat pinned + Chat unpinned + Undo 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 ea084cfe0ea7..bbb477681a20 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 @@ -858,6 +858,66 @@ class ChatHistoryViewModelTest { assertTrue(repository.pinnedChats.isEmpty()) } + @Test + fun `onTogglePin emits PinToggled message event with previous pinned state false`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = false)) + + viewModel.messageEvents.test { + // Drain the initial Loaded state so latestItems is primed. + viewModel.uiState.test { awaitInitialLoaded() } + viewModel.onTogglePin("a") + + val event = awaitItem() + assertTrue(event is ChatHistoryViewModel.MessageEvent.PinToggled) + event as ChatHistoryViewModel.MessageEvent.PinToggled + assertEquals("a", event.chatId) + assertEquals(false, event.wasPinned) + } + } + + @Test + fun `onTogglePin emits PinToggled message event with previous pinned state true`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = true)) + + viewModel.messageEvents.test { + viewModel.uiState.test { awaitInitialLoaded() } + viewModel.onTogglePin("a") + + val event = awaitItem() + assertTrue(event is ChatHistoryViewModel.MessageEvent.PinToggled) + event as ChatHistoryViewModel.MessageEvent.PinToggled + assertEquals("a", event.chatId) + assertEquals(true, event.wasPinned) + } + } + + @Test + fun `onTogglePin emits no message event when chatId is unknown`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = false)) + + viewModel.messageEvents.test { + viewModel.uiState.test { awaitInitialLoaded() } + viewModel.onTogglePin("missing") + expectNoEvents() + } + } + + @Test + fun `onUndoTogglePin restores the requested pinned state via the repository`() = coroutineRule.testScope.runTest { + source.value = listOf(item("a", pinned = true)) + + viewModel.uiState.test { + awaitInitialLoaded() + viewModel.onTogglePin("a") // a → unpinned + awaitItem() // Loaded after the unpin write + viewModel.onUndoTogglePin("a", restorePinned = true) + val restored = awaitItem() as Loaded + assertEquals(listOf("a"), restored.pinned.map { it.chatId }) + assertEquals(emptyList(), restored.recent.map { it.chatId }) + } + assertEquals(listOf("a" to false, "a" to true), repository.pinnedChats) + } + /** * `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 06c472fa6e316737303a538ded598d67aeae03da Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Tue, 19 May 2026 12:09:57 +0200 Subject: [PATCH 3/4] Split DuckAiChatStore.setPinned into pinChat / unpinChat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Intent-named methods read more naturally at call sites that already know which direction they're going, and dropping the Boolean return keeps the public contract free of JSON-validity concerns — not-found and malformed JSON are silent no-ops. The shared mutation logic stays as a private setPinned helper inside RealDuckAiChatStore. The repository keeps its boolean-toggle setPinned signature because its caller (ChatHistoryViewModel) computes the next state as a boolean; the impl dispatches to pinChat / unpinChat. --- .../impl/history/ChatHistoryRepository.kt | 4 ++- .../store/impl/RealDuckAiChatStore.kt | 19 ++++++++---- .../store/impl/RealDuckAiChatStoreTest.kt | 31 ++++++++++++++----- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt index 971d4aaa7d8e..834ff150143d 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt @@ -65,7 +65,9 @@ class RealChatHistoryRepository @Inject constructor( withContext(dispatchers.io()) { chatStore.renameChat(chatId, newTitle) } override suspend fun setPinned(chatId: String, pinned: Boolean) { - withContext(dispatchers.io()) { chatStore.setPinned(chatId, pinned) } + withContext(dispatchers.io()) { + if (pinned) chatStore.pinChat(chatId) else chatStore.unpinChat(chatId) + } } private fun toChatHistoryItem(chat: DuckAiChat): ChatHistoryItem = ChatHistoryItem( diff --git a/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt b/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt index 5b7391dbaf16..5523d26183ea 100644 --- a/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt +++ b/duckchat/duckchat-store/src/main/java/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStore.kt @@ -67,8 +67,11 @@ interface DuckAiChatStore { /** Renames [chatId] in the FE-owned JSON blob. Returns true if the chat existed and was updated, false otherwise. */ suspend fun renameChat(chatId: String, newTitle: String): Boolean - /** Sets the pinned flag on [chatId]. Returns true if the chat existed and was updated, false otherwise. */ - suspend fun setPinned(chatId: String, pinned: Boolean): Boolean + /** Pins [chatId]. Silent no-op if the chat is not found or the stored JSON is malformed. */ + suspend fun pinChat(chatId: String) + + /** Unpins [chatId]. Silent no-op if the chat is not found or the stored JSON is malformed. */ + suspend fun unpinChat(chatId: String) } @SingleInstanceIn(AppScope::class) @@ -146,16 +149,20 @@ class RealDuckAiChatStore @Inject constructor( true } - override suspend fun setPinned(chatId: String, pinned: Boolean): Boolean = + override suspend fun pinChat(chatId: String) = setPinned(chatId, pinned = true) + + override suspend fun unpinChat(chatId: String) = setPinned(chatId, pinned = false) + + private suspend fun setPinned(chatId: String, pinned: Boolean) { withContext(dispatchers.io()) { logcat { "DuckAI: RealDuckAiChatStore.setPinned($chatId, $pinned)" } - val entity = chatsDao.getById(chatId) ?: return@withContext false + val entity = chatsDao.getById(chatId) ?: return@withContext val updatedJson = runCatching { JSONObject(entity.data).put("pinned", pinned).toString() - }.getOrNull() ?: return@withContext false + }.getOrNull() ?: return@withContext chatsDao.upsert(entity.copy(data = updatedJson)) - true } + } private fun DuckAiBridgeChatEntity.toDuckAiChat(): DuckAiChat? = runCatching { val json = JSONObject(data) diff --git a/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt b/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt index 907be0ffaf9a..674d2ebbe6dd 100644 --- a/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt +++ b/duckchat/duckchat-store/src/test/kotlin/com/duckduckgo/duckchat/store/impl/RealDuckAiChatStoreTest.kt @@ -337,24 +337,25 @@ class RealDuckAiChatStoreTest { verify(chatsDao).deleteAll() } - // --- setPinned --- + // --- pinChat / unpinChat --- @Test - fun `setPinned returns false when chat not found`() = runTest { + fun `pinChat is a no-op when chat not found`() = runTest { whenever(chatsDao.getById("missing")).thenReturn(null) - assertFalse(store.setPinned("missing", true)) + store.pinChat("missing") + verify(chatsDao, never()).upsert(any()) } @Test - fun `setPinned updates only the pinned flag and preserves other JSON fields`() = runTest { + fun `pinChat sets pinned to true and preserves other JSON fields`() = runTest { val originalJson = """ {"chatId":"abc","title":"Old","model":"gpt-5-mini","lastEdit":"2026-04-01T21:31:54.260Z","pinned":false,"fileRefs":["uuid1"],"messages":[{"role":"user","text":"hi"}]} """.trimIndent() whenever(chatsDao.getById("abc")).thenReturn(DuckAiBridgeChatEntity("abc", originalJson)) - assertTrue(store.setPinned("abc", true)) + store.pinChat("abc") val entityCaptor = argumentCaptor() verify(chatsDao).upsert(entityCaptor.capture()) @@ -369,10 +370,26 @@ class RealDuckAiChatStoreTest { } @Test - fun `setPinned returns false when stored JSON is malformed`() = runTest { + fun `unpinChat sets pinned to false`() = runTest { + val originalJson = """ + {"chatId":"abc","title":"Old","model":"gpt-5-mini","lastEdit":"2026-04-01T21:31:54.260Z","pinned":true} + """.trimIndent() + whenever(chatsDao.getById("abc")).thenReturn(DuckAiBridgeChatEntity("abc", originalJson)) + + store.unpinChat("abc") + + val entityCaptor = argumentCaptor() + verify(chatsDao).upsert(entityCaptor.capture()) + val json = JSONObject(entityCaptor.firstValue.data) + assertFalse(json.getBoolean("pinned")) + } + + @Test + fun `pinChat is a no-op when stored JSON is malformed`() = runTest { whenever(chatsDao.getById("abc")).thenReturn(DuckAiBridgeChatEntity("abc", "not a json")) - assertFalse(store.setPinned("abc", true)) + store.pinChat("abc") + verify(chatsDao, never()).upsert(any()) } } From 83e7fe3338f69ab906595ca001d9444bf871bb7c Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Wed, 20 May 2026 17:05:23 +0200 Subject: [PATCH 4/4] Restore setPinned override on RecordingRenameRepository after rebase The rebase dropped the one-line override on this private test fake, breaking compileDebugUnitTestKotlin because RecordingRenameRepository implements ChatHistoryRepository which declares setPinned. --- .../duckduckgo/duckchat/impl/history/RenameChatViewModelTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/RenameChatViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/RenameChatViewModelTest.kt index 56986dba4d9a..92796310b243 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/RenameChatViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/RenameChatViewModelTest.kt @@ -81,4 +81,6 @@ private class RecordingRenameRepository : ChatHistoryRepository { renames += chatId to newTitle return nextResult } + + override suspend fun setPinned(chatId: String, pinned: Boolean) = Unit }