feat: Add ACH payment method #3616
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: src/services/navigation/useNavLinks/useNavLinks.ts
Did you find this useful? React with a 👍 or 👎 |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3616 +/- ##
==========================================
- Coverage 98.94% 98.80% -0.14%
==========================================
Files 815 820 +5
Lines 14676 14752 +76
Branches 4156 4202 +46
==========================================
+ Hits 14521 14576 +55
- Misses 148 170 +22
+ Partials 7 6 -1
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3616 +/- ##
==========================================
- Coverage 98.94% 98.80% -0.14%
==========================================
Files 815 820 +5
Lines 14676 14752 +76
Branches 4164 4202 +38
==========================================
+ Hits 14521 14576 +55
- Misses 148 170 +22
+ Partials 7 6 -1
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3616 +/- ##
==========================================
- Coverage 98.94% 98.80% -0.14%
==========================================
Files 815 820 +5
Lines 14676 14752 +76
Branches 4164 4210 +46
==========================================
+ Hits 14521 14576 +55
- Misses 148 170 +22
+ Partials 7 6 -1
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3616 +/- ##
==========================================
- Coverage 98.94% 98.80% -0.14%
==========================================
Files 815 820 +5
Lines 14676 14752 +76
Branches 4164 4202 +38
==========================================
+ Hits 14521 14576 +55
- Misses 148 170 +22
+ Partials 7 6 -1
Continue to review full report in Codecov by Sentry.
|
suejung-sentry
force-pushed
the
sshin/3108
branch
from
6142180 to
0340859
Compare
suejung-sentry
force-pushed
the
sshin/3108
branch
from
0340859 to
0f6d1c3
Compare
|
@codecov-ai-reviewer review |
suejung-sentry
force-pushed
the
sshin/3108
branch
3 times, most recently
from
5e48a4e to
f8c1578
Compare
suejung-sentry
force-pushed
the
sshin/3108
branch
from
f8c1578 to
8912835
Compare
src/pages/PlanPage/PlanPage.jsx
Outdated
| @@ -45,7 +50,7 @@ function PlanPage() { | |||
| return ( | |||
| <div className="flex flex-col gap-4"> | |||
| <Tabs /> | |||
| <Elements stripe={stripePromise}> | |||
| <Elements stripe={stripePromise} options={StripeAppearance(isDarkMode)}> | |||
There was a problem hiding this comment.
the Theme was getting duped and passed in both the AddressForm and CreditCardForm before, so I consolidated them in file stripe.ts
| import { ViewPaymentMethod } from './ViewPaymentMethod' | ||
|
|
||
| // Remove this when we build Secondary Payment Method feature | ||
| export const SECONDARY_PAYMENT_FEATURE_ENABLED = false |
There was a problem hiding this comment.
This flag can get removed when we build out the rest of the secondary payment method feature (next quarter)
suejung-sentry
force-pushed
the
sshin/3108
branch
2 times, most recently
from
169de52 to
d64d415
Compare
Bundle ReportChanges will increase total bundle size by 6.08MB (100.3%) ⬆️
|
Bundle ReportChanges will decrease total bundle size by 12.18MB (-50.1%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
suejung-sentry
force-pushed
the
sshin/3108
branch
from
d64d415 to
1489237
Compare
suejung-sentry
force-pushed
the
sshin/3108
branch
from
1489237 to
2ade2fc
Compare
suejung-sentry
force-pushed
the
sshin/3108
branch
from
2ade2fc to
eaaf86d
Compare
|
|
||
| interface EditPaymentMethodsProps { | ||
| setEditMode: (isEditMode: boolean) => void | ||
| provider: string |
There was a problem hiding this comment.
nit: This can be typed as Provider now
| owner, | ||
| subscriptionDetail, | ||
| }: EditPaymentMethodsProps) => { | ||
| const [activeTab, setActiveTab] = useState<'primary' | 'secondary'>('primary') |
There was a problem hiding this comment.
might be able to clean this up with a specific type. Like type PaymentMethod = 'primary' | 'secondary', which you could then use on line 45 as well
|
|
||
| interface PaymentMethodFormProps { | ||
| closeForm: () => void | ||
| provider: string |
There was a problem hiding this comment.
Same comment here with Provider
| <Elements | ||
| stripe={stripePromise} | ||
| options={{ | ||
| ...StripeAppearance(isDarkMode), |
There was a problem hiding this comment.
I hadn't considered adding the appearance on the top level here but I really like this
Do we need the 'mode' and 'currency' defined?
There was a problem hiding this comment.
Yeah those are parameters required in order to use this Stripe Elements provider along with the setupIntent for the PaymentElement UI. There are complaints in the console if they don't get passed unfortunately
| interface AddressProps { | ||
| setEditMode: (isEditMode: boolean) => void | ||
| subscriptionDetail: z.infer<typeof SubscriptionDetailSchema> | ||
| provider: string |
There was a problem hiding this comment.
Same comment here with Provider
| {isAddressSameAsPrimary ? ( | ||
| <p>Same as primary address</p> | ||
| ) : isEmptyAddress ? ( | ||
| <p>-</p> |
There was a problem hiding this comment.
Maybe for the empty address case we could use the stuff on line 33 (like move that condition to line 33 and show that component)
| @@ -20,6 +20,7 @@ export function useUpdateBillingEmail({ provider, owner }: UsePlanDataArgs) { | |||
| const body = { | |||
| /* eslint-disable camelcase */ | |||
| new_email: formData?.newEmail, | |||
| should_propagate_to_payment_methods: true, | |||
There was a problem hiding this comment.
reminder to update this param to the new name
| provider, | ||
| owner, | ||
| }: { | ||
| provider: string |
There was a problem hiding this comment.
This file has 2 actually
| @@ -29,7 +29,9 @@ interface ThemeContextProviderProps { | |||
| export const ThemeContextProvider: FC<ThemeContextProviderProps> = ({ | |||
| children, | |||
| }) => { | |||
| const prefersDark = window.matchMedia('(prefers-color-scheme: dark)').matches | |||
| const prefersDark = | |||
| typeof window !== 'undefined' && | |||
There was a problem hiding this comment.
Was this throwing an error somewhere? Jw
There was a problem hiding this comment.
Yeah it was in one of the tests where window is not defined. Should not error in the real world, but adding this check in doesn't hurt
| @@ -134,7 +134,8 @@ function Button({ | |||
| const className = cs( | |||
| baseClass, | |||
| { [baseDisabledClasses]: !isLoading }, | |||
| pickVariant(variant, isLoading) | |||
| pickVariant(variant, isLoading), | |||
| props.className | |||
There was a problem hiding this comment.
what was this one for? :O
There was a problem hiding this comment.
I had needed to add a flex-none for formatting on one of the buttons
This PR adds ability to choose ACH as a payment method. It also sets sockets for a secondary payment method (to be fully built out next quarter).
clientSecretfromapiwhich establishes a StripesetupIntentOther notes:
AddressElementandPaymentElementElementsprovider inmode: setup, currency: usdas required per here, here if using thePaymentElement. Otherwise stripe console errors that mode or client secret missingfrom:
to:
Figma: https://www.figma.com/design/ftdTGGX43YyurDlxPCLTFy/GH-2621?node-id=1-2&p=f&t=lQMWI0ok827VpgVr-0
Closes codecov/engineering-team#3108