(SP: 3) [Shop/Backend/UI] Close critical checkout/refund invariants + unify shop layouts/theme + fix catalog pagination & cart price display#144
Conversation
…ata (refunds[]), merge PSP metadata safely, and harden full-refund gating/partial-refund ignore
…lease failure + regression test
…emantics (502 STRIPE_ERROR, 409 CHECKOUT_CONFLICT) + contract tests
… appends items without navigation + canonicalize ?page= under locale
…errides (remove shop-theme.css)
…ing with platform
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. |
📝 WalkthroughWalkthroughRemoved the legacy shop admin layout and theme file, added a ShopAdminTopbar and server guard, moved catalog rendering to a client-side incremental loader with a new /api/shop/catalog route, overhauled Stripe refund flow with idempotent refunds and merged PSP metadata, and standardized monetary variables to "minor" units. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Browser
participant Catalog as CatalogProductsClient (client)
participant API as /api/shop/catalog
participant DB as Database
User->>Catalog: initial render (initialCatalog)
Catalog->>User: render grid + LoadMore
User->>Catalog: click Load more
Catalog->>API: GET /api/shop/catalog?page=2&...
API->>DB: query paged products
DB-->>API: products + hasMore
API-->>Catalog: { products, hasMore, page }
Catalog->>Catalog: dedupe & append products
Catalog->>User: render updated grid
sequenceDiagram
participant Admin as Admin Browser
participant Page as Server page (orders/[id])
participant Guard as guardShopAdminPage()
participant Auth as requireAdminPage()
Admin->>Page: request /shop/admin/orders/123
Page->>Guard: await guardShopAdminPage()
Guard->>Auth: requireAdminPage()
Auth-->>Guard: OK / throws admin errors
Guard-->>Page: allow or perform notFound/redirect
Page->>Page: render ShopAdminTopbar + order panels
Page-->>Admin: HTML/SSR response
sequenceDiagram
participant Admin as Admin UI
participant API as POST /api/shop/admin/orders/[id]/refund
participant RefundSvc as refundOrder()
participant Stripe as Stripe API
participant DB as Database
participant Webhook as Stripe Webhook
Admin->>API: POST refund request
API->>RefundSvc: refundOrder(orderId, { requestedBy: 'admin' })
RefundSvc->>DB: read order & pspMetadata
RefundSvc->>RefundSvc: compute idempotency key, check existing refunds
RefundSvc->>Stripe: createRefund(...)
Stripe-->>RefundSvc: refundId, status
RefundSvc->>DB: upsert pspMetadata (preserve refunds)
RefundSvc-->>API: return updated order summary
API-->>Admin: 200 OK
Stripe->>Webhook: send refund/charge webhook
Webhook->>DB: mergePspMetadata, update payment state
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
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/project-structure.txt (1)
147-147: Regenerateproject-structure.txtto reflect removal ofshop-theme.css.The file
shop-theme.csswas deleted from the codebase, butproject-structure.txtstill lists it at line 147. Since this file is auto-generated bysave-structure.cjs, run the script to regenerate it with the current file structure.
🤖 Fix all issues with AI agents
In `@frontend/components/shop/catalog-products-client.tsx`:
- Around line 75-77: The race-condition guard is ineffective because
activeQueryRef.current is overwritten at the start of each request; remove the
assignment activeQueryRef.current = requestQueryKey from the fetch-start path
and instead rely on the existing update in the useEffect that watches baseQuery
and locale so that activeQueryRef.current remains the canonical value to compare
against when resolving responses (ensure the comparison activeQueryRef.current
!== requestQueryKey stays in the response handling branch to drop stale
responses).
In `@frontend/lib/services/orders/restock.ts`:
- Around line 205-237: The check for a falsey `ok` is dead because
applyReleaseMove returns Promise<{ ok: true; applied: boolean }> or throws, so
remove the unreachable branching and casts: in the loop over reservedMoves
delete the whole "if (res && typeof res === 'object' && 'ok' in (res as any) &&
(res as any).ok === false) { ... }" block and rely on the existing try/catch to
push failures into releaseFailures; alternatively, if you intend
applyReleaseMove to sometimes return {ok: false}, update applyReleaseMove's
return type and implementation (and callers) to allow { ok: false; reason?:
string } so the current guard becomes valid—choose one approach and make sure
reservedMoves loop and releaseFailures handling are consistent with that
decision.
🧹 Nitpick comments (18)
frontend/components/shop/admin/shop-admin-topbar.tsx (1)
3-42: LGTM! Clean and well-structured navigation component.The component is well-implemented with appropriate responsive styling. Two optional improvements to consider:
- Accessibility: Adding a
<nav>wrapper with anaria-labelwould improve screen reader navigation.- Active state: The current route isn't visually distinguished. If needed later, this could accept the current path as a prop.
💡 Optional: Add semantic nav wrapper
export function ShopAdminTopbar() { return ( - <div className="border-b border-border bg-background"> + <nav aria-label="Admin navigation" className="border-b border-border bg-background"> <div className="mx-auto max-w-7xl px-4 sm:px-6 lg:px-8"> ... </div> - </div> + </nav> ); }frontend/app/[locale]/shop/admin/products/page.tsx (1)
73-227: Clean table implementation with proper accessibility attributes.The table structure is well-organized with:
- Fixed column widths totaling 100%
- Truncation handling for long text with
titleattributes- Appropriate
aria-labelattributes on status indicators- Defensive null handling for optional fields
Optional consideration: This page fetches all products without pagination. For large catalogs, consider adding server-side pagination or virtualization to avoid performance issues.
frontend/lib/services/products/cart/rehydrate.ts (1)
158-162: Consider renamingfromCentsfor clarity.The function converts minor units to major/display values, but the name
fromCentsis misleading for zero-decimal currencies like JPY where the minor unit equals the major unit. Consider renaming tofromMinorUnitsorminorToMajorfor consistency with the codebase terminology shift to "minor units."frontend/components/header/AppMobileMenu.tsx (1)
72-80: Home link correctly added for shop variant.The conditional rendering and
close()handler are properly implemented.Minor styling inconsistency: The link styling here (
hover:bg-secondary) differs fromNavLinksdesktop styling (hover:bg-muted/50). This may be intentional for mobile/desktop differentiation, but if you want full consistency, consider extracting the shared link styles.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (2)
31-34: Consider specifying locale for date formatting.
toLocaleString()without arguments uses the server's default locale, which may differ from the user's expected locale. Consider passing thelocaleparameter for consistent formatting.♻️ Suggested improvement
-function formatDateTime(value: Date | null | undefined) { +function formatDateTime(value: Date | null | undefined, locale?: string) { if (!value) return '-'; - return value.toLocaleString(); + return value.toLocaleString(locale); }Then update calls like
formatDateTime(order.createdAt, locale).
87-96: IIFE pattern for inline currency formatting works but could be cleaner.The immediately-invoked function expressions for formatting are functional but add cognitive overhead. Consider extracting to a helper or computing values earlier.
frontend/app/api/shop/catalog/route.ts (1)
28-32: Silent validation failure may mask client issues.When
catalogQuerySchema.safeParsefails, the code silently falls back to defaults without any indication to the caller. This could make debugging difficult for API consumers who send malformed parameters.Consider returning a 400 response for invalid parameters, or at minimum logging the validation error server-side.
♻️ Option 1: Return 400 for invalid params
const parsed = catalogQuerySchema.safeParse(rest); - const filters = parsed.success - ? parsed.data - : { page: 1, limit: CATALOG_PAGE_SIZE }; + if (!parsed.success) { + return NextResponse.json( + { error: 'Invalid query parameters', details: parsed.error.flatten() }, + { status: 400 } + ); + } + const filters = parsed.data;♻️ Option 2: Log and fallback (current behavior with observability)
const parsed = catalogQuerySchema.safeParse(rest); - const filters = parsed.success - ? parsed.data - : { page: 1, limit: CATALOG_PAGE_SIZE }; + const filters = parsed.success + ? parsed.data + : (console.warn('[catalog] Invalid query params, using defaults:', parsed.error.flatten()), { page: 1, limit: CATALOG_PAGE_SIZE });frontend/components/shop/catalog-products-client.tsx (1)
103-104: Consider logging the error for debugging.The catch block discards the actual error object. While the UI displays a generic message, logging the error would aid debugging in production.
Proposed improvement
- } catch { + } catch (err) { + console.error('Failed to load more products:', err); setError('Failed to load more');frontend/app/[locale]/shop/products/page.tsx (1)
53-58: Redundant page override after canonicalization.Since the redirect at line 44 ensures
resolvedSearchParams.pageis never present when this code executes, thepage: 1override at line 56 is always applied. The parsedpagefromcatalogQuerySchema(which defaults to 1) is immediately overwritten.This is functionally correct but slightly redundant. Consider simplifying if desired:
Optional simplification
const parsed = parsedParams.success ? parsedParams.data : { page: 1, limit: CATALOG_PAGE_SIZE }; - // Для "Load more" UX: починаємо завжди з 1-ї сторінки (URL ?page=... ігноруємо). - const filters = { - ...parsed, - page: 1, - limit: parsed.limit ?? CATALOG_PAGE_SIZE, - }; + // After canonicalization, page param is always absent, so parsed.page defaults to 1. + const filters = { ...parsed, limit: parsed.limit ?? CATALOG_PAGE_SIZE };frontend/lib/services/orders/checkout.ts (2)
196-216: Minor: DuplicateinventoryStatusupdate.Lines 197-200 update
inventoryStatustorelease_pending, then lines 204-216 update it again to the same value. The second update also sets other fields, so it's not incorrect, but the first update at lines 197-200 is redundant.Optional consolidation
} catch (e) { const failAt = new Date(); - - // Mark as "release pending" only. Finalization must happen via restockOrder(). - await db - .update(orders) - .set({ inventoryStatus: 'release_pending', updatedAt: failAt }) - .where(eq(orders.id, orderId)); const isOos = e instanceof InsufficientStockError; + // Mark as "release pending" only. Finalization must happen via restockOrder(). await db .update(orders) .set({ status: 'INVENTORY_FAILED', inventoryStatus: 'release_pending',
759-779: Minor: Same duplicateinventoryStatusupdate pattern.Similar to
reconcileNoPaymentOrder, lines 760-763 and 767-779 both updateinventoryStatusto'release_pending'. Consider consolidating into a single update for clarity.frontend/lib/tests/restock-release-failure-invariant.test.ts (1)
38-68: Consider using test database transactions for isolation.The test seeds real data into the database and relies on manual cleanup in the
finallyblock. If cleanup fails partially (e.g., FK constraint order), test pollution can occur.Consider wrapping the test in a database transaction that rolls back, or using a test helper that ensures atomic cleanup.
frontend/app/api/shop/admin/orders/[id]/refund/route.ts (1)
31-33: LGTM: ExplicitrequestedByimproves audit trail.Passing
{ requestedBy: 'admin' }explicitly documents the refund source for traceability in PSP metadata, even though the service defaults to'admin'. This makes the intent clearer.The comment on line 31 restating the file path is unnecessary and could be removed in a future cleanup.
frontend/lib/tests/checkout-stripe-error-contract.test.ts (1)
47-57: Minor inconsistency:selectedSizeandselectedColorpresent here but absent in the other test file.The
makeReqhelper includesselectedSizeandselectedColorfields here, but the similar helper incheckout-set-payment-intent-reject-contract.test.tsomits them. While both likely pass validation, consider aligning for consistency.frontend/lib/psp/stripe.ts (1)
64-67: Consider preserving the original error for debugging.The catch block logs the error but throws a generic
STRIPE_REFUND_FAILED. This loses Stripe-specific error details (e.g.,StripeCardError, decline codes) that could aid debugging or enable more granular error handling upstream.♻️ Optional: Preserve original error context
} catch (error) { logError('Stripe refund creation failed', error); - throw new Error('STRIPE_REFUND_FAILED'); + const wrappedError = new Error('STRIPE_REFUND_FAILED'); + (wrappedError as any).cause = error; + throw wrappedError; }frontend/app/api/shop/webhooks/stripe/route.ts (2)
57-57: Redundant null-coalescing.
((prevMeta ?? {}) as any) ?? {}is redundant since(prevMeta ?? {})already ensures a non-null value.♻️ Simplify
- const base = ((prevMeta ?? {}) as any) ?? {}; + const base = (prevMeta ?? {}) as any;
218-254: Merge order could unintentionally overwriterefunds[]in edge cases.The spread order
{ ...metaWithRefunds, ...cleanedDelta }means ifcleanedDeltaever contains arefundskey, it would overwrite the carefully upserted refunds array. CurrentlybuildPspMetadatadoesn't producerefunds, but this is fragile.Consider explicitly preserving
refundsafter the merge:♻️ Proposed fix to explicitly preserve refunds
// IMPORTANT: merge, not overwrite (preserves refunds[]) return { ...metaWithRefunds, ...cleanedDelta, + refunds: metaWithRefunds.refunds, // explicit preservation };frontend/lib/services/orders/refund.ts (1)
9-50: Duplicate code:RefundMetaRecordandnormalizeRefundsare duplicated from the webhook route.These types and functions are nearly identical to those in
frontend/app/api/shop/webhooks/stripe/route.ts. Consider extracting them to a shared module (e.g.,@/lib/services/orders/refund-metadata.ts) to maintain consistency and reduce duplication.♻️ Extract shared refund metadata utilities
Create a shared module:
// frontend/lib/services/orders/refund-metadata.ts export type RefundMetaRecord = { refundId: string; idempotencyKey: string; amountMinor: number; currency: string; createdAt: string; createdBy: string; status?: string | null; }; export function normalizeRefunds( meta: unknown, fallback: { currency: string; createdAt: string } ): RefundMetaRecord[] { // ... implementation }Then import in both files.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
frontend/app/[locale]/shop/admin/layout.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/new/page.tsxfrontend/app/[locale]/shop/admin/products/page.tsxfrontend/app/[locale]/shop/cart/page.tsxfrontend/app/[locale]/shop/layout.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/app/[locale]/shop/shop-theme.cssfrontend/app/api/shop/admin/orders/[id]/refund/route.tsfrontend/app/api/shop/catalog/route.tsfrontend/app/api/shop/checkout/route.tsfrontend/app/api/shop/webhooks/stripe/route.tsfrontend/app/globals.cssfrontend/components/header/AppMobileMenu.tsxfrontend/components/header/UnifiedHeader.tsxfrontend/components/shop/admin/shop-admin-topbar.tsxfrontend/components/shop/catalog-load-more.tsxfrontend/components/shop/catalog-products-client.tsxfrontend/components/shop/header/nav-links.tsxfrontend/components/tests/CookieBanner.test.tsxfrontend/lib/auth/guard-shop-admin-page.tsfrontend/lib/psp/stripe.tsfrontend/lib/services/orders/checkout.tsfrontend/lib/services/orders/payment-intent.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/restock-release-failure-invariant.test.tsfrontend/project-structure.txt
💤 Files with no reviewable changes (3)
- frontend/app/[locale]/shop/shop-theme.css
- frontend/app/[locale]/shop/layout.tsx
- frontend/app/[locale]/shop/admin/layout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:01:02.523Z
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 88
File: frontend/drizzle/schema.ts:153-153
Timestamp: 2025-12-30T16:01:02.523Z
Learning: The file frontend/drizzle/schema.ts is a legacy file not used by Drizzle or the runtime. The canonical schemas live in frontend/db/schema/* as configured in drizzle-config.ts, and that's what migrations and runtime use.
Applied to files:
frontend/project-structure.txt
🧬 Code graph analysis (13)
frontend/lib/auth/guard-shop-admin-page.ts (1)
frontend/lib/auth/admin.ts (4)
requireAdminPage(62-70)AdminApiDisabledError(7-13)AdminUnauthorizedError(15-21)AdminForbiddenError(23-29)
frontend/components/shop/header/nav-links.tsx (1)
frontend/lib/utils.ts (1)
cn(4-6)
frontend/app/api/shop/admin/orders/[id]/refund/route.ts (1)
frontend/lib/services/orders/refund.ts (1)
refundOrder(81-185)
frontend/lib/psp/stripe.ts (3)
frontend/lib/env/stripe.ts (1)
getStripeEnv(17-49)frontend/lib/logging.ts (1)
logError(87-93)frontend/db/legacy-migrations/drizzle_legacy/schema.ts (1)
currency(7-7)
frontend/app/[locale]/shop/products/page.tsx (3)
frontend/lib/validation/shop.ts (1)
catalogQuerySchema(62-72)frontend/lib/config/catalog.ts (1)
CATALOG_PAGE_SIZE(39-39)frontend/components/shop/catalog-products-client.tsx (1)
CatalogProductsClient(25-132)
frontend/app/[locale]/shop/cart/page.tsx (1)
frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/lib/services/orders/checkout.ts (4)
frontend/lib/services/orders/restock.ts (1)
restockOrder(56-314)frontend/lib/services/orders.ts (2)
restockOrder(5-5)restockOrder(5-5)frontend/lib/logging.ts (1)
logError(87-93)frontend/lib/services/errors.ts (1)
InsufficientStockError(18-24)
frontend/lib/tests/restock-release-failure-invariant.test.ts (4)
frontend/db/index.ts (1)
db(54-54)frontend/lib/shop/money.ts (1)
toDbMoney(58-60)frontend/lib/services/orders/restock.ts (1)
restockOrder(56-314)frontend/db/schema/shop.ts (1)
inventoryMoves(334-363)
frontend/app/api/shop/catalog/route.ts (3)
frontend/lib/validation/shop.ts (1)
catalogQuerySchema(62-72)frontend/lib/config/catalog.ts (1)
CATALOG_PAGE_SIZE(39-39)frontend/lib/shop/data.ts (1)
getCatalogProducts(207-234)
frontend/app/[locale]/shop/admin/products/new/page.tsx (2)
frontend/lib/auth/guard-shop-admin-page.ts (1)
guardShopAdminPage(9-18)frontend/components/shop/admin/shop-admin-topbar.tsx (1)
ShopAdminTopbar(3-43)
frontend/lib/services/orders/restock.ts (3)
frontend/lib/services/errors.ts (1)
OrderStateInvalidError(93-116)frontend/lib/services/orders/payment-state.ts (1)
guardedPaymentStatusUpdate(122-263)frontend/lib/services/inventory.ts (1)
applyReleaseMove(118-178)
frontend/components/header/UnifiedHeader.tsx (1)
frontend/components/shop/header/nav-links.tsx (1)
NavLinks(32-102)
frontend/app/[locale]/shop/admin/products/page.tsx (4)
frontend/lib/auth/guard-shop-admin-page.ts (1)
guardShopAdminPage(9-18)frontend/components/shop/admin/shop-admin-topbar.tsx (1)
ShopAdminTopbar(3-43)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)frontend/components/shop/admin/admin-product-status-toggle.tsx (1)
AdminProductStatusToggle(10-67)
🔇 Additional comments (64)
frontend/components/tests/CookieBanner.test.tsx (1)
1-3: LGTM!The
@vitest-environment jsdomdirective is correctly placed and necessary for this test file, which relies on browser APIs likelocalStorageand DOM rendering via@testing-library/react.frontend/lib/auth/guard-shop-admin-page.ts (1)
9-17: Well-implemented authorization guard with appropriate error mapping.The guard correctly:
- Uses
notFound()for disabled API and forbidden access (security best practice—doesn't reveal page existence)- Uses
redirect('/login')for unauthenticated users- Re-throws unexpected errors for proper error boundary handling
Since
notFound()andredirect()throw internally in Next.js, the control flow is correct—throw erron line 16 only executes for unhandled error types.frontend/app/[locale]/shop/admin/products/page.tsx (1)
38-44: Guard placement and async params handling are correct.The guard is invoked before any data fetching or rendering, ensuring unauthorized access is blocked early. The async
paramspattern follows Next.js 15+ conventions.frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (2)
24-32: Correct guard placement before parameter validation and data access.The guard runs first (line 29), then params are validated with Zod schema. This ensures authorization is checked before any database queries, following the fail-fast principle.
73-97: Clean integration of topbar with existing form.The fragment wrapper appropriately adds the
ShopAdminTopbarwithout introducing unnecessary DOM nesting. TheProductFormprops remain unchanged, preserving existing functionality.frontend/app/globals.css (2)
166-181: Circular reference in--radiuswill cause unexpected behavior.Line 168 defines
--radius: calc(var(--radius) - 2px)which creates a self-referential loop. CSS custom properties don't resolve previous values in the same scope—this will result in the property being invalid or using the initial value.🐛 Proposed fix: Reference the global radius value explicitly
.shop-scope { /* keep shop rounding slightly tighter than platform */ - --radius: calc(var(--radius) - 2px); + --radius: calc(0.625rem - 2px); /* 0.625rem is the global --radius value from line 36 */Alternatively, define a separate variable at the root level (e.g.,
--radius-base: 0.625rem) and reference that in both places.Likely an incorrect or invalid review comment.
183-199: Dark mode shop-scope overrides are correct.The
color-mix()usage for derived border/input colors is clean and avoids hardcoded hex values, maintaining consistency with the design system. Browser support (Chrome 111+, Firefox 113+, Safari 16.2+) is well-established with ~92% global coverage and aligns with the project's modern tech stack (Next.js 16, Tailwind v4, React 19).frontend/app/[locale]/shop/admin/page.tsx (1)
5-14: LGTM! Clean guard and layout integration.The async guard pattern correctly enforces access control before rendering, and the ShopAdminTopbar provides consistent navigation. The fragment wrapper appropriately groups the topbar with the content container.
frontend/app/[locale]/shop/admin/products/new/page.tsx (1)
5-13: LGTM! Consistent admin page pattern.The guard and topbar integration follows the same pattern as other admin pages, maintaining consistency across the admin section.
frontend/app/[locale]/shop/admin/orders/page.tsx (3)
15-28: LGTM! Well-structured helper functions.The
pickMinor,orderCurrency, andformatDatehelpers cleanly encapsulate data transformation logic with appropriate null handling.
56-141: LGTM! Proper semantic table structure.The table uses correct
<thead>/<tbody>semantics, and thecolSpan={7}on the empty state row correctly matches the column count.
46-53: The endpoint is properly protected with authentication—no action required.The
/api/shop/admin/orders/reconcile-staleroute validates authentication viarequireAdminApi(), which verifies a valid session and requires theadminrole. CSRF risk is minimal here because the form is same-origin and the endpoint is strictly authenticated (only accessible to logged-in admins). Using a native form POST for authenticated admin actions is a standard, secure pattern in Next.js.frontend/lib/services/products/cart/rehydrate.ts (2)
103-143: LGTM! Robust price validation.The validation logic correctly ensures
priceMinoris:
- A finite number (line 106-107)
- An integer (line 110)
- A safe integer and positive (line 119)
The same validation is applied to the legacy fallback path (line 138), maintaining consistency.
177-189: LGTM! Consistent summary structure.The summary correctly provides both canonical (
totalAmountMinor) and display (totalAmount) values, maintaining the same pattern used for individual items.frontend/app/[locale]/shop/cart/page.tsx (2)
224-226: LGTM! Correct usage of minor-unit field.The change from
lineTotaltolineTotalMinoraligns withformatMoney's expectedamountMinorparameter and the rehydration logic changes.
241-266: LGTM! Consistent minor-unit usage for totals.Both Subtotal and Total now correctly use
cart.summary.totalAmountMinor, matching the updatedrehydrateCartItemsoutput andformatMoney's expected input type.frontend/components/shop/header/nav-links.tsx (3)
42-46: Good extraction of shared link styling.The
baseLinkconstant consolidates common styling, improving maintainability. The focus-visible states are properly included for keyboard accessibility.
48-64: Home link implementation looks correct.The conditional rendering with
aria-currentfor active state is properly implemented. The active/inactive styling is consistent with the other navigation links.
80-84: Consistent active state handling across all links.The
aria-current="page"attribute and conditional styling are consistently applied to category links, which improves accessibility and visual feedback.frontend/components/header/UnifiedHeader.tsx (1)
54-63: Good accessibility and delegation of Home link rendering.Adding
aria-label="Primary"improves navigation landmark identification for screen readers. PassingincludeHomeLinktoNavLinkswhenisShopis true properly delegates the Home link rendering to the appropriate component.frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (2)
36-44: Guard and data fetching correctly implemented.The
guardShopAdminPage()is properly awaited before page rendering, ensuring unauthorized access is blocked early. ThenotFound()call for missing orders is appropriate.
154-231: Items table implementation is solid.The table correctly handles:
- Rendering items with proper fallback for missing data (
?? '-')- Currency formatting with minor unit handling
- Empty state with appropriate colspan
- Hover states for row interactivity
frontend/app/api/shop/catalog/route.ts (2)
7-8: Appropriate cache control configuration.Setting
dynamic = 'force-dynamic'andrevalidate = 0ensures fresh catalog data on each request, which aligns with theCache-Control: no-storeheader.
21-38: Overall implementation is functional.The route correctly:
- Extracts and validates query parameters
- Delegates to
getCatalogProductsfor data fetching- Returns JSON with appropriate cache headers
frontend/components/header/AppMobileMenu.tsx (1)
116-118: Event handling works as expected; no issue detected.The
LogoutButtoncomponent does not callstopPropagation()orpreventDefault(), so click events bubble to the parent<div>normally. BothhandleLogout()andclose()will execute as intended. The subsequent navigation to the login page does not prevent theclose()callback from being called, though the menu state becomes irrelevant after the page navigates.Likely an incorrect or invalid review comment.
frontend/components/shop/catalog-load-more.tsx (1)
1-25: LGTM!Clean refactor to a prop-driven presentational component. The separation of concerns is improved—loading state and callback are now controlled by the parent, making this component more reusable and testable.
frontend/components/shop/catalog-products-client.tsx (1)
112-131: LGTM!The rendering logic is clean with proper grid layout, conditional error display, and integration with the
CatalogLoadMorecomponent. The product deduplication inonLoadMoreusing aSetis a good defensive measure.frontend/app/[locale]/shop/products/page.tsx (1)
31-45: LGTM on URL canonicalization.The redirect logic correctly strips the
?page=parameter to ensure infinite-scroll URLs aren't bookmarked with stale page numbers. This aligns with the PR objective of fixing catalog pagination UX.frontend/app/api/shop/checkout/route.ts (4)
270-296: LGTM on existing order PI retrieval path.The 502 status for Stripe retrieval failures is appropriate—it correctly signals an upstream service issue rather than a client error.
334-355: Verify: No restock on attach failure for existing orders is intentional?For new orders (lines 449-454), attach failures trigger a restock attempt. For existing orders here, no restock is performed on attach failure—the order is left for retry or janitor cleanup.
This asymmetry may be intentional (existing orders might have complex state), but please confirm this is the desired behavior.
438-447: Good: Conflict errors preserve inventory reservation.The decision to skip restock on
InvalidPayloadError(conflict) is correct—the inventory remains reserved so the client can retry with the same idempotency key. The comment at lines 439-441 clearly documents this intent.
393-413: LGTM on PI creation failure handling.The best-effort restock on Stripe PI creation failure prevents stock lock-up when the payment provider is unavailable. Logging the restock error (line 409) ensures visibility if cleanup also fails.
frontend/lib/services/orders/checkout.ts (4)
49-51: LGTM on PaymentStatus documentation.The comment clearly documents the Stripe payment status state machine:
pending → requires_payment → paid/failed/refunded. This helps future maintainers understand the expected flow.
97-114: LGTM on release_pending guard.This correctly prevents re-reservation attempts while a release is in progress. The restock attempt before throwing ensures the cleanup workflow continues even if this request fails.
413-418: LGTM on initialPaymentStatus logic.The conditional correctly handles the DB CHECK constraint:
provider='none'requirespayment_status in ('paid','failed'), so starting at'paid'is correct. For Stripe,'pending'is appropriate until the PI is attached.
781-793: LGTM on restock-on-failure pattern.The best-effort restock with error logging ensures cleanup is attempted without masking the original error. If restock fails, the order remains in
release_pendingfor the janitor to handle—this aligns with the PR's goal of hardening inventory release invariants.frontend/lib/services/orders/restock.ts (3)
1-1: LGTM: Modern Node.js import style.Using
node:cryptois the recommended approach for Node.js 16+ built-in modules, providing clearer intent and avoiding potential conflicts with npm packages.
106-174: Well-structured orphan order handling with proper guard rails.The no-reservation path correctly:
- Throws
OrderStateInvalidErrorfor paid orders without refund reason (safety invariant)- Uses fail-closed semantics when no reason is provided
- Derives appropriate failure codes based on reason
- Guards payment status transitions with
extraWherebinding to prevent races
239-271: Critical invariant enforcement looks correct.The fail-safe block properly preserves the janitor-retryable state by:
- Keeping
inventoryStatus: 'release_pending'- Setting
stockRestored: falseandrestockedAt: null- Only setting
failureCodeforfailedorstalereasonsOne consideration: if a release fails during a
refundedorcanceledrestock, nofailureCodeis set. This may be intentional (webhook-driven finalization handles those), but verify this aligns with your observability/alerting requirements.frontend/lib/tests/restock-release-failure-invariant.test.ts (2)
162-176: LGTM: Key invariant assertions are correct.The test properly validates:
inventoryStatusis NOT'released'stockRestoredisfalserestockedAtisnull- Product stock remains at 0 (release not applied)
These assertions directly align with the fail-safe semantics documented in the restock service.
32-34: No issues found — the test setup is correct.The spy on
applyReleaseMovedoes not affectapplyReserveMovebecause they are completely independent functions with no internal calls between them. Both execute direct database operations without invoking each other. The test correctly:
- Sets up the spy on
applyReleaseMovebefore any calls (line 32)- Calls
applyReserveMovenormally at line 116, which is unaffected by the spy and executes as expected- Calls
restockOrderat line 141, which internally triggers the mockedapplyReleaseMove, consuming the singlemockResolvedValueOnceas intendedThe
mockResolvedValueOnceonly affects the first invocation of the spied function, and the test correctly validates that a failed release does not finalize the order state.frontend/lib/services/orders/payment-intent.ts (2)
35-42: LGTM: Clean state transition validation.The explicit allowed states array with backward compatibility for
requires_paymentis well-documented. The early validation prevents invalid transitions before hitting the guarded update.
58-76: Guarded payment status update correctly enforces atomicity.Using
guardedPaymentStatusUpdatewithallowSameStateUpdate: trueproperly handles:
pending→requires_payment(new orders)requires_payment→requires_payment(PI update with same state)The error handling preserves semantics and prevents dangerous transitions like
failed/refunded→requires_payment.frontend/lib/tests/checkout-set-payment-intent-reject-contract.test.ts (2)
1-67: LGTM! Well-structured test setup for contract testing.The mock setup cleanly isolates the checkout route from external dependencies (Stripe, auth, DB), enabling focused testing of the error handling path when
setOrderPaymentIntentrejects.
69-133: LGTM! Good coverage of both new and existing order scenarios.Both test cases correctly verify:
- 409 status code (not 400) when
InvalidPayloadErroris thrownCHECKOUT_CONFLICTerror code in the responserestockOrderis not invoked on conflictThis aligns with the PR objective to constrain checkout failure semantics appropriately.
frontend/lib/tests/checkout-stripe-error-contract.test.ts (2)
1-62: LGTM! Test setup correctly simulates Stripe failure after DB write.The mock configuration ensures
createPaymentIntentthrows aftercreateOrderWithItemssucceeds, validating the post-DB-write error handling path.
64-111: LGTM! Comprehensive coverage of 502 STRIPE_ERROR scenarios.Both test cases correctly verify:
- 502 status code for Stripe PI creation failures
STRIPE_ERRORcode in the response body- Message is a string
This aligns with the PR objective to map post-create Stripe failures to 502.
frontend/lib/psp/stripe.ts (5)
23-68: Well-implemented Stripe refund function with proper validation and idempotency support.The function correctly:
- Validates PSP availability and at least one refund target
- Validates
amountMinorwhen provided- Supports both
payment_intentandchargebased refunds- Passes idempotencyKey to Stripe for safe retries
70-82: LGTM! Efficient Stripe client caching.The caching strategy correctly reuses the Stripe instance when the secret key hasn't changed, avoiding unnecessary client instantiation.
84-125: LGTM! Robust payment intent creation with proper validation.The function correctly validates the amount as a safe positive integer before calling Stripe, preventing issues with floats, NaN, or excessively large values.
127-202: LGTM! Well-implemented retrieval and verification functions.All functions follow consistent patterns:
- Validate PSP availability
- Validate input parameters
- Log and wrap errors appropriately
77-79: No action needed. The Stripe API version2025-11-17.cloveris the official pinned version for stripe-node v20.0.0 and is fully compatible with the project's current configuration. This is not a future or unreleased version.Likely an incorrect or invalid review comment.
frontend/app/api/shop/webhooks/stripe/route.ts (7)
12-46: LGTM! Clean refund metadata normalization with backward compatibility.The
normalizeRefundsFromMetafunction correctly handles:
- New array format (
refunds[])- Legacy single refund format (
refund)- Empty/missing metadata
48-86: LGTM! Proper idempotency for refund upsert.The function correctly prevents duplicate refund entries by checking both
refundIdandidempotencyKey, and preservesrefundInitiatedAtif already set.
509-537: LGTM! Proper metadata merging for mismatch scenarios.The code correctly builds delta metadata with mismatch details and merges it with the previous metadata, preserving the refunds history.
562-607: LGTM! Consistent metadata handling for successful payments.The payment success path correctly:
- Builds delta metadata
- Merges with previous metadata
- Preserves refunds history
- Uses
guardedPaymentStatusUpdatewith proper guards
644-687: LGTM! Proper metadata handling for failed payments.The failure path correctly merges PSP metadata and triggers restock when appropriate.
714-757: LGTM! Consistent metadata handling for canceled payments.Follows the same pattern as failed payments, correctly merging metadata and triggering restock.
1013-1091: LGTM! Comprehensive refund event handling with metadata preservation.The refund paths correctly:
- Distinguish between partial and full refunds
- Store refund gate decisions in metadata for partial refunds
- Trigger
guardedPaymentStatusUpdateand restock for full refunds- Use the new
mergePspMetadatapattern consistentlyfrontend/lib/services/orders/refund.ts (5)
19-23: LGTM! Clean error helper with code attachment.The
invalidhelper provides a consistent way to createInvalidPayloadErrorwith a stable code for programmatic error handling.
52-79: LGTM! Well-designed idempotency key and append logic.The
appendRefundfunction correctly:
- Prevents duplicates by checking both
idempotencyKeyandrefundId- Preserves existing refunds
- Sets
refundInitiatedAtfor UI stateThe idempotency key format
refund:{orderId}:{amount}:{currency}is deterministic and appropriate.
81-101: LGTM! Proper precondition validation for refund eligibility.The function correctly validates:
- Order exists
- Payment provider is Stripe
- Order is in
paidstatus
103-149: LGTM! Robust preconditions and domain-level idempotency.The code correctly:
- Validates currency and amount before proceeding
- Requires at least one Stripe identifier
- Checks for existing refund with same idempotency key before calling Stripe
- Returns early if refund already initiated
151-184: LGTM! Clean Stripe refund initiation with proper separation of concerns.The implementation correctly:
- Calls Stripe
createRefundwith idempotency support- Persists only metadata (not
paymentStatus), leaving final state to webhook- Returns the order summary for API response
The comment about
payment_statussource of truth (webhook) is helpful documentation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const requestQueryKey = `${baseQuery}|l=${locale}`; | ||
| activeQueryRef.current = requestQueryKey; | ||
| query.set('locale', locale); |
There was a problem hiding this comment.
Race condition guard is ineffective as written.
At line 76, activeQueryRef.current is set to requestQueryKey, and then the same requestQueryKey is used for comparison at line 93. This means the check activeQueryRef.current !== requestQueryKey will always be false for the current request.
The intent appears to be: if filters change during the fetch, ignore stale responses. For that to work, the ref should only be updated when filters change (in the useEffect), not at the start of each request.
Proposed fix
const requestQueryKey = `${baseQuery}|l=${locale}`;
- activeQueryRef.current = requestQueryKey;
query.set('locale', locale);The activeQueryRef is already updated in the useEffect when baseQuery or locale changes, which is the correct place for it.
📝 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.
| const requestQueryKey = `${baseQuery}|l=${locale}`; | |
| activeQueryRef.current = requestQueryKey; | |
| query.set('locale', locale); | |
| const requestQueryKey = `${baseQuery}|l=${locale}`; | |
| query.set('locale', locale); |
🤖 Prompt for AI Agents
In `@frontend/components/shop/catalog-products-client.tsx` around lines 75 - 77,
The race-condition guard is ineffective because activeQueryRef.current is
overwritten at the start of each request; remove the assignment
activeQueryRef.current = requestQueryKey from the fetch-start path and instead
rely on the existing update in the useEffect that watches baseQuery and locale
so that activeQueryRef.current remains the canonical value to compare against
when resolving responses (ensure the comparison activeQueryRef.current !==
requestQueryKey stays in the response handling branch to drop stale responses).
| // Apply release moves. IMPORTANT invariant: | ||
| // do NOT mark released/stockRestored/restockedAt unless all releases are CONFIRMED ok. | ||
| const releaseFailures: Array<{ productId: string; reason: string }> = []; | ||
|
|
||
| for (const item of reservedMoves) { | ||
| try { | ||
| const res: unknown = await applyReleaseMove( | ||
| orderId, | ||
| item.productId, | ||
| item.quantity | ||
| ); | ||
|
|
||
| // Support both styles: | ||
| // - void return (treat as success) | ||
| // - { ok: boolean, reason?: string } return (explicit fail if ok === false) | ||
| if ( | ||
| res && | ||
| typeof res === 'object' && | ||
| 'ok' in (res as any) && | ||
| (res as any).ok === false | ||
| ) { | ||
| releaseFailures.push({ | ||
| productId: item.productId, | ||
| reason: String((res as any).reason ?? 'unknown'), | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| releaseFailures.push({ | ||
| productId: item.productId, | ||
| reason: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the applyReleaseMove function definition
ast-grep --pattern 'export async function applyReleaseMove($$$): $_'Repository: DevLoversTeam/devlovers.net
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Search for applyReleaseMove function definition
rg 'applyReleaseMove' -A 10 -B 2Repository: DevLoversTeam/devlovers.net
Length of output: 5835
🏁 Script executed:
#!/bin/bash
# Get the full implementation of applyReleaseMove
rg 'export async function applyReleaseMove' -A 50 frontend/lib/services/inventory.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1655
🏁 Script executed:
#!/bin/bash
# Get the complete applyReleaseMove function
rg 'export async function applyReleaseMove' -A 80 frontend/lib/services/inventory.ts | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 1956
The type check for ok === false is unreachable in the current implementation.
applyReleaseMove has a return type of Promise<{ ok: true; applied: boolean }> where ok is always the literal true. The function either returns { ok: true, applied: boolean } or throws an error—it never returns { ok: false }.
The defensive as any casts and the check for ok === false are dead code. The only path that populates releaseFailures is the catch block. Either update the applyReleaseMove signature to support ok: false returns, or remove the unnecessary type guards to clarify that failures are caught via exceptions only.
🤖 Prompt for AI Agents
In `@frontend/lib/services/orders/restock.ts` around lines 205 - 237, The check
for a falsey `ok` is dead because applyReleaseMove returns Promise<{ ok: true;
applied: boolean }> or throws, so remove the unreachable branching and casts: in
the loop over reservedMoves delete the whole "if (res && typeof res === 'object'
&& 'ok' in (res as any) && (res as any).ok === false) { ... }" block and rely on
the existing try/catch to push failures into releaseFailures; alternatively, if
you intend applyReleaseMove to sometimes return {ok: false}, update
applyReleaseMove's return type and implementation (and callers) to allow { ok:
false; reason?: string } so the current guard becomes valid—choose one approach
and make sure reservedMoves loop and releaseFailures handling are consistent
with that decision.
…e param + page size + server query)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/shop/admin/orders/page.tsx:
- Around line 68-75: The reconcile-stale form in
frontend/app/[locale]/shop/admin/orders/page.tsx posts to
/api/shop/admin/orders/reconcile-stale without a CSRF token; add a hidden CSRF
input to that <form> (generate the token on render using your app's CSRF helper,
e.g., getCsrfToken/getServerCsrfToken) and include it as part of the POST
payload, and then update the API handler for
/api/shop/admin/orders/reconcile-stale to validate that token server-side before
calling requireAdminApi or executing the reconcile logic (implement/ call a
verifyCsrfToken or equivalent function in the API route to reject requests with
missing/invalid tokens).
🧹 Nitpick comments (5)
frontend/app/[locale]/shop/admin/orders/page.tsx (2)
93-135: Consider reordering empty state check for clarity.The empty state check (lines 129-135) comes after
items.map(). While functionally correct (React flattens arrays), placing the empty check first would be more readable:{items.length === 0 ? ( <tr>...</tr> ) : ( items.map(order => ...) )}This is a minor style preference—the current code works correctly.
16-16: Inconsistent PAGE_SIZE across admin pages.Orders page uses
PAGE_SIZE = 50while Products page usesPAGE_SIZE = 25. If this is intentional (orders rows are more compact), consider adding a comment explaining the rationale. Otherwise, consider unifying for consistent UX.frontend/app/[locale]/shop/admin/products/page.tsx (3)
16-19: Extract sharedparsePagehelper to avoid duplication.This function is duplicated in
orders/page.tsx. Consider extracting it to a shared utility (e.g.,@/lib/pagination) to maintain DRY principles.Example shared utility
// `@/lib/pagination.ts` export function parsePage(input: string | undefined): number { const n = Number.parseInt(input ?? '1', 10); return Number.isFinite(n) && n > 0 ? n : 1; }
21-24: InconsistentformatDateimplementations across admin pages.This version correctly uses
localefor localized date formatting. However,orders/page.tsxhas a different implementation that ignores locale. Consider unifying both to use the locale-aware version for consistent date display.
83-86: Use English for code comments.The comment "стабільне сортування (tie-breaker)" is in Ukrainian. For consistency and team accessibility, consider using English: "stable sort (tie-breaker)".
- // стабільне сортування (tie-breaker) + // stable sort with tie-breaker .orderBy(desc(products.createdAt), desc(products.id))
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/app/[locale]/shop/admin/orders/page.tsxfrontend/app/[locale]/shop/admin/products/page.tsxfrontend/components/shop/admin/admin-pagination.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/components/shop/admin/admin-pagination.tsx (1)
frontend/lib/utils.ts (1)
cn(4-6)
frontend/app/[locale]/shop/admin/products/page.tsx (6)
frontend/lib/auth/guard-shop-admin-page.ts (1)
guardShopAdminPage(9-18)frontend/lib/shop/currency.ts (2)
resolveCurrencyFromLocale(23-28)formatMoney(115-128)frontend/db/schema/shop.ts (2)
productPrices(278-332)products(56-105)frontend/components/shop/admin/shop-admin-topbar.tsx (1)
ShopAdminTopbar(3-43)frontend/components/shop/admin/admin-product-status-toggle.tsx (1)
AdminProductStatusToggle(10-67)frontend/components/shop/admin/admin-pagination.tsx (1)
AdminPagination(16-57)
🔇 Additional comments (6)
frontend/components/shop/admin/admin-pagination.tsx (2)
11-14: LGTM on URL canonicalization logic.The
pageHrefhelper correctly omits?page=1for page 1 (and below), producing cleaner canonical URLs. This prevents duplicate content issues for search engines.
29-55: Well-structured pagination with good accessibility.The component correctly uses
aria-labelon the nav element,aria-disabledon inactive controls, andrel="prev"/"next"on links for SEO benefits. The conditional rendering pattern is clean.frontend/app/[locale]/shop/admin/orders/page.tsx (2)
16-21: LGTM on pagination helpers.
parsePagecorrectly handles edge cases: defaults to 1 for invalid/missing input, rejects NaN and non-positive numbers. The logic is sound.
52-59: Efficient pagination without COUNT query.The overfetch-by-one strategy correctly determines
hasNextwithout requiring a separate COUNT query, which is good for performance.frontend/app/[locale]/shop/admin/products/page.tsx (2)
106-121: Well-structured table with fixed layout.The table uses
table-fixedwith explicit column widths summing to 100%, ensuring consistent layout. The truncation withtitleattributes for hover tooltips is a good accessibility pattern.
197-201: LGTM on pagination integration.The
AdminPaginationcomponent is correctly wired withbasePath,page, andhasNextprops matching the overfetch-based detection logic.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <form action="/api/shop/admin/orders/reconcile-stale" method="post"> | ||
| <button | ||
| type="submit" | ||
| className="rounded-md border border-border px-3 py-1.5 text-sm font-medium text-foreground transition-colors hover:bg-secondary" | ||
| > | ||
| Reconcile stale | ||
| </button> | ||
| </form> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the reconcile-stale API route file
find . -type f -path "*/api/shop/admin/orders/*reconcile*" 2>/dev/nullRepository: DevLoversTeam/devlovers.net
Length of output: 132
🏁 Script executed:
#!/bin/bash
# Search for reconcile-stale in the codebase
rg -t ts -t tsx -t js "reconcile-stale" --files-with-matchesRepository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Read the reconcile-stale API route
cat -n ./frontend/app/api/shop/admin/orders/reconcile-stale/route.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1456
🏁 Script executed:
#!/bin/bash
# Find and read the admin auth file
find . -type f -path "*/lib/auth/admin*" | head -5Repository: DevLoversTeam/devlovers.net
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Search for requireAdminApi definition
rg -A 10 "export.*requireAdminApi|function requireAdminApi" --max-count 1Repository: DevLoversTeam/devlovers.net
Length of output: 689
Add CSRF protection to the reconcile-stale form submission.
The form POSTs to /api/shop/admin/orders/reconcile-stale without a CSRF token. The API route only validates authentication/authorization via requireAdminApi, which does not protect against CSRF attacks. Add a CSRF token to the form and validate it server-side to prevent cross-site request forgery on this admin state-changing operation.
🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/admin/orders/page.tsx around lines 68 - 75, The
reconcile-stale form in frontend/app/[locale]/shop/admin/orders/page.tsx posts
to /api/shop/admin/orders/reconcile-stale without a CSRF token; add a hidden
CSRF input to that <form> (generate the token on render using your app's CSRF
helper, e.g., getCsrfToken/getServerCsrfToken) and include it as part of the
POST payload, and then update the API handler for
/api/shop/admin/orders/reconcile-stale to validate that token server-side before
calling requireAdminApi or executing the reconcile logic (implement/ call a
verifyCsrfToken or equivalent function in the API route to reject requests with
missing/invalid tokens).
| isFeatured: products.isFeatured, | ||
| createdAt: products.createdAt, | ||
| price: productPrices.price, // numeric (major) from product_prices | ||
| price: productPrices.price, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check how productPrices.price and productPrices.priceMinor are used across the codebase
rg -n 'productPrices\.(price|priceMinor)' --type ts --type tsx -B 2 -A 2Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Find the schema definitions for productPrices
fd -t ts -t tsx | xargs grep -l "priceMinor\|productPrices" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 240
🏁 Script executed:
# Look at the specific file and context around line 73
cat -n frontend/app/\[locale\]/shop/admin/products/page.tsx | sed -n '65,85p'Repository: DevLoversTeam/devlovers.net
Length of output: 825
🏁 Script executed:
# Search for productPrices field usage across TypeScript/TSX files
rg 'productPrices\.(price|priceMinor)' -B 2 -A 2Repository: DevLoversTeam/devlovers.net
Length of output: 4276
🏁 Script executed:
# Find where productPrices schema/type is defined
rg 'productPrices.*=|export.*productPrices|const productPrices' -B 1 -A 3Repository: DevLoversTeam/devlovers.net
Length of output: 1621
🏁 Script executed:
# Check the broader context of the file to understand the schema and conversion logic
head -100 frontend/app/\[locale\]/shop/admin/products/page.tsx | tail -50Repository: DevLoversTeam/devlovers.net
Length of output: 1583
🏁 Script executed:
# Look at the admin queries to understand the pattern for admin pages
cat -n frontend/lib/services/products/admin/queries.ts | head -80Repository: DevLoversTeam/devlovers.net
Length of output: 2969
🏁 Script executed:
# Check if the admin products page is using the admin queries or doing its own query
cat -n frontend/app/\[locale\]/shop/admin/products/page.tsx | head -120Repository: DevLoversTeam/devlovers.net
Length of output: 5334
🏁 Script executed:
# Find the schema definition for productPrices to see all fields available
cat -n frontend/db/schema/shop.ts | grep -A 20 "export const productPrices"Repository: DevLoversTeam/devlovers.net
Length of output: 898
🏁 Script executed:
# Look at how the price field is rendered in the admin products page
cat -n frontend/app/\[locale\]/shop/admin/products/page.tsx | sed -n '120,180p'Repository: DevLoversTeam/devlovers.net
Length of output: 3341
🏁 Script executed:
# Check if there's currency info also selected that would be needed for priceMinor conversion
cat -n frontend/app/\[locale\]/shop/admin/products/page.tsx | sed -n '61,90p'Repository: DevLoversTeam/devlovers.net
Length of output: 1082
🏁 Script executed:
# Look for the rendering/formatting of the price in the table rows
cat -n frontend/app/\[locale\]/shop/admin/products/page.tsx | sed -n '140,200p'Repository: DevLoversTeam/devlovers.net
Length of output: 3326
🏁 Script executed:
# Check the fromDbMoney function to understand what conversion is happening
rg -A 5 'export.*function fromDbMoney'Repository: DevLoversTeam/devlovers.net
Length of output: 429
Use priceMinor instead of the legacy price field.
The schema designates priceMinor as the canonical field (integer, minor units), while price is a legacy mirror. The current code selects the legacy field and converts it at runtime via safeFromDbMoney. Since displayCurrency is already available from context, you can select and use priceMinor directly to eliminate the conversion overhead:
Changes needed
In the select statement (line 73):
- price: productPrices.price,
+ priceMinor: productPrices.priceMinor,In the rendering logic (lines 125-141):
- const priceMinor = safeFromDbMoney(row.price, {
- productId: row.id,
- currency: displayCurrency,
- });
-
return (
<tr key={row.id} className="hover:bg-muted/50">
...
<td className="px-3 py-2 text-foreground whitespace-nowrap">
- {priceMinor === null ? '-' : formatMoney(priceMinor, displayCurrency, locale)}
+ {row.priceMinor === null ? '-' : formatMoney(row.priceMinor, displayCurrency, locale)}
</td>This aligns with the schema's design and the PR's standardization to minor units.
2 participants
Description
This PR closes several P0 holes in the shop’s critical payment/inventory flows (refund, release invariants, post-create Stripe failures), while also finishing shop UI/layout unification (shared container/theme tokens, admin topbar), improving catalog pagination, and fixing cart price rendering.
Related Issue
Issue: #<issue_number>
Changes
stripe.refunds.create) with idempotency; stores provider refund reference inpspMetadata.refunds[]; admin action marks refund initiated (not final). Finalization remains webhook-only (refund →refunded+ exactly-once restock).applyReleaseMovefails (throw or{ ok:false }), the system does not setinventoryStatus='released',stockRestored=true, orrestockedAt; order remains in a safe retry/janitor-repairable state. Includes regression test.502 STRIPE_ERROR, PI attach conflict →409 CHECKOUT_CONFLICT, invariant →500) with contract tests..shop-scopeoverrides; restores/aligns admin topbar container spacing; shop header nav gets “Home” link and hover/active styling aligned with platform nav.Database Changes (if applicable)
How Has This Been Tested?
Commands / checks (PowerShell):
npm run buildnpx vitest runnpx vitest run .\lib\tests\restock-release-failure-invariant.test.tsScreenshots (if applicable)
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.