-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Cut over to Stripe PaymentElement #3665
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #3665 +/- ##
==========================================
- Coverage 98.91% 98.80% -0.12%
==========================================
Files 817 820 +3
Lines 14720 14783 +63
Branches 4165 4203 +38
==========================================
+ Hits 14561 14607 +46
- Misses 152 167 +15
- Partials 7 9 +2
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #3665 +/- ##
==========================================
- Coverage 98.91% 98.80% -0.12%
==========================================
Files 817 820 +3
Lines 14720 14783 +63
Branches 4165 4203 +38
==========================================
+ Hits 14561 14607 +46
- Misses 152 167 +15
- Partials 7 9 +2
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #3665 +/- ##
==========================================
- Coverage 98.91% 98.80% -0.12%
==========================================
Files 817 820 +3
Lines 14720 14783 +63
Branches 4165 4211 +46
==========================================
+ Hits 14561 14607 +46
- Misses 152 167 +15
- Partials 7 9 +2
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
Bundle ReportChanges will increase total bundle size by 8.82kB (0.04%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #3665 +/- ##
==========================================
- Coverage 98.91% 98.80% -0.12%
==========================================
Files 817 820 +3
Lines 14720 14784 +64
Branches 4173 4206 +33
==========================================
+ Hits 14561 14608 +47
- Misses 152 167 +15
- Partials 7 9 +2
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
455f129
to
91a732a
Compare
91a732a
to
ea0b00d
Compare
31557fe
to
0d33704
Compare
interface BankInformationProps { | ||
subscriptionDetail: z.infer<typeof SubscriptionDetailSchema> | ||
} | ||
function BankInformation({ subscriptionDetail }: BankInformationProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can you get away with just passing bankAccount into this component?
provider, | ||
owner, | ||
name: | ||
subscriptionDetail?.defaultPaymentMethod?.billingDetails?.name || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is it possible to pass just billingDetails to this component or set it to a const before this hook?
@@ -213,3 +213,20 @@ export function useAccountDetails({ | |||
...opts, | |||
}) | |||
} | |||
|
|||
export const stripeAddress = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on keeping this next to where it's being used in PaymentMethodForm?
@@ -62,7 +67,9 @@ export function useUpdatePaymentMethod({ | |||
payment_method_data: { | |||
// eslint-disable-next-line camelcase | |||
billing_details: { | |||
name: name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think we can tidy this up to { name, email, address }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple small comments but nothing blocking
This PR adds ability to choose ACH as a payment method.
mode: setup, currency: usd
as required per here, here if using the PaymentElement. Otherwise stripe console errors that mode or client secret missingCloses codecov/engineering-team#3108