diff --git a/.changeset/orders-search-by-snapshot-sku.md b/.changeset/orders-search-by-snapshot-sku.md new file mode 100644 index 0000000..10d9831 --- /dev/null +++ b/.changeset/orders-search-by-snapshot-sku.md @@ -0,0 +1,75 @@ +--- +"@otta-sh/domain": minor +"@otta-sh/store-postgres": minor +"@otta-sh/admin-presentation": patch +"@otta-sh/admin-react": patch +"@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. + +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/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 + 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. +- **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. 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 `