feat(web): add web support for collectBankAccount and verifyMicrodeposits#2396
Open
BiAksoy wants to merge 4 commits into
Open
feat(web): add web support for collectBankAccount and verifyMicrodeposits#2396BiAksoy wants to merge 4 commits into
BiAksoy wants to merge 4 commits into
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:
📝 WalkthroughWalkthroughAdds bank-account collection and microdeposit verification end-to-end: new Freezed data models, JS interop extensions, web implementation branching on intent type, a new sealed-union result type, and platform/surface API updates to return the union result. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DartStripe as Stripe (Dart)
participant JSInterop
participant StripeJS as Stripe.js
participant Parser as Result Parser
Client->>DartStripe: collectBankAccount(isPaymentIntent, clientSecret, params)
alt isPaymentIntent
DartStripe->>JSInterop: collectBankAccountForPayment(clientSecret, params?.toJson())
JSInterop->>StripeJS: collectBankAccountForPayment(options)
else isSetupIntent
DartStripe->>JSInterop: collectBankAccountForSetup(clientSecret, params?.toJson())
JSInterop->>StripeJS: collectBankAccountForSetup(options)
end
StripeJS-->>JSInterop: JSPromise<JSIntentResponse>
JSInterop-->>DartStripe: Future<IntentResponse>
alt response.error
DartStripe->>Parser: throw StripeError(response.error)
else success
DartStripe->>Parser: CollectBankAccountResult.paymentIntent/setupIntent(response)
end
Parser-->>Client: CollectBankAccountResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.dart`:
- Around line 9-18: Add constructor-time validation to
VerifyMicrodepositsForPaymentData (and mirror it in the setup-intent variant) so
the model cannot be constructed with both or neither of amounts and
descriptorCode and so amounts, if provided, has exactly two entries: add asserts
in the generated factory/redirecting constructor for
_VerifyMicrodepositsForPaymentData that enforce (1) (amounts == null) !=
(descriptorCode == null) and (2) amounts == null || amounts.length == 2, and
update the equivalent constructor/asserts in the setup-intent variant to match.
🪄 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
Run ID: d2e76daf-c9bf-4748-b126-0fb104e3bf1d
📒 Files selected for processing (21)
packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.freezed.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.g.dartpackages/stripe_js/lib/src/api/payment_intents/payment_intents.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.freezed.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.g.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.freezed.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.g.dartpackages/stripe_js/lib/src/api/setup_intents/setup_intents.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.freezed.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.g.dartpackages/stripe_js/lib/src/js/payment_intents/collect_bank_account_for_payment.dartpackages/stripe_js/lib/src/js/payment_intents/payment_intents.dartpackages/stripe_js/lib/src/js/payment_intents/verify_microdeposits_for_payment.dartpackages/stripe_js/lib/src/js/setup_intents/collect_bank_account_for_setup.dartpackages/stripe_js/lib/src/js/setup_intents/setup_intents.dartpackages/stripe_js/lib/src/js/setup_intents/verify_microdeposits_for_setup.dartpackages/stripe_web/lib/src/web_stripe.dart
Comment on lines
+9
to
+18
| const factory VerifyMicrodepositsForPaymentData({ | ||
| /// The amounts of the microdeposits that were deposited on the | ||
| /// customer's bank account. Must contain exactly 2 values. | ||
| /// Mutually exclusive with [descriptorCode]. | ||
| List<int>? amounts, | ||
|
|
||
| /// The descriptor code from the microdeposit to the customer's | ||
| /// bank account. Mutually exclusive with [amounts]. | ||
| @JsonKey(name: 'descriptor_code') String? descriptorCode, | ||
| }) = _VerifyMicrodepositsForPaymentData; |
There was a problem hiding this comment.
Enforce the documented amounts/descriptorCode invariants in the model.
Right now this public type allows both fields, neither field, or an amounts list with the wrong length. That pushes an invalid request all the way to Stripe.js instead of failing fast at construction time.
Proposed fix
`@freezed`
abstract class VerifyMicrodepositsForPaymentData
with _$VerifyMicrodepositsForPaymentData {
+ `@Assert`(
+ 'amounts == null || amounts.length == 2',
+ 'amounts must contain exactly 2 values',
+ )
+ `@Assert`(
+ '(amounts == null) != (descriptorCode == null)',
+ 'Provide either amounts or descriptorCode',
+ )
const factory VerifyMicrodepositsForPaymentData({
/// The amounts of the microdeposits that were deposited on the
/// customer's bank account. Must contain exactly 2 values.
/// Mutually exclusive with [descriptorCode].
List<int>? amounts,Please mirror the same constructor asserts in the setup-intent variant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.dart`
around lines 9 - 18, Add constructor-time validation to
VerifyMicrodepositsForPaymentData (and mirror it in the setup-intent variant) so
the model cannot be constructed with both or neither of amounts and
descriptorCode and so amounts, if provided, has exactly two entries: add asserts
in the generated factory/redirecting constructor for
_VerifyMicrodepositsForPaymentData that enforce (1) (amounts == null) !=
(descriptorCode == null) and (2) amounts == null || amounts.length == 2, and
update the equivalent constructor/asserts in the setup-intent variant to match.
BiAksoy
force-pushed
the
feat/web-us-bank-account
branch
from
708c6ed to
4fae05b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stripe_platform_interface/lib/src/stripe_platform_interface.dart (1)
160-170:⚠️ Potential issue | 🟠 MajorBreaking change on a public platform-interface API — bump major version and add migration notes.
collectBankAccountandverifyPaymentIntentWithMicrodepositschanged return type fromFuture<PaymentIntent>toFuture<CollectBankAccountResult>. This breaks:
- every third-party/federated plugin that extends
StripePlatformand still returnsFuture<PaymentIntent>(compile error);- every app calling the public
Stripe.instance.collectBankAccount(...)inpackages/stripe/lib/src/stripe.dart.Please make sure the PR bumps the major version of both
stripe_platform_interfaceandflutter_stripe, and adds a CHANGELOG entry with a short migration snippet (switch on the sealed union variants as shown inus_bank_account_direct_debit_screen.dart). Without this,pubconsumers will get silent source breakage on upgrade.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_platform_interface/lib/src/stripe_platform_interface.dart` around lines 160 - 170, The public API for collectBankAccount and verifyPaymentIntentWithMicrodeposits on StripePlatform changed return types from Future<PaymentIntent> to Future<CollectBankAccountResult>; update the release to a breaking major bump for both stripe_platform_interface and flutter_stripe, and add a CHANGELOG entry with a short migration snippet showing how to handle the new sealed-union CollectBankAccountResult (e.g., switching on variants as demonstrated in us_bank_account_direct_debit_screen.dart) so plugin implementers and app callers know to update implementations that previously returned Future<PaymentIntent>.
🧹 Nitpick comments (2)
packages/stripe_web/lib/src/web_stripe.dart (2)
511-601: Consider extracting the shared error-handling / response-mapping to remove duplication.Both
collectBankAccountandverifyPaymentIntentWithMicrodepositsrepeat the same pattern four times: call JS API → checkresponse.error→ throwStripeError→ wrap intent in aCollectBankAccountResultvariant. A small helper makes each method ~6 lines and centralises the error construction (which is also slightly inconsistent today —response.error?.messageis null-safe butresponse.error!.codeforce-unwraps on the same line, even though the earlierif (response.error != null)guard already proved it non-null).♻️ Suggested refactor (illustrative)
CollectBankAccountResult _unwrapPaymentIntent( stripe_js.PaymentIntentResponse response, ) { final error = response.error; if (error != null) { throw StripeError(message: error.message ?? '', code: error.code); } return CollectBankAccountResult.paymentIntent( response.paymentIntent!.parse(), ); } CollectBankAccountResult _unwrapSetupIntent( stripe_js.SetupIntentResponse response, ) { final error = response.error; if (error != null) { throw StripeError(message: error.message ?? '', code: error.code); } return CollectBankAccountResult.setupIntent( response.setupIntent!.parse(), ); }Then each branch becomes a single
return _unwrapPaymentIntent(await js.collectBankAccountForPayment(...))style call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_web/lib/src/web_stripe.dart` around lines 511 - 601, The collectBankAccount and verifyPaymentIntentWithMicrodeposits methods duplicate error-checking and response-to-result mapping; extract two helpers (e.g., _unwrapPaymentIntent and _unwrapSetupIntent) that take the respective JS response types, read response.error into a local variable, throw StripeError with error.message ?? '' and error.code when non-null, and otherwise return CollectBankAccountResult.paymentIntent(response.paymentIntent!.parse()) or CollectBankAccountResult.setupIntent(response.setupIntent!.parse()); then replace the in-place checks in collectBankAccount and verifyPaymentIntentWithMicrodeposits with single-line returns calling these helpers to centralize logic and fix the null-safety inconsistency.
512-558:params.onEventis silently dropped on web — document or warn.
CollectBankAccountParams.onEventis wired to_financialConnectionsEventHandlerin the method-channel implementation, but on web no callback is ever invoked. Since the standalone Financial Connections JS SDK isn't integrated, events cannot be forwarded — but today users who passonEventon web will see no events with no indication why. Consider either a one-line doc note on the public API for web, or adev.log/ debug warning whenparams.onEvent != nullon web. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_web/lib/src/web_stripe.dart` around lines 512 - 558, The web implementation of collectBankAccount (in collectBankAccount) currently ignores CollectBankAccountParams.onEvent so callers get no events or feedback; add a simple dev-time warning when params.onEvent != null to surface this (or update public docs) — specifically, in the collectBankAccount method, check params.onEvent and if non-null call debugPrint/dev.log (or use package:logging) to indicate Financial Connections events are not supported on web and that _financialConnectionsEventHandler will not be invoked; keep the message concise and guarded so it only runs in debug/dev builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.dart`:
- Around line 9-18: Add runtime validation to the
VerifyMicrodepositsForSetupData union by adding `@Assert` annotations on the
generated data class/factory: ensure that if amounts is non-null it has exactly
2 elements (e.g., `@Assert`('amounts == null || amounts.length == 2', 'amounts
must contain exactly 2 values')) and that amounts and descriptorCode are
mutually exclusive (e.g., `@Assert`('!(amounts != null && descriptorCode !=
null)', 'amounts and descriptorCode are mutually exclusive')); apply these
asserts to the VerifyMicrodepositsForSetupData
factory/_VerifyMicrodepositsForSetupData so the constraints are enforced at
construction time.
---
Outside diff comments:
In `@packages/stripe_platform_interface/lib/src/stripe_platform_interface.dart`:
- Around line 160-170: The public API for collectBankAccount and
verifyPaymentIntentWithMicrodeposits on StripePlatform changed return types from
Future<PaymentIntent> to Future<CollectBankAccountResult>; update the release to
a breaking major bump for both stripe_platform_interface and flutter_stripe, and
add a CHANGELOG entry with a short migration snippet showing how to handle the
new sealed-union CollectBankAccountResult (e.g., switching on variants as
demonstrated in us_bank_account_direct_debit_screen.dart) so plugin implementers
and app callers know to update implementations that previously returned
Future<PaymentIntent>.
---
Nitpick comments:
In `@packages/stripe_web/lib/src/web_stripe.dart`:
- Around line 511-601: The collectBankAccount and
verifyPaymentIntentWithMicrodeposits methods duplicate error-checking and
response-to-result mapping; extract two helpers (e.g., _unwrapPaymentIntent and
_unwrapSetupIntent) that take the respective JS response types, read
response.error into a local variable, throw StripeError with error.message ?? ''
and error.code when non-null, and otherwise return
CollectBankAccountResult.paymentIntent(response.paymentIntent!.parse()) or
CollectBankAccountResult.setupIntent(response.setupIntent!.parse()); then
replace the in-place checks in collectBankAccount and
verifyPaymentIntentWithMicrodeposits with single-line returns calling these
helpers to centralize logic and fix the null-safety inconsistency.
- Around line 512-558: The web implementation of collectBankAccount (in
collectBankAccount) currently ignores CollectBankAccountParams.onEvent so
callers get no events or feedback; add a simple dev-time warning when
params.onEvent != null to surface this (or update public docs) — specifically,
in the collectBankAccount method, check params.onEvent and if non-null call
debugPrint/dev.log (or use package:logging) to indicate Financial Connections
events are not supported on web and that _financialConnectionsEventHandler will
not be invoked; keep the message concise and guarded so it only runs in
debug/dev builds.
🪄 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
Run ID: 0ef126df-c9dd-4728-b3a8-4c9bde3caa69
📒 Files selected for processing (28)
example/lib/screens/regional_payment_methods/us_bank_account_direct_debit_screen.dartpackages/stripe/lib/src/stripe.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.freezed.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.g.dartpackages/stripe_js/lib/src/api/payment_intents/payment_intents.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.freezed.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.g.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.freezed.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.g.dartpackages/stripe_js/lib/src/api/setup_intents/setup_intents.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.freezed.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.g.dartpackages/stripe_js/lib/src/js/payment_intents/collect_bank_account_for_payment.dartpackages/stripe_js/lib/src/js/payment_intents/payment_intents.dartpackages/stripe_js/lib/src/js/payment_intents/verify_microdeposits_for_payment.dartpackages/stripe_js/lib/src/js/setup_intents/collect_bank_account_for_setup.dartpackages/stripe_js/lib/src/js/setup_intents/setup_intents.dartpackages/stripe_js/lib/src/js/setup_intents/verify_microdeposits_for_setup.dartpackages/stripe_platform_interface/lib/src/method_channel_stripe.dartpackages/stripe_platform_interface/lib/src/models/collect_bank_account_result.dartpackages/stripe_platform_interface/lib/src/models/collect_bank_account_result.freezed.dartpackages/stripe_platform_interface/lib/src/stripe_platform_interface.dartpackages/stripe_platform_interface/lib/stripe_platform_interface.dartpackages/stripe_web/lib/src/web_stripe.dart
✅ Files skipped from review due to trivial changes (16)
- packages/stripe_platform_interface/lib/stripe_platform_interface.dart
- packages/stripe_js/lib/src/api/payment_intents/payment_intents.dart
- packages/stripe_js/lib/src/api/setup_intents/setup_intents.dart
- packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.g.dart
- packages/stripe_js/lib/src/js/setup_intents/setup_intents.dart
- packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.g.dart
- packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.freezed.dart
- packages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.dart
- packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.g.dart
- packages/stripe_platform_interface/lib/src/models/collect_bank_account_result.freezed.dart
- packages/stripe_js/lib/src/js/setup_intents/collect_bank_account_for_setup.dart
- packages/stripe_js/lib/src/js/payment_intents/payment_intents.dart
- packages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.freezed.dart
- packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.dart
- packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.freezed.dart
- packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.freezed.dart
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.g.dart
- packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.dart
- packages/stripe_js/lib/src/js/setup_intents/verify_microdeposits_for_setup.dart
- packages/stripe_js/lib/src/js/payment_intents/verify_microdeposits_for_payment.dart
- packages/stripe_js/lib/src/js/payment_intents/collect_bank_account_for_payment.dart
Comment on lines
+9
to
+18
| const factory VerifyMicrodepositsForSetupData({ | ||
| /// The amounts of the microdeposits that were deposited on the | ||
| /// customer's bank account. Must contain exactly 2 values. | ||
| /// Mutually exclusive with [descriptorCode]. | ||
| List<int>? amounts, | ||
|
|
||
| /// The descriptor code from the microdeposit to the customer's | ||
| /// bank account. Mutually exclusive with [amounts]. | ||
| @JsonKey(name: 'descriptor_code') String? descriptorCode, | ||
| }) = _VerifyMicrodepositsForSetupData; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify json_serializable null-handling for Stripe JS data models.
set -euo pipefail
echo "Global/local json_serializable null handling:"
rg -n -C3 'include_if_null|includeIfNull' build.yaml pubspec.yaml packages/stripe_js || true
echo
echo "Generated serializer for VerifyMicrodepositsForSetupData, if present:"
fd -i 'verify_microdeposits_for_setup_data.g.dart' packages/stripe_js --exec sed -n '1,160p' {} || true
echo
echo "Comparable microdeposit data models:"
fd -i 'verify_microdeposits.*data.dart' packages/stripe_js --exec sed -n '1,120p' {} || trueRepository: flutter-stripe/flutter_stripe
Length of output: 3184
Add runtime validation for mutually-exclusive and constrained fields.
The model documents Stripe's constraints (amounts must contain exactly 2 values; amounts and descriptorCode are mutually exclusive), but doesn't enforce them at construction time. Null-field omission is already configured globally (include_if_null: false in build.yaml), so that concern is satisfied. Add @Assert annotations to validate the documented constraints:
+ `@Assert`(
+ 'amounts == null || amounts.length == 2',
+ 'amounts must contain exactly 2 values.',
+ )
+ `@Assert`(
+ 'amounts == null || descriptorCode == null',
+ 'amounts and descriptorCode are mutually exclusive.',
+ )
const factory VerifyMicrodepositsForSetupData({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.dart`
around lines 9 - 18, Add runtime validation to the
VerifyMicrodepositsForSetupData union by adding `@Assert` annotations on the
generated data class/factory: ensure that if amounts is non-null it has exactly
2 elements (e.g., `@Assert`('amounts == null || amounts.length == 2', 'amounts
must contain exactly 2 values')) and that amounts and descriptorCode are
mutually exclusive (e.g., `@Assert`('!(amounts != null && descriptorCode !=
null)', 'amounts and descriptorCode are mutually exclusive')); apply these
asserts to the VerifyMicrodepositsForSetupData
factory/_VerifyMicrodepositsForSetupData so the constraints are enforced at
construction time.
4fae05b to
339d0e6
Compare
Contributor
Author
|
I'll rebase onto main once #2395 merges to drop the duplicated fix commit. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/stripe_web/lib/src/web_stripe.dart (1)
512-558:collectBankAccountweb implementation — LGTM, with one optional cleanup.Routing on
isPaymentIntent, theus_bank_accountpayment-method-type defaulting, and wrapping the result in the correctCollectBankAccountResultvariant all look right. The two branches do, however, duplicate theresponse.error→StripeErrorconversion block; extracting a small helper would de-duplicate this and the two branches inverifyPaymentIntentWithMicrodepositsbelow.♻️ Optional refactor — share the error-to-StripeError mapping
StripeError _toStripeError(dynamic error) => StripeError( message: error?.message ?? '', code: error!.code, );Then each branch becomes:
if (response.error != null) throw _toStripeError(response.error);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_web/lib/src/web_stripe.dart` around lines 512 - 558, Extract the duplicated response.error → StripeError mapping into a small private helper (e.g., _toStripeError) and call it from both branches of collectBankAccount (replace both occurrences of "if (response.error != null) throw StripeError(...)" with "if (response.error != null) throw _toStripeError(response.error)"), and do the same refactor in verifyPaymentIntentWithMicrodeposits so both functions reuse the helper; ensure the helper returns a StripeError built from the dynamic error.message and error.code.packages/stripe_platform_interface/lib/src/stripe_platform_interface.dart (1)
160-170: Breaking API change on public platform interface — confirmed intentional.Both
collectBankAccountandverifyPaymentIntentWithMicrodepositsnow returnFuture<CollectBankAccountResult>instead ofFuture<PaymentIntent>. This is a breaking change for any third-partyStripePlatformsubclass and for direct callers ofStripe.instance.*. The example app has been updated to use pattern matching, which is good, but consumers outside this repo will need migration.Consider calling this out explicitly in the changelog / migration notes, and (optionally) renaming
verifyPaymentIntentWithMicrodepositssince it now semantically covers both payment and setup intents — though that's a second breaking change and may be better deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_platform_interface/lib/src/stripe_platform_interface.dart` around lines 160 - 170, The public platform interface changed the return types of collectBankAccount and verifyPaymentIntentWithMicrodeposits from Future<PaymentIntent> to Future<CollectBankAccountResult>, which is a breaking API change for any StripePlatform subclass and external callers; update the changelog and migration notes to explicitly call out this breaking change (mention collectBankAccount, verifyPaymentIntentWithMicrodeposits, CollectBankAccountResult, and the old PaymentIntent return type) and provide migration guidance/examples for adapting subclasses and callers (including StripePlatform implementations and usages of Stripe.instance.*), and optionally evaluate renaming verifyPaymentIntentWithMicrodeposits to a neutral name covering both payment and setup intents in a future breaking release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/stripe_js/lib/src/js/payment_intents/verify_microdeposits_for_payment.dart`:
- Around line 14-31: The Dart wrapper is merging clientSecret and data into one
JS object and the external JS declaration only accepts one argument; change
verifyMicrodepositsForPayment to call the external interop with two positional
args (pass clientSecret as the first arg and data?.toJson().jsify() or null as
the second), and update the external declaration _verifyMicrodepositsForPayment
to accept two parameters (e.g., external JSPromise<JSIntentResponse>
_verifyMicrodepositsForPayment(JSAny? clientSecret, JSAny? options)); apply the
same change to verifyMicrodepositsForSetup so Stripe.js receives (clientSecret,
data) as intended.
---
Nitpick comments:
In `@packages/stripe_platform_interface/lib/src/stripe_platform_interface.dart`:
- Around line 160-170: The public platform interface changed the return types of
collectBankAccount and verifyPaymentIntentWithMicrodeposits from
Future<PaymentIntent> to Future<CollectBankAccountResult>, which is a breaking
API change for any StripePlatform subclass and external callers; update the
changelog and migration notes to explicitly call out this breaking change
(mention collectBankAccount, verifyPaymentIntentWithMicrodeposits,
CollectBankAccountResult, and the old PaymentIntent return type) and provide
migration guidance/examples for adapting subclasses and callers (including
StripePlatform implementations and usages of Stripe.instance.*), and optionally
evaluate renaming verifyPaymentIntentWithMicrodeposits to a neutral name
covering both payment and setup intents in a future breaking release.
In `@packages/stripe_web/lib/src/web_stripe.dart`:
- Around line 512-558: Extract the duplicated response.error → StripeError
mapping into a small private helper (e.g., _toStripeError) and call it from both
branches of collectBankAccount (replace both occurrences of "if (response.error
!= null) throw StripeError(...)" with "if (response.error != null) throw
_toStripeError(response.error)"), and do the same refactor in
verifyPaymentIntentWithMicrodeposits so both functions reuse the helper; ensure
the helper returns a StripeError built from the dynamic error.message and
error.code.
🪄 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
Run ID: acf206ca-b5fd-486f-948f-da3f316ddfbe
📒 Files selected for processing (36)
example/lib/screens/regional_payment_methods/us_bank_account_direct_debit_screen.dartpackages/stripe/CHANGELOG.mdpackages/stripe/lib/src/stripe.dartpackages/stripe/pubspec.yamlpackages/stripe_js/CHANGELOG.mdpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.freezed.dartpackages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.g.dartpackages/stripe_js/lib/src/api/payment_intents/payment_intents.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.freezed.dartpackages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.g.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.freezed.dartpackages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.g.dartpackages/stripe_js/lib/src/api/setup_intents/setup_intents.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.freezed.dartpackages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.g.dartpackages/stripe_js/lib/src/js/payment_intents/collect_bank_account_for_payment.dartpackages/stripe_js/lib/src/js/payment_intents/payment_intents.dartpackages/stripe_js/lib/src/js/payment_intents/verify_microdeposits_for_payment.dartpackages/stripe_js/lib/src/js/setup_intents/collect_bank_account_for_setup.dartpackages/stripe_js/lib/src/js/setup_intents/setup_intents.dartpackages/stripe_js/lib/src/js/setup_intents/verify_microdeposits_for_setup.dartpackages/stripe_js/pubspec.yamlpackages/stripe_platform_interface/CHANGELOG.mdpackages/stripe_platform_interface/lib/src/method_channel_stripe.dartpackages/stripe_platform_interface/lib/src/models/collect_bank_account_result.dartpackages/stripe_platform_interface/lib/src/models/collect_bank_account_result.freezed.dartpackages/stripe_platform_interface/lib/src/stripe_platform_interface.dartpackages/stripe_platform_interface/lib/stripe_platform_interface.dartpackages/stripe_platform_interface/pubspec.yamlpackages/stripe_web/CHANGELOG.mdpackages/stripe_web/lib/src/web_stripe.dartpackages/stripe_web/pubspec.yaml
✅ Files skipped from review due to trivial changes (23)
- packages/stripe_platform_interface/lib/stripe_platform_interface.dart
- packages/stripe_js/pubspec.yaml
- packages/stripe_js/lib/src/api/setup_intents/setup_intents.dart
- packages/stripe_js/lib/src/js/setup_intents/setup_intents.dart
- packages/stripe_js/lib/src/api/payment_intents/payment_intents.dart
- packages/stripe_js/CHANGELOG.md
- packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.g.dart
- packages/stripe/CHANGELOG.md
- packages/stripe_web/pubspec.yaml
- packages/stripe/pubspec.yaml
- packages/stripe_web/CHANGELOG.md
- packages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.g.dart
- packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.g.dart
- packages/stripe_js/lib/src/js/payment_intents/payment_intents.dart
- packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.g.dart
- packages/stripe_platform_interface/CHANGELOG.md
- packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.dart
- packages/stripe_platform_interface/pubspec.yaml
- packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.freezed.dart
- packages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.freezed.dart
- packages/stripe/lib/src/stripe.dart
- packages/stripe_js/lib/src/api/setup_intents/collect_bank_account_for_setup_params.dart
- packages/stripe_js/lib/src/api/setup_intents/verify_microdeposits_for_setup_data.freezed.dart
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/stripe_platform_interface/lib/src/models/collect_bank_account_result.dart
- example/lib/screens/regional_payment_methods/us_bank_account_direct_debit_screen.dart
- packages/stripe_js/lib/src/js/setup_intents/verify_microdeposits_for_setup.dart
- packages/stripe_js/lib/src/api/payment_intents/collect_bank_account_for_payment_params.dart
- packages/stripe_js/lib/src/api/payment_intents/verify_microdeposits_for_payment_data.freezed.dart
- packages/stripe_platform_interface/lib/src/method_channel_stripe.dart
BiAksoy
force-pushed
the
feat/web-us-bank-account
branch
from
339d0e6 to
40ad8c1
Compare
Contributor
Author
|
Rebased onto the updated #2395. |
…n and microdeposit verification Add JS interop extensions and freezed data classes for four Stripe.js v3 methods that were previously missing from the stripe_js package: - collectBankAccountForPayment / collectBankAccountForSetup - verifyMicrodepositsForPayment / verifyMicrodepositsForSetup These methods enable the Financial Connections authentication flow for US bank account (ACH) payments and setup intents on the web platform. Each method takes a single options object containing the clientSecret and params/data, matching the Stripe.js API contract.
Replace the UnimplementedError stubs in WebStripe with real implementations backed by the stripe_js bindings. Both collectBankAccount and verifyPaymentIntentWithMicrodeposits dispatch on isPaymentIntent and delegate to the corresponding Stripe.js call, returning a CollectBankAccountResult that wraps either the PaymentIntent or SetupIntent parsed from the JS response.
…fyMicrodeposits
Stripe.js exposes verifyMicrodepositsForPayment and verifyMicrodepositsForSetup
as (clientSecret, data) — two positional arguments — not as a single options
object. The previous bindings merged the two into {clientSecret, ...data}.jsify(),
which produced a call shaped like verifyMicrodeposits({clientSecret, amounts, ...})
and does not match the Stripe.js signature.
Update both external interop declarations to take (String clientSecret, [JSAny?
data]) and the Dart wrappers to call them with two arguments, matching the same
pattern used by confirmCardPayment / confirmP24Payment / etc.
BiAksoy
force-pushed
the
feat/web-us-bank-account
branch
from
e78b479 to
176e5b0
Compare
Fixes the CI build check that runs `dart format --set-exit-if-changed .`.
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
1 participant
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.
Summary
collectBankAccountForPayment,collectBankAccountForSetup,verifyMicrodepositsForPayment, andverifyMicrodepositsForSetupin thestripe_jspackage.collectBankAccountandverifyPaymentIntentWithMicrodepositsinWebStripe, replacing the previousUnimplementedErrorstubs.CollectBankAccountResultintroduced in fix!: return sealed CollectBankAccountResult from collectBankAccount / verifyMicrodeposits #2395 —response.paymentIntent!.parse()orresponse.setupIntent!.parse()wrapped in the matching variant. No hand-written field mapping.collectBankAccountTokenandcollectFinancialConnectionsAccountsremain unsupported on web (they require the standalone Financial Connections JS SDK).Depends on
Affected packages
stripe_jsstripe_webSummary by CodeRabbit