Skip to content

[PM-39708] fix: Show correct UI for unpaid subscription status#2862

Open
KatherineInCode wants to merge 11 commits into
mainfrom
pm-39708/unpaid-status-details
Open

[PM-39708] fix: Show correct UI for unpaid subscription status#2862
KatherineInCode wants to merge 11 commits into
mainfrom
pm-39708/unpaid-status-details

Conversation

@KatherineInCode

@KatherineInCode KatherineInCode commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-39708

📔 Objective

SubscriptionStatus.unpaid is the state Stripe assigns after ~8 failed payment attempts, at which point the server sets premium = FALSE. The app was aliasing it to updatePayment (a dunning state where premium is still active), causing two bugs:

  • The vault list showed the generic "Upgrade to Premium" action card instead of "Subscription needs attention"
  • Settings → Plan showed "Update payment" description and copy, instead of unpaid-specific copy directing the user to reinstate via the web vault

Changes:

  • PremiumPlanStatus: Added .unpaid as a first-class case (danger badge, isTroubleState = true); added isPaymentProblemState property (true for pastDue/unpaid/updatePayment) to drive the attention card
  • BillingService.refreshSubscriptionAttentionCard: Removed doesActiveAccountHavePremiumPersonally() guard so lapsed users are surfaced on every sync regardless of premium flag; free users hit a silent 404 (GetSubscriptionRequestError). Matches Android's explicit design decision in PremiumStateManagerImpl
  • VaultListProcessor: Extracted refreshPremiumActionCards() — attention card and upgrade card are now mutually exclusive; attention card takes priority
  • Plan screen: unpaid-specific description ("Your subscription was suspended on [date]. To reactivate, please resolve your past due invoices."), danger badge labeled "Unpaid", cancel button hidden

📸 Screenshots

Unpaid is a lapsed state (premium = FALSE after repeated payment
failures) that was being aliased to updatePayment, causing the vault
list to show the generic "Upgrade to Premium" card and the Plan screen
to show "Update payment" details instead of unpaid-specific copy.

- Add PremiumPlanStatus.unpaid as a first-class case (danger badge,
  isTroubleState = true)
- Add isPaymentProblemState to PremiumPlanStatus covering pastDue,
  unpaid, and updatePayment — drives the attention card
- Show "subscription needs attention" card for unpaid; suppress the
  upgrade card when attention card is visible (extracted to
  refreshPremiumActionCards())
- Remove doesActiveAccountHavePremiumPersonally() guard from
  refreshSubscriptionAttentionCard so lapsed users are surfaced on
  every sync regardless of premium flag (matches Android behavior)
- Plan screen: unpaid description, hidden cancel button, danger badge
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:bug Change Type - Bug labels Jul 7, 2026
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.60%. Comparing base (438d632) to head (9df760f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2862       +/-   ##
===========================================
- Coverage   81.27%   40.60%   -40.68%     
===========================================
  Files        1028      357      -671     
  Lines       66164    16657    -49507     
===========================================
- Hits        53773     6763    -47010     
+ Misses      12391     9894     -2497     

☔ 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 9, 2026 15:11
@KatherineInCode KatherineInCode requested review from a team and matt-livefront as code owners July 9, 2026 15:11
@KatherineInCode KatherineInCode added the ai-review Request a Claude code review label Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of .unpaid as a first-class PremiumPlanStatus, the BillingService.refreshSubscriptionAttentionCard guard removal with GetSubscriptionRequestError handling, the mutually-exclusive premium action card logic in VaultListProcessor, and the unpaid-specific Plan screen copy. The two compilation blockers flagged in the previous review (assignment to the private computed subscription property) are now fixed by routing through loadingState. Switch statements remain exhaustive, error handling for the free-user 404 is explicit and intentional, and test coverage is thorough (Codecov reports full coverage).

Code Review Details

No blocking findings.

Notes considered and cleared during review:

  • Removing the doesActiveAccountHavePremiumPersonally() guard means a getSubscription() call now runs on every sync for all non-self-hosted accounts, including free users. This is intentional (matches Android's PremiumStateManagerImpl, documented in the doc comment and PR description); the free-user 404 is caught as GetSubscriptionRequestError and hidden silently.
  • isTroubleState now includes .unpaid, so the Settings → Plan navigation (SettingsProcessor.navigateToPlan) correctly routes unpaid users to the plan screen rather than the upgrade flow.
  • The open thread from @matt-livefront regarding whether shouldShowUpgradedToPremiumActionCard should also live in refreshPremiumActionCards() is an active human discussion and was intentionally not duplicated here.

func descriptionText_unpaid() {
var state = PremiumPlanState()
state.planStatus = .unpaid
state.subscription = .fixture(status: .unpaid, suspension: testDate)

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.

CRITICAL: Assigning to state.subscription will not compile.

Details and fix

subscription is declared as a private, getter-only computed property in PremiumPlanState.swift:

private var subscription: PremiumSubscription? { loadingState.data }

@testable import elevates internal access but does not expose private members, and a computed property with no setter is never assignable. This line breaks compilation of the BitwardenShared test target. Every other test in this file sets the subscription through loadingState, matching the source-of-truth pattern:

Suggested change
state.subscription = .fixture(status: .unpaid, suspension: testDate)
state.loadingState = .data(.fixture(status: .unpaid, suspension: testDate))

The same issue exists at PremiumPlanView+SnapshotTests.swift:88 (processor.state.subscription = .fixture(...)) — that method is disabletest_-prefixed, but disabling only skips execution, not compilation, so it must be fixed the same way.

Comment on lines 250 to 251
state.shouldShowUpgradedToPremiumActionCard = await services.billingService
.shouldShowUpgradedToPremiumActionCard()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔 Should we include this in refreshPremiumActionCards() as well?

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:authenticator Bitwarden Authenticator app context 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