From fb9d16730e104b488cb2d6e2c0f2ef7c196f3a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Mon, 16 Feb 2026 15:51:10 +0000 Subject: [PATCH] [PM-30870] Fix editing blocked autofill URIs (#6532) --- .../blockautofill/BlockAutoFillScreen.kt | 10 +- .../blockautofill/BlockAutoFillViewModel.kt | 30 ++++-- .../BlockAutoFillViewModelTest.kt | 92 ++++++++++++++++++- 3 files changed, 120 insertions(+), 12 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillScreen.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillScreen.kt index 6f1d430759..13834bb0a5 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillScreen.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillScreen.kt @@ -72,7 +72,11 @@ fun BlockAutoFillScreen( { viewModel.trySendAction(BlockAutoFillAction.UriTextChange(uri = it)) } }, onSaveClick = remember(viewModel) { - { viewModel.trySendAction(BlockAutoFillAction.SaveUri(newUri = it)) } + { newUri, originalUri -> + viewModel.trySendAction( + BlockAutoFillAction.SaveUri(newUri = newUri, originalUri = originalUri), + ) + } }, onRemoveClick = remember(viewModel) { { viewModel.trySendAction(BlockAutoFillAction.RemoveUriClick(it)) } @@ -180,7 +184,7 @@ fun BlockAutoFillScreen( private fun BlockAutoFillDialogs( dialogState: BlockAutoFillState.DialogState? = null, onUriTextChange: (String) -> Unit, - onSaveClick: (String) -> Unit, + onSaveClick: (String, String?) -> Unit, onRemoveClick: (String) -> Unit, onDismissRequest: () -> Unit, ) { @@ -198,7 +202,7 @@ private fun BlockAutoFillDialogs( null }, onCancelClick = onDismissRequest, - onSaveClick = onSaveClick, + onSaveClick = { newUri -> onSaveClick(newUri, dialogState.originalUri) }, ) } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModel.kt index 6e5406d45a..63ab805f4c 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModel.kt @@ -101,12 +101,20 @@ class BlockAutoFillViewModel @Inject constructor( private fun handleSaveUri(action: BlockAutoFillAction.SaveUri) { val uriList = action.newUri.split(",").map { it.trim() } + // When editing, exclude the original URI from duplicate validation + val existingUrisForValidation = action + .originalUri + ?.let { original -> + settingsRepository.blockedAutofillUris.filter { it != original } + } + ?: settingsRepository.blockedAutofillUris + val errorText = uriList .filter { uri -> - uri in settingsRepository.blockedAutofillUris || !uri.isValidPattern() + uri in existingUrisForValidation || !uri.isValidPattern() } .firstNotNullOfOrNull { uri -> - uri.validateUri(settingsRepository.blockedAutofillUris) + uri.validateUri(existingUrisForValidation) } if (errorText != null) { @@ -114,6 +122,7 @@ class BlockAutoFillViewModel @Inject constructor( currentState.copy( dialog = BlockAutoFillState.DialogState.AddEdit( uri = action.newUri, + originalUri = action.originalUri, errorMessage = errorText, ), ) @@ -122,11 +131,12 @@ class BlockAutoFillViewModel @Inject constructor( } val currentUris = settingsRepository.blockedAutofillUris.toMutableList() + + // Remove the original URI if editing + action.originalUri?.let { currentUris.remove(it) } + uriList.forEach { newUri -> - val uriIndex = currentUris.indexOf(newUri) - if (uriIndex != -1) { - currentUris[uriIndex] = newUri - } else { + if (newUri !in currentUris) { currentUris.add(newUri) } } @@ -244,8 +254,14 @@ sealed class BlockAutoFillAction { /** * User click to save or edit a URI. + * + * @property newUri The new URI to save. + * @property originalUri The original URI being edited, or null if adding a new URI. */ - data class SaveUri(val newUri: String) : BlockAutoFillAction() + data class SaveUri( + val newUri: String, + val originalUri: String? = null, + ) : BlockAutoFillAction() /** * User click to remove URI. diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModelTest.kt index 15dad6b572..6de569f2e6 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/autofill/blockautofill/BlockAutoFillViewModelTest.kt @@ -3,6 +3,8 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.autofill.blockautofill import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test import com.bitwarden.ui.platform.base.BaseViewModelTest +import com.bitwarden.ui.platform.resource.BitwardenString +import com.bitwarden.ui.util.asText import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import io.mockk.every import io.mockk.mockk @@ -16,7 +18,6 @@ class BlockAutoFillViewModelTest : BaseViewModelTest() { every { blockedAutofillUris } returns listOf("blockedUri") } - @Suppress("MaxLineLength") @Test fun `initial state with blocked URIs updates state to ViewState Content`() = runTest { @@ -30,7 +31,6 @@ class BlockAutoFillViewModelTest : BaseViewModelTest() { assertEquals(expectedState, viewModel.stateFlow.value) } - @Suppress("MaxLineLength") @Test fun `initial state with empty blocked URIs maintains state as ViewState Empty`() = runTest { @@ -168,6 +168,94 @@ class BlockAutoFillViewModelTest : BaseViewModelTest() { } } + @Test + fun `on SaveUri action with originalUri should replace the original URI`() = runTest { + val blockedUris = mutableListOf("http://old.com", "http://other.com") + + every { settingsRepository.blockedAutofillUris } answers { blockedUris.toList() } + every { settingsRepository.blockedAutofillUris = any() } answers { + blockedUris.clear() + blockedUris.addAll(firstArg()) + } + + val viewModel = createViewModel() + viewModel.trySendAction( + BlockAutoFillAction.SaveUri( + newUri = "http://new.com", + originalUri = "http://old.com", + ), + ) + + val expectedState = BlockAutoFillState( + dialog = null, + viewState = BlockAutoFillState.ViewState.Content( + blockedUris = listOf("http://other.com", "http://new.com"), + ), + ) + + assertEquals(expectedState, viewModel.stateFlow.value) + } + + @Test + fun `on SaveUri action editing to same value should succeed`() = runTest { + val blockedUris = mutableListOf("http://same.com", "http://other.com") + + every { settingsRepository.blockedAutofillUris } answers { blockedUris.toList() } + every { settingsRepository.blockedAutofillUris = any() } answers { + blockedUris.clear() + blockedUris.addAll(firstArg()) + } + + val viewModel = createViewModel() + viewModel.trySendAction( + BlockAutoFillAction.SaveUri( + newUri = "http://same.com", + originalUri = "http://same.com", + ), + ) + + val expectedState = BlockAutoFillState( + dialog = null, + viewState = BlockAutoFillState.ViewState.Content( + blockedUris = listOf("http://other.com", "http://same.com"), + ), + ) + + assertEquals(expectedState, viewModel.stateFlow.value) + } + + @Test + fun `on SaveUri action editing to existing URI should show duplicate error`() = runTest { + val blockedUris = mutableListOf("http://a.com", "http://b.com") + + every { settingsRepository.blockedAutofillUris } answers { blockedUris.toList() } + every { settingsRepository.blockedAutofillUris = any() } answers { + blockedUris.clear() + blockedUris.addAll(firstArg()) + } + + val viewModel = createViewModel() + viewModel.trySendAction( + BlockAutoFillAction.SaveUri( + newUri = "http://b.com", + originalUri = "http://a.com", + ), + ) + + val expectedState = BlockAutoFillState( + dialog = BlockAutoFillState.DialogState.AddEdit( + uri = "http://b.com", + originalUri = "http://a.com", + errorMessage = BitwardenString.the_urix_is_already_blocked.asText("http://b.com"), + ), + viewState = BlockAutoFillState.ViewState.Content( + blockedUris = listOf("http://a.com", "http://b.com"), + ), + ) + + assertEquals(expectedState, viewModel.stateFlow.value) + } + private fun createViewModel( state: BlockAutoFillState? = DEFAULT_STATE, ): BlockAutoFillViewModel = BlockAutoFillViewModel(