One-page checkout - #58
Merged
Merged
Conversation
Contributor
|
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:
WalkthroughThis PR converts checkout from step-based UIs to modular sections (AddressSection, ShippingMethodSection, PaymentSection + Summary), migrates API/data model from order→cart (getCart/getOrder/updateCart), updates payment flows to use cartId, adjusts UI/layout/styles, updates exports, and bumps Spree dependencies to v0.10.x. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CheckoutPage as Checkout Page
participant AddressSection
participant ShippingSection as ShippingMethodSection
participant PaymentSection
participant API as Spree API
User->>CheckoutPage: Open checkout (cartId)
CheckoutPage->>API: getCheckoutOrder(cartId)
API-->>CheckoutPage: cart data
User->>AddressSection: Enter/select address
AddressSection->>API: updateCart(cartId, ship_address)
API-->>AddressSection: updated cart
AddressSection-->>CheckoutPage: onAutoSave / updated cart
User->>ShippingSection: Select shipping rate
ShippingSection->>API: selectShippingRate(cartId, shipmentId, rateId)
API-->>ShippingSection: updated cart/shipments
User->>CheckoutPage: Click "Pay now"
CheckoutPage->>PaymentSection: validateAndPay (invoke submit via ref)
PaymentSection->>API: createCheckoutPaymentSession(cartId) / confirm payment
API-->>PaymentSection: session/clientSecret / payment result
PaymentSection-->>CheckoutPage: onPaymentComplete
CheckoutPage->>API: completeCheckoutOrder(cartId)
API-->>CheckoutPage: completed cart/order
CheckoutPage->>User: Navigate to order-placed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
damianlegawiec
marked this pull request as ready for review
March 13, 2026 14:16
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/components/checkout/ShippingMethodSection.tsx (1)
18-20: Consider adding error handling for the async rate change.If
onShippingRateSelectrejects, the error will propagate as an unhandled promise rejection sincehandleRateChangedoesn't catch it. While the parent component may handle errors, defensive error handling here would prevent potential issues.🛡️ Optional: Add try-catch for defensive error handling
const handleRateChange = async (shipmentId: string, rateId: string) => { - await onShippingRateSelect(shipmentId, rateId); + try { + await onShippingRateSelect(shipmentId, rateId); + } catch (error) { + // Error should be handled by parent via errors prop + console.error("Failed to select shipping rate:", error); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/ShippingMethodSection.tsx` around lines 18 - 20, handleRateChange currently awaits onShippingRateSelect without catching rejections; wrap the await in a try-catch inside handleRateChange to prevent unhandled promise rejections. In the catch block, call an optional error handler prop if available (e.g., props.onShippingRateError or similar), otherwise log the error (console.error or process logger) and/or set a local error state (e.g., setShippingError) so the UI can surface it; do not swallow the error silently—either surface it to the parent handler or log it for debugging.src/components/checkout/PaymentSection.tsx (1)
31-36: Prefer@/aliases for the checkout component imports.These new relative imports are inconsistent with the repo's alias convention.
As per coding guidelines, "Use absolute imports with @ alias (e.g.,
@/components/...,@/lib/...) instead of relative imports".🤖 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 31 - 36, The imports in PaymentSection.tsx use relative paths; update them to the repo's alias convention by replacing the relative import paths with "@/components/..." aliases for AddressFormFields, and "@/components/..." (or the correct alias path) for confirmWithSavedCard, StripePaymentForm, and the StripePaymentFormHandle type so all four symbols (AddressFormFields, confirmWithSavedCard, StripePaymentForm, StripePaymentFormHandle) are imported via the `@/` alias rather than relative paths.src/components/checkout/AddressSection.tsx (1)
13-15: Use@/aliases for these component imports.These new
./...paths go against the repo import convention.As per coding guidelines, "Use absolute imports with @ alias (e.g.,
@/components/...,@/lib/...) instead of relative imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/AddressSection.tsx` around lines 13 - 15, Replace the relative imports for the checkout address components with the repository's @ alias form: change imports that reference "./AddressEditModal", "./AddressFormFields", and "./AddressSelector" to use "@/components/..." (or the correct `@/` path that maps to those component files) so AddressEditModal, AddressFormFields, and AddressSelector are imported via the @ alias instead of relative paths; keep the imported symbols unchanged.src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
491-494: Remove or implement the empty conditional block.The
if (result.error)block is empty with only a comment. This is a code smell that can confuse future maintainers. If no action is needed, remove the conditional entirely:Suggested fix
setProcessing(true); - const result = await paymentRef.current.submit(); - if (result.error) { - // Processing is already set to false by PaymentSection on error - } + await paymentRef.current.submit(); + // Note: PaymentSection handles setProcessing(false) on error internally };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx around lines 491 - 494, The empty if-block checking result.error after calling paymentRef.current.submit() should be removed or replaced with an explicit handling path; locate the call to paymentRef.current.submit() (variable paymentRef and its submit() invocation) and either delete the entire if (result.error) { /* ... */ } block if no action is required, or implement a clear handler (e.g., call a PaymentSection method, set state, or log the error) using the PaymentSection/error-handling utilities so the code no longer contains a no-op conditional.
🤖 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/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx:
- Around line 48-56: completeCheckoutOrder(orderId) is unsafe because the server
action completeCheckoutOrder (and its internal complete in
src/lib/data/payment.ts) ignores the passed orderId and uses the session cart,
so after refresh or when cookies differ the confirmation can show or track the
wrong order; update the page handler to either (A) call a server-side fetch by
order id as a fallback when result.order?.id !== orderId (or when
completeCheckoutOrder does not return an order or returns a different id) and
use that fetched order to populate orderData, or (B) fix the server action
completeCheckoutOrder/complete to actually accept and use the route param
orderId when provided so it returns the specific order for the page—locate
completeCheckoutOrder and the internal complete function in
src/lib/data/payment.ts and implement one of these fixes, then ensure the page
checks result.order.id against orderId before trusting result.order.
In `@src/components/checkout/AddressFormFields.tsx`:
- Around line 31-160: The form removed programmatic labels causing accessibility
loss; add back visually-hidden labels for each input/select using label elements
tied to the existing IDs (e.g., `${idPrefix}-country`, `${idPrefix}-firstname`,
`${idPrefix}-lastname`, `${idPrefix}-company`, `${idPrefix}-address1`,
`${idPrefix}-address2`, `${idPrefix}-city`, `${idPrefix}-state`,
`${idPrefix}-zipcode`, `${idPrefix}-phone`) so screen readers have stable names
while keeping the current placeholders/visual layout; implement labels with
htmlFor matching the id and a visually-hidden utility class (or
aria-label/aria-labelledby if a hidden label element is used) and ensure selects
(country/state) also receive these labels and required fields reflect the label
text.
In `@src/components/checkout/AddressSection.tsx`:
- Around line 129-149: The code updates lastSavedRef.current before onAutoSave
completes, causing failed writes to be treated as successful; change the
autosave flow so you only set lastSavedRef.current after a confirmed success
from onAutoSave (e.g., have onAutoSave return a Promise or accept a completion
callback) and on failure clear or do not set lastSavedRef.current so retries
occur; update both branches that call onAutoSave (the savedAddrId branch and the
ship_address branch using formDataToAddress, buildAutoSaveHash,
isAddressComplete) to await/handle the result and only persist the hash on
success, and explicitly clear it on error.
In `@src/components/checkout/AddressSelector.tsx`:
- Line 18: The onFieldBlur prop in AddressSelector should accept the blur event
instead of being a no-arg callback so callers can detect whether focus actually
left the whole selector; change onFieldBlur's type to accept a React.FocusEvent
(or FocusEvent<HTMLDivElement>) and update usages where onBlur={onFieldBlur} to
pass the event through. In AddressSection (and the other occurrence at lines
~155-157) use the event's relatedTarget (or
e.currentTarget.contains(relatedTarget)) to guard and only trigger autosave when
focus moved outside the selector, preventing the race between the autosave and
saved-address click/edit.
In `@src/components/checkout/CouponCode.tsx`:
- Around line 73-80: The remove button in CouponCode.tsx is icon-only and lacks
an accessible name; update the button that calls handleRemove(promotion.code!)
(which checks removing === promotion.code) to include an accessible label (e.g.,
aria-label or visually hidden text) that references the promotion code so screen
readers announce which code will be removed (for example: "Remove coupon
{promotion.code}"); ensure the label is dynamic and uses promotion.code so it
matches the button's action.
In `@src/components/checkout/OrderSummary.tsx`:
- Line 12: Replace the price-based shipping check with a shipment/rate presence
check: instead of using hasShipping = parseFloat(order.ship_total) > 0, set
hasShipping to something like order.shipments?.some(s => Array.isArray(s.rates)
&& s.rates.length > 0) (or the equivalent in your data shape) and update the
other branches (the logic around lines referencing shipment/rate presence at
57-65) to use this predicate so free-shipping or digital/no-shipping orders are
handled correctly.
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 112-149: createSession and the other payment-init callers (around
the blocks at 196-203 and 236-240) can commit out-of-order responses; add a
monotonic request token (e.g., a number or UUID) stored in a ref
(latestSessionRequestRef) and capture its value at the start of each
createSession call, then before any state writes (setClientSecret,
setPaymentSessionId, setGatewayError, setLoading, setGatewayReady,
gatewayHandleRef.current) verify the captured token matches the current ref
value so only the most recent response mutates state; increment/regenerate the
token at the start of each new session attempt and apply the same token-guarding
pattern to the other two request flows that also write
clientSecret/paymentSessionId.
In `@src/lib/data/checkout.ts`:
- Around line 16-27: getCheckoutOrder currently returns the active cart from
getCart() without verifying it matches the requested orderId, which can return
the wrong data; change getCheckoutOrder to call getCart(), and only return that
cart if cart.id (or cart.orderId field if different) strictly equals the
provided orderId, otherwise fall back to fetching the completed order via
withFallback(async () => (await getOrder(orderId)) as unknown as Cart, null).
Update the logic in getCheckoutOrder to perform this ID check so the function
uses getCart() only when it truly represents the requested order.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 491-494: The empty if-block checking result.error after calling
paymentRef.current.submit() should be removed or replaced with an explicit
handling path; locate the call to paymentRef.current.submit() (variable
paymentRef and its submit() invocation) and either delete the entire if
(result.error) { /* ... */ } block if no action is required, or implement a
clear handler (e.g., call a PaymentSection method, set state, or log the error)
using the PaymentSection/error-handling utilities so the code no longer contains
a no-op conditional.
In `@src/components/checkout/AddressSection.tsx`:
- Around line 13-15: Replace the relative imports for the checkout address
components with the repository's @ alias form: change imports that reference
"./AddressEditModal", "./AddressFormFields", and "./AddressSelector" to use
"@/components/..." (or the correct `@/` path that maps to those component files)
so AddressEditModal, AddressFormFields, and AddressSelector are imported via the
@ alias instead of relative paths; keep the imported symbols unchanged.
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 31-36: The imports in PaymentSection.tsx use relative paths;
update them to the repo's alias convention by replacing the relative import
paths with "@/components/..." aliases for AddressFormFields, and
"@/components/..." (or the correct alias path) for confirmWithSavedCard,
StripePaymentForm, and the StripePaymentFormHandle type so all four symbols
(AddressFormFields, confirmWithSavedCard, StripePaymentForm,
StripePaymentFormHandle) are imported via the `@/` alias rather than relative
paths.
In `@src/components/checkout/ShippingMethodSection.tsx`:
- Around line 18-20: handleRateChange currently awaits onShippingRateSelect
without catching rejections; wrap the await in a try-catch inside
handleRateChange to prevent unhandled promise rejections. In the catch block,
call an optional error handler prop if available (e.g.,
props.onShippingRateError or similar), otherwise log the error (console.error or
process logger) and/or set a local error state (e.g., setShippingError) so the
UI can surface it; do not swallow the error silently—either surface it to the
parent handler or log it for debugging.
🪄 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: 5cace63c-f70e-4012-81c6-0f2979e035ca
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
package.jsonsrc/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsxsrc/components/checkout/AddressFormFields.tsxsrc/components/checkout/AddressSection.tsxsrc/components/checkout/AddressSelector.tsxsrc/components/checkout/AddressStep.tsxsrc/components/checkout/CouponCode.tsxsrc/components/checkout/DeliveryStep.tsxsrc/components/checkout/OrderSummary.tsxsrc/components/checkout/PaymentSection.tsxsrc/components/checkout/PaymentStep.tsxsrc/components/checkout/ShippingMethodSection.tsxsrc/components/checkout/index.tssrc/components/ui/button.tsxsrc/components/ui/input.tsxsrc/components/ui/native-select.tsxsrc/lib/constants.tssrc/lib/data/__tests__/cart.test.tssrc/lib/data/__tests__/checkout.test.tssrc/lib/data/checkout.ts
💤 Files with no reviewable changes (4)
- src/lib/constants.ts
- src/components/checkout/PaymentStep.tsx
- src/components/checkout/AddressStep.tsx
- src/components/checkout/DeliveryStep.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
107-127:⚠️ Potential issue | 🟠 MajorKeep shipping state in sync when coupon handlers replace the order.
ShippingMethodSectionrenders fromshipments, not fromorder.shipments. These handlers only callsetOrder(result.order), so applying or removing a coupon can leave shipping rates and selection UI showing the previous order until another reload happens.💡 Suggested fix
const handleApplyCoupon = useCallback(async (code: string) => { const currentOrder = orderRef.current; if (!currentOrder) return { success: false, error: "No order" }; const result = await applyCouponCode(currentOrder.id, code); if (result.success && result.order) { setOrder(result.order); + setShipments(result.order.shipments || []); } return result; }, []); const handleRemoveCoupon = useCallback(async (couponCode: string) => { const currentOrder = orderRef.current; if (!currentOrder) return { success: false, error: "No order" }; const result = await removeCouponCode(currentOrder.id, couponCode); if (result.success && result.order) { setOrder(result.order); + setShipments(result.order.shipments || []); } return result; }, []);Also applies to: 583-590
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx around lines 107 - 127, handleApplyCoupon and handleRemoveCoupon update the order via setOrder(result.order) but don't sync the separate shipping state used by ShippingMethodSection (which renders from shipments), causing UI desync; after successfully setting the order in both handleApplyCoupon and handleRemoveCoupon, also update the shipping state to reflect the new order (e.g., call the existing setter that manages shipments or selected shipment with result.order.shipments and any selected shipment id) so shipments-based UI is replaced with the updated result.order.shipments whenever a coupon changes the order.
🤖 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/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 237-279: The blur autosave (handleAutoSave) can run concurrently
with validateAndPay allowing a race; ensure payment waits for any in-flight
autosave by tracking the autosave promise or a flag (e.g., inFlightAutoSaveRef
or isAutoSaving state) inside handleAutoSave and await it from validateAndPay
(and the Pay button handler) before proceeding; update references:
handleAutoSave, validateAndPay, orderRef/current usage, and the Pay now click
handler to check/await the autosave completion and prevent starting payment
while autosave is active.
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 204-213: Effect only updates mounted Elements via
gatewayHandleRef.current and skips saved-card flows where
gatewayHandleRef.current is null, leaving clientSecret/paymentSessionId stale;
update the effect to, when initRef.current is true and order.total changes and
gatewayHandleRef.current is null, call the same payment-session refresh logic
used to obtain a new clientSecret/paymentSessionId (the same code path used by
submit() or the existing session-creation helper) so coupon/shipping changes
regenerate the payment session for saved-card flows; reference initRef,
lastTotalRef, gatewayHandleRef, clientSecret, paymentSessionId and submit when
locating where to invoke the refresh.
---
Outside diff comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 107-127: handleApplyCoupon and handleRemoveCoupon update the order
via setOrder(result.order) but don't sync the separate shipping state used by
ShippingMethodSection (which renders from shipments), causing UI desync; after
successfully setting the order in both handleApplyCoupon and handleRemoveCoupon,
also update the shipping state to reflect the new order (e.g., call the existing
setter that manages shipments or selected shipment with result.order.shipments
and any selected shipment id) so shipments-based UI is replaced with the updated
result.order.shipments whenever a coupon changes the order.
🪄 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: 5a495152-3bdb-4478-b531-546a40261139
📒 Files selected for processing (6)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/components/checkout/AddressFormFields.tsxsrc/components/checkout/AddressSection.tsxsrc/components/checkout/CouponCode.tsxsrc/components/checkout/PaymentSection.tsxsrc/components/checkout/StripePaymentForm.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/data/payment.ts (2)
27-35:⚠️ Potential issue | 🟡 MinorUnused
cartIdparameter.Similar to
createCheckoutPaymentSession, thecartIdparameter is declared but not used in the function body.🔧 Suggested fix
export async function completeCheckoutPaymentSession( - cartId: string, sessionId: string, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/payment.ts` around lines 27 - 35, The cartId parameter on completeCheckoutPaymentSession is unused; either remove it from the function signature (and update any callers) or use it meaningfully (for example validate or pass it into completePaymentSession). If cartId is not needed, delete the parameter from completeCheckoutPaymentSession; if it is required, update the function to call completePaymentSession(sessionId, cartId) or perform cart-related validation/lookup before returning, and ensure callers are adjusted accordingly.
11-25:⚠️ Potential issue | 🟡 MinorUnused
cartIdparameter.The
cartIdparameter is declared but never used in the function body. This appears to be dead code from the refactor. Either use it when creating the payment session or remove it to avoid confusion.🔧 Suggested fix
export async function createCheckoutPaymentSession( - cartId: string, paymentMethodId: string, stripePaymentMethodId?: string, ) {Or if the parameter is needed for future use, add a comment explaining why.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/payment.ts` around lines 11 - 25, The cartId parameter on createCheckoutPaymentSession is unused and should be removed or actually passed into the createPaymentSession payload; either delete cartId from the function signature and update all callers of createCheckoutPaymentSession, or include cartId in the createPaymentSession call (e.g., add cart_id: cartId to the object passed to createPaymentSession) and, if you keep cartId for future use, add a short comment explaining why; adjust any call sites accordingly to match the new signature or behavior.
♻️ Duplicate comments (3)
src/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsx (1)
48-66:⚠️ Potential issue | 🟠 MajorVerify the returned order matches the requested
cartId.The code trusts
result.orderwithout verifying it corresponds to thecartIdparameter. IfcompleteCheckoutOrderinternally relies on session state or returns a different order, the confirmation page could display/track the wrong order. The double castas unknown as Cartalso suggests a type mismatch between the returned order and theCarttype.🛡️ Suggested fix: Add order ID verification
if (result.success) { const orderData = result.order as unknown as Cart; + // Verify we got the expected order + if (orderData?.id !== cartId) { + setError("Order mismatch. Please contact support."); + setLoading(false); + return; + } setOrder(orderData); try { trackPurchase(orderData);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx around lines 48 - 66, The code uses completeCheckoutOrder and blindly trusts result.order (cast via "as unknown as Cart")—verify the returned order's id matches the requested cartId before calling setOrder or trackPurchase: after receiving result, check result.order?.id (or the appropriate id field on the returned object) equals cartId, and if not treat it as an error (setError and do not setOrder or call trackPurchase); replace the unsafe double-cast with a simple runtime type check or guard that ensures required Cart fields exist before calling setOrder.src/components/checkout/PaymentSection.tsx (1)
214-223:⚠️ Potential issue | 🟠 MajorPayment session not refreshed for saved card flows when total changes.
When a saved card is selected (
gatewayHandleRef.currentisnull), this effect only callsfetchUpdates()on an existing Elements instance. Coupon or shipping changes that modifycart.totalwon't recreate the payment session, leavingclientSecretandpaymentSessionIdtied to the previous total.🔧 Suggested fix: Recreate session for saved cards on total change
useEffect(() => { if (!initRef.current) return; if (lastTotalRef.current === cart.total) return; lastTotalRef.current = cart.total; if (gatewayHandleRef.current) { gatewayHandleRef.current.fetchUpdates(); + return; } + + // For saved card flows, recreate the session with new total + createSession(selectedCardRef.current); - }, [cart.total]); + }, [cart.total, createSession]);🤖 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 214 - 223, The effect currently only calls gatewayHandleRef.current.fetchUpdates() when a gateway Elements instance exists, so changes to cart.total don't refresh the payment session for saved-card flows; modify the useEffect that reads initRef.current, lastTotalRef.current and gatewayHandleRef.current so that after setting lastTotalRef.current = cart.total it calls fetchUpdates() if gatewayHandleRef.current is present, otherwise invoke the payment session creation logic used during initialization (the same function that creates the clientSecret/paymentSessionId on mount—e.g., your createPaymentSession/initPaymentSession routine) to recreate the session for saved-card flows; keep the initRef and lastTotalRef checks as-is.src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx (1)
236-277:⚠️ Potential issue | 🟠 MajorPayment can still race with in-flight autosave.
handleAutoSaveis async, butvalidateAndPayand the Pay button only gate onprocessing. This allows payment to start before the latest blur save settles.🛠️ Proposed fix (track and await autosave)
+ const inFlightAutoSaveRef = useRef<Promise<void> | null>(null); const handleAutoSave = useCallback( async (addressData: { email: string; ship_address?: AddressParams; ship_address_id?: string; }) => { - const currentOrder = cartRef.current; - if (!currentOrder) return; - - setSaving(true); - setError(null); - - try { - const updateResult = await updateOrderAddresses(currentOrder.id, { - email: addressData.email, - ...(addressData.ship_address && { - ship_address: addressData.ship_address, - }), - ...(addressData.ship_address_id && { - ship_address_id: addressData.ship_address_id, - }), - }); - ... - } finally { - setSaving(false); - } + const run = (async () => { + const currentOrder = cartRef.current; + if (!currentOrder) return; + setSaving(true); + setError(null); + try { + const updateResult = await updateOrderAddresses(currentOrder.id, { + email: addressData.email, + ...(addressData.ship_address && { ship_address: addressData.ship_address }), + ...(addressData.ship_address_id && { ship_address_id: addressData.ship_address_id }), + }); + ... + } finally { + setSaving(false); + } + })(); + + inFlightAutoSaveRef.current = run; + try { + await run; + } finally { + if (inFlightAutoSaveRef.current === run) inFlightAutoSaveRef.current = null; + } }, [], ); const validateAndPay = async () => { if (!cart) return; + if (inFlightAutoSaveRef.current) { + await inFlightAutoSaveRef.current; + } ... }; - disabled={processing} + disabled={processing || saving}Also applies to: 431-490, 606-610
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx around lines 236 - 277, handleAutoSave can be in-flight while validateAndPay starts payment because the payment gating only checks processing; fix by tracking the autosave promise and ensuring payment waits for it: store the current handleAutoSave promise in a ref (e.g., autosavePromiseRef) or mirror saving state in a ref and, inside validateAndPay (and wherever the Pay button triggers), await autosavePromiseRef.current or block when saving is true before proceeding; update the Pay button disabled logic to consider both processing and saving, and clear/reset autosavePromiseRef when the handleAutoSave finally resolves or rejects so subsequent payments proceed correctly.
🧹 Nitpick comments (3)
src/components/checkout/Summary.tsx (1)
68-80: Consider defensive parsing for potentially undefined totals.
parseFloat()returnsNaNwhen givenundefinedor an empty string, which would cause the conditions to evaluate unexpectedly. The analytics module (src/lib/analytics/gtm.ts) uses asafeParseFloathelper for this reason.🛡️ Optional: Add fallback for safer parsing
- {parseFloat(cart.promo_total) !== 0 && ( + {parseFloat(cart.promo_total || "0") !== 0 && ( <div className="flex justify-between text-sm"> <span className="text-gray-700">Discount</span> <span className="text-green-700">{cart.display_promo_total}</span> </div> )} - {parseFloat(cart.tax_total) > 0 && ( + {parseFloat(cart.tax_total || "0") > 0 && ( <div className="flex justify-between text-sm"> <span className="text-gray-700">Tax</span> <span className="text-gray-900">{cart.display_tax_total}</span> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/Summary.tsx` around lines 68 - 80, The conditional checks in Summary.tsx use parseFloat on cart.promo_total and cart.tax_total which can return NaN for undefined/empty values; update the conditions to use a defensive parser (reuse the existing safeParseFloat from src/lib/analytics/gtm.ts or add a local helper) when evaluating values for display and comparison, e.g., replace parseFloat(cart.promo_total) and parseFloat(cart.tax_total) with safeParseFloat(cart.promo_total) and safeParseFloat(cart.tax_total) and ensure the same safe parser is used when deciding to render {cart.display_promo_total} and {cart.display_tax_total} so undefined input won’t cause unexpected rendering.src/components/checkout/CouponCode.tsx (1)
26-41: Harden async handlers withtry/catch/finallyto avoid stuck UI state.If
onApplyoronRemoverejects,applying/removingmay stay set and block interaction. Add guarded cleanup infinally.♻️ Proposed refactor
const handleApply = async (e: React.FormEvent) => { e.preventDefault(); if (!code.trim()) return; setApplying(true); setError(null); - const result = await onApply(code.trim()); - if (result.success) { - setCode(""); - } else { - setError(result.error || "Invalid coupon code"); - } - - setApplying(false); + try { + const result = await onApply(code.trim()); + if (result.success) { + setCode(""); + } else { + setError(result.error || "Invalid coupon code"); + } + } catch { + setError("Failed to apply coupon code"); + } finally { + setApplying(false); + } }; const handleRemove = async (code: string) => { setRemoving(code); setError(null); - const result = await onRemove(code); - if (!result.success) { - setError(result.error || "Failed to remove coupon code"); - } - - setRemoving(null); + try { + const result = await onRemove(code); + if (!result.success) { + setError(result.error || "Failed to remove coupon code"); + } + } catch { + setError("Failed to remove coupon code"); + } finally { + setRemoving(null); + } };Also applies to: 43-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkout/CouponCode.tsx` around lines 26 - 41, The async handlers handleApply and handleRemove must be guarded with try/catch/finally so a rejected promise won't leave applying/removing true; wrap the await onApply(code.trim()) and await onRemove(...) calls in a try block, set success/clear code inside the try when result.success, catch exceptions to call setError(err.message || String(err) || "Invalid coupon code"), and always reset setApplying(false) / setRemoving(false) in a finally block; keep existing logic that uses result.success to clear code or set result.error but move state resets into finally to guarantee cleanup.src/lib/data/checkout.ts (1)
29-92: Add explicit return types on exported server actions.Multiple changed exported functions still rely on inferred return types. Please declare explicit return types for API clarity and stricter contracts.
As per coding guidelines,
Use strict TypeScript type checking; define explicit return types for functions and avoid 'any' type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/checkout.ts` around lines 29 - 92, Add explicit TypeScript return types for the exported server action functions (updateOrderAddresses, updateOrderMarket, getShipments, selectShippingRate, applyCouponCode, removeCouponCode, completeOrder) instead of relying on inference; update each signature to return the precise Promise type (for example, Promise<ActionResult<{ cart: Cart }>> for functions that return { cart } and Promise<Shipment[]> or Promise<WithFallback<Shipment[]>> for getShipments) by importing/using the existing ActionResult/WithFallback/Cart/Shipment types from your types module, and avoid any usage of `any`. Ensure the declared return types match the actual returned values inside actionResult/withFallback wrappers.
🤖 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/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 98-99: The component seeds const [error, setError] =
useState<string | null>(paymentError) but loadOrder later unconditionally clears
it, so payment_error never surfaces; update loadOrder (or the code path that
calls setError around lines mentioning loadOrder) to avoid overwriting an
existing paymentError—only call setError(null) when you are intentionally
clearing prior non-payment errors or when paymentError is not present, or
alternatively preserve the initial paymentError by checking if error is already
set before clearing; reference the state variables error and setError and the
loadOrder function when making this change.
In `@src/components/checkout/PaymentSection.tsx`:
- Line 304: In PaymentSection where returnUrl is constructed (the line setting
const returnUrl =
`${window.location.origin}${window.location.pathname.replace(/\/checkout\/.*/,
`/order-placed/${order.id}`)}`)` replace the undefined variable order with the
correct cart reference (e.g., `/order-placed/${cart.id}`) and guard against a
missing cart by using a safe access or fallback (e.g., `${cart?.id || ''}` or
return early if cart is required) so the build no longer fails due to an
undefined identifier.
In `@src/lib/data/payment.ts`:
- Around line 65-76: The code calls getCart() without using the cartId parameter
so it may load the wrong session cart; update the logic to fetch/validate by ID:
call getCart(cartId) if the helper supports an ID (or use a dedicated
fetchCartById(cartId)), then check that returned cart.id === cartId (or treat
null/mismatch as absent) before using cart.current_step; keep the existing
fallback of calling complete(cartId) when no matching cart is found. Ensure you
update references to getCart() and the subsequent cart null / cart.current_step
checks accordingly.
---
Outside diff comments:
In `@src/lib/data/payment.ts`:
- Around line 27-35: The cartId parameter on completeCheckoutPaymentSession is
unused; either remove it from the function signature (and update any callers) or
use it meaningfully (for example validate or pass it into
completePaymentSession). If cartId is not needed, delete the parameter from
completeCheckoutPaymentSession; if it is required, update the function to call
completePaymentSession(sessionId, cartId) or perform cart-related
validation/lookup before returning, and ensure callers are adjusted accordingly.
- Around line 11-25: The cartId parameter on createCheckoutPaymentSession is
unused and should be removed or actually passed into the createPaymentSession
payload; either delete cartId from the function signature and update all callers
of createCheckoutPaymentSession, or include cartId in the createPaymentSession
call (e.g., add cart_id: cartId to the object passed to createPaymentSession)
and, if you keep cartId for future use, add a short comment explaining why;
adjust any call sites accordingly to match the new signature or behavior.
---
Duplicate comments:
In `@src/app/`[country]/[locale]/(checkout)/checkout/[id]/page.tsx:
- Around line 236-277: handleAutoSave can be in-flight while validateAndPay
starts payment because the payment gating only checks processing; fix by
tracking the autosave promise and ensuring payment waits for it: store the
current handleAutoSave promise in a ref (e.g., autosavePromiseRef) or mirror
saving state in a ref and, inside validateAndPay (and wherever the Pay button
triggers), await autosavePromiseRef.current or block when saving is true before
proceeding; update the Pay button disabled logic to consider both processing and
saving, and clear/reset autosavePromiseRef when the handleAutoSave finally
resolves or rejects so subsequent payments proceed correctly.
In `@src/app/`[country]/[locale]/(checkout)/order-placed/[id]/page.tsx:
- Around line 48-66: The code uses completeCheckoutOrder and blindly trusts
result.order (cast via "as unknown as Cart")—verify the returned order's id
matches the requested cartId before calling setOrder or trackPurchase: after
receiving result, check result.order?.id (or the appropriate id field on the
returned object) equals cartId, and if not treat it as an error (setError and do
not setOrder or call trackPurchase); replace the unsafe double-cast with a
simple runtime type check or guard that ensures required Cart fields exist
before calling setOrder.
In `@src/components/checkout/PaymentSection.tsx`:
- Around line 214-223: The effect currently only calls
gatewayHandleRef.current.fetchUpdates() when a gateway Elements instance exists,
so changes to cart.total don't refresh the payment session for saved-card flows;
modify the useEffect that reads initRef.current, lastTotalRef.current and
gatewayHandleRef.current so that after setting lastTotalRef.current = cart.total
it calls fetchUpdates() if gatewayHandleRef.current is present, otherwise invoke
the payment session creation logic used during initialization (the same function
that creates the clientSecret/paymentSessionId on mount—e.g., your
createPaymentSession/initPaymentSession routine) to recreate the session for
saved-card flows; keep the initRef and lastTotalRef checks as-is.
---
Nitpick comments:
In `@src/components/checkout/CouponCode.tsx`:
- Around line 26-41: The async handlers handleApply and handleRemove must be
guarded with try/catch/finally so a rejected promise won't leave
applying/removing true; wrap the await onApply(code.trim()) and await
onRemove(...) calls in a try block, set success/clear code inside the try when
result.success, catch exceptions to call setError(err.message || String(err) ||
"Invalid coupon code"), and always reset setApplying(false) / setRemoving(false)
in a finally block; keep existing logic that uses result.success to clear code
or set result.error but move state resets into finally to guarantee cleanup.
In `@src/components/checkout/Summary.tsx`:
- Around line 68-80: The conditional checks in Summary.tsx use parseFloat on
cart.promo_total and cart.tax_total which can return NaN for undefined/empty
values; update the conditions to use a defensive parser (reuse the existing
safeParseFloat from src/lib/analytics/gtm.ts or add a local helper) when
evaluating values for display and comparison, e.g., replace
parseFloat(cart.promo_total) and parseFloat(cart.tax_total) with
safeParseFloat(cart.promo_total) and safeParseFloat(cart.tax_total) and ensure
the same safe parser is used when deciding to render {cart.display_promo_total}
and {cart.display_tax_total} so undefined input won’t cause unexpected
rendering.
In `@src/lib/data/checkout.ts`:
- Around line 29-92: Add explicit TypeScript return types for the exported
server action functions (updateOrderAddresses, updateOrderMarket, getShipments,
selectShippingRate, applyCouponCode, removeCouponCode, completeOrder) instead of
relying on inference; update each signature to return the precise Promise type
(for example, Promise<ActionResult<{ cart: Cart }>> for functions that return {
cart } and Promise<Shipment[]> or Promise<WithFallback<Shipment[]>> for
getShipments) by importing/using the existing
ActionResult/WithFallback/Cart/Shipment types from your types module, and avoid
any usage of `any`. Ensure the declared return types match the actual returned
values inside actionResult/withFallback wrappers.
🪄 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: 94ea973d-44ee-4c9b-8df5-b6c15d5a6a3e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
package.jsonsrc/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(checkout)/order-placed/[id]/page.tsxsrc/components/checkout/AddressFormFields.tsxsrc/components/checkout/AddressSection.tsxsrc/components/checkout/AddressSelector.tsxsrc/components/checkout/CouponCode.tsxsrc/components/checkout/PaymentSection.tsxsrc/components/checkout/Summary.tsxsrc/components/checkout/index.tssrc/lib/data/checkout.tssrc/lib/data/payment.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/data/payment.ts (2)
27-35:⚠️ Potential issue | 🟠 MajorUnused
cartIdparameter is misleading.Same issue as
createCheckoutPaymentSession— thecartIdparameter is accepted but never used. OnlysessionIdis passed tocompletePaymentSession. Either use the parameter, remove it, or prefix with underscore if intentionally unused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/payment.ts` around lines 27 - 35, The function completeCheckoutPaymentSession currently accepts a cartId parameter that is never used; update the implementation to either use cartId in the logic, remove the cartId parameter from the function signature, or explicitly mark it unused by prefixing it with an underscore (e.g., _cartId) to make intent clear. Locate the completeCheckoutPaymentSession function and choose one of the three fixes: (A) pass cartId where needed or validate it before calling completePaymentSession(sessionId), (B) remove cartId from the signature and all callers, or (C) rename cartId to _cartId to indicate it is intentionally unused; ensure consistency with the related createCheckoutPaymentSession function.
11-25:⚠️ Potential issue | 🟠 MajorUnused
cartIdparameter is misleading.The
cartIdparameter is accepted but never used in the function body. Callers passcart.idexpecting the session to be created for that specific cart, butcreatePaymentSessionis called without any cart context. This could cause issues if the session cart differs from the caller's cart.Either pass
cartIdto the underlying SDK call if supported, remove the parameter entirely, or prefix with underscore (_cartId) if intentionally unused to indicate reliance on session-based cart resolution. As per coding guidelines, "Remove unused variables and imports; prefix intentionally unused parameters with underscore if required by type signature."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/payment.ts` around lines 11 - 25, The createCheckoutPaymentSession function currently accepts cartId but never uses it; update the implementation to either (1) forward cartId into the createPaymentSession call (e.g. add a cart_id or cartId property in the payload) if the SDK supports creating a session for a specific cart, or (2) remove the cartId parameter from createCheckoutPaymentSession and all callers if cart context is resolved elsewhere, or (3) if the parameter must remain for signature compatibility, rename it to _cartId to signal it is intentionally unused; locate the function createCheckoutPaymentSession and the createPaymentSession call to apply one of these fixes.
🧹 Nitpick comments (1)
src/lib/data/payment.ts (1)
62-64: Consider using Spree SDK types instead ofunknown.The return type uses
unknownfor the order object. For better type safety, consider usingStoreOrderfrom the Spree SDK.+import type { StoreOrder } from "@spree/next"; + export async function confirmPaymentAndCompleteCart( cartId: string, sessionId?: string, ): Promise< - { success: true; order: unknown } | { success: false; error: string } + { success: true; order: StoreOrder } | { success: false; error: string } >As per coding guidelines, "Import and use Spree SDK types (StoreProduct, StoreVariant, StoreOrder, StoreLineItem, PaginatedResponse) for type safety."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/payment.ts` around lines 62 - 64, The return type for the function in src/lib/data/payment.ts currently uses "unknown" for the order; import the Spree SDK StoreOrder type and replace the unknown with StoreOrder in the Promise union so the signature becomes Promise<{ success: true; order: StoreOrder } | { success: false; error: string }>, and ensure you add the StoreOrder import from the Spree SDK at the top of the file (refer to the function name or export in this file to locate the signature to update).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/data/payment.ts`:
- Around line 27-35: The function completeCheckoutPaymentSession currently
accepts a cartId parameter that is never used; update the implementation to
either use cartId in the logic, remove the cartId parameter from the function
signature, or explicitly mark it unused by prefixing it with an underscore
(e.g., _cartId) to make intent clear. Locate the completeCheckoutPaymentSession
function and choose one of the three fixes: (A) pass cartId where needed or
validate it before calling completePaymentSession(sessionId), (B) remove cartId
from the signature and all callers, or (C) rename cartId to _cartId to indicate
it is intentionally unused; ensure consistency with the related
createCheckoutPaymentSession function.
- Around line 11-25: The createCheckoutPaymentSession function currently accepts
cartId but never uses it; update the implementation to either (1) forward cartId
into the createPaymentSession call (e.g. add a cart_id or cartId property in the
payload) if the SDK supports creating a session for a specific cart, or (2)
remove the cartId parameter from createCheckoutPaymentSession and all callers if
cart context is resolved elsewhere, or (3) if the parameter must remain for
signature compatibility, rename it to _cartId to signal it is intentionally
unused; locate the function createCheckoutPaymentSession and the
createPaymentSession call to apply one of these fixes.
---
Nitpick comments:
In `@src/lib/data/payment.ts`:
- Around line 62-64: The return type for the function in src/lib/data/payment.ts
currently uses "unknown" for the order; import the Spree SDK StoreOrder type and
replace the unknown with StoreOrder in the Promise union so the signature
becomes Promise<{ success: true; order: StoreOrder } | { success: false; error:
string }>, and ensure you add the StoreOrder import from the Spree SDK at the
top of the file (refer to the function name or export in this file to locate the
signature to update).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 076894ab-a280-4a4e-bcf2-75a099067adf
📒 Files selected for processing (1)
src/lib/data/payment.ts
… the initial loadOrder call so the user actually sees the error message when redirected back from a failed offsite payment
This was referenced Mar 17, 2026
Merged
Merged
This was referenced Mar 24, 2026
Merged
Merged
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.
TODO
Summary by CodeRabbit
New Features
Style
Bug Fixes