Fix: Add missing payment method recovery dialog#305
Fix: Add missing payment method recovery dialog#305sarimrmalik wants to merge 8 commits intomainfrom
Conversation
- Introduces a new `PaymentMethodsSession` interface to handle payment method sessions. - Updates the `BillingRepository` to include a method for creating payment methods sessions. - Implements a new `MissingPaymentMethodDialog` component to prompt users for payment method input when their team is blocked due to missing payment methods. - Enhances the `TeamBlockageAlert` to open the payment method dialog when appropriate. - Adds utility functions to check for missing payment method blockage reasons. This update improves the user experience by allowing teams to add payment methods directly from the dashboard when blocked.
- Introduces a constant for the loading message in the MissingPaymentMethodDialog component to improve consistency and maintainability. - Updates the LoadingState component to use the new constant for loading payment method messages, enhancing clarity in the user interface.
- Removes the standalone `parsePaymentMethodsSession` function and integrates its logic directly into the `getCustomerSession` method. - Updates the API endpoint for fetching the customer session to align with the new structure. - Enhances error handling for invalid payment methods session responses, ensuring consistent error reporting.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f33b1b3ba6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const teams = await queryClient.fetchQuery({ | ||
| ...teamListQueryOptions, | ||
| staleTime: 0, | ||
| }) |
There was a problem hiding this comment.
If queryClient.fetchQuery rejects during polling (e.g., transient network/auth error after confirmSetup succeeds), pollUntilTeamUnblocked() throws and handleSubmit exits before clearing isSaving/isCheckingTeamStatus. In that state the dialog remains stuck in a loading/disabled state and the user cannot continue without a full reload. Wrap the poll fetch in error handling (or catch around pollUntilTeamUnblocked) and surface a recoverable error path.
Useful? React with 👍 / 👎.
- Introduces a new `capitalize` utility function for consistent string formatting. - Updates `TeamBlockageAlert` to use the new `capitalize` function for displaying blocked reasons, improving readability. - Replaces the deprecated `isMissingPaymentMethodBlockReason` function with a new implementation that utilizes the `capitalize` function. - Removes the unused `team-blockage.ts` file, streamlining the codebase.
…syntax - Changes function declarations to arrow function expressions for `MissingPaymentMethodDialog`, `MissingPaymentMethodDialogContent`, `PaymentMethodsSessionError`, `LoadingState`, `PaymentMethodsSetupElements`, and `PaymentMethodsSetupForm`. - This update enhances consistency in the codebase and aligns with modern React practices.
…remove unused components - Eliminates the `PaymentMethodsSessionError` component and associated error handling logic, simplifying the dialog's structure. - Updates the `PaymentMethodsSetupForm` to remove unnecessary state variables and error messages, enhancing clarity and maintainability. - Integrates toast notifications for error handling, improving user feedback during payment method interactions. - Adjusts the loading state management to ensure a smoother user experience when loading payment elements.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const { error } = await stripe.confirmSetup({ | ||
| elements, | ||
| redirect: 'if_required', | ||
| confirmParams: { | ||
| return_url: window.location.href, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🟡 stripe.confirmSetup is invoked with redirect: 'if_required' and return_url: window.location.href, but the dialog has no mount-time logic to detect the ?setup_intent_client_secret=...&redirect_status=... query params Stripe appends when a payment method requires a full-page redirect (SEPA, iDEAL, Bancontact, some bank-driven 3DS). On return, the dialog re-mounts and immediately calls createPaymentMethodsSession again, showing a fresh empty form with no success/failure indication — the user may unknowingly enter a card a second time. Fix by reading setup_intent_client_secret from the URL on mount and calling stripe.retrieveSetupIntent to surface the prior result before starting a new session.
Extended reasoning...
What
`MissingPaymentMethodDialogContent` calls `stripe.confirmSetup` with:
const { error } = await stripe.confirmSetup({
elements,
redirect: "if_required",
confirmParams: { return_url: window.location.href },
})Setting return_url and using redirect: "if_required" is a deliberate signal that the developer expects some flows to navigate away. Per the Stripe docs, when a payment method requires a full-page redirect (SEPA Debit setup, iDEAL, Bancontact, Sofort, certain issuer-mandated 3DS), the browser navigates to the bank/auth page and Stripe redirects back to return_url with ?setup_intent=...&setup_intent_client_secret=...&redirect_status=... appended. The expected pattern is to read these params on mount, call stripe.retrieveSetupIntent(client_secret), and surface success/failure.
Why existing code does not prevent it
Grepping the codebase confirms there is no reference to setup_intent, retrieveSetupIntent, or redirect_status anywhere on the client. On return:
- Dialog re-mounts (the URL still contains the missing-payment-method block reason until the backend syncs).
useEffectfirespaymentMethodsSessionMutation.mutate(...), creating a brand new SetupIntent server-side.PaymentElementkeys off the newsetup_intent_client_secretand renders an empty form.- User sees no toast/banner indicating their first attempt may have already succeeded.
Step-by-step proof
- Team is blocked.
blocked-banner.tsxauto-opens the dialog. - User selects, e.g., SEPA Debit (or a card whose issuer mandates redirect 3DS) and submits.
confirmSetupredirects the browser to the issuer/bank page.- Issuer authenticates and redirects to
window.location.href(the dashboard URL) with?setup_intent=seti_..&setup_intent_client_secret=seti_.._secret_..&redirect_status=succeeded. - Backend webhook has not yet flipped
isBlocked(eventual consistency), soblocked-banner.tsxre-opens the dialog on mount. MissingPaymentMethodDialogContentmounts, ignores the URL params, callscreatePaymentMethodsSession, and renders a fresh PaymentElement against a brand-new SetupIntent.- User has no idea their previous attempt succeeded; they re-enter their card, creating a duplicate setup.
Impact
Real but bounded:
- Card-only flows in many regions use iframe-based 3DS, so the typical user is unaffected.
- Even if the user duplicates, the backend self-heals via webhook + the existing
pollUntilTeamUnblockedloop, so the dashboard eventually unblocks regardless of which session created the SetupIntent. - The user-visible failure mode is missing success feedback and a duplicated card entry — a UX regression rather than a hard break.
The new .CheckboxInput / .CheckboxLabel appearance rules added in this PR (src/features/dashboard/billing/hooks.ts) suggest non-card methods (which use a "save for future use" checkbox) may show up here, raising the chance of redirect flows.
Addressing the refutation
The refuting verifier is right that this only manifests for redirect-based PMs, that the system self-heals server-side, and that the existing concurrent-sandboxes-addon-dialog uses a similar pattern. Those points justify treating this as a nit rather than a blocker — the dialog will not get stuck in a broken state. It does not, however, change that the PR explicitly opts into the redirect path (return_url: window.location.href) without implementing the documented Stripe return-handling pattern, leaving a user-visible gap whenever a redirect actually occurs.
Suggested fix
On mount, before calling createPaymentMethodsSession, read setup_intent_client_secret from window.location.search. If present, call stripe.retrieveSetupIntent(client_secret), switch on setupIntent.status (succeeded / processing / requires_payment_method), surface the appropriate toast, strip the params from the URL via router.replace, and only then create a new session if the prior intent is not in a terminal-success state.
1 participant
Ticket
Summary
Test plan
Images