[PM-32009] feat: Add add/edit and view UI for new cipher types#6829
[PM-32009] feat: Add add/edit and view UI for new cipher types#6829SaintPatrck wants to merge 4 commits intonew-item-types/phase-01-04_infrastructurefrom
Conversation
…nd Passport cipher types Implements Phases 5-7 of the new item types feature: Compose add/edit forms, view screens, ViewModel actions/handlers, and handler classes for all three new cipher types. Includes hidden field masking for Bank Account (account number, PIN), copy actions, account type dropdown, and month/year date pickers for Driver's License and Passport date fields.
Fix typographic apostrophes in 5 driver's license strings, sort import statements alphabetically in 5 files, document hardcoded strings in VaultBankAccountTypeExtensions as SDK/network values, and add 6 VaultItemViewModel tests for Bank Account view actions.
Add 9 missing ViewModel action tests: 3 for Driver's License (MiddleName, IssuingState, LicenseClass) and 6 for Passport (DobYear, PassportType, IssuingCountry, IssuingAuthority, IssueYear, ExpirationYear). Re-add month/year string resources that were lost during prior string edits.
SaintPatrck
added
the
ai-review-vnext
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the Phase 5–7 UI implementation for Bank Account, Driver's License, and Passport cipher types. The changes introduce add/edit Compose screens, view screens, per-type handler classes, and ViewModel wiring, faithfully mirroring established patterns from the existing Card/Identity/SshKey types ( Code Review Details
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## new-item-types/phase-01-04_infrastructure #6829 +/- ##
=============================================================================
- Coverage 85.32% 84.19% -1.13%
=============================================================================
Files 834 847 +13
Lines 59179 60462 +1283
Branches 8611 8711 +100
=============================================================================
+ Hits 50492 50904 +412
- Misses 5708 6556 +848
- Partials 2979 3002 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| label = stringResource(id = BitwardenString.date_of_birth) + " - " + | ||
| stringResource(id = BitwardenString.month), |
There was a problem hiding this comment.
🎨 SUGGESTED: String concatenation for field labels hampers localization and is inconsistent with the sibling screen.
Details and fix
The six month/year labels in this file build their text by concatenating two stringResource(...) calls with a hard-coded " - " separator (e.g. lines 80–81, 106–107, 190–191, 216–217, 235–236, 261–262). Two problems:
- Localization — concatenating translated fragments with a hard-coded separator produces labels that cannot be reordered for languages with different word order (e.g. Japanese, German compounds). The established pattern elsewhere in the module is a single, fully-translated string per label.
- Inconsistency with
VaultAddEditDriversLicenseItems.kt— the sibling screen usesBitwardenString.expiration_month/expiration_yeardirectly (line 132, 157).VaultAddEditPassportItems.ktbuilds"Expiration date - Month"the hard way despite the same string resources existing.
Minimal fix: use the existing expiration_month / expiration_year resources for the expiration rows, and introduce dedicated strings (e.g. date_of_birth_month, date_of_birth_year, issue_month, issue_year) for the DOB and issue rows — or use a parameterized format string ("%1$s - %2$s") so translators can control ordering.
|
Great job! No new security vulnerabilities introduced in this pull request |
1 participant
🎟️ Tracking
PM-32009
📔 Objective
Second of three stacked PRs. Builds the Compose add/edit forms, view screens, and associated handlers for Bank Account, Driver's License, and Passport on top of the infrastructure landed in #6828.
Kept separate from the vault-integration PR so reviewers can focus on screen-level concerns — field layouts, masking, handlers, and ViewModel state — without also triaging listing/search/navigation changes.
Why it looks the way it does
Vault…TypeHandlers) isolate each new type's action plumbing fromVaultAddEditViewModelandVaultItemViewModel, which already carry significant complexity for Login/Card/Identity/SecureNote. Keeping new surface area out of those files reduces merge risk with parallel cipher-type work.Stacked on: #6828
Followed by:
new-item-types/phase-08-11_vault-integration(vault listing, search, item-type selection)📸 Screenshots
To be added before marking ready for review.