Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions .changeset/orders-search-by-snapshot-sku.md
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 11 additions & 3 deletions packages/admin-presentation/src/orders-copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<input type="date">` that shows none —
Expand Down
22 changes: 22 additions & 0 deletions packages/admin-presentation/test/presentation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
});

/**
Expand Down
86 changes: 86 additions & 0 deletions packages/admin-react/test/orders-search-label-dom.test.tsx
Original file line number Diff line number Diff line change
@@ -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 `<label>` that names it, and
* read THAT text. A label that names the wrong control is the same defect as no
* label at all.
*/
import * as React from "react";
import { afterEach, beforeEach, expect, test, vi } from "vitest";
import { mount, type Mounted } from "./dom.js";

const apiFetch = vi.fn<(input: string, init?: RequestInit) => Promise<Response>>();

vi.mock("emdash/plugin-utils", async (importOriginal) => {
const actual = await importOriginal<typeof import("emdash/plugin-utils")>();
return { ...actual, apiFetch };
});

const { OrdersList } = await import("../src/orders/orders-list.js");
const { ORDERS_SEARCH_LABEL } = await import("@otta-sh/admin-presentation");

let mounted: Mounted | null = null;

beforeEach(() => {
apiFetch.mockReset();
apiFetch.mockResolvedValue(
new Response(
JSON.stringify({
data: {
ok: true,
orders: [],
nextCursor: null,
vocabulary: {
statuses: ["paid"],
statusAny: "any",
periods: [{ key: "any", label: "Any time" }],
cancellationReasons: [],
oneClickCancellationReasons: [],
},
},
}),
{ status: 200, headers: { "Content-Type": "application/json" } },
),
);
});

afterEach(async () => {
await mounted?.unmount();
mounted = null;
});

test("the search input an operator types into is LABELLED with all three axes", async () => {
const node = <OrdersList onOpen={() => undefined} />;
mounted = await mount(node);
// The first page resolves a microtask or two after the mount's own flush.
await mounted.rerender(node);

const input = mounted.container.querySelector<HTMLInputElement>('[data-testid="filter-search"]');
expect(input).not.toBeNull();
// The control is a search box, so assistive tech and the browser both treat a
// partial entry as a query — which is exactly why the label has to say that
// one of the three axes will not accept one.
expect(input?.type).toBe("search");

const label = input?.closest("label");
expect(label).not.toBeNull();
expect(label?.textContent).toContain(ORDERS_SEARCH_LABEL);
// And the words are the operator-facing ones, not a schema field name: the
// label is the only place the SKU axis is announced at all.
expect(label?.textContent).toContain("SKU");
});
9 changes: 6 additions & 3 deletions packages/domain/src/ports/coupon-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,12 @@ export interface CouponStore {
* identifier a merchant looks up precisely, and the strictest `search` in the
* product: NEITHER `ProductListFilter.search`'s title-substring half NOR
* `OrderListFilter.search`'s id-PREFIX / buyer_ref-SUBSTRING widening applies
* here. A coupon has no free-text field to partially remember, and a code is
* short, chosen and quoted whole — it never renders as a truncated prefix the
* way an order uuid does, which is what earned orders their prefix match. No
* here (that filter's THIRD arm, an exact-lower purchase-time line sku, is a
* widening only in what it reaches, not in how it matches — it is the same
* exact-identifier rule this one keeps). A coupon has no free-text field to
* partially remember, and a code is short, chosen and quoted whole — it never
* renders as a truncated prefix the way an order uuid does, which is what
* earned orders their prefix match. No
* other filter axis ships this slice (coupons have no soft-delete/
* publish-gate/kind axis to mirror `deleted`/`active`/`productKind`) —
* deliberately minimal, not "filterable where cheap".
Expand Down
Loading
Loading