(SP: 1) [Cart] adding route for user orders to cart page#337
(SP: 1) [Cart] adding route for user orders to cart page#337ViktorSvertoka merged 21 commits intodevelopfrom
Conversation
…ebhook + janitor (ORIGIN_BLOCKED)
…wnership test and pass all pre-prod invariants
…ests; minor security/log/UI cleanups
…s) and remove duplicate sha256 hashing
liudmylasovetovs
requested review from
AM1007 and
ViktorSvertoka
as code owners
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
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:
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds a server API to return authenticated user orders, a client-side orders summary and Orders card on the cart page, per-user cart storage and CartProvider owner scoping, a DB index and migration for orders(user_id, created_at), a drizzle-kit devDependency bump, and related translation strings. Changes
Sequence DiagramsequenceDiagram
participant User
participant CartClient as Cart Page Client
participant OrdersAPI as Orders API (/api/shop/orders)
participant DB as Database
User->>CartClient: Open cart page
CartClient->>CartClient: set isClientReady, isOrdersLoading
CartClient->>OrdersAPI: GET /api/shop/orders (AbortController, 2500ms)
OrdersAPI->>OrdersAPI: getCurrentUser() (auth)
alt unauthenticated
OrdersAPI-->>CartClient: 401/403 (no-store)
CartClient->>CartClient: clear ordersSummary, stop loading
else authenticated
OrdersAPI->>DB: query orders JOIN orderItems (aggregate label, count)
DB-->>OrdersAPI: rows
OrdersAPI-->>CartClient: 200 JSON (orders summary)
CartClient->>CartClient: update ordersSummary, stop loading
CartClient->>User: render Orders card (count, link)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e694a46ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) : null} | ||
| </div> | ||
| ) : null; | ||
| if (!isClientReady || !hasLoadedOrdersSummary) { |
There was a problem hiding this comment.
CartPageClient blocks all cart content behind the loader until hasLoadedOrdersSummary becomes true, but that flag is only flipped after the /api/shop/orders request settles (including timeout/abort in the effect above). In slow/error cases, this adds up to a 2.5s delay to viewing cart items and starting checkout for a non-essential summary call, which regresses the primary cart flow; render the cart independently and load the orders card asynchronously.
Useful? React with 👍 / 👎.
| try { | ||
| const user = await getCurrentUser(); | ||
| if (!user) { | ||
| logWarn('public_orders_list_unauthorized', { |
There was a problem hiding this comment.
This route logs public_orders_list_unauthorized as a warning for every unauthenticated request, but the cart page now calls /api/shop/orders on mount and explicitly treats 401/403 as normal public behavior. That means routine anonymous cart traffic will emit warning-level logs, creating alert noise and obscuring real failures; this path should be downgraded (or skipped) for expected anonymous access.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
frontend/app/api/shop/orders/route.ts (1)
46-59: Consider reducing log level for expected unauthenticated requests.The cart page is public, so unauthenticated hits to this endpoint are expected and frequent.
logWarnat Line 49 may generate significant noise in production logs. Adebugorinfolevel (or skipping the log entirely) would be more appropriate for a non-anomalous code path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/shop/orders/route.ts` around lines 46 - 59, The review points out that using logWarn for expected unauthenticated accesses floods production logs; update the handler in route.ts so when getCurrentUser() returns null it uses a lower-severity logging call (e.g., logDebug or logInfo) or omits logging entirely instead of logWarn, keeping the same metadata (baseMeta, code: 'UNAUTHORIZED', durationMs) and still returning the existing noStoreJson response; locate the block that calls getCurrentUser() and replace the logWarn invocation accordingly.frontend/app/[locale]/shop/cart/CartPageClient.tsx (4)
133-183: AbortController not aborted on unmount — fetch continues in background.The cleanup function sets
cancelled = trueto prevent state updates, but theAbortControlleris scoped insideloadOrdersSummaryand never aborted on unmount. If the user navigates away quickly, the request stays in flight.Suggested fix
useEffect(() => { let cancelled = false; + const controller = new AbortController(); async function loadOrdersSummary() { setIsOrdersLoading(true); - const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 2500); try { const res = await fetch('/api/shop/orders', { method: 'GET', headers: { Accept: 'application/json' }, cache: 'no-store', signal: controller.signal, }); // ... rest unchanged } finally { clearTimeout(timeoutId); if (!cancelled) { setIsOrdersLoading(false); setHasLoadedOrdersSummary(true); } } } void loadOrdersSummary(); return () => { cancelled = true; + controller.abort(); }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 133 - 183, The fetch's AbortController is created inside loadOrdersSummary and never aborted on component unmount, allowing the request to continue; refactor so the controller is accessible in the outer scope (e.g., a ref or variable captured by the effect) and call controller.abort() in the cleanup returned by the useEffect, while preserving the cancelled flag and existing finally logic in loadOrdersSummary; ensure the fetch uses controller.signal and that aborting doesn't attempt state updates (keep the cancelled checks in setOrdersSummary/setIsOrdersLoading/setHasLoadedOrdersSummary).
318-327: Spinner insideordersCardis dead code.
ordersCardis only rendered whenordersSummaryis non-null (Line 311).ordersSummaryis set in the same async execution frame assetIsOrdersLoading(false)(Lines 165 → 172), so React batches them. By the timeordersCardrenders,isOrdersLoadingis alreadyfalse; the<Loader2>branch is unreachable.Either remove the spinner or, if you want a loading state for the orders card, render the card shell before
ordersSummaryis available and show the spinner there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 318 - 327, The Loader2 branch inside the ordersCard is dead because ordersCard only mounts when ordersSummary is non-null and state updates are batched; remove the unreachable spinner branch or change the render logic so the card shell mounts before ordersSummary is set and shows the spinner while isOrdersLoading is true. Locate the JSX that renders ordersCard (references: ordersCard, ordersSummary, isOrdersLoading, setIsOrdersLoading) and either a) simplify the conditional to always render the badge with ordersSummary.count (removing Loader2), or b) move the parent conditional so the card renders when isOrdersLoading is true and show Loader2 until ordersSummary is populated. Ensure you update any aria-live attributes accordingly.
303-309: Hoist constant class-name strings outside the component.
ORDERS_LINK,ORDERS_COUNT_BADGE, andORDERS_CARDare staticcn(…)calls recomputed on every render. All similar constants in this file (SHOP_PRODUCT_LINK,SHOP_STEPPER_BTN,SHOP_HERO_CTA) are already defined at module scope. Move these to match the existing pattern.Suggested diff
const SHOP_HERO_CTA = cn( ... ); + +const ORDERS_LINK = cn(SHOP_LINK_BASE, SHOP_LINK_MD, SHOP_FOCUS); + +const ORDERS_COUNT_BADGE = cn( + 'border-border bg-muted/40 text-foreground inline-flex items-center rounded-full border px-2.5 py-1 text-xs font-semibold tabular-nums' +); + +const ORDERS_CARD = cn('border-border rounded-md border p-4'); // ... inside the component, remove the three local declarations🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 303 - 309, ORDERS_LINK, ORDERS_COUNT_BADGE and ORDERS_CARD are being recomputed inside the CartPageClient component; hoist them to module scope like the existing SHOP_PRODUCT_LINK, SHOP_STEPPER_BTN and SHOP_HERO_CTA constants so they are computed once at import time. Locate the cn(...) definitions (symbols: ORDERS_LINK, ORDERS_COUNT_BADGE, ORDERS_CARD) in CartPageClient.tsx and move their const declarations to the top of the file alongside the other SHOP_* constants, leaving the component to reference those module-scoped constants instead.
150-151: Early return on 401/403 skips settingordersSummary— confirm this is intentional for the loading gate.When the user is unauthenticated, the function returns early at Line 151 without calling
setOrdersSummary, soordersSummarystaysnullandordersCardisnull. Thefinallyblock still fires and setshasLoadedOrdersSummary = true, which correctly unblocks the gate. This path works, but it's fragile: any new early-return added later without afinallycompanion would leave the page stuck on the loader forever.Consider restructuring so the gate doesn't depend on this fetch at all (per the major issue above).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 150 - 151, The early return on res.status 401/403 skips setting ordersSummary which can make the UI fragile; change that branch to explicitly setOrdersSummary to an empty/explicit "no orders" value (e.g. [] or a well-formed empty OrdersSummary) before returning so ordersCard consumers get a known state, and keep the existing finally that sets hasLoadedOrdersSummary = true; locate the check around res.status === 401 || res.status === 403 in the fetch handler and update it to call setOrdersSummary(...) (not just return) so ordersSummary/ordersCard are deterministic even for unauthenticated responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx:
- Line 356: Replace the hardcoded "Loading…" string in the CartPageClient
component with a translation key (e.g., t('loading')): locate the JSX that
renders "Loading…" (in CartPageClient) and change it to use the translator
function t('loading'); if t is not in scope, import and call the project's i18n
hook (e.g., useTranslation or useLocaleTranslate) at the top of CartPageClient
and use its t function so the fallback/loading text is localized.
- Around line 346-361: The full-page loading gate that checks isClientReady &&
hasLoadedOrdersSummary should be removed so the cart UI renders immediately;
instead let ordersCard (which already handles ordersSummary === null by
returning null) manage its own loading/skeleton state. Keep any minimal gating
for isClientReady only if needed to avoid hydration flash (handle it separately
from the orders fetch), and ensure hasLoadedOrdersSummary does not block
rendering of cart items, checkout controls, or empty-state UI; move any
orders-summary-specific spinner into the ordersCard area so slow/401 responses
don’t show a full-page spinner.
In `@frontend/app/api/shop/orders/route.ts`:
- Around line 35-101: Add a composite index on orders.userId and
orders.createdAt to speed up the GET query that filters by userId and orders by
createdAt; in the table definition for orders in frontend/db/schema/shop.ts
(where orders and the existing orders_sweep_claim_expires_idx are declared) add
an index definition equivalent to idx_orders_user_id_created_at on (user_id,
created_at DESC) so the db can use it for the WHERE eq(orders.userId, user.id)
and ORDER BY desc(orders.createdAt) used in the GET handler.
---
Nitpick comments:
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx:
- Around line 133-183: The fetch's AbortController is created inside
loadOrdersSummary and never aborted on component unmount, allowing the request
to continue; refactor so the controller is accessible in the outer scope (e.g.,
a ref or variable captured by the effect) and call controller.abort() in the
cleanup returned by the useEffect, while preserving the cancelled flag and
existing finally logic in loadOrdersSummary; ensure the fetch uses
controller.signal and that aborting doesn't attempt state updates (keep the
cancelled checks in
setOrdersSummary/setIsOrdersLoading/setHasLoadedOrdersSummary).
- Around line 318-327: The Loader2 branch inside the ordersCard is dead because
ordersCard only mounts when ordersSummary is non-null and state updates are
batched; remove the unreachable spinner branch or change the render logic so the
card shell mounts before ordersSummary is set and shows the spinner while
isOrdersLoading is true. Locate the JSX that renders ordersCard (references:
ordersCard, ordersSummary, isOrdersLoading, setIsOrdersLoading) and either a)
simplify the conditional to always render the badge with ordersSummary.count
(removing Loader2), or b) move the parent conditional so the card renders when
isOrdersLoading is true and show Loader2 until ordersSummary is populated.
Ensure you update any aria-live attributes accordingly.
- Around line 303-309: ORDERS_LINK, ORDERS_COUNT_BADGE and ORDERS_CARD are being
recomputed inside the CartPageClient component; hoist them to module scope like
the existing SHOP_PRODUCT_LINK, SHOP_STEPPER_BTN and SHOP_HERO_CTA constants so
they are computed once at import time. Locate the cn(...) definitions (symbols:
ORDERS_LINK, ORDERS_COUNT_BADGE, ORDERS_CARD) in CartPageClient.tsx and move
their const declarations to the top of the file alongside the other SHOP_*
constants, leaving the component to reference those module-scoped constants
instead.
- Around line 150-151: The early return on res.status 401/403 skips setting
ordersSummary which can make the UI fragile; change that branch to explicitly
setOrdersSummary to an empty/explicit "no orders" value (e.g. [] or a
well-formed empty OrdersSummary) before returning so ordersCard consumers get a
known state, and keep the existing finally that sets hasLoadedOrdersSummary =
true; locate the check around res.status === 401 || res.status === 403 in the
fetch handler and update it to call setOrdersSummary(...) (not just return) so
ordersSummary/ordersCard are deterministic even for unauthenticated responses.
In `@frontend/app/api/shop/orders/route.ts`:
- Around line 46-59: The review points out that using logWarn for expected
unauthenticated accesses floods production logs; update the handler in route.ts
so when getCurrentUser() returns null it uses a lower-severity logging call
(e.g., logDebug or logInfo) or omits logging entirely instead of logWarn,
keeping the same metadata (baseMeta, code: 'UNAUTHORIZED', durationMs) and still
returning the existing noStoreJson response; locate the block that calls
getCurrentUser() and replace the logWarn invocation accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx:
- Around line 157-162: The current 401/403 handling sets ordersSummary to a
truthy object which causes the Orders card to render for unauthenticated users;
update the handler in CartPageClient.tsx (the fetch response branch using
res.status === 401 || res.status === 403) to call setOrdersSummary(null) instead
of setOrdersSummary({ count: 0, latestOrderId: null }) while preserving the
existing cancelled guard, so ordersSummary stays null and the ordersCard will
not render for unauthenticated visitors.
In `@frontend/app/api/shop/orders/route.ts`:
- Around line 61-101: The query in route.ts currently limits results to 50 and
the client reads the badge count from orders.length, so users with >50 orders
see "50"; modify the DB select to include a totalCount (e.g., add sql`count(*)
OVER()` as totalCount to the select projection that builds rows) or run a
separate count query and attach totalCount to the response object returned from
the handler that sets rows, and then update the client (CartPageClient.tsx) to
read data.totalCount (or response.totalCount) instead of orders.length; locate
the select that builds rows (variable rows using select with primaryItemLabel
and itemCount) and add the totalCount field there or add a paired count query in
the same route handler.
…lCount for orders badge
There was a problem hiding this comment.
🧹 Nitpick comments (5)
frontend/app/[locale]/shop/cart/CartPageClient.tsx (2)
368-376: Hardcoded "Loading…" was removed — but the loader still lacks accessible labeling.The full-page
<Loader>has no text oraria-labelfor screen readers. Consider adding a visually-hidden status announcement so assistive technology users know the page is loading.Suggested improvement
<div className="flex flex-col items-center justify-center gap-4"> <Loader size={160} className="opacity-90" /> + <span className="sr-only" role="status" aria-live="polite"> + {t('loading')} + </span> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 368 - 376, The Loader component inside the early return when !isClientReady lacks accessible labeling; add an offscreen/live status so screen readers announce loading (e.g., a visually-hidden text "Loading…" or role="status" with aria-live="polite") alongside the <Loader /> in CartPageClient.tsx to provide an accessible loading announcement for assistive tech users.
164-165: Silent discard on non-OK / malformed responses could hide server issues.Lines 164–165 silently bail on any non-OK status (other than 401/403) or unparseable body. This is fine for the UI, but there's no logging or telemetry, making it harder to diagnose issues like repeated 500s from the orders endpoint.
Consider at minimum a
console.warnfor unexpected status codes in development, or integrating with whatever client-side error reporting the platform uses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 164 - 165, The fetch handling in CartPageClient.tsx currently bails silently when responses are non-OK or the body is unparseable; update the logic around the res/data checks to surface unexpected server issues by emitting a warning or reporting telemetry (e.g., console.warn in development or calling the existing client-side error reporter) whenever res.ok is false (excluding 401/403) or when data is null/typeof !== 'object'; include res.status, res.statusText and the parsed/raw body (or the parse error) in the log/report to aid debugging and keep the early-return behavior for the UI intact (refer to the res and data variables in the current code block).frontend/app/api/shop/orders/route.ts (3)
94-94:itemCountlacks a SQL type annotation.
sql\count(${orderItems.id})`has no generic type parameter, so Drizzle infers it asunknown. This is safely handled bytoCount` on Line 116, but adding the annotation improves IDE support and self-documentation.Suggested fix
- itemCount: sql`count(${orderItems.id})`, + itemCount: sql<string>`count(${orderItems.id})`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/shop/orders/route.ts` at line 94, The selected field itemCount uses sql`count(${orderItems.id})` without a generic type so Drizzle infers unknown; add an explicit generic type to the sql template (e.g., number or bigint as appropriate for your DB) for sql`count(${orderItems.id})` so IDEs and typings work correctly and remain consistent with the subsequent toCount usage (see itemCount and the call to toCount).
60-65: Separate count query is fine, buttotalCounttype fromsql<number>may mislead.PostgreSQL
count(*)returnsbigint, which Drizzle typically delivers as astring. Thesql<number>generic is only a TypeScript hint and doesn't coerce the runtime value. This works correctly here only becausetoCounthandlesstring | bigint | number— but a future reader might trust the<number>annotation and skiptoCount.Consider annotating as
sql<string>to match the actual runtime type, keepingtoCountas the safe coercion layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/shop/orders/route.ts` around lines 60 - 65, The count query annotation is misleading: change the generic on sql from number to string so the expression using sql in totalCountRows better reflects PostgreSQL returning bigint as a string (e.g., replace sql<number>`count(*)` with sql<string>`count(*)`), keep using the toCount(...) call when deriving totalCount from totalCountRows[0]?.value to perform the safe coercion of string|bigint|number into a numeric count.
35-134: No success-path log emitted — consider adding one for observability parity.Unauthorized access and failures are both logged, but a successful orders fetch produces no log entry. For request tracing and latency monitoring, a brief success log would maintain parity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/api/shop/orders/route.ts` around lines 35 - 134, The GET handler emits logs for unauthorized and error paths but not on success; add a success log using the same baseMeta and startedAtMs so traces and latency are recorded. After preparing the response (before returning noStoreJson on success) call logInfo (e.g., logInfo('public_orders_list_success', { ...baseMeta, code: 'OK', durationMs: Date.now() - startedAtMs, totalCount })) to include duration and totalCount (or orders.length) for observability parity with the existing logError/logInfo calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx:
- Around line 157-162: The 401/403 branch correctly sets ordersSummary to null
to prevent the orders card rendering for unauthenticated users, so no code
change is required; confirm the conditional in CartPageClient.tsx that checks
res.status === 401 || res.status === 403 and the early return after
setOrdersSummary(null) is preserved, and ensure the surrounding variables
referenced (ordersSummary state setter setOrdersSummary and the cancelled flag)
remain intact and tested to avoid regressions.
---
Nitpick comments:
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx:
- Around line 368-376: The Loader component inside the early return when
!isClientReady lacks accessible labeling; add an offscreen/live status so screen
readers announce loading (e.g., a visually-hidden text "Loading…" or
role="status" with aria-live="polite") alongside the <Loader /> in
CartPageClient.tsx to provide an accessible loading announcement for assistive
tech users.
- Around line 164-165: The fetch handling in CartPageClient.tsx currently bails
silently when responses are non-OK or the body is unparseable; update the logic
around the res/data checks to surface unexpected server issues by emitting a
warning or reporting telemetry (e.g., console.warn in development or calling the
existing client-side error reporter) whenever res.ok is false (excluding
401/403) or when data is null/typeof !== 'object'; include res.status,
res.statusText and the parsed/raw body (or the parse error) in the log/report to
aid debugging and keep the early-return behavior for the UI intact (refer to the
res and data variables in the current code block).
In `@frontend/app/api/shop/orders/route.ts`:
- Line 94: The selected field itemCount uses sql`count(${orderItems.id})`
without a generic type so Drizzle infers unknown; add an explicit generic type
to the sql template (e.g., number or bigint as appropriate for your DB) for
sql`count(${orderItems.id})` so IDEs and typings work correctly and remain
consistent with the subsequent toCount usage (see itemCount and the call to
toCount).
- Around line 60-65: The count query annotation is misleading: change the
generic on sql from number to string so the expression using sql in
totalCountRows better reflects PostgreSQL returning bigint as a string (e.g.,
replace sql<number>`count(*)` with sql<string>`count(*)`), keep using the
toCount(...) call when deriving totalCount from totalCountRows[0]?.value to
perform the safe coercion of string|bigint|number into a numeric count.
- Around line 35-134: The GET handler emits logs for unauthorized and error
paths but not on success; add a success log using the same baseMeta and
startedAtMs so traces and latency are recorded. After preparing the response
(before returning noStoreJson on success) call logInfo (e.g.,
logInfo('public_orders_list_success', { ...baseMeta, code: 'OK', durationMs:
Date.now() - startedAtMs, totalCount })) to include duration and totalCount (or
orders.length) for observability parity with the existing logError/logInfo
calls.
…obank logging safety invariant, namespace localStorage cart by user and reset on auth change
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/shop/CartProvider.tsx`:
- Around line 149-157: The didHydrate boolean guard in CartProvider (didHydrate
ref used in the useEffect that calls getStoredCartItems and syncCartWithServer)
creates an implicit dependency on the parent remounting via a key tied to
cartOwnerId; change the guard to track the last-hydrated owner instead so
re-initialization occurs when cartOwnerId changes: replace didHydrate with a ref
like lastHydratedOwnerRef, compare lastHydratedOwnerRef.current to cartOwnerId
at the start of the effect, bail if equal, otherwise set
lastHydratedOwnerRef.current = cartOwnerId and proceed to load stored items and
call syncCartWithServer; update the useEffect deps to include cartOwnerId and
syncCartWithServer.
2 participants
Description
This PR adds a lightweight “My Orders” entry point to the Shop Cart page and wires it to the existing Shop Orders API, while fixing the initial render flicker on refresh by using the platform’s global loader.
Related Issue
Issue: #<issue_number>
Changes
GET /api/shop/orderswith safe public-page behavior (silently ignores401/403when user is not authenticated).Loaderuntil client/cart state is ready (single, consistent first paint).Database Changes (if applicable)
How Has This Been Tested?
Screenshots (if applicable)
N/A (minor UI placement + loading behavior)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
UX Improvements
Chores