[PM-33946] feat: Add dynamic pricing and fix checkout flow#6793
[PM-33946] feat: Add dynamic pricing and fix checkout flow#6793SaintPatrck wants to merge 6 commits intomainfrom
Conversation
Add GET /plans/premium endpoint to fetch premium plan pricing dynamically instead of using hardcoded placeholder values. The ViewModel now starts in a Loading state, fetches pricing on init, and transitions to Free (with formatted monthly rate) or Error (with retry) based on the API result.
- Restructure handlePricingResultReceive to use onFreeContent wrapper, preserving checkoutUrl on success - Add ClosePricingErrorClick action that clears dialog and navigates back - Wire GetPricingError dialog dismiss to ClosePricingErrorClick with "Close" button text instead of "Okay" - Fix stale preview rate ($1.65 -> $1.67) and test assertions to use Free(rate="--") with GetPricingError dialog instead of ViewState.Error - Remove unused DEFAULT_LOADING_STATE test constant
SaintPatrck
added
the
ai-review-vnext
github-actions
bot
added
app:password-manager
Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR replaces static placeholder premium pricing with dynamic data fetched from the plans API and implements correct checkout result handling for the premium upgrade flow. The changes span the network layer (new Code Review Details
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6793 +/- ##
==========================================
+ Coverage 85.00% 85.69% +0.68%
==========================================
Files 971 828 -143
Lines 61312 58730 -2582
Branches 8647 8595 -52
==========================================
- Hits 52121 50327 -1794
+ Misses 6190 5422 -768
+ Partials 3001 2981 -20
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:
|
|
Great job! No new security vulnerabilities introduced in this pull request |
| } | ||
| } | ||
|
|
||
| private fun handlePremiumUpgradeSuccess() { |
There was a problem hiding this comment.
Can we drop the handle prefix since this is not for a specific action.
There was a problem hiding this comment.
Changed to onPremiumUpgradeSuccess
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| private fun handleSyncCompleteReceive() { | ||
| onFreeContent { freeState -> | ||
| if (!freeState.isAwaitingPremiumStatus) return@onFreeContent | ||
|
|
||
| val isPremium = authRepository | ||
| .userStateFlow | ||
| .value | ||
| ?.activeAccount | ||
| ?.isPremium == true | ||
| if (isPremium) { | ||
| onPremiumUpgradeSuccess() | ||
| } else { | ||
| // Sync completed but premium not yet provisioned — | ||
| // prompt the user to retry or continue as free. | ||
| mutableStateFlow.update { | ||
| it.copy( | ||
| dialogState = PlanState.DialogState.PendingUpgrade, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Details and fix
handleSyncCompleteReceive() ignores the SyncVaultDataResult carried by SyncCompleteReceive. When sync fails (e.g., network error), the code falls through to the else branch and shows the "Upgrade pending" dialog, telling the user their upgrade is being processed when the actual problem is a failed sync.
Consider checking the result and showing an appropriate error when sync fails:
private fun handleSyncCompleteReceive(action: PlanAction.Internal.SyncCompleteReceive) {
onFreeContent { freeState ->
if (!freeState.isAwaitingPremiumStatus) return@onFreeContent
when (action.result) {
is SyncVaultDataResult.Error -> {
// Sync failed — show PendingUpgrade so user can retry
// or consider a distinct error dialog
mutableStateFlow.update {
it.copy(
dialogState = PlanState.DialogState.PendingUpgrade,
)
}
}
is SyncVaultDataResult.Success -> {
val isPremium = authRepository
.userStateFlow
.value
?.activeAccount
?.isPremium == true
if (isPremium) {
onPremiumUpgradeSuccess()
} else {
mutableStateFlow.update {
it.copy(
dialogState = PlanState.DialogState.PendingUpgrade,
)
}
}
}
}
}
}At minimum, the action's result should be forwarded to the handler so the failure path can be handled (or explicitly acknowledged as intentional). The current behavior may mislead the user on a flaky connection.
There was a problem hiding this comment.
@claude The current behavior is intentional. Both sync failure and "premium not yet provisioned" present the same recovery path — the user can tap "Sync now" to retry or "Continue" to move on. A distinct sync error dialog wouldn't change the available actions, so the extra state adds complexity without UX benefit.
There was a problem hiding this comment.
Claude finished @SaintPatrck's task in 11s —— View job
Thanks for the explanation, @SaintPatrck. That makes sense — since both sync failure and "not yet provisioned" offer the same "Sync now" / "Continue" recovery actions, adding a distinct error dialog would just be extra state without changing the UX. Acknowledged as intentional. 👍
| MutableStateFlow<SpecialCircumstance?>(null) | ||
| private val mockSpecialCircumstanceManager: SpecialCircumstanceManager = | ||
| mockk(relaxed = true) { | ||
| every { specialCircumstance } returns null |
There was a problem hiding this comment.
Should we update this this too?
every { specialCircumstance } answers { mutableSpecialCircumstanceStateFlow.value }2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33946
📔 Objective
Replace the static placeholder rate with dynamic pricing from the plans API, and implement correct checkout result handling for the premium upgrade flow.
Dynamic pricing:
GET /api/plans/premiumon PlanScreen loadCheckout result handling:
bitwarden://premium-checkout-resultto match the server-configured Stripe redirect (PM-34173)?result=success/?result=canceledquery parameter viaPremiumCheckoutCallbackResultsealed class, following the existingCookieCallbackResultpatternspecialCircumstanceStateFlowat runtime to detect when the user returns from checkoutsyncForResult(), navigate back with snackbar if premium is detected, or show "Upgrade pending" dialog with Sync now / Continue options if not yet provisionedisAwaitingPremiumStatusonViewState.Free📸 Screenshots