Fix API Error before M5

README.md

@@ -269,13 +269,13 @@ the variant model under real concurrency.
 
 Everything below was run, not assumed:
 
-- `pnpm lint` · `pnpm typecheck` · `pnpm test` · `pnpm build` — 26/26 Turborepo tasks pass;
+- `pnpm lint` · `pnpm typecheck` · `pnpm test` · `pnpm build` — 27/27 Turborepo tasks pass;
   `pnpm format:check` clean
-- 5 migrations, 32 tables; seed loads 36 permissions, 6 roles, 3 brands, 8 categories,
+- 6 migrations, 32 tables; seed loads 36 permissions, 6 roles, 3 brands, 8 categories,
   3 collections, 12 products, **155 variants**, 64 uploaded images and 3 dev accounts
-- **44 tests** — RBAC guards, password hashing, translation fallback, `Accept-Language`, the
+- **48 tests** — RBAC guards, password hashing, translation fallback, `Accept-Language`, the
-  variant matrix planner, the HTTP client's fetch receiver and retry recursion, and the inventory
+  variant matrix planner, the HTTP client's fetch receiver and retry recursion, the inventory
-  list's variant-driven projection
+  list's variant-driven projection, and the two admin-schema defects that caused silent data loss
 - Catalog: listings with filters/facets/cursor paging, PDP, navigation — correctly localised in
   both `vi` and `en`; money formats per locale from one integer (`690.000 ₫` / `₫690,000`)
 - Storefront: every route 200 in both locales; `/en/products/<vi-slug>` → 307 →
@@ -303,6 +303,13 @@ through Chrome with the console and network panel open:
   media library upload driven from the browser (presign → PUT to MinIO → register, 400×500 PNG
   landed at 10,962 bytes with a date-partitioned UUID key); inventory adjustment from the table
   wrote a ledger entry and the storefront went `OUT_OF_STOCK` → `IN_STOCK` on the next request
+- **Catalog write correctness**, each reproduced before the fix and re-run after: a product
+  authored in one language is accepted; renaming a product no longer clears its gender/sport
+  targeting, collections or attributes; adding a size inherits the sibling price and the stored
+  SKU prefix (`VEL-NOC-BLACK-S` at 1.590.000 ₫, not `AO-GIO-…-S` at 0 ₫); a sale price above the
+  regular price is rejected; two products with the same name get distinct per-locale slugs; a
+  retired colourway leaves both the filter and the facet count; 404s no longer write junk keys
+  into Redis
 - **Listing controls (M4):** sort menu changes the order and the URL together
   (`?sort=price_asc`), and is shareable; "load more" appends the next page in place without
   touching the address bar, updates "showing N of M", and disappears when the set is exhausted;

apps/admin/src/features/products/product-form.tsx

