(SP: 1) [Shop UI] Improve semantics + styles across shop; add admin h…#148
(SP: 1) [Shop UI] Improve semantics + styles across shop; add admin h…#148ViktorSvertoka merged 7 commits intodevelopfrom
Conversation
…eader button (admin role only)
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. |
📝 WalkthroughWalkthroughRefactors many shop/admin UI pages for accessibility and semantics, tightens admin-link gating, centralizes refund metadata/idempotency handling with new helpers, adds stateless CSRF utilities and parsePage, alters cart/checkout flows, removes theme/footer components, updates tests and assets. Changes
Sequence Diagram(s)mermaid AdminUI->>AppServer: POST /api/admin/refund (orderId, idempotencyKey, amount) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)frontend/app/[locale]/shop/products/page.tsx (3)
frontend/lib/shop/currency.ts (2)
🔇 Additional comments (12)
✏️ Tip: You can disable this entire section by setting 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (1)
188-190: Avoid auto-updating the read‑only slug in edit mode.The slug now displays as read‑only and isn’t submitted on edit, but it still updates when the title changes, which can mislead admins. Consider syncing slug only in create mode (or freezing it on edit).
🔧 Suggested fix
-useEffect(() => { - setSlug(localSlugify(title)); -}, [title]); +useEffect(() => { + if (mode === 'create') { + setSlug(localSlugify(title)); + } +}, [title, mode]);Also applies to: 434-448
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/shop/admin/orders/[id]/page.tsx:
- Around line 33-35: formatDateTime is calling toLocaleString() without a
locale, causing server-default formatting; update formatDateTime to accept a
locale string (e.g., formatDateTime(value: Date | null | undefined, locale?:
string): string) and call value.toLocaleString(locale) so timestamps respect the
route locale, then pass the current route locale (the page's locale param or the
variable named locale used in this file) into every formatDateTime call (also
update the other occurrences around the 152-175 area) to ensure admin views are
locale-aware.
In `@frontend/app/`[locale]/shop/admin/orders/page.tsx:
- Around line 35-37: formatDate currently calls toLocaleDateString with no
locale, causing server-default (often US) formatting; update the formatDate
function to accept a locale parameter (e.g., formatDate(value: Date | null |
undefined, locale?: string)) and pass that locale into
value.toLocaleDateString(locale); then update every call site that renders dates
(including where formatDate is used around the admin orders list rendering) to
forward the route locale (the [locale] route param) into formatDate so dates
render correctly for the current locale.
In `@frontend/components/shop/add-to-cart-button.tsx`:
- Line 1: Remove the accidental local file path comment at the top of
frontend/components/shop/add-to-cart-button.tsx (the "// C:\Users\..." line);
simply delete that single-line comment so the component (AddToCartButton) file
contains only source code and no machine-specific debug/path information.
In `@frontend/components/shop/catalog-load-more.tsx`:
- Around line 1-2: Remove the machine-specific Windows absolute path comment at
the top of frontend/components/shop/catalog-load-more.tsx: delete the stray
comment line containing "C:\Users\..." so the file has no environment-specific
metadata, trim any leftover blank line or trailing whitespace, and commit the
change (ensure no other code is altered and the component/symbols in
catalog-load-more.tsx remain unchanged).
In `@frontend/components/shop/header/nav-links.tsx`:
- Around line 34-52: Remove the unused showAdminLink prop: delete showAdminLink
from the NavLinksProps interface and stop passing or referencing it in the
NavLinks component; update any call sites that pass showAdminLink (e.g.,
UnifiedHeader) to no longer supply that prop, since admin links are handled
elsewhere (see AppMobileMenu and UnifiedHeader render logic), and ensure
NavLinks function signature and destructuring only include className,
onNavigate, and includeHomeLink.
In `@frontend/components/shop/product-filters.tsx`:
- Around line 153-179: The Size buttons use role="radio" but support
deselection, so replace the radio semantics with toggle semantics: in the map
over SIZES (using SIZES, currentSize, updateFilter, sizeGroupId) remove
role="radio" and aria-checked and instead add aria-pressed={isSelected} (or
role="checkbox" if you prefer checkbox semantics), keeping the existing onClick
toggle logic; ensure the surrounding container can remain
aria-labelledby={sizeGroupId} but the individual <button> uses aria-pressed for
correct accessibility.
- Around line 112-141: The color swatch buttons use role="radio" but implement
toggle/deselect behavior (see COLORS, currentColor, updateFilter), which is
semantically incorrect; change the semantics to a toggle role by replacing
role="radio" with either role="checkbox" and keep aria-checked={isSelected} or
use aria-pressed={isSelected} (to match the Type section pattern), and ensure
any associated aria-labelledby (colorGroupId) and keyboard behavior remain
consistent with the chosen role. Update the button attributes accordingly
(remove radio-specific assumptions) so the accessibility semantics match the
click-to-deselect behavior.
In `@frontend/components/shop/products-toolbar.tsx`:
- Around line 92-133: The modal lacks a Tab focus trap: add a keydown handler on
the dialog container (the element with id={dialogId}) that intercepts Tab and
Shift+Tab, queries focusable elements inside the dialog (buttons, links, inputs,
selects, textareas, [tabindex]:not([tabindex="-1"])), and when focus would move
past the last/first element, preventDefault and move focus to the first/last
element to cycle focus; attach this handler when open and remove it on close
(use the existing close/open state and closeBtnRef for initial focus
management). Also add aria-hidden="true" to the backdrop button (the element
with className "absolute inset-0 bg-black/40" that calls onClick={close}) so it
is hidden from screen readers. Ensure cleanup to avoid duplicate listeners when
the component re-renders.
In `@frontend/components/shop/shop-hero.tsx`:
- Around line 1-2: Remove the accidental local Windows path comment from the top
of frontend/components/shop/shop-hero.tsx (the line showing
"C:\Users\milka\...")—open the shop-hero.tsx file and delete that comment so no
local file paths or usernames are committed; ensure only relevant component code
(e.g., ShopHero component) remains and run a quick lint/format check before
committing.
🧹 Nitpick comments (24)
frontend/components/shop/category-tile.tsx (1)
41-46: Redundantaria-hiddenattribute on nested element.The inner
<span aria-hidden="true">→</span>on line 45 is unnecessary since the parent span (line 41) already hasaria-hidden="true". When a parent is hidden from assistive technology, all descendants are automatically hidden as well.🧹 Suggested cleanup
<span className="mt-2 inline-flex items-center gap-1 text-sm font-medium text-white/90 opacity-0 transition-opacity group-hover:opacity-100" aria-hidden="true" > - Shop now <span aria-hidden="true">→</span> + Shop now → </span>frontend/lib/shop/data.ts (2)
266-283: Consider extracting thefillTohelper to module scope.This utility function is recreated on every
getHomepageContentcall. Moving it to module scope would improve reusability and avoid repeated function allocation.♻️ Suggested refactor
+/** + * Merge primary items with fallback items up to a target count, avoiding duplicates. + */ +function fillTo( + primary: ShopProduct[], + fallback: ShopProduct[], + count: number +): ShopProduct[] { + const used = new Set(primary.map(p => p.id)); + const merged = [...primary]; + + for (const p of fallback) { + if (merged.length >= count) break; + if (used.has(p.id)) continue; + used.add(p.id); + merged.push(p); + } + + return merged.slice(0, count); +} + export async function getHomepageContent( locale: string = 'en' ): Promise<HomepageContent> { // ... rest of function without inline fillTo definition
287-295: RedundantMath.max(12, CATALOG_PAGE_SIZE)— always evaluates to 12.
CATALOG_PAGE_SIZEis12, soMath.max(12, CATALOG_PAGE_SIZE)is always12. Given the comment states you want extra items for filtering after dedup, consider whether a larger limit (e.g.,CATALOG_PAGE_SIZE + 4or a multiplier) better serves that goal.♻️ Suggested clarification
- limit: Math.max(12, CATALOG_PAGE_SIZE), + limit: CATALOG_PAGE_SIZE + 8, // extra buffer for dedup filteringOr if
12is intentionally the minimum regardless of config changes:- limit: Math.max(12, CATALOG_PAGE_SIZE), + limit: Math.max(12, CATALOG_PAGE_SIZE + 4),frontend/app/[locale]/shop/checkout/success/OrderStatusAutoRefresh.tsx (1)
42-43:aria-livehas no effect on an empty, never-updated element.The
aria-live="polite"attribute announces content changes to screen readers, but this span's content is never updated—the page refresh happens viarouter.refresh(), which reloads server components rather than updating this span's text.If the intent is simply to render a non-null utility element, remove
aria-live. If the intent is to announce status changes to screen reader users, the span would need dynamic content reflecting the current state.Option 1: Remove ineffective aria-live
// Non-visual utility component (keeps page data fresh while payment settles). - return <span className="sr-only" aria-live="polite" />; + return <span className="sr-only" />;Option 2: Return null if no DOM presence is needed
// Non-visual utility component (keeps page data fresh while payment settles). - return <span className="sr-only" aria-live="polite" />; + return null;frontend/app/[locale]/shop/products/[slug]/page.tsx (3)
43-52: Consider removing redundantaria-labelfrom the Link.The
aria-label="Back to all products"on Line 47 duplicates the visible link text on Line 50. Screen readers already announce the visible text, so this label is unnecessary and may cause redundant announcements. Thearia-labelon the<nav>element (Line 43) is appropriate since it labels the landmark.Suggested change
<nav aria-label="Product navigation"> <Link href="/shop/products" className="inline-flex items-center gap-2 text-sm text-muted-foreground transition-colors hover:text-foreground" - aria-label="Back to all products" > <ArrowLeft className="h-4 w-4" aria-hidden="true" /> Back to all products </Link> </nav>
97-106: Same redundantaria-labelissue as above.This navigation block has the same redundant
aria-labelon the Link element that duplicates the visible text.Suggested change
<nav aria-label="Product navigation"> <Link href="/shop/products" className="inline-flex items-center gap-2 text-sm text-muted-foreground transition-colors hover:text-foreground" - aria-label="Back to all products" > <ArrowLeft className="h-4 w-4" aria-hidden="true" /> Back to all products </Link> </nav>
137-137:aria-labelon a generic<div>has no effect.The
aria-labelattribute only applies to elements with an implicit or explicit ARIA role (landmarks, interactive elements, or elements with explicit roles). A plain<div>has no role, so screen readers will ignore this label.If you want to label this region for assistive technology, consider using a
<section>element or adding an explicit role.Option 1: Use a section element
- <div className="mt-4 flex items-center gap-3" aria-label="Price"> + <section className="mt-4 flex items-center gap-3" aria-label="Price"> ... - </div> + </section>Option 2: Remove the ineffective aria-label
- <div className="mt-4 flex items-center gap-3" aria-label="Price"> + <div className="mt-4 flex items-center gap-3">frontend/app/[locale]/shop/cart/page.tsx (2)
74-82: Consider the timing ofclearCart()relative to navigation.If
router.push()throws or fails (e.g., network hiccup, navigation cancelled), the cart is already cleared and the user loses their items. Consider clearing the cart only after confirming the order was successfully placed on the server (which it is at this point), or storing a flag to prevent re-checkout rather than clearing immediately.That said, since the order is already created server-side at this point, clearing early may be acceptable to prevent duplicate orders. Just ensure the UX handles a failed navigation gracefully (user can still navigate to their order).
217-222:aria-labelon a non-interactive<span>has limited effect.The
aria-labelattribute is primarily intended for interactive elements or elements with specific roles. On a plain<span>, screen readers may ignore it. Since this quantity is already contextually placed between decrement/increment buttons with clear labels, it should be understandable.Consider either removing the
aria-labelor wrapping with a<span>that hasrole="status"if you want quantity changes to be announced.Option 1: Remove aria-label (context is sufficient)
- <span - className="w-8 text-center text-sm font-medium" - aria-label="Quantity" - > + <span className="w-8 text-center text-sm font-medium"> {item.quantity} </span>Option 2: Add visually-hidden label for clarity
<span className="w-8 text-center text-sm font-medium" - aria-label="Quantity" > + <span className="sr-only">Quantity: </span> {item.quantity} </span>frontend/components/shop/shop-hero.tsx (1)
31-39: Thearia-labelis redundant with visible text content.The
aria-label={ctaText}duplicates the accessible name already provided by the visible<span>{ctaText}</span>content. Since the arrow is correctly markedaria-hidden="true", screen readers will naturally read only the CTA text.Consider removing the redundant
aria-label:Suggested simplification
<Link href={ctaLink} className="inline-flex items-center gap-2 rounded-md bg-foreground px-6 py-3 text-sm font-semibold uppercase tracking-wide text-background transition-colors hover:bg-foreground/90" - aria-label={ctaText} > <span>{ctaText}</span> <span aria-hidden="true">→</span> </Link>frontend/components/shop/catalog-products-client.tsx (1)
115-147: Consider explicit list-style reset for the new<ul>/<li>structure.
If this component is ever rendered without Tailwind preflight, default bullets/padding can appear. Addinglist-none m-0 p-0keeps layout consistent.♻️ Suggested tweak
- <ul - className="grid gap-6 sm:grid-cols-2 lg:grid-cols-3" + <ul + className="list-none m-0 p-0 grid gap-6 sm:grid-cols-2 lg:grid-cols-3" aria-label="Products" >frontend/app/[locale]/shop/products/page.tsx (1)
91-95: Redundant ARIA:role="status"already impliesaria-live="polite".The
role="status"implicitly setsaria-live="polite", so the explicitaria-liveattribute is redundant. Consider removing it for cleaner markup.♻️ Suggested simplification
<div className="flex flex-col items-center justify-center py-16 text-center" role="status" - aria-live="polite" >frontend/components/shop/product-sort.tsx (1)
59-66: Consider usingcn()utility for class composition consistency.Other components in this PR use the
cn()utility for conditional class merging. Usingarray.join(' ')here works but is inconsistent with the codebase pattern.♻️ Suggested refactor for consistency
<select id={selectId} name="sort" value={currentSort} onChange={e => handleSort(e.target.value)} - className={[ - 'h-10 w-full rounded-md border border-input px-3 text-sm transition-colors sm:w-48', - isActive - ? 'bg-muted text-foreground' - : 'bg-background text-muted-foreground', - 'hover:text-foreground', - 'focus:outline-none focus:ring-0 focus:ring-offset-0', - ].join(' ')} + className={cn( + 'h-10 w-full rounded-md border border-input px-3 text-sm transition-colors sm:w-48', + isActive + ? 'bg-muted text-foreground' + : 'bg-background text-muted-foreground', + 'hover:text-foreground', + 'focus:outline-none focus:ring-0 focus:ring-offset-0' + )} >frontend/components/header/AppMobileMenu.tsx (1)
93-101: Consider moving admin-related links into the authenticated section.The "New product" link is placed outside the
userExistsblock (lines 93-101), while the "Admin" link is inside it (lines 115-124). Since both requireshowAdminLink(which already gates on user existence at the layout level), this works correctly, but grouping all admin-related links together inside the authenticated section would be more consistent and maintainable.frontend/app/[locale]/layout.tsx (1)
38-40: Remove or update the placeholder comment.The Ukrainian comment on line 38 appears to be a development note ("substitute your real path to the role"). If the role check
user?.role === 'admin'is finalized, consider removing this comment to avoid confusion for future maintainers.Suggested cleanup
- // підстав свій реальний шлях до ролі - const isAdmin = user?.role === 'admin'; // або 'ADMIN', або isShopAdmin === true + const isAdmin = user?.role === 'admin';frontend/components/shop/admin/admin-product-status-toggle.tsx (2)
1-1: Remove debug file path comment.This absolute Windows file path appears to be a debug artifact and should be removed.
Suggested fix
-// C:\Users\milka\devlovers.net-clean\frontend\components\shop\admin\admin-product-status-toggle.tsx - 'use client';
42-44: Consider structured error logging for observability.Silencing errors entirely removes debugging capability for production issues. Consider logging to an error tracking service or at minimum preserving the error in development mode.
Optional: Add conditional logging
} catch (err) { - // Avoid noisy console in UI components; keep UX deterministic. + if (process.env.NODE_ENV === 'development') { + console.error('Failed to toggle product status:', err); + } setError('Failed to update status'); } finally {frontend/components/shop/header/nav-links.tsx (1)
1-2: Remove debug file path comment.Same as in the other file - this absolute Windows path should be removed.
Suggested fix
-// C:\Users\milka\devlovers.net-clean\frontend\components\shop\header\nav-links.tsx - 'use client';frontend/components/header/UnifiedHeader.tsx (3)
3-4: Remove commented-out import.The commented import line is no longer needed and adds noise. If the search feature is being deferred, track it in an issue rather than leaving commented code.
Suggested fix
-// import { LogIn, Search, Settings, User } from 'lucide-react'; import { LogIn, Settings, User } from 'lucide-react';
28-28: Remove commented-out search code.The
enableSearchprop (line 28) and the search UI block (lines 82-97) are commented out. If the search feature is being deferred, consider removing this code entirely and tracking it in an issue to avoid code rot. The prop can also be removed fromUnifiedHeaderPropsif not currently in use.Also applies to: 82-97
60-64: Remove unusedshowAdminLinkprop from NavLinks.The
showAdminLinkprop is passed here butNavLinkscomponent doesn't use it (as noted in the nav-links.tsx review). Remove this prop to avoid confusion.Suggested fix
<NavLinks className="md:flex" - showAdminLink={showAdminLink} includeHomeLink />frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
72-81: Consider removing redundantaria-liveattribute.When using
role="alert", the element already has an implicitaria-live="assertive"behavior. Addingaria-live="polite"creates conflicting semantics. Since this is an error message, the implicit assertive behavior fromrole="alert"is likely more appropriate.💡 Suggested fix
{error ? ( <span id={errorId} role="alert" - aria-live="polite" className="text-xs text-destructive" > {error} </span> ) : null}frontend/app/[locale]/shop/checkout/success/page.tsx (1)
36-63: Consider extracting shared shell component.
CheckoutShellis nearly identical toPageShellin the payment page. Consider extracting a shared component to@/components/shop/checkout/CheckoutShell.tsxto reduce duplication and ensure consistent styling across checkout flows.frontend/app/[locale]/shop/orders/[id]/page.tsx (1)
99-133: Stabilize item ordering for consistent UI.Without an explicit ordering, item rows can appear in different orders between requests. Adding an
orderBymakes the list deterministic.♻️ Proposed tweak
.from(orders) .leftJoin(orderItems, eq(orderItems.orderId, orders.id)) .where(whereClause); + .orderBy(orderItems.id);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
frontend/public/apparel.jpgis excluded by!**/*.jpgfrontend/public/collectibles.jpgis excluded by!**/*.jpgfrontend/public/lifestyle.jpgis excluded by!**/*.jpg
📒 Files selected for processing (46)
.hintrcfrontend/app/[locale]/layout.tsxfrontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsxfrontend/app/[locale]/shop/admin/orders/[id]/page.tsxfrontend/app/[locale]/shop/admin/orders/page.tsxfrontend/app/[locale]/shop/admin/page.tsxfrontend/app/[locale]/shop/admin/products/[id]/edit/page.tsxfrontend/app/[locale]/shop/admin/products/_components/product-form.tsxfrontend/app/[locale]/shop/admin/products/new/page.tsxfrontend/app/[locale]/shop/admin/products/page.tsxfrontend/app/[locale]/shop/cart/page.tsxfrontend/app/[locale]/shop/checkout/error/page.tsxfrontend/app/[locale]/shop/checkout/payment/StripePaymentClient.tsxfrontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsxfrontend/app/[locale]/shop/checkout/success/OrderStatusAutoRefresh.tsxfrontend/app/[locale]/shop/checkout/success/page.tsxfrontend/app/[locale]/shop/orders/[id]/page.tsxfrontend/app/[locale]/shop/orders/error.tsxfrontend/app/[locale]/shop/orders/page.tsxfrontend/app/[locale]/shop/page.tsxfrontend/app/[locale]/shop/products/[slug]/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/components/header/AppChrome.tsxfrontend/components/header/AppMobileMenu.tsxfrontend/components/header/UnifiedHeader.tsxfrontend/components/shop/add-to-cart-button.tsxfrontend/components/shop/admin/admin-pagination.tsxfrontend/components/shop/admin/admin-product-status-toggle.tsxfrontend/components/shop/admin/shop-admin-topbar.tsxfrontend/components/shop/cart-provider.tsxfrontend/components/shop/catalog-load-more.tsxfrontend/components/shop/catalog-products-client.tsxfrontend/components/shop/category-tile.tsxfrontend/components/shop/header/cart-button.tsxfrontend/components/shop/header/nav-links.tsxfrontend/components/shop/header/theme-toggle.tsxfrontend/components/shop/product-card.tsxfrontend/components/shop/product-filters.tsxfrontend/components/shop/product-sort.tsxfrontend/components/shop/products-toolbar.tsxfrontend/components/shop/shop-footer.tsxfrontend/components/shop/shop-hero.tsxfrontend/components/shop/theme-provider.tsxfrontend/db/queries/shop/products.tsfrontend/lib/config/catalog.tsfrontend/lib/shop/data.ts
💤 Files with no reviewable changes (3)
- frontend/components/shop/shop-footer.tsx
- frontend/components/shop/header/theme-toggle.tsx
- frontend/components/shop/theme-provider.tsx
🧰 Additional context used
🧬 Code graph analysis (20)
frontend/components/shop/products-toolbar.tsx (2)
frontend/components/shop/product-sort.tsx (1)
ProductSort(15-76)frontend/components/shop/product-filters.tsx (1)
ProductFilters(9-183)
frontend/lib/shop/data.ts (4)
frontend/lib/types/shop.ts (1)
DbProduct(22-22)frontend/lib/validation/shop.ts (2)
DbProduct(430-430)ShopProduct(431-431)frontend/db/queries/shop/products.ts (1)
getFeaturedProducts(270-283)frontend/lib/config/catalog.ts (1)
CATALOG_PAGE_SIZE(39-39)
frontend/components/shop/header/cart-button.tsx (2)
frontend/components/shop/cart-provider.tsx (1)
useCart(293-295)frontend/hooks/use-mounted.ts (1)
useMounted(17-19)
frontend/app/[locale]/shop/cart/page.tsx (1)
frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/app/[locale]/shop/orders/page.tsx (3)
frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/lib/auth.ts (1)
getCurrentUser(95-116)frontend/lib/logging.ts (1)
logError(87-93)
frontend/app/[locale]/shop/products/page.tsx (2)
frontend/components/shop/products-toolbar.tsx (1)
ProductsToolbar(9-136)frontend/components/shop/product-filters.tsx (1)
ProductFilters(9-183)
frontend/db/queries/shop/products.ts (1)
frontend/db/schema/shop.ts (1)
products(56-105)
frontend/components/header/AppChrome.tsx (1)
frontend/components/header/UnifiedHeader.tsx (1)
UnifiedHeader(24-148)
frontend/components/shop/add-to-cart-button.tsx (4)
frontend/lib/shop/data.ts (1)
ShopProduct(26-26)frontend/components/shop/cart-provider.tsx (1)
useCart(293-295)frontend/lib/utils.ts (1)
cn(4-6)frontend/save-structure.cjs (1)
output(6-6)
frontend/components/shop/product-sort.tsx (2)
frontend/lib/utils.ts (1)
cn(4-6)frontend/lib/config/catalog.ts (1)
SORT_OPTIONS(32-37)
frontend/components/shop/catalog-products-client.tsx (2)
frontend/components/shop/product-card.tsx (1)
ProductCard(38-99)frontend/components/shop/catalog-load-more.tsx (1)
CatalogLoadMore(11-31)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (3)
frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)frontend/components/shop/admin/shop-admin-topbar.tsx (1)
ShopAdminTopbar(5-58)
frontend/app/[locale]/shop/admin/orders/page.tsx (4)
frontend/lib/shop/currency.ts (2)
resolveCurrencyFromLocale(23-28)formatMoney(115-128)frontend/save-structure.cjs (1)
items(10-10)frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)frontend/components/shop/admin/admin-pagination.tsx (1)
AdminPagination(18-76)
frontend/app/[locale]/shop/admin/products/new/page.tsx (15)
frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
dynamic(17-17)frontend/app/[locale]/shop/admin/orders/page.tsx (1)
dynamic(15-15)frontend/app/[locale]/shop/admin/page.tsx (1)
dynamic(7-7)frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (1)
dynamic(15-15)frontend/app/[locale]/shop/admin/products/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/shop/checkout/error/page.tsx (1)
dynamic(9-9)frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (1)
dynamic(14-14)frontend/app/[locale]/shop/checkout/success/page.tsx (1)
dynamic(11-11)frontend/app/[locale]/shop/orders/[id]/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/shop/orders/page.tsx (1)
dynamic(15-15)frontend/app/[locale]/shop/products/[slug]/page.tsx (1)
dynamic(13-13)frontend/lib/auth/guard-shop-admin-page.ts (1)
guardShopAdminPage(9-18)frontend/components/shop/admin/shop-admin-topbar.tsx (1)
ShopAdminTopbar(5-58)frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (1)
ProductForm(128-893)
frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (2)
frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/app/[locale]/shop/checkout/error/page.tsx (6)
frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (2)
dynamic(14-14)revalidate(15-15)frontend/lib/validation/shop.ts (1)
orderIdParamSchema(394-396)frontend/lib/services/orders.ts (1)
getOrderSummary(2-2)frontend/lib/services/errors.ts (1)
OrderNotFoundError(26-32)frontend/lib/shop/currency.ts (2)
resolveCurrencyFromLocale(23-28)formatMoney(115-128)
frontend/components/shop/admin/admin-pagination.tsx (1)
frontend/lib/utils.ts (1)
cn(4-6)
frontend/components/shop/product-filters.tsx (2)
frontend/lib/config/catalog.ts (4)
CATEGORIES(1-9)PRODUCT_TYPES(11-16)COLORS(18-28)SIZES(30-30)frontend/lib/utils.ts (1)
cn(4-6)
frontend/app/[locale]/shop/page.tsx (2)
frontend/components/shop/product-card.tsx (1)
ProductCard(38-99)frontend/components/shop/category-tile.tsx (1)
CategoryTile(9-50)
frontend/components/shop/category-tile.tsx (1)
frontend/lib/shop/data.ts (1)
ShopCategory(28-33)
🪛 Biome (2.1.2)
frontend/app/[locale]/shop/orders/error.tsx
[error] 8-8: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (84)
frontend/components/shop/admin/admin-pagination.tsx (1)
32-73: Looks good — semantic and accessible pagination structure.The new
<nav>+<ul>/<li>structure witharia-labelandaria-currentis solid and keeps the same navigation logic..hintrc (1)
11-18: Clarify rationale for disabling accessibility and style hints.The PR aims to improve semantics and accessibility, but these changes disable related linter checks:
axe/structure→list: "off"suppresses list semantics validationno-inline-styles: "off"suppresses inline style warningsIf these are false positives or framework constraints (e.g., CSS-in-JS), consider adding a brief comment in the file or PR description explaining why. Otherwise, disabling these hints may mask real issues that conflict with the PR's accessibility goals.
frontend/components/shop/header/cart-button.tsx (1)
13-33: Looks good—guarded count and clearer labeling.The mounted guard prevents hydration mismatches, and the dynamic aria-label improves accessibility.
frontend/app/[locale]/shop/page.tsx (4)
1-1: Import consolidation looks clean.No concerns with the Link import placement at Line 1.
24-53: Accessible section structure looks solid.The
aria-labelledby+ heading id pairing and list-based grid for New Arrivals are well-aligned with semantics at Lines 24–53.
56-77: Shop-by-category section semantics are solid.The header +
aria-labelledbyand UL/LI structure read clean and accessible at Lines 56–77.
80-105: CTA labeling and structure are well done.The section heading id and explicit CTA
aria-labelat Lines 80–105 are appropriate.frontend/components/shop/product-card.tsx (2)
74-89: Good accessibility labeling for the price cluster.
The price group is clearer for screen readers now.
92-94: Stock status announcement is well-marked.
role="status"is appropriate for conveying availability.frontend/lib/config/catalog.ts (1)
41-44: All image assets exist in the static public directory and are correctly referenced. The paths/apparel.jpg,/collectibles.jpg, and/lifestyle.jpgwill resolve correctly to the files at./frontend/public/.frontend/components/shop/category-tile.tsx (3)
1-7: LGTM!Clean imports with explicit type import for
ShopCategoryand properly typed interface. The use ofimport typeis a good practice for type-only imports.
9-22: Good accessibility and URL encoding practices.The precomputed
hrefwithencodeURIComponent(category.slug)correctly handles special characters in slugs. Thearia-labelprovides a clear accessible name, and thefocus-visiblestyles ensure proper keyboard navigation feedback. The className array pattern with the explanatory comment is a nice touch for maintainability.
23-34: LGTM!The gradient overlay is correctly marked with
aria-hidden="true"as it's purely decorative. The gradient improves text readability against the image background.frontend/components/shop/catalog-load-more.tsx (1)
19-29: LGTM!Good accessibility improvements:
type="button"prevents unintended form submissions.aria-busy={isLoading}properly signals loading state to assistive technologies.- Disabled state with visual feedback ("Loading..." text) provides good UX.
The centering wrapper and styling are appropriate for the component's purpose.
frontend/app/[locale]/shop/checkout/success/OrderStatusAutoRefresh.tsx (2)
13-15: LGTM!The terminal state checks cover the standard payment outcome states. The implementation is clean and readable.
25-40: LGTM!The polling logic is well-structured: proper early-exit for terminal states, correct cleanup on unmount, and appropriate use of refs to persist timing state across renders. The dependency array is complete.
frontend/db/queries/shop/products.ts (2)
7-7: LGTM!Import addition is necessary for the new OR-based filtering logic.
146-153: Logic change is correct; 'NEW' badge value is valid.The semantic shift from
isFeaturedto badge-based filtering for "New Arrivals" is sound, and the OR clause provides good back-compat for existing rows withcategory='new-arrivals'. TheproductBadgeEnumexplicitly includes'NEW'as a valid value.frontend/app/[locale]/shop/products/[slug]/page.tsx (1)
157-159: Accessibility improvements look good overall.The
<section>witharia-label="Purchase"is appropriate here. While it currently wraps only theAddToCartButton, it establishes a clear landmark for the purchase interaction area, which could accommodate future additions (quantity controls, variant selectors, etc.).frontend/app/[locale]/shop/cart/page.tsx (6)
19-20: LGTM!Good use of
useParamswith TypeScript generics and a sensible fallback for the locale.
102-124: LGTM!Good semantic structure with
<main>, proper heading hierarchy, and correctaria-hiddenon the decorative icon. The i18nLinkcomponent handles locale prefixing automatically.
136-145: Excellent accessibility pattern for removed items notification.Good use of
role="status"andaria-live="polite"to announce removed items to screen reader users without interrupting them.
266-275: Well-implemented accessible aside pattern.Good use of
aria-labelledbylinking the<aside>to the headingid, making the relationship explicit for assistive technologies.
313-321: Good loading state accessibility.The
aria-busyattribute correctly communicates the loading state to assistive technologies, and the button text change provides visual feedback.
328-332: Correct use ofrole="alert"for error messages.This ensures checkout errors are immediately announced to screen reader users.
frontend/components/shop/shop-hero.tsx (2)
14-25: LGTM!The
aria-labelledbyon the section paired with theidon theh1is a well-implemented accessibility pattern that properly associates the section with its heading for assistive technology.
44-51: LGTM!Correctly marking the decorative gradient container as
aria-hidden="true"ensures assistive technology ignores this purely visual element.frontend/components/shop/catalog-products-client.tsx (1)
8-9: No issues to flag here.frontend/components/shop/cart-provider.tsx (4)
5-26: No issues in import reordering.
71-75: Formatting-only change.
79-84: Clean props typing.
Improves readability and keeps the public API explicit.
157-160: Deferral looks safe.
The microtask deferral keeps behavior intact while sidestepping lint noise.frontend/components/shop/add-to-cart-button.tsx (2)
127-158: Well-structured quantity controls with appropriate semantics.Good use of
<section>witharia-labelledbyfor the quantity controls, and<output>is semantically appropriate for displaying the current value. The icons are correctly markedaria-hidden="true".Consider whether an upper bound for quantity should be enforced (e.g., based on stock level or a maximum order limit) to prevent unrealistic cart quantities.
49-88: Excellent accessibility improvements with proper radiogroup semantics.The refactoring to use
<fieldset>,<legend>, androle="radiogroup"with individualrole="radio"buttons is a significant accessibility improvement. The pattern correctly:
- Groups related controls with fieldset/legend
- Uses
aria-labelledbyto associate the legend text with the radiogroup- Sets
aria-checkedstate on each option- Provides accessible names via
aria-labelon color buttons- Adds
type="button"to prevent unintended form submissionAlso applies to: 91-124
frontend/app/[locale]/shop/products/page.tsx (4)
32-47: LGTM! URL canonicalization prevents shareable pagination states.The logic correctly strips the
pageparameter while preserving other filter params. This ensures infinite-load URLs aren't bookmarkable with stale page states, which aligns with the PR objectives.
49-59: LGTM! Safe fallback handling for query parameters.The Zod schema validation with a fallback to sensible defaults (
page: 1,limit: CATALOG_PAGE_SIZE) ensures the page renders even with invalid query params. Explicitly settingpage: 1infiltersis appropriate since the canonicalization above already strips anypageparam.
64-73: LGTM! Well-structured semantic header with responsive layout.The header uses proper semantic elements with responsive flexbox layout. Wrapping
ProductsToolbarinSuspenseis correct since it uses client-side hooks (useSearchParams).
76-83: LGTM! Accessible filter aside with proper labeling.The
sr-onlyheading witharia-labelledbyprovides screen reader context while hiding the visual heading on desktop. Thehidden lg:blockpattern appropriately shows filters only on larger viewports.frontend/components/shop/products-toolbar.tsx (3)
9-18: LGTM! Well-organized state and ref setup.Using
useIdfor stable ARIA IDs is the recommended React 19 pattern. The ref setup for focus management follows the dialog focus restoration pattern correctly.
20-35: LGTM! Responsive auto-close prevents stuck dialog state.The media query listener correctly closes the dialog when the viewport crosses the
lgbreakpoint, preventing a stuck mobile dialog on desktop. The early check onmq.matcheshandles the edge case of opening while already at desktop width.
37-64: LGTM! Solid keyboard and focus management.The implementation correctly:
- Closes on Escape key
- Locks body scroll with
overflow-hidden- Moves focus to close button on open
- Restores focus to trigger button on close
Using
requestAnimationFrameensures focus changes happen after the DOM updates.frontend/components/shop/product-filters.tsx (3)
17-21: LGTM! Proper use ofuseIdfor accessibility IDs.Using
useId()for each filter group ensures stable, unique IDs across server and client renders, which is essential foraria-labelledbyassociations.
36-65: LGTM! Category section uses correct ARIA pattern.Using
aria-current="true"for the selected category is semantically appropriate for navigation-style selection where one option is always active.
67-101: LGTM! Type section uses correct toggle pattern.Using
aria-pressedfor buttons that toggle on/off is the correct ARIA pattern for this behavior.frontend/components/shop/product-sort.tsx (3)
15-26: LGTM! Well-structured locale-aware sorting setup.Good practices:
- Locale derived from route params with sensible
'en'fallbackcurrentSortdefaults to'featured'when no param presentuseId()ensures accessible label association
28-40: LGTM! Clean URL state management.Correctly removes
pageparam on sort change to reset pagination. The'featured'check to delete vs set the sort param keeps URLs clean.
42-74: LGTM! Accessible form structure.Using a
<form>element witharia-labeland proper<label>/<select>association provides good accessibility. The form doesn't submit (no action/onSubmit) which is fine—it's used for semantic grouping.frontend/components/header/AppMobileMenu.tsx (1)
115-124: LGTM with a minor note.The Admin link implementation is correct. The
aria-labelandtitleare slightly redundant since the link has visible text "Admin", but this provides consistency with the icon-only Admin link in the desktop header. Acceptable as-is.frontend/app/[locale]/layout.tsx (1)
34-40: Admin gating logic is sound.The three-way check (
Boolean(user) && isAdmin && enableAdmin) correctly ensures the admin link only appears when: (1) a user is logged in, (2) they have the admin role, and (3) the admin feature is enabled via environment variable. The case-insensitive comparison handles common env var value variations.frontend/components/shop/admin/admin-product-status-toggle.tsx (1)
56-79: Well-implemented accessibility pattern.Good use of
useIdfor stable error association,aria-busyfor loading state, andaria-describedby+aria-live="polite"for screen reader announcements. This follows WCAG guidelines for form feedback.frontend/components/shop/header/nav-links.tsx (1)
65-119: Good semantic structure and memoization.The
useMemocorrectly depends onpathnameandcurrentCategory, and the semanticnav > ul > listructure witharia-current="page"for active links follows accessibility best practices.frontend/components/header/UnifiedHeader.tsx (1)
109-118: Admin link implementation is well done.Proper use of the Settings icon with
aria-hidden="true"since the link has an accessible label. The styling is consistent with other header icons.frontend/components/header/AppChrome.tsx (1)
34-39: LGTM!The
showAdminLinkprop is now correctly forwarded toUnifiedHeaderfor both shop and platform variants, ensuring consistent admin link behavior across the application.frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
53-70: LGTM!Good accessibility implementation with
aria-disabled,aria-busy, andaria-describedbyproviding comprehensive screen reader support for the button's various states.frontend/app/[locale]/shop/checkout/payment/StripePaymentClient.tsx (3)
124-146: LGTM!The form accessibility improvements are well-implemented:
- Form has a descriptive
aria-label- Error messages use
role="alert"for proper announcement- Button state is communicated via both native
disabledandaria-disabled
177-199: Good semantic structure.Using
<section>witharia-labeland<nav>witharia-label="Next steps"provides clear landmarks for assistive technology users navigating the disabled payments state.
227-246: LGTM!The type assertion
stripePromise as Promise<Stripe>on line 229 is safe here since the null case is already handled by the guard on line 219. The semantic<section>wrapper witharia-label="Secure payment"provides good context for screen reader users.frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (4)
14-15: LGTM!Using
force-dynamicandrevalidate = 0is appropriate for payment pages where fresh order data is critical for security and correctness.
49-76: Good abstraction for consistent page structure.
PageShellcleanly encapsulates the common layout pattern for error and status states, with properaria-labelledbylinking to the heading for accessibility.
228-229: LGTM!The
itemsCountcalculation correctly sums quantities across all order items usingreduce.
287-313: Excellent semantic structure.Using
<aside>for the order summary and<dl>/<dt>/<dd>for the data pairs is semantically correct and improves accessibility. Thearia-labelprovides clear context for screen reader users.frontend/app/[locale]/shop/checkout/success/page.tsx (3)
171-176: Good use ofrole="note".Using
role="note"for the payments-disabled message is semantically appropriate as it's supplementary information that doesn't require immediate attention, distinct from error alerts.
187-206: LGTM!The definition list structure for order summary data is semantically correct and consistent with the payment page implementation.
144-225: Well-structured success page.The main flow correctly implements:
- Semantic
<main>/<section>structure with properaria-labelledby- Auto-refresh component for webhook status updates
- Accessible navigation with
aria-label="Next steps"frontend/app/[locale]/shop/checkout/error/page.tsx (5)
9-40: Search param normalization looks solid.
Centralizing param extraction and handling Promise vs. object keeps parsing predictable. Please confirm this aligns with your Next.js 16searchParamsbehavior.
44-81: Clear “missing order” state.
The main/section/nav structure is clean and the CTA grouping is easy to scan.
83-145: Error states are consistent and accessible.
Order-not-found and generic error layouts follow the same semantic pattern and read well.
207-230: Action bar looks good.
Retry is scoped to failed payments and the link set is consistent with other states.
149-195: Remove misleading comment or implement legacy fallback correctly.The code checks for
totalAmountMinorbut falls back tonullinstead of convertingtotalAmount(the legacy major field) to minor units as the comment claims. By type contract,OrderSummaryWithMinorguaranteestotalAmountMinoris always populated via the service layer, making this check redundant. Either remove the comment or implement the fallback:Fallback implementation
const totalMinor = typeof (order as any).totalAmountMinor === 'number' ? (order as any).totalAmountMinor - : null; + : typeof (order as any).totalAmount === 'string' + ? Math.round(parseFloat((order as any).totalAmount) * 100) + : null;frontend/app/[locale]/shop/admin/products/new/page.tsx (1)
7-17: Dynamic render flag + layout wrapper look good.
Please confirmforce-dynamicis still desired for this admin route under your current caching strategy.frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (2)
15-46: Guard + param validation flow is clear.
Please verifynotFound()is still the intended short-circuit behavior in this Next.js version.
82-103: Form wrapper integration looks clean.
Main container layout keeps the edit form consistent with other admin pages.frontend/components/shop/admin/shop-admin-topbar.tsx (1)
7-54: Nice semantic header/nav structure.
The topbar reads cleanly and matches the new admin layout patterns.frontend/app/[locale]/shop/admin/page.tsx (1)
7-63: Admin landing layout is clear and well-structured.
Please confirmforce-dynamicremains appropriate for this page’s caching behavior.frontend/app/[locale]/shop/admin/orders/page.tsx (1)
68-210: Semantic layout + table structure look solid.The main/section layout, accessible table caption, and preformatted totals read cleanly and align with the admin UI conventions.
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (1)
382-891: Great accessibility upgrades across the form.The
useIdwiring,aria-describedby/invalid/busy, and semantic sections materially improve screen‑reader usability.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
65-281: Clean semantic detail layout.The header + summary/stock/items sections with captions and formatted totals are clear and consistent with the admin UI direction.
frontend/app/[locale]/shop/admin/products/page.tsx (1)
100-305: Admin products table refactor looks solid.Semantic layout, accessible caption, and per‑row formatting read well and align with the admin UI refactor goals.
frontend/app/[locale]/shop/orders/page.tsx (3)
20-94: Helper utilities are clear and defensive.The trimming, UUID guard, and numeric normalization should keep labels and counts safe for display.
96-169: Data fetch + aggregation look solid.Auth gating, deterministic label selection, and structured error handling make this reliable.
171-305: Accessible structure and empty state are nicely handled.The main/header/nav structure and table semantics read well for assistive tech.
frontend/app/[locale]/shop/orders/[id]/page.tsx (2)
45-74:toOrderItemvalidation is strict and readable.Guarding null rows and throwing on corrupt data keeps the detail view trustworthy.
156-273: Detail view layout is well structured.Clear grouping for summary, admin-only details, and items keeps the page easy to scan.
frontend/app/[locale]/shop/orders/error.tsx (1)
8-21: Rename the component to avoid shadowing the globalError.Biome flags this with the
noShadowRestrictedNamesrule (enabled via recommended rules), and shadowing the built-in Error constructor can cause confusion. The ErrorPageProps type on line 4 references the global Error type, then the function declaration on line 8 shadows it.🛠 Proposed fix
-export default function Error({ reset }: ErrorPageProps) { +export default function OrdersError({ reset }: ErrorPageProps) {
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| interface NavLinksProps { | ||
| className?: string; | ||
| onNavigate?: () => void; | ||
| showAdminLink?: boolean; | ||
| includeHomeLink?: boolean; // NEW | ||
| includeHomeLink?: boolean; | ||
| } | ||
|
|
||
| function getLinkCategory(href: string): string | null { | ||
| const [, query] = href.split('?'); | ||
| if (!query) return null; | ||
| const params = new URLSearchParams(query); | ||
| return params.get('category'); | ||
| } | ||
|
|
||
| export function NavLinks({ | ||
| className, | ||
| onNavigate, | ||
| showAdminLink = false, | ||
| includeHomeLink = false, | ||
| }: NavLinksProps) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all mentions of showAdminLink in the codebase
echo "=== All occurrences of showAdminLink ==="
rg -n "showAdminLink" --type=tsx --type=ts
echo -e "\n=== Checking nav-links.tsx specifically ==="
rg -n "showAdminLink" frontend/components/shop/header/nav-links.tsx
echo -e "\n=== Checking where NavLinks is imported/used ==="
rg -n "NavLinks" frontend/components/shop/header/ --type=tsx --type=ts -A 3Repository: DevLoversTeam/devlovers.net
Length of output: 309
🏁 Script executed:
#!/bin/bash
# Search without type filter
echo "=== All occurrences of showAdminLink ==="
rg -n "showAdminLink"
echo -e "\n=== Content of nav-links.tsx ==="
cat -n frontend/components/shop/header/nav-links.tsx
echo -e "\n=== Where NavLinks is used ==="
rg -n "NavLinks" --glob="*.tsx" --glob="*.ts"Repository: DevLoversTeam/devlovers.net
Length of output: 6193
Remove unused showAdminLink prop.
The showAdminLink prop is declared in NavLinksProps (line 37) but is never destructured or used in the component. The prop is passed from UnifiedHeader (line 62) but NavLinks ignores it entirely. Admin link rendering is handled in other components like AppMobileMenu and UnifiedHeader, not in NavLinks.
Suggested fix
interface NavLinksProps {
className?: string;
onNavigate?: () => void;
- showAdminLink?: boolean;
includeHomeLink?: boolean;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface NavLinksProps { | |
| className?: string; | |
| onNavigate?: () => void; | |
| showAdminLink?: boolean; | |
| includeHomeLink?: boolean; // NEW | |
| includeHomeLink?: boolean; | |
| } | |
| function getLinkCategory(href: string): string | null { | |
| const [, query] = href.split('?'); | |
| if (!query) return null; | |
| const params = new URLSearchParams(query); | |
| return params.get('category'); | |
| } | |
| export function NavLinks({ | |
| className, | |
| onNavigate, | |
| showAdminLink = false, | |
| includeHomeLink = false, | |
| }: NavLinksProps) { | |
| interface NavLinksProps { | |
| className?: string; | |
| onNavigate?: () => void; | |
| includeHomeLink?: boolean; | |
| } | |
| function getLinkCategory(href: string): string | null { | |
| const [, query] = href.split('?'); | |
| if (!query) return null; | |
| const params = new URLSearchParams(query); | |
| return params.get('category'); | |
| } | |
| export function NavLinks({ | |
| className, | |
| onNavigate, | |
| includeHomeLink = false, | |
| }: NavLinksProps) { |
🤖 Prompt for AI Agents
In `@frontend/components/shop/header/nav-links.tsx` around lines 34 - 52, Remove
the unused showAdminLink prop: delete showAdminLink from the NavLinksProps
interface and stop passing or referencing it in the NavLinks component; update
any call sites that pass showAdminLink (e.g., UnifiedHeader) to no longer supply
that prop, since admin links are handled elsewhere (see AppMobileMenu and
UnifiedHeader render logic), and ensure NavLinks function signature and
destructuring only include className, onNavigate, and includeHomeLink.
… harden catalog fetch race-guard, simplify restock release flow, and apply CSS/logging/query-policy cleanups
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/shop/products/page.tsx:
- Around line 75-82: The outer landmark wrapper around ProductFilters is an
<aside> that nests the same landmark rendered inside ProductFilters; change the
outer <aside> to a non-landmark wrapper or make ProductFilters render a
non-landmark wrapper when embedded. Fix option A: in the page component replace
the outer <aside aria-labelledby="filters-heading"> ...
<Suspense><ProductFilters/></Suspense> </aside> with a <div> (keeping the h2 and
Suspense/ProductFilters intact). Fix option B: add a prop to ProductFilters
(e.g., containerElement or as) so callers can request a <div> instead of an
<aside>, and update the page to pass that prop into ProductFilters inside the
existing Suspense. Ensure aria-labelledby/aria-label semantics remain correct
after the change.
In `@frontend/lib/psp/stripe.ts`:
- Around line 64-66: The code uses the ES2022 Error.cause option (e.g., new
Error('STRIPE_REFUND_FAILED', { cause: error }) in frontend/lib/psp/stripe.ts)
which is incompatible with the current tsconfig target ES2017; either bump the
frontend tsconfig "target" to "ES2022" (or later) so Error.cause is emitted
safely, or remove the cause usage by wrapping and attaching the inner error
manually (e.g., set a .cause or .inner property on a new Error or include the
original error message in the new Error) at each location that constructs Errors
with the second options argument (lines shown around the STRIPE_* error
constructions).
In `@frontend/lib/tests/checkout-stripe-error-contract.test.ts`:
- Around line 6-13: The mocked Stripe env in vi.mock (getStripeEnv and
isPaymentsEnabled) uses realistic-looking keys that trigger secret scans; update
the mock to use clearly non-secret placeholders or add your repository's
allowed-scan pragma so CI won't flag it. Specifically, change secretKey and
webhookSecret returned by getStripeEnv to non-sensitive tokens like
"<stripe_test_key_placeholder>" and "<stripe_webhook_placeholder>" or apply the
approved suppression comment/annotation used in other tests, ensuring
getStripeEnv and isPaymentsEnabled remain functionally identical for the tests.
In `@frontend/lib/tests/stripe-webhook-mismatch.test.ts`:
- Around line 99-101: The test builds a checkoutBody with empty-string variant
fields which can fail min-length validation; in the test
(stripe-webhook-mismatch.test.ts) update the checkoutBody construction used by
the test to omit selectedSize and selectedColor when variants are not needed (or
supply valid non-empty values) so the payload only includes items: [{ productId,
quantity: 1 }], ensuring the checkoutBody variable and any assertions that read
it (e.g., in the webhook/mismatch test) are adjusted accordingly.
♻️ Duplicate comments (2)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
33-39: Locale-aware timestamp formatting now implemented.The
formatDateTimefunction correctly accepts and uses the locale parameter, addressing the previous review feedback. The basic locale codes from Next.js routing ('en', 'uk', 'pl') are valid BCP 47 tags and work correctly withtoLocaleString().frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts (1)
6-13: Duplicate: secret‑scan hit on mock Stripe keys.
Same concern as in the other checkout test; consider allowlisting or non‑secret placeholders.
🧹 Nitpick comments (8)
frontend/components/shop/catalog-products-client.tsx (2)
1-2: Remove development artifact.The Windows file path comment appears to be a development artifact and should be removed before merging.
Suggested fix
-// C:\Users\milka\devlovers.net-clean\frontend\components\shop\catalog-products-client.tsx - 'use client';
132-135: Consider using<nav>for pagination controls.The
<footer>element is a landmark that typically represents concluding content for its sectioning ancestor. For pagination controls,<nav>with an appropriatearia-labelis more semantically accurate and helps assistive technologies identify it as a navigation region.Suggested change
- <footer + <nav className="mt-12 flex flex-col items-center gap-3" aria-label="Catalog pagination" > <CatalogLoadMore hasMore={hasMore} isLoading={isLoadingMore} onLoadMore={onLoadMore} /> {error ? ( <p className="text-sm text-muted-foreground" role="status" aria-live="polite" > {error} </p> ) : null} - </footer> + </nav>frontend/lib/services/orders/psp-metadata/refunds.ts (1)
21-23: Missing validation when returningm.refundsdirectly.Line 23 returns
m.refundscast asRefundMetaRecord[]without validating that the array elements conform to the expected shape. If corrupted data exists in the database, this could propagate invalid records downstream.🛡️ Optional: Add runtime validation for array elements
- if (Array.isArray(m.refunds)) return m.refunds as RefundMetaRecord[]; + if (Array.isArray(m.refunds)) { + return m.refunds.filter( + (r: unknown): r is RefundMetaRecord => + r != null && + typeof r === 'object' && + typeof (r as any).refundId === 'string' + ); + }frontend/app/api/shop/webhooks/stripe/route.ts (1)
781-782: Consider centralizing the repeatedREFUND_FULLNESS_UNDETERMINEDerror.The same error is thrown in multiple places throughout the refund handling logic. A helper function would reduce repetition and ensure consistent behavior.
♻️ Optional: Extract error throw to helper
function throwRefundFullnessUndetermined(): never { throw new Error('REFUND_FULLNESS_UNDETERMINED'); }Then replace all instances of
throw new Error('REFUND_FULLNESS_UNDETERMINED');withthrowRefundFullnessUndetermined();.Also applies to: 811-812, 844-844, 897-898, 929-930, 963-964
frontend/lib/services/products/cart/rehydrate.ts (1)
159-160: Verify minor-unit formatting matches supported currencies.
fromMinorUnitscurrently assumes 2 decimals viafromCents. If you support currencies like JPY (0 decimals) or BHD (3 decimals), display values will be off—please confirm the currency set or consider a currency-aware formatter.Also applies to: 183-183
frontend/lib/tests/restock-release-failure-invariant.test.ts (1)
177-216: Consider using a test utility orbeforeEach/afterEachfor cleanup.The manual cleanup in the
finallyblock is thorough but verbose and error-prone if schema changes. A shared test utility or Vitest's lifecycle hooks with transaction rollback would reduce boilerplate and ensure consistent cleanup across tests.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (2)
58-62: Minor: Redundant optional chaining.After the
if (!order) notFound()guard on line 51,orderis guaranteed to be non-null. The optional chaining onorder?is redundant but harmless.♻️ Suggested simplification
const currency = orderCurrency(order, locale); - const totalMinor = pickMinor(order?.totalAmountMinor, order?.totalAmount); + const totalMinor = pickMinor(order.totalAmountMinor, order.totalAmount);
223-241: Consider removing redundant optional chaining onitem.The
itemis an element fromorder.items.map(), which should always be defined (unless the array contains null/undefined elements, which is atypical). Theitem?.optional chaining is defensive but may be unnecessary.♻️ Suggested simplification
{order.items.map(item => { const unitMinor = pickMinor( - item?.unitPriceMinor, - item?.unitPrice + item.unitPriceMinor, + item.unitPrice ); const lineMinor = pickMinor( - item?.lineTotalMinor, - item?.lineTotal + item.lineTotalMinor, + item.lineTotal );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/app/api/shop/catalog/route.tsfrontend/app/api/shop/webhooks/stripe/route.tsfrontend/app/globals.cssfrontend/components/shop/catalog-products-client.tsxfrontend/lib/psp/stripe.tsfrontend/lib/services/orders/checkout.tsfrontend/lib/services/orders/psp-metadata/refunds.tsfrontend/lib/services/orders/refund.tsfrontend/lib/services/orders/restock.tsfrontend/lib/services/products/cart/rehydrate.tsfrontend/lib/tests/checkout-set-payment-intent-reject-contract.test.tsfrontend/lib/tests/checkout-stripe-error-contract.test.tsfrontend/lib/tests/helpers/makeCheckoutReq.tsfrontend/lib/tests/restock-release-failure-invariant.test.tsfrontend/lib/tests/stripe-webhook-mismatch.test.tsfrontend/project-structure.txt
💤 Files with no reviewable changes (1)
- frontend/lib/services/orders/checkout.ts
🧰 Additional context used
🧬 Code graph analysis (10)
frontend/lib/services/orders/restock.ts (1)
frontend/lib/services/inventory.ts (1)
applyReleaseMove(118-178)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (2)
frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/lib/tests/stripe-webhook-mismatch.test.ts (1)
frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
paymentStatus(8-8)
frontend/lib/services/orders/refund.ts (2)
frontend/lib/services/errors.ts (1)
InvalidPayloadError(37-43)frontend/lib/services/orders/psp-metadata/refunds.ts (2)
normalizeRefundsFromMeta(17-41)appendRefundToMeta(43-65)
frontend/lib/services/products/cart/rehydrate.ts (1)
frontend/lib/shop/money.ts (1)
fromCents(44-46)
frontend/lib/tests/checkout-stripe-error-contract.test.ts (2)
frontend/app/api/shop/checkout/route.ts (1)
POST(151-527)frontend/lib/tests/helpers/makeCheckoutReq.ts (1)
makeCheckoutReq(10-58)
frontend/app/api/shop/webhooks/stripe/route.ts (1)
frontend/lib/services/orders/psp-metadata/refunds.ts (1)
appendRefundToMeta(43-65)
frontend/app/[locale]/shop/products/page.tsx (2)
frontend/components/shop/products-toolbar.tsx (1)
ProductsToolbar(9-136)frontend/components/shop/product-filters.tsx (1)
ProductFilters(9-183)
frontend/app/api/shop/catalog/route.ts (1)
frontend/lib/logging.ts (1)
logWarn(95-97)
frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts (2)
frontend/lib/services/errors.ts (1)
InvalidPayloadError(37-43)frontend/lib/tests/helpers/makeCheckoutReq.ts (1)
makeCheckoutReq(10-58)
🪛 Gitleaks (8.30.0)
frontend/lib/tests/checkout-stripe-error-contract.test.ts
[high] 95-95: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (46)
frontend/app/globals.css (2)
35-37: Clean base radius variable addition.Nice separation of base vs derived radius values; looks consistent with the rest of the theme variables.
166-169: Shop scope override aligns with new base radius.Switch to --radius-base keeps the override consistent with the theme refactor.
frontend/app/api/shop/catalog/route.ts (1)
6-6: LGTM! Good observability improvement.The
logWarnaddition appropriately captures invalid query parameters with structured metadata (original query + flattened Zod issues) without altering the existing fallback behavior. This will help diagnose malformed requests in production.Also applies to: 31-36
frontend/components/shop/catalog-products-client.tsx (3)
105-111: LGTM!Good improvement to error handling. The contextual logging with
locale,baseQuery, andnextPagewill aid debugging without exposing sensitive details to users.
119-130: Good semantic improvements.The change from
div-based layout to semanticsection > ul > listructure improves accessibility and document outline. Themin-w-0class on list items correctly prevents grid overflow issues with long content.
142-150: LGTM!The
role="status"witharia-live="polite"correctly announces errors to screen reader users without being intrusive.frontend/lib/services/orders/psp-metadata/refunds.ts (4)
1-9: LGTM! Clean type definition.The
RefundMetaRecordtype is well-structured with appropriate fields for tracking refund metadata, including the optionalstatusfield.
11-15: LGTM! Safe coercion helper.The
ensureMetaObjectfunction correctly handles null, non-object, and array inputs by returning an empty object as a safe fallback.
25-38: LGTM! Legacy format migration is well-handled.The legacy refund format conversion correctly maps fields and provides sensible defaults (
'legacy:webhook'for idempotencyKey,'webhook'for createdBy).
43-65: LGTM! Deduplication logic is correct.The
appendRefundToMetafunction properly:
- Normalizes existing refunds using the new record's fallback values
- Checks for duplicates by both
refundIdandidempotencyKey- Preserves
refundInitiatedAtif already setfrontend/app/api/shop/webhooks/stripe/route.ts (4)
11-14: LGTM! Clean import of new utilities.The imports from the new
psp-metadata/refundsmodule are properly structured, bringing in both the type and the helper function needed for refund metadata management.
25-28: LGTM! Proper delegation to centralized utility.The
upsertRefundIntoMetafunction now correctly:
- Validates
prevMetashape before use (lines 25-28)- Returns early if no refund data exists (line 30)
- Delegates to
appendRefundToMetafor the actual merge logic (line 42)This eliminates code duplication and ensures consistent refund handling across the codebase.
Also applies to: 42-42
206-213: Good defensive guard for canonical fields.Stripping
refundsandrefundInitiatedAtfrom the delta before merging prevents accidental overwrites of these canonical fields that are exclusively managed byupsertRefundIntoMeta. The comment on line 206 is helpful for future maintainers.
1080-1094: LGTM! Proper error handling for refund fullness issues.The catch block correctly identifies
REFUND_FULLNESS_UNDETERMINEDerrors and returns a 500 status without acknowledging the event, allowing Stripe to retry. This is the correct approach for transient issues where charge data may not be fully populated yet.frontend/lib/services/orders/refund.ts (5)
8-11: LGTM! Proper imports from new utility module.The imports correctly bring in both
appendRefundToMetaandnormalizeRefundsFromMetafor domain-level idempotency checking and metadata persistence.
13-17: LGTM! Helper for custom error codes.The
invalidhelper correctly creates anInvalidPayloadErrorand attaches a custom code for more specific error handling downstream.
84-95: LGTM! Domain-level idempotency check.The idempotency check using
normalizeRefundsFromMetais well-designed:
- Extracts existing refunds with appropriate fallback values
- Finds matching refund by
idempotencyKey- Returns early with order summary if already processed
This prevents duplicate Stripe API calls for the same refund request.
109-120: LGTM! Clean metadata construction.The
appendRefundToMetacall properly constructs the new refund record with all required fields from the Stripe response and request context.
122-130: LGTM! Correct persistence strategy.The comment on line 122 clearly documents the design decision: metadata is persisted immediately, but
paymentStatusis not touched because the webhook is the source of truth for payment state transitions. This prevents race conditions between the API response and webhook delivery.frontend/lib/services/products/cart/rehydrate.ts (1)
5-21: Clean import + alias addition looks good.No behavior change; improves readability around display conversions.
frontend/lib/services/orders/restock.ts (1)
210-219: Ignoringapplied: falsereturn value is a semantic change.The new logic only treats thrown exceptions as failures. Per the
applyReleaseMovecontract ininventory.ts, it returns{ ok: true, applied: false }foralready,no_reserve, ornoopstatuses—these are now silently treated as successes.This simplification appears intentional and is safe for idempotent retries (
already). However,no_reserveindicates the reservation row we just queried no longer exists, which could mask a race condition or data inconsistency.Consider logging a warning when
applied === falsefor observability, even if not treating it as a failure.frontend/lib/tests/restock-release-failure-invariant.test.ts (1)
32-34: Mock change correctly aligns with the new exception-based failure semantics.The switch from a resolved failure object to
mockRejectedValue(new Error(...))properly tests the updated control flow where only thrown errors are collected asreleaseFailures.frontend/app/[locale]/shop/products/page.tsx (3)
33-47: LGTM - Clean canonicalization logic.The redirect logic correctly strips the
pageparameter while preserving other filters, which is appropriate for infinite-scroll UX where shareable URLs shouldn't include page state.
63-72: Well-structured semantic header.Good use of semantic
<header>element with responsive flex layout. TheSuspenseboundary aroundProductsToolbaris appropriate since the toolbar uses client-side hooks (useSearchParams).
84-105: Good accessibility pattern for results section.The
role="status"witharia-live="polite"correctly announces empty results to assistive technologies without interrupting the user. The helpful hint text improves UX.frontend/project-structure.txt (1)
140-144: LGTM!The project structure updates accurately reflect the new admin functionality: order pages, admin UI components (
admin-pagination.tsx,shop-admin-topbar.tsx,products-toolbar.tsx), auth guard (guard-shop-admin-page.ts), HTTP helper (getBaseUrl.ts), and category images. The organization follows existing patterns.Also applies to: 220-222, 233-233, 362-362, 384-385, 499-503
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (5)
19-31: LGTM!The
pickMinorhelper gracefully handles both the new minor-unit field and legacy major field with proper conversion. TheorderCurrencyfunction correctly sanitizes to the supported currency enum values (UAH|USD).
64-92: Well-structured header with proper accessibility.Good use of semantic HTML (
<main>,<header>), ARIA labeling (aria-labelledby), and responsive layout for the actions container. Thebreak-allclass appropriately handles long order IDs.
94-182: LGTM!Excellent semantic structure using
<section>,<article>, and<dl>elements with proper ARIA attributes. The two-column responsive grid provides a clean layout, and locale-aware formatting is consistently applied to all timestamps.
184-221: Excellent table accessibility.Proper use of
<caption>(sr-only),scope="col"on headers, and semantic table structure. The visually-hidden section heading and caption ensure screen reader users understand the content context.
274-280: LGTM!The empty state fallback is correctly placed after the
.map()call, ensuring it renders only when the items array is empty.frontend/lib/psp/stripe.ts (4)
121-123: LGTM: preserving Stripe error cause is valuable.
146-148: LGTM: consistent error cause propagation.
166-168: LGTM: improved diagnostics on charge retrieval failures.
199-200: LGTM: richer context for webhook signature errors.frontend/lib/tests/checkout-stripe-error-contract.test.ts (3)
3-3: Shared request helper import is clean.
66-74: Stripe error path assertions are solid.
94-102: Existing-order Stripe error path looks consistent.frontend/lib/tests/stripe-webhook-mismatch.test.ts (4)
8-9: Typed row wrapper is a nice clarity win.
24-37: Safer row extraction reads well.
124-133: Expected amount/currency retrieval is clearer now.
197-210: Typed status/metadata extraction looks good.frontend/lib/tests/helpers/makeCheckoutReq.ts (1)
1-58: Helper encapsulation is clean and reusable.frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts (3)
2-2: Helper import aligns test scaffolding.
73-82: Updated rejection path reads well.
111-121: Existing-order rejection path is consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…e, switch admin prices to minor units, and DRY pagination/date/empty-state tweaks
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/.env.example (1)
20-21: DuplicateNEXT_PUBLIC_SITE_URLentry.Line 21 duplicates line 20. Remove one to avoid confusion.
Proposed fix
NEXT_PUBLIC_SITE_URL= -NEXT_PUBLIC_SITE_URL=
🧹 Nitpick comments (5)
frontend/app/api/shop/admin/orders/reconcile-stale/route.ts (1)
33-38: Consider a distinct error code for malformed requests.Returning
CSRF_INVALIDwhenformData()parsing fails conflates "missing/invalid token" with "malformed request body." A separate code likeINVALID_REQUEST_BODYwould aid debugging.Proposed refinement
let form: FormData; try { form = await request.formData(); } catch { - return NextResponse.json({ code: 'CSRF_INVALID' }, { status: 403 }); + return NextResponse.json({ code: 'INVALID_REQUEST_BODY' }, { status: 400 }); }frontend/app/[locale]/shop/admin/orders/page.tsx (2)
27-30: Consider adding a type annotation instead ofany.Using
anyfor theorderparameter bypasses type safety. If a proper type exists (e.g., from the query result), consider using it.
203-207: Consider placingAdminPaginationoutside theoverflow-x-autocontainer.Placing pagination controls inside the horizontally scrollable container may cause them to be hidden or require scrolling on narrow viewports. Moving them outside ensures they remain accessible.
Proposed adjustment
</table> - - <AdminPagination - basePath="/shop/admin/orders" - page={page} - hasNext={hasNext} - /> </div> + + <AdminPagination + basePath="/shop/admin/orders" + page={page} + hasNext={hasNext} + /> </section>frontend/app/[locale]/shop/admin/products/page.tsx (1)
273-277: Consider placingAdminPaginationoutside theoverflow-x-autocontainer.Same suggestion as the orders page — pagination controls may be hidden or require scrolling on narrow viewports when nested inside the scrollable container.
Proposed adjustment
</table> - - <AdminPagination - basePath="/shop/admin/products" - page={page} - hasNext={hasNext} - /> </div> + + <AdminPagination + basePath="/shop/admin/products" + page={page} + hasNext={hasNext} + /> </section>frontend/lib/security/csrf.ts (1)
59-76: Harden payload validation (iat/exp sanity).Add basic
iatchecks and ensureexp >= iatto reject malformed payloads and reduce edge-case acceptance.♻️ Proposed hardening
- if (!payload || payload.p !== purpose) return false; - if (typeof payload.exp !== 'number' || payload.exp < now) return false; + if (!payload || payload.p !== purpose) return false; + if (typeof payload.iat !== 'number' || !Number.isFinite(payload.iat)) return false; + if (typeof payload.exp !== 'number' || !Number.isFinite(payload.exp)) return false; + if (payload.exp < now || payload.exp < payload.iat) return false;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/.env.examplefrontend/app/[locale]/shop/admin/orders/page.tsxfrontend/app/[locale]/shop/admin/products/page.tsxfrontend/app/api/shop/admin/orders/reconcile-stale/route.tsfrontend/lib/pagination.tsfrontend/lib/security/csrf.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/[locale]/shop/admin/orders/page.tsx (3)
frontend/lib/shop/currency.ts (2)
resolveCurrencyFromLocale(23-28)formatMoney(115-128)frontend/lib/security/csrf.ts (2)
issueCsrfToken(42-57)CSRF_FORM_FIELD(6-6)frontend/components/shop/admin/admin-pagination.tsx (1)
AdminPagination(18-76)
🪛 dotenv-linter (4.0.0)
frontend/.env.example
[warning] 45-45: [UnorderedKey] The GMAIL_APP_PASSWORD key should go before the GMAIL_USER key
(UnorderedKey)
[warning] 46-46: [UnorderedKey] The EMAIL_FROM key should go before the GMAIL_APP_PASSWORD key
(UnorderedKey)
[warning] 47-47: [UnorderedKey] The APP_URL key should go before the EMAIL_FROM key
(UnorderedKey)
🔇 Additional comments (12)
frontend/.env.example (1)
40-49: New environment variables look good.The additions for
CSRF_SECRET, email configuration, and other variables align with the new CSRF protection and feature requirements introduced in this PR.frontend/lib/pagination.ts (1)
1-5: LGTM!Clean utility with proper edge-case handling. The
Number.isFinitecheck correctly handlesNaNandInfinity, and the fallback to1is a safe default for pagination.frontend/app/api/shop/admin/orders/reconcile-stale/route.ts (1)
22-46: CSRF protection implementation looks solid.The layered approach (auth → origin check → token verification) provides good defense-in-depth. The purpose string
'admin:orders:reconcile-stale'correctly matches the token issued on the admin orders page.frontend/app/[locale]/shop/admin/orders/page.tsx (3)
32-35: Locale-aware date formatting is now correctly implemented.The
formatDatefunction now accepts and uses thelocaleparameter, addressing the previous review feedback.
78-79: CSRF token integration looks correct.The token is issued with purpose
'admin:orders:reconcile-stale'and passed via hidden input, matching the server-side verification.
66-92: Semantic structure and accessibility improvements look good.The use of
<main>,<header>,<section>,aria-labelledby, table<caption>, and properscope="col"on headers follows accessibility best practices.frontend/app/[locale]/shop/admin/products/page.tsx (4)
16-18: Dynamic rendering and reduced page size are appropriate for admin pages.The
force-dynamicexport ensures fresh data on each request, and PAGE_SIZE of 25 balances usability with load.
20-23: Locale-awareformatDateis consistent with the orders page.Good consistency across admin pages.
96-100: Semantic structure and accessibility improvements look good.Consistent use of
<main>,<section>,aria-label, table<caption>, andscope="col"attributes follows accessibility best practices.
172-258: Row rendering and price formatting look correct.The
priceMinorhandling withformatMoneyand the truncated text cells withtitleattributes for tooltip fallback are well implemented.frontend/lib/security/csrf.ts (2)
84-95: Same-origin fallback logic looks solid.Origin → Referer fallback is a good, defensive pattern here.
42-57: Same-origin enforcement is present at the only call site; consider documenting the security assumption.The token payload is stateless and not user-bound, which is a valid design concern. However, the current call site in
frontend/app/api/shop/admin/orders/reconcile-stale/route.tsproperly mitigates this by enforcingisSameOrigin()(line 26) before verifying the token (line 43), and additionally requires admin authentication (line 23). This binding viarequireAdminApi()provides the session/user context that prevents cross-user replay.If additional CSRF endpoints are added in future, ensure they follow the same pattern: enforce same-origin or user binding before token verification. Consider adding a brief comment in the CSRF module documenting this assumption for future maintainers.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…-aware admin dates, a11y toggle+modal focus trap, cleanup props/lint, stable order UI + safer cart clearing
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/components/shop/product-filters.tsx (1)
3-32: Preserve the locale segment when updating filter URLs.Hardcoding
/shop/productsdrops the[locale]segment, causing navigation failures for non-default locales. Theproduct-sort.tsxcomponent in the same directory already handles this correctly usinguseParams()to extract locale. Use eitherusePathname()or the same pattern asproduct-sort.tsxto build the correct path.💡 Suggested fix
-import { useRouter, useSearchParams } from 'next/navigation'; +import { usePathname, useRouter, useSearchParams } from 'next/navigation'; @@ const router = useRouter(); const searchParams = useSearchParams(); + const pathname = usePathname(); @@ - router.push( - queryString ? `/shop/products?${queryString}` : '/shop/products' - ); + router.push(queryString ? `${pathname}?${queryString}` : pathname);
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/shop/admin/products/_components/product-form.tsx:
- Around line 240-248: The slug can be stale during quick submits in create mode
because slugValue currently prefers the possibly-lagging state variable slug;
change the useMemo for slugValue so that in create mode it always derives from
the current title (use localSlugify(title)) rather than using slug, while
retaining the existing behavior for mode === 'edit'. Update the useMemo that
defines slugValue to: if (mode === 'edit') return slug; else return
localSlugify(title), and ensure title is in the dependency array so slugValue
updates immediately when title changes (keep useEffect/setSlug for syncing but
don't rely on slug for create-mode output).
♻️ Duplicate comments (1)
frontend/app/[locale]/shop/products/page.tsx (1)
75-82: Avoid nested<aside>landmarks.
ProductFiltersalready renders an<aside>, so wrapping it in another<aside>creates duplicate landmarks for screen readers. Consider using a<div>wrapper or make the filter container configurable.
🧹 Nitpick comments (7)
frontend/components/header/UnifiedHeader.tsx (1)
15-20: Remove unusedenableSearchprop from the interface.The
enableSearchprop is defined inUnifiedHeaderPropsbut is not destructured or used anywhere in the component. This creates a misleading API where callers might pass this prop expecting it to have an effect.♻️ Suggested fix
export type UnifiedHeaderProps = { variant: UnifiedHeaderVariant; userExists: boolean; showAdminLink?: boolean; - enableSearch?: boolean; };frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
51-61: Good accessibility improvements.The use of
useId()for linking the error message viaaria-describedbyandrole="alert"is well-implemented for screen reader support.Minor note:
aria-disabledis redundant on native<button>elements whendisabledis already set—the browser conveys the disabled state to assistive technologies automatically. You can safely removearia-disabledhere, but keeping it causes no harm.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
57-61: Unnecessary optional chaining.At line 59,
order?.totalAmountMinoruses optional chaining, butorderis guaranteed to be non-null after the guard on line 50. This is harmless but slightly misleading.Suggested fix
- const totalMinor = pickMinor(order?.totalAmountMinor, order?.totalAmount); + const totalMinor = pickMinor(order.totalAmountMinor, order.totalAmount);frontend/app/[locale]/shop/cart/page.tsx (1)
219-224: Consider removingaria-labelfrom static content.The
aria-label="Quantity"on a<span>containing the quantity number may cause screen readers to announce "Quantity" instead of the actual number. Screen readers will naturally read the text content.♻️ Suggested improvement
- <span - className="w-8 text-center text-sm font-medium" - aria-label="Quantity" - > - {item.quantity} - </span> + <span className="w-8 text-center text-sm font-medium"> + {item.quantity} + </span>Alternatively, if you want to provide context, consider a visually hidden label before the quantity:
<span className="sr-only">Quantity:</span> <span className="w-8 text-center text-sm font-medium">{item.quantity}</span>frontend/app/[locale]/shop/orders/[id]/page.tsx (1)
203-221: Consider locale-aware formatting for totals and timestamps.Right now totals and dates are rendered as raw strings (ISO timestamps, unformatted amounts). Since
localeis available, formatting will improve readability and align with the PR’s semantic/UX goals.♻️ Example (if no shared formatter exists)
+ const currencyFormatter = new Intl.NumberFormat(locale, { + style: 'currency', + currency: String(order.currency), + }); + const dateFormatter = new Intl.DateTimeFormat(locale, { + dateStyle: 'medium', + timeStyle: 'short', + }); ... - <dd className="text-sm font-medium"> - {order.totalAmount} {String(order.currency)} - </dd> + <dd className="text-sm font-medium"> + {currencyFormatter.format(Number(order.totalAmount))} + </dd> ... - <dd className="text-sm">{order.createdAt}</dd> + <dd className="text-sm"> + {dateFormatter.format(new Date(order.createdAt))} + </dd>If
totalAmountis a decimal string, prefer any existing money formatter in the codebase to avoid floating‑point issues.frontend/components/shop/admin/admin-product-status-toggle.tsx (1)
48-64: Addaria-pressedto expose toggle state to assistive technology.This is a toggle button and
aria-pressedcommunicates the on/off state to screen readers. Update the attribute when the button is pressed.♻️ Suggested change
aria-busy={isLoading} + aria-pressed={isActive} aria-describedby={error ? errorId : undefined}frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (1)
188-238: UseproductIdas the hydration key when available.
Keying only by slug/title can accidentally suppress re-hydration if those fields match across products.♻️ Proposed tweak
- const key = - (typeof initialValues.slug === 'string' && - initialValues.slug.trim().length - ? initialValues.slug - : null) ?? - (typeof initialValues.title === 'string' && - initialValues.title.trim().length - ? initialValues.title - : null); + const key = + productId ?? + (typeof initialValues.slug === 'string' && + initialValues.slug.trim().length + ? initialValues.slug + : null) ?? + (typeof initialValues.title === 'string' && + initialValues.title.trim().length + ? initialValues.title + : null); @@ - }, [mode, initialValues]); + }, [mode, initialValues, productId]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
frontend/app/[locale]/layout.tsxfrontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsxfrontend/app/[locale]/shop/admin/orders/[id]/page.tsxfrontend/app/[locale]/shop/admin/products/_components/product-form.tsxfrontend/app/[locale]/shop/cart/page.tsxfrontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsxfrontend/app/[locale]/shop/checkout/success/page.tsxfrontend/app/[locale]/shop/orders/[id]/page.tsxfrontend/app/[locale]/shop/orders/error.tsxfrontend/app/[locale]/shop/orders/page.tsxfrontend/app/[locale]/shop/products/[slug]/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/components/header/AppChrome.tsxfrontend/components/header/UnifiedHeader.tsxfrontend/components/shop/add-to-cart-button.tsxfrontend/components/shop/admin/admin-pagination.tsxfrontend/components/shop/admin/admin-product-status-toggle.tsxfrontend/components/shop/admin/shop-admin-topbar.tsxfrontend/components/shop/cart-provider.tsxfrontend/components/shop/catalog-load-more.tsxfrontend/components/shop/catalog-products-client.tsxfrontend/components/shop/category-tile.tsxfrontend/components/shop/clear-cart-on-mount.tsxfrontend/components/shop/header/cart-button.tsxfrontend/components/shop/header/nav-links.tsxfrontend/components/shop/product-filters.tsxfrontend/components/shop/products-toolbar.tsxfrontend/components/shop/shop-hero.tsxfrontend/lib/tests/checkout-currency-policy.test.tsfrontend/lib/tests/stripe-webhook-paid-status-repair.test.ts
💤 Files with no reviewable changes (1)
- frontend/lib/tests/stripe-webhook-paid-status-repair.test.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/lib/tests/checkout-currency-policy.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/components/shop/header/cart-button.tsx
- frontend/app/[locale]/shop/products/[slug]/page.tsx
- frontend/components/shop/add-to-cart-button.tsx
- frontend/components/shop/cart-provider.tsx
- frontend/components/shop/shop-hero.tsx
- frontend/components/shop/catalog-products-client.tsx
🧰 Additional context used
🧬 Code graph analysis (11)
frontend/components/shop/category-tile.tsx (1)
frontend/lib/shop/data.ts (1)
ShopCategory(28-33)
frontend/components/shop/clear-cart-on-mount.tsx (1)
frontend/components/shop/cart-provider.tsx (1)
useCart(291-293)
frontend/components/shop/admin/admin-pagination.tsx (1)
frontend/lib/utils.ts (1)
cn(4-6)
frontend/components/shop/product-filters.tsx (2)
frontend/lib/config/catalog.ts (4)
CATEGORIES(1-9)PRODUCT_TYPES(11-16)COLORS(18-28)SIZES(30-30)frontend/lib/utils.ts (1)
cn(4-6)
frontend/components/shop/products-toolbar.tsx (2)
frontend/components/shop/product-sort.tsx (1)
ProductSort(15-76)frontend/components/shop/product-filters.tsx (1)
ProductFilters(9-179)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (2)
frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/components/shop/header/nav-links.tsx (2)
frontend/lib/config/catalog.ts (1)
CATEGORIES(1-9)frontend/lib/utils.ts (1)
cn(4-6)
frontend/app/[locale]/shop/cart/page.tsx (2)
frontend/components/shop/cart-provider.tsx (1)
useCart(291-293)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/app/[locale]/shop/products/page.tsx (2)
frontend/components/shop/products-toolbar.tsx (1)
ProductsToolbar(9-202)frontend/components/shop/product-filters.tsx (1)
ProductFilters(9-179)
frontend/app/[locale]/shop/orders/page.tsx (3)
frontend/lib/auth.ts (1)
getCurrentUser(95-116)frontend/db/index.ts (1)
db(54-54)frontend/lib/logging.ts (1)
logError(87-93)
frontend/components/header/UnifiedHeader.tsx (1)
frontend/components/shop/header/nav-links.tsx (1)
NavLinks(45-119)
🔇 Additional comments (57)
frontend/components/shop/catalog-load-more.tsx (1)
17-27: Nice accessibility + layout polish. The centered wrapper, explicittype="button", andaria-busystate are solid improvements with no behavioral regressions.frontend/components/shop/category-tile.tsx (5)
1-7: LGTM!Clean imports and properly typed interface aligned with the
ShopCategorytype definition.
10-21: Good accessibility and URL safety improvements.
encodeURIComponentcorrectly handles special characters in category slugs.- The
aria-labelprovides clear context for screen reader users.- Focus-visible styles ensure keyboard navigation is visible.
- The className array pattern improves readability for long class lists.
23-29: LGTM!Image component is well-configured with appropriate alt text, fill behavior, and responsive
sizeshints for optimization.
31-34: LGTM!Gradient overlay correctly uses
aria-hidden="true"since it's purely decorative and improves text contrast over the image.
36-46: Good semantic and accessibility structure.
- The
aria-hidden="true"on the "Shop now" span correctly prevents redundant announcements since the Link'saria-labelalready conveys the action.- Responsive typography scales well across breakpoints.
One consideration: The
<h3>heading level assumes this tile appears in a context where<h2>exists above it. If this component is used in varying contexts, you might consider accepting the heading level as a prop for flexibility. However, this is a minor point if the component is consistently used within a known page structure.frontend/components/shop/admin/shop-admin-topbar.tsx (1)
5-54: LGTM! Well-structured semantic markup.The refactor to use
<header>,<nav>, and<ol>/<li>elements improves accessibility. The breadcrumb separator is correctly markedaria-hidden="true", and the ARIA label on the nav provides clear context for screen reader users.frontend/components/shop/admin/admin-pagination.tsx (1)
30-72: LGTM! Excellent pagination accessibility.The semantic
<nav>with<ul>/<li>structure, combined with proper ARIA attributes (aria-label,aria-current="page",aria-disabled) andrel="prev"/rel="next"on links, follows pagination best practices for accessibility.frontend/components/shop/header/nav-links.tsx (2)
62-73: LGTM! Appropriate use of memoization.The
useMemocorrectly captures the dependencies[pathname, currentCategory]for recomputing active link states. This avoids unnecessary recalculations when other state changes.
75-117: LGTM! Well-structured semantic navigation.The refactor to
<nav>><ul>><li>structure with properaria-current="page"for active states follows accessibility best practices.frontend/components/header/UnifiedHeader.tsx (1)
85-94: LGTM! Clean admin link implementation.The admin link is properly gated by
showAdminLink, uses appropriate ARIA labeling, and the Settings icon is correctly markedaria-hidden="true"since the link has an accessible name viaaria-label.frontend/app/[locale]/layout.tsx (1)
34-42: LGTM! Proper admin link visibility logic.The three-way condition (
user exists,admin role,feature flag) is a sensible approach for gating admin UI visibility. The?? ''fallback ensures safe string operations on potentially undefined env vars.Note: Ensure that actual admin route access is enforced server-side, as client-side visibility controls can be bypassed.
frontend/components/header/AppChrome.tsx (1)
15-19: Formatting-only change looks good.
No behavioral impact; just cleaner destructuring.frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
72-76: LGTM!The error message is properly accessible with
role="alert"for live announcement and linked to the button viaaria-describedby.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (4)
32-38: Locale-aware date formatting is now correctly implemented.The
formatDateTimefunction properly accepts and uses the locale parameter, addressing the previous review feedback.
67-70: Well-structured semantic markup.Good use of
<main>witharia-labelledbylinking to the page heading. The overall semantic structure with<section>,<article>, and proper heading hierarchy improves accessibility significantly.
222-270: Item rendering is well-implemented.The items table correctly handles:
- Locale-aware currency formatting per item
- Fallback to '-' for missing values
- Proper semantic table structure with
scope="col"headers
183-190: Good accessibility for the items section.The visually hidden
<h2>and<caption>elements ensure screen reader users understand the table context without cluttering the visual design.frontend/app/[locale]/shop/cart/page.tsx (5)
1-22: LGTM!The imports and state setup are appropriate. The
useMemoforshopBaseis lightweight and correctly tracks thelocaledependency.
24-102: LGTM!The checkout flow is well-structured:
- Idempotent API call with proper error extraction.
createdOrderIdis tracked for fallback navigation.- Both Stripe and non-Stripe payment paths are handled with appropriate redirects.
- The
clearCart=1query param provides a clean mechanism for downstream cart clearing.
104-127: LGTM!The empty cart state is semantically correct with
<main>, proper heading hierarchy, and an accessible icon. TheLinkcomponent from@/i18n/routinghandles locale prefixing automatically, so the/shop/productshref is appropriate.
136-166: LGTM!Good use of semantic elements (
<section>,<ul>,<li>,<article>) and accessibility attributes. Thearia-live="polite"on the removed items notification ensures screen reader users are informed without interruption. The image fallback to placeholder is defensive.
268-366: LGTM!The order summary
<aside>is properly labeled witharia-labelledby. The checkout button correctly usesaria-busyduring loading. The fallback navigation link (Lines 330-340) and error state with "Go to order" link (Lines 351-362) provide excellent resilience for edge cases where navigation might fail after order creation.frontend/app/[locale]/shop/orders/[id]/page.tsx (1)
83-90: Theparamstyping in your code is correct per Next.js App Router specification.In Next.js 16+ App Router, route
paramsare always passed as aPromise. The current implementation withparams: Promise<{ locale: string; id: string }>andawait paramsfollows the official Next.js pattern and should not be changed.frontend/components/shop/admin/admin-product-status-toggle.tsx (4)
1-3: Nice accessibility-enabling import.Pulling in
useIdhere is a clean way to support per-instance IDs.
18-20: Good per-instance error ID wiring.This keeps aria references stable and unique per component instance.
26-42: Solid failure-path handling.Non‑OK and exception cases now converge on user‑visible feedback.
68-76: Accessible error announcement looks good.
role="status"+aria-live="polite"is a good fit here.frontend/app/[locale]/shop/orders/error.tsx (1)
1-25: LGTM — clear, focused error fallback with recovery action.The component provides a concise message and a single recovery path that keeps the error state easy to understand and navigate.
frontend/app/[locale]/shop/orders/page.tsx (11)
18-22: LGTM — concise ID shortening helper.Short, deterministic output with no edge-case surprises.
24-33: LGTM — defensive date formatting helper.The fallback keeps the UI resilient if formatting fails.
35-49: LGTM — clear status labeling mapping.
52-64: LGTM — consistent styling mapping for statuses.
66-75: LGTM — count normalization is safe and bounded.
77-80: LGTM — UUID detection helper is straightforward.
83-91: LGTM — headline composition handles empty and multi-item cases well.
94-109: LGTM — auth guard and locale extraction are straightforward.
110-167: LGTM — query assembly and error handling are clean.The capped fetch keeps payloads bounded.
169-203: LGTM — header and empty state structure are clear.
204-302: LGTM — table layout and per-row rendering look consistent.frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (3)
134-141: Good use ofuseIdfor stable ARIA wiring.
This sets you up for consistent, collision-freearia-*relationships across the form.
392-406: SALE rule errors are mapped cleanly to field-level feedback.
Nice mapping of server rule violations to per-currency errors plus a form-level message.
434-947: Strong semantic + accessibility overhaul.
The section/fieldset structure, explicit IDs, and live error regions noticeably improve screen-reader navigation and error discovery.frontend/components/shop/clear-cart-on-mount.tsx (1)
1-23: ClearCartOnMount is a clean, single-shot side-effect.The ref guard keeps the clear operation from re-running on re-renders while still honoring the
enabledflag.frontend/app/[locale]/shop/checkout/success/page.tsx (4)
36-39: Helper + shell setup looks good.The clear-cart flag parsing and the reusable shell keep the page structure consistent while staying straightforward.
Also applies to: 41-68, 79-80
84-103: Missing-order fallback UX is consistent.CTA layout and messaging align with the rest of the success flows.
114-144: Error fallbacks are clear and consistent.Both the not-found and generic failure paths keep the same structural pattern.
147-233: Order summary layout and semantics look solid.The structure is clear, and the summary presentation is easy to scan.
frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (5)
2-2: Dynamic rendering + clear-cart handling look good.The flag parsing and query handling are straightforward and keep the flow explicit.
Also applies to: 13-15, 43-49, 90-92
56-83: PageShell provides consistent chrome.Good consolidation for repeated UI structure.
98-157: Error states are consistent and actionable.The CTA grouping and messaging are uniform across all error variants.
213-239: Paid-order branch looks consistent.Matches the rest of the updated UI patterns.
243-332: Main payment layout and summary are cleanly structured.The layout reads well and keeps the details organized.
frontend/components/shop/product-filters.tsx (1)
35-176: Filter section semantics look solid.Headings, labeled sections, and toggle affordances read well and should be easy to navigate.
frontend/components/shop/products-toolbar.tsx (1)
9-199: Mobile filter dialog behavior looks robust.Focus handling, Escape-to-close, and scroll locking are thoughtfully implemented for the mobile overlay.
frontend/app/[locale]/shop/products/page.tsx (2)
33-47: Canonicalization block is clear and keeps URLs clean.Dropping
pagebefore fetching avoids shareable infinite-scroll states.
63-73: Semantic structure + empty-state status are nice improvements.The main/header/section layout and
role="status"help with navigation and announcements.Also applies to: 84-104
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx
Outdated
Show resolved
Hide resolved
…admin a11y polish (csrf hardening, tests, env hygiene)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@frontend/.env.example`:
- Around line 39-44: The example .env includes a default value for
INTERNAL_JANITOR_MIN_INTERVAL_SECONDS (=60) which is inconsistent with the other
empty variables and may mislead; either remove the "=60" so the example matches
the other keys, or keep "=60" but add a clarifying comment after
INTERNAL_JANITOR_MIN_INTERVAL_SECONDS explaining it is a recommended default
(e.g., "recommended default in seconds") and not required; update the line for
INTERNAL_JANITOR_MIN_INTERVAL_SECONDS accordingly and ensure consistency with
other entries like ENABLE_ADMIN_API and INTERNAL_JANITOR_SECRET.
- Around line 19-23: Update the .env.example to include the missing LOCAL and
PROD GitHub OAuth variables expected by frontend/lib/env/auth.ts: add
GITHUB_CLIENT_ID_LOCAL, GITHUB_CLIENT_SECRET_LOCAL,
GITHUB_CLIENT_REDIRECT_URI_LOCAL and GITHUB_CLIENT_ID_PROD,
GITHUB_CLIENT_SECRET_PROD, GITHUB_CLIENT_REDIRECT_URI_PROD following the same
pattern/format as the existing DEVELOP entries (and analogous to the Google
OAuth block) so the auth.ts code that reads LOCAL, DEVELOP, and PROD variants
will find all three environments' keys.
In `@frontend/app/`[locale]/shop/admin/products/page.tsx:
- Around line 211-213: The table cell currently only maps the string 'NONE' to
'-' and leaves a null/undefined badge blank; update the JSX conditional that
renders {row.badge === 'NONE' ? '-' : row.badge} to treat null/undefined the
same as 'NONE' (e.g., check row.badge == null || row.badge === 'NONE' or use a
nullish-coalescing fallback) so the cell shows '-' for missing badges; locate
and change the expression where row.badge is rendered in the product admin page
component.
♻️ Duplicate comments (1)
frontend/lib/tests/checkout-stripe-error-contract.test.ts (1)
6-14: Mock Stripe keys may trigger secret scanners (duplicate concern).This has already been flagged in a previous review. The same suggestion applies: use clearly dummy prefixes or add a suppression pragma.
🧹 Nitpick comments (11)
frontend/components/shop/catalog-products-client.tsx (1)
117-149: Solid accessibility improvements.Good semantic upgrades:
<section>witharia-labelprovides a navigable landmark.<ul>/<li>structure correctly conveys a list to assistive tech (announces item count).min-w-0on list items prevents grid overflow from long content.<nav>for pagination is semantically appropriate.aria-live="polite"on the error ensures screen readers announce failures without interrupting.One minor note:
role="status"implicitly setsaria-live="polite", so specifying both is redundant—though harmless.Optional: Remove redundant aria-live
<p className="text-sm text-muted-foreground" role="status" - aria-live="polite" >frontend/app/[locale]/shop/products/page.tsx (1)
74-82: Previous nested landmark issue addressed.The change from
<aside>to<div>correctly resolves the previously flagged nested landmark issue, sinceProductFiltersalready renders its own<aside aria-label="Product filters">.However, the
h2#filters-heading(lines 76-78) is not referenced by anyaria-labelledbyattribute on its parent container, making it an orphaned heading. SinceProductFiltersprovides its own accessible label, consider removing the orphaned heading to avoid redundancy:Optional: Remove orphaned sr-only heading
<div className="hidden lg:block"> - <h2 id="filters-heading" className="sr-only"> - Filters - </h2> <Suspense fallback={null}> <ProductFilters /> </Suspense> </div>frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts (1)
6-14: Avoid triggering secret scanners with mock Stripe keys.The placeholder values
stripe_secret_key_placeholderandstripe_webhook_secret_placeholdermay still trigger secret scanning tools in CI pipelines (as noted in the sibling test file). Consider using clearly dummy prefixes likesk_test_dummyor adding agitleaks:allowpragma comment to suppress false positives consistently across all test files.Additionally, the Ukrainian comment on line 13 (
// якщо десь ще використовується) may benefit from being in English for codebase consistency.♻️ Suggested suppression pragma
// Force payments enabled so route goes into Stripe flow +// gitleaks:allow vi.mock('@/lib/env/stripe', () => ({ getStripeEnv: () => ({ paymentsEnabled: true, mode: 'test', secretKey: 'sk_test_dummy', webhookSecret: 'whsec_test_dummy', }), - isPaymentsEnabled: () => true, // якщо десь ще використовується + isPaymentsEnabled: () => true, // kept for backward compatibility }));frontend/components/shop/product-filters.tsx (2)
111-113: Redundantaria-labelledbyon inner container.The
<section>at line 103 already hasaria-labelledby={colorGroupId}, so repeating it on the inner<div>is unnecessary—child elements inherit the labeled context from their parent section.♻️ Suggested cleanup
<div className="mt-4 flex flex-wrap gap-2" - aria-labelledby={colorGroupId} >
150-152: Same redundancy:aria-labelledbyduplicated on Size container.Similar to the Color section, the
<section>already provides the accessible label viaaria-labelledby={sizeGroupId}.♻️ Suggested cleanup
<div className="mt-4 flex flex-wrap gap-2" - aria-labelledby={sizeGroupId} >frontend/app/api/shop/webhooks/stripe/route.ts (1)
215-222: LGTM!Good safeguard to prevent
deltafrom overwriting the canonicalrefundsandrefundInitiatedAtfields managed byupsertRefundIntoMeta. The shallow copy beforedeletecorrectly avoids mutatingcleanedDelta.Minor style preference: destructuring with rest could achieve the same result more idiomatically:
♻️ Optional refactor using destructuring
- // Do NOT allow delta to overwrite refunds/refundInitiatedAt (canonical fields managed by upsertRefundIntoMeta) - const safeDelta: any = { ...cleanedDelta }; - delete safeDelta.refunds; - delete safeDelta.refundInitiatedAt; + // Do NOT allow delta to overwrite refunds/refundInitiatedAt (canonical fields managed by upsertRefundIntoMeta) + const { refunds: _, refundInitiatedAt: __, ...safeDelta } = cleanedDelta as any;frontend/lib/services/products/cart/rehydrate.ts (1)
23-35: Consider coupling the currency allowlist to the source of truth.The hardcoded check for
'USD' || 'UAH'duplicates knowledge from the currency enum. If new currencies are added to the schema, this guard must be updated manually.A few options to reduce drift:
- Import and reference the currency enum values directly.
- Add a comment referencing the schema location for maintainers.
- Create a shared
TWO_DECIMAL_CURRENCIESconstant.Also, using
productId: '__cart__'as a sentinel is unconventional—consider usingundefinedor a dedicated error property instead.♻️ Suggested improvement
+// Keep in sync with: frontend/db/legacy-migrations/drizzle_legacy/schema.ts currency enum +const TWO_DECIMAL_CURRENCIES: ReadonlySet<CurrencyCode> = new Set(['USD', 'UAH']); + function assertTwoDecimalCurrency(currency: CurrencyCode): void { // fromCents/toCents assume 2 fraction digits. // Guard against 0-decimal (JPY) / 3-decimal (BHD) and any future non-2-decimal currency. - if (currency === 'USD' || currency === 'UAH') return; + if (TWO_DECIMAL_CURRENCIES.has(currency)) return; throw new PriceConfigError( 'Unsupported currency minor units exponent in cart rehydrate (expected 2-decimal currency).', { - productId: '__cart__', currency, } ); }frontend/.env.example (2)
1-58: Consider alphabetical ordering within sections (dotenv-linter preference).The dotenv-linter tool flags several ordering inconsistencies where keys aren't alphabetically sorted within their sections. While the current logical grouping (e.g.,
*_IDbefore*_SECRETbefore*_REDIRECT_URI_*) is intuitive, alphabetical ordering can improve scanability and reduce merge conflicts.Examples:
- Lines 15-17: Redirect URIs could be alphabetically ordered
- Line 36:
NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYcould come beforePAYMENTS_ENABLED- Lines 54-55:
EMAIL_FROMcould come beforeGMAIL_APP_PASSWORD
39-58: Consider adding inline documentation for new variables.Several new environment variables lack explanatory comments about their purpose, expected format, or how to obtain values. Adding brief inline documentation would improve developer experience, especially for:
ENABLE_ADMIN_API: What features does this enable?INTERNAL_JANITOR_SECRET: How to generate, minimum entropy requirements?JANITOR_URL: Expected URL format, purpose?QUIZ_ENCRYPTION_KEY: Key format/length requirements, generation method?CSRF_SECRET: Key requirements, rotation policy?Example documentation style:
# --- Admin / Internal ops -ENABLE_ADMIN_API= +ENABLE_ADMIN_API= # Set to 'true' to enable admin dashboard and API routes -INTERNAL_JANITOR_SECRET= +INTERNAL_JANITOR_SECRET= # Secret token for internal maintenance endpoints (32+ chars recommended)frontend/app/[locale]/shop/admin/orders/page.tsx (1)
29-32: Consider extractingorderCurrencyto shared utilities.This helper is duplicated in
[id]/page.tsx. For maintainability, consider moving it to@/lib/shop/currency.tsalongside the other currency utilities.frontend/app/[locale]/shop/cart/page.tsx (1)
149-152: Prefer a stable key without the array index (if variants are unique).
Index keys can cause remounts when items are removed/reordered. If each cart line is unique per variant, a stable key is safer.♻️ Suggested key change
- key={`${item.productId}-${item.selectedSize}-${item.selectedColor}-${index}`} + key={`${item.productId}-${item.selectedSize ?? 'na'}-${item.selectedColor ?? 'na'}`}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
frontend/.env.examplefrontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsxfrontend/app/[locale]/shop/admin/orders/[id]/page.tsxfrontend/app/[locale]/shop/admin/orders/page.tsxfrontend/app/[locale]/shop/admin/products/_components/product-form.tsxfrontend/app/[locale]/shop/admin/products/page.tsxfrontend/app/[locale]/shop/cart/page.tsxfrontend/app/[locale]/shop/orders/[id]/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/app/api/shop/admin/orders/reconcile-stale/route.tsfrontend/app/api/shop/webhooks/stripe/route.tsfrontend/components/header/UnifiedHeader.tsxfrontend/components/shop/admin/admin-product-status-toggle.tsxfrontend/components/shop/catalog-products-client.tsxfrontend/components/shop/product-filters.tsxfrontend/lib/psp/stripe.tsfrontend/lib/security/csrf.tsfrontend/lib/services/orders/psp-metadata/refunds.tsfrontend/lib/services/orders/restock.tsfrontend/lib/services/products/cart/rehydrate.tsfrontend/lib/tests/checkout-set-payment-intent-reject-contract.test.tsfrontend/lib/tests/checkout-stripe-error-contract.test.tsfrontend/lib/tests/stripe-webhook-mismatch.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/lib/services/orders/restock.ts
- frontend/components/header/UnifiedHeader.tsx
- frontend/components/shop/admin/admin-product-status-toggle.tsx
- frontend/lib/services/orders/psp-metadata/refunds.ts
- frontend/lib/tests/stripe-webhook-mismatch.test.ts
🧰 Additional context used
🧬 Code graph analysis (8)
frontend/app/api/shop/admin/orders/reconcile-stale/route.ts (3)
frontend/lib/auth/admin.ts (1)
requireAdminApi(45-56)frontend/lib/security/csrf.ts (3)
isSameOrigin(94-108)CSRF_FORM_FIELD(6-6)verifyCsrfToken(59-88)frontend/lib/services/orders.ts (1)
restockStalePendingOrders(7-7)
frontend/lib/services/products/cart/rehydrate.ts (6)
frontend/lib/shop/money.ts (1)
fromCents(44-46)frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)frontend/lib/services/errors.ts (1)
PriceConfigError(71-84)frontend/lib/services/products.ts (1)
rehydrateCartItems(16-16)frontend/save-structure.cjs (1)
items(10-10)frontend/lib/validation/shop.ts (1)
CartRehydrateResult(438-438)
frontend/lib/tests/checkout-stripe-error-contract.test.ts (1)
frontend/lib/tests/helpers/makeCheckoutReq.ts (1)
makeCheckoutReq(10-58)
frontend/components/shop/product-filters.tsx (2)
frontend/lib/config/catalog.ts (4)
CATEGORIES(1-9)PRODUCT_TYPES(11-16)COLORS(18-28)SIZES(30-30)frontend/lib/utils.ts (1)
cn(4-6)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (4)
frontend/lib/validation/shop.ts (1)
CurrencyCode(32-32)frontend/lib/shop/currency.ts (3)
CurrencyCode(2-2)resolveCurrencyFromLocale(23-28)formatMoney(115-128)frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)frontend/db/seed-quiz-types.ts (1)
p(37-40)
frontend/app/[locale]/shop/admin/products/page.tsx (16)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/shop/admin/orders/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/shop/orders/[id]/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/app/[locale]/shop/products/[slug]/page.tsx (1)
dynamic(13-13)frontend/app/[locale]/shop/checkout/success/page.tsx (1)
dynamic(11-11)frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (1)
dynamic(13-13)frontend/app/[locale]/shop/orders/page.tsx (1)
dynamic(13-13)frontend/app/[locale]/shop/admin/page.tsx (1)
dynamic(7-7)frontend/app/[locale]/shop/admin/products/new/page.tsx (1)
dynamic(7-7)frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (1)
dynamic(15-15)frontend/app/[locale]/shop/checkout/error/page.tsx (1)
dynamic(9-9)frontend/db/schema/shop.ts (1)
productPrices(278-332)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)frontend/components/shop/admin/admin-product-status-toggle.tsx (1)
AdminProductStatusToggle(10-81)frontend/components/shop/admin/admin-pagination.tsx (1)
AdminPagination(16-74)
frontend/app/[locale]/shop/products/page.tsx (2)
frontend/components/shop/products-toolbar.tsx (1)
ProductsToolbar(9-202)frontend/components/shop/product-filters.tsx (1)
ProductFilters(9-178)
frontend/app/api/shop/webhooks/stripe/route.ts (1)
frontend/lib/services/orders/psp-metadata/refunds.ts (1)
appendRefundToMeta(128-150)
🪛 dotenv-linter (4.0.0)
frontend/.env.example
[warning] 15-15: [UnorderedKey] The GOOGLE_CLIENT_REDIRECT_URI_LOCAL key should go before the GOOGLE_CLIENT_SECRET key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The GOOGLE_CLIENT_REDIRECT_URI_DEVELOP key should go before the GOOGLE_CLIENT_REDIRECT_URI_LOCAL key
(UnorderedKey)
[warning] 17-17: [UnorderedKey] The GOOGLE_CLIENT_REDIRECT_URI_PROD key should go before the GOOGLE_CLIENT_SECRET key
(UnorderedKey)
[warning] 22-22: [UnorderedKey] The GITHUB_CLIENT_REDIRECT_URI_DEVELOP key should go before the GITHUB_CLIENT_SECRET_DEVELOP key
(UnorderedKey)
[warning] 26-26: [UnorderedKey] The CLOUDINARY_API_KEY key should go before the CLOUDINARY_CLOUD_NAME key
(UnorderedKey)
[warning] 27-27: [UnorderedKey] The CLOUDINARY_API_SECRET key should go before the CLOUDINARY_CLOUD_NAME key
(UnorderedKey)
[warning] 33-33: [UnorderedKey] The NEXT_PUBLIC_PAYMENTS_ENABLED key should go before the PAYMENTS_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY key should go before the PAYMENTS_ENABLED key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The INTERNAL_JANITOR_MIN_INTERVAL_SECONDS key should go before the INTERNAL_JANITOR_SECRET key
(UnorderedKey)
[warning] 54-54: [UnorderedKey] The GMAIL_APP_PASSWORD key should go before the GMAIL_USER key
(UnorderedKey)
[warning] 55-55: [UnorderedKey] The EMAIL_FROM key should go before the GMAIL_APP_PASSWORD key
(UnorderedKey)
🪛 Gitleaks (8.30.0)
frontend/lib/tests/checkout-stripe-error-contract.test.ts
[high] 94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts
[high] 119-119: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (65)
frontend/components/shop/catalog-products-client.tsx (2)
7-8: LGTM!The addition of
logErrorfor structured error logging is a good improvement for observability.
102-108: LGTM!Good error handling pattern: structured logging with contextual metadata (
locale,baseQuery,nextPage) for debugging while showing a user-friendly message. This improves observability without leaking implementation details to users.frontend/app/[locale]/shop/products/page.tsx (4)
1-11: LGTM!Imports are appropriate for the new functionality. The
redirectimport supports URL canonicalization, andProductsToolbaris correctly imported for the new toolbar component.
32-47: LGTM!The URL canonicalization logic correctly strips the
pageparameter to prevent shareable infinite-load states. The implementation properly preserves other filter parameters while redirecting to a clean URL.
49-60: LGTM!The parameter parsing with
safeParseand fallback defaults is defensive and correct. The double fallback forlimit(line 57) provides safety in case the schema doesn't set a default.
84-104: LGTM!The results section is well-structured with proper accessibility:
aria-labelledby="results-heading"correctly associates the section with its headingrole="status"on the empty state enables polite announcements to assistive technology- Clear, helpful messaging guides users when no products match filters
frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts (2)
73-91: LGTM!Good refactoring to use the centralized
makeCheckoutReqhelper. The test correctly verifies thatsetOrderPaymentIntentrejection after order creation returns a 409CHECKOUT_CONFLICTresponse rather than a 400 error.Note: The static analysis hint flagging line 119 as a "generic-api-key" is a false positive—the value
idem_key_test_new_attach_reject_0001is an idempotency key identifier, not a secret.
93-129: LGTM!The second test case mirrors the first appropriately for the
isNew=falsescenario. The refactoring tomakeCheckoutReqmaintains test clarity while centralizing request construction.frontend/lib/tests/checkout-stripe-error-contract.test.ts (2)
66-74: LGTM!Good refactoring to use
makeCheckoutReqand the added assertionexpect(createOrderWithItems).toHaveBeenCalledTimes(1)strengthens the test by verifying the order creation was attempted exactly once before the Stripe failure.Note: The static analysis hint on line 94 flagging a "generic-api-key" is a false positive—the idempotency key
idem_key_test_new_0001is not a secret.
93-102: LGTM!Consistent with the first test case—uses the centralized helper and includes the
createOrderWithItemscall count assertion for the existing order scenario.frontend/components/shop/product-filters.tsx (7)
3-7: LGTM!The imports are appropriate—
useIdfor generating stable ARIA IDs andusePathnamefor dynamic path-based navigation are correctly sourced from their respective packages.
18-21: Good use ofuseIdfor accessible labeling.Generating stable unique IDs for each filter section ensures proper
aria-labelledbyassociations without ID collisions, which is especially important for hydration consistency in Next.js.
23-32: LGTM!The
updateFilterfunction correctly resets pagination on filter changes, uses the dynamicpathnamefor flexibility, and handles the "all" category case appropriately by removing the param rather than setting it.
45-63: LGTM!Good accessibility pattern:
role="list"ensures list semantics are preserved even when CSS removes default list styling (Safari/VoiceOver issue), andaria-currentcorrectly indicates the active category in this navigation-style selection.
76-98: LGTM!The Type filter now correctly uses
aria-pressedfor its toggle behavior (allowing deselection), which properly addresses the previous review feedback about semantic mismatch with radio buttons.
115-137: Color swatches: toggle semantics now correct.Using
aria-pressedwitharia-labelfor each color button correctly conveys the toggle behavior and provides screen reader users with the color name. The visual selected state (ring styling) is also well-implemented.
154-173: Size buttons: toggle semantics now correct.The Size filter properly uses
aria-pressedfor toggle behavior, matching the Type and Color sections. The styling for selected/unselected states is clear and accessible.frontend/lib/psp/stripe.ts (2)
23-28: Nice centralization of error wrapping.
withCausekeeps the error code consistent while attaching original context in a single place.
72-74: Consistent error propagation.The updated throw sites uniformly preserve the originating error while retaining stable error codes.
Also applies to: 129-131, 155-156, 175-176, 206-208
frontend/app/api/shop/webhooks/stripe/route.ts (6)
10-13: LGTM!Clean extraction of refund metadata handling to a dedicated module. The imports align with the usage in
upsertRefundIntoMeta.
15-23: LGTM!Good refactoring to centralize the sentinel error pattern. The
as constassertion enables type-safe usage in the response, and theneverreturn type correctly documents the throwing behavior.
34-37: LGTM!Good defensive handling for edge cases where
prevMetamight be null, a primitive, or an array. This prevents runtime errors during object spreading.
41-51: LGTM!The
RefundMetaRecordconstruction and delegation toappendRefundToMetaaligns with the expected interface. The helper handles deduplication viarefundIdandidempotencyKeymatching.
790-790: LGTM!Consistent replacement of inline throws with
throwRefundFullnessUndetermined()across all six call sites. This centralizes the error creation for easier maintenance.
1089-1096: LGTM!Correct error handling strategy: returning 500 without calling
ack()leavesprocessedAtNULL, allowing Stripe to retry the webhook. Using the constant directly in the response body ensures consistency with the error identification logic.frontend/lib/services/products/cart/rehydrate.ts (6)
21-22: Good semantic alias for domain clarity.The alias
fromMinorUnits = fromCentsimproves readability by using domain-appropriate terminology while reusing the existing utility function.
41-42: Good fail-fast validation placement.Validating currency assumptions before any database queries or processing ensures early failure with clear error messages.
117-144: Price resolution logic is well-structured.The two-path approach (prefer
priceMinor, fallback to legacyprice) with thorough validation ensures data integrity. The earlier guard at lines 101-109 guarantees at least one price field exists before reaching this code.
165-169: Clean separation of canonical and display values.Storing both minor-unit values (
unitPriceMinor,lineTotalMinor) for precision and derived display values (unitPrice,lineTotal) is a solid pattern that prevents floating-point arithmetic issues during calculations while maintaining convenient access for rendering.
186-191: Summary uses consistent minor-unit pattern.The summary correctly provides both
totalAmountMinor(canonical) andtotalAmount(display) values, matching the approach used at the item level.
183-192: No action needed. The schema is properly configured with bothunitPriceMinorandlineTotalMinorfields inCartRehydrateItem, so thecartRehydrateResultSchema.parse()validation will succeed.frontend/.env.example (4)
1-11: LGTM! Clear structure and essential additions.The new
APP_ENVandAPP_URLvariables provide necessary environment configuration, and the section organization with comments improves readability.
29-29: AI summary incorrectly claims CLOUDINARY_URL was removed.The AI summary states "Removed CLOUDINARY_URL", but line 29 shows
CLOUDINARY_URL=is still present in the file.
31-38: LGTM! Stripe configuration is well-structured.The addition of
STRIPE_MODEand proper organization of Stripe keys support the payment features mentioned in the PR. The separation of public (NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY) and secret keys follows best practices.
45-58: LGTM! Essential security and feature configurations.The additions support key PR objectives:
QUIZ_ENCRYPTION_KEYenables secure quiz data handling- Gmail SMTP configuration enables email notifications
CSRF_SECRETimplements the CSRF hardening mentioned in the PR summaryAll variables are appropriately named and grouped.
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (8)
3-49: LGTM — hook import + sale rule types are clear.
134-150: Nice — stable ID base keeps ARIA wiring deterministic.
186-249: Edit-mode hydration looks solid and well-scoped.
251-255: Slug derivation logic is clear.
343-355: Good guard around price parsing.
398-413: Server sale-rule errors are surfaced cleanly.
441-443: ARIA describedBy wiring looks correct.
446-957: Semantic structure + accessibility attributes look consistent end-to-end.frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx (1)
4-4: Well-implemented accessibility enhancements.The use of
useIdfor stable error ID generation,aria-busyto indicate pending state,aria-describedbyto associate the error message with the button, androle="alert"on the error span follows ARIA best practices for form controls with error states.Also applies to: 15-15, 51-60, 71-75
frontend/lib/security/csrf.ts (2)
1-57: Solid stateless CSRF implementation.The token design covers the key security properties:
- HMAC-SHA256 signature prevents tampering
- Timing-safe comparison mitigates timing attacks
- Nonce ensures uniqueness
- TTL via
expclaim prevents replay- Purpose binding scopes tokens to specific operations
94-108: Origin check logic is appropriate as a secondary defense.The preference for
OriginoverRefereris correct. Note that requests lacking both headers will fail the check—this is the safe default for a CSRF defense layer, and admin form submissions from the browser should reliably includeOrigin.frontend/app/api/shop/admin/orders/reconcile-stale/route.ts (1)
20-54: CSRF protection flow is well-structured.The layered checks (auth → origin → body → token) follow secure patterns:
- Authentication happens before any body parsing
- Origin check precedes CSRF token validation
- Purpose string
'admin:orders:reconcile-stale'correctly matches the token issuer inAdminOrdersPagefrontend/app/[locale]/shop/admin/orders/page.tsx (2)
29-37: Currency and date helpers now respect locale.
orderCurrencycorrectly derives currency from order data with locale fallback, andformatDatenow accepts locale parameter—addressing the previous review feedback.
91-212: Excellent semantic table structure with accessibility enhancements.Proper use of
<thead>/<tbody>,scope="col"on headers, screen-reader-only caption, and contextualaria-labelon View links provide a solid accessible table implementation.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (3)
32-38: Locale-aware timestamp formatting implemented correctly.
formatDateTimenow accepts locale and is consistently applied to all date fields (createdAt, updatedAt, restockedAt), addressing previous review feedback.Also applies to: 155-156, 162-163, 176-177
221-280: Items table rendering with proper fallback.The empty-state row renders inside
<tbody>as expected, maintaining valid table structure. Currency formatting is consistently applied across unit prices and line totals.
52-56: RefundButton integration is correct.The
canRefundguard properly restricts refund availability to paid Stripe orders with a payment intent, and the disabled state is correctly propagated to the button component.Also applies to: 89-89
frontend/app/[locale]/shop/admin/products/page.tsx (6)
1-18: LGTM: dynamic rendering + page sizing align with admin routes.
Please confirm this page should remain uncached (force-dynamic) given admin data visibility.
41-70: Query update topriceMinor+ stable tie-breaker sort looks solid.
75-170: Semantic structure and table headers look good.
231-253: Row actions + ARIA labels are clear and consistent.
259-279: Empty-state row and pagination placement look good.
24-38: No action required—parsePagecorrectly clamps all invalid values (NaN, negative, zero, undefined) to 1, ensuring the offset calculation(page - 1) * PAGE_SIZEalways produces a valid result ≥ 0.frontend/app/[locale]/shop/orders/[id]/page.tsx (4)
50-66: Defensive formatting helpers look good.
The fallback paths keep the UI stable if formatting throws or dates are invalid.
68-101: Item mapping validation is robust.
Guarding required fields and filtering null rows prevents silent data corruption in the UI.
103-184: Access control and data loading flow is clean.
Admin vs user scoping +notFound()on missing rows avoids information leakage.
202-349: Semantic structure and admin-only details are well organized.
The headings/sections read clearly and the conditional admin blocks are tidy.frontend/app/[locale]/shop/cart/page.tsx (3)
13-101: Checkout flow + createdOrderId fallback looks solid.
Capturing the order ID before navigation provides a safe recovery path if routing fails.
104-125: Empty-cart state accessibility is improved.
The semantic<main>structure and aria-hidden icon are well handled.
265-363: Order summary + CTA state handling is clean.
Dynamic totals andaria-busyon the action button improve UX clarity.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…xample with OAuth envs; shop a11y + cart key + badge fallback
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/.env.example`:
- Around line 41-47: Update the .env.example to document STRIPE_MODE valid
values and address the unused client flag: add a comment next to STRIPE_MODE
stating "Options: test, live (defaults to test in development, live in
production)". Then either remove NEXT_PUBLIC_PAYMENTS_ENABLED from .env.example
if it is not referenced anywhere in the codebase, or integrate it consistently
by wiring the NEXT_PUBLIC_PAYMENTS_ENABLED environment variable into the
client-side code/config (where PAYMENTS_ENABLED is used server-side) so both
server (PAYMENTS_ENABLED) and client (NEXT_PUBLIC_PAYMENTS_ENABLED) flags are
purposeful and synchronized.
🧹 Nitpick comments (2)
frontend/lib/services/products/cart/rehydrate.ts (1)
21-48: Verify the 2‑decimal currency allowlist matches supported currencies.
IfCurrencyCodeincludes other 2‑decimal currencies (e.g., EUR/GBP), this guard will now throw during cart rehydration. Consider centralizing the allowlist in the currency module (single source of truth) to avoid drift.frontend/app/[locale]/shop/products/page.tsx (1)
55-58: Optional: defaultpageexplicitly if schema doesn’t.If
catalogQuerySchemadoesn’t set a default,pagecan beundefinedhere. Consider defaulting it the same way you dolimitto avoid downstream ambiguity.♻️ Suggested tweak
const filters = { ...parsed, + page: parsed.page ?? 1, limit: parsed.limit ?? CATALOG_PAGE_SIZE, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
frontend/.env.examplefrontend/app/[locale]/shop/admin/products/page.tsxfrontend/app/[locale]/shop/cart/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/components/shop/catalog-products-client.tsxfrontend/components/shop/product-filters.tsxfrontend/lib/services/products/cart/rehydrate.tsfrontend/lib/tests/checkout-set-payment-intent-reject-contract.test.tsfrontend/lib/tests/checkout-stripe-error-contract.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts
- frontend/components/shop/catalog-products-client.tsx
- frontend/lib/tests/checkout-stripe-error-contract.test.ts
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/components/shop/product-filters.tsx (2)
frontend/lib/config/catalog.ts (4)
CATEGORIES(1-9)PRODUCT_TYPES(11-16)COLORS(18-28)SIZES(30-30)frontend/lib/utils.ts (1)
cn(4-6)
frontend/app/[locale]/shop/admin/products/page.tsx (15)
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/shop/admin/orders/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/shop/orders/[id]/page.tsx (1)
dynamic(16-16)frontend/app/[locale]/layout.tsx (1)
dynamic(17-17)frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (1)
dynamic(13-13)frontend/app/[locale]/shop/checkout/success/page.tsx (1)
dynamic(11-11)frontend/app/[locale]/shop/products/[slug]/page.tsx (1)
dynamic(13-13)frontend/app/[locale]/shop/orders/page.tsx (1)
dynamic(13-13)frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (1)
dynamic(15-15)frontend/app/[locale]/shop/admin/page.tsx (1)
dynamic(7-7)frontend/app/[locale]/shop/admin/products/new/page.tsx (1)
dynamic(7-7)frontend/app/[locale]/shop/checkout/error/page.tsx (1)
dynamic(9-9)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)frontend/components/shop/admin/admin-product-status-toggle.tsx (1)
AdminProductStatusToggle(10-81)frontend/components/shop/admin/admin-pagination.tsx (1)
AdminPagination(16-74)
frontend/app/[locale]/shop/cart/page.tsx (2)
frontend/components/shop/cart-provider.tsx (1)
useCart(291-293)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/app/[locale]/shop/products/page.tsx (1)
frontend/components/shop/products-toolbar.tsx (1)
ProductsToolbar(9-202)
🔇 Additional comments (28)
frontend/components/shop/product-filters.tsx (4)
3-21: Well-structured accessibility foundation.Good use of
useIdto generate stable, unique IDs for the ARIA relationships. Each filter section gets a properaria-labelledbyconnection to its heading, which is the correct pattern for accessible grouped controls.
23-31: Clean filter update logic with proper pagination reset.The simplified conditional (lines 27-28) is readable, and resetting
pageon filter changes (line 25) prevents stale pagination state. Usingpathnameinstead of a hardcoded path makes this component reusable across different routes.
34-64: Category section uses appropriatearia-currentsemantics.Using
aria-currenthere is correct since categories behave like navigation (always one selected, no deselection) rather than toggles. This distinguishes it from the Type/Color/Size sections that usearia-pressedfor their toggle behavior.
66-100: Toggle sections correctly usearia-pressed— past accessibility concerns addressed.The Type, Color, and Size sections now correctly use
aria-pressedfor their toggle buttons, which accurately represents the click-to-deselect behavior. This resolves the previous semantic mismatch whererole="radio"was incorrectly paired with toggle functionality.Also applies to: 102-136, 138-169
frontend/lib/services/products/cart/rehydrate.ts (4)
5-6: LGTM: import alignment for new helpers.
129-133: Good: currency added to priceMinor validation payload.
152-157: Confirm that zero‑priced items are intentionally disallowed.
This now throws forunitPriceMinor < 1; if the shop supports free items/promos, cart rehydration will fail.
171-175: Nice: display values derived from canonical minor units.Also applies to: 194-194
frontend/.env.example (2)
23-33: LGTM! GitHub OAuth environment variants are now complete.The missing LOCAL and PROD environment variants flagged in the previous review have been addressed. All three environments (LOCAL, DEVELOP, PROD) are now consistently defined, matching the Google OAuth pattern.
49-54: LGTM! Janitor configuration inconsistency resolved.The default value for
INTERNAL_JANITOR_MIN_INTERVAL_SECONDS(previously=60) has been removed, making it consistent with other environment variables in the file. This addresses the concern raised in the previous review.frontend/app/[locale]/shop/admin/products/page.tsx (8)
1-15: LGTM! Clean imports and appropriate dynamic rendering strategy.The
force-dynamicexport is consistent with other admin and shop pages in the codebase. Centralizing pagination logic viaparsePagefromlib/paginationis a good refactor.
17-22: LGTM! Simple and effective helper.The
formatDatehelper properly handles null values and the return type annotation is correct.
24-40: LGTM! Correct Next.js 15+ async params pattern and proper authorization.The
guardShopAdminPage()is appropriately called before any data access, and the pagination setup using the sharedparsePageutility is clean.
41-69: LGTM! Well-structured query with good practices.The stable sort with tie-breaker (line 63-64) prevents non-deterministic ordering. The
PAGE_SIZE + 1fetch pattern for hasNext detection is efficient, avoiding a separate count query.
71-93: LGTM! Excellent semantic structure and accessibility.The
aria-labelledbycorrectly references the headingid, and the header layout with the "New product" action is well-organized.
95-169: LGTM! Proper semantic table with accessibility enhancements.Good use of
scope="col"on all table headers,sr-onlycaption for screen readers, andtable-fixedwith well-proportioned column widths.
261-284: LGTM! Proper empty state and pagination integration.The
colSpan={11}correctly spans all columns, and theAdminPaginationcomponent is properly configured with the required props.
171-259: LGTM! Comprehensive row rendering with proper null handling.The badge null handling on line 212 correctly addresses the previous review feedback. The
aria-labelattributes on action links improve accessibility. Title attributes on truncated cells provide good UX for overflow content.The
stockfield on line 208 is safely rendered without a fallback—the schema defines it asinteger().default(0).notNull(), so it cannot be null.frontend/app/[locale]/shop/products/page.tsx (4)
3-8: Imports align with new toolbar + redirect usage.No concerns here.
62-72: Header + toolbar semantics look solid.The structure and spacing are clean, and the toolbar placement is intuitive.
81-101: Results section + empty state are accessible and clear.The hidden heading and status messaging read well for assistive tech.
33-47: No action needed—filters are single-value only.The
URLSearchParams.set()usage is correct. The type system and validation schema both enforce single-value parameters (category, type, color, size); the UI provides single-select buttons for each dimension, and the validation schema defensively extracts only the first element if an array is somehow passed. Switching toappend()would be unnecessary and doesn't address the actual design pattern here.frontend/app/[locale]/shop/cart/page.tsx (6)
14-23: Clean state + locale wiring.
24-101: Checkout flow looks robust and well-guarded.
136-147: Nice use of live region for removed-item notices.
186-259: Item controls and stock guardrails look solid.
267-365: Order summary + fallback/error UX is clean and user-friendly.
118-123: This review concern is incorrect and requires no changes.The
Linkcomponent is fromnext-intl'screateNavigation()withlocalePrefix: 'always'configured. This means next-intl automatically prefixes the current locale to allhrefvalues. Hardcoded paths likehref="/shop/products"will correctly resolve to/{locale}/shop/productsat runtime—they will not drop the locale or cause 404s.The suggested fix using
shopBasewould be harmful, as it would cause double locale-prefixing (e.g.,/en/en/shop/products). The router.push calls useshopBasebecauseuseRouter()from next-intl does not auto-prefix, butLinkdoes by design. The code is correct and follows next-intl best practices.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…EXT_PUBLIC_PAYMENTS_ENABLED + centralize 2-decimal currency guard + default catalog page=1
2 participants
(Shop UI) Improve semantics + styles across shop; add admin header button (admin role only)
Description
This PR stabilizes the Shop UI by aligning markup with semantic/accessibility requirements, fixing layout issues across breakpoints, and improving navigation ergonomics for admin users. It also finalizes pagination behavior for both public catalog browsing and admin listings to ensure consistent, shareable URLs and predictable UX.
Related Issue
Issue: #<issue_number>
Changes
?page=Nstates.Database Changes (if applicable)
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removed
✏️ Tip: You can customize this high-level summary in your review settings.