feat: filter by Sold / Not sold / All on the storefront and the admin (#105)
Linting / lint (pull_request) Successful in 2m3s
SonarQube Analysis / sonarqube (pull_request) Failing after 15m56s

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>
This commit is contained in:
2026-08-22 11:58:21 -05:00
co-authored by Claude Opus 5
parent f2ab4e6565
commit 596db7a3c8
9 changed files with 618 additions and 42 deletions
+55 -5
View File
@@ -9,7 +9,17 @@ export interface ItemFilters {
tagIds: number[]; tagIds: number[];
minPriceCents: number | null; minPriceCents: number | null;
maxPriceCents: 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 // 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 // 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. // 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. // to refuse it themselves rather than the parser refusing it for everyone.
export const NON_PUBLIC_STATUSES: readonly ItemStatus[] = ['pending']; 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 { export interface BuiltFilter {
clauses: string[]; clauses: string[];
params: unknown[]; params: unknown[];
@@ -115,15 +138,39 @@ function parseTagIds(value: unknown): number[] {
return tagIds; 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'); const raw = singleValue(value, 'status');
if (raw === null || raw === '') { if (raw === null || raw === '') {
return null; 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'); throw new FilterError('invalid status');
} }
return raw as ItemStatus; return statuses;
} }
function parseFavoritesOnly(value: unknown): boolean { function parseFavoritesOnly(value: unknown): boolean {
@@ -221,7 +268,10 @@ export function buildItemFilterSql(
if (filters.status !== null) { if (filters.status !== null) {
params.push(filters.status); 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++; next++;
} }
+33 -3
View File
@@ -2,7 +2,14 @@ import { Router, Request, Response } from 'express';
import { pool } from '../db'; import { pool } from '../db';
import { asyncRoute } from '../asyncRoute'; import { asyncRoute } from '../asyncRoute';
import { PUBLIC_ITEM_SELECT } from '../itemSelect'; 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 // 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 // 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 // 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 // the exclusion below it would return an empty list, which reads as "no items
// match" rather than "you may not ask that". // 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' }); 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 where = [EXCLUDE_PENDING, ...clauses].join(' AND ');
const { rows } = await pool.query( const { rows } = await pool.query(
`${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC`, `${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC`,
@@ -0,0 +1,196 @@
import request from 'supertest';
import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
beforeEach(async () => {
await resetDb();
});
afterAll(async () => {
await pool.end();
await closeDb();
});
// Direct insert so a test can put an item in a specific state without going
// through the transitions that put it there.
async function insertItem(name: string, status: string): Promise<number> {
const { rows } = await pool.query(
`INSERT INTO items (name, price_cents, status) VALUES ($1, $2, $3) RETURNING id`,
[name, 10000, status]
);
return rows[0].id;
}
async function seedOneOfEach() {
await insertItem('Pending piece', 'pending');
await insertItem('Available piece', 'available');
await insertItem('Reserved piece', 'reserved');
await insertItem('Sold piece', 'sold');
}
const namesFrom = (body: { name: string }[]) => body.map((i) => i.name).sort();
describe('the storefront status filter', () => {
// The product decision this change carries: with no filter the storefront no
// longer lists sold items. Asserted on the default rather than on the
// parameter, because the default is the part that changed for everyone.
it('lists neither sold nor pending items by default', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/items');
expect(res.status).toBe(200);
expect(namesFrom(res.body)).toEqual(['Available piece', 'Reserved piece']);
});
it('lists only sold items when asked for them', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/items?status=sold');
expect(namesFrom(res.body)).toEqual(['Sold piece']);
});
it('brings sold items back for an explicit All', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/items?status=available,reserved,sold');
expect(namesFrom(res.body)).toEqual(['Available piece', 'Reserved piece', 'Sold piece']);
});
it('lists not-sold items for an explicit Not Sold, matching the default', async () => {
await seedOneOfEach();
const explicit = await request(app).get('/api/items?status=available,reserved');
const implied = await request(app).get('/api/items');
expect(namesFrom(explicit.body)).toEqual(namesFrom(implied.body));
});
// The guarantee that must survive the filter being generalised. Pending is
// excluded from every public read by a clause the caller cannot opt out of,
// and these assert it directly rather than trusting the parser to go on
// refusing the name.
describe('pending stays unreachable from the storefront', () => {
it('refuses a request naming pending on its own', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/items?status=pending');
expect(res.status).toBe(400);
expect(res.body.error).toBe('invalid status');
});
// The case a single-value check would have let through: the list is refused
// for naming pending at all, not accepted because the first name is fine.
it('refuses a request naming pending among allowed statuses', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/items?status=available,pending');
expect(res.status).toBe(400);
expect(res.body.error).toBe('invalid status');
});
it('never returns a pending item under any accepted combination', async () => {
await seedOneOfEach();
for (const query of ['', '?status=sold', '?status=available,reserved,sold', '?status=reserved']) {
const res = await request(app).get(`/api/items${query}`);
expect(res.status).toBe(200);
expect(res.body.map((i: { name: string }) => i.name)).not.toContain('Pending piece');
}
});
});
it('refuses an unknown status rather than ignoring it', async () => {
const res = await request(app).get('/api/items?status=available,sold_out');
expect(res.status).toBe(400);
expect(res.body.error).toBe('invalid status');
});
});
describe('the admin inventory status filter', () => {
// All means all four here, unlike the storefront. Same word, two meanings,
// which is why each is asserted where it applies rather than assumed shared.
it('lists every status including pending when none is named', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/admin/items');
expect(namesFrom(res.body)).toEqual([
'Available piece',
'Pending piece',
'Reserved piece',
'Sold piece'
]);
});
it('still accepts a single status, which is what the old dropdown sent', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/admin/items?status=pending');
expect(namesFrom(res.body)).toEqual(['Pending piece']);
});
// Not Sold in the admin includes pending, which it does not on the storefront.
it('accepts a multi-status Not Sold that includes pending', async () => {
await seedOneOfEach();
const res = await request(app).get('/api/admin/items?status=available,reserved,pending');
expect(namesFrom(res.body)).toEqual(['Available piece', 'Pending piece', 'Reserved piece']);
});
});
describe('the favorites view keeps its own default', () => {
// Favorites defaulted to showing sold items before this filter existed, on
// the reasoning that a favorite which has just sold is often exactly what the
// customer came to look at — they were emailed to say so. Defaulting them to
// Not Sold alongside everything else would have quietly reversed that.
it('shows a sold favorite with no status named', async () => {
const agent = request.agent(app);
expect(
(await agent.post('/api/customers/register').send({
email: 'favdefault@example.com',
password: 'supersecret123',
firstName: 'Thom',
lastName: 'Lamb'
})).status
).toBe(200);
const soldId = await insertItem('Sold favorite', 'sold');
const availableId = await insertItem('Available favorite', 'available');
expect((await agent.post(`/api/customers/me/favorites/${soldId}`)).status).toBeLessThan(300);
expect((await agent.post(`/api/customers/me/favorites/${availableId}`)).status).toBeLessThan(300);
const res = await agent.get('/api/items?favorites=1');
expect(res.status).toBe(200);
expect(namesFrom(res.body)).toEqual(['Available favorite', 'Sold favorite']);
});
// The default is a default, not an override: naming a status still wins.
it('honours an explicit Not Sold within the favorites view', async () => {
const agent = request.agent(app);
await agent.post('/api/customers/register').send({
email: 'favexplicit@example.com',
password: 'supersecret123',
firstName: 'Thom',
lastName: 'Lamb'
});
const soldId = await insertItem('Sold favorite', 'sold');
const availableId = await insertItem('Available favorite', 'available');
await agent.post(`/api/customers/me/favorites/${soldId}`);
await agent.post(`/api/customers/me/favorites/${availableId}`);
const res = await agent.get('/api/items?favorites=1&status=available,reserved');
expect(namesFrom(res.body)).toEqual(['Available favorite']);
});
});
+45 -5
View File
@@ -75,10 +75,41 @@ describe('parseItemFilters', () => {
expect(() => parseItemFilters({ category: ['1', '2'] })).toThrow(FilterError); expect(() => parseItemFilters({ category: ['1', '2'] })).toThrow(FilterError);
}); });
// A single status parses to a list of one, which is what lets the admin's
// existing ?status=sold keep working unchanged against the multi-value shape.
it('parses each of the item statuses', () => { it('parses each of the item statuses', () => {
expect(parseItemFilters({ status: 'available' }).status).toBe('available'); expect(parseItemFilters({ status: 'available' }).status).toEqual(['available']);
expect(parseItemFilters({ status: 'reserved' }).status).toBe('reserved'); expect(parseItemFilters({ status: 'reserved' }).status).toEqual(['reserved']);
expect(parseItemFilters({ status: 'sold' }).status).toBe('sold'); expect(parseItemFilters({ status: 'sold' }).status).toEqual(['sold']);
expect(parseItemFilters({ status: 'pending' }).status).toEqual(['pending']);
});
it('parses several statuses from one comma-separated value', () => {
expect(parseItemFilters({ status: 'available,reserved' }).status).toEqual([
'available',
'reserved'
]);
expect(parseItemFilters({ status: ' available , sold ' }).status).toEqual([
'available',
'sold'
]);
});
it('drops a repeated status rather than listing it twice', () => {
expect(parseItemFilters({ status: 'sold,sold' }).status).toEqual(['sold']);
});
// Refused rather than dropped. Ignoring the unknown name would turn this into
// "available only" - narrower than what was asked for, and indistinguishable
// from a filter that worked.
it('rejects a list containing an unknown status', () => {
expect(() => parseItemFilters({ status: 'available,sold_out' })).toThrow(FilterError);
});
// Asked for something, named nothing. Answering null would mean "no status
// filter", which on the storefront is the default rather than everything.
it('rejects a list that names nothing', () => {
expect(() => parseItemFilters({ status: ',,' })).toThrow(FilterError);
}); });
it('treats an absent or empty status as no status filter', () => { it('treats an absent or empty status as no status filter', () => {
@@ -145,8 +176,17 @@ describe('buildItemFilterSql', () => {
it('filters on status', () => { it('filters on status', () => {
const built = buildItemFilterSql(parseItemFilters({ status: 'reserved' }), 1, null); const built = buildItemFilterSql(parseItemFilters({ status: 'reserved' }), 1, null);
expect(built.clauses.join(' ')).toContain('i.status'); expect(built.clauses.join(' ')).toContain('i.status = ANY');
expect(built.params).toEqual(['reserved']); expect(built.params).toEqual([['reserved']]);
});
// One clause for one status and for several, which is the whole reason the
// filter was generalised rather than joined by a second dimension.
it('filters on several statuses with the same single clause', () => {
const built = buildItemFilterSql(parseItemFilters({ status: 'available,reserved' }), 1, null);
expect(built.clauses).toHaveLength(1);
expect(built.clauses[0]).toContain('i.status = ANY');
expect(built.params).toEqual([['available', 'reserved']]);
}); });
it('restricts to the favorites of the given customer', () => { it('restricts to the favorites of the given customer', () => {
+36 -1
View File
@@ -10,6 +10,7 @@ import theme from 'antd/es/theme';
import Badge from 'antd/es/badge'; import Badge from 'antd/es/badge';
import Empty from 'antd/es/empty'; import Empty from 'antd/es/empty';
import Alert from 'antd/es/alert'; import Alert from 'antd/es/alert';
import Segmented from 'antd/es/segmented';
import { ShoppingCartOutlined, FilterOutlined } from '@ant-design/icons'; import { ShoppingCartOutlined, FilterOutlined } from '@ant-design/icons';
import { Link, useLocation, useSearchParams } from 'react-router-dom'; import { Link, useLocation, useSearchParams } from 'react-router-dom';
import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api'; import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api';
@@ -19,10 +20,13 @@ import FilterDrawer from './components/FilterDrawer';
import ActiveFilterChips from './components/ActiveFilterChips'; import ActiveFilterChips from './components/ActiveFilterChips';
import { import {
ItemFilters, ItemFilters,
SaleState,
STOREFRONT_SALE_STATUSES,
activeFilterCount, activeFilterCount,
filtersFromSearchParams, filtersFromSearchParams,
filtersToSearchParams, filtersToSearchParams,
hasActiveFilters hasActiveFilters,
saleStateFromStatuses
} from './filters'; } from './filters';
import AuthPromptModal from './customer/AuthPromptModal'; import AuthPromptModal from './customer/AuthPromptModal';
import { useThemeMode } from './theme/ThemeContext'; import { useThemeMode } from './theme/ThemeContext';
@@ -248,6 +252,37 @@ export default function App() {
</Header> </Header>
<Content style={{ padding: 24 }}> <Content style={{ padding: 24 }}>
<div className="filter-bar"> <div className="filter-bar">
{/* In the bar rather than inside the drawer, deliberately. The default
now hides sold pieces, so a customer who never opens the drawer
would otherwise have no way to know sold items exist — and on a
one-of-a-kind catalogue the sold pieces are part of the story. */}
<Segmented
aria-label="Filter by availability"
// The fallback matches the server's: the favorites view defaults to
// everything, so the control must not claim Not Sold while sold
// favorites are on screen.
value={saleStateFromStatuses(
filters.status,
STOREFRONT_SALE_STATUSES,
filters.favoritesOnly ? 'all' : 'not-sold'
)}
onChange={(value) => {
const state = value as SaleState;
applyFilters({
...filters,
// Not Sold is the default, so it is stored as "no preference"
// rather than as an explicit list. That keeps it out of the URL
// and out of the Filters (N) count, where it would otherwise
// show as an active filter nobody chose.
status: state === 'not-sold' ? null : STOREFRONT_SALE_STATUSES[state]
});
}}
options={[
{ label: 'Not sold', value: 'not-sold' },
{ label: 'Sold', value: 'sold' },
{ label: 'All', value: 'all' }
]}
/>
<Button <Button
icon={<FilterOutlined />} icon={<FilterOutlined />}
onClick={() => setDrawerOpen(true)} onClick={() => setDrawerOpen(true)}
+36 -16
View File
@@ -1,10 +1,19 @@
import { useMemo } from 'react'; import { useMemo } from 'react';
import TreeSelect from 'antd/es/tree-select'; import TreeSelect from 'antd/es/tree-select';
import Select from 'antd/es/select'; import Select from 'antd/es/select';
import Segmented from 'antd/es/segmented';
import InputNumber from 'antd/es/input-number'; import InputNumber from 'antd/es/input-number';
import Button from 'antd/es/button'; import Button from 'antd/es/button';
import type { Category, Tag } from '../api'; import type { Category, Tag } from '../api';
import { ItemFilters, ItemStatus, buildCategoryTree, CategoryNode, hasActiveFilters } from '../filters'; import {
ItemFilters,
SaleState,
ADMIN_SALE_STATUSES,
buildCategoryTree,
CategoryNode,
hasActiveFilters,
saleStateFromStatuses
} from '../filters';
interface CategoryTreeOption { interface CategoryTreeOption {
value: number; value: number;
@@ -20,13 +29,6 @@ function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
})); }));
} }
const STATUS_OPTIONS: { value: ItemStatus; label: string }[] = [
{ value: 'pending', label: 'Pending' },
{ value: 'available', label: 'Available' },
{ value: 'reserved', label: 'Reserved' },
{ value: 'sold', label: 'Sold' }
];
interface Props { interface Props {
categories: Category[]; categories: Category[];
tags: Tag[]; tags: Tag[];
@@ -91,14 +93,32 @@ export default function InventoryFilters({ categories, tags, filters, onChange,
onChange={(value) => onChange({ ...filters, maxPriceCents: dollarsToCents(value) })} onChange={(value) => onChange({ ...filters, maxPriceCents: dollarsToCents(value) })}
/> />
<Select {/* Replaces the four-way status dropdown that used to sit here. One
allowClear control instead of two overlapping ways to say the same thing.
placeholder="Any status" Note what it costs: a single status can no longer be isolated, so
aria-label="Filter by status" there is no longer a way to view only Reserved, or only Pending.
style={{ minWidth: 150 }} Not Sold folds pending in with available and reserved. If isolating
value={filters.status ?? undefined} one status turns out to matter — the pending workflow from #90 is the
onChange={(value: ItemStatus | undefined) => onChange({ ...filters, status: value ?? null })} likeliest candidate — the fix is to put that back alongside this
options={STATUS_OPTIONS} preset, not to remove it. See the decision recorded on #105. */}
<Segmented
aria-label="Filter by availability"
value={saleStateFromStatuses(filters.status, ADMIN_SALE_STATUSES, 'all')}
onChange={(value) => {
const state = value as SaleState;
onChange({
...filters,
// All is the admin's default, so it is held as "no preference"
// rather than as a list naming every status — which keeps it out of
// the active-filter count and out of Clear filters' way.
status: state === 'all' ? null : ADMIN_SALE_STATUSES[state]
});
}}
options={[
{ label: 'Not sold', value: 'not-sold' },
{ label: 'Sold', value: 'sold' },
{ label: 'All', value: 'all' }
]}
/> />
{hasActiveFilters(filters) && <Button onClick={onClear}>Clear filters</Button>} {hasActiveFilters(filters) && <Button onClick={onClear}>Clear filters</Button>}
+81 -9
View File
@@ -7,15 +7,69 @@ export interface ItemFilters {
tagIds: number[]; tagIds: number[];
minPriceCents: number | null; minPriceCents: number | null;
maxPriceCents: number | null; maxPriceCents: number | null;
// Only the admin Inventory tab sets this; the storefront leaves it null and // Several statuses, because the control this serves is not a status filter:
// shows every status, as it always has. // "Not Sold" is available-or-reserved on the storefront and includes pending
status: ItemStatus | null; // in the admin, neither of which is one value.
//
// Null means "no preference", which each side turns into its own default -
// Not Sold on the storefront, every status in the admin. Keeping the default
// as null rather than as an explicit list is what keeps it out of the URL and
// out of the active-filter count.
status: ItemStatus[] | null;
// Storefront only, and only meaningful when signed in. Sold favorites are // Storefront only, and only meaningful when signed in. Sold favorites are
// included: the storefront shows sold items everywhere else, and a favorite // included: the storefront shows sold items everywhere else, and a favorite
// that has just sold is often exactly what the customer came to look at. // that has just sold is often exactly what the customer came to look at.
favoritesOnly: boolean; favoritesOnly: boolean;
} }
// The three-way control both screens offer. It is a preset over the status
// list rather than a filter of its own, so there is only ever one dimension and
// no way to express a contradiction like "sold and not sold".
export type SaleState = 'sold' | 'not-sold' | 'all';
// The same three words mean different sets in the two places, which is worth
// stating twice rather than sharing one table that would be wrong for one of
// them. Pending is excluded from every public read regardless of filter, so on
// the storefront "All" cannot and must not include it — a label promising more
// than it delivers.
export const STOREFRONT_SALE_STATUSES: Record<SaleState, ItemStatus[]> = {
'not-sold': ['available', 'reserved'],
sold: ['sold'],
all: ['available', 'reserved', 'sold']
};
// In the admin, Not Sold includes pending: an item awaiting publication has
// certainly not been sold, and hiding it from the default view would hide the
// items most likely to need attention.
export const ADMIN_SALE_STATUSES: Record<SaleState, ItemStatus[]> = {
'not-sold': ['available', 'reserved', 'pending'],
sold: ['sold'],
all: ['available', 'reserved', 'sold', 'pending']
};
function isPublicStatus(value: string): value is ItemStatus {
return value === 'available' || value === 'reserved' || value === 'sold';
}
const sameSet = (a: readonly string[], b: readonly string[]) =>
a.length === b.length && [...a].sort().join() === [...b].sort().join();
// Which preset a status list corresponds to, for showing the control's current
// position. Null means no preference, which each screen renders as its default.
// A list matching none of the three - only reachable by hand-editing the URL -
// reports as the default rather than leaving the control blank.
export function saleStateFromStatuses(
statuses: ItemStatus[] | null,
table: Record<SaleState, ItemStatus[]>,
fallback: SaleState = 'not-sold'
): SaleState {
if (statuses === null) return fallback;
const match = (Object.keys(table) as SaleState[]).find((state) =>
sameSet(statuses, table[state])
);
return match ?? fallback;
}
export const EMPTY_FILTERS: ItemFilters = { export const EMPTY_FILTERS: ItemFilters = {
categoryId: null, categoryId: null,
tagIds: [], tagIds: [],
@@ -34,7 +88,7 @@ export function filtersToSearchParams(filters: ItemFilters): URLSearchParams {
if (filters.tagIds.length) params.set('tags', filters.tagIds.join(',')); if (filters.tagIds.length) params.set('tags', filters.tagIds.join(','));
if (filters.minPriceCents !== null) params.set('min_price', String(filters.minPriceCents)); if (filters.minPriceCents !== null) params.set('min_price', String(filters.minPriceCents));
if (filters.maxPriceCents !== null) params.set('max_price', String(filters.maxPriceCents)); if (filters.maxPriceCents !== null) params.set('max_price', String(filters.maxPriceCents));
if (filters.status !== null) params.set('status', filters.status); if (filters.status !== null) params.set('status', filters.status.join(','));
if (filters.favoritesOnly) params.set('favorites', '1'); if (filters.favoritesOnly) params.set('favorites', '1');
return params; return params;
} }
@@ -58,10 +112,19 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
// fail. The admin's status filter holds its value in React state and never // fail. The admin's status filter holds its value in React state and never
// round-trips through this function, so it is unaffected. Do not "complete" // round-trips through this function, so it is unaffected. Do not "complete"
// this list to match the type. // this list to match the type.
//
// A list containing anything unreadable yields null - the default - rather
// than the readable subset, so a mangled link falls back to a view that is
// explainable instead of one silently narrower than it looks.
const rawStatus = params.get('status'); const rawStatus = params.get('status');
const status = rawStatus === 'available' || rawStatus === 'reserved' || rawStatus === 'sold' const parsedStatus = (rawStatus || '')
? rawStatus .split(',')
: null; .map((part) => part.trim())
.filter((part) => part !== '');
const status =
parsedStatus.length > 0 && parsedStatus.every(isPublicStatus)
? (parsedStatus as ItemStatus[])
: null;
const favorites = params.get('favorites'); const favorites = params.get('favorites');
@@ -77,18 +140,27 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
// One count for the "Filters (N)" button. A price range counts once however // One count for the "Filters (N)" button. A price range counts once however
// many ends are set, since it reads as a single filter to the user. // many ends are set, since it reads as a single filter to the user.
//
// Status is deliberately not counted. It has its own always-visible control
// beside this button rather than living in the drawer, so counting it would put
// a number on a button whose drawer shows nothing set — and the control already
// displays its own position.
export function activeFilterCount(filters: ItemFilters): number { export function activeFilterCount(filters: ItemFilters): number {
let count = 0; let count = 0;
if (filters.categoryId !== null) count++; if (filters.categoryId !== null) count++;
count += filters.tagIds.length; count += filters.tagIds.length;
if (filters.minPriceCents !== null || filters.maxPriceCents !== null) count++; if (filters.minPriceCents !== null || filters.maxPriceCents !== null) count++;
if (filters.status !== null) count++;
if (filters.favoritesOnly) count++; if (filters.favoritesOnly) count++;
return count; return count;
} }
// Broader than the count above, and intentionally so: this decides whether an
// empty result reads as "no items match these filters" with a way out, or as an
// empty shop. A status filter that matched nothing is exactly the case where
// that distinction matters, so it counts here even though it is not in the
// drawer's tally.
export function hasActiveFilters(filters: ItemFilters): boolean { export function hasActiveFilters(filters: ItemFilters): boolean {
return activeFilterCount(filters) > 0; return activeFilterCount(filters) > 0 || filters.status !== null;
} }
export interface CategoryNode extends Category { export interface CategoryNode extends Category {
@@ -40,6 +40,15 @@ test.beforeAll(async ({ playwright }) => {
await api.dispose(); await api.dispose();
}); });
// antd Segmented hides the real radio input behind a styled label, so the input
// is found by role but cannot be clicked. The label carries a title attribute,
// which is the same handle this suite already uses for antd Select options.
// The input is still the right thing to assert checked-ness on: toBeChecked
// does not require visibility.
async function chooseAvailability(page: Page, label: string) {
await page.getByTitle(label, { exact: true }).click();
}
const row = (page: Page, name: string) => page.getByRole('row').filter({ hasText: name }); const row = (page: Page, name: string) => page.getByRole('row').filter({ hasText: name });
// The Inventory table paginates and other specs create items concurrently, so // The Inventory table paginates and other specs create items concurrently, so
@@ -77,17 +86,28 @@ test.describe('Admin inventory filters', () => {
await expect(row(page, NAMES.dear)).toHaveCount(0); await expect(row(page, NAMES.dear)).toHaveCount(0);
}); });
test('filters by status, which is how Reserved is reached', async ({ page }) => { // The four-way status dropdown is gone, replaced by the Sold / Not sold / All
// preset from #105. Isolating a single status went with it, so this no longer
// covers "which is how Reserved is reached" — that ability was given up
// deliberately and is recorded on the issue. What remains testable, and what
// matters, is that Sold and Not sold partition the inventory.
test('filters by availability', async ({ page }) => {
await page.goto('/admin'); await page.goto('/admin');
await filterToOwnCategory(page); await filterToOwnCategory(page);
await page.getByRole('combobox', { name: 'Filter by status' }).click(); await chooseAvailability(page, 'Sold');
await page.getByTitle('Sold', { exact: true }).click();
// Every fixture is available, so a Sold filter must exclude them all. // Every fixture is available, so a Sold filter must exclude them all.
await expect(row(page, NAMES.cheap)).toHaveCount(0); await expect(row(page, NAMES.cheap)).toHaveCount(0);
await expect(row(page, NAMES.mid)).toHaveCount(0); await expect(row(page, NAMES.mid)).toHaveCount(0);
await expect(row(page, NAMES.dear)).toHaveCount(0); await expect(row(page, NAMES.dear)).toHaveCount(0);
// And Not sold brings back exactly what Sold excluded, which is the property
// that makes the two-way split trustworthy rather than merely plausible.
await chooseAvailability(page, 'Not sold');
await expect(row(page, NAMES.cheap)).toBeVisible();
await expect(row(page, NAMES.mid)).toBeVisible();
await expect(row(page, NAMES.dear)).toBeVisible();
}); });
test('combines filters, and clearing restores them', async ({ page }) => { test('combines filters, and clearing restores them', async ({ page }) => {
+113
View File
@@ -0,0 +1,113 @@
import { test, expect } from './fixtures';
// The storefront shows every item ever seeded and the e2e database is not reset
// between runs, so every fixture carries a unique run id and assertions name
// only the items this run created.
const RUN = `s${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
const NAMES = {
available: `Available piece ${RUN}`,
sold: `Sold piece ${RUN}`
};
// Scoped to the item's own grid cell: the SOLD ribbon sits outside the card,
// and other runs' sold items share the page.
const cell = (page: import('@playwright/test').Page, name: string) =>
page.locator('.ant-col').filter({ hasText: name });
// antd Segmented hides the real radio input behind a styled label, so the input
// is found by role but cannot be clicked. The label carries a title attribute,
// which is the same handle this suite already uses for antd Select options.
// The input is still the right thing to assert checked-ness on: toBeChecked
// does not require visibility.
async function chooseAvailability(page: import('@playwright/test').Page, label: string) {
await page.getByTitle(label, { exact: true }).click();
}
test.beforeAll(async ({ playwright }) => {
const api = await playwright.request.newContext({ baseURL: 'http://localhost:5173' });
for (const name of [NAMES.available, NAMES.sold]) {
const res = await api.post('/api/admin/items', {
multipart: { name, description: '', price: '250.00' }
});
expect(res.ok()).toBeTruthy();
const { id } = await res.json();
// Items arrive pending since #90, so publishing is what makes them public.
expect((await api.post(`/api/admin/items/${id}/mark-available`)).ok()).toBeTruthy();
if (name === NAMES.sold) {
expect((await api.post(`/api/admin/items/${id}/mark-sold`)).ok()).toBeTruthy();
}
}
await api.dispose();
});
test.describe('Filtering the storefront by availability', () => {
// The change customers actually see. Asserted on a bare visit rather than on
// a parameter, because the default is what changed for everyone.
test('hides sold pieces by default', async ({ page }) => {
await page.goto('/');
await expect(cell(page, NAMES.available)).toBeVisible();
await expect(cell(page, NAMES.sold)).toHaveCount(0);
});
test('All brings them back, ribbon and all', async ({ page }) => {
await page.goto('/');
await chooseAvailability(page, 'All');
await expect(cell(page, NAMES.sold)).toBeVisible();
await expect(cell(page, NAMES.sold)).toContainText('SOLD');
await expect(cell(page, NAMES.available)).toBeVisible();
});
test('Sold shows only the sold ones', async ({ page }) => {
await page.goto('/');
await chooseAvailability(page, 'Sold');
await expect(cell(page, NAMES.sold)).toBeVisible();
await expect(cell(page, NAMES.available)).toHaveCount(0);
});
// The filter is view state that belongs in the URL, like every other filter
// here, so a chosen view can be linked and survives a reload.
test('the choice survives a reload, because it lives in the URL', async ({ page }) => {
await page.goto('/');
await chooseAvailability(page, 'All');
await expect(cell(page, NAMES.sold)).toBeVisible();
await page.reload();
await expect(cell(page, NAMES.sold)).toBeVisible();
await expect(page.getByRole('radio', { name: 'All' })).toBeChecked();
});
// Not sold is the default, so it is held as "no preference" rather than as an
// explicit list. Putting it in the URL would make the default look like a
// choice somebody made, and would show it in the Filters (N) count.
test('returning to Not sold leaves no status in the URL', async ({ page }) => {
await page.goto('/');
await chooseAvailability(page, 'All');
await expect(page).toHaveURL(/status=/);
await chooseAvailability(page, 'Not sold');
await expect(page).not.toHaveURL(/status=/);
await expect(cell(page, NAMES.sold)).toHaveCount(0);
});
// The control sits beside the Filters button rather than inside the drawer,
// so its state must not be counted as one of the drawer's filters.
test('does not inflate the Filters count', async ({ page }) => {
await page.goto('/');
await chooseAvailability(page, 'Sold');
// Matched loosely and asserted on the text, because antd's icon contributes
// its own aria-label to the button's accessible name. The text is the part
// that would gain a "(1)" if status were counted as a drawer filter.
await expect(page.getByRole('button', { name: /Filters/ })).toHaveText('Filters');
});
});