feat(filters): make the storefront filter panel searchable and multi-select (#139)

The filter drawer did not scale with the taxonomy behind it. Categories were a bare antd `Tree` rendered at whatever depth it had grown to, with no search and single selection, and tags were a wall of every tag in the system. Neither said what was selected except through highlighting and chip colour.

Both are now searchable multi-selects. Categories keep their hierarchy in a `TreeSelect`, matching the admin's `CategoryTreeSelect` so the two screens behave alike; tags become a multiple `Select` whose selected pills keep their colours, which is the only place a tag's colour was load-bearing.

Several categories combine as OR. A customer picking Furniture and Decor wants both, not the empty intersection, and each selected id still expands to its descendants, so the answer is the union of the subtrees. That is deliberately the opposite of the tag rule, which stays AND, and both headings now state their rule rather than leaving it to be discovered.

`ItemFilters.categoryId` becomes `categoryIds` end to end. The recursive CTE is seeded with `= ANY($n::int[])` rather than one id, which walks every selected root in one recursion and gives the OR for free; matching on `IN` keeps it a set test, so an item under two selected branches still appears once. The query parameter keeps its singular name and becomes comma-separated, the shape `tags` and `status` already use, so every `?category=1` link written before this still parses as a list of one. A list containing anything unreadable is still a 400, per decision 9 — honouring the readable half would answer a narrower question than the one asked and look indistinguishable from a filter that worked.

The admin's inventory filter stays single-select, since it asks what is in a category rather than in any of several, but reads and writes a list of at most one so there is one shared filter type rather than two that drift.

Closes #139
This commit is contained in:
2026-08-24 16:02:33 -05:00
parent 70cc3056e7
commit faf38be91a
9 changed files with 308 additions and 79 deletions
+44 -11
View File
@@ -8,7 +8,14 @@ export type { ItemStatus };
export class FilterError extends Error {}
export interface ItemFilters {
categoryId: number | null;
// Several categories, combined as OR — a customer picking Furniture and
// Decor wants both, not the empty intersection. Deliberately the opposite of
// how tagIds combine below, which is AND; the two controls say so in the UI
// rather than leaving it to be discovered.
//
// Each selected id still expands to its descendants, so choosing two parents
// means "anything filed under either of them".
categoryIds: number[];
tagIds: number[];
minPriceCents: number | null;
maxPriceCents: number | null;
@@ -110,12 +117,33 @@ function parsePrice(value: unknown, name: string): number | null {
return parseNonNegativeInteger(raw, name);
}
function parseCategoryId(value: unknown): number | null {
/**
* Comma-separated, the same shape `tags` and `status` already use.
*
* The parameter keeps its singular name so that every `?category=1` link,
* bookmark and shared URL written before this went multi-valued still parses —
* as a list of one, needing no alias and leaving no way to give the filter
* twice with different meanings.
*/
function parseCategoryIds(value: unknown): number[] {
const raw = singleValue(value, 'category');
if (raw === null || raw === '') {
return null;
const categoryIds: number[] = [];
if (!raw) {
return categoryIds;
}
return parseId(raw, 'category');
for (const part of raw.split(',')) {
const trimmed = part.trim();
if (trimmed === '') {
continue;
}
const id = parseId(trimmed, 'category');
// Duplicates are harmless to the OR below, but they would show twice in
// any caller that renders the parsed filter back.
if (!categoryIds.includes(id)) {
categoryIds.push(id);
}
}
return categoryIds;
}
function parseTagIds(value: unknown): number[] {
@@ -193,7 +221,7 @@ function parseFavoritesOnly(value: unknown): boolean {
// deliberate and observable: a query wrong in two ways reports the first
// field, so moving these lines around changes which error a caller sees.
export function parseItemFilters(query: Record<string, unknown>): ItemFilters {
const categoryId = parseCategoryId(query.category);
const categoryIds = parseCategoryIds(query.category);
const tagIds = parseTagIds(query.tags);
const minPriceCents = parsePrice(query.min_price, 'min_price');
@@ -205,7 +233,7 @@ export function parseItemFilters(query: Record<string, unknown>): ItemFilters {
const status = parseStatus(query.status);
const favoritesOnly = parseFavoritesOnly(query.favorites);
return { categoryId, tagIds, minPriceCents, maxPriceCents, status, favoritesOnly };
return { categoryIds, tagIds, minPriceCents, maxPriceCents, status, favoritesOnly };
}
// Returns WHERE fragments plus their parameters, with placeholders numbered
@@ -226,15 +254,20 @@ export function buildItemFilterSql(
const params: unknown[] = [];
let next = startIndex;
if (filters.categoryId !== null) {
params.push(filters.categoryId);
if (filters.categoryIds.length) {
params.push(filters.categoryIds);
// Selecting a category means "and everything filed beneath it", so walk the
// tree down from the chosen node. A recursive CTE keeps the tree
// tree down from each chosen node. A recursive CTE keeps the tree
// un-denormalized: reparenting stays a single UPDATE with no stored paths
// to rewrite.
//
// Seeded with `= ANY(...)` rather than one id, so every selected root is
// walked in the same recursion. That also gives the OR for free: the union
// of the subtrees is exactly "filed under any of these", and an item filed
// under two selected branches appears once because IN is a set test.
clauses.push(`i.category_id IN (
WITH RECURSIVE subtree AS (
SELECT id FROM categories WHERE id = $${next}
SELECT id FROM categories WHERE id = ANY($${next}::int[])
UNION ALL
SELECT c.id FROM categories c JOIN subtree s ON c.parent_id = s.id
)
@@ -371,6 +371,34 @@ describe('GET /api/items filtering', () => {
expect(res.body.map((i: { name: string }) => i.name).sort()).toEqual(['Deep', 'Mid', 'Top']);
});
it('combines several categories as any-of rather than all-of', async () => {
const furniture = await createCategory('Furniture');
const tables = await createCategory('Tables', furniture);
const decor = await createCategory('Decor');
const art = await createCategory('Art');
await createItem('Nested', 1000, tables);
await createItem('Elsewhere', 1000, decor);
await createItem('Unrelated', 1000, art);
// Two branches sharing no items, so an AND would answer with nothing at
// all — which is the failure this exists to catch.
const res = await request(app).get(`/api/items?category=${furniture},${decor}`);
expect(res.body.map((i: { name: string }) => i.name).sort()).toEqual(['Elsewhere', 'Nested']);
});
it('lists an item once when its category sits under two selected branches', async () => {
const furniture = await createCategory('Furniture');
const tables = await createCategory('Tables', furniture);
await createItem('Nested', 1000, tables);
// The subtrees overlap: Tables is walked from both seeds, so the recursion
// yields its id twice. Selecting on IN makes that a set test rather than a
// join, so the item still appears once.
const res = await request(app).get(`/api/items?category=${furniture},${tables}`);
expect(res.body.map((i: { name: string }) => i.name)).toEqual(['Nested']);
});
it('excludes uncategorized items from a category filter', async () => {
const furniture = await createCategory('Furniture');
await createItem('Filed', 1000, furniture);
@@ -444,6 +472,16 @@ describe('GET /api/items filtering', () => {
expect(res.status).toBe(400);
});
it('rejects a category list with one unreadable entry rather than honouring the rest', async () => {
const furniture = await createCategory('Furniture');
await createItem('A', 1000, furniture);
// Filtering on the readable half would answer a narrower question than the
// one asked, and look indistinguishable from a filter that worked.
const res = await request(app).get(`/api/items?category=${furniture},furniture`);
expect(res.status).toBe(400);
});
it('rejects an inverted price range', async () => {
const res = await request(app).get('/api/items?min_price=5000&max_price=1000');
expect(res.status).toBe(400);
+39 -5
View File
@@ -3,7 +3,7 @@ import { parseItemFilters, FilterError, buildItemFilterSql } from '../../src/ite
describe('parseItemFilters', () => {
it('returns empty filters for an empty query', () => {
expect(parseItemFilters({})).toEqual({
categoryId: null,
categoryIds: [],
tagIds: [],
minPriceCents: null,
maxPriceCents: null,
@@ -12,8 +12,30 @@ describe('parseItemFilters', () => {
});
});
it('parses a category id', () => {
expect(parseItemFilters({ category: '7' }).categoryId).toBe(7);
// The parameter stayed singular when it went multi-valued (#139), so every
// link written before that still parses — as a list of one.
it('parses a single category id, as older links still send it', () => {
expect(parseItemFilters({ category: '7' }).categoryIds).toEqual([7]);
});
it('parses a comma-separated category list', () => {
expect(parseItemFilters({ category: '4,9,2' }).categoryIds).toEqual([4, 9, 2]);
});
it('collapses duplicate category ids', () => {
expect(parseItemFilters({ category: '3,3,8' }).categoryIds).toEqual([3, 8]);
});
it('treats an empty category list as no category filter', () => {
expect(parseItemFilters({ category: '' }).categoryIds).toEqual([]);
});
// Decision 9: a malformed filter shows itself rather than quietly returning
// the whole catalogue, and going multi-valued must not weaken that.
it('refuses a category list containing anything unreadable', () => {
expect(() => parseItemFilters({ category: '4,nope' })).toThrow(FilterError);
expect(() => parseItemFilters({ category: '0' })).toThrow(FilterError);
expect(() => parseItemFilters({ category: '-1,2' })).toThrow(FilterError);
});
it('parses a comma-separated tag list', () => {
@@ -158,7 +180,19 @@ describe('buildItemFilterSql', () => {
it('matches a category and all of its descendants', () => {
const built = buildItemFilterSql(parseItemFilters({ category: '4' }), 1, null);
expect(built.clauses.join(' ')).toContain('RECURSIVE');
expect(built.params).toEqual([4]);
// One array parameter rather than one id: the CTE is seeded with ANY so
// several selected roots are walked in the same recursion.
expect(built.params).toEqual([[4]]);
});
it('seeds the descendant walk with every selected category', () => {
const built = buildItemFilterSql(parseItemFilters({ category: '4,9' }), 1, null);
const sql = built.clauses.join(' ');
expect(sql).toContain('RECURSIVE');
// ANY over the seeds is what makes several categories combine as OR: the
// result is the union of their subtrees.
expect(sql).toContain('= ANY($1::int[])');
expect(built.params).toEqual([[4, 9]]);
});
it('requires every listed tag rather than any of them', () => {
@@ -215,7 +249,7 @@ describe('buildItemFilterSql', () => {
1,
null
);
expect(built.params).toEqual([4, 100, 900]);
expect(built.params).toEqual([[4], 100, 900]);
const sql = built.clauses.join(' ');
expect(sql).toContain('$1');
expect(sql).toContain('$2');
+10 -2
View File
@@ -65,8 +65,16 @@ export default function InventoryFilters({ categories, tags, filters, onChange,
aria-label="Filter by category"
style={{ minWidth: 200 }}
treeData={treeData}
value={filters.categoryId ?? undefined}
onChange={(value) => onChange({ ...filters, categoryId: value ?? null })}
// The filter shape went multi-valued for the storefront (#139). This
// control stays single-select — the admin asks "what is in this
// category", not "in any of these" — so it reads and writes a list of
// at most one rather than growing a second shape.
value={filters.categoryIds[0] ?? undefined}
// Annotated nullable because allowClear hands back undefined, which
// the control's own onChange type does not admit.
onChange={(value: number | undefined) =>
onChange({ ...filters, categoryIds: value === undefined ? [] : [value] })
}
/>
<Select
+10 -7
View File
@@ -28,17 +28,20 @@ export default function ActiveFilterChips({ options, filters, onChange, onClear
});
}
if (filters.categoryId !== null) {
const path = categoryPath(categories, filters.categoryId);
// One chip per selected category, each removable on its own — removing the
// whole set at once is what Clear all is for.
for (const categoryId of filters.categoryIds) {
const path = categoryPath(categories, categoryId);
// Falls back to the raw id while /api/filters is still loading, so the chip
// never renders as an empty box.
const label = path || `Category ${filters.categoryId}`;
const label = path || `Category ${categoryId}`;
chips.push({
key: `category-${filters.categoryId}`,
// The removable name is the leaf, matching what the user clicked in the
// tree, while the chip itself shows the full path for context.
key: `category-${categoryId}`,
// The chip shows the full path for context, since two categories can
// share a leaf name under different parents.
label,
onRemove: () => onChange({ ...filters, categoryId: null })
onRemove: () =>
onChange({ ...filters, categoryIds: filters.categoryIds.filter((id) => id !== categoryId) })
});
}
+61 -42
View File
@@ -1,6 +1,7 @@
import Drawer from 'antd/es/drawer';
import Button from 'antd/es/button';
import Tree from 'antd/es/tree';
import TreeSelect from 'antd/es/tree-select';
import Select from 'antd/es/select';
import Tag from 'antd/es/tag';
import Slider from 'antd/es/slider';
import InputNumber from 'antd/es/input-number';
@@ -47,19 +48,13 @@ export default function FilterDrawer({
const tags = options?.tags ?? [];
const bounds = options?.priceRange ?? { min_cents: 0, max_cents: 0 };
function toggleTag(tagId: number) {
const next = filters.tagIds.includes(tagId)
? filters.tagIds.filter((id) => id !== tagId)
: [...filters.tagIds, tagId];
onChange({ ...filters, tagIds: next });
function selectCategories(ids: number[]) {
onChange({ ...filters, categoryIds: ids });
}
// Selecting the already-selected node clears the filter, so the tree doubles
// as its own "all items" control.
function selectCategory(keys: React.Key[]) {
const picked = keys.length ? Number(keys[0]) : null;
onChange({ ...filters, categoryId: picked === filters.categoryId ? null : picked });
}
// The selected pills are rendered by the Select, which is handed ids rather
// than tags, so the colour has to be looked up rather than carried along.
const tagColors = new Map(tags.map((tag) => [tag.id, tag.color]));
const sliderMax = Math.max(bounds.max_cents, bounds.min_cents + 100);
@@ -106,15 +101,27 @@ export default function FilterDrawer({
<section style={{ marginBottom: 28 }}>
<h4 style={{ margin: '0 0 8px', fontSize: 12, letterSpacing: '.06em', textTransform: 'uppercase', opacity: 0.65 }}>
Category
Categories any of these
</h4>
{categories.length ? (
<Tree
// A TreeSelect rather than a Tree: it keeps the hierarchy a customer
// browses by while adding search and multi-select, and it lists what
// is chosen inside the control instead of leaving the selection to be
// read off highlighting. The admin's CategoryTreeSelect is the same
// control, so the two screens behave alike.
<TreeSelect
treeData={toTreeData(buildCategoryTree(categories))}
selectedKeys={filters.categoryId === null ? [] : [filters.categoryId]}
onSelect={selectCategory}
defaultExpandAll
blockNode
value={filters.categoryIds}
onChange={selectCategories}
multiple
showSearch
// Search the visible label, not the value, which is a numeric id.
treeNodeFilterProp="title"
treeDefaultExpandAll
allowClear
placeholder="Any category"
style={{ width: '100%' }}
aria-label="Filter by category"
/>
) : (
<Empty image={Empty.PRESENTED_IMAGE_SIMPLE} description="No categories yet" />
@@ -123,32 +130,44 @@ export default function FilterDrawer({
<section style={{ marginBottom: 28 }}>
<h4 style={{ margin: '0 0 8px', fontSize: 12, letterSpacing: '.06em', textTransform: 'uppercase', opacity: 0.65 }}>
Tags must have all
Tags must have all of these
</h4>
{tags.length ? (
<div style={{ display: 'flex', flexWrap: 'wrap', gap: 6 }}>
{tags.map((tag) => {
const selected = filters.tagIds.includes(tag.id);
return (
// A real button rather than a styled span, so the pills are
// reachable by keyboard and announce their on/off state.
<button
key={tag.id}
type="button"
aria-pressed={selected}
onClick={() => toggleTag(tag.id)}
style={{ background: 'none', border: 'none', padding: 0, cursor: 'pointer' }}
>
<Tag
color={selected ? tag.color : undefined}
style={{ margin: 0, opacity: selected ? 1 : 0.75 }}
>
{tag.name}
</Tag>
</button>
);
})}
</div>
// Was a wall of every tag in the system, which read fine at a dozen
// and not at a hundred. A searchable multi-select scales with the
// taxonomy and, like the category control above it, states its
// selection inside the control instead of in chip colouring.
//
// The colours survive as the selected pills, since that is the only
// place a tag's colour was ever load-bearing.
<Select
mode="multiple"
showSearch
optionFilterProp="label"
allowClear
placeholder="Any tags"
style={{ width: '100%' }}
aria-label="Filter by tags"
value={filters.tagIds}
onChange={(tagIds: number[]) => onChange({ ...filters, tagIds })}
options={tags.map((tag) => ({ value: tag.id, label: tag.name }))}
tagRender={({ value, label, closable, onClose }) => (
<Tag
color={tagColors.get(Number(value))}
closable={closable}
onClose={onClose}
// antd's default, which the custom renderer replaces: without
// it the pill swallows the mousedown and reopens the list.
onMouseDown={(event) => {
event.preventDefault();
event.stopPropagation();
}}
style={{ marginInlineEnd: 4 }}
>
{label}
</Tag>
)}
/>
) : (
<Empty image={Empty.PRESENTED_IMAGE_SIMPLE} description="No tags yet" />
)}
+19 -5
View File
@@ -3,7 +3,14 @@ import type { Category } from './api';
export type ItemStatus = 'pending' | 'available' | 'reserved' | 'sold';
export interface ItemFilters {
categoryId: number | null;
// Several categories, combined as OR — picking Furniture and Decor means
// either, not the empty intersection. Deliberately the opposite of tagIds
// below, which is AND, and both controls label their rule so the difference
// is stated rather than discovered.
//
// The admin's inventory filter is single-select and holds a list of one; the
// type is shared, and one shape is better than two that drift.
categoryIds: number[];
tagIds: number[];
minPriceCents: number | null;
maxPriceCents: number | null;
@@ -69,7 +76,7 @@ export function saleStateFromStatuses(
}
export const EMPTY_FILTERS: ItemFilters = {
categoryId: null,
categoryIds: [],
tagIds: [],
minPriceCents: null,
maxPriceCents: null,
@@ -82,7 +89,9 @@ export const EMPTY_FILTERS: ItemFilters = {
// what GET /api/items accepts, so the same object serializes for both.
export function filtersToSearchParams(filters: ItemFilters): URLSearchParams {
const params = new URLSearchParams();
if (filters.categoryId !== null) params.set('category', String(filters.categoryId));
// Comma-separated under the singular name it has always had, so a link
// written before this went multi-valued still means what it meant.
if (filters.categoryIds.length) params.set('category', filters.categoryIds.join(','));
if (filters.tagIds.length) params.set('tags', filters.tagIds.join(','));
if (filters.minPriceCents !== null) params.set('min_price', String(filters.minPriceCents));
if (filters.maxPriceCents !== null) params.set('max_price', String(filters.maxPriceCents));
@@ -98,6 +107,11 @@ function readInt(raw: string | null): number | null {
}
export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
const categories = (params.get('category') || '')
.split(',')
.map((part) => readInt(part))
.filter((id): id is number => id !== null && id > 0);
const tags = (params.get('tags') || '')
.split(',')
.map((part) => readInt(part))
@@ -127,7 +141,7 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
const favorites = params.get('favorites');
return {
categoryId: readInt(params.get('category')),
categoryIds: categories,
tagIds: tags,
minPriceCents: readInt(params.get('min_price')),
maxPriceCents: readInt(params.get('max_price')),
@@ -145,7 +159,7 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
// displays its own position.
export function activeFilterCount(filters: ItemFilters): number {
let count = 0;
if (filters.categoryId !== null) count++;
count += filters.categoryIds.length;
count += filters.tagIds.length;
if (filters.minPriceCents !== null || filters.maxPriceCents !== null) count++;
if (filters.favoritesOnly) count++;
+35
View File
@@ -89,6 +89,41 @@ test.describe('Storefront filters', () => {
await expect(storefront.card(NAMES.midItem)).toBeHidden();
});
test('several categories combine as any-of rather than all-of', async ({
page,
storefront,
filterDrawer
}) => {
await storefront.goto();
await storefront.openFilters();
// Different branches with no items in common, so an AND would show nothing.
await filterDrawer.chooseCategories(NAMES.tables, NAMES.decor);
await expect(storefront.card(NAMES.deepItem)).toBeVisible();
await expect(storefront.card(NAMES.otherItem)).toBeVisible();
// Filed directly in Furniture, which was not among the selections.
await expect(storefront.card(NAMES.midItem)).toBeHidden();
// Both ride in the one parameter the filter has always used, so links
// written before it went multi-valued still mean what they meant.
await expect(page).toHaveURL(/category=\d+,\d+/);
});
test('each selected category gets its own removable chip', async ({ storefront, filterDrawer }) => {
await storefront.goto();
await storefront.openFilters();
await filterDrawer.chooseCategories(NAMES.tables, NAMES.decor);
await filterDrawer.close();
await storefront.removeFilterChip(NAMES.decor).click();
// Removing one chip narrows the filter to the other rather than clearing
// the category filter outright.
await expect(storefront.card(NAMES.deepItem)).toBeVisible();
await expect(storefront.card(NAMES.otherItem)).toBeHidden();
});
test('requires every selected tag rather than any of them', async ({ storefront, filterDrawer }) => {
await storefront.goto();
await storefront.openFilters();
+52 -7
View File
@@ -3,13 +3,17 @@ import { Locator, Page, expect } from '@playwright/test';
/**
* The storefront's filter drawer.
*
* Categories are a tree rather than a list, because a category filter matches
* the node and everything filed beneath it — so the locator is `treeitem`, and
* a nested category is only reachable once the tree has loaded its data.
* Both taxonomy filters are searchable multi-selects whose options only exist
* while the list is open, so every choose/toggle here opens the list, acts, and
* closes it again.
*
* Tags are buttons that toggle. The rule they follow is AND, not OR: selecting
* two tags means "must have both", which is deliberately different from how
* categories combine and is the thing several tests exist to pin down.
* Categories keep their hierarchy — a category filter matches the node and
* everything filed beneath it — so their options are `treeitem`. Tags are flat,
* so theirs are `option`.
*
* The two follow opposite rules, which is the thing several tests exist to pin
* down: categories are OR (any of the selected branches), tags are AND (the
* item must carry all of them).
*/
export class FilterDrawer {
readonly minimumPrice: Locator;
@@ -17,6 +21,9 @@ export class FilterDrawer {
readonly closeButton: Locator;
readonly clearAllButton: Locator;
readonly favoritesOnlySwitch: Locator;
readonly categorySelect: Locator;
readonly tagSelect: Locator;
readonly title: Locator;
constructor(private readonly page: Page) {
this.minimumPrice = page.getByLabel('Minimum price');
@@ -24,6 +31,9 @@ export class FilterDrawer {
this.closeButton = page.getByRole('button', { name: 'Close' });
this.clearAllButton = page.getByRole('button', { name: 'Clear all' });
this.favoritesOnlySwitch = page.getByRole('switch', { name: 'Only my favorites' });
this.categorySelect = page.getByRole('combobox', { name: 'Filter by category' });
this.tagSelect = page.getByRole('combobox', { name: 'Filter by tags' });
this.title = page.getByRole('heading', { name: 'Filters' });
}
category(name: string): Locator {
@@ -31,15 +41,50 @@ export class FilterDrawer {
}
tag(name: string): Locator {
return this.page.getByRole('button', { name });
return this.page.getByRole('option', { name });
}
/**
* Opens the category list and leaves it open.
*
* Separate from choosing, so a test that picks two categories opens once —
* which is also what a customer does, the list staying open being the point
* of a multi-select.
*/
async openCategoryList(): Promise<void> {
await this.categorySelect.click();
}
/**
* Closes whichever option list is open, without closing the drawer.
*
* Escape would do both — the drawer listens for it too — so this clicks off
* the control instead, onto the one part of the drawer nothing overlaps.
*/
async closeOptionList(): Promise<void> {
await this.title.click();
}
async chooseCategory(name: string): Promise<void> {
await this.openCategoryList();
await this.category(name).click();
await this.closeOptionList();
}
/** Picks several categories from one opening of the list, as the UI intends. */
async chooseCategories(...names: string[]): Promise<void> {
await this.openCategoryList();
for (const name of names) {
await this.category(name).click();
}
await this.closeOptionList();
}
/** Clicking a selected option in a multi-select deselects it, so this toggles. */
async toggleTag(name: string): Promise<void> {
await this.tagSelect.click();
await this.tag(name).click();
await this.closeOptionList();
}
async setPriceRange(minimum?: string, maximum?: string): Promise<void> {