@@ -146,8 +146,17 @@ export function ProductForm({
       translations: filled,
       genderTargets: genders,
       sportTypes: sports,
+      // SKU prefix and base price are create-only fields — the inputs are not
+      // even rendered when editing. Sending them anyway would post
+      // `basePriceAmount: 0`, and the API cannot distinguish "the operator
+      // wants zero" from "the form always sends this", so every variant added
+      // by a later edit would be created free.
+      ...(product
+        ? {}
+        : {
             ...(skuPrefix.trim() ? { skuPrefix: skuPrefix.trim() } : {}),
             basePriceAmount: Number.parseInt(basePrice, 10) || 0,
+          }),
       options: options.map((option, index) => ({
         key: option.key,
         position: index,

apps/api/prisma/migrations/20260812081721_add_product_sku_prefix/migration.sql

@@ -0,0 +1,2 @@
+-- AlterTable
+ALTER TABLE "products" ADD COLUMN     "sku_prefix" VARCHAR(24);

apps/api/prisma/schema.prisma

@@ -476,6 +476,14 @@ model Product {
   metaDescription String? @map("meta_description")
   metadata        Json?
 
+  /// Seeds generated variant SKUs, e.g. "VEL-NOC" -> "VEL-NOC-BLACK-M".
+  ///
+  /// Stored rather than re-derived, because adding a colourway a year later
+  /// must extend the same SKU family. Falling back to the slug produced
+  /// "AO-GIO-CHAY-DEM-NOCTURNE-BLACK-XL" sitting next to "VEL-NOC-BLACK-M" on
+  /// the same product.
+  skuPrefix String? @map("sku_prefix") @db.VarChar(24)
+
   /// ---- Read-model projection, derived from this product's variants --------
   ///
   /// Denormalised deliberately. Sorting a listing by price and rendering a

apps/api/src/infrastructure/redis/redis.service.ts

@@ -79,11 +79,17 @@ export class RedisService implements OnModuleDestroy {
 
     const value = await factory();
 
+    // A null is never served back — the read above treats it as a miss — so
+    // writing it only fills Redis with entries that can never hit. A flood of
+    // requests for slugs that do not exist was enough to grow memory
+    // indefinitely for no benefit.
+    if (value !== null && value !== undefined) {
       try {
         await this.set(key, value, ttlSeconds);
       } catch (error) {
         this.logger.warn(`Cache write failed for ${key}: ${messageOf(error)}`);
       }
+    }
 
     return value;
   }

apps/api/src/modules/products/products-admin.service.ts

@@ -139,6 +139,8 @@ export class ProductsAdminService {
           primaryCategoryId: input.primaryCategoryId ?? null,
           genderTargets: input.genderTargets,
           sportTypes: input.sportTypes,
+          // Remembered so a colourway added later joins the same SKU family.
+          skuPrefix: input.skuPrefix?.trim() || null,
         },
         select: { id: true },
       });
@@ -169,7 +171,7 @@ export class ProductsAdminService {
   ): Promise<AdminProductDetail> {
     const existing = await this.prisma.product.findFirst({
       where: { id, deletedAt: null },
-      select: { id: true, slug: true, status: true },
+      select: { id: true, slug: true, status: true, skuPrefix: true },
     });
     if (!existing) throw AppException.notFound('Product');
 
@@ -192,28 +194,35 @@ export class ProductsAdminService {
           ...(input.primaryCategoryId === undefined
             ? {}
             : { primaryCategoryId: input.primaryCategoryId ?? null }),
-          ...(input.genderTargets ? { genderTargets: input.genderTargets } : {}),
+          // `!== undefined`, never truthiness: an explicit `[]` means "clear
-          ...(input.sportTypes ? { sportTypes: input.sportTypes } : {}),
+          // this", an absent key means "leave it alone". Conflating the two is
+          // how a rename used to wipe a product's targeting.
+          ...(input.genderTargets === undefined ? {} : { genderTargets: input.genderTargets }),
+          ...(input.sportTypes === undefined ? {} : { sportTypes: input.sportTypes }),
+          ...(input.skuPrefix === undefined ? {} : { skuPrefix: input.skuPrefix.trim() || null }),
           ...(input.status ? this.statusPatch(input.status) : {}),
         },
       });
 
-      if (input.translations) {
+      if (input.translations !== undefined) {
         await this.writeTranslations(tx, id, input as CreateProductInput, existing.slug);
       }
-      if (input.collectionIds) {
+      if (input.collectionIds !== undefined) {
         await this.writeCollections(tx, id, input.collectionIds);
       }
-      if (input.attributes) {
+      if (input.attributes !== undefined) {
         await this.writeAttributes(tx, id, input.attributes);
       }
-      if (input.options) {
+      if (input.options !== undefined) {
         const matrix = await this.writeOptions(tx, id, input.options);
         await this.syncVariants(
           tx,
           id,
-          input.skuPrefix ?? existing.slug,
+          // The stored prefix, not the slug. The editor only sends a prefix on
-          input.basePriceAmount ?? 0,
+          // create, so falling back to the slug renamed every SKU generated by
+          // a later edit.
+          input.skuPrefix?.trim() || existing.skuPrefix || existing.slug,
+          input.basePriceAmount ?? (await this.inheritedBasePrice(tx, id)),
           matrix,
         );
       }
@@ -246,6 +255,8 @@ export class ProductsAdminService {
       throw AppException.badRequest('One or more variants do not belong to this product.');
     }
 
+    await this.assertSalePricesBelowRegular(productId, input);
+
     await this.prisma.$transaction(
       input.variants.map((variant) =>
         this.prisma.productVariant.update({
@@ -330,6 +341,28 @@ export class ProductsAdminService {
 
   // ---- internals -----------------------------------------------------------
 
+  /**
+   * What a newly generated variant should cost when the caller did not say.
+   *
+   * Adding a size to a live product used to create it at 0, because the update
+   * path defaulted `basePriceAmount` to zero — the product then advertised
+   * "from 0 ₫" on every listing until somebody noticed. The cheapest existing
+   * variant is the only defensible guess, and it is never zero for a product
+   * that was already priced.
+   */
+  private async inheritedBasePrice(
+    tx: Prisma.TransactionClient,
+    productId: string,
+  ): Promise<number> {
+    const cheapest = await tx.productVariant.findFirst({
+      where: { productId, status: 'ACTIVE', deletedAt: null },
+      orderBy: { priceAmount: 'asc' },
+      select: { priceAmount: true },
+    });
+
+    return cheapest?.priceAmount ?? 0;
+  }
+
   private statusPatch(status: 'DRAFT' | 'ACTIVE' | 'ARCHIVED') {
     return {
       status,
@@ -367,6 +400,41 @@ export class ProductsAdminService {
     throw AppException.conflict('Could not generate a unique slug for this product.');
   }
 
+  /**
+   * A sale price must be below the price it discounts.
+   *
+   * Checked here rather than in the schema because a patch may carry only one
+   * of the two: setting `salePriceAmount` alone has to be compared against the
+   * variant's *stored* price. Without this, a typo propagated straight into
+   * `Product.maxPriceAmount` and the storefront showed a "sale" dearer than the
+   * original, struck through the wrong way round.
+   */
+  private async assertSalePricesBelowRegular(
+    productId: string,
+    input: BulkUpdateVariantsInput,
+  ): Promise<void> {
+    const stored = await this.prisma.productVariant.findMany({
+      where: { productId },
+      select: { id: true, sku: true, priceAmount: true, salePriceAmount: true },
+    });
+    const byId = new Map(stored.map((variant) => [variant.id, variant]));
+
+    for (const patch of input.variants) {
+      const current = byId.get(patch.id);
+      if (!current) continue;
+
+      const price = patch.priceAmount ?? current.priceAmount;
+      const sale =
+        patch.salePriceAmount === undefined ? current.salePriceAmount : patch.salePriceAmount;
+
+      if (sale !== null && sale !== undefined && sale >= price) {
+        throw AppException.badRequest(
+          `Sale price for ${current.sku} must be below its price of ${price}.`,
+        );
+      }
+    }
+  }
+
   private async writeTranslations(
     tx: Prisma.TransactionClient,
     productId: string,
@@ -377,7 +445,15 @@ export class ProductsAdminService {
       const fields = input.translations[locale];
       if (!fields) continue;
 
-      const slug = fields.slug ?? slugify(fields.name) ?? fallbackSlug;
+      // `product_translations` is UNIQUE(locale, slug) and these slugs are the
+      // real URLs. Two products named the same used to fail the whole
+      // transaction with a bare "That value is already taken".
+      const slug = await this.uniqueTranslationSlug(
+        tx,
+        toDbLocale(locale),
+        fields.slug ?? slugify(fields.name) ?? fallbackSlug,
+        productId,
+      );
 
       await tx.productTranslation.upsert({
         where: { productId_locale: { productId, locale: toDbLocale(locale) } },
@@ -403,6 +479,28 @@ export class ProductsAdminService {
     }
   }
 
+  /** Appends `-2`, `-3`… until the slug is free within this locale. */
+  private async uniqueTranslationSlug(
+    tx: Prisma.TransactionClient,
+    locale: 'VI' | 'EN',
+    base: string,
+    productId: string,
+  ): Promise<string> {
+    const candidate = base || 'product';
+
+    for (let suffix = 0; suffix < 50; suffix += 1) {
+      const slug = suffix === 0 ? candidate : `${candidate}-${suffix + 1}`;
+      const clash = await tx.productTranslation.findFirst({
+        where: { locale, slug, productId: { not: productId } },
+        select: { productId: true },
+      });
+
+      if (!clash) return slug;
+    }
+
+    throw AppException.conflict(`Could not generate a unique ${locale} slug from "${base}".`);
+  }
+
   private async writeCollections(
     tx: Prisma.TransactionClient,
     productId: string,

apps/api/src/modules/products/products.mapper.ts

@@ -294,8 +294,18 @@ export class ProductsMapper {
     }
 
     const effective = variants.map((variant) => variant.salePriceAmount ?? variant.priceAmount);
+
+    // A variant can be discounted two ways, and both need something to strike
+    // through: `compareAtAmount` is an explicit "was" price, and a
+    // `salePriceAmount` is a markdown off the variant's own price. Reading only
+    // the first left every sale set from the admin's variant grid rendering as
+    // a lone red number with no original beside it.
     const compareAt = variants
-      .map((variant) => variant.compareAtAmount)
+      .map(
+        (variant) =>
+          variant.compareAtAmount ??
+          (variant.salePriceAmount !== null ? variant.priceAmount : null),
+      )
       .filter((amount): amount is number => amount !== null);
 
     return {

apps/api/src/modules/products/products.repository.ts

@@ -273,10 +273,24 @@ export class ProductsRepository {
     return { AND: and };
   }
 
+  /**
+   * "Offers this value on something you can actually buy."
+   *
+   * Matched through the variants, not through `Product.options`. Option values
+   * are retained when archived variants still reference them (ADR-0016), so the
+   * option set includes colourways that are no longer sold — filtering on it
+   * returned products with no red variant for `?colors=red`.
+   */
   private optionValueFilter(key: string, values: readonly string[]): Prisma.ProductWhereInput {
     return {
-      options: {
+      variants: {
-        some: { key, values: { some: { value: { in: [...values] } } } },
+        some: {
+          status: 'ACTIVE',
+          deletedAt: null,
+          optionValues: {
+            some: { optionValue: { value: { in: [...values] }, option: { key } } },
+          },
+        },
       },
     };
   }
@@ -528,7 +542,12 @@ export class ProductsRepository {
   optionValueFacet(where: Prisma.ProductWhereInput, key: string) {
     return this.prisma.productOptionValue.groupBy({
       by: ['value'],
-      where: { option: { key, product: where } },
+      where: {
+        option: { key, product: where },
+        // Same reason as `optionValueFilter`: without this a retired colourway
+        // keeps its count in the rail, and clicking it returns nothing.
+        variantLinks: { some: { variant: { status: 'ACTIVE', deletedAt: null } } },
+      },
       _count: { _all: true },
       orderBy: { _count: { value: 'desc' } },
       take: 50,
@@ -543,7 +562,11 @@ export class ProductsRepository {
     locale: Locale,
   ) {
     return this.prisma.productOptionValue.findMany({
-      where: { option: { key, product: where }, value: { in: [...values] } },
+      where: {
+        option: { key, product: where },
+        value: { in: [...values] },
+        variantLinks: { some: { variant: { status: 'ACTIVE', deletedAt: null } } },
+      },
       distinct: ['value'],
       select: {
         value: true,

docs/architecture.md

@@ -490,7 +490,25 @@ sent an English shopper following "load more" to the Vietnamese listing — invi
 default locale, which is exactly why it survived.
 
 So: **exercise a flow on data you just created, not only on seeded data, perform each action
-twice, and do it in a non-default locale.** Seed data has been through every code path already; new data has been through none. The
+twice, and do it in a non-default locale.**
+
+The sweep before M5 added one more, and it is the sharpest of the set. Two of the worst defects
+in the catalog write path were not logic errors at all — they were **library behaviour that had
+drifted out from under the intent written in the comments**:
+
+- `z.record(z.enum(LOCALES), …)` is exhaustive in Zod 4. The helper was named
+  `requiredTranslations`, its comment said "at least the default locale", and its `.refine`
+  checked exactly that — and was unreachable. Every single-language product was rejected.
+- `.partial()` makes a field optional but keeps its `.default([])`. So an omitted
+  `genderTargets` arrived as `[]`, and `[]` is truthy. Renaming a product cleared its targeting,
+  dropped every collection link and deleted all of its attributes.
+
+Both typechecked. Both read correctly. Neither had a test, because the behaviour they depended on
+was assumed rather than asserted. `packages/validation/src/catalog-admin.spec.ts` now pins both.
+
+So: **when a schema encodes an intent, assert the intent — not the shape.** "Absent means leave
+alone" and "one language is enough" are claims about behaviour, and a type signature cannot make
+either of them true. Seed data has been through every code path already; new data has been through none. The
 inventory case is pinned in `apps/api/src/modules/inventory/inventory-list.spec.ts`.
 
 ---

packages/validation/package.json

@@ -19,7 +19,8 @@
     "dev": "tsc -p tsconfig.json --watch --preserveWatchOutput",
     "clean": "rm -rf dist .turbo *.tsbuildinfo",
     "lint": "eslint src",
-    "typecheck": "tsc -p tsconfig.json --noEmit"
+    "typecheck": "tsc -p tsconfig.json --noEmit",
+    "test": "jest --passWithNoTests"
   },
   "dependencies": {
     "@sport/types": "workspace:*",
@@ -28,7 +29,18 @@
   "devDependencies": {
     "@sport/config": "workspace:*",
     "@sport/eslint-config": "workspace:*",
+    "@types/jest": "^30.0.0",
     "eslint": "catalog:",
+    "jest": "^30.4.2",
+    "ts-jest": "^29.4.12",
     "typescript": "catalog:"
+  },
+  "jest": {
+    "preset": "ts-jest",
+    "testEnvironment": "node",
+    "roots": [
+      "<rootDir>/src"
+    ],
+    "testRegex": ".*\\.spec\\.ts$"
   }
 }

packages/validation/src/catalog-admin.spec.ts

@@ -0,0 +1,49 @@
+import { createProductSchema, updateProductSchema } from './catalog-admin';
+
+/**
+ * These two schemas each caused a data-loss bug, and neither was visible from
+ * reading the code — both came from a library behaviour that changed underneath
+ * the intent expressed in the comments.
+ */
+describe('createProductSchema translations', () => {
+  it('accepts a product authored in one language only', () => {
+    // `z.record` keyed by an enum is exhaustive in Zod 4, which made this fail
+    // and left the "at least one language" refine unreachable. The editor
+    // advertises single-language authoring, so this is the supported path.
+    const result = createProductSchema.safeParse({
+      translations: { vi: { name: 'Áo Chạy Bộ' } },
+      basePriceAmount: 100_000,
+    });
+
+    expect(result.success).toBe(true);
+  });
+
+  it('still rejects a product with no language at all', () => {
+    const result = createProductSchema.safeParse({ translations: {}, basePriceAmount: 0 });
+    expect(result.success).toBe(false);
+  });
+});
+
+describe('updateProductSchema', () => {
+  it('leaves omitted collections absent rather than defaulting them to empty', () => {
+    // `.partial()` keeps `.default([])`, so an omitted key arrived as `[]` —
+    // and `[]` is truthy, so the service wrote it. Renaming a product cleared
+    // its gender and sport targeting, dropped every collection link and deleted
+    // all of its attributes.
+    const parsed = updateProductSchema.parse({
+      translations: { vi: { name: 'Chỉ Đổi Tên' } },
+    });
+
+    expect(parsed.genderTargets).toBeUndefined();
+    expect(parsed.sportTypes).toBeUndefined();
+    expect(parsed.collectionIds).toBeUndefined();
+    expect(parsed.attributes).toBeUndefined();
+    expect(parsed.options).toBeUndefined();
+    expect(parsed.basePriceAmount).toBeUndefined();
+  });
+
+  it('still applies an explicit empty array, which means "clear this"', () => {
+    const parsed = updateProductSchema.parse({ genderTargets: [] });
+    expect(parsed.genderTargets).toEqual([]);
+  });
+});

packages/validation/src/catalog-admin.ts

@@ -16,10 +16,19 @@ import { offsetPageQuerySchema } from './pagination';
 
 const localeKeySchema = z.enum(LOCALES);
 
-/** At least the default locale must be present, or the product has no name. */
+/**
-function requiredTranslations<T extends z.ZodTypeAny>(fields: T) {
+ * At least one locale must be present; the rest are optional.
+ *
+ * `z.partialRecord`, NOT `z.record`. In Zod 4 a record keyed by an enum is
+ * *exhaustive* — `z.record(localeKeySchema, …)` demands every locale and makes
+ * the refine below unreachable. That silently rejected any product authored in
+ * one language, which is precisely the workflow the editor advertises ("a
+ * language with no name is skipped"). The field-level fallback in the database
+ * exists so partial translations are normal, not exceptional.
+ */
+function partialTranslations<T extends z.ZodTypeAny>(fields: T) {
   return z
-    .record(localeKeySchema, fields)
+    .partialRecord(localeKeySchema, fields)
     .refine((value) => Object.keys(value).length > 0, 'Provide content for at least one language');
 }
 
@@ -47,7 +56,7 @@ export const productOptionValueInputSchema = z.object({
     .string()
     .regex(/^#[0-9a-fA-F]{6}$/, 'Use a hex colour like #1a2b3c')
     .nullish(),
-  labels: requiredTranslations(z.string().trim().min(1).max(80)),
+  labels: partialTranslations(z.string().trim().min(1).max(80)),
 });
 
 export const productOptionInputSchema = z.object({
@@ -59,14 +68,14 @@ export const productOptionInputSchema = z.object({
     .max(40)
     .regex(/^[a-z][a-z0-9_]*$/, 'Use lowercase letters, digits and underscores'),
   position: z.coerce.number().int().min(0).default(0),
-  names: requiredTranslations(z.string().trim().min(1).max(60)),
+  names: partialTranslations(z.string().trim().min(1).max(60)),
   values: z.array(productOptionValueInputSchema).min(1, 'An option needs at least one value'),
 });
 
 export const productAttributeInputSchema = z.object({
   key: z.string().trim().min(1).max(60),
   position: z.coerce.number().int().min(0).default(0),
-  translations: requiredTranslations(
+  translations: partialTranslations(
     z.object({
       label: z.string().trim().min(1).max(120),
       value: z.string().trim().min(1).max(500),
@@ -75,7 +84,7 @@ export const productAttributeInputSchema = z.object({
 });
 
 export const createProductSchema = z.object({
-  translations: requiredTranslations(productTranslationSchema),
+  translations: partialTranslations(productTranslationSchema),
   brandId: anyIdSchema.nullish(),
   primaryCategoryId: anyIdSchema.nullish(),
   genderTargets: z.array(genderTargetSchema).default([]),
@@ -89,7 +98,26 @@ export const createProductSchema = z.object({
   basePriceAmount: z.int().min(0).default(0),
 });
 
-export const updateProductSchema = createProductSchema.partial().extend({
+/**
+ * Written out rather than derived with `.partial()`.
+ *
+ * `.partial()` makes a field optional but leaves its `.default([])` in place,
+ * so an omitted `genderTargets` arrived as `[]` — and `[]` is truthy, so the
+ * service wrote it. Renaming a product silently cleared its gender and sport
+ * targeting, dropped it from every collection and deleted all of its
+ * attributes. Absent must mean "leave alone"; only an explicit `[]` clears.
+ */
+export const updateProductSchema = z.object({
+  translations: partialTranslations(productTranslationSchema).optional(),
+  brandId: anyIdSchema.nullish(),
+  primaryCategoryId: anyIdSchema.nullish(),
+  genderTargets: z.array(genderTargetSchema).optional(),
+  sportTypes: z.array(sportTypeSchema).optional(),
+  collectionIds: z.array(anyIdSchema).optional(),
+  skuPrefix: z.string().trim().max(24).optional(),
+  options: z.array(productOptionInputSchema).optional(),
+  attributes: z.array(productAttributeInputSchema).optional(),
+  basePriceAmount: z.int().min(0).optional(),
   status: z.enum(['DRAFT', 'ACTIVE', 'ARCHIVED']).optional(),
 });
 

pnpm-lock.yaml

@@ -476,9 +476,18 @@ importers:
       '@sport/eslint-config':
         specifier: workspace:*
         version: link:../eslint-config
+      '@types/jest':
+        specifier: ^30.0.0
+        version: 30.0.0
       eslint:
         specifier: 'catalog:'
         version: 9.39.5(jiti@2.7.0)
+      jest:
+        specifier: ^30.4.2
+        version: 30.4.2(@types/node@22.20.1)
+      ts-jest:
+        specifier: ^29.4.12
+        version: 29.4.12(@babel/core@7.29.7)(@jest/transform@30.4.1)(@jest/types@30.4.1)(babel-jest@30.4.1(@babel/core@7.29.7))(jest-util@30.4.1)(jest@30.4.2(@types/node@22.20.1))(typescript@5.9.3)
       typescript:
         specifier: 'catalog:'
         version: 5.9.3