[PM-41836] fix: Promote Plan screen off Free view without requiring an in-flight checkout - #7337
Conversation
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the two changed files at head ( Code Review DetailsNo findings. Notes on prior review threads, for context (no action requested):
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7337 +/- ##
==========================================
+ Coverage 85.27% 85.67% +0.39%
==========================================
Files 987 947 -40
Lines 68237 67965 -272
Branches 10198 10186 -12
==========================================
+ Hits 58192 58230 +38
+ Misses 6461 6149 -312
- Partials 3584 3586 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| loadedState.viewState, | ||
| ) | ||
| expectNoEvents() | ||
| assertEquals(null, loadedState.dialogState) |
There was a problem hiding this comment.
Can we use assertNull
There was a problem hiding this comment.
Actually, can we assert the complete state
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Promote unconditionally, not only while `isAwaitingPremiumStatus`. | ||
| if (isPremium) { | ||
| promoteFreeCloudToPremiumView(isConfirmedPremium = true) | ||
| } |
There was a problem hiding this comment.
Details and fix
Account.isPremium is hasPremiumPersonally || hasPremiumFromOrganization, and an org-granted-premium account has no GatewaySubscriptionId, so getSubscription() returns NotFound and handleSubscriptionResultReceive drops the screen back to Content.Free.Cloud (covered by the existing SubscriptionResultReceive NotFound falls back to Free view and fetches pricing test, which uses markUserPremium()).
That Free.Cloud state is also the first moment the init gate first { it.viewState is Content } lets the initial UserStateUpdateReceive through, so the new unconditional promotion fires immediately:
Free.Cloud → Loading("loading subscription") → getSubscription → NotFound → Loading → getPremiumPlanPricing → Free.Cloud
The user sees the Free view flash back into a full-screen loader on every open, with two extra API calls, and it repeats on every distinct userStateFlow emission while the screen is open. The old isAwaitingPremiumStatus guard suppressed this. It is invisible to the tests because UnconfinedTestDispatcher runs the whole cycle before collection starts.
One option — gate on the transition, which keeps the intended fix (a premium flip arriving from a push/sync while on the Free view) and drops the redundant reload:
val isPremium = action.userState?.activeAccount?.isPremium == true
val wasShowingPremiumView = state.showsPremiumView
mutableStateFlow.update {
it.copy(
showsPremiumView = isPremium ||
premiumStateManager.subscriptionStatusStateFlow.value.isPremiumViewEligible(),
)
}
if (isPremium && !wasShowingPremiumView) {
promoteFreeCloudToPremiumView(isConfirmedPremium = true)
}The free → premium flip tests start with showsPremiumView = false, so they stay green.
| } | ||
| // ACTIVE status for a premium account must promote too, not just trouble statuses. | ||
| val isPremium = authRepository.userStateFlow.value?.activeAccount?.isPremium == true | ||
| if (!isPremium && !status.isPremiumViewEligible()) { |
There was a problem hiding this comment.
🎨 SUGGESTED: The new isPremium branch — the headline behavior of this fix — has no test exercising it.
Details and fix
Every test that drives mutableSubscriptionStatusStateFlow into Available uses a trouble status with a free account (SubscriptionStatusUpdateReceive promotes Free view to Premium on trouble status, init opens Premium view when account is free but status is in a trouble state, and the new ...trouble status while awaiting checkout...). The tests that set isPremium = true (lines 169, 469, 590, 607, 889, 1753) never emit an Available(ACTIVE) status while the view state is Content.Free.Cloud.
So the case the comment on line 469 describes — a premium account whose status flow reports ACTIVE while sitting on the Free view — is never executed. Under the old if (!status.isPremiumViewEligible()) return, ACTIVE always returned early; the whole point of adding isPremium here is to let it through, and that path is exactly what Codecov reports as a partial.
A test would look like the sibling at line 965, with the account flipped premium first:
@Test
fun `SubscriptionStatusUpdateReceive with ACTIVE status promotes premium account off Free view`() =
runTest {
val viewModel = createViewModel(subscriptionResult = SUBSCRIPTION_SUCCESS_ACTIVE)
viewModel.stateEventFlow(backgroundScope) { stateFlow, eventFlow ->
assertEquals(DEFAULT_FREE_STATE, stateFlow.awaitItem())
markUserPremium()
mutableSubscriptionStatusStateFlow.value = SubscriptionStatusState.Available(
status = PremiumSubscriptionStatus.ACTIVE,
)
// ... assert Loading then DEFAULT_PREMIUM_LOADED_STATE
eventFlow.expectNoEvents()
}
}This also pins down the interaction with the !wasShowingPremiumView gate added to handleUserStateUpdateReceive, which is what makes this handler the fallback promoter rather than a redundant one.
| // Fires the celebration event only when isConfirmedPremium and a checkout was actually | ||
| // in flight; otherwise this is a silent state recovery. | ||
| private fun promoteFreeCloudToPremiumView(isConfirmedPremium: Boolean) { | ||
| val isAwaitingPremiumStatus = (state.viewState as? PlanState.ViewState.Content.Free.Cloud) |
There was a problem hiding this comment.
Can we make this cleaner?
onFreeCloudContent { freeState ->
if (isConfirmedPremium && freeState.isAwaitingPremiumStatus) {
onPremiumUpgradeSuccess()
} else {
promoteToPremiumView()
}
}
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-41836
📔 Objective
The Plan screen only promoted a premium account off the Free-upgrade view while
isAwaitingPremiumStatuswas true (immediately after a Stripe checkout round-trip), andhandleSubscriptionStatusUpdateReceiveonly reacted to trouble-status subscription updates, never a healthyACTIVEone. A premium account reaching the screen outside that narrow window — via a push notification, a background sync, or simply reopening the screen later — stayed stuck on the Free-upgrade UI despite already being premium.Both signal handlers (
handleUserStateUpdateReceive,handleSubscriptionStatusUpdateReceive) now promote unconditionally whenever the account is premium. The upgrade-celebration event (NavigateToUpgradedToPremium) still only fires when a checkout was genuinely in flight (isAwaitingPremiumStatus), so a stale trouble-status update arriving mid-checkout can't fire a false celebration.Out of scope, tracked separately: the
isPremiumvs. org-granted-premium (isPremiumFromSelf) distinction, and a client-side wiring gap in the Stripe checkout callback (a missingAndroidManifest.xmlintent-filter affecting devices without Auth Tab support) — both real findings from this investigation, but independent of this fix and better reviewed on their own.Review note: a local review caught that an early version of this change promoted unconditionally on every
UserStateUpdateReceive, including for non-premium accounts — a regression, since fixed and verified against the fullPlanViewModelTestsuite.