Skip to content

[PM-39767] fix: Show upgrade pending alert when upgrading via Settings > Plan - #2873

Merged
KatherineInCode merged 2 commits into
mainfrom
pm-39767/upgrade-pending
Jul 17, 2026
Merged

[PM-39767] fix: Show upgrade pending alert when upgrading via Settings > Plan#2873
KatherineInCode merged 2 commits into
mainfrom
pm-39767/upgrade-pending

Conversation

@KatherineInCode

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-39767

📔 Objective

Fixes a bug where initiating the Premium upgrade flow via Settings > Plan never showed the "Upgrade Pending" dialog when the post-payment sync call was interrupted — even though the identical scenario worked correctly from the Vault tab's upgrade CTA.

Root cause: the "Upgrade Pending" alert is driven by DefaultPremiumUpgradeHelper, which subscribes to the checkout-status publisher before navigating to the Premium upgrade screen. Vault and Send screens already construct this helper; SettingsProcessor.navigateToPlan() navigated straight to .premiumUpgrade without ever creating it, so nothing was ever subscribed to catch the .pending status on that path.

Fix: wire SettingsProcessor into the same helper the other 8 entry points already use — SettingsRoute now conforms to PremiumUpgradeRoute, gaining an associated DismissAction on its .dismiss case (mirroring VaultRoute), and navigateToPlan() routes through premiumUpgradeHelper.startInAppPremiumUpgrade() instead of navigating directly.

Giving .dismiss an associated value surfaced a few unrelated pre-existing call sites that referenced it bare (no parens) in ExportVaultProcessor, PendingRequestsProcessor, and DeleteAccountProcessor — fixed those to the explicit .dismiss() call form, since a defaulted associated value isn't auto-applied outside of a direct call expression.

This is a narrower fix than a fuller centralization (moving pending-checkout detection into a shared app-lifetime service) that was evaluated and intentionally shelved as out of scope for this ticket — worth a follow-up ticket if we want to prevent a future entry point from repeating this same gap.

📸 Screenshots

Not applicable — no UI changes.

…s > Plan

Settings > Plan navigated straight to the Premium upgrade screen without
going through PremiumUpgradeHelper, so it never subscribed to the checkout
status publisher and silently missed the "Upgrade Pending" alert when sync
was interrupted after payment. Vault and Send screens already used the
helper; Settings now does too.

Giving SettingsRoute.dismiss an associated DismissAction (mirroring
VaultRoute) also required fixing several unrelated pre-existing call sites
that referenced the bare case, since a defaulted associated value isn't
auto-applied outside of a direct call expression.
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug labels Jul 9, 2026
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.26%. Comparing base (82ffbf3) to head (3c667cb).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...Platform/Settings/Settings/SettingsProcessor.swift 37.50% 5 Missing ⚠️
...red/UI/Platform/Settings/SettingsCoordinator.swift 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2873      +/-   ##
==========================================
+ Coverage   81.25%   81.26%   +0.01%     
==========================================
  Files        1028     1028              
  Lines       66164    66177      +13     
==========================================
+ Hits        53760    53782      +22     
+ Misses      12404    12395       -9     

☔ 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.

@KatherineInCode
KatherineInCode marked this pull request as ready for review July 13, 2026 21:36
@KatherineInCode
KatherineInCode requested review from a team and matt-livefront as code owners July 13, 2026 21:36
@KatherineInCode KatherineInCode added the ai-review Request a Claude code review label Jul 13, 2026
@github-actions github-actions Bot removed the ai-review Request a Claude code review label Jul 13, 2026
@KatherineInCode KatherineInCode added the ai-review Request a Claude code review label Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the fix wiring SettingsProcessor into the shared DefaultPremiumUpgradeHelper so the "Upgrade Pending" alert fires when upgrading via Settings > Plan. The change extends SettingsRoute to conform to PremiumUpgradeRoute, gives SettingsRoute.dismiss an optional DismissAction, and updates the affected call sites and tests. The coordinator's new dismiss-with-completion logic is a verbatim copy of the proven VaultCoordinator implementation, and DI wiring for the new HasBillingRepository/HasEnvironmentService dependencies is correctly threaded through the coordinator. No blocking issues found.

Code Review Details

No findings. Verified:

  • SettingsRoute conformance to PremiumUpgradeRoute is satisfied — both premiumUpgrade and dismiss(_:) exist.
  • The three bare .dismiss.dismiss() fixes cover all SettingsRoute call sites; remaining bare .dismiss references belong to unrelated route enums (PasswordAutofillRoute, AddEditFolderRoute).
  • New SettingsCoordinator dismiss logic mirrors VaultCoordinator and is behaviorally equivalent to the prior stackNavigator?.dismiss() in the single-modal case.
  • Both planPressed branches (subscription success and GetSubscriptionRequestError) are covered by updated tests asserting startInAppPremiumUpgradeCalled.
  • HasBillingRepository added to SettingsCoordinator.Services; HasEnvironmentService already present. DI chain resolves.

@KatherineInCode
KatherineInCode merged commit e0ad5b9 into main Jul 17, 2026
25 of 30 checks passed
@KatherineInCode
KatherineInCode deleted the pm-39767/upgrade-pending branch July 17, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants