[PM-30870] Fix editing blocked autofill URIs #6532
Conversation
github-actions
bot
added
the
app:password-manager
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where editing a blocked autofill URI incorrectly triggered duplicate validation errors. The fix adds an originalUri parameter to track which URI is being edited and excludes it from duplicate validation, allowing users to save edits without false positive errors.
Changes:
- Added
originalUriparameter toSaveUriaction to track which URI is being edited - Modified duplicate validation logic to exclude the original URI when editing
- Updated URI replacement logic to properly remove the original before adding the updated value
- Added comprehensive test coverage for edit scenarios including same-value edits and duplicate detection
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| BlockAutoFillViewModel.kt | Enhanced handleSaveUri to filter out originalUri from validation and properly replace URIs during edits; updated SaveUri action to include optional originalUri parameter |
| BlockAutoFillScreen.kt | Updated onSaveClick callback signature to accept originalUri and pass it from dialog state to the action |
| BlockAutoFillViewModelTest.kt | Added three new tests covering URI replacement, same-value editing, and duplicate detection during edits |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6532 +/- ##
==========================================
+ Coverage 86.35% 86.39% +0.04%
==========================================
Files 789 777 -12
Lines 56424 56200 -224
Branches 8175 8175
==========================================
- Hits 48724 48555 -169
+ Misses 4853 4801 -52
+ Partials 2847 2844 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // When editing, exclude the original URI from duplicate validation | ||
| val existingUrisForValidation = action.originalUri?.let { original -> | ||
| settingsRepository.blockedAutofillUris.filter { it != original } | ||
| } ?: settingsRepository.blockedAutofillUris |
There was a problem hiding this comment.
Can we use proper chain formatting:
val existingUrisForValidation = action
.originalUri
?.let { original ->
settingsRepository.blockedAutofillUris.filter { it != original }
}
?: settingsRepository.blockedAutofillUris| val addEditDialog = dialog as BlockAutoFillState.DialogState.AddEdit | ||
| assertEquals("http://b.com", addEditDialog.uri) | ||
| assertEquals("http://a.com", addEditDialog.originalUri) | ||
| assert(addEditDialog.errorMessage != null) |
There was a problem hiding this comment.
Can we just assert the entire dialog state instead.
Also, we should be using assertEquals not assert
| val addEditDialog = dialog as BlockAutoFillState.DialogState.AddEdit | ||
| assertEquals("http://b.com", addEditDialog.uri) | ||
| assertEquals("http://a.com", addEditDialog.originalUri) | ||
| assertNotNull(addEditDialog.errorMessage) |
There was a problem hiding this comment.
We should be asserting the whole state
There was a problem hiding this comment.
oh sorry I misread your comment above... for sure!
| assertEquals(expectedState, viewModel.stateFlow.value) | ||
| } | ||
|
|
||
| @Suppress("MaxLineLength") |
There was a problem hiding this comment.
This suppression is not needed?
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-30870
📔 Objective
Fix an issue where editing a blocked autofill URI would incorrectly trigger duplicate validation errors. Previously, when a user attempted to edit an existing blocked URI, the validation would compare against the full list including the original URI, causing false duplicate errors even when saving the same value or making minor modifications.
Changes
📸 Screenshots
Screen.Recording.2026-02-13.at.12.17.11.mov