Lso/feat/shop#83
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds multi-currency support and locale-aware formatting across frontend and backend: introduces per-product price rows, canonical minor-unit money columns, locale→currency resolution, formatting utilities, DB migrations, API changes, admin UI updates, and tests covering currency behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FrontendAPI as App API (checkout route)
participant OrdersSvc as Orders Service
participant DB
participant Stripe
Client->>FrontendAPI: POST /api/shop/checkout (cart payload, Accept-Language)
note right of FrontendAPI#DDEBF7: resolveRequestLocale → locale + currency
FrontendAPI->>OrdersSvc: createOrderWithItems(items, locale)
OrdersSvc->>DB: getProductsForCheckout(productIds, currency)
DB-->>OrdersSvc: product rows with priceMinor/originalPriceMinor/currency
OrdersSvc->>OrdersSvc: validate PriceConfig (throw PriceConfigError on missing)
OrdersSvc->>DB: insert order & order_items (store totalAmountMinor, unitPriceMinor, lineTotalMinor)
DB-->>OrdersSvc: persisted orderId
OrdersSvc->>Stripe: create PaymentIntent(amount = totalAmountMinor, currency)
Stripe-->>OrdersSvc: paymentIntentId / client_secret
OrdersSvc-->>FrontendAPI: order response (id, paymentIntentId, currency, totals)
FrontendAPI-->>Client: 201 Created (order + payment info)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!p.price?.trim()) { | ||
| throw new InvalidPayloadError(`${p.currency}: price is required.`); |
There was a problem hiding this comment.
Accept minor-unit prices in createProduct
The admin create flow now sends prices as JSON that parseAdminProductForm converts into {currency, priceMinor} objects, but createProduct still validates using p.price?.trim() and assertMoneyString(p.price, ...). That means a normal POST /api/shop/admin/products with the current UI payload will always trip InvalidPayloadError (“price is required”) because price is undefined, effectively breaking product creation whenever the form includes prices. Consider updating createProduct/normalizePricesFromInput to handle priceMinor (or converting back to strings before validation).
Useful? React with 👍 / 👎.
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/page.tsx (1)
41-46: Missing locale prefix in "New product" link.The href is hardcoded as
/shop/admin/products/newbut should include the locale prefix for consistency with other links in this file.🔎 Suggested fix
<Link - href="/shop/admin/products/new" + href={`/${locale}/shop/admin/products/new`} className="rounded-md border border-border px-3 py-1.5 text-sm font-medium text-foreground transition-colors hover:bg-secondary" > New product </Link>
🧹 Nitpick comments (30)
frontend/components/shop/shop-header.tsx (1)
46-47: Minor: accidental whitespace inside button element.There's an extra blank line inside the Search button element that appears to be an unintentional formatting artifact.
🔎 Proposed fix
<button className="flex h-9 w-9 items-center justify-center rounded-md text-muted-foreground transition-colors hover:bg-secondary hover:text-foreground" aria-label="Search" > - <Search className="h-5 w-5" /> </button>frontend/app/api/shop/orders/[id]/route.ts (2)
63-72: Consider graceful handling instead of throwing on corrupt data.Throwing an error for corrupt order items will result in a 500 response, obscuring the root cause from API consumers. Consider logging and filtering out corrupt items, or returning a specific error code indicating data integrity issues.
🔎 Alternative: log and skip corrupt items
if ( !item.productId || item.quantity === null || !item.unitPrice || !item.lineTotal ) { - throw new Error('Corrupt order item row: required columns are null'); + console.error('Corrupt order item row: required columns are null', { itemId: item.id }); + return null; }
140-144: Missing semicolon after query chain.The Drizzle query chain ends without a semicolon after
.where(whereClause). While JavaScript's ASI may handle this, explicit semicolons improve readability and prevent potential issues with subsequent code.🔎 Proposed fix
.from(orders) .leftJoin(orderItems, eq(orderItems.orderId, orders.id)) - .where(whereClause) - + .where(whereClause);frontend/lib/shop/currency.ts (2)
4-9: Duplicate validation logic withmoney.ts.
assertMinorUnitsStrictduplicates the exact logic ofassertIntegerCentsStrictinmoney.ts. Consider importing and reusing the existing function to maintain DRY principles.🔎 Proposed fix
+import { assertIntegerCentsStrict } from "./money"; + export const currencyValues = ["USD", "UAH"] as const; export type CurrencyCode = (typeof currencyValues)[number]; -function assertMinorUnitsStrict(minor: number): number { - if (!Number.isFinite(minor) || !Number.isInteger(minor) || minor < 0) { - throw new Error("Invalid money minor-units value"); - } - return minor; -}Then replace usages of
assertMinorUnitsStrictwithassertIntegerCentsStrict.
93-103: Unreachable default case ingetCurrencyFractionDigits.Since
CurrencyCodeis exhaustively"USD" | "UAH", thedefaultcase is unreachable. This is fine for future-proofing, but consider adding an exhaustiveness check or removing the redundant default.🔎 Optional: add exhaustiveness check
function getCurrencyFractionDigits(currency: CurrencyCode): number { switch (currency) { case "USD": case "UAH": return 2; - default: - return 2; + default: { + const _exhaustive: never = currency; + return 2; + } } }frontend/lib/services/errors.ts (1)
1-15: Remove commented-out code.The commented code blocks at lines 1-3 and 13-15 should be removed. These definitions are now implemented below (lines 16-22 and 35-41 respectively), making the commented sections dead code.
🔎 Proposed cleanup
-// export class InvalidPayloadError extends Error { -// code = "INVALID_PAYLOAD" as const -// } - export class InsufficientStockError extends Error { code = "INSUFFICIENT_STOCK" as const } export class OrderNotFoundError extends Error { code = "ORDER_NOT_FOUND" as const } -// export class SlugConflictError extends Error { -// code = "SLUG_CONFLICT" as const -// } export class InvalidPayloadError extends Error {frontend/components/shop/product-card.tsx (1)
59-66: Remove redundant null check.Line 63 contains redundant logic: the outer conditional already checks
product.originalPrice &&, so the inner ternaryproduct.originalPrice ? formatMoney(...) : nullwill never evaluate tonull.🔎 Proposed simplification
{product.originalPrice && ( <span className="text-sm text-muted-foreground line-through"> - {product.originalPrice ? formatMoney(product.originalPrice, product.currency, locale) : null} - + {formatMoney(product.originalPrice, product.currency, locale)} </span> )}frontend/app/api/shop/cart/rehydrate/route.ts (1)
74-74: Remove or translate non-English comment.The comment "якщо ти десь все ще кидаєш MoneyValueError (coercePriceFromDb)" is in Ukrainian. For codebase consistency and maintainability by all team members, translate to English or remove if it's a development note.
frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (1)
43-48: Consider simplifying the type guard.The filter's type guard is verbose. Since
productPrices.currencyis a DB enum that should matchCurrencyCode, you could simplify:🔎 Optional simplification
-.filter((p): p is typeof p & { currency: CurrencyCode } => - currencyValues.includes(p.currency as CurrencyCode) -) -.map((p) => ({ - currency: p.currency as CurrencyCode, +.map((p) => ({ + currency: p.currency,This assumes the DB enum values are guaranteed to match
CurrencyCode. If there's a concern about data inconsistency, the current defensive approach is fine.frontend/lib/tests/checkout-currency-policy.test.ts (1)
105-114: RedundantproductPricesdeletion — cascade handles this.Line 111 explicitly deletes from
productPrices, but sinceproductPrices.productIdreferencesproducts.idwithonDelete: 'cascade', deleting products (line 112) would cascade-delete the prices automatically.This is harmless but could be simplified:
🔎 Optional simplification
afterAll(async () => { if (createdOrderIds.length) { await db.delete(orders).where(inArray(orders.id, createdOrderIds)); } if (createdProductIds.length) { - await db.delete(productPrices).where(inArray(productPrices.productId, createdProductIds)); await db.delete(products).where(inArray(products.id, createdProductIds)); } });frontend/app/api/shop/admin/products/[id]/route.ts (2)
113-147: Track the HOTFIX for proper resolution.The comment "HOTFIX: parse/validate prices прямо з formData (не довіряємо parseAdminProductForm)" suggests this bypasses the standard form parser. While the implementation is correct, this should be tracked:
- Consider updating
parseAdminProductFormto handle prices properly- Remove the HOTFIX once the standard parser supports prices
- The Ukrainian comment should be translated for consistency
Do you want me to open an issue to track refactoring
parseAdminProductFormto properly handle the prices field?
21-40: Consider adding duplicate currency validation.The
adminPricesSchemaallows multiple entries with the same currency. If the intent is one price per currency, add a refinement:🔎 Optional enhancement
-const adminPricesSchema = z.array(adminPriceRowSchema); +const adminPricesSchema = z.array(adminPriceRowSchema).refine( + (prices) => { + const currencies = prices.map(p => p.currency); + return new Set(currencies).size === currencies.length; + }, + { message: 'Duplicate currencies are not allowed' } +);frontend/app/api/shop/checkout/route.ts (1)
413-419: Remove unnecessaryas anycasts.
OrderStateInvalidErroralready definesfieldandrawValueas class properties (per the error class definition infrontend/lib/services/errors.ts), so theas anycasts are unnecessary and reduce type safety.🔎 Suggested fix
if (error instanceof OrderStateInvalidError) { return errorResponse(error.code, error.message, 500, { orderId: error.orderId, - field: (error as any).field, - rawValue: (error as any).rawValue, + field: error.field, + rawValue: error.rawValue, }); }frontend/app/[locale]/shop/admin/products/page.tsx (1)
14-19: Consider extractingtoCurrencyCodeto a shared utility.This helper is duplicated in
StripePaymentClient.tsx. For consistency and maintainability, consider moving it to@/lib/shop/currency.tsalongside the existing currency utilities.Not blocking, but worth addressing in a follow-up.
frontend/app/[locale]/shop/admin/orders/[id]/page.tsx (1)
10-18: Extract duplicated helpers to a shared module.
pickMinorandorderCurrencyare duplicated fromfrontend/app/[locale]/shop/admin/orders/page.tsx. Consider extracting these to a shared utility (e.g.,@/lib/shop/admin-helpers.ts) to reduce maintenance burden.Not blocking, but recommended for a follow-up.
frontend/lib/shop/data.ts (1)
75-76: Translate comment to English for codebase consistency.The comment is in Ukrainian. For maintainability by all team members, consider using English.
🔎 Suggested fix
- // якщо валідатор/маппер впав — не "видаємо" биті дані + // If validator/mapper failed, don't expose broken data return { kind: "not_found" };frontend/app/[locale]/shop/checkout/payment/StripePaymentClient.tsx (1)
31-36:toCurrencyCodeis duplicated across multiple files.This helper now appears in:
StripePaymentClient.tsxAdminProductsPageConsider consolidating into
@/lib/shop/currency.tsas a single source of truth.frontend/db/queries/shop/admin-orders.ts (1)
82-86: Type assertion may mask shape mismatches.The spread
...rincludes the DB-selectedtotalAmount(legacy string from DB), but line 85 immediately overwrites it with the derived value fromtotalAmountMinor. Theas AdminOrderListItemcast hides any potential field mismatches.Consider explicitly picking required fields or using a type-safe mapper function to ensure the returned shape matches
AdminOrderListItemwithout relying on a cast.frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (1)
36-49: Consider tightening the type cast.The
as anycast on line 37 loses type safety. While the runtime checks on lines 39-40 validate the currency, consider using a more explicit approach:🔎 Suggested improvement
function normalizePriceRow(row: unknown): PriceRow | null { - const r = row as any; - - const currency = r?.currency as CurrencyCode | undefined; + if (typeof row !== 'object' || row === null) return null; + const r = row as Record<string, unknown>; + + const currency = typeof r.currency === 'string' ? r.currency : undefined; if (currency !== 'USD' && currency !== 'UAH') return null;frontend/db/queries/shop/products.ts (1)
179-190: Sorting byproductPrices.priceis a string (numeric) column.The
productPrices.pricecolumn is defined asnumericin the schema. Sorting by this column should work correctly for monetary ordering. However, for consistency with the minor-unit approach, consider sorting byproductPrices.priceMinor(integer) instead, which would be more efficient.🔎 Suggested change
function applySorting(sort?: CatalogSort): SQL { switch (sort) { case "price-asc": - return asc(productPrices.price); + return asc(productPrices.priceMinor); case "price-desc": - return desc(productPrices.price); + return desc(productPrices.priceMinor); case "newest": return desc(products.createdAt); default: return desc(products.createdAt); } }frontend/lib/services/products.ts (7)
189-244: Type safety concern withas anycasts in transitional code.While the transitional support for both
input.prices[]and legacyinput.price/originalPrice/currencyis necessary, the extensive use ofas anybypasses TypeScript's type checking. Consider adding a discriminated union type or runtime type guard to make this safer.The validation logic in
validatePriceRowsis thorough and correctly handles all edge cases.🔎 Optional: Add a type guard for safer input handling
type NewPriceInput = { prices: PriceRowInput[] }; type LegacyPriceInput = { price: string; originalPrice?: string | null; currency?: CurrencyCode }; function hasNewPriceFormat(input: unknown): input is NewPriceInput { return typeof input === 'object' && input !== null && 'prices' in input && Array.isArray((input as any).prices); }
246-349: Consider extracting price insertion logic to reduce duplication.The
createProductfunction correctly:
- Uses a transaction for atomicity
- Handles image cleanup on failure
- Enforces USD requirement
- Writes both canonical minor units and legacy mirror fields
However, the price insertion logic (lines 306-329) is duplicated in
updateProduct. Consider extracting to a helper.
351-426: Potential issue: SKU field handling may unintentionally clear values.The SKU update logic on lines 401-406 has a subtle issue: when
input.skuis an empty string"", it will be coerced tonull, but ifinput.skuisundefined, it preservesexisting.sku. This is likely intentional, but the nested ternary is hard to follow.🔎 Suggested clarification
- sku: - (input as any).sku !== undefined - ? (input as any).sku - ? (input as any).sku - : null - : existing.sku, + // Preserve existing SKU if not provided; clear if explicitly set to empty string + sku: (input as any).sku === undefined + ? existing.sku + : ((input as any).sku || null),
440-482: Upsert pattern is correct but could benefit from batch operation.The sequential update-then-insert pattern for price rows is functionally correct. However, for products with many currencies, this creates N+1 queries. Drizzle supports
onConflictDoUpdatewhich could consolidate this.🔎 Consider using onConflictDoUpdate for batch upsert
// Instead of looping with update-then-insert: await tx.insert(productPrices) .values(prices.map(p => ({ productId: id, currency: p.currency, priceMinor, originalPriceMinor: originalMinor, price: priceDb, originalPrice: originalDb, }))) .onConflictDoUpdate({ target: [productPrices.productId, productPrices.currency], set: { priceMinor: sql`excluded.price_minor`, originalPriceMinor: sql`excluded.original_price_minor`, price: sql`excluded.price`, originalPrice: sql`excluded.original_price`, updatedAt: new Date(), }, });
552-567: Return type usesunknownfor price fields - potential type safety gap.The
getAdminProductPricesfunction returnsprice: unknownandoriginalPrice: unknown. This forces downstream consumers to perform their own type assertions. Consider using the actual DB column types or a coerced number type for better type safety.🔎 Suggested type improvement
export async function getAdminProductPrices(productId: string): Promise< Array<{ currency: CurrencyCode; - price: unknown; - originalPrice: unknown; + price: string; // DB decimal as string + originalPrice: string | null; }> > {
668-674: Use English for code comments to maintain consistency.The comment on line 668 is in Ukrainian. For maintainability and team collaboration, use English throughout.
🔎 Suggested fix
- // критично: ціна має бути з product_prices для поточної currency + // Critical: price must come from product_prices for the current currency if (!product.priceCurrency || product.price == null) {
682-697: Redundant type assertion and fallback logic.The
priceMinorfield is already selected in the query (line 630), so the(product as any).priceMinorcast is unnecessary. Additionally, the fallback tocoercePriceFromDbshould rarely trigger given the earlier null check. Consider simplifying.🔎 Suggested simplification
- const unitPriceCents = - typeof (product as any).priceMinor === 'number' && - Number.isFinite((product as any).priceMinor) - ? Math.trunc((product as any).priceMinor) - : toCents( - coercePriceFromDb(product.price, { - field: 'price', - productId: product.id, - }) - ); + // priceMinor is the canonical source; fallback handles legacy data + const unitPriceCents = + product.priceMinor != null && Number.isFinite(product.priceMinor) + ? Math.trunc(product.priceMinor) + : toCents( + coercePriceFromDb(product.price, { + field: 'price', + productId: product.id, + }) + );frontend/lib/validation/shop.ts (3)
270-294: Code duplication in update schema validation.The duplicate currency and USD-required validation is duplicated between
productAdminSchemaandproductAdminUpdateSchema. Consider extracting to a shared refinement function.🔎 Extract shared validation
function validatePricesRefinement( prices: Array<{ currency: string }> | undefined, ctx: z.RefinementCtx ) { if (!prices) return; const seen = new Set<string>(); prices.forEach((p, idx) => { if (seen.has(p.currency)) { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ['prices', idx, 'currency'], message: 'Duplicate currency in prices', }); } else { seen.add(p.currency); } }); if (!prices.find(p => p.currency === 'USD')) { ctx.addIssue({ code: z.ZodIssueCode.custom, path: ['prices'], message: 'USD price is required', }); } }
344-359: Redundant.or(z.literal('NONE'))for badge.Line 355:
badgeSchema.or(z.literal('NONE'))is redundant since'NONE'is already included inproductBadgeValues(line 21) and thus inbadgeSchema.🔎 Simplify badge validation
- badge: badgeSchema.or(z.literal('NONE')), + badge: badgeSchema,
366-377: Minor: Extra blank lines.Lines 376-377 have unnecessary consecutive blank lines.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (51)
frontend/_audit/admin-currency-manifest.txtfrontend/app/[locale]/shop/admin/orders/[id]/page.tsxfrontend/app/[locale]/shop/admin/orders/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/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/page.tsxfrontend/app/[locale]/shop/page.tsxfrontend/app/[locale]/shop/products/[slug]/page.tsxfrontend/app/[locale]/shop/products/page.tsxfrontend/app/api/shop/admin/products/[id]/route.tsfrontend/app/api/shop/cart/rehydrate/route.tsfrontend/app/api/shop/checkout/route.tsfrontend/app/api/shop/orders/[id]/route.tsfrontend/checkout.jsonfrontend/components/shop/header/mobile-nav.tsxfrontend/components/shop/product-card.tsxfrontend/components/shop/shop-header.tsxfrontend/db/queries/shop/admin-orders.tsfrontend/db/queries/shop/products.tsfrontend/db/schema/shop.tsfrontend/drizzle/0006_minor_units_money.sqlfrontend/drizzle/0007_add-payment-intent-id-to-orders.sqlfrontend/drizzle/meta/0006_snapshot.jsonfrontend/drizzle/meta/0007_snapshot.jsonfrontend/drizzle/meta/_journal.jsonfrontend/eslint.config.mjsfrontend/lib/admin/parseAdminProductForm.tsfrontend/lib/cart.tsfrontend/lib/services/errors.tsfrontend/lib/services/orders.tsfrontend/lib/services/products.tsfrontend/lib/shop/currency.tsfrontend/lib/shop/data.tsfrontend/lib/shop/money.tsfrontend/lib/shop/payments.tsfrontend/lib/shop/request-locale.tsfrontend/lib/tests/__mocks__/server-only.tsfrontend/lib/tests/checkout-currency-policy.test.tsfrontend/lib/tests/currency.test.tsfrontend/lib/tests/format-money.test.tsfrontend/lib/tests/prices.test.tsfrontend/lib/types/shop.tsfrontend/lib/validation/shop.tsfrontend/package.jsonfrontend/project-structure.txtfrontend/vitest.config.ts
🧰 Additional context used
🧬 Code graph analysis (30)
frontend/components/shop/product-card.tsx (3)
frontend/lib/shop/data.ts (1)
ShopProduct(22-22)frontend/lib/validation/shop.ts (1)
ShopProduct(409-409)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/lib/cart.ts (2)
frontend/lib/validation/shop.ts (1)
CartRehydrateItem(414-414)frontend/db/queries/shop/orders.ts (1)
calculateOrderTotal(56-60)
frontend/app/[locale]/shop/cart/page.tsx (2)
frontend/components/shop/cart-provider.tsx (1)
useCart(139-141)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/lib/shop/payments.ts (3)
frontend/lib/validation/shop.ts (2)
PaymentStatus(15-15)PaymentProvider(15-15)frontend/components/shop/cart-provider.tsx (1)
CartContextType(19-25)frontend/db/queries/shop/orders.ts (1)
OrderItemSummary(24-31)
frontend/checkout.json (1)
frontend/components/shop/cart-provider.tsx (1)
CartContextType(19-25)
frontend/app/[locale]/shop/products/page.tsx (2)
frontend/lib/shop/data.ts (1)
getCatalogProducts(195-222)frontend/app/[locale]/blog/page.tsx (2)
BlogPage(20-69)generateMetadata(6-18)
frontend/components/shop/shop-header.tsx (3)
frontend/components/shared/LanguageSwitcher.tsx (3)
LanguageSwitcher(8-72)locale(53-67)event(19-23)frontend/app/[locale]/shop/layout.tsx (1)
ShopLayout(4-16)frontend/components/header/SiteHeader.tsx (1)
SiteHeader(14-93)
frontend/lib/shop/currency.ts (1)
frontend/lib/validation/shop.ts (1)
CurrencyCode(32-32)
frontend/lib/tests/currency.test.ts (1)
frontend/lib/shop/currency.ts (3)
resolveCurrencyFromLocale(23-28)parsePrimaryLocaleFromAcceptLanguage(33-43)resolveCurrencyFromHeaders(49-54)
frontend/app/[locale]/shop/checkout/error/page.tsx (2)
frontend/lib/shop/currency.ts (1)
formatPrice(134-150)frontend/db/queries/shop/orders.ts (1)
calculateOrderTotal(56-60)
frontend/lib/tests/checkout-currency-policy.test.ts (3)
frontend/db/index.ts (1)
db(17-17)frontend/db/schema/shop.ts (3)
products(33-76)productPrices(198-239)orders(78-138)frontend/app/api/shop/checkout/route.ts (1)
POST(123-447)
frontend/db/queries/shop/admin-orders.ts (5)
frontend/lib/shop/currency.ts (1)
CurrencyCode(2-2)frontend/lib/shop/payments.ts (2)
PaymentStatus(9-9)PaymentProvider(13-13)frontend/lib/types/shop.ts (2)
PaymentStatus(38-38)PaymentProvider(39-39)frontend/db/schema/shop.ts (1)
orders(78-138)frontend/lib/shop/money.ts (1)
toDbMoney(58-60)
frontend/lib/tests/format-money.test.ts (1)
frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/lib/tests/prices.test.ts (1)
frontend/lib/validation/shop.ts (1)
adminPriceRowSchema(173-198)
frontend/app/api/shop/checkout/route.ts (2)
frontend/lib/shop/request-locale.ts (1)
resolveRequestLocale(17-35)frontend/lib/services/errors.ts (2)
OrderStateInvalidError(43-59)PriceConfigError(24-34)
frontend/lib/services/orders.ts (7)
frontend/db/schema/shop.ts (4)
orderItems(140-181)orders(78-138)products(33-76)productPrices(198-239)frontend/lib/shop/currency.ts (2)
CurrencyCode(2-2)resolveCurrencyFromLocale(23-28)frontend/lib/validation/shop.ts (2)
CurrencyCode(32-32)OrderSummary(410-410)frontend/lib/services/errors.ts (4)
OrderStateInvalidError(43-59)InvalidPayloadError(16-22)PriceConfigError(24-34)OrderNotFoundError(9-11)frontend/lib/types/shop.ts (3)
OrderSummary(26-28)CheckoutItem(24-24)CheckoutResult(32-36)frontend/db/queries/shop/orders.ts (4)
OrderSummary(33-33)coercePriceFromDb(35-53)MoneyValueError(10-22)constructor(15-21)frontend/lib/shop/money.ts (3)
fromDbMoney(51-56)toDbMoney(58-60)sumLineTotals(70-76)
frontend/app/api/shop/orders/[id]/route.ts (1)
frontend/db/schema/shop.ts (1)
orders(78-138)
frontend/lib/shop/data.ts (7)
frontend/lib/validation/shop.ts (6)
ProductBadge(22-22)productBadgeValues(21-21)DbProduct(408-408)dbProductSchema(89-138)CatalogFilters(407-407)catalogFilterSchema(74-87)frontend/lib/shop/currency.ts (1)
resolveCurrencyFromLocale(23-28)frontend/db/queries/shop/products.ts (3)
getPublicProductBySlug(252-265)getPublicProductBaseBySlug(59-69)getActiveProductsPage(202-234)frontend/lib/types/shop.ts (1)
DbProduct(22-22)frontend/lib/shop/money.ts (1)
fromDbMoney(51-56)frontend/lib/config/catalog.ts (3)
CatalogSort(48-48)CATALOG_PAGE_SIZE(39-39)CATEGORY_TILES(41-45)frontend/db/schema/shop.ts (1)
products(33-76)
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (4)
frontend/lib/validation/shop.ts (2)
ProductAdminInput(411-411)CurrencyCode(32-32)frontend/lib/shop/currency.ts (1)
CurrencyCode(2-2)frontend/lib/config/catalog.ts (4)
CATEGORIES(1-9)PRODUCT_TYPES(11-16)COLORS(18-28)SIZES(30-30)frontend/app/[locale]/shop/admin/products/new/page.tsx (1)
NewProductPage(3-5)
frontend/drizzle/meta/0006_snapshot.json (2)
frontend/drizzle/schema.ts (7)
table(174-196)table(261-271)table(282-292)table(81-92)table(240-250)table(107-127)table(55-66)frontend/db/schema/quiz.ts (1)
table(72-77)
frontend/lib/admin/parseAdminProductForm.ts (3)
frontend/lib/shop/money.ts (1)
toCents(38-42)frontend/lib/shop/currency.ts (2)
currencyValues(1-1)CurrencyCode(2-2)frontend/lib/validation/shop.ts (3)
CurrencyCode(32-32)productAdminSchema(200-246)productAdminUpdateSchema(248-294)
frontend/lib/shop/money.ts (1)
frontend/db/queries/shop/orders.ts (1)
MoneyValueError(10-22)
frontend/db/queries/shop/products.ts (5)
frontend/db/schema/shop.ts (2)
products(33-76)productPrices(198-239)frontend/db/index.ts (1)
db(17-17)frontend/lib/shop/currency.ts (1)
CurrencyCode(2-2)frontend/lib/types/shop.ts (1)
DbProduct(22-22)frontend/lib/config/catalog.ts (1)
CatalogSort(48-48)
frontend/lib/services/errors.ts (1)
frontend/db/queries/shop/orders.ts (2)
MoneyValueError(10-22)constructor(15-21)
frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx (1)
frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
frontend/app/[locale]/shop/checkout/payment/StripePaymentClient.tsx (2)
frontend/lib/shop/currency.ts (3)
CurrencyCode(2-2)currencyValues(1-1)resolveCurrencyFromLocale(23-28)frontend/lib/validation/shop.ts (1)
CurrencyCode(32-32)
frontend/db/schema/shop.ts (4)
frontend/lib/validation/shop.ts (2)
PaymentStatus(15-15)PaymentProvider(15-15)frontend/lib/shop/payments.ts (2)
PaymentStatus(9-9)PaymentProvider(13-13)frontend/lib/types/shop.ts (2)
PaymentStatus(38-38)PaymentProvider(39-39)frontend/db/seed-quiz-types.ts (1)
t(42-48)
frontend/lib/types/shop.ts (2)
frontend/lib/validation/shop.ts (9)
productAdminSchema(200-246)productAdminUpdateSchema(248-294)DbProduct(408-408)dbProductSchema(89-138)checkoutItemSchema(296-303)OrderSummary(410-410)orderSummarySchema(383-404)paymentStatusSchema(28-28)paymentProviderSchema(29-29)frontend/db/queries/shop/orders.ts (1)
OrderSummary(33-33)
frontend/app/[locale]/shop/page.tsx (1)
frontend/lib/shop/data.ts (1)
getHomepageContent(242-271)
frontend/app/[locale]/shop/products/[slug]/page.tsx (3)
frontend/i18n/config.ts (1)
locales(1-1)frontend/lib/shop/data.ts (2)
getCatalogProducts(195-222)getProductPageData(65-98)frontend/lib/shop/currency.ts (1)
formatMoney(115-128)
| function formatDateTime(value: Date | null | undefined) { | ||
| if (!value) return "-" | ||
| return value.toLocaleString() | ||
| if (!value) return "-"; | ||
| return value.toLocaleString(); | ||
| } |
There was a problem hiding this comment.
Pass locale to toLocaleString for consistent datetime formatting.
Same issue as the orders list page — formatDateTime should use the locale for consistent formatting.
🔎 Suggested fix
-function formatDateTime(value: Date | null | undefined) {
+function formatDateTime(value: Date | null | undefined, locale?: string) {
if (!value) return "-";
- return value.toLocaleString();
+ return value.toLocaleString(locale);
}🤖 Prompt for AI Agents
In frontend/app/[locale]/shop/admin/orders/[id]/page.tsx around lines 20 to 23,
formatDateTime currently calls value.toLocaleString() without a locale; change
it to accept a locale parameter (or read the locale from the route
params/context) and call value.toLocaleString(locale) (or use new
Intl.DateTimeFormat(locale, options).format(value)) so datetime rendering is
consistent with the page locale.
| function formatDate(value: Date | null | undefined) { | ||
| if (!value) return "-" | ||
| return value.toLocaleDateString() | ||
| if (!value) return "-"; | ||
| return value.toLocaleDateString(); | ||
| } |
There was a problem hiding this comment.
Pass locale to toLocaleDateString for consistent formatting.
Unlike AdminProductsPage, this formatDate helper doesn't accept or use the locale, which may cause inconsistent date formats across the admin UI.
🔎 Suggested fix
-function formatDate(value: Date | null | undefined) {
+function formatDate(value: Date | null | undefined, locale?: string) {
if (!value) return "-";
- return value.toLocaleDateString();
+ return value.toLocaleDateString(locale);
}Then update the usage at line 64:
-<td className="px-3 py-2 text-muted-foreground">{formatDate(order.createdAt)}</td>
+<td className="px-3 py-2 text-muted-foreground">{formatDate(order.createdAt, locale)}</td>📝 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.
| function formatDate(value: Date | null | undefined) { | |
| if (!value) return "-" | |
| return value.toLocaleDateString() | |
| if (!value) return "-"; | |
| return value.toLocaleDateString(); | |
| } | |
| function formatDate(value: Date | null | undefined, locale?: string) { | |
| if (!value) return "-"; | |
| return value.toLocaleDateString(locale); | |
| } |
🤖 Prompt for AI Agents
In frontend/app/[locale]/shop/admin/orders/page.tsx around lines 19 to 22,
formatDate currently ignores the locale and calls toLocaleDateString() without a
locale argument; change the function signature to accept a locale (e.g.,
formatDate(value: Date | null | undefined, locale?: string)) and pass that
locale into toLocaleDateString(locale) (falling back to undefined if not
provided) so formatting matches the active locale, and update its call site at
line 64 to pass the page's locale variable.
| <div className="flex items-center justify-between"> | ||
| <span className="text-muted-foreground">Total</span> | ||
| <span className="font-semibold">{formatPrice(order.totalAmount)}</span> | ||
| <span className="font-semibold">{formatPrice(order.totalAmount, order.currency)}</span> |
There was a problem hiding this comment.
Potential type mismatch: order.totalAmount is a string.
Based on the schema, orders.totalAmount is a numeric column stored as a string. The formatPrice function expects a number for amountMajor. Without explicit conversion, this will fail the Number.isFinite() check and return "-".
🔎 Proposed fix
- <span className="font-semibold">{formatPrice(order.totalAmount, order.currency)}</span>
+ <span className="font-semibold">{formatPrice(Number(order.totalAmount), order.currency)}</span>📝 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.
| <span className="font-semibold">{formatPrice(order.totalAmount, order.currency)}</span> | |
| <span className="font-semibold">{formatPrice(Number(order.totalAmount), order.currency)}</span> |
🤖 Prompt for AI Agents
In frontend/app/[locale]/shop/checkout/error/page.tsx around line 107,
order.totalAmount is a string (DB numeric stored as string) but formatPrice
expects a number; convert the value to a number before passing it and guard
against NaN/null/undefined. Replace the direct use of order.totalAmount with a
parsed value (e.g., parseFloat or Number) and check Number.isFinite(parsed) so
you pass a valid number to formatPrice and fall back to a safe default or "-" if
the parsed value is not finite.
| <span className="text-sm text-muted-foreground">Amount due</span> | ||
| <span className="text-xl font-bold text-foreground"> | ||
| {formatPrice(order.totalAmount)} | ||
| {formatMoney(order.totalAmount, order.currency, locale)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the getOrderSummary function
rg -n 'getOrderSummary' --type=ts --type=tsx -B 2 -A 20 | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Check the order type definition
rg -n 'interface.*Order|type.*Order' --type=ts --type=tsx | head -30Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Look for totalAmount and totalAmountMinor fields in the codebase
rg -n 'totalAmount|totalAmountMinor' --type=ts --type=tsx | head -50Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Check formatMoney function signature
rg -n -A 5 'function formatMoney|export.*formatMoney' --type=ts --type=tsx | head -40Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Find getOrderSummary function
rg -n 'getOrderSummary' -A 20 | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 9183
🏁 Script executed:
# Look for order type definitions
rg -n 'type.*Order\s*=|interface.*Order\s*{' -A 15 | head -80Repository: DevLoversTeam/devlovers.net
Length of output: 206
🏁 Script executed:
# Search for totalAmount and totalAmountMinor usage
rg -n 'totalAmount|totalAmountMinor' | head -60Repository: DevLoversTeam/devlovers.net
Length of output: 4671
🏁 Script executed:
# Check formatMoney implementation
rg -n 'function formatMoney|const formatMoney|export.*formatMoney' -A 8 | head -50Repository: DevLoversTeam/devlovers.net
Length of output: 668
🏁 Script executed:
# Also check for the currency utilities file
fd -e ts -e js | grep -i 'currency\|format.*money' | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 228
🏁 Script executed:
# Find OrderSummary type definition
rg -n 'type OrderSummary|interface OrderSummary' -A 15Repository: DevLoversTeam/devlovers.net
Length of output: 4412
🏁 Script executed:
# Check the fromCents function to understand conversion
rg -n 'function fromCents|export.*fromCents' -A 5Repository: DevLoversTeam/devlovers.net
Length of output: 467
🏁 Script executed:
# Find the orderSummarySchema definition
rg -n 'const orderSummarySchema|orderSummarySchema\s*=' -A 30 | head -80Repository: DevLoversTeam/devlovers.net
Length of output: 2276
🏁 Script executed:
# Double-check what fields are returned by getOrderById
rg -n 'function getOrderById|export.*getOrderById' -A 40 | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 5170
🏁 Script executed:
# Find parseOrderSummary implementation
rg -n 'function parseOrderSummary|const parseOrderSummary' -A 30Repository: DevLoversTeam/devlovers.net
Length of output: 2149
🏁 Script executed:
# Get more of the parseOrderSummary function
rg -n 'function parseOrderSummary' -A 80 | head -120Repository: DevLoversTeam/devlovers.net
Length of output: 5414
Use order.totalCents instead of order.totalAmount when calling formatMoney.
formatMoney expects minor units (cents) as the first argument. The order.totalAmount field is in major units (e.g., 19.99), which will result in incorrect display values (e.g., $19.99 displays as $0.19). The order object includes a totalCents field that contains the correct minor units value.
Same issue at line 202.
🤖 Prompt for AI Agents
In frontend/app/[locale]/shop/checkout/payment/[orderId]/page.tsx around lines
169 and 202, formatMoney is being called with order.totalAmount (major units)
but expects minor units (cents); replace order.totalAmount with order.totalCents
in both calls so formatMoney receives the correct minor-unit value, keeping the
same currency and locale arguments.
| @@ -0,0 +1,7 @@ | |||
| ALTER TABLE "product_prices" DROP CONSTRAINT "product_prices_original_price_valid";--> statement-breakpoint | |||
There was a problem hiding this comment.
Constraint logic changed from >= to > — potential data integrity issue.
Migration 0006 (line 28-29) adds product_prices_original_price_valid with original_price_minor >= price_minor, but this migration drops it (line 1) and re-adds it with original_price_minor > price_minor (line 7).
If any existing rows have original_price_minor = price_minor, this migration will fail. Ensure no such data exists before applying, or adjust the constraint to match the intended business rule.
Also applies to: 7-7
🤖 Prompt for AI Agents
In frontend/drizzle/0007_add-payment-intent-id-to-orders.sql around lines 1 and
7, the migration changes the product_prices_original_price_valid constraint from
original_price_minor >= price_minor to original_price_minor > price_minor which
will fail if any rows have original_price_minor = price_minor; fix by either (A)
preserving the original semantics and re-create the constraint using >= to match
migration 0006, or (B) if the strict > is intended, add a pre-migration step
that updates or deletes rows where original_price_minor = price_minor (e.g., set
original_price_minor = NULL/adjust values or remove offending rows) and/or add
an explicit check that prevents the migration from running when such rows exist,
then re-add the > constraint.
| // Allow explicit any | ||
| { | ||
| rules: { | ||
| "@typescript-eslint/no-explicit-any": "off", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find explicit any usage in the frontend codebase
rg -n --type ts --type tsx -g '!*.test.*' -g '!*.spec.*' ': any\b' frontend/Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Find explicit any usage in the frontend codebase
# Search .ts files with typescript type, and .tsx files using glob pattern
rg -n --type ts -g '!*.test.*' -g '!*.spec.*' -g '*.tsx' ': any\b' frontend/Repository: DevLoversTeam/devlovers.net
Length of output: 1213
🏁 Script executed:
#!/bin/bash
# Check package.json for dependencies
cat frontend/package.json | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 2100
🏁 Script executed:
#!/bin/bash
# Look for TypeScript config and any type definitions
find frontend -maxdepth 2 -name "tsconfig.json" -o -name "types.ts" -o -name "types.d.ts" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Check TypeScript config
cat frontend/tsconfig.jsonRepository: DevLoversTeam/devlovers.net
Length of output: 819
🏁 Script executed:
#!/bin/bash
# Look at actual any usage in one of the main files
sed -n '15,40p' frontend/app/[locale]/blog/[slug]/PostDetails.tsxRepository: DevLoversTeam/devlovers.net
Length of output: 614
🏁 Script executed:
#!/bin/bash
# Check if there are any Sanity-specific type definitions in the project
find frontend -name "*sanity*" -o -name "*portable*" -o -name "*types*" | grep -E "\.(ts|tsx)$" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Check the actual usage in the admin orders file to see if unknown would be practical
sed -n '10,25p' frontend/app/[locale]/shop/admin/orders/page.tsxRepository: DevLoversTeam/devlovers.net
Length of output: 554
Consider creating type definitions for Sanity CMS data instead of disabling no-explicit-any globally.
The 11 instances of any in the frontend are primarily driven by Sanity CMS Portable Text and order objects—untyped external data sources. While the global disable is a pragmatic workaround, it unnecessarily weakens type safety across the entire codebase.
Better alternatives:
- Create Sanity type definitions (recommended): Define local types for Sanity schema objects (e.g.,
PortableTextBlock,Author,Post) instead of relying onany. - Inline suppressions: If type definitions aren't feasible, use targeted suppressions at the Sanity data entry points rather than globally.
For example, replace the global override with inline suppressions in specific files:
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const portableText: any = post.body;🤖 Prompt for AI Agents
In frontend/eslint.config.mjs around lines 16 to 21 the project currently
disables "@typescript-eslint/no-explicit-any" globally which weakens type
safety; instead remove that global rule and create local Sanity type definitions
(e.g., PortableTextBlock, Author, Post, Order) in a shared types file and
replace the 11 occurrences of any with these types at Sanity data boundaries; if
immediate typing is not possible, replace the global disable with targeted
ESLint suppressions (inline eslint-disable-next-line or an overrides block
scoped to the specific Sanity files/globs) so only Sanity parsing sites allow
explicit any while the rest of the codebase remains typed.
| const legacyRawPrices = [ | ||
| ...(priceUsdMinor !== undefined || originalPriceUsdMinor !== undefined | ||
| ? [ | ||
| { | ||
| currency: 'USD' as const, | ||
| priceMinor: priceUsdMinor ?? null, | ||
| originalPriceMinor: originalPriceUsdMinor ?? null, | ||
| }, | ||
| ] | ||
| : []), | ||
| ...(priceUahMinor !== undefined || originalPriceUahMinor !== undefined | ||
| ? [ | ||
| { | ||
| currency: 'UAH' as const, | ||
| priceMinor: priceUahMinor ?? null, | ||
| originalPriceMinor: originalPriceUahMinor ?? null, | ||
| }, | ||
| ] | ||
| : []), | ||
| ]; | ||
|
|
||
| // Resolve final prices with PATCH semantics | ||
| const prices = | ||
| pricesJson && 'value' in pricesJson | ||
| ? pricesJson.value | ||
| : mode === 'update' && legacyRawPrices.length === 0 | ||
| ? undefined | ||
| : legacyRawPrices; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's search for the productAdminSchema definition
find . -type f -name "*.ts" -o -name "*.js" | xargs grep -l "productAdminSchema" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 179
🏁 Script executed:
# Let's examine the productAdminSchema definition
cat -n ./frontend/lib/validation/shop.ts | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 4099
🏁 Script executed:
# Search for productAdminSchema specifically and show context
rg -A 50 "productAdminSchema" ./frontend/lib/validation/shop.ts | head -80Repository: DevLoversTeam/devlovers.net
Length of output: 2342
🏁 Script executed:
# Find the adminPriceRowSchema definition
rg "adminPriceRowSchema" ./frontend/lib/validation/shop.ts -B 5 -A 10Repository: DevLoversTeam/devlovers.net
Length of output: 2111
🏁 Script executed:
# Let's verify the exact validation by checking the moneyMinor definitions again and the full context
rg -B 10 "const moneyMinor" ./frontend/lib/validation/shop.tsRepository: DevLoversTeam/devlovers.net
Length of output: 490
priceMinor cannot be null; schema requires a positive integer.
The legacyRawPrices objects set priceMinor: priceUsdMinor ?? null, but adminPriceRowSchema expects priceMinor to satisfy moneyMinorPositive (a positive integer ≥ 1). When only originalPrice is defined and price is undefined, the resulting null value will fail schema validation.
🤖 Prompt for AI Agents
In frontend/lib/admin/parseAdminProductForm.ts around lines 217 to 244, the
legacyRawPrices entries currently set priceMinor to null when price is undefined
which violates adminPriceRowSchema (priceMinor must be a positive integer);
change the object construction to avoid assigning null — only include the
priceMinor property when priceUsdMinor/priceUahMinor is a number (>=1),
otherwise omit the priceMinor key (and similarly only include originalPriceMinor
if present) so the produced payload doesn't contain null for priceMinor and will
pass validation; use conditional property inclusion instead of the `?? null`
pattern.
| const numeric = | ||
| typeof parsed === "string" && parsed.length > 0 ? Number(parsed) : Number(parsed); |
There was a problem hiding this comment.
Redundant conditional in parseAmount.
The ternary condition always evaluates to Number(parsed) regardless of the string check:
const numeric =
typeof parsed === "string" && parsed.length > 0 ? Number(parsed) : Number(parsed);Both branches return the same expression.
🔎 Proposed fix
- const numeric =
- typeof parsed === "string" && parsed.length > 0 ? Number(parsed) : Number(parsed);
+ const numeric = Number(parsed);🤖 Prompt for AI Agents
In frontend/lib/shop/money.ts around lines 11 to 12, the ternary assignment to
`numeric` is redundant because both branches call `Number(parsed)`; replace the
entire conditional expression with a single assignment `numeric =
Number(parsed)` (or equivalent single call) so the code is simplified and
returns the same numeric conversion without the unnecessary type/length check.
| import { describe, expect, it } from "vitest"; | ||
| import { adminPriceRowSchema } from "../validation/shop"; | ||
|
|
||
| describe("pricing validation", () => { | ||
| it("rejects originalPrice == price (SALE must be strict)", () => { | ||
| const r = adminPriceRowSchema.safeParse({ | ||
| currency: "USD", | ||
| price: "10.00", | ||
| originalPrice: "10.00", | ||
| }); | ||
| expect(r.success).toBe(false); | ||
| }); | ||
|
|
||
| it("accepts originalPrice > price", () => { | ||
| const r = adminPriceRowSchema.safeParse({ | ||
| currency: "USD", | ||
| price: "10.00", | ||
| originalPrice: "12.00", | ||
| }); | ||
| expect(r.success).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Critical: Test uses incorrect field names.
The tests use price and originalPrice fields, but according to the schema snippet from frontend/lib/validation/shop.ts (lines 172-197), adminPriceRowSchema expects priceMinor and originalPriceMinor fields. This will cause both tests to fail.
🔎 Proposed fix
describe("pricing validation", () => {
it("rejects originalPrice == price (SALE must be strict)", () => {
const r = adminPriceRowSchema.safeParse({
currency: "USD",
- price: "10.00",
- originalPrice: "10.00",
+ priceMinor: 1000,
+ originalPriceMinor: 1000,
});
expect(r.success).toBe(false);
});
it("accepts originalPrice > price", () => {
const r = adminPriceRowSchema.safeParse({
currency: "USD",
- price: "10.00",
- originalPrice: "12.00",
+ priceMinor: 1000,
+ originalPriceMinor: 1200,
});
expect(r.success).toBe(true);
});
});🤖 Prompt for AI Agents
In frontend/lib/tests/prices.test.ts around lines 1 to 22, the tests use
incorrect field names (price and originalPrice); update them to the schema's
expected fields priceMinor and originalPriceMinor and supply values in minor
units (integers, e.g. 1000 for $10.00, 1200 for $12.00) while keeping currency;
this will align the tests with adminPriceRowSchema and make the success/failure
assertions valid.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (3)
44-65: Consider centralizingparseMajorToMinorandformatMinorToMajorhelpers.These functions are duplicated across multiple files (
route.ts,edit/page.tsx). Consider extracting them to a shared module like@/lib/shop/currency.tsto ensure consistent behavior and easier maintenance.🔎 Verification script to find duplicates
#!/bin/bash # Find all occurrences of parseMajorToMinor across the codebase rg -n "function parseMajorToMinor" --type ts
67-92: Avoidanytype casting innormalizeUiPriceRow.Using
as anybypasses type safety. Consider defining a more precise input type or using a Zod schema for runtime validation.🔎 Suggested improvement
-function normalizeUiPriceRow(row: unknown): UiPriceRow | null { - const r = row as any; - - const currency = r?.currency as CurrencyCode | undefined; +type RawPriceRow = { + currency?: string; + price?: string; + priceMinor?: number | string; + originalPrice?: string | null; + originalPriceMinor?: number | string | null; +}; + +function normalizeUiPriceRow(row: unknown): UiPriceRow | null { + if (typeof row !== 'object' || row === null) return null; + const r = row as RawPriceRow; + + const currency = r.currency; if (currency !== 'USD' && currency !== 'UAH') return null;
119-121: Type assertion oninitialValuescould mask type errors.The cast
(initialValues as any)?.pricescircumvents type checking. IfProductFormProps['initialValues']doesn't includeprices, consider updating the type definition to include it.- const [prices, setPrices] = useState<UiPriceRow[]>( - ensureUiPriceRows((initialValues as any)?.prices) - ); + const [prices, setPrices] = useState<UiPriceRow[]>( + ensureUiPriceRows(initialValues?.prices) + );This requires updating
ProductFormProps.initialValuesto include thepricesfield.frontend/app/api/shop/admin/products/[id]/route.ts (2)
26-38: DuplicatedparseMajorToMinorfunction.This is the third copy of this function (also in
product-form.tsxandedit/page.tsx). Extract to a shared utility module.
150-151: Remove or clarify the HOTFIX comment.The comment
// 2) HOTFIX: parse/validate prices прямо з formData (не довіряємо parseAdminProductForm)suggests this was a temporary workaround. Consider either:
- Documenting why
parseAdminProductFormcan't be trusted for prices- Fixing
parseAdminProductFormto handle prices correctly- At minimum, translating the Ukrainian comment for broader team understanding
frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (2)
40-47: Direct DB query bypasses service layer.This page queries
productPricesdirectly instead of using a service function likegetAdminProductByIdWithPrices. Consider using the service layer for consistency and to centralize data access logic.The API route already uses
getAdminProductByIdWithPrices- this page could potentially fetch from the API or use the same service function server-side.
51-52: Type guard could be simplified.The type predicate
(p): p is typeof p & { currency: CurrencyCode }is verbose. SincecurrencyValues.includesalready narrows the type conceptually, consider a cleaner approach.- .filter((p): p is typeof p & { currency: CurrencyCode } => - currencyValues.includes(p.currency as CurrencyCode) - ) + .filter((p) => (currencyValues as readonly string[]).includes(p.currency))Or use a type guard function defined elsewhere for reuse.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.hintrcfrontend/app/[locale]/shop/admin/products/[id]/edit/page.tsxfrontend/app/[locale]/shop/admin/products/_components/product-form.tsxfrontend/app/api/shop/admin/products/[id]/route.ts
✅ Files skipped from review due to trivial changes (1)
- .hintrc
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (2)
frontend/db/schema/shop.ts (2)
products(33-76)productPrices(198-239)frontend/lib/shop/currency.ts (2)
CurrencyCode(2-2)currencyValues(1-1)
🔇 Additional comments (6)
frontend/app/[locale]/shop/admin/products/_components/product-form.tsx (2)
183-224: Solid validation flow for price submission.The validation properly ensures:
- USD price is required
- Original price requires a corresponding price
- Conversion errors are caught and displayed
The error handling is user-friendly with specific messages.
326-403: Well-structured multi-currency pricing UI.The two-column layout with clear labels (USD required, UAH optional) and helpful guidance text provides a good admin experience. The form correctly uses
requiredonly on the USD price field.frontend/app/api/shop/admin/products/[id]/route.ts (2)
40-78: Comprehensive price schema handling both formats.The
adminPriceRowSchemaunion correctly handles both the canonical minor-unit format and the legacy major-unit format, with proper transformation. This provides good backward compatibility.
96-97: Appropriate use ofgetAdminProductByIdWithPrices.The GET endpoint correctly uses the new function to include pricing data in the response.
frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx (2)
49-69: Good fallback handling for legacy products.The logic correctly handles products that don't yet have entries in
productPricesby falling back to the product's legacypricefield with USD currency. This ensures backward compatibility during migration.
71-92: Clean component rendering with proper initial values.The
ProductFormreceives well-structured initial values including the newpricesarray. All fields are properly mapped with appropriate defaults.
| function parseMajorToMinor(value: string | number): number { | ||
| const s = String(value).trim().replace(',', '.'); | ||
| // допускаємо "10", "10.5", "10.50" | ||
| if (!/^\d+(\.\d{1,2})?$/.test(s)) { | ||
| throw new Error(`Invalid money value: "${value}"`); | ||
| } | ||
| const [whole, frac = ''] = s.split('.'); | ||
| const frac2 = (frac + '00').slice(0, 2); | ||
| return Number(whole) * 100 + Number(frac2); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fourth duplicate of parseMajorToMinor.
This function now exists in four locations. This is a clear candidate for extraction to @/lib/shop/currency.ts.
🔎 Proposed centralization
Create or update frontend/lib/shop/currency.ts:
export function parseMajorToMinor(value: string | number): number {
const s = String(value).trim().replace(',', '.');
if (!/^\d+(\.\d{1,2})?$/.test(s)) {
throw new Error(`Invalid money value: "${value}"`);
}
const [whole, frac = ''] = s.split('.');
const frac2 = (frac + '00').slice(0, 2);
return Number(whole) * 100 + Number(frac2);
}
export function formatMinorToMajor(value: number): string {
if (!Number.isFinite(value)) return '';
const abs = Math.abs(Math.trunc(value));
const whole = Math.floor(abs / 100);
const frac = abs % 100;
const sign = value < 0 ? '-' : '';
return `${sign}${whole}.${String(frac).padStart(2, '0')}`;
}Then import from all files that need it.
🤖 Prompt for AI Agents
In frontend/app/[locale]/shop/admin/products/[id]/edit/page.tsx around lines 12
to 21, you’ve duplicated the parseMajorToMinor implementation (this is the
fourth copy); extract this logic into a single module
frontend/lib/shop/currency.ts exporting parseMajorToMinor (and the suggested
formatMinorToMajor helper), then replace the local function with an import from
that module; update all other files that currently contain the duplicate to
import from frontend/lib/shop/currency.ts and remove the duplicated function
definitions so the codebase uses the centralized implementation.
| const base = { ...(parsed.data as any) }; | ||
| delete base.prices; | ||
|
|
||
| const updated = await updateProduct(parsedParams.data.id, { | ||
| ...parsed.data, | ||
| ...base, | ||
| ...(pricesOverride ? { prices: pricesOverride } : {}), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the parseAdminProductForm function and check its return type
rg "parseAdminProductForm" -A 20 --type ts --type tsxRepository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Find the parseAdminProductForm function
rg "parseAdminProductForm" -A 20 --type tsRepository: DevLoversTeam/devlovers.net
Length of output: 9827
🏁 Script executed:
#!/bin/bash
# Find the productAdminSchema and productAdminUpdateSchema definitions
rg "productAdminSchema|productAdminUpdateSchema" -B 2 -A 10 --type ts | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 6732
Remove the any cast and rely on proper typing.
The parsed.data has a fully known type from the productAdminUpdateSchema which includes a prices property (optional). The cast to any defeats TypeScript's type safety and hides potential issues if the schema changes. Use type narrowing or direct property access instead:
const base = { ...parsed.data };
delete base.prices;🤖 Prompt for AI Agents
In frontend/app/api/shop/admin/products/[id]/route.ts around lines 193 to 198,
remove the unsafe any cast when spreading parsed.data; instead rely on the known
productAdminUpdateSchema type by directly spreading parsed.data into base (e.g.,
const base = { ...parsed.data }) and then delete base.prices; this preserves
type safety and avoids masking schema changes while keeping the existing logic
that conditionally overrides prices with pricesOverride.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/app/api/shop/admin/products/[id]/route.ts (1)
195-200: Remove theanycast and rely on proper typing.This was flagged in a previous review. The cast to
anydefeats TypeScript's type safety. Consider using a more type-safe approach:-const base = { ...(parsed.data as any) }; -delete base.prices; +const { prices: _ignoredPrices, ...base } = parsed.data;This destructures
pricesout (discarding it) and spreads the rest intobase, preserving type information.
🧹 Nitpick comments (2)
frontend/app/api/shop/admin/products/[id]/route.ts (2)
22-80: Consider importing shared schemas from@/lib/validation/shop.These schemas largely duplicate
adminPriceRowSchemaandadminPricesSchemaalready defined infrontend/lib/validation/shop.ts. Maintaining two copies risks drift and inconsistent validation.Additionally, line 46 uses
nonnegative()(allows 0), while the canonical schema inshop.tsusesmoneyMinorPositive(requires > 0). This discrepancy could allow zero-priced products through this admin route when the shared schema would reject them.🔎 Suggested approach
Import and reuse the shared schemas:
import { adminPriceRowSchema, adminPricesSchema, } from '@/lib/validation/shop';If the route needs slightly different behavior (e.g., allowing zero prices for promotions), document that intent explicitly rather than silently diverging.
152-189: HOTFIX comment indicates tech debt — consider consolidating price parsing.The comment acknowledges bypassing
parseAdminProductFormfor price validation. This results in prices being parsed twice (once inparseAdminProductForm, once here), adding complexity and maintenance burden.For now the approach works, but consider either:
- Removing price parsing from
parseAdminProductFormentirely and handling it here, or- Trusting
parseAdminProductFormand removing this HOTFIX blockThis would eliminate the duplication and reduce the chance of the two paths drifting.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app/api/shop/admin/products/[id]/route.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/api/shop/admin/products/[id]/route.ts (3)
frontend/lib/validation/shop.ts (1)
adminPriceRowSchema(173-198)frontend/lib/services/products.ts (2)
getAdminProductByIdWithPrices(569-581)updateProduct(351-514)frontend/lib/admin/parseAdminProductForm.ts (1)
parseAdminProductForm(196-272)
🔇 Additional comments (2)
frontend/app/api/shop/admin/products/[id]/route.ts (2)
14-18: LGTM!Import change to
getAdminProductByIdWithPricescorrectly aligns with the new multi-currency pricing model.
82-122: LGTM!The GET handler correctly uses the new
getAdminProductByIdWithPricesfunction and maintains comprehensive error handling for auth and not-found scenarios.
Description
This PR completes the P0 currency D1 rollout for the shop: currency is now server-authoritative based on locale (
uk → UAH, otherwiseUSD), product pricing is stored per-currency inproduct_prices, all order/cart calculations are performed in integer minor units, and shop UI money display is standardized via a single Intl-based formatter. This eliminates mixed-currency edge cases, float/decimal rounding risks in services, and$hardcoding across shop screens.Related Issue
Issue: #<issue_number>
Changes
resolveCurrencyFromLocale(and request boundary helper) so server determines currency from locale/Accept-Language.product_pricesstorage with(product_id, currency)uniqueness and canonical*_minorfields.orders.total_amount_minor,order_items.unit_price_minor,order_items.line_total_minor) alongside legacy decimal mirrors where required.product_pricesby resolved currency.PRICE_CONFIG_ERROR).formatMoney(minor, currency, locale)helper (Intl).$hardcoding and ad-hoc formatting in shop UI (Catalog/Product/Cart/Admin).formatMoney(fromDbMoney(...), currency, locale)), with locale-aware dates.Database Changes (if applicable)
How Has This Been Tested?
Automated tests
frontend/lib/tests/currency.test.ts(locale/header → currency)frontend/lib/tests/checkout-currency-policy.test.ts:uk→order.currency=UAH, totals correcten→order.currency=USD, totals correctPRICE_CONFIG_ERROR)frontend/lib/tests/format-money.test.ts(Intl formatting:$for USD/en,₴for UAH/uk)Manual smoke tests (PowerShell)
POST /api/shop/cart/rehydratewithAccept-Language: en→currency=USD, correct minor totalsPOST /api/shop/cart/rehydratewithAccept-Language: uk→currency=UAH, correct minor totalsRepo checks
$hardcodes or ad-hoc Intl/currency formatters (non-shop components excluded by scope).Screenshots (if applicable)
envsukverified via PowerShell (attach terminal screenshots if needed).Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.