[PM-30870] Fix editing blocked autofill URIs (#6532)

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) },
             )
         }
 

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.

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(