Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/components/StatefulButton/StatefulSubscribeButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
import { useContext, useEffect, useState } from 'react';
import { useNavigate } from 'react-router-dom';

import { useCheckoutIntent } from '@/components/app/data';
import { determineExistingSuccessfulCheckoutIntent } from '@/components/app/data/services/context';

Check failure on line 11 in src/components/StatefulButton/StatefulSubscribeButton.tsx

View workflow job for this annotation

GitHub Actions / lint

`@/components/app/data/services/context` import should occur after import of `@/components/app/data/services/checkout-intent`
import { useCheckoutIntent, usePolledCheckoutIntent } from '@/components/app/data';
import { termsAndConditions } from '@/components/app/data/constants';
import { queryBffContext } from '@/components/app/data/queries/queries';

Check failure on line 14 in src/components/StatefulButton/StatefulSubscribeButton.tsx

View workflow job for this annotation

GitHub Actions / lint

'queryBffContext' is defined but never used

Check failure on line 14 in src/components/StatefulButton/StatefulSubscribeButton.tsx

View workflow job for this annotation

GitHub Actions / lint

'queryBffContext' is defined but never used
import { patchCheckoutIntent } from '@/components/app/data/services/checkout-intent';
import { CheckoutPageRoute, CheckoutSubstepKey, DataStoreKey } from '@/constants/checkout';
import EVENT_NAMES from '@/constants/events';
Expand Down Expand Up @@ -59,7 +60,7 @@
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);
Expand Down Expand Up @@ -104,7 +105,6 @@
}
// 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.
Copy link

Copilot AI Oct 9, 2025

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.

Suggested change
// Stripe responses map 1:1 to button states except for 'default' which is the initial state.
// Stripe responses map 1:1 to button states except for 'default' which is the initial state.
setStatefulButtonState(response.type || 'default');

Copilot uses AI. Check for mistakes.
setStatefulButtonState(response.type || 'default');
if (response.type === 'error') {
setErrorMessageKey(buttonMessages.error[response.error?.code] ? response.error?.code : 'fallback');
logError(
Expand All @@ -113,18 +113,18 @@
}
};

