feat(subscription): v2 web payments API layer — checkout, discounts, status, cancel, portal, history#7666
Closed
bbsatvik01 wants to merge 10 commits into
Closed
feat(subscription): v2 web payments API layer — checkout, discounts, status, cancel, portal, history#7666bbsatvik01 wants to merge 10 commits into
bbsatvik01 wants to merge 10 commits into
Conversation
… mappers Client data layer for the web-only Subscriptions-v2 Stripe flow, gated by the SUBS_V2_WEB flag. Adds the wire models and repos for the four core v2 endpoints (/products, /checkout, /status, /cancel), the pure status/product mappers into the existing SubscriptionDetails/SubscriptionState shapes, and the v2 UI-gating and cancel-eligibility helpers. - models: checkout_response, products_v2_response (+ productV2ToSubscriptionDetails mapper + plan-id whitelist), subscription_status_v2 (+ mapStatusV2ToState), cancel_response; subscription_details / subscription_state extended for the currency-aware, v2-trial shapes. - repos: checkout_v2_repo (bounded creating-poll), products_v2_repo, status_v2_repo, cancel_v2_repo — all with an injectable http.Client seam. - utils: v2_subscription_catalog, v2_ui_gating, cancel_eligibility. - pangea/common: Requests gains an injectable client + a contextInjector seam and an injectUserContext flag (verbatim bodies for extra="forbid" schemas); v2 URL constants; SUBS_V2_WEB env flag; currency-aware begin_checkout analytics. - 109 unit tests covering the models, mappers, poll logic, gating, and the no-injected-context money-safety contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…behind SUBS_V2_WEB Wires the v2 data layer into the existing subscription surfaces behind the SUBS_V2_WEB + kIsWeb guard, leaving the RevenueCat/mobile path byte-for-byte unchanged when the flag is off. - web_subscription_info_manager: flag-on branches for getCurrentSubscriptionInfo (/status -> mapStatusV2ToState) and submitSubscriptionChange (/checkout -> bounded poll -> redirect), falling back to the RC path otherwise. - subscription_controller / subscription_info_manager / mobile manager: route the v2 state + trial synthesis through the shared getters. - settings (subscription, change_subscription, security) + paywall: render the v2 state, management, and account-delete warning off the flag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…l API logic
Extends the v2 client API surface to the full contract: the discount flow, the
display-only promo preview, and the history/portal reads.
- Discount checkout: CheckoutV2Repo.checkout gains an optional promoCode (body is
EXACTLY {planId[, promoCode]}, extra="forbid", injectUserContext stays false),
returns the resolved CheckoutResponse (now parsing appliedPromoCode — the code
actually on the session, which a reused open session keeps). The
promo_not_applicable 422 maps to a typed PromoNotApplicableException carrying a
CheckoutPromoRejectionReason (9 reasons + unknown), DISTINGUISHED from a
FastAPI list-shaped schema 422 (surfaced as a CheckoutException client bug).
- Promo preview: documents ValidatePromoCodeRepo as display-only (the server
re-validates at checkout); the existing model already matches the contract —
adds field-by-field parse tests (valid terms, valid:false reasons,
below_minimum restrictions echo).
- Invoice history: confirms InvoiceSummary matches the real v2 shape (total /
subtotal / amount_paid, `created` as a Zulu ISO string, no `amount` field) with
a regression-guard test.
- Billing portal: revives BillingPortalRepo with a testable getWith core + a
typed NoBillingAccountException for the 404 "No billing account" (hide manage).
- Free trial: verified SubscriptionRepo.activateFreeTrial already matches the
GET /free_trial 201/400 contract (no change).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…v2 manage wiring, re-entry guards, num parsing Addresses a RED fresh-eyes review (4 MAJOR + 1 MINOR), all in the wiring/flow layer: - billing_portal_repo: portal session URLs are SHORT-LIVED — remove the legacy 10-minute URL/error cache entirely. The repo now only dedupes an in-flight request (evicted on completion via a block-bodied whenComplete — an arrow body would return the evicted future and deadlock it on itself) and NEVER caches an error, so one 5xx cannot poison manage-billing. Tests: error-then-retry succeeds, concurrent calls dedupe to one request, sequential calls mint fresh sessions. - settings_subscription (manage tiles): on the v2 web path BOTH the payment-method and payment-history tiles mint a fresh billing-portal session via BillingPortalRepo (the Stripe portal shows the payment method AND invoice history — the minimal wiring onto the canonical v2 APIs without new client UI; PaymentHistoryRepo stays fully tested and exposed for the history page). NoBillingAccountException renders the management-unavailable message, not an error dialog. The legacy static stripeManagementUrl is never launched on the v2 path; off the flag the legacy behavior is byte-for-byte unchanged. Routing is the unit-tested classifyManagementLaunch. - change_subscription (checkout button): re-entry guard + disabled button while in flight (a double-tap cannot fire concurrent checkouts or multiple redirects); failures surface via showFutureLoadingDialog (the paywall idiom) instead of escaping the tap handler. - settings_subscription (v2 cancel): wrapped in showFutureLoadingDialog for loading + a dismissible, retryable error surface; re-entry guarded. - validate_promo_code_response / payment_history_response: minor-unit money (and expires_at) parse via (v as num).toInt() so a serializer emitting 500.0 cannot crash parsing (consistent with products_v2). New: SingleFlightGuard (pure, unit-tested re-entry guard shared by the checkout/cancel/portal handlers). Module suite: 145 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… v2 status, precise portal 404, guarded submit Addresses 4 round-2 deep-review findings. FIXED FULLY (repo/controller robustness): - #2 /products failure no longer swallowed to an empty catalog. ProductsV2Repo now distinguishes a real empty 200 (returns empty plans) from a fetch failure (transport / 5xx-503 / malformed -> THROWS) via a testable getWith(Requests) core. The controller's v2 catalog build lets that throw propagate, so a products failure becomes a retryable SubscriptionError instead of a "no plans" paywall that strands a buyer. - #3 stale _statusV2 after trial activation. Introduced _refreshV2State() as the single source of truth: re-fetch /status, store _statusV2, rebuild the catalog from that SAME snapshot, and map state — used by both init and updateCurrentSubscription, so _statusV2 / v2TrialOfferable / _state can no longer drift after activate/cancel. buildV2SubscriptionCatalog now reuses ONE synthesized trial object across all/available. - #4 billing-portal 404 too broad. Map 404 -> NoBillingAccountException ONLY when detail == "No billing account"; any other 404 (route/deploy mismatch, FastAPI "Not Found") surfaces the real ChoreoException instead of masking an integration failure as "management unavailable". CONTROLLER-SUPPORT + SEAM (Gabby owns the widget UX): - #1 buy path is now single-flight-guarded at the CONTROLLER (submitSubscriptionChange: a concurrent call is a no-op) with an isSubmitting getter for the disabled/spinner state — protecting any caller (paywall or settings) without touching the paywall widget. Failures still propagate for the caller's showFutureLoadingDialog to render. A client-rules note was added to the frontend contract (await the future, stay disabled while in-flight, dismiss only on success). Tests: ProductsV2Repo throw-vs-empty (4), catalog reused-trial + post-activation transition (2), portal 404-body discrimination (2). Module suite: 153 green; analyze clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… single-flight tests
Final scoped review (YELLOW): 1 MAJOR + 1 test gap, both our layer.
- MAJOR: ProductsV2Response.fromJson no longer defaults a malformed 200 into an
empty catalog. A real empty catalog is {"plans": []}; a body with no `plans`
LIST (absent / null / non-list) is contract-malformed and now THROWS a
FormatException, so ProductsV2Repo.getWith propagates it as a fetch failure and
the controller lands in SubscriptionError instead of rendering "no plans". The
round-2 repo-level distinction stays; this closes the model-default hole.
- TEST GAP: added direct tests of the REAL
SubscriptionController.submitSubscriptionChange (not just the SingleFlightGuard
primitive), via a minimal managerOverride seam (@VisibleForTesting; null in
production so runtime behavior is unchanged) + an L10n BuildContext.
GoogleAnalytics.logEvent is a no-op under `flutter test`, so the non-trial buy
path reaches the injected manager without RevenueCat/MatrixState. The real
method is tested (no extract-for-testability shim needed).
New tests:
- products_v2_repo_test: `{}` -> throws; `{"plans": null}` -> throws;
`{"plans": "oops"}` -> throws (200 empty + valid still parse — kept).
- products_v2_response_test: missing plans key throws; null/non-list plans throw.
- subscription_controller_submit_test: (a) a concurrent second submit while one
is in-flight is a NO-OP (manager invoked once) and isSubmitting stays true
until release; (b) the guard RELEASES on success (a later submit proceeds);
(c) the guard RELEASES on throw (exit() is in finally — a subsequent submit
proceeds).
Module suite: 161 green; analyze clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ia constructor injection The @VisibleForTesting managerOverride was a PUBLIC MUTABLE field — analyzer guidance only, not access control, so any production code with a controller reference could reassign it and change real submit/refresh behavior. Replace it with constructor injection into a private `final`: - constructor: `SubscriptionController({@VisibleForTesting SubscriptionInfoManager? managerOverride})` - stored in `final SubscriptionInfoManager? _managerOverride;` - `_manager => _managerOverride ?? (kIsWeb ? Web : Mobile)` — identical resolution. A final field set only at construction cannot be mutated post-construction, and the field is now PRIVATE so external code cannot even name it. Production constructs `SubscriptionController()` with no param (pangea_controller.dart:46 unchanged) -> _managerOverride null -> real web/mobile manager, byte-for-byte unchanged. Tests inject the fake manager via the constructor. Module suite: 161 green; analyze clean; grep confirms no `.managerOverride =` assignment anywhere. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Apply dart format to the 11 subscription files (whitespace/line-wrapping only, no logic change) to satisfy the repo's 'Check formatting' CI gate - Merge origin/main (branch was out-of-date; picks up the formatted navigation test + new repo-cache utils) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
lib/features/subscription/models/subscription_status_v2.dart (2)
261-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the
"full"/"trial"literals likekV2PaidTypes. The sameaccessLevel == "full"and winningtype == "trial"string comparisons are re-typed independently in three files; a typo in any one would silently diverge from the others. The PR already solved this exact risk for paid types viakV2PaidTypes/isV2PaidType— apply the same pattern here.
lib/features/subscription/models/subscription_status_v2.dart#L261-L266: extractconst String kV2FullAccess = "full";andconst String kV2TrialType = "trial";(or anisV2FullAccess/isV2TrialTypehelper) next tokV2PaidTypes, and use them inmapStatusV2ToState.lib/features/subscription/utils/v2_subscription_catalog.dart#L57-L60: replace the inline"full"/"trial"checks intrialActivewith the new shared constants/helpers.lib/features/subscription/utils/v2_ui_gating.dart#L26-L32: replace the inline"full"check inisPaidWithoutPlanwith the new shared constant/helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/subscription/models/subscription_status_v2.dart` around lines 261 - 266, Centralize the repeated full-access and trial-type literals alongside kV2PaidTypes, exposing shared constants or helpers and updating mapStatusV2ToState in lib/features/subscription/models/subscription_status_v2.dart#261-266, trialActive in lib/features/subscription/utils/v2_subscription_catalog.dart#57-60, and isPaidWithoutPlan in lib/features/subscription/utils/v2_ui_gating.dart#26-32 to use them instead of inline comparisons.Source: Coding guidelines
261-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMagic strings
"full"/"trial"duplicated across files.
kV2PaidTypes/isV2PaidTypewere introduced specifically to centralize the paid-type literal so "every paid/promotional decision agrees" (Lines 4-10), but the"full"access-level and"trial"winning-type literals used here recur unguarded inv2_ui_gating.dartandv2_subscription_catalog.dart. A typo in any one site would silently diverge from the others. See consolidated comment.Also applies to: 266-266
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/subscription/models/subscription_status_v2.dart` at line 261, Replace the duplicated "full" access-level and "trial" winning-type literals in the status evaluation logic with the shared constants or predicates already used to centralize V2 paid-type decisions, including the condition around status.accessLevel and winning. Ensure the implementation continues distinguishing full access and trial winning types consistently with v2_ui_gating.dart and v2_subscription_catalog.dart.Source: Coding guidelines
lib/features/subscription/repo/billing_portal_repo.dart (1)
77-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplicitly cast to
Map<String, dynamic>and remove redundant.toString().
utf8.decode(res.bodyBytes)already returns aString, making.toString()unnecessary. Additionally,jsonDecodereturnsdynamic, which should be explicitly cast toMap<String, dynamic>to avoid implicit downcasts and maintain consistency with other v2 repositories.♻️ Proposed refactor
- final Map<String, dynamic> json = jsonDecode( - utf8.decode(res.bodyBytes).toString(), - ); + final Map<String, dynamic> json = + jsonDecode(utf8.decode(res.bodyBytes)) as Map<String, dynamic>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/subscription/repo/billing_portal_repo.dart` around lines 77 - 79, Update the jsonDecode call in the billing portal response parsing to remove the redundant toString() from utf8.decode(res.bodyBytes), and explicitly cast the decoded result to Map<String, dynamic> before assigning it to json.test/features/subscription/billing_portal_repo_test.dart (1)
84-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate test of Lines 51-65.
This test asserts the exact same scenario as
'404 "No billing account" -> typed NoBillingAccountException (hide manage)'above (same 404 body, same assertions), differing only in thededupeKeystring. Consider removing it to avoid redundant coverage.As per path instructions, "write meaningful tests that cover explicit functionality + edge cases without being redundant."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/subscription/billing_portal_repo_test.dart` around lines 84 - 94, Remove the redundant test case `'404 "No billing account" -> NoBillingAccountException'` from the billing portal repository tests, preserving the earlier equivalent typed-exception coverage and its assertions.Source: Path instructions
test/features/subscription/v2_ui_gating_test.dart (1)
10-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
status()/active()fixture builders across subscription-v2 test files.Both
status()(buildingSubscriptionStatusV2) andactive()(buildingSubscriptionActive) are defined identically in multiple spec files. Per code-style guidelines, extract shared, well-named test fixtures instead of re-declaring the same builder in each file.
test/features/subscription/v2_ui_gating_test.dart#L10-L21: replace localstatus()with an import from a shared fixtures helper (e.g.test/features/subscription/fixtures/subscription_v2_fixtures.dart).test/features/subscription/v2_subscription_catalog_test.dart#L26-L37: same — replace the duplicatestatus()with the shared fixture.test/features/subscription/v2_ui_gating_test.dart#L180-L189: replace localactive()with an import from the shared fixtures helper.test/features/subscription/cancel_eligibility_test.dart#L7-L16: same — replace the duplicateactive()with the shared fixture.As per path instructions, code-style.instructions.md states to "keep code DRY by searching for existing constants/functions before adding new ones."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/subscription/v2_ui_gating_test.dart` around lines 10 - 21, Extract the duplicated status() and active() test fixture builders into a shared helper at test/features/subscription/fixtures/subscription_v2_fixtures.dart, then import and reuse them from test/features/subscription/v2_ui_gating_test.dart lines 10-21 and 180-189, test/features/subscription/v2_subscription_catalog_test.dart lines 26-37, and test/features/subscription/cancel_eligibility_test.dart lines 7-16; remove the local duplicate definitions while preserving their existing defaults and construction behavior.Source: Path instructions
test/features/subscription/status_v2_to_state_test.dart (1)
1-500: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit this file; it's ~500 lines, well over the project's ~400-line target.
The file covers five distinct concerns (inactive mapping, paid-active mapping, cancel mirroring, promotional shapes, and cancel-entitlement selection — the latter alone spans ~215 lines). Consider moving the cancel-entitlement-selection group (Lines 286-499) into its own file (e.g.
cancel_entitlement_selection_test.dart), and extracting a small helper for the repeatedmapStatusV2ToState(status(...), stripeAppId: stripeAppId) as SubscriptionActivepattern to reduce duplication.As per path instructions, "keep file sizes reasonable (target < ~400 lines)" and "keep code DRY."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/subscription/status_v2_to_state_test.dart` around lines 1 - 500, Split the cancel-entitlement-selection tests from the main status mapping test file into a dedicated test file, preserving the existing mapStatusV2ToState coverage and group behavior. In the extracted tests, add a small local helper for the repeated mapStatusV2ToState(..., stripeAppId: stripeAppId) as SubscriptionActive setup, while keeping shared fixtures and assertions unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/subscription/controllers/subscription_controller.dart`:
- Around line 411-479: Protect _refreshV2State with a single in-flight Future so
concurrent calls from updateCurrentSubscription and initialize/_initialize share
the same refresh operation instead of interleaving mutations. Add and reuse an
in-flight field around the full _statusV2, _setAvailableSubscriptions, and
_state update sequence, clearing it when the operation completes or fails while
preserving error propagation and existing caller notification behavior.
In `@lib/features/subscription/models/products_v2_response.dart`:
- Around line 104-120: Move productV2ToSubscriptionDetails into an appropriate
class as a static member, preserving its required stripeAppId parameter,
plan-duration validation, exception behavior, and SubscriptionDetails mapping
unchanged. Update callers to use the class-qualified method.
- Around line 64-66: Update the rawPlans mapping that constructs ProductV2
instances to cast each entry directly to Map<String, dynamic> and pass it to
ProductV2.fromJson, removing the intermediate untyped Map cast and Map.from
conversion.
In `@lib/features/subscription/repo/checkout_v2_repo.dart`:
- Line 227: Update the type check in the promo error handling around the
promo_not_applicable condition to use Map<String, dynamic> instead of the
untyped Map, preserving the existing code comparison and behavior.
In `@lib/features/subscription/repo/products_v2_repo.dart`:
- Around line 43-45: Use UTF-8 decoding of response body bytes before JSON
parsing in the affected repositories: update jsonDecode in
lib/features/subscription/repo/products_v2_repo.dart lines 43-45 and
lib/features/subscription/repo/status_v2_repo.dart lines 22-24 to decode
res.bodyBytes with utf8.decode, preserving the existing ProductsV2Response and
status response parsing flows.
---
Nitpick comments:
In `@lib/features/subscription/models/subscription_status_v2.dart`:
- Around line 261-266: Centralize the repeated full-access and trial-type
literals alongside kV2PaidTypes, exposing shared constants or helpers and
updating mapStatusV2ToState in
lib/features/subscription/models/subscription_status_v2.dart#261-266,
trialActive in
lib/features/subscription/utils/v2_subscription_catalog.dart#57-60, and
isPaidWithoutPlan in lib/features/subscription/utils/v2_ui_gating.dart#26-32 to
use them instead of inline comparisons.
- Line 261: Replace the duplicated "full" access-level and "trial" winning-type
literals in the status evaluation logic with the shared constants or predicates
already used to centralize V2 paid-type decisions, including the condition
around status.accessLevel and winning. Ensure the implementation continues
distinguishing full access and trial winning types consistently with
v2_ui_gating.dart and v2_subscription_catalog.dart.
In `@lib/features/subscription/repo/billing_portal_repo.dart`:
- Around line 77-79: Update the jsonDecode call in the billing portal response
parsing to remove the redundant toString() from utf8.decode(res.bodyBytes), and
explicitly cast the decoded result to Map<String, dynamic> before assigning it
to json.
In `@test/features/subscription/billing_portal_repo_test.dart`:
- Around line 84-94: Remove the redundant test case `'404 "No billing account"
-> NoBillingAccountException'` from the billing portal repository tests,
preserving the earlier equivalent typed-exception coverage and its assertions.
In `@test/features/subscription/status_v2_to_state_test.dart`:
- Around line 1-500: Split the cancel-entitlement-selection tests from the main
status mapping test file into a dedicated test file, preserving the existing
mapStatusV2ToState coverage and group behavior. In the extracted tests, add a
small local helper for the repeated mapStatusV2ToState(..., stripeAppId:
stripeAppId) as SubscriptionActive setup, while keeping shared fixtures and
assertions unchanged.
In `@test/features/subscription/v2_ui_gating_test.dart`:
- Around line 10-21: Extract the duplicated status() and active() test fixture
builders into a shared helper at
test/features/subscription/fixtures/subscription_v2_fixtures.dart, then import
and reuse them from test/features/subscription/v2_ui_gating_test.dart lines
10-21 and 180-189, test/features/subscription/v2_subscription_catalog_test.dart
lines 26-37, and test/features/subscription/cancel_eligibility_test.dart lines
7-16; remove the local duplicate definitions while preserving their existing
defaults and construction behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c014ffa-2cf0-4289-a168-b7230419e9c5
📒 Files selected for processing (50)
lib/features/subscription/controllers/subscription_controller.dartlib/features/subscription/models/cancel_response.dartlib/features/subscription/models/checkout_response.dartlib/features/subscription/models/mobile_subscription_info_manager.dartlib/features/subscription/models/products_v2_response.dartlib/features/subscription/models/subscription_details.dartlib/features/subscription/models/subscription_info_manager.dartlib/features/subscription/models/subscription_state.dartlib/features/subscription/models/subscription_status_v2.dartlib/features/subscription/models/web_subscription_info_manager.dartlib/features/subscription/repo/billing_portal_repo.dartlib/features/subscription/repo/cancel_v2_repo.dartlib/features/subscription/repo/checkout_v2_repo.dartlib/features/subscription/repo/payment_history_response.dartlib/features/subscription/repo/products_v2_repo.dartlib/features/subscription/repo/status_v2_repo.dartlib/features/subscription/repo/validate_promo_code_repo.dartlib/features/subscription/repo/validate_promo_code_response.dartlib/features/subscription/subscription_constants.dartlib/features/subscription/utils/cancel_eligibility.dartlib/features/subscription/utils/single_flight_guard.dartlib/features/subscription/utils/v2_subscription_catalog.dartlib/features/subscription/utils/v2_ui_gating.dartlib/features/subscription/widgets/subscription_paywall.dartlib/pangea/common/config/environment.dartlib/pangea/common/network/requests.dartlib/pangea/common/network/urls.dartlib/pangea/common/utils/firebase_analytics.dartlib/routes/settings/settings_security/settings_security.dartlib/routes/settings/settings_subscription/change_subscription.dartlib/routes/settings/settings_subscription/settings_subscription.dartlib/routes/settings/settings_subscription/settings_subscription_view.darttest/features/subscription/billing_portal_repo_test.darttest/features/subscription/cancel_eligibility_test.darttest/features/subscription/cancel_response_test.darttest/features/subscription/checkout_response_test.darttest/features/subscription/checkout_v2_repo_poll_test.darttest/features/subscription/payment_history_response_test.darttest/features/subscription/products_v2_repo_test.darttest/features/subscription/products_v2_response_test.darttest/features/subscription/requests_inject_context_test.darttest/features/subscription/single_flight_guard_test.darttest/features/subscription/status_v2_to_state_test.darttest/features/subscription/subscription_controller_submit_test.darttest/features/subscription/subscription_details_display_price_test.darttest/features/subscription/subscription_details_tojson_test.darttest/features/subscription/subscription_status_v2_test.darttest/features/subscription/v2_subscription_catalog_test.darttest/features/subscription/v2_ui_gating_test.darttest/features/subscription/validate_promo_code_response_test.dart
Comment on lines
411
to
+479
| Future<void> updateCurrentSubscription() async { | ||
| _state = await _manager.getCurrentSubscriptionInfo(); | ||
| // v2 web: refresh through the single source of truth so _statusV2 (and the | ||
| // catalog / v2TrialOfferable) can never go stale after a status-changing | ||
| // action such as trial activation or cancel (finding #3). A fetch failure | ||
| // sets a retryable error state instead of leaving the caller with an | ||
| // unhandled throw (safe for the paywall's fire-and-forget trial tap). | ||
| if (Environment.subsV2WebEnabled && kIsWeb) { | ||
| try { | ||
| await _refreshV2State(); | ||
| } catch (e, s) { | ||
| ErrorHandler.logError(e: e, s: s, data: {}); | ||
| _state = SubscriptionError(error: e); | ||
| } | ||
| notifyListeners(); | ||
| return; | ||
| } | ||
|
|
||
| // stripeAppId is web-v2 only; the mobile/RC manager ignores it. | ||
| _state = await _manager.getCurrentSubscriptionInfo( | ||
| stripeAppId: _appIds?.stripeId, | ||
| ); | ||
| notifyListeners(); | ||
| } | ||
|
|
||
| /// The single source of truth for the v2 web state: re-fetch `/status`, store | ||
| /// it in [_statusV2], rebuild the sellable catalog from that SAME snapshot, | ||
| /// and map the state — so [_statusV2], the catalog ([v2TrialOfferable]), and | ||
| /// [_state] can never drift (finding #3). Throws on a fetch failure (e.g. a | ||
| /// `/products` 503) so the caller sets an error state instead of an empty | ||
| /// sellable catalog (finding #2). Does NOT notify — the caller decides when. | ||
| Future<void> _refreshV2State() async { | ||
| _statusV2 = await StatusV2Repo.get(); | ||
| final status = _statusV2!; | ||
| if (isPaidWithoutPlan(status)) { | ||
| // #4b: a paid entitlement should always map to a catalog plan; log the | ||
| // anomaly (the generic-tile fallback in `subscription` keeps management | ||
| // working) rather than stranding a paying user on a broken tile. | ||
| ErrorHandler.logError( | ||
| m: "v2 paid entitlement missing a catalog planId", | ||
| s: StackTrace.current, | ||
| data: {}, | ||
| ); | ||
| } | ||
| await _setAvailableSubscriptions(); | ||
| _state = mapStatusV2ToState( | ||
| status, | ||
| stripeAppId: _appIds?.stripeId ?? kStripeAppIdFallback, | ||
| ); | ||
| } | ||
|
|
||
| /// In-app v2 cancel (S4/D4): POST /cancel for the user-owned entitlement, then | ||
| /// refresh from /status so the UI reflects the server-confirmed | ||
| /// cancel-at-period-end. [entitlementRef] MUST come from `/status` (I5) — the | ||
| /// caller sources it from the active state's `entitlementRef`. Errors are | ||
| /// logged and rethrown (matching submitSubscriptionChange) so the UI can react. | ||
| Future<void> cancelSubscription(String entitlementRef) async { | ||
| try { | ||
| await CancelV2Repo.cancel(entitlementRef); | ||
| await updateCurrentSubscription(); | ||
| notifyListeners(); | ||
| } catch (e, s) { | ||
| ErrorHandler.logError( | ||
| e: e, | ||
| s: s, | ||
| data: {"entitlementRef": entitlementRef}, | ||
| ); | ||
| rethrow; | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
_refreshV2State() has no reentrancy guard, unlike every other new v2 mutation path.
updateCurrentSubscription() and initialize()/_initialize() both call _refreshV2State(), which mutates _statusV2, _allSubscriptions/_availableSubscriptions (via _setAvailableSubscriptions()), and _state across multiple await boundaries. Unlike submitSubscriptionChange, cancelSubscription's caller, and the billing-portal flow — all of which got a SingleFlightGuard — this path has no protection against concurrent invocation.
SettingsSubscriptionController.initState() (in settings_subscription.dart) triggers exactly this: it calls subscriptionController.initialize(...) via .then(...) without awaiting, then immediately calls subscriptionController.updateCurrentSubscription() synchronously after. Both can run _refreshV2State() concurrently, so the catalog built in one call can be paired with a _statusV2/_state snapshot from the other interleaved call — the exact "never drift" invariant the docstring on _refreshV2State claims to guarantee.
🔒 Suggested fix: collapse concurrent refreshes into one in-flight future (same pattern already used in `BillingPortalRepo._inflight`)
+ Future<void>? _refreshV2InFlight;
+
Future<void> _refreshV2State() async {
+ final inflight = _refreshV2InFlight;
+ if (inflight != null) return inflight;
+ final future = _doRefreshV2State();
+ _refreshV2InFlight = future;
+ try {
+ await future;
+ } finally {
+ _refreshV2InFlight = null;
+ }
+ }
+
+ Future<void> _doRefreshV2State() async {
_statusV2 = await StatusV2Repo.get();
final status = _statusV2!;
if (isPaidWithoutPlan(status)) {
ErrorHandler.logError(
m: "v2 paid entitlement missing a catalog planId",
s: StackTrace.current,
data: {},
);
}
await _setAvailableSubscriptions();
_state = mapStatusV2ToState(
status,
stripeAppId: _appIds?.stripeId ?? kStripeAppIdFallback,
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Future<void> updateCurrentSubscription() async { | |
| _state = await _manager.getCurrentSubscriptionInfo(); | |
| // v2 web: refresh through the single source of truth so _statusV2 (and the | |
| // catalog / v2TrialOfferable) can never go stale after a status-changing | |
| // action such as trial activation or cancel (finding #3). A fetch failure | |
| // sets a retryable error state instead of leaving the caller with an | |
| // unhandled throw (safe for the paywall's fire-and-forget trial tap). | |
| if (Environment.subsV2WebEnabled && kIsWeb) { | |
| try { | |
| await _refreshV2State(); | |
| } catch (e, s) { | |
| ErrorHandler.logError(e: e, s: s, data: {}); | |
| _state = SubscriptionError(error: e); | |
| } | |
| notifyListeners(); | |
| return; | |
| } | |
| // stripeAppId is web-v2 only; the mobile/RC manager ignores it. | |
| _state = await _manager.getCurrentSubscriptionInfo( | |
| stripeAppId: _appIds?.stripeId, | |
| ); | |
| notifyListeners(); | |
| } | |
| /// The single source of truth for the v2 web state: re-fetch `/status`, store | |
| /// it in [_statusV2], rebuild the sellable catalog from that SAME snapshot, | |
| /// and map the state — so [_statusV2], the catalog ([v2TrialOfferable]), and | |
| /// [_state] can never drift (finding #3). Throws on a fetch failure (e.g. a | |
| /// `/products` 503) so the caller sets an error state instead of an empty | |
| /// sellable catalog (finding #2). Does NOT notify — the caller decides when. | |
| Future<void> _refreshV2State() async { | |
| _statusV2 = await StatusV2Repo.get(); | |
| final status = _statusV2!; | |
| if (isPaidWithoutPlan(status)) { | |
| // #4b: a paid entitlement should always map to a catalog plan; log the | |
| // anomaly (the generic-tile fallback in `subscription` keeps management | |
| // working) rather than stranding a paying user on a broken tile. | |
| ErrorHandler.logError( | |
| m: "v2 paid entitlement missing a catalog planId", | |
| s: StackTrace.current, | |
| data: {}, | |
| ); | |
| } | |
| await _setAvailableSubscriptions(); | |
| _state = mapStatusV2ToState( | |
| status, | |
| stripeAppId: _appIds?.stripeId ?? kStripeAppIdFallback, | |
| ); | |
| } | |
| /// In-app v2 cancel (S4/D4): POST /cancel for the user-owned entitlement, then | |
| /// refresh from /status so the UI reflects the server-confirmed | |
| /// cancel-at-period-end. [entitlementRef] MUST come from `/status` (I5) — the | |
| /// caller sources it from the active state's `entitlementRef`. Errors are | |
| /// logged and rethrown (matching submitSubscriptionChange) so the UI can react. | |
| Future<void> cancelSubscription(String entitlementRef) async { | |
| try { | |
| await CancelV2Repo.cancel(entitlementRef); | |
| await updateCurrentSubscription(); | |
| notifyListeners(); | |
| } catch (e, s) { | |
| ErrorHandler.logError( | |
| e: e, | |
| s: s, | |
| data: {"entitlementRef": entitlementRef}, | |
| ); | |
| rethrow; | |
| } | |
| } | |
| Future<void>? _refreshV2InFlight; | |
| Future<void> updateCurrentSubscription() async { | |
| // v2 web: refresh through the single source of truth so _statusV2 (and the | |
| // catalog / v2TrialOfferable) can never go stale after a status-changing | |
| // action such as trial activation or cancel (finding `#3`). A fetch failure | |
| // sets a retryable error state instead of leaving the caller with an | |
| // unhandled throw (safe for the paywall's fire-and-forget trial tap). | |
| if (Environment.subsV2WebEnabled && kIsWeb) { | |
| try { | |
| await _refreshV2State(); | |
| } catch (e, s) { | |
| ErrorHandler.logError(e: e, s: s, data: {}); | |
| _state = SubscriptionError(error: e); | |
| } | |
| notifyListeners(); | |
| return; | |
| } | |
| // stripeAppId is web-v2 only; the mobile/RC manager ignores it. | |
| _state = await _manager.getCurrentSubscriptionInfo( | |
| stripeAppId: _appIds?.stripeId, | |
| ); | |
| notifyListeners(); | |
| } | |
| /// The single source of truth for the v2 web state: re-fetch `/status`, store | |
| /// it in [_statusV2], rebuild the sellable catalog from that SAME snapshot, | |
| /// and map the state — so [_statusV2], the catalog ([v2TrialOfferable]), and | |
| /// [_state] can never drift (finding `#3`). Throws on a fetch failure (e.g. a | |
| /// `/products` 503) so the caller sets an error state instead of an empty | |
| /// sellable catalog (finding `#2`). Does NOT notify — the caller decides when. | |
| Future<void> _refreshV2State() async { | |
| final inflight = _refreshV2InFlight; | |
| if (inflight != null) return inflight; | |
| final future = _doRefreshV2State(); | |
| _refreshV2InFlight = future; | |
| try { | |
| await future; | |
| } finally { | |
| _refreshV2InFlight = null; | |
| } | |
| } | |
| Future<void> _doRefreshV2State() async { | |
| _statusV2 = await StatusV2Repo.get(); | |
| final status = _statusV2!; | |
| if (isPaidWithoutPlan(status)) { | |
| // `#4b`: a paid entitlement should always map to a catalog plan; log the | |
| // anomaly (the generic-tile fallback in `subscription` keeps management | |
| // working) rather than stranding a paying user on a broken tile. | |
| ErrorHandler.logError( | |
| m: "v2 paid entitlement missing a catalog planId", | |
| s: StackTrace.current, | |
| data: {}, | |
| ); | |
| } | |
| await _setAvailableSubscriptions(); | |
| _state = mapStatusV2ToState( | |
| status, | |
| stripeAppId: _appIds?.stripeId ?? kStripeAppIdFallback, | |
| ); | |
| } | |
| /// In-app v2 cancel (S4/D4): POST /cancel for the user-owned entitlement, then | |
| /// refresh from /status so the UI reflects the server-confirmed | |
| /// cancel-at-period-end. [entitlementRef] MUST come from `/status` (I5) — the | |
| /// caller sources it from the active state's `entitlementRef`. Errors are | |
| /// logged and rethrown (matching submitSubscriptionChange) so the UI can react. | |
| Future<void> cancelSubscription(String entitlementRef) async { | |
| try { | |
| await CancelV2Repo.cancel(entitlementRef); | |
| await updateCurrentSubscription(); | |
| notifyListeners(); | |
| } catch (e, s) { | |
| ErrorHandler.logError( | |
| e: e, | |
| s: s, | |
| data: {"entitlementRef": entitlementRef}, | |
| ); | |
| rethrow; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/subscription/controllers/subscription_controller.dart` around
lines 411 - 479, Protect _refreshV2State with a single in-flight Future so
concurrent calls from updateCurrentSubscription and initialize/_initialize share
the same refresh operation instead of interleaving mutations. Add and reuse an
in-flight field around the full _statusV2, _setAvailableSubscriptions, and
_state update sequence, clearing it when the operation completes or fails while
preserving error propagation and existing caller notification behavior.
Comment on lines
+64
to
+66
| plans: rawPlans | ||
| .map((e) => ProductV2.fromJson(Map<String, dynamic>.from(e as Map))) | ||
| .toList(), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a strongly typed Map.
As per path instructions, "keep things well-typed (don’t pass around untyped Maps with dynamic unless unavoidable)". Casting directly to Map<String, dynamic> avoids the untyped Map cast and the performance overhead of using .from().
♻️ Proposed fix
- plans: rawPlans
- .map((e) => ProductV2.fromJson(Map<String, dynamic>.from(e as Map)))
- .toList(),
+ plans: rawPlans
+ .map((e) => ProductV2.fromJson(e as Map<String, dynamic>))
+ .toList(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| plans: rawPlans | |
| .map((e) => ProductV2.fromJson(Map<String, dynamic>.from(e as Map))) | |
| .toList(), | |
| plans: rawPlans | |
| .map((e) => ProductV2.fromJson(e as Map<String, dynamic>)) | |
| .toList(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/subscription/models/products_v2_response.dart` around lines 64 -
66, Update the rawPlans mapping that constructs ProductV2 instances to cast each
entry directly to Map<String, dynamic> and pass it to ProductV2.fromJson,
removing the intermediate untyped Map cast and Map.from conversion.
Source: Path instructions
Comment on lines
+104
to
+120
| SubscriptionDetails productV2ToSubscriptionDetails( | ||
| ProductV2 product, { | ||
| required String stripeAppId, | ||
| }) { | ||
| final duration = kPlanIdToDuration[product.planId]; | ||
| if (duration == null) { | ||
| throw UnknownPlanIdException(product.planId); | ||
| } | ||
| return SubscriptionDetails( | ||
| id: product.planId, | ||
| price: product.amount / 100.0, | ||
| currency: product.currency, | ||
| duration: duration, | ||
| appId: stripeAppId, | ||
| isVisible: true, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move top-level function into a class.
As per path instructions, "keep functions inside classes unless they’re utility functions grouped as static members". productV2ToSubscriptionDetails is a top-level function and should be encapsulated within a class.
♻️ Proposed fix
-SubscriptionDetails productV2ToSubscriptionDetails(
- ProductV2 product, {
- required String stripeAppId,
-}) {
- final duration = kPlanIdToDuration[product.planId];
- if (duration == null) {
- throw UnknownPlanIdException(product.planId);
- }
- return SubscriptionDetails(
- id: product.planId,
- price: product.amount / 100.0,
- currency: product.currency,
- duration: duration,
- appId: stripeAppId,
- isVisible: true,
- );
-}
+class SubscriptionDetailsMapper {
+ static SubscriptionDetails fromProductV2(
+ ProductV2 product, {
+ required String stripeAppId,
+ }) {
+ final duration = kPlanIdToDuration[product.planId];
+ if (duration == null) {
+ throw UnknownPlanIdException(product.planId);
+ }
+ return SubscriptionDetails(
+ id: product.planId,
+ price: product.amount / 100.0,
+ currency: product.currency,
+ duration: duration,
+ appId: stripeAppId,
+ isVisible: true,
+ );
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SubscriptionDetails productV2ToSubscriptionDetails( | |
| ProductV2 product, { | |
| required String stripeAppId, | |
| }) { | |
| final duration = kPlanIdToDuration[product.planId]; | |
| if (duration == null) { | |
| throw UnknownPlanIdException(product.planId); | |
| } | |
| return SubscriptionDetails( | |
| id: product.planId, | |
| price: product.amount / 100.0, | |
| currency: product.currency, | |
| duration: duration, | |
| appId: stripeAppId, | |
| isVisible: true, | |
| ); | |
| } | |
| class SubscriptionDetailsMapper { | |
| static SubscriptionDetails fromProductV2( | |
| ProductV2 product, { | |
| required String stripeAppId, | |
| }) { | |
| final duration = kPlanIdToDuration[product.planId]; | |
| if (duration == null) { | |
| throw UnknownPlanIdException(product.planId); | |
| } | |
| return SubscriptionDetails( | |
| id: product.planId, | |
| price: product.amount / 100.0, | |
| currency: product.currency, | |
| duration: duration, | |
| appId: stripeAppId, | |
| isVisible: true, | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/subscription/models/products_v2_response.dart` around lines 104
- 120, Move productV2ToSubscriptionDetails into an appropriate class as a static
member, preserving its required stripeAppId parameter, plan-duration validation,
exception behavior, and SubscriptionDetails mapping unchanged. Update callers to
use the class-qualified method.
Source: Path instructions
| } | ||
|
|
||
| // Business rejection: detail is an OBJECT {code, reason}. | ||
| if (detail is Map && detail['code'] == 'promo_not_applicable') { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a strongly typed Map.
As per path instructions, "keep things well-typed (don’t pass around untyped Maps with dynamic unless unavoidable)". Checking against Map<String, dynamic> aligns with this requirement.
♻️ Proposed fix
- if (detail is Map && detail['code'] == 'promo_not_applicable') {
+ if (detail is Map<String, dynamic> && detail['code'] == 'promo_not_applicable') {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (detail is Map && detail['code'] == 'promo_not_applicable') { | |
| if (detail is Map<String, dynamic> && detail['code'] == 'promo_not_applicable') { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/subscription/repo/checkout_v2_repo.dart` at line 227, Update the
type check in the promo error handling around the promo_not_applicable condition
to use Map<String, dynamic> instead of the untyped Map, preserving the existing
code comparison and behavior.
Source: Path instructions
Comment on lines
+43
to
+45
| final Map<String, dynamic> json = | ||
| jsonDecode(res.body) as Map<String, dynamic>; | ||
| return ProductsV2Response.fromJson(json); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent potential text corruption by using utf8.decode(res.bodyBytes).
Relying on res.body for JSON decoding can lead to character corruption (mojibake) if the API response omits the charset=utf-8 directive in its Content-Type header, as the http package defaults to ISO-8859-1 (Latin-1) in its absence. Using utf8.decode(res.bodyBytes) ensures safe UTF-8 decoding for localized strings and currency symbols, maintaining consistency with BillingPortalRepo.
lib/features/subscription/repo/products_v2_repo.dart#L43-L45: ReplacejsonDecode(res.body)withjsonDecode(utf8.decode(res.bodyBytes)).lib/features/subscription/repo/status_v2_repo.dart#L22-L24: ReplacejsonDecode(res.body)withjsonDecode(utf8.decode(res.bodyBytes)).
📍 Affects 2 files
lib/features/subscription/repo/products_v2_repo.dart#L43-L45(this comment)lib/features/subscription/repo/status_v2_repo.dart#L22-L24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/subscription/repo/products_v2_repo.dart` around lines 43 - 45,
Use UTF-8 decoding of response body bytes before JSON parsing in the affected
repositories: update jsonDecode in
lib/features/subscription/repo/products_v2_repo.dart lines 43-45 and
lib/features/subscription/repo/status_v2_repo.dart lines 22-24 to decode
res.bodyBytes with utf8.decode, preserving the existing ProductsV2Response and
status response parsing flows.
Collaborator
|
@bbsatvik01 Hey, I've already implemented the client-side consumer logic for these new endpoints in the PR attached to the subscription pages ticket (#7655). I will read through this and see if there's any details I've missed, and integrate the unit tests. |
Author
|
Hey @ggurdin , just for a quick reference these are the meanings : paid — an active, self-owned Stripe subscription the user directly pays for (a real paying customer). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the client-side API logic for the new web subscription flow (#4977) — everything between the UI and the backend, so the subscription screens can be built directly on typed calls.
What's in it: models and repos for all 8
/subscription/*endpoints (products, promo-code preview, checkout with discount support, status, cancel, invoice history, billing portal, free trial), the controller wiring behind theSUBS_V2_WEBflag, and typed errors the UI can render — e.g. a rejected promo code comes back as one of 9 specific reasons (expired, wrong plan, already used…) instead of a generic failure.Money-safety lives in this layer, not the widgets: checkout can only ever send
{planId, promoCode?}, the submit path is single-flight-guarded so a double-tap can't fire two checkouts, and a products fetch failure surfaces as a retryable error instead of an empty catalog. Mobile and flag-off web are byte-for-byte unchanged (every v2 branch is gatedSUBS_V2_WEB && kIsWeb).161 subscription-module tests; the whole-repo suite runs clean on this branch in a fresh checkout. Verified against staging, which now serves the CMS entitlement source. UI/UX on top of this (screens, dialogs, buy-button states) is intentionally left for the UI work — see the client-rules notes in the contract doc referenced on #4977.
Closes nothing; part of #4977.
🤖 Generated with Claude Code
Summary by CodeRabbit