-
Notifications
You must be signed in to change notification settings - Fork 6
fix: poll checkout intent API within billing details #75
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
7073fa0
fd7807f
3addeac
bac1678
f2c8c32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,8 +8,8 @@ import { useQueryClient } from '@tanstack/react-query'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useContext, useEffect, useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useNavigate } from 'react-router-dom'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCheckoutIntent } from '@/components/app/data'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { queryBffContext } from '@/components/app/data/queries/queries'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCheckoutIntent, usePolledCheckoutIntent } from '@/components/app/data'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { determineExistingSuccessfulCheckoutIntent } from '@/components/app/data/services/context'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { CheckoutPageRoute, CheckoutSubstepKey, DataStoreKey } from '@/constants/checkout'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import EVENT_NAMES from '@/constants/events'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCheckoutFormStore } from '@/hooks/useCheckoutFormStore'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -57,7 +57,7 @@ const StatefulSubscribeButton = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [errorMessageKey, setErrorMessageKey] = useState('fallback'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { data: checkoutIntent } = useCheckoutIntent(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const intl = useIntl(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { data: polledCheckoutIntent } = usePolledCheckoutIntent(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const queryClient = useQueryClient(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const navigate = useNavigate(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { authenticatedUser }: AppContextValue = useContext(AppContext); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -92,7 +92,6 @@ const StatefulSubscribeButton = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Set the button to the appropriate state based on the response. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Stripe responses map 1:1 to button states except for 'default' which is the initial state. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setStatefulButtonState(response.type || 'default'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (response.type === 'error') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setErrorMessageKey(buttonMessages.error[response.error?.code] ? response.error?.code : 'fallback'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -101,18 +100,18 @@ const StatefulSubscribeButton = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Refetch the checkout intent if the polled intent state changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
brobro10000 marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (statefulButtonState === 'pending' && determineExistingSuccessfulCheckoutIntent(polledCheckoutIntent?.state)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setStatefulButtonState('success'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise, we can't just set the button appearance to look successful here because we aren't yet sure if the stripe checkout session status has type=complete and paymentStatus=paid yet, as it would be misleading to the admin that everything completed successfully. This is another reason to combine the two useEffects.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please observe of how the Stripe API responds versus the status.
Should any doubts remain, I encourage you to check out the branch and validate.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, the screenshot shows what I expect---initial status from the Stripe API does not yet show paid, then eventually becomes paid after a few moments.
Yes, this is the goal. We need to wait potentially longer for not just Stripe APIs to show paid, but for polled checkout-intent APIs to also show paid.
This PR introduces drift between actual/backoffice success and visual success. Let me demonstrate with event tables: Here's what we had before this PR. Note the possibility of navigating to the success page BEFORE confirming that the checkout intent has transitioned to paid.
Below is what this PR currently does. Note that the appearance of the button changes possibly BEFORE stripe API payment completion.
Here's what I think we need. Note that the button appearance only changes to "Success" once both stripe and enterprise APIs indicate payment success. Since they both can happen in any order, my proposal has been to combine them into one single useEffect to handle the joint logic of checking both before performing button state changes and navigate() calls.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [polledCheckoutIntent?.state, navigate, statefulButtonState]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (statefulButtonState === 'success') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If the payment succeeded, update the checkout session status. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If the payment succeeded from the stripe API, update the checkout session status. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If the payment succeeded from the stripe API, update the checkout session status. | |
| // If the payment succeeded from the Stripe API, update the checkout session status. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,8 @@ import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; | |||||||||||
| import { QueryClient } from '@tanstack/react-query'; | ||||||||||||
| import { redirect } from 'react-router-dom'; | ||||||||||||
|
|
||||||||||||
| import { queryBffContext } from '@/components/app/data/queries/queries'; | ||||||||||||
| import { queryBffContext, queryBffSuccess, queryCheckoutIntent } from '@/components/app/data/queries/queries'; | ||||||||||||
| import { determineExistingSuccessfulCheckoutIntent } from '@/components/app/data/services/context'; | ||||||||||||
| import { getCheckoutSessionClientSecret, validateFormState } from '@/components/app/routes/loaders/utils'; | ||||||||||||
| import { CheckoutPageRoute } from '@/constants/checkout'; | ||||||||||||
| import { checkoutFormStore } from '@/hooks/useCheckoutFormStore'; | ||||||||||||
|
|
@@ -144,12 +145,32 @@ async function billingDetailsSuccessLoader(queryClient: QueryClient): Promise<Re | |||||||||||
| const contextMetadata: CheckoutContextResponse = await queryClient.ensureQueryData( | ||||||||||||
| queryBffContext(authenticatedUser?.userId || null), | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| const { checkoutIntent } = contextMetadata; | ||||||||||||
|
|
||||||||||||
| if (!checkoutIntent) { | ||||||||||||
| return redirect(CheckoutPageRoute.PlanDetails); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| await queryClient.ensureQueryData( | ||||||||||||
| queryBffSuccess(authenticatedUser?.userId || null), | ||||||||||||
| ); | ||||||||||||
| if (checkoutIntent.id) { | ||||||||||||
| await queryClient.ensureQueryData( | ||||||||||||
| queryCheckoutIntent(checkoutIntent.id), | ||||||||||||
| ); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const checkoutIntentType = checkoutFormStore.getState().checkoutSessionStatus?.type; | ||||||||||||
|
brobro10000 marked this conversation as resolved.
Outdated
|
||||||||||||
|
|
||||||||||||
| if (checkoutIntentType !== 'complete' && !checkoutIntent?.existingSuccessfulCheckoutIntent) { | ||||||||||||
| // Explicitly check that the intent is in a successful state | ||||||||||||
| // If the intent is successful but the type is not 'complete', | ||||||||||||
| // or if there is no existingSuccessfulCheckoutIntent flag, | ||||||||||||
| // redirect to Plan Details to restart the process. | ||||||||||||
| if ( | ||||||||||||
| !determineExistingSuccessfulCheckoutIntent(checkoutIntent.state) | ||||||||||||
| && checkoutIntentType !== 'complete' | ||||||||||||
| && !checkoutIntent.existingSuccessfulCheckoutIntent | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ Logical expression should be chained using
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was, so we can probably stick to
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can also remain as an && since we map the checkout intent state to the stripe response in the Basically this can be reverted to the original implementation. |
||||||||||||
| ) { | ||||||||||||
| return redirect(CheckoutPageRoute.PlanDetails); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||

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.
The line
setStatefulButtonState(response.type || 'default');was removed but the button state is no longer being set for non-error responses. This could leave the button in an incorrect state for successful or other response types.