[PM-35455] feat: Wire premium subscription data into Plan screen#6819
[PM-35455] feat: Wire premium subscription data into Plan screen#6819SaintPatrck merged 9 commits intomainfrom
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6819 +/- ##
==========================================
- Coverage 85.73% 85.73% -0.01%
==========================================
Files 832 836 +4
Lines 58955 59300 +345
Branches 8617 8654 +37
==========================================
+ Hits 50543 50838 +295
- Misses 5437 5480 +43
- Partials 2975 2982 +7
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:
|
c3887d5 to
915e983
Compare
cbf4111 to
0cb83ba
Compare
5230d82 to
06622de
Compare
| <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_past_due_description">You have a grace period of %1$d days from your subscription expiration date. Please resolve the past due amount by %2$s.</string> | ||
| <string name="subscription_paused_description">Your subscription is paused. Resume it to continue using premium features.</string> |
There was a problem hiding this comment.
@RishikaSG-28 is this ok for the "Paused" status message?
There was a problem hiding this comment.
@SaintPatrck For the paused state, here's the copy:
Badge: "Paused" (orange/warning)
Message: "Your subscription is paused. Resume to continue using premium features."
049e150 to
e50bdb6
Compare
f383be5 to
5ebe4ce
Compare
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR wires premium subscription data into the Plan screen, replacing the previous placeholder. The wiring follows the established UDF/BaseViewModel patterns: Code Review Details
|
2c65004 to
f1c6fa8
Compare
| PremiumSubscriptionStatus.ACTIVE -> BitwardenString.subscription_status_active | ||
| PremiumSubscriptionStatus.CANCELED -> BitwardenString.subscription_status_canceled | ||
| PremiumSubscriptionStatus.OVERDUE_PAYMENT -> | ||
| BitwardenString.subscription_status_overdue_payment |
There was a problem hiding this comment.
Wanna wrap this in curly braces
| PremiumSubscriptionStatus.OVERDUE_PAYMENT, | ||
| PremiumSubscriptionStatus.PAST_DUE, | ||
| PremiumSubscriptionStatus.PAUSED, | ||
| -> BitwardenTheme.colorScheme.statusBadge.warning |
| .cardStyle( | ||
| cardStyle = CardStyle.Full, | ||
| // Override bottom padding; the final row owns its own spacing. | ||
| paddingBottom = 0.dp, |
| paddingBottom = 0.dp, | ||
| ), | ||
| ) { | ||
| Column( |
There was a problem hiding this comment.
Do we need this outer column?
| Row( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .standardHorizontalMargin() |
There was a problem hiding this comment.
Can we move the width and standard margin up to be passed in.
| } | ||
|
|
||
| private fun Instant.toLocalizedDate(): String = | ||
| dateFormatter.format(this.atZone(clock.zone)) |
There was a problem hiding this comment.
Can we use the helper functions for this?
this.toFormattedDateStyle(
dateStyle = FormatStyle.LONG,
locale = Locale.US,
clock = clock,
)There was a problem hiding this comment.
Isn't this already here?
There was a problem hiding this comment.
Yup. Had to rebase for it to stop showing up.
Renders a premium user's actual billed rate, storage cost, discount, estimated tax, and next-charge summary in the Plan screen, along with the subscription status badge and the existing Manage plan / Cancel Premium actions. Line items default to a "--" placeholder while the subscription fetch is loading and for any field that resolves to null or 0.00.
Renders the paused status as a warning badge with a generic resume message and swaps the Plan screen's currency formatting helpers off Double now that SubscriptionInfo is BigDecimal-backed.
Backfill the Compose UI tests for the premium subscription card, status badge variants, line items, action buttons, and portal/subscription dialog states that Codecov flagged as uncovered.
Adds tests for non-ACTIVE subscription statuses (overdue, past due with and without grace period, paused), monthly cadence formatting, and the placeholder fallbacks for null or zero line items so a regression in any of those user-facing strings can no longer ship silently.
The previous isDigitsOnly() check on the formatted discount string was always false, so the success color never rendered. Switch to a direct comparison against the placeholder string instead.
Wrap multi-line arrow bodies in braces, drop the redundant header Column wrapper, hoist row sizing modifiers to the call site, and swap the ad-hoc DateTimeFormatter for the toFormattedDateStyle helper.
a12ce02 to
4d73154
Compare
| SubscriptionLineItem( | ||
| label = stringResource(id = BitwardenString.discount), | ||
| value = viewState.discountAmountText, | ||
| valueColor = if (viewState.discountAmountText == "--") { |
There was a problem hiding this comment.
🎨 SUGGESTED: Discount color check is coupled to a duplicated "--" literal.
Details and fix
PLACEHOLDER_TEXT = "--" is private to PlanViewModel.kt, but here the screen hard-codes the same literal to decide the color. If PLACEHOLDER_TEXT is ever renamed, localized, or swapped (e.g. to an em-dash), the discount row will silently render the placeholder in success-green or color real values in primary text without any compile-time signal.
Consider modeling the "no discount" condition explicitly on the view state instead of inferring it from the formatted string — for example a hasDiscount: Boolean on PlanState.ViewState.Premium, set in toPremiumViewState() alongside discountAmountText. The screen would then use viewState.hasDiscount for the color choice and presentation/data concerns stay separated.
| <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_past_due_description">You have a grace period of %1$d days from your subscription expiration date. Please resolve the past due amount by %2$s.</string> |
There was a problem hiding this comment.
It looks like you forgot to add this string as a plural string.
There was a problem hiding this comment.
Thanks for catching that @mKoonrad
I've opened a new PR here to address it.

🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-35455
📔 Objective
Wires premium subscription data into the Plan screen so premium users see their actual billed rate, storage cost, discount, estimated tax, and next-charge summary — plus a status badge reflecting their current subscription state.
Stacks on PM-35454. Line items default to a
"--"placeholder while the subscription fetch is loading and for any field that resolves to null or0.00(e.g. no additional storage, no discount, no tax). Currency is hardcoded to USD for now.The existing Manage plan / Cancel Premium flows (via the Stripe customer portal) are surfaced here; the cancel button is hidden when status is already CANCELED.
📸 Screenshots