-
Notifications
You must be signed in to change notification settings - Fork 4
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
[WEB-2010] Split the checkout flow into two parts #305
base: fon/new-sdk-views
Are you sure you want to change the base?
[WEB-2010] Split the checkout flow into two parts #305
Conversation
f2696f6
to
37e81be
Compare
@@ -19,8 +19,6 @@ const config: StorybookConfig = { | |||
...config, | |||
VITE_STORYBOOK_PUBLISHABLE_API_KEY: | |||
process.env.VITE_STORYBOOK_PUBLISHABLE_API_KEY || "", | |||
VITE_STORYBOOK_RESTRICTED_SECRET: | |||
process.env.VITE_STORYBOOK_RESTRICTED_SECRET || "", |
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.
This env var is no longer needed. With the new flow, we don't need a payment_intent or setup_intent to render the payment form 😄
@@ -92,7 +92,6 @@ | |||
"storybook": "^8.5.0", | |||
"svelte": "^5.12.0", | |||
"svelte-check": "^4.1.1", | |||
"svelte-stripe": "^1.3.0", |
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.
I removed this third-party library and initialized the stripe elements as recommended by Stripe.
37e81be
to
42589e7
Compare
if (result.next_action === "completed") { | ||
lastError = null; | ||
currentView = "success"; | ||
return; | ||
} |
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.
This was unreachable code, since next_action
is always set to collect_payment_info
by the backend
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.
Question:
Did we break this unreachable code for some previous version by always returning "collect_payment_info"?
When was completed
being returned?
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.
That's correct Nicola, the old endpoints (/subscribe
and /purchase
endpoints) are kept untouched and will return the field collect_payment_info
.
I tested all of this in a canary using the published version of the SDK before shipping the BE changes.
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.
My question was related to the completed
value.
When was that value returned?
In what scenario?
I understand that "recently" we updated to always return collect_payment_info
my question is:
Did we break older version of the sdk that supported completed
when we stopped sending it?
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.
hmm, I’m not following. completed is not returned by the backend, even before any deferred flow changes were shipped. I don’t think that status was ever used, IIRC
This PR should not break the existing flow. I tested the live WPL version before shipping the backend changes, and no issues were detected.
paymentMethodTypes: ["card"], | ||
setup_future_usage: "off_session", | ||
amount: productDetails.currentPrice.amountMicros / 10000, | ||
currency: productDetails.currentPrice.currency.toLowerCase(), |
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.
Since these parameters should match the settings from the PaymentIntent/SetupIntent, I was considering including them in the CheckoutStart
response. Wdyt @RevenueCat/web?
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.
Makes sense, the risk is that the productDetails.currentPrice.amountMicros
might be different from the one show at the time of downloading the product.
However it is worse if we show one price and then charge another one. So yes, returning the latest up to date price for the product from the backend it's a good idea.
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.
I changed this to retrieve these values from the start purchase response so that we have more control over them from the backend
f9ecb2e
to
f5fc27c
Compare
ff2d793
to
d1a61b3
Compare
…and `CheckoutCompleteEndpoint`
d1a61b3
to
ae53ece
Compare
d9121c4
to
b6cae2f
Compare
ac94730
to
1136121
Compare
1136121
to
71c033e
Compare
Motivation / Description
This PR changes the checkout flow by using the two new endpoints:
/checkout/start
: Called right after entering the billing email. Returns the operation ID and the necessary configuration to initialize the Stripe Elements component./checkout/complete
: Called after submitting the payment method form, it creates the corresponding intent (SetupIntent
orPayentIntent
) and returns the client secret.Note: All the necessary backend changes have been already shipped
Changes introduced
b6cae2f - chore: Improve error handling during payment info collection
8b3a7e9 - fix: Set font family in Stripe Elements
ae53ece - chore: rename
data
field togateway_params
d3a4daf - chore: Remove unnecessary fields from
CheckoutCompleteResponse
7917239 - feat: Pick
elements_configuration
from the checkout start response6204845 - chore: Remove environment var
VITE_STORYBOOK_RESTRICTED_SECRET
429630d - chore: Fix purchase flow stories
32fe846 - chore: fix unit tests
48ea8b6 - feat: Replace the
PurchaseEndpoint
withCheckoutStartEndpoint
andCheckoutCompleteEndpoint
209c078 - chore: Remove
svelte-stripe
dependencyLinear ticket (if any)
Additional comments