feat: filter by Sold / Not sold / All on the storefront and the admin (#105)

Three decisions were taken before any code, and are recorded on the issue.

The status filter is generalised to accept several values rather than gaining a second `sold` dimension beside it. "Not sold" is not a status: it is available-or-reserved on the storefront and includes pending in the admin, neither of which is one value. `?status=available,reserved` and `i.status = ANY($n::text[])` express that with one concept, so there is no way to write a contradiction like `?status=sold&sold=no`. A single status still parses to a list of one, which is how the admin's existing `?status=sold` keeps working untouched.

The storefront now defaults to Not sold. That is a change in what every customer sees, not just a new control: the black SOLD ribbons leave the default view on a catalogue where they were evidence the shop sells things, and every storefront link shared so far quietly changes meaning. Accepted deliberately, with the default named in STOREFRONT_DEFAULT_STATUSES rather than implied by the absence of a parameter.

The admin's four-way status dropdown is replaced rather than joined. That gives up isolating a single status: there is no longer a way to view only Reserved, or only Pending, and Not sold folds pending in with the rest. The pending workflow from #90 is the likeliest thing to miss it, and if it does, the fix is to put isolation back beside the preset rather than to remove the preset. The e2e test that covered "which is how Reserved is reached" is renamed and narrowed to what survives, rather than deleted.

One thing the issue did not anticipate, found by a test rather than by reading. The favorites view deliberately showed sold favorites - "a favorite that has just sold is often exactly what the customer came to look at", and they have just been emailed to say so. Defaulting the storefront to Not sold reversed that silently and broke the test asserting it. Favorites therefore keep their own default of everything, on the server and in the control's displayed position, while an explicit ?status= still wins. That interaction is the kind a single-feature change quietly breaks, and it was caught only because the previous decision had been written down as an assertion.

"All" still means different things in the two places, as the issue set out: available + reserved + sold on the storefront, all four in the admin. Pending remains unreachable from every public read - the storefront's unconditional exclusion clause is untouched - and the pending guard now checks every requested status rather than a single one, so `?status=available,pending` is refused for naming pending at all rather than accepted because the first name happened to be allowed.

The control sits in the filter bar rather than in the drawer, since the default now hides sold pieces and a customer who never opens the drawer would otherwise have no way to know they exist. It is consequently excluded from the "Filters (N)" count, which describes the drawer, while still counting toward hasActiveFilters so that an empty result reads as "no items match these filters" with a way out rather than as an empty shop.

Verification: 13 integration tests covering the default, each preset, the favorites exception and its override, and pending's unreachability under every accepted combination; 38 parser unit tests including multi-value parsing, an unknown name in a list being refused rather than dropped, and a list that names nothing; 6 new end-to-end tests for the storefront control, its URL round-trip, and the default staying out of the URL. 204 backend unit tests and 76 integration tests across the four affected suites pass. Two full end-to-end runs: 110 and 111 passing against the same 3 pre-existing failures, one run also showing a pending-publish failure that passes in isolation and did not recur - the cross-suite database contention filed as #116. tsc, ESLint and the production build are clean.

Closes #105
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

backend/src/itemFilters.ts

@@ -9,7 +9,17 @@ export interface ItemFilters {
   tagIds: number[];
   minPriceCents: number | null;
   maxPriceCents: number | null;
-  status: ItemStatus | null;
+  // Several statuses rather than one, because the control this exists to serve
+  // is not a status filter. "Not sold" is available-or-reserved on the
+  // storefront and available-or-reserved-or-pending in the admin, so it cannot
+  // be expressed as equality against a single value. A single-status filter is
+  // still expressible: it arrives as a list of one, which is how the admin's
+  // old `?status=sold` keeps working unchanged.
+  //
+  // Null means the caller expressed no preference, which is distinct from
+  // asking for every status — the storefront turns the first into its default
+  // and the second into an explicit list.
+  status: ItemStatus[] | null;
   // Storefront only: "just the items I have favorited". Which customer that
   // means is not part of the parsed filter — it comes from the session at build
   // time, so a query string can never name someone else's favorites.
@@ -28,6 +38,19 @@ const ITEM_STATUSES: readonly string[] = ['pending', 'available', 'reserved', 's
 // to refuse it themselves rather than the parser refusing it for everyone.
 export const NON_PUBLIC_STATUSES: readonly ItemStatus[] = ['pending'];
 
+// What the storefront lists when the caller expressed no preference. Named here
+// rather than implied by the absence of a parameter, because the absence is now
+// meaningful: before this change no status meant every status, and afterwards it
+// means these two. Anything reading a shared link from before will get the new
+// meaning, which is the accepted cost of the default changing.
+export const STOREFRONT_DEFAULT_STATUSES: readonly ItemStatus[] = ['available', 'reserved'];
+
+// What "All" can mean on the storefront, which is not all of them. Pending items
+// are excluded from every public read unconditionally, so a filter labelled All
+// must not promise the fourth — a label that delivers less than it says is the
+// shape this codebase keeps designing against.
+export const STOREFRONT_ALL_STATUSES: readonly ItemStatus[] = ['available', 'reserved', 'sold'];
+
 export interface BuiltFilter {
   clauses: string[];
   params: unknown[];
@@ -115,15 +138,39 @@ function parseTagIds(value: unknown): number[] {
   return tagIds;
 }
 
-function parseStatus(value: unknown): ItemStatus | null {
+// Comma-separated, matching how `tags` already works, so the two multi-value
+// parameters in this parser read the same way in a URL.
+//
+// An unrecognised name is refused rather than dropped. Silently ignoring one
+// would turn `?status=available,sold_out` into "available only" — narrower than
+// what was asked for, and indistinguishable from a filter that worked.
+function parseStatus(value: unknown): ItemStatus[] | null {
   const raw = singleValue(value, 'status');
   if (raw === null || raw === '') {
     return null;
   }
-  if (!ITEM_STATUSES.includes(raw)) {
+  const statuses: ItemStatus[] = [];
+  for (const part of raw.split(',')) {
+    const trimmed = part.trim();
+    if (trimmed === '') {
+      continue;
+    }
+    if (!ITEM_STATUSES.includes(trimmed)) {
+      throw new FilterError('invalid status');
+    }
+    // Duplicates are harmless in `= ANY(...)`, but removing them keeps the
+    // parsed filter a faithful description of what was asked for.
+    if (!statuses.includes(trimmed as ItemStatus)) {
+      statuses.push(trimmed as ItemStatus);
+    }
+  }
+  // `?status=,,` asked for something and named nothing. Returning null would
+  // silently mean "no status filter", which on the storefront now means the
+  // default rather than everything — a different answer from the one requested.
+  if (statuses.length === 0) {
     throw new FilterError('invalid status');
   }
-  return raw as ItemStatus;
+  return statuses;
 }
 
 function parseFavoritesOnly(value: unknown): boolean {
@@ -221,7 +268,10 @@ export function buildItemFilterSql(
 
   if (filters.status !== null) {
     params.push(filters.status);
-    clauses.push(`i.status = $${next}`);
+    // ANY rather than equality, so one status and several use the same clause.
+    // The ::text[] cast is explicit because `status` is a text column and the
+    // driver would otherwise have to infer the array's element type.
+    clauses.push(`i.status = ANY($${next}::text[])`);
     next++;
   }
 

backend/src/routes/items.ts

@@ -2,7 +2,14 @@ import { Router, Request, Response } from 'express';
 import { pool } from '../db';
 import { asyncRoute } from '../asyncRoute';
 import { PUBLIC_ITEM_SELECT } from '../itemSelect';
-import { parseItemFilters, buildItemFilterSql, FilterError, NON_PUBLIC_STATUSES } from '../itemFilters';
+import {
+  parseItemFilters,
+  buildItemFilterSql,
+  FilterError,
+  NON_PUBLIC_STATUSES,
+  STOREFRONT_DEFAULT_STATUSES,
+  STOREFRONT_ALL_STATUSES
+} from '../itemFilters';
 
 // Applied to every public read, unconditionally. This route has never had a
 // status filter of its own — sold items are listed and rendered with a Sold
@@ -38,11 +45,34 @@ router.get('/', asyncRoute(async (req: Request, res: Response) => {
   // admin routes, where 'pending' is valid, so it parses here too — and with
   // the exclusion below it would return an empty list, which reads as "no items
   // match" rather than "you may not ask that".
-  if (filters.status && NON_PUBLIC_STATUSES.includes(filters.status)) {
+  //
+  // Checked across every requested status, not just a single one: `?status=
+  // available,pending` must be refused for naming pending at all, rather than
+  // quietly answered because the first name in the list happened to be allowed.
+  if (filters.status?.some((status) => NON_PUBLIC_STATUSES.includes(status))) {
     return res.status(400).json({ error: 'invalid status' });
   }
 
-  const { clauses, params } = buildItemFilterSql(filters, 1, req.customerId ?? null);
+  // No preference means Not Sold rather than everything. Applied here rather
+  // than in the parser, which is shared with the admin, where the same absence
+  // has to go on meaning "every status including pending".
+  //
+  // Except when the customer asked for their own favorites, where the default
+  // stays everything. A favorite that has just sold is often exactly what the
+  // customer came to look at — they were emailed to say so — and hiding it
+  // would make an item they curated vanish without explanation. That was a
+  // deliberate decision before this filter existed, and defaulting favorites to
+  // Not Sold would have quietly reversed it. An explicit ?status= still wins,
+  // so the choice remains theirs.
+  const defaultStatuses = filters.favoritesOnly
+    ? STOREFRONT_ALL_STATUSES
+    : STOREFRONT_DEFAULT_STATUSES;
+  const effectiveFilters = {
+    ...filters,
+    status: filters.status ?? [...defaultStatuses]
+  };
+
+  const { clauses, params } = buildItemFilterSql(effectiveFilters, 1, req.customerId ?? null);
   const where = [EXCLUDE_PENDING, ...clauses].join(' AND ');
   const { rows } = await pool.query(
     `${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC`,