Stripe ghost subscription fix - #8941
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change centralizes staging detection, supplies Stripe configuration from plan data, separates service initialization, and defers subscription creation until payment confirmation. It adds payment and setup intent handling, filtered Stripe errors, renewal selection, and Stripe flow tests. ChangesStaging runtime configuration
Deferred Stripe payment flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ChoosePaymentMethod
participant StripeService
participant StripePaymentSheet
participant SubscriptionBackend
ChoosePaymentMethod->>StripeService: startStripeSDK(amount, email, intentMode)
StripeService->>StripePaymentSheet: present payment sheet
StripePaymentSheet->>StripeService: request deferred subscription
StripeService->>ChoosePaymentMethod: invoke onCreateSubscription
ChoosePaymentMethod->>SubscriptionBackend: create subscription
SubscriptionBackend-->>StripeService: return intent secret
StripeService->>StripePaymentSheet: confirm selected intent
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🟡 Not ready to approve
The updated Stripe flow is missing an applySettings() propagation step and has inconsistent Stripe error filtering that can surface developer-facing messages to users.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This pull request refactors the Stripe subscription flow to a deferred-intent approach (to avoid creating abandoned “ghost” subscriptions), sources the Stripe publishable key dynamically from the backend plans response, and centralizes staging-environment detection in lantern-core.
Changes:
- Moves Android Stripe subscriptions to a deferred-intent PaymentSheet flow where the backend subscription is created only after the user taps Pay.
- Refactors plans/provider modeling to expose a typed Stripe publishable key (
PlansData.stripePubKey) and keeps the Stripe SDK key in sync when plans change. - Centralizes environment selection via
Opts.IsStaging()and ensures staging is set early enough on Android IPC startup.
File summaries
| File | Description |
|---|---|
| lib/features/plans/provider/plans_notifier.dart | Adds a state listener to sync Stripe publishable key from fetched/cached plans. |
| lib/features/auth/choose_payment_method.dart | Switches Android Stripe flow to deferred-intent callbacks; improves Stripe error handling in UI. |
| lib/core/services/stripe_service.dart | Reworks Stripe service to deferred-intent PaymentSheet initialization/confirmation and adds user-facing error filtering. |
| lib/core/services/injection_container.dart | Registers StripeService on Android without upfront initialization; simplifies notification init path. |
| lib/core/models/plan_data.dart | Adds platform provider accessors and typed provider data (incl. Stripe pubKey). |
| lib/core/extensions/plan.dart | Adds monthlyUsdCents helper for Stripe amount quoting. |
| lib/core/extensions/error.dart | Adds StripeException-specific localized description filtering to avoid developer-facing messages. |
| lib/core/common/app_secrets.dart | Removes Stripe publishable key accessors from env-based secrets. |
| lantern-core/utils/common.go | Adds Opts.IsStaging() helper to centralize staging environment checks. |
| lantern-core/mobile/mobile.go | Ensures staging env is set before backend construction on Android IPC startup. |
| lantern-core/core.go | Replaces ad-hoc staging checks with opts.IsStaging(). |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
lantern-core/utils/common.go (1)
18-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for
Opts.IsStaging.
Opts.IsStaging()controls stage-environment selection, but the test files do not cover"stage","staging", or a non-staging value. Add focused cases for those inputs.🤖 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 `@lantern-core/utils/common.go` around lines 18 - 23, Add focused regression tests for Opts.IsStaging covering both accepted values, "stage" and "staging", which must return true, plus a non-staging environment value, which must return false. Keep the tests table-driven or otherwise scoped specifically to this method.
🤖 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/core/extensions/plan.dart`:
- Around line 15-20: Update the monthlyUsdCents getter to convert the Stripe
expectedMonthlyPrice amount from major USD units to cents by multiplying the
parsed value by 100 before rounding. Keep _amountOf and _formatPriceMap
unchanged so display currency formatting continues to use the existing
major-unit conversion.
In `@lib/core/models/plan_data.dart`:
- Around line 9-12: Update the platformProviders getter in the plan data model
to use PlatformUtils.isAndroid instead of PlatformUtils.isMobile, returning
providers.android on Android and providers.desktop on iOS or other platforms to
match choose_payment_method.dart and sort the displayed provider list.
In `@lib/core/services/stripe_service.dart`:
- Around line 163-174: Unify Stripe user-facing error filtering by making
StripeErrorMessage.userFacingMessage delegate to
StripeErrorExtension.localizedDescription, or by sharing its allowlist that
permits only card_error, validation_error, and decline-coded errors. Update
lib/core/services/stripe_service.dart:163-174 to remove the inconsistent
_hiddenErrorTypes denylist; retain lib/core/extensions/error.dart:159-174 as the
single implementation if delegation is used, otherwise remove it only if it
becomes redundant.
In `@lib/features/auth/choose_payment_method.dart`:
- Around line 209-246: Guard both the onSuccess and onError callbacks passed to
startStripeSDK with an early context.mounted check before invoking
finishPaymentRedirect, onPurchaseResult, or any context.showSnackBar call.
Preserve the existing success, cancellation, logging, and error-message behavior
when the widget remains mounted.
---
Nitpick comments:
In `@lantern-core/utils/common.go`:
- Around line 18-23: Add focused regression tests for Opts.IsStaging covering
both accepted values, "stage" and "staging", which must return true, plus a
non-staging environment value, which must return false. Keep the tests
table-driven or otherwise scoped specifically to this method.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44fe5435-fe81-4a4b-9ee0-65f1aeafdae7
📒 Files selected for processing (11)
lantern-core/core.golantern-core/mobile/mobile.golantern-core/utils/common.golib/core/common/app_secrets.dartlib/core/extensions/error.dartlib/core/extensions/plan.dartlib/core/models/plan_data.dartlib/core/services/injection_container.dartlib/core/services/stripe_service.dartlib/features/auth/choose_payment_method.dartlib/features/plans/provider/plans_notifier.dart
💤 Files with no reviewable changes (1)
- lib/core/common/app_secrets.dart
There was a problem hiding this comment.
🟡 Not ready to approve
The Stripe publishable key is updated but Stripe.instance.applySettings() is no longer called anywhere, which can prevent the native SDK from receiving the key before initPaymentSheet runs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
lib/core/services/stripe_service.dart:62
Stripe.publishableKeyis set viaupdatePublishableKey, but I can't find any call toStripe.instance.applySettings()in the repo. flutter_stripe's docs/examples requireapplySettings()after setting the key so the native SDK picks it up; without it,initPaymentSheetmay fail due to a missing/stale publishable key.
// initPaymentSheet applies any pending settings (including the
// publishable key) to the native SDK itself. If plans never provided
// a key, this throws StripeConfigException into the catch below.
await Stripe.instance.initPaymentSheet(
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
The deferred PaymentSheet is initialized with IntentMode.paymentMode but the flow can return a SetupIntent client secret, which can break the PaymentSheet confirm step due to intent-type mismatch.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/core/extensions/plan.dart`:
- Around line 13-15: Update the yearlyAmountInt getter to return the annual
Stripe amount in USD cents: convert the monthly major-unit price by multiplying
by 12 and 100 before rounding, while preserving the empty-price result of 0.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f22c0539-49ea-427f-826e-7a2cd5a30656
📒 Files selected for processing (6)
lib/core/extensions/plan.dartlib/core/models/app_setting.dartlib/core/services/stripe_service.dartlib/features/auth/choose_payment_method.dartlib/features/developer/developer_mode.dartlib/lantern_app.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/features/auth/choose_payment_method.dart
- lib/core/services/stripe_service.dart
There was a problem hiding this comment.
🟡 Not ready to approve
The Stripe amount passed to the PaymentSheet is currently derived from expectedMonthlyPrice (not guaranteed to be USD cents) and the Stripe SDK settings aren’t applied before initPaymentSheet, both of which can break or mis-price payments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
lib/core/services/stripe_service.dart:63
- After setting
Stripe.publishableKeyviaupdatePublishableKey, the SDK settings are never applied before callinginitPaymentSheet.flutter_stripe’s recommended setup callsStripe.instance.applySettings()after updating the publishable key; otherwise the native SDK may still be configured with the old/missing key when the sheet is initialized.
// initPaymentSheet applies any pending settings (including the
// publishable key) to the native SDK itself. If plans never provided
// a key, this throws StripeConfigException into the catch below.
await Stripe.instance.initPaymentSheet(
paymentSheetParameters: SetupPaymentSheetParameters(
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
There are correctness issues that can break payments (SetupIntent secret ignored) and misquote/charge amounts (Stripe amount derived from monthly-equivalent pricing), plus a potential null-crash in the app builder.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (3)
lib/core/extensions/plan.dart:15
yearlyAmountIntis computed fromexpectedMonthlyPrice(the monthly-equivalent display price), but it’s used as the USD-cents amount passed to Stripe’s payment sheet. This will quote/charge the wrong amount for 1y/2y plans. Use the backend-providedusdPrice(already an int in USD cents) instead.
int get yearlyAmountInt => expectedMonthlyPrice.isEmpty
? 0
: _amountOf(expectedMonthlyPrice).round();
lib/core/services/stripe_service.dart:130
_createSubscriptionAndConfirmalways usesoptions.clientSecret, but the comment/log indicate the backend can return a SetupIntent secret (stored insetupIntentClientSecret) for the trial path. In that caseclientSecretwill be empty and this will always throw, breaking the flow. Prefer the setup-intent secret when the payment-intent secret is absent.
// Normal path returns a PaymentIntent secret; the trial path (user
// still has an unexpired one-time purchase) returns a SetupIntent
// secret instead.
final secret = options.clientSecret;
if (secret.isEmpty) {
lib/lantern_app.dart:248
MaterialApp.router'sbuildercan receive a nullchild; force-unwrapping withchild!will crash in that case (both in the early return and in theBanner). Handle the null case and avoid repeated!by unwrapping once.
builder: (context, child) {
if (!isStaging) return child!;
return Banner(
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
It introduces a potential runtime crash in MaterialApp.router.builder by force-unwrapping a nullable child (child!) instead of safely handling null.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
lib/lantern_app.dart:254
- MaterialApp.router.builder receives a nullable
child; usingchild!will throw if Flutter ever calls the builder withchild == null(e.g., during certain transitions/tests). Handle null safely and reuse the same non-null widget for both branches.
builder: (context, child) {
if (!isStaging) return child!;
return Banner(
message: 'STAGING',
location: BannerLocation.topEnd,
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
* Harden deferred Stripe subscription flow * code review updates
There was a problem hiding this comment.
🔵 Human review recommended
It changes core payment/subscription behavior and environment/key selection, which is high-impact and warrants careful human validation across platforms and backend integration paths.
Review details
Suppressed comments (3)
test/core/services/stripe_service_test.dart:327
- The fake Stripe platform records that initialise ran, but it discards the publishableKey argument, so the "publishable key is applied" test can't actually verify which key was used. Capture the key (or include it in the event log) so tests can assert it.
events.add('initialise:start');
events.add('initialise:end');
}
lib/core/services/stripe_service.dart:43
- The docstring says this method "no-ops on an unchanged key", but the implementation always assigns Stripe.publishableKey. Either update the comment or add an equality guard so repeated plans refreshes don't imply a settings change.
void updatePublishableKey(String? pubKey) {
if (pubKey == null || pubKey.isEmpty) return;
Stripe.publishableKey = pubKey;
test/core/services/stripe_service_test.dart:71
- This test name implies we assert the publishable key used by StripePlatform.initialise(), but the current expectation only checks call ordering. Once the fake platform records the key, assert it here to make the test meaningful.
This issue also appears on line 325 of the same file.
'initialise:start',
'initialise:end',
'payment-sheet:init',
'payment-sheet:present',
]);
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
test/core/services/stripe_service_test.dart (5)
245-296: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for null
userData.
stripeIntentModeForRenewalreturnsStripeIntentMode.paymentwhenuserDatais null. That branch is reachable in production, because_stripeIntentModeinlib/features/auth/choose_payment_method.dartpassesref.read(homeProvider).value?.legacyUserData, which is null while the provider loads or after it fails. No test covers it.♻️ Proposed test
+ test('uses payment mode when user data is unavailable', () { + expect( + stripeIntentModeForRenewal(null, currentTimeSeconds: 1000), + StripeIntentMode.payment, + ); + });🤖 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/core/services/stripe_service_test.dart` around lines 245 - 296, Add a test in the stripeIntentModeForRenewal group that passes null userData and currentTimeSeconds, then asserts the result is StripeIntentMode.payment. Keep the existing non-null purchaser and subscription cases unchanged.
66-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the forwarded publishable key, not only the call order.
The test name states that the key is applied before
initPaymentSheetruns. The assertions only check event order._FakeStripePlatform.initialisediscardspublishableKey, so a regression that forwards the wrong key still passes.Capture the key in the fake and assert its value.
♻️ Proposed refactor
expect(platform.events, [ 'initialise:start', 'initialise:end', 'payment-sheet:init', 'payment-sheet:present', ]); + expect(platform.initialisedKeys, ['pk_test_deferred_settings']);In
_FakeStripePlatform:final events = <String>[]; + final initialisedKeys = <String>[];events.add('initialise:start'); + initialisedKeys.add(publishableKey); events.add('initialise:end');Also clear
initialisedKeysinreset().🤖 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/core/services/stripe_service_test.dart` around lines 66 - 71, Update _FakeStripePlatform to capture each publishableKey passed to initialise in an initialisedKeys collection, clear that collection in reset(), and extend the test assertions to verify the expected key value alongside the existing event order checks.
24-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset the global publishable key in
setUp.
Stripe.publishableKeyis static and persists between tests in this file. Only the first test sets it. The later tests then depend on state that the first test leaked. If a test is skipped, reordered, or run in isolation, that dependency becomes visible.Set a known key in
setUpso each test starts from the same state.♻️ Proposed refactor
setUp(() { platform.reset(); + Stripe.publishableKey = 'pk_test_default'; });🤖 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/core/services/stripe_service_test.dart` around lines 24 - 26, Update the setUp block to assign Stripe.publishableKey a known test key after resetting platform, ensuring every test starts with isolated, deterministic global state.
155-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the empty-secret mismatch path.
No test covers the case where the selected
intentModeand the returned secret disagree. That path callsintentCreationCallbackwith an error and leaves a created subscription unconfirmed. A test would lock in the behavior and the message.Add a case that requests
StripeIntentMode.setupand returns onlyclientSecret, then assert that the recorded callback carries an error.🤖 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/core/services/stripe_service_test.dart` around lines 155 - 159, Add a test in the Stripe service test suite for the setup intent mode receiving only a clientSecret, using the existing subscription creation callback setup. Capture the intentCreationCallback invocation and assert it carries an error while the created subscription remains unconfirmed, including the expected mismatch message.
344-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCall
_confirmHandlerdirectly instead of usingFunction.apply.
_confirmHandlerhas the concreteConfirmHandlersignature, so a direct call preserves compile-time checking for that callback.Function.apply(_confirmHandler!, [_paymentMethod, false])hides signature changes behind a runtime failure.♻️ Proposed refactor
- final confirmation = Function.apply(_confirmHandler!, [ - _paymentMethod, - false, - ]); - if (confirmation is Future<void>) { - await confirmation; - } + await _confirmHandler!(_paymentMethod, false);🤖 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/core/services/stripe_service_test.dart` around lines 344 - 354, In the confirmation loop, replace the Function.apply invocation with a direct call to the non-null _confirmHandler callback, passing _paymentMethod and false according to its ConfirmHandler signature. Preserve the existing Future<void> handling and _intentCallbackCompleter wait.
🤖 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/core/services/stripe_service.dart`:
- Around line 20-27: Replace the raw `hasPurchaseHistory` string comparison in
the purchase-status logic with a JSON-aware helper such as
`_hasPurchaseHistory`, treating blank text, `null`, empty arrays,
whitespace-only arrays, and empty objects as no history while preserving a
textual fallback for non-JSON values. Update the `hasActiveOneTimePurchase`
calculation to use this helper.
In `@lib/features/auth/choose_payment_method.dart`:
- Around line 268-269: Update _stripeIntentMode in
lib/features/auth/choose_payment_method.dart at lines 268-269 to await or
otherwise obtain the required homeProvider data before selecting the renewal
intent mode, and log the degraded selection when user data is unavailable
instead of treating it as confirmed non-renewal. In
lib/core/services/stripe_service.dart at lines 186-193, update
_createSubscriptionAndConfirm to log an empty secret together with the selected
intentMode and subscriptionId; do not substitute the alternate secret.
---
Nitpick comments:
In `@test/core/services/stripe_service_test.dart`:
- Around line 245-296: Add a test in the stripeIntentModeForRenewal group that
passes null userData and currentTimeSeconds, then asserts the result is
StripeIntentMode.payment. Keep the existing non-null purchaser and subscription
cases unchanged.
- Around line 66-71: Update _FakeStripePlatform to capture each publishableKey
passed to initialise in an initialisedKeys collection, clear that collection in
reset(), and extend the test assertions to verify the expected key value
alongside the existing event order checks.
- Around line 24-26: Update the setUp block to assign Stripe.publishableKey a
known test key after resetting platform, ensuring every test starts with
isolated, deterministic global state.
- Around line 155-159: Add a test in the Stripe service test suite for the setup
intent mode receiving only a clientSecret, using the existing subscription
creation callback setup. Capture the intentCreationCallback invocation and
assert it carries an error while the created subscription remains unconfirmed,
including the expected mismatch message.
- Around line 344-354: In the confirmation loop, replace the Function.apply
invocation with a direct call to the non-null _confirmHandler callback, passing
_paymentMethod and false according to its ConfirmHandler signature. Preserve the
existing Future<void> handling and _intentCallbackCompleter wait.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b04a3f9d-17c8-451c-aa66-3199e28b3369
📒 Files selected for processing (4)
lib/core/services/stripe_service.dartlib/features/auth/choose_payment_method.dartlib/lantern_app.darttest/core/services/stripe_service_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/lantern_app.dart
There was a problem hiding this comment.
🟡 Changes recommended
The Stripe confirm-step error mapping can leak developer/diagnostic details to users, and the new publishable-key test doesn’t currently assert the key passed into Stripe initialisation without the suggested fixes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (4)
test/core/services/stripe_service_test.dart:342
- initialise() should record the publishableKey it was called with (e.g., lastPublishableKey = publishableKey) so the test can assert the updated key was actually applied before PaymentSheet initialization.
}) async {
events.add('initialise:start');
events.add('initialise:end');
}
lib/core/services/stripe_service.dart:237
- _createSubscriptionAndConfirm() builds the PaymentSheet inline error message from e.toString() for all non-StripeException failures. If an unexpected exception occurs (e.g., TypeError/StateError), this can surface developer/diagnostic details to end users. Prefer only surfacing the explicit user-facing Exception message you throw from onCreateSubscription, and otherwise fall back to a generic localizedDescription.
final message = e is StripeException
? e.userFacingMessage
: e.toString().replaceFirst('Exception: ', '');
test/core/services/stripe_service_test.dart:71
- This test asserts initialise() happens before initPaymentSheet(), but it never verifies that initialise() was called with the updated publishable key. With the fake recording lastPublishableKey, assert it here so the test matches its name/purpose.
expect(platform.events, [
'initialise:start',
'initialise:end',
'payment-sheet:init',
'payment-sheet:present',
test/core/services/stripe_service_test.dart:329
- _FakeStripePlatform doesn’t capture the publishableKey passed to initialise(), so tests can’t verify which key was applied. Add a field to record it (and reset it between tests).
This issue also appears on line 339 of the same file.
class _FakeStripePlatform extends StripePlatform {
final events = <String>[];
final intentCallbacks = <IntentCreationCallbackParams>[];
SetupPaymentSheetParameters? paymentSheetParameters;
ConfirmHandler? _confirmHandler;
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🔵 Human review recommended
It changes a critical payment flow (Stripe deferred-intent + dynamic keying) across multiple layers and needs manual end-to-end validation on Android/Stripe to ensure no regressions in real checkout scenarios.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
This pull request refactors and improves the Stripe payment flow, particularly for Android, by switching to a deferred-intent approach. It ensures that the Stripe publishable key is dynamically sourced from the backend, centralizes environment selection logic, and improves error handling and user messaging for Stripe failures. Additionally, it cleans up and modernizes how provider data is modeled and accessed.
Stripe Payment Flow Improvements:
lib/core/services/stripe_service.dart,lib/features/auth/choose_payment_method.dart) [1] [2]lib/core/services/stripe_service.dart,lib/core/models/plan_data.dart,lib/core/services/injection_container.dart) [1] [2] [3]Provider and Plan Modeling:
ProviderDataclass, making it easier and safer to access provider-specific fields like Stripe'spubKey. (lib/core/models/plan_data.dart)PlansDatafor retrieving the publishable key for the current platform and for sorting providers by subscription support. (lib/core/models/plan_data.dart)Error Handling and User Messaging:
lib/core/extensions/error.dart,lib/core/services/stripe_service.dart) [1] [2]lib/core/extensions/plan.dart)Environment Handling:
IsStagingmethod onOpts, ensuring consistent environment checks across the codebase. (lantern-core/utils/common.go,lantern-core/core.go,lantern-core/mobile/mobile.go) [1] [2] [3]Other Cleanups:
AppSecretsand updates imports accordingly. (lib/core/common/app_secrets.dart)lib/core/services/injection_container.dart)Summary by CodeRabbit