Make payment section provider agnostic + support manual payment metho…#129
Merged
damianlegawiec merged 30 commits intomainfrom Apr 27, 2026
Merged
Make payment section provider agnostic + support manual payment metho…#129damianlegawiec merged 30 commits intomainfrom
damianlegawiec merged 30 commits intomainfrom
Conversation
…ds (non session ones)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds multi-gateway payment support: new Adyen and PayPal client components, Stripe conditional initialization, PaymentSection refactor to support session vs direct payments, server-side direct-payment action and Changes
Sequence DiagramsequenceDiagram
actor User
participant Page as Checkout Page
participant PaymentSection as PaymentSection
participant GatewayForm as Gateway Form (Stripe/Adyen/PayPal)
participant PaymentAPI as Payment API
participant OrderAPI as Order Completion
User->>Page: open checkout
Page->>PaymentSection: render cart & payment methods
User->>PaymentSection: select payment method
PaymentSection->>PaymentSection: resolveGatewayId() / determine session-based
alt Session-based
PaymentSection->>PaymentAPI: createCheckoutPaymentSession(cartId, methodId, externalData)
PaymentAPI-->>PaymentSection: { sessionId, sessionData }
PaymentSection->>GatewayForm: init with sessionData/sessionId
GatewayForm-->>PaymentSection: onReady (handle)
User->>GatewayForm: submit payment
GatewayForm->>PaymentAPI: confirm/process payment
PaymentAPI-->>GatewayForm: confirmed
GatewayForm-->>PaymentSection: onPaymentCompleted
PaymentSection->>Page: onPaymentComplete({type:"session", sessionId, sessionResult?})
Page->>OrderAPI: completeCheckoutPaymentSession(cartId, sessionId, { session_result?: sessionResult })
else Direct/manual
PaymentSection->>User: show manual instructions
User->>PaymentSection: confirm order
PaymentSection->>PaymentAPI: createDirectPayment(cartId, methodId)
PaymentAPI-->>PaymentSection: { payment }
PaymentSection->>Page: onPaymentComplete({type:"direct"})
Page->>OrderAPI: complete order (no session)
else Zero-amount
PaymentSection->>Page: onPaymentComplete({type:"direct"})
end
OrderAPI-->>Page: order confirmation
Page->>User: show confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/checkout/AdyenPaymentForm.tsx (1)
28-36: Consider using a more specific type for the drop-in ref.While the comment explains the reasoning, using
anyweakens type safety. Consider usingunknownor a minimal interface.♻️ Suggested alternative
- // Adyen SDK types use Preact internally — use a loose ref to avoid - // coupling to their internal component hierarchy. - const dropinRef = useRef<any>(null); + // Adyen SDK types use Preact internally — use a minimal interface + // to avoid coupling to their internal component hierarchy. + const dropinRef = useRef<{ submit: () => void; unmount: () => void } | null>( + null, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/AdyenPaymentForm.tsx` around lines 28 - 36, The dropinRef currently uses an unsafe any type; replace it with a narrower type (e.g., useRef<unknown>(null) or define a minimal interface matching the Adyen Dropin methods you call such as mount/destroy/submit and useRef<AdyenDropin | null>(null)) so you retain type safety while avoiding coupling to Preact internals; update references to dropinRef throughout the component (e.g., where you call methods on the drop-in) to match the new type/interface and adjust resolvePaymentRef and related handlers if necessary.src/components/checkout/PaymentSection.tsx (1)
300-350: Consider extracting the shared initialization logic.The initialization sequence in
handleMethodSelect(lines 310-338) duplicates the logic in the mount effect (lines 229-256). This could lead to drift if one is updated without the other.♻️ Suggested refactor
Extract the shared logic into a reusable function:
const initializeSessionMethod = useCallback( async (method: PaymentMethod) => { setLoading(true); let cardId: string | null = null; if (isAuthenticated) { try { const result = await getCreditCards(); const gatewayCards = result.data.filter( (card) => card.gateway_payment_profile_id, ); setSavedCards(gatewayCards); if (gatewayCards.length > 0) { const defaultCard = gatewayCards.find((c) => c.default) || gatewayCards[0]; cardId = defaultCard.gateway_payment_profile_id; setSelectedCardId(cardId); } } catch { // proceed without saved cards } } selectedCardRef.current = cardId; lastTotalRef.current = cart.total; await createSession(cardId, method); }, [isAuthenticated, cart.total, createSession], );Then use it in both places:
- Mount effect:
initializeSessionMethod(selectedMethod)handleMethodSelect:initializeSessionMethod(newMethod)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentSection.tsx` around lines 300 - 350, handleMethodSelect duplicates the session initialization logic found in the mount effect; extract that shared sequence into a single reusable async function (e.g., initializeSessionMethod) implemented with useCallback and proper deps (isAuthenticated, cart.total, createSession) that performs loading state, fetches and filters getCreditCards, sets saved cards and selectedCardId, updates selectedCardRef/lastTotalRef, and calls createSession; then call initializeSessionMethod(selectedMethod) from the mount effect and initializeSessionMethod(newMethod) from handleMethodSelect (replace the inline init/else branches) to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/utils/payment-gateway.ts`:
- Around line 48-50: The exported adyenEnvironment constant currently unsafely
casts NEXT_PUBLIC_ADYEN_ENVIRONMENT with as "test" | "live"; replace this with a
small validation step that reads process.env.NEXT_PUBLIC_ADYEN_ENVIRONMENT,
checks whether it equals "test" or "live" (e.g., via a helper isValidAdyenEnv or
a switch), and then export either the validated value or a safe default ("test")
or throw a clear error; update the exported symbol adyenEnvironment to use the
validated result so only "test" or "live" can be passed to the Adyen SDK.
---
Nitpick comments:
In `@src/components/checkout/AdyenPaymentForm.tsx`:
- Around line 28-36: The dropinRef currently uses an unsafe any type; replace it
with a narrower type (e.g., useRef<unknown>(null) or define a minimal interface
matching the Adyen Dropin methods you call such as mount/destroy/submit and
useRef<AdyenDropin | null>(null)) so you retain type safety while avoiding
coupling to Preact internals; update references to dropinRef throughout the
component (e.g., where you call methods on the drop-in) to match the new
type/interface and adjust resolvePaymentRef and related handlers if necessary.
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 300-350: handleMethodSelect duplicates the session initialization
logic found in the mount effect; extract that shared sequence into a single
reusable async function (e.g., initializeSessionMethod) implemented with
useCallback and proper deps (isAuthenticated, cart.total, createSession) that
performs loading state, fetches and filters getCreditCards, sets saved cards and
selectedCardId, updates selectedCardRef/lastTotalRef, and calls createSession;
then call initializeSessionMethod(selectedMethod) from the mount effect and
initializeSessionMethod(newMethod) from handleMethodSelect (replace the inline
init/else branches) to avoid drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5a80e80-ed4b-4208-8b37-a3c54cd6c36f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
messages/de.jsonmessages/en.jsonmessages/es.jsonmessages/fr.jsonmessages/pl.jsonpackage.jsonsrc/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/components/checkout/AdyenPaymentForm.tsxsrc/components/checkout/ExpressCheckoutButton.tsxsrc/components/checkout/PaymentSection.tsxsrc/components/checkout/index.tssrc/lib/data/__tests__/payment.test.tssrc/lib/data/express-checkout-flow.tssrc/lib/data/payment.tssrc/lib/utils/payment-gateway.tssrc/lib/utils/stripe.ts
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/checkout/PaymentSection.tsx (2)
368-497:⚠️ Potential issue | 🟠 MajorAdd an in-flight guard around imperative submit.
submit()ignores the existingprocessingprop before starting backend calls. A double click or repeated imperative call can create duplicate direct payments or concurrently complete the same session. Keep a localprocessingRefin sync and flip it before the first await. Based on learnings, checkout backend calls should be guarded with processing state to prevent redundant saves/calls during processing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentSection.tsx` around lines 368 - 497, The submit() imperative handler doesn't guard against concurrent calls; add a local ref (e.g., processingRef via useRef<boolean>(false)) kept in sync with setProcessing and check processingRef.current at the very start of submit to early-return an error (or { error: t("processing") }) if already true, set processingRef.current = true immediately before any awaited work begins, and ensure you set it back to false whenever you call setProcessing(false) or on all catch/early-return paths (also update it when you call setProcessing(true) so useImperativeHandle's submit, setProcessing, and processingRef remain consistent).
246-259:⚠️ Potential issue | 🔴 CriticalScope saved-card readiness to Stripe only.
Saved cards are selected for every session gateway. For PayPal/Adyen,
selectedCardIdcan be non-null whilegatewayHandleRef.currentis still null; then Line 435 passes and Line 460 can dereference null. Restrict saved-card loading/selection to Stripe, or make the submit guard gateway-aware.🐛 Harden the submit branch
const clientSecret = sessionExternalData.client_secret as | string | undefined; + const isStripeSavedCard = + resolveGatewayId(selectedMethod.type) === "stripe" && + selectedCardId !== null && + typeof clientSecret === "string"; - if (selectedCardId && clientSecret) { + if (!isStripeSavedCard && !gatewayHandleRef.current) { + setProcessing(false); + return { error: t("failedToInitPayment") }; + } + + if (isStripeSavedCard) { // Stripe saved card flow const result = await confirmWithSavedCard( clientSecret, selectedCardId, returnUrl, ); error = result.error; } else { // New payment via gateway handle (Stripe PaymentElement or Adyen Drop-in) const result = - await gatewayHandleRef.current!.confirmPayment(returnUrl); + await gatewayHandleRef.current.confirmPayment(returnUrl); error = result.error; }Also applies to: 328-340, 428-461
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentSection.tsx` around lines 246 - 259, The saved-card loading/selection must be restricted to Stripe to avoid selecting a card when gatewayHandleRef.current is null or non-Stripe; update the effect/logic that calls getCreditCards (the block using getCreditCards, setSavedCards and setSelectedCardId) to first verify the current gateway is Stripe (e.g., gatewayHandleRef.current?.gateway === 'stripe' or use your app's Stripe identifier) and only then fetch and set gatewayCards/initialCardId; likewise, harden the submit handler (the function that reads selectedCardId and uses gatewayHandleRef, e.g., submitPayment/onSubmit) to check gatewayHandleRef.current exists and matches Stripe before using selectedCardId, or else ignore saved-card flow for non-Stripe gateways—apply the same gateway-aware guard wherever getCreditCards and selectedCardId are used (the other saved-card loading blocks and submit branch).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 106-121: The parent is being notified that the method is
session-based even for zero-amount orders; derive an effective session flag
(e.g., const isSessionEffective = (selectedMethod?.session_required ?? false) &&
!isZeroAmount) and use that instead of isSessionBased in the
prevIsSessionRef/useEffect/onSessionMethodChangeRef flow so free orders are
treated as direct; update references to isSessionBased in the useEffect block
(prevIsSessionRef, onSessionMethodChangeRef.current call) to use the new
isSessionEffective value so the parent receives false for zero-amount carts.
- Around line 354-361: When switching to a direct method, first invalidate the
in-flight session guard used by createSession (e.g. increment the session
request nonce / call the same abort/invalidate function that createSession
checks) so any pending createSession resolution will bail; after advancing that
stale-request guard, then clear state with setSessionExternalData(null),
setPaymentSessionId(null), setGatewayError(null), setLoading(false) and set
gatewayHandleRef.current = null. This ensures a late-resolving createSession
cannot repopulate sessionExternalData/paymentSessionId for an unselected
gateway.
In `@src/components/checkout/PayPalPaymentForm.tsx`:
- Around line 34-45: confirmPayment currently returns a Promise that only
resolves when resolvePaymentRef is invoked by the PayPalButtons onApprove, which
leaves the checkout submit pending if the user clicks the main CTA before
approving; update confirmPayment (the useCallback) to immediately return a
resolved object with a clear error (e.g. { error: "Please approve the PayPal
payment in the PayPal dialog" }) when approvedRef.current is false instead of
creating a never-resolving promise, or alternatively programmatically trigger
the PayPal approval flow from the submit handler; modify the logic around
approvedRef, resolvePaymentRef and the promise creation inside confirmPayment so
the submit path always receives a synchronous error rather than waiting
indefinitely.
- Around line 109-136: The PayPal-facing messages in PayPalPaymentForm (the
onError handler fallback message, the onCancel message, and the approvedForUI
paragraph) are hard-coded English strings; update PayPalPaymentForm to use the
app's localization (e.g., the existing translation hook/function used elsewhere
such as t or useTranslation) to produce localized strings instead of literal
text: replace the fallback error "An error occurred with PayPal. Please try
again.", the cancel text "PayPal payment was cancelled.", and the approval text
"PayPal payment approved. Completing your order..." with calls to the
translation function (pass any dynamic parts like error.message into the
translator if needed), and ensure AlertDescription and resolvePaymentRef.current
use the localized messages.
- Around line 60-68: Move the dynamic import out of the render path into a
useEffect that runs when relevant dependencies (e.g., session or gateway props)
change, and ensure you cancel stale work on unmount: in useEffect check if
PayPalSDK already exists before importing, perform
import("@paypal/react-paypal-js").then(mod => { if (!mounted) return;
setPayPalSDK({...}); if (mounted) onReady({ confirmPayment, fetchUpdates }); }),
and in the effect cleanup set a mounted flag (or use an AbortController) to
prevent calling onReady or setPayPalSDK after unmount; this prevents duplicate
imports/onReady calls and stale gateway handles in PaymentSection.
---
Outside diff comments:
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 368-497: The submit() imperative handler doesn't guard against
concurrent calls; add a local ref (e.g., processingRef via
useRef<boolean>(false)) kept in sync with setProcessing and check
processingRef.current at the very start of submit to early-return an error (or {
error: t("processing") }) if already true, set processingRef.current = true
immediately before any awaited work begins, and ensure you set it back to false
whenever you call setProcessing(false) or on all catch/early-return paths (also
update it when you call setProcessing(true) so useImperativeHandle's submit,
setProcessing, and processingRef remain consistent).
- Around line 246-259: The saved-card loading/selection must be restricted to
Stripe to avoid selecting a card when gatewayHandleRef.current is null or
non-Stripe; update the effect/logic that calls getCreditCards (the block using
getCreditCards, setSavedCards and setSelectedCardId) to first verify the current
gateway is Stripe (e.g., gatewayHandleRef.current?.gateway === 'stripe' or use
your app's Stripe identifier) and only then fetch and set
gatewayCards/initialCardId; likewise, harden the submit handler (the function
that reads selectedCardId and uses gatewayHandleRef, e.g.,
submitPayment/onSubmit) to check gatewayHandleRef.current exists and matches
Stripe before using selectedCardId, or else ignore saved-card flow for
non-Stripe gateways—apply the same gateway-aware guard wherever getCreditCards
and selectedCardId are used (the other saved-card loading blocks and submit
branch).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37bb01de-affd-41d2-9fe5-8b201526f8b7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/components/checkout/PayPalPaymentForm.tsxsrc/components/checkout/PaymentSection.tsxsrc/lib/utils/payment-gateway.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/utils/payment-gateway.ts
- PayPal: return error instead of hanging promise when not yet approved - PayPal: move SDK loading to useEffect with cancellation - PayPal: localize all user-facing strings (5 locales) - Adyen: validate environment value instead of unsafe type cast - PaymentSection: treat zero-amount orders as non-session for button text - PaymentSection: invalidate in-flight sessions when switching to direct method Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The PayPal flow is user-initiated via the PayPal button, not via the checkout page's "Place Order" button. After the user approves in the PayPal popup, onApproved fires and PaymentSection directly updates the billing address and completes the payment session + order. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/components/checkout/PayPalPaymentForm.tsx (1)
25-30: Add explicit return types to component functions.These component functions lack explicit return type annotations, which violates the coding guideline: "Use strict TypeScript type checking. Always define explicit return types for functions."
PayPalPaymentFormInnershould returnJSX.Element, andPayPalPaymentFormshould returnJSX.Element | nullsince it conditionally returnsnull.♻️ Proposed typing cleanup
-import { useCallback, useEffect, useRef, useState } from "react"; +import { type JSX, useCallback, useEffect, useRef, useState } from "react"; @@ function PayPalPaymentFormInner({ paypalOrderId, currency, onReady, onApproved, -}: PayPalPaymentFormProps) { +}: PayPalPaymentFormProps): JSX.Element { @@ -export function PayPalPaymentForm(props: PayPalPaymentFormProps) { +export function PayPalPaymentForm( + props: PayPalPaymentFormProps, +): JSX.Element | null {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PayPalPaymentForm.tsx` around lines 25 - 30, The component functions lack explicit TypeScript return types: update PayPalPaymentFormInner to declare its return type as JSX.Element and update PayPalPaymentForm to declare its return type as JSX.Element | null (since it conditionally returns null); locate the function signatures for PayPalPaymentFormInner and PayPalPaymentForm in the file and add the respective return type annotations to their definitions to satisfy strict typing rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 99-105: selectedMethodId is only initialized once so when
paymentMethods changes the stored id can become invalid and selectedMethod
becomes undefined; add a useEffect watching paymentMethods that checks if the
current selectedMethodId still exists in paymentMethods and if not calls
setSelectedMethodId to either the first paymentMethods[0].id or "" (or leave
current id if still present). Update the logic that derives selectedMethod
(paymentMethods.find((pm) => pm.id === selectedMethodId)) to rely on this effect
so the radio group always has a valid selected value; use the existing
identifiers selectedMethodId, setSelectedMethodId, selectedMethod and
paymentMethods to locate and implement the fix.
- Around line 372-376: Add a local in-flight guard ref (e.g., processingRef and
completionInFlightRef) and use it inside handlePayPalApproved and the submit
function to prevent re-entrancy: at the very top of handlePayPalApproved and
submit check if processingRef.current or completionInFlightRef.current is true
and return early; otherwise set them true immediately (aligned with
setProcessing(true)) and clear them on all exit paths (including before every
early error return in submit and on success/finally). Ensure the ref state is
mounted-guarded (check mounted or use a mount ref) so it stays aligned with the
processing prop and is reset appropriately when the component unmounts.
- Around line 473-505: The saved-card branch must only run for Stripe session
flows and you must never dereference gatewayHandleRef.current when it's null;
update the condition around confirmWithSavedCard to require that the
session/gateway is Stripe (e.g., check selectedMethod identifier or
sessionExternalData indicating Stripe) in addition to selectedCardId and
clientSecret so you don't take the saved-card path for PayPal/Adyen sessions,
and before calling gatewayHandleRef.current.confirmPayment ensure
gatewayHandleRef.current exists and return a handled error if it does not.
Reference symbols to change: selectedCardId, clientSecret, sessionExternalData
(or selectedMethod id/gateway), confirmWithSavedCard, and
gatewayHandleRef.current.confirmPayment.
In `@src/components/checkout/PayPalPaymentForm.tsx`:
- Around line 64-84: The dynamic import in the useEffect that loads PayPal SDK
(checking PayPalSDK, calling setPayPalSDK) does not handle promise rejection and
can leave the component hanging; update the
import("@paypal/react-paypal-js").then(...) chain to add a .catch handler that
sets an error state (e.g., setPayPalError or setPayPalLoadFailed) and/or calls
onReadyRef.current with a failure object so the parent knows the component
failed to initialize, ensure the cancelled flag is checked in the catch as well,
and keep existing cleanup logic so confirmPaymentRef and fetchUpdatesRef are not
referenced if loading failed.
---
Nitpick comments:
In `@src/components/checkout/PayPalPaymentForm.tsx`:
- Around line 25-30: The component functions lack explicit TypeScript return
types: update PayPalPaymentFormInner to declare its return type as JSX.Element
and update PayPalPaymentForm to declare its return type as JSX.Element | null
(since it conditionally returns null); locate the function signatures for
PayPalPaymentFormInner and PayPalPaymentForm in the file and add the respective
return type annotations to their definitions to satisfy strict typing rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1c7c304-8389-48c2-a0c0-d1994d180a97
📒 Files selected for processing (8)
messages/de.jsonmessages/en.jsonmessages/es.jsonmessages/fr.jsonmessages/pl.jsonsrc/components/checkout/PayPalPaymentForm.tsxsrc/components/checkout/PaymentSection.tsxsrc/lib/utils/payment-gateway.ts
✅ Files skipped from review due to trivial changes (3)
- messages/de.json
- messages/pl.json
- src/lib/utils/payment-gateway.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- messages/en.json
- messages/es.json
… error - Critical: gate saved-card confirmation to Stripe only, preventing null dereference crash when PayPal/Adyen is selected with authenticated user - Major: fall back to first payment method when selectedMethodId becomes stale after cart.payment_methods changes - Major: add completionInFlightRef guard to prevent re-entrant submit/ PayPal approval from double-completing the order - Minor: handle PayPal SDK import failure with error message instead of infinite spinner Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ternal_data Spree's Adyen gateway stores the Adyen session ID in PaymentSession.external_id, not inside external_data. The external_data only contains session_data, channel, and return_url. Include external_id as _external_id in the stored session data so the Adyen form can access it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adyen Web v6 requires explicit registration of payment method components (Card, GooglePay, ApplePay, PayPal, Klarna, Bancontact, Redirect) via the paymentMethodComponents config. Without them the Drop-in mounts but renders nothing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ister() paymentMethodComponents on the Dropin constructor alone wasn't sufficient. Register components globally via AdyenCheckout.register() before creating the checkout instance, which is the v6 recommended approach. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Next.js doesn't process dynamic import() of CSS from node_modules. Move adyen.css to a static top-level import so it's bundled correctly. Also add debug logging for Adyen init flow (temporary). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/components/checkout/PaymentSection.tsx (1)
99-107:⚠️ Potential issue | 🟠 MajorSynchronize stale payment-method fallback with session lifecycle.
The render fallback on Lines 104-107 can switch the effective method when
cart.payment_methodschanges, but it does not runhandleMethodSelect, recreate a session, or clear stale session state. If the previous method’s session is still in state, the UI/submit path can use oldsessionExternalDatafor the newly effective method.🐛 Suggested direction
Centralize method-change side effects so both user selection and
paymentMethodsrefreshes go through the same path:- const selectedMethod: PaymentMethod | undefined = - paymentMethods.find((pm) => pm.id === selectedMethodId) ?? - paymentMethods[0]; + const selectedMethod: PaymentMethod | undefined = paymentMethods.find( + (pm) => pm.id === selectedMethodId, + ); const effectiveSelectedMethodId = selectedMethod?.id ?? "";Then add a small sync effect after
handleMethodSelectis available:+ useEffect(() => { + if (paymentMethods.length === 0) { + if (selectedMethodId !== "") setSelectedMethodId(""); + sessionRequestIdRef.current += 1; + setSessionExternalData(null); + setPaymentSessionId(null); + setGatewayError(null); + gatewayHandleRef.current = null; + setLoading(false); + return; + } + + if (!paymentMethods.some((pm) => pm.id === selectedMethodId)) { + handleMethodSelect(paymentMethods[0].id); + } + }, [paymentMethods, selectedMethodId, handleMethodSelect]);This requires wrapping
handleMethodSelectinuseCallbackso the effect does not fire on every render.Also applies to: 242-299, 322-373
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PaymentSection.tsx` around lines 99 - 107, The fallback that computes selectedMethod and effectiveSelectedMethodId can change when paymentMethods updates but doesn't run the selection side-effects (session recreation / clearing sessionExternalData); update the flow so method changes always go through a single handler: wrap handleMethodSelect in useCallback and replace any implicit fallback assignment logic (selectedMethodId / setSelectedMethodId) with a useEffect that watches paymentMethods and calls handleMethodSelect(paymentMethods[0]?.id ?? "") when the previously selected id becomes stale; ensure handleMethodSelect performs recreating the payment session and clearing any sessionExternalData so the UI/submit path never retains stale session state (affecting selectedMethodId, selectedMethod, effectiveSelectedMethodId).src/components/checkout/PayPalPaymentForm.tsx (1)
81-104:⚠️ Potential issue | 🟠 MajorRender the PayPal load error instead of the spinner.
The catch block sets
error, but Lines 99-104 return the loading spinner wheneverPayPalSDKis stillnull, so SDK load failures remain visually indistinguishable from loading.🐛 Proposed fix
if (!PayPalSDK) { + if (error) { + return ( + <Alert variant="destructive" className="mt-3"> + <CircleAlert /> + <AlertDescription>{error}</AlertDescription> + </Alert> + ); + } + return ( <div className="flex items-center justify-center py-8"> <div className="w-5 h-5 border-2 border-gray-300 border-t-gray-600 rounded-full animate-spin" /> </div> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/PayPalPaymentForm.tsx` around lines 81 - 104, The UI currently always shows the loading spinner when PayPalSDK is null, hiding load failures set via setError; update the render logic to check the error state first (the variable updated in the catch that calls setError) and return a clear error UI when error is truthy (e.g., show the error message or a retry hint), otherwise keep the existing spinner when PayPalSDK is still null; locate the relevant render branch using PayPalSDK and setError (and the error state variable) and adjust the conditional order so error is rendered before the spinner.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/checkout/AdyenPaymentForm.tsx`:
- Around line 39-165: The AdyenPaymentForm currently uses hardcoded user-facing
strings (e.g., "Adyen has not loaded yet", "An error occurred during payment.",
"Payment ... failed. Please try again.", "Failed to initialize payment form.")
which bypass next-intl; update the component to use checkout translation keys
instead. In AdyenPaymentForm, import/use the existing translation helper (e.g.,
useTranslations('checkout') or accept the same t function used by
PaymentSection), then replace all hardcoded messages in confirmPayment,
onPaymentCompleted, onPaymentFailed, onError, and the init catch block with
calls to the translation keys (e.g., t('adyen.notLoaded'),
t('adyen.genericError'), t('adyen.paymentFailed'), t('adyen.initFailed')) and
pass any dynamic parts (like resultCode) to the translator; ensure
resolvePaymentRef.current and setError receive the translated strings.
- Around line 36-180: The effect that initializes Adyen Drop-in should not
include the local mounted state because setMounted(true) inside init triggers
the effect cleanup which unmounts the Drop-in; remove the mounted state and all
uses of setMounted and the mounted variable, and remove mounted from the
useEffect dependency array so the effect depends only on sessionId, sessionData,
onReady, confirmPayment, and fetchUpdates; ensure dropinRef.current is still
mounted/unmounted in the cleanup and that onReady({ confirmPayment, fetchUpdates
}) is called after dropinRef.current is set.
---
Duplicate comments:
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 99-107: The fallback that computes selectedMethod and
effectiveSelectedMethodId can change when paymentMethods updates but doesn't run
the selection side-effects (session recreation / clearing sessionExternalData);
update the flow so method changes always go through a single handler: wrap
handleMethodSelect in useCallback and replace any implicit fallback assignment
logic (selectedMethodId / setSelectedMethodId) with a useEffect that watches
paymentMethods and calls handleMethodSelect(paymentMethods[0]?.id ?? "") when
the previously selected id becomes stale; ensure handleMethodSelect performs
recreating the payment session and clearing any sessionExternalData so the
UI/submit path never retains stale session state (affecting selectedMethodId,
selectedMethod, effectiveSelectedMethodId).
In `@src/components/checkout/PayPalPaymentForm.tsx`:
- Around line 81-104: The UI currently always shows the loading spinner when
PayPalSDK is null, hiding load failures set via setError; update the render
logic to check the error state first (the variable updated in the catch that
calls setError) and return a clear error UI when error is truthy (e.g., show the
error message or a retry hint), otherwise keep the existing spinner when
PayPalSDK is still null; locate the relevant render branch using PayPalSDK and
setError (and the error state variable) and adjust the conditional order so
error is rendered before the spinner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2c88f1f-750f-44de-a5dc-e9988bb6a906
📒 Files selected for processing (3)
src/components/checkout/AdyenPaymentForm.tsxsrc/components/checkout/PayPalPaymentForm.tsxsrc/components/checkout/PaymentSection.tsx
Comment on lines
+39
to
+165
| async (_returnUrl: string): Promise<{ error?: string }> => { | ||
| if (!dropinRef.current) { | ||
| return { error: "Adyen has not loaded yet" }; | ||
| } | ||
|
|
||
| // The Drop-in handles its own submit flow. We trigger it | ||
| // programmatically and wait for onPaymentCompleted/onPaymentFailed. | ||
| return new Promise<{ error?: string }>((resolve) => { | ||
| resolvePaymentRef.current = resolve; | ||
| try { | ||
| dropinRef.current.submit(); | ||
| } catch (err) { | ||
| resolve({ | ||
| error: | ||
| err instanceof Error | ||
| ? err.message | ||
| : "An error occurred during payment.", | ||
| }); | ||
| resolvePaymentRef.current = null; | ||
| } | ||
| }); | ||
| }, | ||
| [], | ||
| ); | ||
|
|
||
| const fetchUpdates = useCallback(async () => { | ||
| // Adyen sessions auto-update; no manual fetch needed | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (mounted) return; | ||
| if (!containerRef.current) return; | ||
|
|
||
| let cancelled = false; | ||
|
|
||
| async function init() { | ||
| try { | ||
| const adyen = await import("@adyen/adyen-web"); | ||
| const { | ||
| AdyenCheckout, | ||
| Dropin, | ||
| Card, | ||
| GooglePay, | ||
| ApplePay, | ||
| PayPal, | ||
| Klarna, | ||
| Bancontact, | ||
| Redirect, | ||
| } = adyen; | ||
| await import("@adyen/adyen-web/styles/adyen.css"); | ||
|
|
||
| if (cancelled || !containerRef.current) return; | ||
|
|
||
| // Register payment method components globally so Drop-in can use them | ||
| AdyenCheckout.register( | ||
| Card, | ||
| GooglePay, | ||
| ApplePay, | ||
| PayPal, | ||
| Klarna, | ||
| Bancontact, | ||
| Redirect, | ||
| ); | ||
|
|
||
| const checkout = await AdyenCheckout({ | ||
| clientKey: adyenClientKey, | ||
| environment: adyenEnvironment, | ||
| session: { | ||
| id: sessionId, | ||
| sessionData, | ||
| }, | ||
| showPayButton: false, | ||
| onPaymentCompleted: (result) => { | ||
| if ( | ||
| result.resultCode === "Authorised" || | ||
| result.resultCode === "Pending" || | ||
| result.resultCode === "Received" | ||
| ) { | ||
| resolvePaymentRef.current?.({}); | ||
| } else { | ||
| const msg = `Payment ${result.resultCode?.toLowerCase() || "failed"}. Please try again.`; | ||
| setError(msg); | ||
| resolvePaymentRef.current?.({ error: msg }); | ||
| } | ||
| resolvePaymentRef.current = null; | ||
| }, | ||
| onPaymentFailed: () => { | ||
| const msg = "An error occurred during payment."; | ||
| setError(msg); | ||
| resolvePaymentRef.current?.({ error: msg }); | ||
| resolvePaymentRef.current = null; | ||
| }, | ||
| onError: (err) => { | ||
| const msg = err?.message || "An error occurred during payment."; | ||
| setError(msg); | ||
| resolvePaymentRef.current?.({ error: msg }); | ||
| resolvePaymentRef.current = null; | ||
| }, | ||
| }); | ||
|
|
||
| if (cancelled || !containerRef.current) return; | ||
|
|
||
| const dropin = new Dropin(checkout, { | ||
| openFirstPaymentMethod: true, | ||
| paymentMethodComponents: [ | ||
| Card, | ||
| GooglePay, | ||
| ApplePay, | ||
| PayPal, | ||
| Klarna, | ||
| Bancontact, | ||
| Redirect, | ||
| ], | ||
| }); | ||
|
|
||
| dropin.mount(containerRef.current); | ||
| dropinRef.current = dropin; | ||
| setMounted(true); | ||
|
|
||
| onReady({ confirmPayment, fetchUpdates }); | ||
| } catch (err) { | ||
| if (!cancelled) { | ||
| setError( | ||
| err instanceof Error | ||
| ? err.message | ||
| : "Failed to initialize payment form.", | ||
| ); |
There was a problem hiding this comment.
Localize Adyen-facing error messages.
These strings are shown in checkout UI but bypass next-intl, unlike the PayPal form and the rest of PaymentSection. Please route them through checkout translation keys.
🌐 Suggested direction
+import { useTranslations } from "next-intl";
import { useCallback, useEffect, useRef, useState } from "react";
@@
}: AdyenPaymentFormProps) {
+ const t = useTranslations("checkout");
const containerRef = useRef<HTMLDivElement>(null);
@@
if (!dropinRef.current) {
- return { error: "Adyen has not loaded yet" };
+ return { error: t("failedToInitPayment") };
}
@@
- : "An error occurred during payment.",
+ : t("paymentError"),
@@
- const msg = `Payment ${result.resultCode?.toLowerCase() || "failed"}. Please try again.`;
+ const msg = t("paymentError");
@@
- const msg = "An error occurred during payment.";
+ const msg = t("paymentError");
@@
- const msg = err?.message || "An error occurred during payment.";
+ const msg = err?.message || t("paymentError");
@@
- : "Failed to initialize payment form.",
+ : t("failedToInitPayment"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/checkout/AdyenPaymentForm.tsx` around lines 39 - 165, The
AdyenPaymentForm currently uses hardcoded user-facing strings (e.g., "Adyen has
not loaded yet", "An error occurred during payment.", "Payment ... failed.
Please try again.", "Failed to initialize payment form.") which bypass
next-intl; update the component to use checkout translation keys instead. In
AdyenPaymentForm, import/use the existing translation helper (e.g.,
useTranslations('checkout') or accept the same t function used by
PaymentSection), then replace all hardcoded messages in confirmPayment,
onPaymentCompleted, onPaymentFailed, onError, and the init catch block with
calls to the translation keys (e.g., t('adyen.notLoaded'),
t('adyen.genericError'), t('adyen.paymentFailed'), t('adyen.initFailed')) and
pass any dynamic parts (like resultCode) to the translator; ensure
resolvePaymentRef.current and setError receive the translated strings.
The Adyen SDK uses Preact internally. When mounting directly into a React ref div, Preact's render() silently fails because React owns that DOM node. Fix: create a child div imperatively via document.createElement and append it to the container ref, then mount the Dropin into that node. This gives Preact full ownership of its mount target without conflicting with React. Also remove debug console.log statements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adyen's sessions flow handles payment internally via its own Pay button. Trying to call dropin.submit() externally caused "unknown error". Switch to showPayButton: true and auto-complete via onApproved callback when onPaymentCompleted fires with Authorised/Pending/Received. Also rename handlePayPalApproved to handleGatewayApproved since it's now shared by both PayPal and Adyen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two Pay buttons is bad UX. Adyen should work like Stripe — our "Pay Now" button calls dropin.submit() which validates and processes via the session. The earlier "unknown error" was a session/config issue, not a submit() issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
For approval-driven gateways (Adyen, PayPal), confirmPayment() returns immediately after triggering the gateway's submit — the actual result arrives asynchronously via onApproved. The submit path now returns early for these gateways, letting handleGatewayApproved complete the order with the correct sessionResult. Stripe continues completing inline since confirmPayment() is synchronous (awaits the Stripe SDK response before returning). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Addresses page: switch from getCountries() (global) to getMarketCountries() so only countries available in the current market are shown in address forms - getCountry: add remote cache with "hours" lifetime so repeated state lookups (e.g. switching countries in address form) don't hit the API every time Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- AddressSection: when shipping address has no country_iso, default to the first country from the market's country list - Checkout page: prefetch states for the default country (from cart's shipping address or first market country) during loadOrder, warming the server cache so states render immediately without a spinner Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Split the checkout page into a Server Component (page.tsx) that fetches all initial data during SSR, and a Client Component (CheckoutPageContent) that receives it as props. Before: page rendered a loading skeleton, hydrated, fired 5 server action calls from the client, then showed the checkout UI. After: Server Component fetches cart, market, countries, addresses, and auth status in parallel during the initial request. The client component initializes with real data — no loading skeleton, no waterfall. loadOrder() is kept as a refresh function for coupon changes, express checkout completion, etc. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Only call getAddresses() when authenticated (guests don't have saved addresses, and the API throws "Not authenticated") - Initialize prevOrderKeyRef to null so the sidebar useEffect fires on first render with SSR data (was skipping because ref and cartKey matched on init) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
getLocaleOptions() calls cookies() which rejects during Next.js static prerendering. Wrap in try/catch and fall back to config defaults. This fixes both the cookies() rejection and the "use cache" timeout in cachedListMarkets (which received options derived from cookies). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract CheckoutSidebar to CheckoutSidebar.tsx (was inline in CheckoutPageContent.tsx) - Use useLayoutEffect instead of useEffect for sidebar context so it renders before first paint (no empty sidebar flash with SSR data) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When initialData is provided from the server component, we already know isAuthenticated — no need to wait for AuthContext to finish its own client-side getCustomer() call. This eliminates the loading skeleton flash after hydration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
With server-side data fetching, the checkout UI arrives fully rendered via streaming. The Suspense fallback skeleton is never visible to users. Keep Suspense for useSearchParams() requirement but with empty fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The sidebar renders via CheckoutContext which populates after hydration. During streaming, the sidebar column was empty. Now CheckoutSummary shows a skeleton (line items + totals) until real content arrives. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The sidebar was in the layout (renders before page data) and populated via CheckoutContext after hydration — causing an empty sidebar during SSR streaming. Now the sidebar (Summary + CouponCode) renders inline in the checkout page content, which has the cart data from the server component. Both form and sidebar stream together in one chunk. - Layout: simplified to just header + two-column grid + footer, no CheckoutContext/CheckoutProvider - CheckoutPageContent: renders both form column and sidebar column, plus mobile summary toggle - order-placed: removed CheckoutContext dependency (was just clearing it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This reverts commit 1d01705.
…tions - Sidebar: use PENDING sentinel so skeleton only shows before first setSummaryContent call, not when content is explicitly cleared (null) - Sidebar skeleton: show 1 product placeholder instead of 2 - getCart: wrap clearCartCookies in try/catch for Server Component compatibility (can't set cookies outside Server Actions) - Tests: update confirm-payment assertions to match 5-param signature Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ds (non session ones)
Fixes #64
Summary by CodeRabbit
New Features
Localization
Bug Fixes / UX
Tests