Merge pull request 'Feature/169 admin filter flyout' (#171) from feature/169-admin-filter-flyout into main
Linting / lint (push) Successful in 2m28s
SonarQube Analysis / sonarqube (push) Failing after 5m31s

Reviewed-on: #171
This commit was merged in pull request #171.
This commit is contained in:
2026-08-24 16:46:47 -05:00
13 changed files with 599 additions and 235 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');
+6 -2
View File
@@ -230,7 +230,8 @@ export default function App() {
Filters{activeCount ? ` (${activeCount})` : ''}
</Button>
<ActiveFilterChips
options={options}
categories={options?.categories ?? []}
tags={options?.tags ?? []}
filters={filters}
onChange={applyFilters}
onClear={clearFilters}
@@ -280,11 +281,14 @@ export default function App() {
<FilterDrawer
open={drawerOpen}
onClose={() => setDrawerOpen(false)}
options={options}
categories={options?.categories ?? []}
tags={options?.tags ?? []}
priceRange={options?.priceRange ?? null}
filters={filters}
onChange={applyFilters}
onClear={clearFilters}
resultCount={items.length}
showFavorites
/>
{/* The same prompt the heart button and Add to Cart use. Signing in
+1
View File
@@ -275,6 +275,7 @@ function Inventory() {
filters={filters}
onChange={applyFilters}
onClear={clearFilters}
resultCount={items.length}
/>
<Table rowKey="id" dataSource={items} columns={columns} scroll={{ x: true }} />
+57 -115
View File
@@ -1,40 +1,10 @@
import { useMemo } from 'react';
import TreeSelect from 'antd/es/tree-select';
import Select from 'antd/es/select';
import InputNumber from 'antd/es/input-number';
import { useState } from 'react';
import Button from 'antd/es/button';
import { FilterOutlined } from '@ant-design/icons';
import type { Category, Tag } from '../api';
import {
ItemFilters,
ItemStatus,
buildCategoryTree,
CategoryNode,
hasActiveFilters
} from '../filters';
// Named individually rather than grouped, because grouping is what the preset
// this replaces did. Pending is listed first: "what is waiting to be published"
// is the question that prompted #132.
const STATUS_OPTIONS: { value: ItemStatus; label: string }[] = [
{ value: 'pending', label: 'Pending' },
{ value: 'available', label: 'Available' },
{ value: 'reserved', label: 'Reserved' },
{ value: 'sold', label: 'Sold' }
];
interface CategoryTreeOption {
value: number;
title: string;
children?: CategoryTreeOption[];
}
function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
return nodes.map((node) => ({
value: node.id,
title: node.name,
children: node.children.length ? toTreeData(node.children) : undefined
}));
}
import { ItemFilters, activeFilterCount } from '../filters';
import FilterDrawer from '../components/FilterDrawer';
import ActiveFilterChips from '../components/ActiveFilterChips';
type Props = Readonly<{
categories: Category[];
@@ -42,95 +12,67 @@ type Props = Readonly<{
filters: ItemFilters;
onChange: (filters: ItemFilters) => void;
onClear: () => void;
resultCount: number;
}>;
const centsToDollars = (cents: number | null): number | null => (cents === null ? null : cents / 100);
const dollarsToCents = (dollars: number | null): number | null =>
dollars === null || Number.isNaN(dollars) ? null : Math.round(dollars * 100);
// The same flyout the storefront uses, rather than the row of controls this
// used to be (#169).
//
// That row was deliberate — it carried a comment arguing that hiding the
// controls above a data table costs more than the space it saves, and that a
// drawer overlays the very rows being filtered. Both are true, and both are
// traded for the two screens asking the same questions through the same UI.
// The chips are what makes the trade bearable: the active filter stays readable
// beside the button without opening anything, which is what the always-visible
// row was really protecting.
export default function InventoryFilters({
categories,
tags,
filters,
onChange,
onClear,
resultCount
}: Props) {
const [drawerOpen, setDrawerOpen] = useState(false);
// An always-visible row rather than the storefront's drawer: this sits above a
// data table, where hiding the controls behind a click costs more than the
// space it saves, and a drawer would overlay the very rows being filtered.
export default function InventoryFilters({ categories, tags, filters, onChange, onClear }: Props) {
const treeData = useMemo(() => toTreeData(buildCategoryTree(categories)), [categories]);
// Status lives in the drawer here, unlike on the storefront, so it belongs in
// the button's tally — activeFilterCount leaves it out precisely because the
// storefront filters status outside the drawer.
const activeCount = activeFilterCount(filters) + (filters.status === null ? 0 : 1);
return (
<div className="inventory-filters">
<TreeSelect
allowClear
showSearch
treeNodeFilterProp="title"
listHeight={256}
placeholder="Any category"
aria-label="Filter by category"
style={{ minWidth: 200 }}
treeData={treeData}
value={filters.categoryId ?? undefined}
onChange={(value) => onChange({ ...filters, categoryId: value ?? null })}
<Button
icon={<FilterOutlined />}
onClick={() => setDrawerOpen(true)}
type={activeCount ? 'primary' : 'default'}
>
Filters{activeCount ? ` (${activeCount})` : ''}
</Button>
<ActiveFilterChips
categories={categories}
tags={tags}
filters={filters}
onChange={onChange}
onClear={onClear}
showStatus
/>
<Select
allowClear
mode="multiple"
placeholder="Any tags"
aria-label="Filter by tags"
style={{ minWidth: 200 }}
value={filters.tagIds}
onChange={(value: number[]) => onChange({ ...filters, tagIds: value })}
options={tags.map((tag) => ({ value: tag.id, label: tag.name }))}
<FilterDrawer
open={drawerOpen}
onClose={() => setDrawerOpen(false)}
categories={categories}
tags={tags}
// No slider: the admin has no catalogue-wide price range to bound one
// with, and inventing bounds would misreport where the prices are.
priceRange={null}
filters={filters}
onChange={onChange}
onClear={onClear}
resultCount={resultCount}
showStatus
/>
<InputNumber
aria-label="Minimum price"
prefix="$"
min={0}
placeholder="Min"
style={{ width: 110 }}
value={centsToDollars(filters.minPriceCents)}
onChange={(value) => onChange({ ...filters, minPriceCents: dollarsToCents(value) })}
/>
<span style={{ opacity: 0.6 }}>to</span>
<InputNumber
aria-label="Maximum price"
prefix="$"
min={0}
placeholder="Max"
style={{ width: 110 }}
value={centsToDollars(filters.maxPriceCents)}
onChange={(value) => onChange({ ...filters, maxPriceCents: dollarsToCents(value) })}
/>
{/* The status dimension itself rather than presets over it, which #105's
Sold / Not sold / All control was. Presets could not express Published
or Unpublished, could not isolate Reserved, and would have grown a new
button for every new question. Selecting statuses answers all of them:
Unpublished is Pending, Published is the other three, and Not sold is
everything except Sold.
A second control for publication would have read more naturally and
reintroduced what #105 avoided — Sold and Unpublished is an impossible
pair, since a sold item is necessarily published. One dimension cannot
contradict itself. See #132.
The storefront keeps the three-way preset: pending is excluded from
every public read, so Published and Unpublished are not distinctions a
customer can draw. */}
<Select
allowClear
mode="multiple"
placeholder="Any status"
aria-label="Filter by status"
style={{ minWidth: 220 }}
value={filters.status ?? []}
onChange={(value: ItemStatus[]) =>
// Empty means no filter, not "no statuses". A multi-select cleared
// back to nothing should show everything rather than an empty table.
onChange({ ...filters, status: value.length ? value : null })
}
options={STATUS_OPTIONS}
/>
{hasActiveFilters(filters) && <Button onClick={onClear}>Clear filters</Button>}
</div>
);
}
+41 -14
View File
@@ -1,21 +1,30 @@
import Tag from 'antd/es/tag';
import Button from 'antd/es/button';
import type { FilterOptions } from '../api';
import { ItemFilters, categoryPath, formatPriceRange, hasActiveFilters } from '../filters';
import type { Category, Tag as ItemTag } from '../api';
import { ItemFilters, categoryPath, formatPriceRange, hasActiveFilters, statusLabel } from '../filters';
type Props = Readonly<{
options: FilterOptions | null;
categories: Category[];
tags: ItemTag[];
filters: ItemFilters;
onChange: (filters: ItemFilters) => void;
onClear: () => void;
// Where status is one of the drawer's controls rather than a preset beside
// it (#169), it needs a chip too — otherwise the one filter most likely to
// empty a table is the one filter invisible without opening the drawer.
showStatus?: boolean;
}>;
export default function ActiveFilterChips({ options, filters, onChange, onClear }: Props) {
export default function ActiveFilterChips({
categories,
tags,
filters,
onChange,
onClear,
showStatus = false
}: Props) {
if (!hasActiveFilters(filters)) return null;
const categories = options?.categories ?? [];
const tags = options?.tags ?? [];
const chips: { key: string; label: string; onRemove: () => void }[] = [];
// Listed first so it matches the drawer's ordering, and because it is the
@@ -28,17 +37,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) })
});
}
@@ -51,6 +63,21 @@ export default function ActiveFilterChips({ options, filters, onChange, onClear
});
}
if (showStatus && filters.status !== null) {
for (const status of filters.status) {
chips.push({
key: `status-${status}`,
label: statusLabel(status),
onRemove: () => {
const rest = (filters.status ?? []).filter((value) => value !== status);
// Back to null rather than an empty list: emptying the control means
// "no status filter", not "no statuses", which would empty the table.
onChange({ ...filters, status: rest.length ? rest : null });
}
});
}
}
if (filters.minPriceCents !== null || filters.maxPriceCents !== null) {
chips.push({
key: 'price',
+147 -55
View File
@@ -1,34 +1,67 @@
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';
import Empty from 'antd/es/empty';
import Switch from 'antd/es/switch';
import Grid from 'antd/es/grid';
import type { DataNode } from 'antd/es/tree';
import type { FilterOptions } from '../api';
import { ItemFilters, buildCategoryTree, CategoryNode } from '../filters';
import type { Category, Tag as ItemTag } from '../api';
import { ItemFilters, ItemStatus, STATUS_OPTIONS, buildCategoryTree, CategoryNode } from '../filters';
// One drawer for the storefront and the admin, with the sections that differ
// driven by props rather than by a second component that would drift (#169).
// What is shared is not just the markup but the phrasing of the rules — that
// categories are OR and tags are AND has to read the same on both screens or it
// stops being one rule.
type Props = Readonly<{
open: boolean;
onClose: () => void;
options: FilterOptions | null;
categories: Category[];
tags: ItemTag[];
// Bounds for the price slider, or null on a screen with no catalogue-wide
// range to draw one from, where the two number inputs stand alone. A slider
// needs real bounds: invented ones would misreport where the prices are.
priceRange: { min_cents: number; max_cents: number } | null;
filters: ItemFilters;
onChange: (filters: ItemFilters) => void;
onClear: () => void;
resultCount: number;
// Storefront only — signing in is what makes favorites mean anything.
showFavorites?: boolean;
// Admin only. The storefront keeps its three-way preset outside the drawer:
// pending is excluded from every public read, so Published and Unpublished
// are not distinctions a customer can draw.
showStatus?: boolean;
}>;
function toTreeData(nodes: CategoryNode[]): DataNode[] {
// `value` rather than `key`: this fed an antd `Tree`, which identifies nodes by
// key, and now feeds a `TreeSelect`, which selects and searches by value. The
// shape matches the admin's CategoryTreeSelect so the two stay comparable.
interface CategoryTreeOption {
value: number;
title: string;
children?: CategoryTreeOption[];
}
function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
return nodes.map((node) => ({
key: node.id,
value: node.id,
title: node.name,
children: node.children.length ? toTreeData(node.children) : undefined
}));
}
const sectionHeading: React.CSSProperties = {
margin: '0 0 8px',
fontSize: 12,
letterSpacing: '.06em',
textTransform: 'uppercase',
opacity: 0.65
};
const centsToDollars = (cents: number | null): number | null => (cents === null ? null : cents / 100);
const dollarsToCents = (dollars: number | null): number | null =>
dollars === null || Number.isNaN(dollars) ? null : Math.round(dollars * 100);
@@ -36,30 +69,26 @@ const dollarsToCents = (dollars: number | null): number | null =>
export default function FilterDrawer({
open,
onClose,
options,
categories,
tags,
priceRange,
filters,
onChange,
onClear,
resultCount
resultCount,
showFavorites = false,
showStatus = false
}: Props) {
const screens = Grid.useBreakpoint();
const categories = options?.categories ?? [];
const tags = options?.tags ?? [];
const bounds = options?.priceRange ?? { min_cents: 0, max_cents: 0 };
const bounds = 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);
@@ -86,8 +115,9 @@ export default function FilterDrawer({
here for their favorites should not have to scroll past the catalogue
controls to find it. Shown to signed-out visitors too: switching it on
prompts them to sign in, which is how they learn favorites exist. */}
{showFavorites && (
<section style={{ marginBottom: 28 }}>
<h4 style={{ margin: '0 0 8px', fontSize: 12, letterSpacing: '.06em', textTransform: 'uppercase', opacity: 0.65 }}>
<h4 style={sectionHeading}>
Favorites
</h4>
{/* Deliberately not wrapped in a <label>: antd renders the switch as a
@@ -103,18 +133,31 @@ export default function FilterDrawer({
<span>Only my favorites</span>
</div>
</section>
)}
<section style={{ marginBottom: 28 }}>
<h4 style={{ margin: '0 0 8px', fontSize: 12, letterSpacing: '.06em', textTransform: 'uppercase', opacity: 0.65 }}>
Category
<h4 style={sectionHeading}>
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" />
@@ -122,42 +165,55 @@ export default function FilterDrawer({
</section>
<section style={{ marginBottom: 28 }}>
<h4 style={{ margin: '0 0 8px', fontSize: 12, letterSpacing: '.06em', textTransform: 'uppercase', opacity: 0.65 }}>
Tags must have all
<h4 style={sectionHeading}>
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' }}
>
// 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={selected ? tag.color : undefined}
style={{ margin: 0, opacity: selected ? 1 : 0.75 }}
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 }}
>
{tag.name}
{label}
</Tag>
</button>
);
})}
</div>
)}
/>
) : (
<Empty image={Empty.PRESENTED_IMAGE_SIMPLE} description="No tags yet" />
)}
</section>
<section>
<h4 style={{ margin: '0 0 8px', fontSize: 12, letterSpacing: '.06em', textTransform: 'uppercase', opacity: 0.65 }}>
<section style={showStatus ? { marginBottom: 28 } : undefined}>
<h4 style={sectionHeading}>
Price
</h4>
{priceRange && (
<Slider
range
min={bounds.min_cents}
@@ -177,6 +233,7 @@ export default function FilterDrawer({
})
}
/>
)}
<div style={{ display: 'flex', alignItems: 'center', gap: 8, marginTop: 8 }}>
<InputNumber
aria-label="Minimum price"
@@ -197,6 +254,41 @@ export default function FilterDrawer({
/>
</div>
</section>
{showStatus && (
<section>
<h4 style={sectionHeading}>
Status any of these
</h4>
{/* The status dimension itself rather than presets over it, which
#105's Sold / Not sold / All control was. Presets could not express
Published or Unpublished, could not isolate Reserved, and would
have grown a new button for every new question. Selecting statuses
answers all of them: Unpublished is Pending, Published is the other
three, and Not sold is everything except Sold.
A second control for publication would have read more naturally and
reintroduced what #105 avoided — Sold and Unpublished is an
impossible pair, since a sold item is necessarily published. One
dimension cannot contradict itself. See #132. */}
<Select
allowClear
mode="multiple"
showSearch
optionFilterProp="label"
placeholder="Any status"
aria-label="Filter by status"
style={{ width: '100%' }}
value={filters.status ?? []}
onChange={(value: ItemStatus[]) =>
// Empty means no filter, not "no statuses". A multi-select cleared
// back to nothing should show everything rather than an empty table.
onChange({ ...filters, status: value.length ? value : null })
}
options={STATUS_OPTIONS}
/>
</section>
)}
</Drawer>
);
}
+37 -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')),
@@ -143,9 +157,27 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
// 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.
// Named individually rather than grouped, because grouping is what the preset
// this replaced did. Pending is listed first: "what is waiting to be published"
// is the question that prompted #132.
//
// Here rather than in the admin screen because the drawer and the active-filter
// chips both need to turn a status into a label, and a second copy of this list
// is a second place for a new status to be forgotten.
export const STATUS_OPTIONS: { value: ItemStatus; label: string }[] = [
{ value: 'pending', label: 'Pending' },
{ value: 'available', label: 'Available' },
{ value: 'reserved', label: 'Reserved' },
{ value: 'sold', label: 'Sold' }
];
export function statusLabel(status: ItemStatus): string {
return STATUS_OPTIONS.find((option) => option.value === status)?.label ?? status;
}
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++;
@@ -43,8 +43,7 @@ test.describe('Admin inventory filters', () => {
await admin.goto();
await adminInventory.filterByCategory(NAMES.category, NAMES.cheap);
await adminInventory.minimumPrice.fill('100');
await adminInventory.maximumPrice.fill('500');
await adminInventory.setPriceRange('100', '500');
await expect(adminInventory.row(NAMES.mid)).toBeVisible();
await expect(adminInventory.row(NAMES.cheap)).toHaveCount(0);
@@ -111,16 +110,20 @@ test.describe('Admin inventory filters', () => {
test('combines filters, and clearing restores them', async ({ admin, adminInventory }) => {
await admin.goto();
await adminInventory.filterByCategory(NAMES.category, NAMES.cheap);
await adminInventory.minimumPrice.fill('800');
await adminInventory.setPriceRange('800');
await expect(adminInventory.row(NAMES.cheap)).toHaveCount(0);
await expect(adminInventory.row(NAMES.dear)).toBeVisible();
await adminInventory.clearFiltersButton.click();
await adminInventory.clearFilters();
// Asserting on the controls rather than on the rows: with the filters gone
// the table is the whole paginated catalogue again, so a given fixture is
// not reliably on the first page.
//
// The chip row renders only while something is filtered, and the button's
// tally is the other half of the same claim — nothing is filtered, and the
// screen says so without the drawer being opened to check.
await expect(adminInventory.clearFiltersButton).toHaveCount(0);
await expect(adminInventory.minimumPrice).toHaveValue('');
await expect(adminInventory.filtersButton).toHaveText('Filters');
});
});
+40
View File
@@ -89,6 +89,46 @@ 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.
//
// The separator arrives percent-encoded because URLSearchParams encodes a
// comma, which is how the `tags` parameter has always looked too. Either
// spelling parses, so the assertion accepts both rather than pinning the
// encoding.
await expect(page).toHaveURL(/category=\d+(,|%2C)\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();
+62 -12
View File
@@ -81,6 +81,31 @@ export class AdminInventory {
}
// ---- The inventory filter bar ----
//
// The controls moved into the storefront's flyout (#169), so each method here
// opens the drawer, acts, and closes it again. Closing matters: the drawer
// overlays the table, and every assertion in these specs is about rows.
get filtersButton(): Locator {
return this.page.getByRole('button', { name: 'Filters' });
}
get filterDrawer(): Locator {
return this.page.getByRole('dialog', { name: 'Filters' });
}
/**
* The chip row's own "Clear all", scoped to the group so it stays distinct
* from the identically-labelled button in the drawer's footer.
*
* It exists only while something is filtered, which is what lets a spec assert
* that clearing worked by its absence.
*/
get clearFiltersButton(): Locator {
return this.page
.getByRole('group', { name: 'Active filters' })
.getByRole('button', { name: 'Clear all' });
}
get categoryFilter(): Locator {
return this.page.getByRole('combobox', { name: 'Filter by category' });
@@ -98,30 +123,37 @@ export class AdminInventory {
return this.page.getByLabel('Maximum price');
}
get clearFiltersButton(): Locator {
return this.page.getByRole('button', { name: 'Clear filters' });
/** Opens the flyout, or leaves it open if it already is. */
async openFilters(): Promise<void> {
if (await this.filterDrawer.isVisible().catch(() => false)) return;
await this.filtersButton.click();
await expect(this.filterDrawer).toBeVisible();
}
/** Closes it through the footer button, which is what a person would click. */
async closeFilters(): Promise<void> {
await this.filterDrawer.getByRole('button', { name: /^Show / }).click();
await expect(this.filterDrawer).toBeHidden();
}
/**
* Toggles one status in the multi-select. Clicking a selected option removes
* it, which is what the clearing test relies on.
*
* Two antd details decide this locator. It renders an invisible role="listbox"
* shim beside the real list for accessibility, so getByRole('option') finds
* something zero-sized that cannot be clicked. And once a status is selected
* it also renders as a tag carrying the same title as the option, so an
* unscoped getByTitle becomes ambiguous. Matching the visible option class
* avoids both.
*
* The dropdown is opened only when it is not already open: antd keeps it open
* after a selection in multiple mode, so clicking the box again would close it.
* The option is matched by class rather than by role because antd renders an
* invisible role="listbox" shim beside the real list for accessibility, so
* getByRole('option') finds something zero-sized that cannot be clicked; and
* once a status is selected it also renders as a tag carrying the same title,
* so an unscoped getByTitle becomes ambiguous.
*/
async toggleStatus(label: string): Promise<void> {
await this.openFilters();
const option = this.page.locator(`.ant-select-item-option[title="${label}"]`);
if (!(await option.isVisible().catch(() => false))) {
await this.statusFilter.click();
}
await option.click();
await this.closeFilters();
}
/**
@@ -131,14 +163,32 @@ export class AdminInventory {
* unfiltered page 1 is not a reliable place to look for a fixture. Waiting for
* a known row is the action's contract — the filter has been applied when the
* table has re-rendered under it.
*
* Typing the name before clicking it is not for realism: the tree is
* virtualized, so against a database holding hundreds of categories the wanted
* row is never rendered until a search narrows to it.
*/
async filterByCategory(categoryName: string, expectedRow: string): Promise<void> {
await this.openFilters();
await this.categoryFilter.click();
await this.categoryFilter.fill(categoryName);
await this.page.getByTitle(categoryName, { exact: true }).click();
await this.page.getByRole('treeitem', { name: categoryName }).click();
await this.closeFilters();
await expect(this.row(expectedRow)).toBeVisible();
}
/** Sets either end of the price range, leaving an omitted end untouched. */
async setPriceRange(minimum?: string, maximum?: string): Promise<void> {
await this.openFilters();
if (minimum !== undefined) await this.minimumPrice.fill(minimum);
if (maximum !== undefined) await this.maximumPrice.fill(maximum);
await this.closeFilters();
}
async clearFilters(): Promise<void> {
await this.clearFiltersButton.click();
}
async openItemForm(): Promise<void> {
await this.addItemButton.click();
}
+76 -8
View File
@@ -3,13 +3,26 @@ 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.
* Each one types the name before clicking it. Not for realism — the lists are
* virtualized, so against a database holding hundreds of categories the wanted
* row is never rendered until a search narrows to it. Scrolling to it would be
* testing the virtual list rather than the filter.
*
* Categories keep their hierarchy — a category filter matches the node and
* everything filed beneath it — so their options are `treeitem`.
*
* Tags are flat, and their options are matched by class rather than by role,
* for the reason AdminInventory.toggleStatus records: antd renders an invisible
* role="listbox" shim beside the real list, so getByRole('option') resolves to
* something zero-sized that can never be clicked.
*
* 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 +30,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 +40,11 @@ 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' });
// Somewhere inside the drawer that is inert and never covered by an
// option list, which opens downward from the controls below it.
this.title = page.getByRole('heading', { name: 'Favorites' });
}
category(name: string): Locator {
@@ -31,15 +52,62 @@ export class FilterDrawer {
}
tag(name: string): Locator {
return this.page.getByRole('button', { name });
return this.page.locator(`.ant-select-item-option[title="${name}"]`);
}
async chooseCategory(name: string): Promise<void> {
/**
* 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();
}
/**
* Narrows to one category and clicks it, with the list already open.
*
* Selecting clears the search box, so consecutive calls each start from the
* unfiltered list.
*/
private async pickCategory(name: string): Promise<void> {
await this.categorySelect.fill(name);
await this.category(name).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.pickCategory(name);
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.pickCategory(name);
}
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.tagSelect.fill(name);
await this.tag(name).click();
await this.closeOptionList();
}
async setPriceRange(minimum?: string, maximum?: string): Promise<void> {