Skip to content

Chore: Account Summaries list in UI should be immutable#7147

Open
david-livefront wants to merge 1 commit into
mainfrom
immutable-account-summaries-list
Open

Chore: Account Summaries list in UI should be immutable#7147
david-livefront wants to merge 1 commit into
mainfrom
immutable-account-summaries-list

Conversation

@david-livefront

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

N/A

📔 Objective

This PR updates the AccountSummaries list in the UI state to use an ImmutableList.

@david-livefront david-livefront requested a review from a team as a code owner July 7, 2026 15:49
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt labels Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This tech-debt PR changes the accountSummaries field in LandingState, LoginState, VaultUnlockState, VaultState, and VaultItemListingState from List<AccountSummary> to ImmutableList<AccountSummary>, and updates UserState.toAccountSummaries() to return an ImmutableList. Producers and tests were updated consistently (persistentListOf, .toImmutableList()), and the pattern matches the existing ImmutableList usage already present in LandingState.environmentTypeOptions. The change is mechanical and low risk.

Code Review Details
  • ♻️ : Redundant .toImmutableList() remains at four account-switcher call sites now that the state fields are already immutable (cleaned up only in LandingScreen)
    • app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt:313
    • app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt:504
    • app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt:137
    • app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt:225

BitwardenAccountSwitcher(
isVisible = isAccountMenuVisible,
accountSummaries = state.accountSummaries.toImmutableList(),
accountSummaries = state.accountSummaries,

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: Redundant .toImmutableList() calls remain in other account-switcher screens

Details

Nice cleanup here removing .toImmutableList() now that LandingState.accountSummaries is an ImmutableList. The same conversion is now also redundant in the other screens whose state fields were changed to ImmutableList in this PR, but wasn't removed:

  • app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreen.kt:313
  • app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt:504
  • app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt:137
  • app/src/main/kotlin/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt:225

Each calls state.accountSummaries.toImmutableList() on a value that is already ImmutableList<AccountSummary>. Not a bug, but removing them here would complete the objective and keep the call sites consistent.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.09%. Comparing base (fe2e7c0) to head (d923ae3).

Files with missing lines Patch % Lines
...warden/ui/auth/feature/landing/LandingViewModel.kt 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7147      +/-   ##
==========================================
- Coverage   86.76%   86.09%   -0.68%     
==========================================
  Files         868      927      +59     
  Lines       63975    66090    +2115     
  Branches     9585     9630      +45     
==========================================
+ Hits        55511    56897    +1386     
- Misses       5023     5739     +716     
- Partials     3441     3454      +13     
Flag Coverage Δ
app-data 17.40% <0.00%> (-0.20%) ⬇️
app-ui-auth-tools 19.02% <76.92%> (+0.21%) ⬆️
app-ui-platform 16.81% <0.00%> (+0.33%) ⬆️
app-ui-vault 28.06% <30.76%> (+0.56%) ⬆️
authenticator 6.15% <0.00%> (-0.02%) ⬇️
lib-core-network-bridge 4.05% <0.00%> (-0.03%) ⬇️
lib-data-ui 1.14% <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 Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@david-livefront david-livefront force-pushed the immutable-account-summaries-list branch from 0d7c02a to d923ae3 Compare July 7, 2026 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant