[PM-36969] feat: Surface subscription substate to premium gates#6931
[PM-36969] feat: Surface subscription substate to premium gates#6931SaintPatrck wants to merge 7 commits into
Conversation
SaintPatrck
added
the
ai-review-vnext
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR surfaces the Stripe subscription substate to premium-gating callers via a new Code Review DetailsNo new findings beyond the existing reviewer threads. Six unresolved threads from |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6931 +/- ##
==========================================
+ Coverage 86.25% 86.38% +0.12%
==========================================
Files 909 864 -45
Lines 64380 62769 -1611
Branches 9146 9125 -21
==========================================
- Hits 55533 54223 -1310
+ Misses 5704 5406 -298
+ Partials 3143 3140 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
github-actions
Bot
added
the
app:authenticator
| <string name="premium_next_charge_summary">Your next charge is for %1$s USD due on %2$s.</string> | ||
| <string name="subscription_canceled_description">Your subscription was canceled on %1$s. Resubscribe to continue using premium features.</string> | ||
| <string name="subscription_overdue_description">We couldn’t process your payment. Update your payment before your subscription ends on %1$s.</string> | ||
| <string name="subscription_update_payment_description">We couldn’t process your payment. Update your payment before your subscription ends on %1$s.</string> |
There was a problem hiding this comment.
Should the injected date also be made bold?
There was a problem hiding this comment.
Indeed. Thanks!
The server keeps `Profile.Premium=true` during the grace window after a subscription enters a recovery or terminal state, so callers that gate purely on `isPremium` either route users away from the Plan-screen badge that explains their situation (PM-36969, PM-36970, PM-37181) or suppress the upgrade CTA users need to recover their account (PM-37093, PM-37177, PM-37180). Expose the Stripe substate via a new `PremiumStateManager.subscriptionStatusStateFlow` and use it to compute "effectively premium" for banner eligibility and Plan-screen routing. Renames the misleading `OVERDUE_PAYMENT` enum value to `UPDATE_PAYMENT` so the badge label matches Figma. The subscription endpoint 404s for free users (no `GatewaySubscriptionId`); a new `SubscriptionResult.NotFound` maps that case to "free, show upgrade CTA" instead of an error dialog.
Avoids a guaranteed 404 round-trip to GET /accounts/subscription for every free user on app launch and on every premium-status push. Re-keys on (userId, isPremium) so the flow refetches when an account transitions to premium or when a push arrives for an already-premium user.
The isPremium gate prevented users whose Stripe subscription had moved to canceled / incomplete_expired (with the server flipping isPremium to false) from reaching the Premium view in Settings. They landed on the Free view with no way to see their canceled status or resubscribe. Drop the gate so the subscription fetch runs whenever there is an active user; the response layer already translates 404 to NoSubscription, so genuine free users still bypass the Premium view.
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
The "Apply suggestions from code review" commit accidentally pasted the property declaration twice, breaking compile. Restore the single declaration and move the fetch-keying / 404 details onto the implementation so the interface only carries the caller-facing contract.
SaintPatrck
force-pushed
the
premium-upgrade/pm-36969-subscription-status-flow
branch
from
ad72ae6 to
d9f1828
Compare
Design calls for the date in each Plan-screen subscription-status description to be visually emphasized so users can quickly spot when their next charge, cancellation, or grace period takes effect. Standardize on the codebase's existing annotation-tag pattern (`<annotation emphasis="bold"><annotation arg="N">`) rather than inventing a new mechanism. Premium ViewState now carries raw data fields so the Composable can render an AnnotatedString with the appropriate bold span per subscription status.
SaintPatrck
force-pushed
the
premium-upgrade/pm-36969-subscription-status-flow
branch
from
d9f1828 to
c5990ff
Compare
| authRepository | ||
| .userStateFlow | ||
| .map { state -> state?.activeAccount?.userId } | ||
| .distinctUntilChanged() |
There was a problem hiding this comment.
We have an extension for this on AuthDiskSource
authDiskSource
.activeUserIdChangesFlow
.flatMapLatest { userId ->| clock = clock, | ||
| ) | ||
| val itemCount = inputs.vaultDataState.activeVaultItemCount() | ||
| val isEffectivelyPremium = activeAccount.isPremium && |
There was a problem hiding this comment.
Looks like the creationDate and isPremium status are the only things that we are pulling from the UserState.
What do you think about just using the AuthDiskSource.userStateFlow to get that data? That would aloow us to remove the dependency on the AuthRepository completely.
| */ | ||
| sealed class SubscriptionStatusState { | ||
|
|
||
| /** No fetch has been attempted yet for the active user. */ |
There was a problem hiding this comment.
How do you feel about these compressed docs?
I usually prefer the expanded ones:
/**
* No fetch has been attempted yet for the active user.
*/There was a problem hiding this comment.
I don't mind them for short docs. For consistency I can make them multi-line.
There was a problem hiding this comment.
It does not bother me either way, more of a question
| if (bitwardenError is BitwardenError.Http && | ||
| bitwardenError.code == HTTP_CODE_NOT_FOUND | ||
| ) { | ||
| SubscriptionResult.NotFound |
There was a problem hiding this comment.
Can we move this into the service?
| checkoutUrl = null, | ||
| isAwaitingPremiumStatus = false, | ||
| ), | ||
| dialogState = null, |
There was a problem hiding this comment.
Should this be a loading state?
The manager only needs the active user id and the raw premium flags from the persisted profile; routing through AuthRepository.userStateFlow just to read derived UserState.Account fields adds an unnecessary layer and a heavier dependency. Read `hasPremiumPersonally` / `hasPremiumFromOrganization` / `creationDate` straight off the disk-side profile and drop the AuthRepository dependency entirely.
2 participants
🎟️ Tracking
📔 Objective
The server keeps
Profile.Premium=trueduring the grace window after a subscription enters a recovery or terminal Stripe state. Surfaces that gate purely onisPremiumeither suppress the Plan-screen badge that would explain the user's situation, or hide the upgrade CTA users need to recover their account.Adds
PremiumStateManager.subscriptionStatusStateFlow(Loading/NoSubscription/Available/Error) so callers can derive "effectively premium" from both the account flag and the Stripe substate. The upgrade banner now flips back on when the active subscription is in a trouble state (past_due,update_payment,canceled,paused) even while the server still reports premium. The Plan screen routes free-with-trouble-substate users to the Premium view so they see the right badge and Manage/Resubscribe affordances.Renames
OVERDUE_PAYMENT → UPDATE_PAYMENTso the badge label matches the Figma frame. A newSubscriptionResult.NotFoundmaps the 404 returned for users without aGatewaySubscriptionIdto "free, show upgrade CTA" instead of an error dialog.📸 Screenshots