Use dynamic checkout steps on checkout#54
Conversation
Every order can have different checkout steps (eg. physical items require shipment), also fixed using next vs advance
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughReplaces hardcoded checkout step handling with dynamic step derivation from order.checkout_steps and introduces Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant CheckoutPage
participant CheckoutService
participant SpreeNext
Browser->>CheckoutPage: Load order (id)
CheckoutPage->>CheckoutService: fetchOrder(id)
CheckoutService->>SpreeNext: getOrder(...)
SpreeNext-->>CheckoutService: order data (state, checkout_steps)
CheckoutService-->>CheckoutPage: order
CheckoutPage->>CheckoutPage: getCheckoutStep(order.state, order.checkout_steps)
Browser->>CheckoutPage: Click "Next" / submit step
CheckoutPage->>CheckoutService: nextCheckoutStep(order.id)
CheckoutService->>SpreeNext: next(order.id)
SpreeNext-->>CheckoutService: updated order
CheckoutService-->>CheckoutPage: updated order/result
CheckoutPage->>Browser: render updated step/state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/data/__tests__/checkout.test.ts (1)
158-178: Add the non-Errorrejection case fornextCheckoutStep.Every other
actionResultwrapper in this file asserts the fallback-message branch, but this suite only coversErrorinstances. Adding the"unexpected"case will pin"Failed to advance checkout"and keep the new action aligned with the existing coverage pattern.🧪 Suggested test
it("returns error on failure", async () => { mockNext.mockRejectedValue(new Error("Cannot advance")); const result = await nextCheckoutStep("order-1"); expect(result).toEqual({ success: false, error: "Cannot advance", }); }); + + it("returns fallback message for non-Error throws", async () => { + mockNext.mockRejectedValue("unexpected"); + + const result = await nextCheckoutStep("order-1"); + + expect(result).toEqual({ + success: false, + error: "Failed to advance checkout", + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/__tests__/checkout.test.ts` around lines 158 - 178, Add a test that covers non-Error rejections for nextCheckoutStep by making mockNext reject with a non-Error value (e.g., a string or null) and asserting the actionResult falls back to the generic message "Failed to advance checkout"; locate the suite using describe("nextCheckoutStep") and the mock named mockNext to add an it(...) case that calls nextCheckoutStep("order-1") and expects { success: false, error: "Failed to advance checkout" } to mirror the other actionResult fallback tests.
🤖 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 46-50: The navigation logic is using hardcoded step ids instead of
the dynamic visible steps, which causes currentStep to be outside the rendered
steps (e.g., getCheckoutStep and goToStep returning "delivery" when that step
was filtered out); update getCheckoutStep and any back/forward handlers (e.g.,
goToStep, the code paths around currentStep/currentStepIndex) to compute the
current and previous steps by using the same filtered checkoutSteps array you
render (use checkoutSteps.indexOf(orderState) and Math.max(0, index-1) to pick
previous, or fallback to checkoutSteps[0] when index is -1), and ensure any step
transitions always use checkoutSteps[n] from that filtered array rather than
hardcoded ids like "delivery" or "payment".
---
Nitpick comments:
In `@src/lib/data/__tests__/checkout.test.ts`:
- Around line 158-178: Add a test that covers non-Error rejections for
nextCheckoutStep by making mockNext reject with a non-Error value (e.g., a
string or null) and asserting the actionResult falls back to the generic message
"Failed to advance checkout"; locate the suite using
describe("nextCheckoutStep") and the mock named mockNext to add an it(...) case
that calls nextCheckoutStep("order-1") and expects { success: false, error:
"Failed to advance checkout" } to mirror the other actionResult fallback tests.
🪄 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: b532d83f-cb01-4189-b490-a160f964a9b5
📒 Files selected for processing (4)
src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsxsrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/lib/data/__tests__/checkout.test.tssrc/lib/data/checkout.ts
- Derive previous step from visible steps array instead of hardcoded IDs - Make onBack optional on DeliveryStep/PaymentStep for edge cases where a step has no predecessor (e.g. digital-only orders) - Add non-Error rejection test for nextCheckoutStep Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Every order can have different checkout steps (eg. physical items require shipment), also fixed using next vs advance
Summary by CodeRabbit
New Features
Refactor
Style
Tests