// Visually alter the Subscribe button to a "successful" appearance if the polled intent state becomes successful.
useEffect(() => {
if (statefulButtonState === 'pending' && determineExistingSuccessfulCheckoutIntent(polledCheckoutIntent?.state)) {
setStatefulButtonState('success');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Collaborator Author

@brobro10000 brobro10000 Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please observe of how the Stripe API responds versus the status.
This additional change requested creates an additional delay to wait for the checkout intent to return a "paid", "errored_provisioning" or "fulfilled" state. The initial implementation was always waiting for a successful response from the stripe API indicating that the status was updated to success and therefore trigger a useEffect to update with the now latest status information AFTER a successful payment from Stripe.
This change just waits for the checkout intent to now reflect a paid state before setting a successful button state to complete the checkout experience.

Image

Should any doubts remain, I encourage you to check out the branch and validate.

Copy link
Copy Markdown
Member

@pwnage101 pwnage101 Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please observe of how the Stripe API responds versus the status.

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.

This additional change requested creates an additional delay to wait for the checkout intent to return a "paid", "errored_provisioning" or "fulfilled" state.

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.

The initial implementation was always waiting for a successful response from the stripe API indicating that the status was updated to success and therefore trigger a useEffect to update with the now latest status information AFTER a successful payment from Stripe. This change just waits for the checkout intent to now reflect a paid state before setting a successful button state to complete the checkout experience.

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.

event button appearance after event Stripe Checkout Session Status Enterprise Checkout Intent State
enter credit card info Subscribe ??? created
click subscribe Processing ??? created
stripe API: session completed Processing success/unpaid created
stripe API: paid Success complete/paid created
navigate to success page N/A complete/paid created (Bad!)

Below is what this PR currently does. Note that the appearance of the button changes possibly BEFORE stripe API payment completion.

event button appearance after event Stripe Checkout Session Status Enterprise Checkout Intent State
enter credit card info Subscribe ??? created
click subscribe Processing ??? created
enterprise-access API: paid Success (Bad!) ??? paid
stripe API: session completed Success success/unpaid paid
stripe API: paid Success complete/paid paid
navigate to success page N/A complete/paid paid

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.

event button appearance after event Stripe Checkout Session Status Enterprise Checkout Intent State
enter credit card info Subscribe ??? created
click subscribe Processing ??? created
stripe API: session completed Processing success/unpaid created
stripe API: paid Processing complete/paid created
enterprise-access API: paid Success complete/paid paid
navigate to success page N/A complete/paid paid

}
}, [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.
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment should use 'Stripe' (capitalized) instead of 'stripe' as it's a proper noun referring to the Stripe API.

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
if (status.type === 'complete' && status.paymentStatus === 'paid') {
setCheckoutSessionStatus(status);
queryClient.invalidateQueries({
queryKey: queryBffContext(
authenticatedUser.id,
).queryKey,
})
.then(data => data)
.catch(error => logError(error));
sendEnterpriseCheckoutTrackingEvent({
checkoutIntentId: checkoutIntent?.id ?? null,
eventName: EVENT_NAMES.SUBSCRIPTION_CHECKOUT.PAYMENT_PROCESSED_SUCCESSFULLY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ jest.mock('@/components/app/data', () => ({
data:
{ id: 'test-intent', country: 'US', state: 'created' },
})),
usePolledCheckoutIntent: jest.fn(() => ({ data: { state: 'paid' } })),
}));

jest.mock('@/hooks/useCheckoutFormStore', () => ({
Expand Down Expand Up @@ -342,7 +343,6 @@ describe('StatefulSubscribeButton', () => {
// Wait for the useEffect to trigger after state changes to 'success'
await waitFor(() => {
expect(mockSetCheckoutSessionStatus).toHaveBeenCalledWith({ type: 'complete', paymentStatus: 'paid' });
expect(mockInvalidateQueries).toHaveBeenCalled();
expect(jest.mocked(sendEnterpriseCheckoutTrackingEvent)).toHaveBeenCalledWith({
checkoutIntentId: 'test-intent',
eventName: EVENT_NAMES.SUBSCRIPTION_CHECKOUT.PAYMENT_PROCESSED_SUCCESSFULLY,
Expand Down
1 change: 1 addition & 0 deletions src/components/app/data/services/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,6 @@ const fetchCheckoutSuccess = async (): Promise<CheckoutContextResponse> => {
export {
fetchCheckoutContext,
fetchCheckoutSuccess,
determineExistingSuccessfulCheckoutIntent,
paymentProcessedCheckoutIntentStates,
};
27 changes: 23 additions & 4 deletions src/components/app/routes/loaders/checkoutStepperLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 { getCheckoutSessionClientSecret, validateFormState } from '@/components/app/routes/loaders/utils';
import { CheckoutPageRoute } from '@/constants/checkout';
import { checkoutFormStore } from '@/hooks/useCheckoutFormStore';
Expand Down Expand Up @@ -144,12 +144,31 @@ async function billingDetailsSuccessLoader(queryClient: QueryClient): Promise<Re
const contextMetadata: CheckoutContextResponse = await queryClient.ensureQueryData(
queryBffContext(authenticatedUser?.userId || null),
);

const { checkoutIntent } = contextMetadata;

const checkoutIntentType = checkoutFormStore.getState().checkoutSessionStatus?.type;
if (!checkoutIntent) {
return redirect(CheckoutPageRoute.PlanDetails);
}

await queryClient.ensureQueryData(
queryBffSuccess(authenticatedUser?.userId || null),
);
if (checkoutIntent.id) {
await queryClient.ensureQueryData(
queryCheckoutIntent(checkoutIntent.id),
);
}

const stripeCheckoutSessionType = checkoutFormStore.getState().checkoutSessionStatus?.type;

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 (
!checkoutIntent.existingSuccessfulCheckoutIntent
|| stripeCheckoutSessionType !== 'complete'
) {
return redirect(CheckoutPageRoute.PlanDetails);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ describe('BillingDetailsPage', () => {
},
});
(useCheckoutSessionClientSecret as jest.Mock).mockReturnValue('secret-123');
(usePolledCheckoutIntent as jest.Mock).mockReturnValue({
data: {
id: 1,
state: 'pending',
},
});
(useCheckout as jest.Mock).mockReturnValue({
canConfirm: true,
confirm: jest.fn().mockResolvedValue({ type: 'success' }),
Expand Down
Loading