Skip to content

[PM-32810] feat: Add Add/Edit support for Bank Account item type#6851

Open
SaintPatrck wants to merge 1 commit intomainfrom
new-item-types/PM-32810_bank-account
Open

[PM-32810] feat: Add Add/Edit support for Bank Account item type#6851
SaintPatrck wants to merge 1 commit intomainfrom
new-item-types/PM-32810_bank-account

Conversation

@SaintPatrck
Copy link
Copy Markdown
Contributor

@SaintPatrck SaintPatrck commented Apr 29, 2026

🎟️ Tracking

PM-32810

Stacked parent: #6875

📔 Objective

Wire the Bank Account form into the Add/Edit screen so users can create new bank account entries and edit existing ones, including locally-managed visibility toggles for sensitive fields.

📸 Screenshots

Figma Actual
N/A

@SaintPatrck SaintPatrck added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 29, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:feature Change Type - Feature Development labels Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR wires the Bank Account Add/Edit form into VaultAddEditScreen, adds the corresponding handlers/actions/state-update plumbing in VaultAddEditViewModel, and removes the temporary FAB carve-outs in VaultItemViewModel and VaultItemListingViewModel so users can now create and edit bank account ciphers. The implementation mirrors the established Card/SSH Key pattern (updateBankAccountContent, VaultAddEditBankAccountTypeHandlers, sealed BankAccountType action subclasses, and rememberSaveable for password-field visibility toggles), and the toBankAccountView() / toViewState() mappers were already in place from the parent stack so the new state fields round-trip correctly to the SDK. Test coverage is thorough: every new action has a VaultAddEditViewModelTest case, every field has a VaultAddEditScreenTest interaction test, and the FAB-visibility tests in VaultItemScreenTest and VaultItemListingViewModelTest were updated to assert the new enabled behavior.

Code Review Details

No findings. Existing review threads on this PR are all resolved or outdated; the test-coverage gap previously noted for CopyAccountNumberClick/CopyRoutingNumberClick in VaultViewModelTest and SearchViewModelTest has since been filled.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 98.63014% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.65%. Comparing base (a9cc18e) to head (ac36549).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../ui/vault/feature/addedit/VaultAddEditViewModel.kt 94.73% 0 Missing and 2 partials ⚠️
...i/vault/feature/addedit/VaultAddEditItemContent.kt 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6851      +/-   ##
==========================================
- Coverage   85.72%   85.65%   -0.07%     
==========================================
  Files         843      848       +5     
  Lines       60719    61016     +297     
  Branches     8915     8926      +11     
==========================================
+ Hits        52049    52265     +216     
- Misses       5606     5687      +81     
  Partials     3064     3064              
Flag Coverage Δ
app-data 17.44% <0.00%> (-0.07%) ⬇️
app-ui-auth-tools 19.77% <0.00%> (-0.08%) ⬇️
app-ui-platform 15.65% <0.00%> (-0.06%) ⬇️
app-ui-vault 26.54% <98.63%> (+0.26%) ⬆️
authenticator 6.46% <0.00%> (-0.01%) ⬇️
lib-core-network-bridge 4.14% <0.00%> (-0.03%) ⬇️
lib-data-ui 1.00% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

text = it,
toastDescriptorOverride = BitwardenString.routing_number.asText(),
)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ DEBT: New CopyAccountNumberClick/CopyRoutingNumberClick handlers added to VaultViewModel lack unit tests, breaking the established overflow-handler test convention.

Details

VaultViewModelTest covers every other overflow copy handler (CopyNoteClick, CopyNumberClick, CopyTotpClick, including success / decryption-failure / cipher-not-found variants), but the two new bank-account handlers added here have no corresponding tests. The same gap exists in SearchViewModelTest for the parallel handlers added in SearchViewModel.

Adding analogous success and "blank/null value should skip clipboard" tests would mirror VaultItemListingViewModelTest (which already covers these new actions) and the existing CopyNoteClick/CopyNumberClick tests in this file.

Base automatically changed from new-item-types/phase-01-04_infrastructure to main May 4, 2026 16:55
@SaintPatrck SaintPatrck force-pushed the new-item-types/PM-32810_bank-account branch from 4111590 to d42dbff Compare May 5, 2026 17:59
@SaintPatrck SaintPatrck force-pushed the new-item-types/PM-32810_bank-account branch from d42dbff to ac06853 Compare May 5, 2026 19:19
@SaintPatrck SaintPatrck changed the title [PM-32810] feat: Add Bank Account item type [PM-32810] feat: Add Add/Edit support for Bank Account item type May 5, 2026
@SaintPatrck SaintPatrck changed the base branch from main to new-item-types/PM-32810_bank-account-view May 5, 2026 19:20
@SaintPatrck SaintPatrck force-pushed the new-item-types/PM-32810_bank-account-view branch 2 times, most recently from 3797c7d to bfcd3f6 Compare May 6, 2026 15:38
Base automatically changed from new-item-types/PM-32810_bank-account-view to main May 6, 2026 16:16
@SaintPatrck SaintPatrck force-pushed the new-item-types/PM-32810_bank-account branch from ac06853 to d90eebb Compare May 7, 2026 19:00
@SaintPatrck SaintPatrck removed the app:authenticator Bitwarden Authenticator app context label May 7, 2026
Wires the Bank Account form into the Add/Edit screen so users can create
new bank account entries and edit existing ones, including locally-managed
visibility toggles for sensitive fields.
@SaintPatrck SaintPatrck force-pushed the new-item-types/PM-32810_bank-account branch from d90eebb to ac36549 Compare May 7, 2026 19:30
@github-actions github-actions Bot added the app:authenticator Bitwarden Authenticator app context label May 7, 2026
@SaintPatrck SaintPatrck marked this pull request as ready for review May 7, 2026 20:22
@SaintPatrck SaintPatrck requested review from a team and david-livefront as code owners May 7, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant