From 80d04d2701f16f7844456dad677754fa95299415 Mon Sep 17 00:00:00 2001 From: Vedanshu Date: Sun, 9 Aug 2026 12:05:30 +0000 Subject: [PATCH 1/2] [Domain] Find an order by the SKU it was bought under MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OrderListFilter.search` gains a third arm: an EXACT, case-folded match on a SKU frozen onto an order's lines at purchase time, ORed with the existing order-id prefix and buyer_ref substring. One search box, one `search` param, no wire or schema change. Exact rather than prefix/substring because a sku is an identifier an operator pastes whole, and because exactness is already the house rule for skus: the products list matches an exact-lower sku beside its substring title, and the orders customer key keeps exact-lower buyer_ref for the same identity reason. A substring would pull a whole variant family into a search for one member. The sku read is the order's own line snapshot, never the live catalogue, so a later product rename leaves old orders findable under the sku they were bought as and moves none of them onto the new one. Reached with a correlated EXISTS and never a join: order_items is 1:N, and a join would return a two-line order twice, inflate the limit + 1 next-page probe and make countOrders — which shares the predicate — over-count the page it captions. Contract cases land on the fake, SQLite and Postgres alike: exactness, the fold, the frozen snapshot, a duplicate-sku order returning once across a page boundary, and count/list agreement. The cost was measured rather than assumed and the port records what EXPLAIN actually says — Postgres de-correlates the EXISTS into a hashed subplan, one sequential pass over order_items that every search pays, not the per-row index probe the shape suggests. --- .changeset/orders-search-by-snapshot-sku.md | 59 ++++++++ packages/domain/src/ports/order-store.ts | 85 ++++++++++-- .../src/testing/in-memory-order-store.ts | 23 +-- .../src/testing/order-store-contract.ts | 117 +++++++++++++++- .../service/test/admin-orders-http.test.ts | 131 ++++++++++++++++++ .../store-postgres/src/kysely-order-store.ts | 45 ++++-- 6 files changed, 428 insertions(+), 32 deletions(-) create mode 100644 .changeset/orders-search-by-snapshot-sku.md diff --git a/.changeset/orders-search-by-snapshot-sku.md b/.changeset/orders-search-by-snapshot-sku.md new file mode 100644 index 0000000..ef10c11 --- /dev/null +++ b/.changeset/orders-search-by-snapshot-sku.md @@ -0,0 +1,59 @@ +--- +"@otta-sh/domain": minor +"@otta-sh/store-postgres": minor +"@otta-sh/service": patch +--- + +Orders search gains a third axis: the SKU frozen onto an order's lines at purchase time. +`OrderListFilter.search` now matches an order-id PREFIX **or** a `buyer_ref` SUBSTRING **or** an +EXACT (case-folded) line sku, ORed. Nothing that matched before stops matching — the two existing +halves are untouched — and the wire is unchanged: one search box, one `search` param, one more +thing it can find. + +`@otta-sh/service` is bumped because its `GET /admin/orders` answers differently for the same +query, though no service source changed — only its test coverage. `@otta-sh/plugin` is NOT bumped: +it forwards `search` verbatim and has no code, wire or copy change here. + +- **The purchase-time snapshot, not the live catalogue.** The sku compared is the one on the + order's own lines — the insert-once snapshot the detail screen renders. Renaming a product's sku + therefore leaves every earlier order findable under the sku it was bought as, and moves none of + them onto the new one. A sku that exists only in the catalogue, on nothing anybody ordered, + matches no order at all. Both directions are pinned in the contract, and the second again over + the wire. +- **Exact, not a prefix and not a substring.** A sku is an identifier an operator pastes whole off + a packing slip or a ticket, and exactness is the settled house rule for skus everywhere else: + the products list already matches an exact-lower sku beside its substring title, and the orders + `customer` key keeps exact-lower `buyer_ref` for the same identity reason. A substring would + drag a whole variant family (`TEE-BLK-S`, `TEE-BLK-M`, …) into a search for one of its members, + which is a different question from the one that was asked. The fold is `lower()` on both sides, + as on the other two halves, and carries the same accepted non-ASCII caveat: SQLite's built-in + `lower()` folds ASCII only where JS `toLowerCase()` is Unicode-aware. Skus are ASCII in + practice, which is why this is accepted rather than solved. No escaping is involved on this + half — an equality has no pattern language, so a sku spelled `50%_OFF` is compared character for + character. +- **`EXISTS`, never a join — this is the correctness point, not a style note.** The list is one + row per order and `order_items` is 1:N. Reaching the lines with a join would return an order + once per matching line: a two-line order would appear twice in the page, the `limit + 1` + next-page probe would count a duplicate as a row, the page would silently shrink, and + `countOrders` — which shares the predicate — would over-count the very page it captions. Every + adapter expresses the half as a correlated existence test and the fake as `lines.some(...)`; a + contract case seeds an order whose lines BOTH match and pins that it comes back once, across a + page boundary and in the count. +- **One more scan, and it is paid by every search.** Planning was measured rather than assumed, + because the intuitive reading is wrong: Postgres does not run the correlated `EXISTS` per + candidate order — it de-correlates it into a hashed subplan, one pass over `order_items` + filtered on `lower(sku)`, hashed by `order_id` and probed in memory. `lower(sku)` has no index, + so that pass is a sequential scan of the line table, and it happens whatever the operator typed: + an id search pays for it too, twice per page (list plus count). `EXPLAIN` on a synthetic + 5k-order / 10k-line set read 5.9–8.1 ms for a searched page and 5.9 ms for its count, against + 4.3 ms for the same id-prefix search with the arm stripped out. Forcing the intuitive plan + instead (`enable_seqscan = off`, which does turn the probe into a per-row index scan on + `idx_order_items_order_product`) was slower — 21 ms over 5000 loops — so that index is not what + keeps this cheap; the hash is. A functional index on `lower(order_items.sku)` is the obvious + lever if the shape stops holding, and is deliberately not pulled now, on the same reasoning that + declined a trigram index for the substring half: measure the real statement first. +- **The cursor gate is unaffected.** It compares the search STRING, not what the string selects, + so the canonical form on the wire is identical before and after; a sku search pages and re-pages + exactly like the other two, and a differently spelled one still fails closed. + +No wire, schema or migration change, and no console copy change. diff --git a/packages/domain/src/ports/order-store.ts b/packages/domain/src/ports/order-store.ts index d7c9be6..cfe8abd 100644 --- a/packages/domain/src/ports/order-store.ts +++ b/packages/domain/src/ports/order-store.ts @@ -286,6 +286,13 @@ export interface OrderStore { * adapter fetches `limit + 1` rows to decide whether a next page exists and * emits `nextCursor` from the LAST RETURNED row (null when the page is the last). * + * ONE ROW PER ORDER, whatever the filter reaches. The only table this may join + * is the 1:1 `order_totals`; any 1:N table a filter has to consult — today + * `order_items`, for `search`'s line-sku half — is reached by an EXISTENCE test + * and never by a join, or an order with two matching lines is returned twice, + * the `limit + 1` next-page detection counts duplicates as rows, and the page + * silently shrinks. `countOrders` shares the predicate and so shares the rule. + * * The date window is HALF-OPEN `[from, to)` — `from` inclusive, `to` * EXCLUSIVE. This deliberately DIFFERS from `ReportingStore`'s inclusive/ * inclusive `BETWEEN` window (MOD-7): the list is a browsing surface where an @@ -515,7 +522,8 @@ export type CreateOrderResult = { created: boolean; order: Order }; /** Filters for the admin Orders list. All optional — an empty filter lists every * order newest-first. `states` is an OR set (`state IN (...)`); `from`/`to` are a * HALF-OPEN `[from, to)` window on `created_at`; `search` matches an order-id - * PREFIX or a `buyer_ref` SUBSTRING — see the field below. */ + * PREFIX, a `buyer_ref` SUBSTRING or an EXACT purchase-time line sku — see the + * field below. */ export interface OrderListFilter { states?: readonly OrderState[]; /** Inclusive lower bound (ISO-8601 UTC). */ @@ -523,11 +531,13 @@ export interface OrderListFilter { /** EXCLUSIVE upper bound (ISO-8601 UTC) — half-open window (MOD-7). */ to?: string; /** - * The operator's free-text lookup: an order-id PREFIX **or** a `buyer_ref` - * SUBSTRING, ORed, with `lower()` applied to BOTH sides of both halves — - * `lower(id) LIKE lower(:s || '%')` OR `lower(buyer_ref) LIKE lower('%' || :s - * || '%')`. The fake, SQLite and Postgres implement exactly this, case for - * case, and the contract suite pins every one of them on all three. + * The operator's free-text lookup: an order-id PREFIX, **or** a `buyer_ref` + * SUBSTRING, **or** an EXACT purchase-time line sku, ORed, with `lower()` + * applied to BOTH sides of all three arms — `lower(id) LIKE lower(:s || '%')` + * OR `lower(buyer_ref) LIKE lower('%' || :s || '%')` OR `EXISTS (SELECT 1 FROM + * order_items WHERE order_id = orders.id AND lower(sku) = lower(:s))`. The + * fake, SQLite and Postgres implement exactly this, case for case, and the + * contract suite pins every one of them on all three. * * WHY A PREFIX ON THE ID. The console never renders a full uuid — it renders * the shortest unique prefix (the git-style short id in @@ -542,6 +552,28 @@ export interface OrderListFilter { * operator arrives with a fragment — a local part, a domain, whatever the * customer wrote in a ticket — not the address exactly as stored. * + * WHY THE SKU HALF READS THE ORDER'S OWN LINES, AND IS EXACT. The sku matched + * is the one FROZEN onto the order's lines at purchase time — the same + * insert-once snapshot the detail screen renders — never the live catalogue's + * current sku for that product. Renaming a product's sku therefore leaves + * every earlier order findable under the sku it was bought as, and moves none + * of them to the new one; that is the point of the snapshot, and the contract + * pins it. The half is EXACT (folded, but no prefix, no substring) because a + * sku is an IDENTIFIER an operator pastes whole off a packing slip or a + * support ticket, and because exactness is the settled house rule for skus: + * `ProductListFilter.search` matches an exact-lower sku beside its substring + * title, and the `customer` key below keeps exact-lower `buyer_ref` for the + * same identity reason. A substring here would drag every variant of a family + * (`TEE-BLK-S`, `TEE-BLK-M`, …) into a search for one of them, which is a + * different question from the one the operator asked. + * + * WHY `EXISTS`, NEVER A JOIN. `order_items` is 1:N; the list's contract is one + * row per order (`listOrders` doc). An order carrying two matching lines must + * appear ONCE — a join would return it twice, inflate the `limit + 1` + * next-page probe, and make `countOrders` (which shares this predicate) + * over-count the page it captions. Every adapter therefore expresses this half + * as a correlated existence test, and the fake as `lines.some(...)`. + * * WHY THE FOLD IS EXPLICIT ON BOTH SIDES. A bare `LIKE` is case-SENSITIVE on * Postgres and ASCII-case-INSENSITIVE on SQLite; only an explicit `lower()` * on both operands makes the two dialects and the fake agree. The pattern @@ -554,18 +586,26 @@ export interface OrderListFilter { * Emails and hex ids are ASCII, which is why this is accepted rather than * solved. Ids are lowercase hex (`crypto.randomUUID()`), so folding the id is * a no-op on the STORED side — it is there to forgive the TYPED side, e.g. a - * uuid pasted back from a client that upper-cased it. + * uuid pasted back from a client that upper-cased it. The sku half folds the + * same way and inherits the same caveat: it spells `lower(sku) = lower(:s)` + * (SQL folding both operands) rather than the products list's `lower(sku) = + * :sJsLowered` — identical for the ASCII skus a catalogue actually carries, + * and one fewer place the two sides can drift apart within a dialect. * * WILDCARDS ARE LITERAL. `%`, `_` and `\` (the escape character itself) are * `LIKE` metacharacters; a search containing them matches them as characters * (the adapters escape the pattern and pass `ESCAPE '\'`; the fake builds no - * pattern at all, so `startsWith`/`includes` are literal by construction). + * pattern at all, so `startsWith`/`includes` are literal by construction). The + * sku half needs no escaping at all — an equality has no pattern language, so + * a sku spelled `50%_OFF` is compared character for character. * * THE EMPTY STRING MATCHES EVERYTHING, because every string starts with `""` * and contains `""`. That is the widest filter this axis has, not the - * narrowest — the inverted reading of "search for nothing". The service's - * query schema requires `min(1)`, so the wire cannot send it; the boundary is - * pinned in the contract for every other caller. + * narrowest — the inverted reading of "search for nothing". (The sku half does + * not widen it further and does not narrow it: `""` equals no real sku, and + * the id arm has already matched every row.) The service's query schema + * requires `min(1)`, so the wire cannot send it; the boundary is pinned in the + * contract for every other caller. * * THE SEQUENTIAL SCAN IS THE DESIGN, not an oversight. An unanchored * substring cannot be served by a b-tree, so `idx_orders_buyer_ref_lower` @@ -590,6 +630,27 @@ export interface OrderListFilter { * statement then. The index still backs every EQUALITY path on `buyer_ref` — * `linkGuestOrders` and the `customer` key below — which is exactly why those * keep exact-lower-equals semantics and did NOT follow this widening. + * + * THE SKU ARM COSTS ONE MORE SCAN, of `order_items` — not a per-row probe, and + * not a free ride on an index. Planning it was measured rather than assumed, + * because the intuitive reading is wrong: Postgres does NOT run the correlated + * `EXISTS` once per candidate order. It DE-CORRELATES it into a hashed + * subplan — one pass over `order_items` filtered on `lower(sku)`, hashed by + * `order_id`, then probed in memory per row. `lower(sku)` has no index, so + * that one pass is a sequential scan of the line table. `EXPLAIN` on a + * synthetic 5k-order / 10k-line set (ANALYZEd, local pg 16) read: a sku search + * 5.9–8.1 ms for the page and 5.9 ms for its count, against 4.3 ms for the + * SAME id-prefix search with this arm stripped out — so the arm cost ~1.4 ms + * there, and it costs it on EVERY search, including ones that are plainly an + * id or an email, twice per page (list + count). Forcing the intuitive plan + * instead (`enable_seqscan = off`, which does make the probe an index scan on + * `idx_order_items_order_product` per candidate row) was SLOWER — 21 ms, 5000 + * loops — so that index is not what keeps this cheap; the hash is. Read those + * as a shape, not a budget, for the same reasons the figures above are a + * floor. The obvious lever, if the shape stops holding, is a functional index + * on `lower(order_items.sku)`, which turns that one scan into an index scan; + * deliberately not pulled now, on the same reasoning that declined the trigram + * index — measure the real statement first. */ search?: string; /** The customer dimension (admin-UX Increment 1) — see `OrderCustomerKey`. @@ -613,7 +674,7 @@ export interface OrderListFilter { * fuzzy lookup: a substring would fold two customers into one person's history, * and equality is what keeps `idx_orders_buyer_ref_lower` on the plan. It exists * as its own key — distinct from `search` — for that reason, and because - * `search` ALSO matches an order-id prefix. + * `search` ALSO matches an order-id prefix and a purchase-time line sku. * At least one half should be set; an empty key matches nothing it constrains * (adapters ignore a key with neither half). */ diff --git a/packages/domain/src/testing/in-memory-order-store.ts b/packages/domain/src/testing/in-memory-order-store.ts index d812830..e0dbacf 100644 --- a/packages/domain/src/testing/in-memory-order-store.ts +++ b/packages/domain/src/testing/in-memory-order-store.ts @@ -607,16 +607,22 @@ export class InMemoryOrderStore implements OrderStore { if (filter.from !== undefined && o.createdAt < filter.from) return false; // inclusive lower if (filter.to !== undefined && o.createdAt >= filter.to) return false; // EXCLUSIVE upper if (filter.search !== undefined) { - // Order-id PREFIX or buyer_ref SUBSTRING, both sides folded (port doc). - // The fake's stand-in for the adapter's `lower(col) LIKE lower(:pattern) - // ESCAPE '\'`: `startsWith`/`includes` build no pattern, so `%` and `_` - // are already ordinary characters here — exactly what the SQL side buys - // by escaping them. The fold is explicit on BOTH sides, matching the SQL - // (whose bare-LIKE case behaviour differs between pg and SQLite). + // Order-id PREFIX, buyer_ref SUBSTRING or an EXACT line sku, all sides + // folded (port doc). The fake's stand-in for the adapter's `lower(col) + // LIKE lower(:pattern) ESCAPE '\'`: `startsWith`/`includes` build no + // pattern, so `%` and `_` are already ordinary characters here — exactly + // what the SQL side buys by escaping them (the sku half is an equality, + // so it is literal in both by construction). The fold is explicit on BOTH + // sides, matching the SQL (whose bare-LIKE case behaviour differs between + // pg and SQLite). const needle = filter.search.toLowerCase(); const byId = o.id.toLowerCase().startsWith(needle); const byRef = o.buyerRef.toLowerCase().includes(needle); - if (!byId && !byRef) return false; + // `some` over the order's OWN line snapshots — the fake's stand-in for the + // adapter's correlated EXISTS over `order_items`, and an existence test + // for the same reason: an order whose lines match twice is still ONE row. + const bySku = o.lines.some((l) => l.sku.toLowerCase() === needle); + if (!byId && !byRef && !bySku) return false; } // The customer dimension: a UNION inside the key (customer_id = :id OR // lower(buyer_ref) = lower(:buyerRef)), ANDed with everything above. A key @@ -644,7 +650,8 @@ export class InMemoryOrderStore implements OrderStore { // EXACT parity with `KyselyOrderStore.listOrders` (MOD-5): same filters // (via the shared `#matchesFilter` predicate), same `created_at DESC, id // DESC` order, same half-open `[from, to)` window, same folded id-PREFIX / - // buyer_ref-SUBSTRING `search`, same `limit + 1` next-page detection. + // buyer_ref-SUBSTRING / exact-line-sku `search`, same `limit + 1` next-page + // detection. const cursor = page.cursor ?? null; const matched = [...this.#orders.values()] diff --git a/packages/domain/src/testing/order-store-contract.ts b/packages/domain/src/testing/order-store-contract.ts index b299fb1..960cdf2 100644 --- a/packages/domain/src/testing/order-store-contract.ts +++ b/packages/domain/src/testing/order-store-contract.ts @@ -58,6 +58,40 @@ function physicalInput(overrides: Partial = {}): CreateOrderIn }; } +/** + * Seed an order that carries REAL line snapshots, through the port's own + * `createFromCart` — the harness's `seedOrder` writes a bare order + totals row + * with NO lines, and the line-sku search half reads the line snapshots. Every + * harness runs a clock fixed to the same instant, so these orders share a + * `created_at` and the list's tie-break (`id DESC`) is what orders them. + */ +async function seedLinedOrder( + store: OrderStore, + input: { id: string; skus: readonly string[]; productIdValue?: string; buyerRef?: string }, +): Promise { + const unit = 500; + const lines: CreateOrderInput["lines"] = input.skus.map((s, i) => ({ + productId: productId(input.productIdValue ?? `p-${input.id}-${String(i)}`), + sku: sku(s), + title: "Widget", + unitPrice: cents(unit), + currency: USD, + quantity: 1, + fulfillmentKind: "physical", + reservationId: reservationId(`res-${input.id}-${String(i)}`), + })); + const total = cents(unit * input.skus.length); + await store.createFromCart( + physicalInput({ + orderId: orderId(input.id), + idempotencyKey: idempotencyKey(`key-${input.id}`), + ...(input.buyerRef !== undefined ? { buyerRef: input.buyerRef } : {}), + lines, + totals: { subtotal: total, total, currency: USD }, + }), + ); +} + /** * The reusable `OrderStore` behavioral spec (§7): create + snapshot, get, * idempotent replay, insert-once snapshot immutability, and legal / illegal @@ -519,17 +553,98 @@ export function orderStoreContract( expect(orders.map((o) => o.id)).toEqual(unfiltered.orders.map((o) => o.id)); }); + test("listOrders search matches a purchase-time LINE SKU, folded but EXACT", async () => { + const h = await makeHarness(); + await seedLinedOrder(h.store, { id: "ord-alpha", skus: ["SKU-ALPHA"] }); + await seedLinedOrder(h.store, { id: "ord-beta", skus: ["SKU-BETA"] }); + // The sku an operator pastes off a packing slip finds the order that + // bought it — read off the ORDER's own line snapshot, not the catalogue. + const exact = await h.store.listOrders({ search: "SKU-ALPHA" }, { limit: 25 }); + expect(exact.orders.map((o) => o.id)).toEqual(["ord-alpha"]); + // Folded on both sides, like every other half of this predicate. + const folded = await h.store.listOrders({ search: "sku-alpha" }, { limit: 25 }); + expect(folded.orders.map((o) => o.id)).toEqual(["ord-alpha"]); + // EXACT, unlike the buyer_ref half: a sku is an identifier the operator + // pastes whole, so neither a PREFIX nor a mid-string fragment is a match. + // (Both would otherwise hit here — `SKU-` is a prefix of both seeded skus.) + expect((await h.store.listOrders({ search: "SKU-" }, { limit: 25 })).orders).toHaveLength(0); + expect((await h.store.listOrders({ search: "ALPHA" }, { limit: 25 })).orders).toHaveLength(0); + // An order with no lines at all is simply not matched by this half. + await h.seedOrder(summaryRow({ id: "ord-lineless", buyerRef: "z@x.test" })); + const still = await h.store.listOrders({ search: "SKU-ALPHA" }, { limit: 25 }); + expect(still.orders.map((o) => o.id)).toEqual(["ord-alpha"]); + expect(await h.store.countOrders({ search: "SKU-ALPHA" })).toBe(1); + }); + + test("listOrders returns a MULTI-LINE order matching on sku exactly ONCE", async () => { + const h = await makeHarness(); + // Two lines of the SAME sku on one order (a split shipment, a re-add), plus + // a third line that does not match. The line half must be an EXISTENCE + // test over the lines, never a join onto them: a join would emit this + // order once PER matching line, double it in the page, and make the + // `limit + 1` next-page detection — and the count that captions it — lie. + await seedLinedOrder(h.store, { id: "ord-dup", skus: ["SKU-DUP", "SKU-DUP", "SKU-OTHER"] }); + const one = await h.store.listOrders({ search: "SKU-DUP" }, { limit: 25 }); + expect(one.orders.map((o) => o.id)).toEqual(["ord-dup"]); + expect(await h.store.countOrders({ search: "SKU-DUP" })).toBe(1); + + // And the page size stays honest across a boundary: two such orders at + // `limit: 1` are two pages of one row, not one page that repeats a row. + await seedLinedOrder(h.store, { id: "ord-dup2", skus: ["SKU-DUP", "SKU-DUP"] }); + const page1 = await h.store.listOrders({ search: "SKU-DUP" }, { limit: 1 }); + expect(page1.orders.map((o) => o.id)).toEqual(["ord-dup2"]); // same clock ⇒ id DESC + expect(page1.nextCursor).not.toBeNull(); + const page2 = await h.store.listOrders( + { search: "SKU-DUP" }, + { limit: 1, cursor: page1.nextCursor }, + ); + expect(page2.orders.map((o) => o.id)).toEqual(["ord-dup"]); + expect(page2.nextCursor).toBeNull(); + expect(await h.store.countOrders({ search: "SKU-DUP" })).toBe(2); + }); + + test("listOrders search reads the FROZEN sku — a later rename never moves an old order", async () => { + const h = await makeHarness(); + // One product, sold under one sku and later renamed to another: the two + // orders differ only in the sku frozen onto their lines at purchase time. + await seedLinedOrder(h.store, { + id: "ord-before", + skus: ["sku-old"], + productIdValue: "p-renamed", + }); + await seedLinedOrder(h.store, { + id: "ord-after", + skus: ["sku-new"], + productIdValue: "p-renamed", + }); + // The old order answers to the sku it was BOUGHT under, forever… + const old = await h.store.listOrders({ search: "sku-old" }, { limit: 25 }); + expect(old.orders.map((o) => o.id)).toEqual(["ord-before"]); + // …and never migrates to the new one, which finds only what shipped as it. + const renamed = await h.store.listOrders({ search: "sku-new" }, { limit: 25 }); + expect(renamed.orders.map((o) => o.id)).toEqual(["ord-after"]); + }); + test("countOrders counts under the SAME search predicate as listOrders", async () => { const h = await makeHarness(); await h.seedOrder(summaryRow({ id: "ord-a", buyerRef: "amy@example.com" })); await h.seedOrder(summaryRow({ id: "ord-b", buyerRef: "bea@example.com" })); await h.seedOrder(summaryRow({ id: "zzz-c", buyerRef: "cal@other.test" })); - // The id half (prefix) and the buyer_ref half (substring) both count. + await seedLinedOrder(h.store, { + id: "yyy-d", + skus: ["SKU-COUNTED"], + buyerRef: "dee@lined.test", + }); + // The id half (prefix), the buyer_ref half (substring) and the line-sku + // half (exact) all count, under the one shared predicate. expect(await h.store.countOrders({ search: "ord-" })).toBe(2); expect(await h.store.countOrders({ search: "example.com" })).toBe(2); expect(await h.store.countOrders({ search: "other.test" })).toBe(1); + expect(await h.store.countOrders({ search: "SKU-COUNTED" })).toBe(1); const { orders } = await h.store.listOrders({ search: "ord-" }, { limit: 25 }); expect(orders).toHaveLength(await h.store.countOrders({ search: "ord-" })); + const lined = await h.store.listOrders({ search: "SKU-COUNTED" }, { limit: 25 }); + expect(lined.orders).toHaveLength(await h.store.countOrders({ search: "SKU-COUNTED" })); }); test("listOrders paginates forward with a keyset cursor — no overlap, no gap", async () => { diff --git a/packages/service/test/admin-orders-http.test.ts b/packages/service/test/admin-orders-http.test.ts index d0c521b..dc3f608 100644 --- a/packages/service/test/admin-orders-http.test.ts +++ b/packages/service/test/admin-orders-http.test.ts @@ -155,6 +155,105 @@ describe.skipIf(PG === undefined)("admin Orders console HTTP contract", () => { expect(wildcard.total).toBe(0); }); + /** Check an order out through the real cart → checkout path, so it carries + * REAL purchase-time line snapshots (`seedOrder` writes a bare order + totals + * row with no lines, and the sku half of `search` reads the lines). */ + async function checkoutOrder(input: { + key: string; + buyerRef: string; + items: ReadonlyArray<{ productId: string; sku: string }>; + }): Promise { + for (const item of input.items) { + await server.seedProduct({ + productId: item.productId, + sku: item.sku, + priceCents: 500, + title: "Item", + kind: "physical", + onHand: 5, + }); + } + const cart = await json( + await fetch(`${server.baseUrl}/carts`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ currency: "USD" }), + }), + ); + const cartId = cart["cartId"] as string; + for (const item of input.items) { + const addRes = await fetch(`${server.baseUrl}/carts/${cartId}/lines`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "Idempotency-Key": `add-${input.key}-${item.sku}`, + }, + body: JSON.stringify({ sku: item.sku, qty: 1, productId: item.productId }), + }); + expect(addRes.status).toBe(200); + } + const coRes = await fetch(`${server.baseUrl}/checkout/orders`, { + method: "POST", + headers: { "Content-Type": "application/json", "Idempotency-Key": `co-${input.key}` }, + body: JSON.stringify({ cartId, paymentMethod: "stripe", buyerRef: input.buyerRef }), + }); + expect(coRes.status).toBe(201); + const order = (await json(coRes))["order"] as Record; + return order["id"] as string; + } + + test("search passes the port's purchase-time SKU semantics through the wire", async () => { + await seed(); // lineless distractors, none of which any sku may drag in + const twoLine = await checkoutOrder({ + key: "sku-two", + buyerRef: "erin@example.com", + items: [ + { productId: "p-alpha", sku: "SKU-ALPHA" }, + { productId: "p-beta", sku: "SKU-BETA" }, + ], + }); + const otherLine = await checkoutOrder({ + key: "sku-one", + buyerRef: "frank@example.com", + items: [{ productId: "p-gamma", sku: "SKU-GAMMA" }], + }); + + // The sku frozen on a line finds the order that bought it, folded… + const alpha = await json(await get("/orders?search=SKU-ALPHA")); + expect((alpha.orders as Array<{ id: string }>).map((o) => o.id)).toEqual([twoLine]); + expect(alpha.total).toBe(1); + const folded = await json(await get("/orders?search=sku-alpha")); + expect((folded.orders as Array<{ id: string }>).map((o) => o.id)).toEqual([twoLine]); + + // …and a two-line order is ONE row, whichever of its lines matched. + const beta = await json(await get("/orders?search=SKU-BETA")); + expect((beta.orders as Array<{ id: string }>).map((o) => o.id)).toEqual([twoLine]); + expect(beta.total).toBe(1); + const gamma = await json(await get("/orders?search=SKU-GAMMA")); + expect((gamma.orders as Array<{ id: string }>).map((o) => o.id)).toEqual([otherLine]); + + // EXACT on the wire too: neither a prefix nor a fragment of a sku matches + // (both would hit here — `SKU-` leads all three). + const prefix = await json(await get("/orders?search=SKU-")); + expect(prefix.orders).toEqual([]); + expect(prefix.total).toBe(0); + const fragment = await json(await get("/orders?search=ALPHA")); + expect(fragment.orders).toEqual([]); + + // The LIVE CATALOGUE is not what is searched: a product nobody ordered + // matches no order, however real its sku is. + await server.seedProductRow({ + id: "p-unsold", + sku: "SKU-UNSOLD", + title: "Unsold", + priceCents: 900, + createdAt: "2026-07-10T00:00:00.000Z", + }); + const unsold = await json(await get("/orders?search=SKU-UNSOLD")); + expect(unsold.orders).toEqual([]); + expect(unsold.total).toBe(0); + }); + test("the cursor gate compares the search STRING, not its semantics", async () => { await seed(); // A search that now matches four rows still mints a cursor whose filter is @@ -179,6 +278,38 @@ describe.skipIf(PG === undefined)("admin Orders console HTTP contract", () => { expect((await get(`/orders?cursor=${cursor}&search=ORD-`)).status).toBe(400); }); + test("a SKU search pages like any other — the gate still compares the raw string", async () => { + // Two orders of the same item: the sku half has to compose with the keyset + // WHERE across a page boundary, and its spelling has to survive the cursor + // the same way the other two halves do. + const first = await checkoutOrder({ + key: "sku-page-1", + buyerRef: "gia@example.com", + items: [{ productId: "p-paged", sku: "SKU-PAGED" }], + }); + const second = await checkoutOrder({ + key: "sku-page-2", + buyerRef: "hal@example.com", + items: [{ productId: "p-paged", sku: "SKU-PAGED" }], + }); + const page1 = await json(await get("/orders?search=SKU-PAGED&limit=1")); + expect((page1.orders as unknown[]).length).toBe(1); + expect(page1.total).toBe(2); // the SET, counted under the same predicate + const cursor = encodeURIComponent(page1.nextCursor as string); + const page2 = await json(await get(`/orders?cursor=${cursor}&search=SKU-PAGED`)); + expect((page2.orders as unknown[]).length).toBe(1); + expect(page2.total).toBe(2); + expect(page2.nextCursor).toBeNull(); + // Union is both orders, once each — no overlap, no gap, no duplicate row. + const paged = [ + ...(page1.orders as Array<{ id: string }>), + ...(page2.orders as Array<{ id: string }>), + ].map((o) => o.id); + expect(paged.toSorted()).toEqual([first, second].toSorted()); + // A different spelling of the same search is still a different filter. + expect((await get(`/orders?cursor=${cursor}&search=SKU-PAGE`)).status).toBe(400); + }); + test("keyset cursor round-trips and preserves the filter across pages (no overlap/gap)", async () => { await seed(); const page1 = await json(await get("/orders?states=paid&limit=2")); diff --git a/packages/store-postgres/src/kysely-order-store.ts b/packages/store-postgres/src/kysely-order-store.ts index 153664e..241627d 100644 --- a/packages/store-postgres/src/kysely-order-store.ts +++ b/packages/store-postgres/src/kysely-order-store.ts @@ -1110,8 +1110,9 @@ export class KyselyOrderStore implements OrderStore { * is a UNION inside the key (`customer_id = :id OR lower(buyer_ref) = * lower(:buyerRef)`) — lazy linking means one person's orders split across the * two columns; a key with neither half set constrains nothing. `search` is the - * operator's fuzzy lookup (id prefix OR buyer_ref substring) and is deliberately - * NOT the same predicate as the customer key's exact `buyerRef`. + * operator's fuzzy lookup (id prefix OR buyer_ref substring OR an exact line + * sku, the last as an EXISTS over `order_items` — never a join) and is + * deliberately NOT the same predicate as the customer key's exact `buyerRef`. */ function orderFilterConditions(filter: OrderListFilter): Expression[] { const eb: ExpressionBuilder = expressionBuilder(); @@ -1122,24 +1123,46 @@ function orderFilterConditions(filter: OrderListFilter): Expression[] { if (filter.from !== undefined) conds.push(eb("orders.created_at", ">=", filter.from)); // inclusive if (filter.to !== undefined) conds.push(eb("orders.created_at", "<", filter.to)); // EXCLUSIVE (half-open, MOD-7) if (filter.search !== undefined) { - // An order-id PREFIX or a buyer_ref SUBSTRING, both folded on BOTH sides - // (port doc). `lower(:pattern)` rather than a JS `.toLowerCase()` so ONE - // function folds both operands — within a dialect the two sides are then - // folded identically by construction. The explicit fold is also what makes - // the dialects agree at all: a bare LIKE is case-sensitive on pg and - // ASCII-case-insensitive on SQLite. `ESCAPE '\'` over an escaped pattern - // keeps a `%`/`_` in the operator's search a literal character. + // An order-id PREFIX, a buyer_ref SUBSTRING, or an EXACT purchase-time line + // sku — all folded on BOTH sides (port doc). `lower(:pattern)` rather than a + // JS `.toLowerCase()` so ONE function folds both operands — within a dialect + // the two sides are then folded identically by construction. The explicit + // fold is also what makes the dialects agree at all: a bare LIKE is + // case-sensitive on pg and ASCII-case-insensitive on SQLite. `ESCAPE '\'` + // over an escaped pattern keeps a `%`/`_` in the operator's search a literal + // character; the sku half is an equality, so it needs no pattern and is + // literal by construction. + // + // The sku half is a CORRELATED `EXISTS`, never a join onto `order_items` + // (port doc — the named hazard). `listOrders` selects one row per order via + // a 1:1 `order_totals` join; joining a 1:N table would emit an order once + // PER matching line, so a two-line order would appear twice, the `limit + 1` + // next-page detection would count duplicates as rows, and `countOrders` + // would over-count the very page it captions. `EXISTS` asks "does this order + // have such a line?" and stops at the first — one row per order, always. // // This is a SEQUENTIAL SCAN and that is the design (port doc): the // unanchored buyer_ref half cannot use `idx_orders_buyer_ref_lower`, and // the anchored id half cannot use the primary key under a default - // collation. Both equality paths that DO use those indices — - // `linkGuestOrders` and the `customer` key below — are untouched. + // collation. The sku arm adds ONE more scan, of `order_items`: pg + // de-correlates this EXISTS into a hashed subplan (one pass over the line + // table filtered on `lower(sku)`, hashed by order_id, probed in memory) — + // NOT a per-row index probe, which EXPLAIN shows to be the slower plan + // here. It is paid by every search, not only a sku-shaped one. Both + // equality paths that DO use the buyer_ref index — `linkGuestOrders` and + // the `customer` key below — are untouched. const escaped = escapeLikePattern(filter.search); conds.push( eb.or([ sql`lower(orders.id) like lower(${`${escaped}%`}) escape '\\'`, sql`lower(orders.buyer_ref) like lower(${`%${escaped}%`}) escape '\\'`, + eb.exists( + eb + .selectFrom("order_items") + .select("order_items.id") + .whereRef("order_items.order_id", "=", "orders.id") + .where(sql`lower(order_items.sku) = lower(${filter.search})`), + ), ]), ); } From 3c744568d7e8bb9cc27e8c48266c20c9b5a6e523 Mon Sep 17 00:00:00 2001 From: Vedanshu Date: Sun, 9 Aug 2026 12:45:38 +0000 Subject: [PATCH 2/2] [Domain] Say SKU in the search box, and say how each dialect plans it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Orders search box now reads "Search order ID, buyer email, or exact SKU". An axis the label does not name ships dark — nobody types into a box for a thing they have no reason to think it reads. It spends one mode word, on the one axis whose mode changes what to type: a partial id or email still finds the order, a partial SKU finds nothing. Same principle as the products list's "Search (SKU exact, or title contains)", and both labels are now pinned side by side, with a mounted check that the sentence reaches the control itself. The plan note was Postgres-only but stated as if universal. It now says both: pg de-correlates the EXISTS into a hashed subplan (one extra sequential pass over order_items, paid by every search and by both statements a page issues), while SQLite keeps it correlated, probes the (order_id, product_id) index per row, and skips the arm entirely for a row the two cheaper arms — written first, deliberately — already matched. Every figure is re-measured off the statement's own Execution Time, including the count baselines that were previously inferred rather than captured. Three cross-references said the two searches diverge on sku; they now agree on it, differing only in which table they read it from. The coupon and index notes enumerate all three arms. Two contract cases close the gaps: a sku spelled with LIKE metacharacters matches itself and nothing else, and one search string that reaches one order by id prefix and another by line SKU returns each exactly once. --- .changeset/orders-search-by-snapshot-sku.md | 48 +++++++---- .../admin-presentation/src/orders-copy.ts | 14 ++- .../test/presentation.test.ts | 22 +++++ .../test/orders-search-label-dom.test.tsx | 86 +++++++++++++++++++ packages/domain/src/ports/coupon-store.ts | 9 +- packages/domain/src/ports/order-store.ts | 57 +++++++----- .../src/ports/product-commerce-store.ts | 23 +++-- .../src/testing/order-store-contract.ts | 42 +++++++++ .../store-postgres/src/kysely-order-store.ts | 18 ++-- .../src/kysely-product-commerce-store.ts | 11 ++- .../migrations/0022_order_lookup_indices.ts | 7 +- 11 files changed, 271 insertions(+), 66 deletions(-) create mode 100644 packages/admin-react/test/orders-search-label-dom.test.tsx diff --git a/.changeset/orders-search-by-snapshot-sku.md b/.changeset/orders-search-by-snapshot-sku.md index ef10c11..10d9831 100644 --- a/.changeset/orders-search-by-snapshot-sku.md +++ b/.changeset/orders-search-by-snapshot-sku.md @@ -1,6 +1,8 @@ --- "@otta-sh/domain": minor "@otta-sh/store-postgres": minor +"@otta-sh/admin-presentation": patch +"@otta-sh/admin-react": patch "@otta-sh/service": patch --- @@ -10,9 +12,18 @@ EXACT (case-folded) line sku, ORed. Nothing that matched before stops matching halves are untouched — and the wire is unchanged: one search box, one `search` param, one more thing it can find. +The Orders list's search box now says so: **Search order ID, buyer email, or exact SKU**. A search +axis the label does not name ships dark — nobody types into a box for a thing they have no reason +to think it reads — so the label change is part of the feature, not a follow-up. It spends exactly +one mode word, on the one axis whose mode changes what to type: a partial id or email still finds +the order, a partial SKU finds nothing. That is the same principle behind the products list's +`Search (SKU exact, or title contains)`, and both labels are now pinned side by side, plus a +mounted check that the sentence actually reaches the control an operator types into. + `@otta-sh/service` is bumped because its `GET /admin/orders` answers differently for the same -query, though no service source changed — only its test coverage. `@otta-sh/plugin` is NOT bumped: -it forwards `search` verbatim and has no code, wire or copy change here. +query, though no service source changed — only its test coverage. `@otta-sh/admin-presentation` +and `@otta-sh/admin-react` are bumped for the label. `@otta-sh/plugin` is NOT bumped: it forwards +`search` verbatim, and the Orders list it renders is the React one. - **The purchase-time snapshot, not the live catalogue.** The sku compared is the one on the order's own lines — the insert-once snapshot the detail screen renders. Renaming a product's sku @@ -39,21 +50,26 @@ it forwards `search` verbatim and has no code, wire or copy change here. adapter expresses the half as a correlated existence test and the fake as `lines.some(...)`; a contract case seeds an order whose lines BOTH match and pins that it comes back once, across a page boundary and in the count. -- **One more scan, and it is paid by every search.** Planning was measured rather than assumed, - because the intuitive reading is wrong: Postgres does not run the correlated `EXISTS` per - candidate order — it de-correlates it into a hashed subplan, one pass over `order_items` - filtered on `lower(sku)`, hashed by `order_id` and probed in memory. `lower(sku)` has no index, - so that pass is a sequential scan of the line table, and it happens whatever the operator typed: - an id search pays for it too, twice per page (list plus count). `EXPLAIN` on a synthetic - 5k-order / 10k-line set read 5.9–8.1 ms for a searched page and 5.9 ms for its count, against - 4.3 ms for the same id-prefix search with the arm stripped out. Forcing the intuitive plan - instead (`enable_seqscan = off`, which does turn the probe into a per-row index scan on - `idx_order_items_order_product`) was slower — 21 ms over 5000 loops — so that index is not what - keeps this cheap; the hash is. A functional index on `lower(order_items.sku)` is the obvious - lever if the shape stops holding, and is deliberately not pulled now, on the same reasoning that - declined a trigram index for the substring half: measure the real statement first. +- **The two dialects plan it oppositely, and both shapes were measured rather than assumed.** On + Postgres the intuitive reading is simply wrong: it does not run the correlated `EXISTS` per + candidate order, it de-correlates it into a hashed subplan — one pass over `order_items` filtered + on `lower(sku)`, hashed by `order_id` and probed in memory. `lower(sku)` has no index, so that + pass is a sequential scan of the line table, and it happens whatever the operator typed: an id + search pays for it too, and so does each of the two statements a searched page issues. Measured + (statement `Execution Time`, synthetic 5k-order / 10k-line set): a SKU search 6.3 ms for the page + and 5.9 ms for its count, against 2.8 ms and 2.8 ms with the arm stripped out; an id-prefix + search 5.4 ms and 5.6 ms against 4.2 ms and 2.7 ms. Forcing the intuitive plan instead + (`enable_seqscan = off`, which does turn the probe into a per-row index scan on + `idx_order_items_order_product` over 5000 loops) was slower — 21–24 ms across runs — so that + index is not what keeps this cheap on Postgres; the hash is. SQLite does the opposite and keeps + the subquery correlated, serving it as a per-row index probe on that same index, and short- + circuits the arm entirely for a row the two cheaper arms already matched (4.4 ms for an id page + against 5.2 ms for a SKU page there). A functional index on `lower(order_items.sku)` is the + obvious lever if the Postgres shape stops holding, and is deliberately not pulled now, on the + same reasoning that declined a trigram index for the substring half: measure the real statement + first. - **The cursor gate is unaffected.** It compares the search STRING, not what the string selects, so the canonical form on the wire is identical before and after; a sku search pages and re-pages exactly like the other two, and a differently spelled one still fails closed. -No wire, schema or migration change, and no console copy change. +No wire, schema or migration change. diff --git a/packages/admin-presentation/src/orders-copy.ts b/packages/admin-presentation/src/orders-copy.ts index 13eb912..dd17f62 100644 --- a/packages/admin-presentation/src/orders-copy.ts +++ b/packages/admin-presentation/src/orders-copy.ts @@ -314,9 +314,17 @@ export function refundsGroupLabel(refunded: string, ceiling: string): string { /** The back control, on the detail and on its failure state. */ export const ORDERS_BACK_LABEL = "← Back to orders"; -/** The list's free-text filter. It names BOTH things it searches, because an - * operator who thinks it is id-only will not paste an email into it. */ -export const ORDERS_SEARCH_LABEL = "Search order ID or buyer email"; +/** The list's free-text filter. It names ALL THREE things it searches, because + * an operator who thinks it is id-only will not paste an email into it — and + * one that reaches a purchased SKU without saying so is a feature that ships + * dark. It also names ONE match mode, following `products-copy.ts`'s + * `Search (SKU exact, or title contains)`: a mode is worth a word exactly when + * it changes what the operator should type. The id and the email FORGIVE a + * fragment (a prefix and a substring), so a partial attempt teaches itself; a + * SKU is matched whole, so a pasted fragment returns nothing and reads as + * "SKU search is broken". `exact` is the word that prevents that, and it is the + * only mode word the label spends. */ +export const ORDERS_SEARCH_LABEL = "Search order ID, buyer email, or exact SKU"; /** The fulfilment form. `Ship date (optional, UTC)` states the zone in the * LABEL because the control is a bare `` that shows none — diff --git a/packages/admin-presentation/test/presentation.test.ts b/packages/admin-presentation/test/presentation.test.ts index b176235..64a381b 100644 --- a/packages/admin-presentation/test/presentation.test.ts +++ b/packages/admin-presentation/test/presentation.test.ts @@ -35,10 +35,12 @@ import { ORDERS_PAGE_FAILED_TITLE, ORDERS_NOUN, ORDERS_NO_MATCH, + ORDERS_SEARCH_LABEL, ORDERS_STALE_CLEARED_NOTE, ORDER_STATES, PAGE_ZERO, PRICE_PENDING_CONTEXT, + PRODUCT_FILTER_LABELS, REFUND_ADDITIVE_NOTE, REFUND_REVIEW_STEP_PREFIX, RESOLVE_RECONCILIATION_NOTE, @@ -1254,6 +1256,26 @@ describe("the Orders detail copy is shared, and says what the Block Kit screen s test("the mark-refunded confirm separates the ledger from the money", () => { expect(MARK_REFUNDED_CONFIRM.text).toContain("does not move money"); }); + + test("the list's search label names EVERY axis the filter searches", () => { + // A search axis the label does not mention ships dark: nobody types into a + // box for a thing they have no reason to believe it looks at. This pins the + // label against the port's `OrderListFilter.search`, which matches an + // order-id PREFIX, a buyer_ref SUBSTRING and an exact purchase-time line + // SKU — so adding a fourth axis without a word here fails right here. + expect(ORDERS_SEARCH_LABEL).toBe("Search order ID, buyer email, or exact SKU"); + for (const axis of ["order ID", "buyer email", "SKU"]) { + expect(ORDERS_SEARCH_LABEL).toContain(axis); + } + // ONE mode word, on the one axis whose mode changes what to type: a partial + // id or email still finds the order, a partial sku finds nothing. The + // products list spends its mode words on the same principle, which is why + // that label is pinned beside this one rather than left to memory. + expect(ORDERS_SEARCH_LABEL).toContain("exact"); + expect(PRODUCT_FILTER_LABELS.search).toBe("Search (SKU exact, or title contains)"); + // A filter label is still a label: it lives inside the §1 budget. + expect(ORDERS_SEARCH_LABEL.length).toBeLessThanOrEqual(LABEL_BUDGET); + }); }); /** diff --git a/packages/admin-react/test/orders-search-label-dom.test.tsx b/packages/admin-react/test/orders-search-label-dom.test.tsx new file mode 100644 index 0000000..d48e00f --- /dev/null +++ b/packages/admin-react/test/orders-search-label-dom.test.tsx @@ -0,0 +1,86 @@ +/** + * @vitest-environment happy-dom + * + * THE SEARCH BOX SAYS WHAT IT SEARCHES — read off the rendered control, not off + * the constant. + * + * WHY THIS EXISTS AT ALL. A search axis nobody is told about ships dark: the + * store can match a purchased SKU perfectly and no operator will ever type one, + * because the box in front of them named two other things. `presentation.test + * .ts` pins the SENTENCE; what it cannot see is whether that sentence reaches + * the affordance. Between the two lives the failure this file exists for — a + * label constant updated in the shared module while the screen renders a + * hand-copied string, which is exactly the drift `admin-presentation` was + * extracted to make impossible and therefore the one worth a mounted check. + * + * WHY THE ASSERTION STARTS AT THE INPUT. Searching the container's markup for + * the sentence would pass if the words appeared anywhere on the screen — a + * heading, an empty state, a tooltip. The proof has to run the other way: find + * the control an operator types into, walk to the `