feat(admin): filter inventory by status directly, so Published and Unpublished are reachable (#132)
There was no way to find unpublished items. Every item has arrived pending since #90 and has to be published, so "what is waiting for me to publish" is a routine question the inventory could not answer. #105 replaced the four-way status dropdown with a Sold / Not sold / All preset and recorded at the time that this gave up isolating a single status, that the pending workflow was the likeliest thing to miss it, and that the fix would be to restore the ability rather than remove the preset. That turned out to be right, and sooner than expected. The admin now selects statuses directly - Pending, Available, Reserved, Sold - rather than choosing among presets over them. The API has accepted several statuses since #105, so this exposes the dimension itself. Everything becomes expressible in one control: Unpublished is Pending, Published is the other three, Sold and Not sold are the sets they always were, and Reserved on its own is reachable again. Two alternatives were rejected. Growing the preset list to five would have kept one click per answer while leaving Reserved unreachable and growing again at the next new question. A second control for publication beside the one for availability would have read more naturally and reintroduced exactly what #105 was built to avoid: Sold and Unpublished is an impossible pair, since a sold item is necessarily published, and two dimensions have to either give that a meaning or block it. One dimension cannot contradict itself. The storefront keeps its three-way preset unchanged. Pending is excluded from every public read, so Published and Unpublished are not distinctions a customer can draw, and the simpler control is the right one there. An empty selection means no filter rather than no statuses, or clearing the box would empty the table. Verification: the admin filter spec is rewritten rather than deleted, and now asserts what the preset could not - Pending alone finds the staged fixture and hides the published ones, and Available plus Reserved plus Sold finds the published ones and hides the staged one. That second case is the one a preset would have had to be invented for. All 7 admin filter tests pass, along with 122 of the suite. Two locator details cost time and are written into the spec so they do not have to be rediscovered: antd renders an invisible role="listbox" shim beside the real option list, so getByRole('option') resolves something zero-sized that cannot be clicked; and a selected status renders as a tag carrying the same title as its option, so an unscoped getByTitle becomes ambiguous once anything is chosen. Beyond the two pre-existing password-reset failures that need a database on port 55432, two storefront specs failed under the full concurrent run and pass six-for-six in isolation, twice. That is the shared-database contention filed as #116, not a regression here: this change touches the admin only. Closes #132 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,20 +1,27 @@
|
|||||||
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 {
|
import {
|
||||||
ItemFilters,
|
ItemFilters,
|
||||||
SaleState,
|
ItemStatus,
|
||||||
ADMIN_SALE_STATUSES,
|
|
||||||
buildCategoryTree,
|
buildCategoryTree,
|
||||||
CategoryNode,
|
CategoryNode,
|
||||||
hasActiveFilters,
|
hasActiveFilters
|
||||||
saleStateFromStatuses
|
|
||||||
} from '../filters';
|
} 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 {
|
interface CategoryTreeOption {
|
||||||
value: number;
|
value: number;
|
||||||
title: string;
|
title: string;
|
||||||
@@ -93,32 +100,34 @@ export default function InventoryFilters({ categories, tags, filters, onChange,
|
|||||||
onChange={(value) => onChange({ ...filters, maxPriceCents: dollarsToCents(value) })}
|
onChange={(value) => onChange({ ...filters, maxPriceCents: dollarsToCents(value) })}
|
||||||
/>
|
/>
|
||||||
|
|
||||||
{/* Replaces the four-way status dropdown that used to sit here. One
|
{/* The status dimension itself rather than presets over it, which #105's
|
||||||
control instead of two overlapping ways to say the same thing.
|
Sold / Not sold / All control was. Presets could not express Published
|
||||||
Note what it costs: a single status can no longer be isolated, so
|
or Unpublished, could not isolate Reserved, and would have grown a new
|
||||||
there is no longer a way to view only Reserved, or only Pending.
|
button for every new question. Selecting statuses answers all of them:
|
||||||
Not Sold folds pending in with available and reserved. If isolating
|
Unpublished is Pending, Published is the other three, and Not sold is
|
||||||
one status turns out to matter — the pending workflow from #90 is the
|
everything except Sold.
|
||||||
likeliest candidate — the fix is to put that back alongside this
|
|
||||||
preset, not to remove it. See the decision recorded on #105. */}
|
A second control for publication would have read more naturally and
|
||||||
<Segmented
|
reintroduced what #105 avoided — Sold and Unpublished is an impossible
|
||||||
aria-label="Filter by availability"
|
pair, since a sold item is necessarily published. One dimension cannot
|
||||||
value={saleStateFromStatuses(filters.status, ADMIN_SALE_STATUSES, 'all')}
|
contradict itself. See #132.
|
||||||
onChange={(value) => {
|
|
||||||
const state = value as SaleState;
|
The storefront keeps the three-way preset: pending is excluded from
|
||||||
onChange({
|
every public read, so Published and Unpublished are not distinctions a
|
||||||
...filters,
|
customer can draw. */}
|
||||||
// All is the admin's default, so it is held as "no preference"
|
<Select
|
||||||
// rather than as a list naming every status — which keeps it out of
|
allowClear
|
||||||
// the active-filter count and out of Clear filters' way.
|
mode="multiple"
|
||||||
status: state === 'all' ? null : ADMIN_SALE_STATUSES[state]
|
placeholder="Any status"
|
||||||
});
|
aria-label="Filter by status"
|
||||||
}}
|
style={{ minWidth: 220 }}
|
||||||
options={[
|
value={filters.status ?? []}
|
||||||
{ label: 'Not sold', value: 'not-sold' },
|
onChange={(value: ItemStatus[]) =>
|
||||||
{ label: 'Sold', value: 'sold' },
|
// Empty means no filter, not "no statuses". A multi-select cleared
|
||||||
{ label: 'All', value: 'all' }
|
// 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>}
|
{hasActiveFilters(filters) && <Button onClick={onClear}>Clear filters</Button>}
|
||||||
|
|||||||
@@ -38,14 +38,12 @@ export const STOREFRONT_SALE_STATUSES: Record<SaleState, ItemStatus[]> = {
|
|||||||
all: ['available', 'reserved', 'sold']
|
all: ['available', 'reserved', 'sold']
|
||||||
};
|
};
|
||||||
|
|
||||||
// In the admin, Not Sold includes pending: an item awaiting publication has
|
// The admin had a table of its own here until #132, where the preset was
|
||||||
// certainly not been sold, and hiding it from the default view would hide the
|
// replaced by a multi-select of the statuses themselves. Presets could not
|
||||||
// items most likely to need attention.
|
// express Published or Unpublished and could not isolate a single status, and
|
||||||
export const ADMIN_SALE_STATUSES: Record<SaleState, ItemStatus[]> = {
|
// the admin is where those questions get asked. The storefront keeps its
|
||||||
'not-sold': ['available', 'reserved', 'pending'],
|
// preset: pending never reaches a customer, so the distinction does not exist
|
||||||
sold: ['sold'],
|
// for them.
|
||||||
all: ['available', 'reserved', 'sold', 'pending']
|
|
||||||
};
|
|
||||||
|
|
||||||
function isPublicStatus(value: string): value is ItemStatus {
|
function isPublicStatus(value: string): value is ItemStatus {
|
||||||
return value === 'available' || value === 'reserved' || value === 'sold';
|
return value === 'available' || value === 'reserved' || value === 'sold';
|
||||||
|
|||||||
@@ -7,7 +7,8 @@ const NAMES = {
|
|||||||
tag: `filt-${RUN}`,
|
tag: `filt-${RUN}`,
|
||||||
cheap: `Cheap item ${RUN}`,
|
cheap: `Cheap item ${RUN}`,
|
||||||
mid: `Mid item ${RUN}`,
|
mid: `Mid item ${RUN}`,
|
||||||
dear: `Dear item ${RUN}`
|
dear: `Dear item ${RUN}`,
|
||||||
|
staged: `Staged item ${RUN}`
|
||||||
};
|
};
|
||||||
|
|
||||||
test.beforeAll(async ({ playwright }) => {
|
test.beforeAll(async ({ playwright }) => {
|
||||||
@@ -20,7 +21,7 @@ test.beforeAll(async ({ playwright }) => {
|
|||||||
|
|
||||||
// Published after creation: new items are pending, and these fixtures stand
|
// Published after creation: new items are pending, and these fixtures stand
|
||||||
// in for ordinary stock rather than staged drafts.
|
// in for ordinary stock rather than staged drafts.
|
||||||
const item = async (name: string, price: string, inCategory: boolean) => {
|
const item = async (name: string, price: string, inCategory: boolean, publish = true) => {
|
||||||
const res = await api.post('/api/admin/items', {
|
const res = await api.post('/api/admin/items', {
|
||||||
multipart: {
|
multipart: {
|
||||||
name,
|
name,
|
||||||
@@ -30,23 +31,38 @@ test.beforeAll(async ({ playwright }) => {
|
|||||||
tags: JSON.stringify(inCategory ? [NAMES.tag] : [])
|
tags: JSON.stringify(inCategory ? [NAMES.tag] : [])
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`);
|
if (publish) {
|
||||||
|
await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`);
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
await item(NAMES.cheap, '50', true);
|
await item(NAMES.cheap, '50', true);
|
||||||
await item(NAMES.mid, '150', true);
|
await item(NAMES.mid, '150', true);
|
||||||
await item(NAMES.dear, '900', true);
|
await item(NAMES.dear, '900', true);
|
||||||
|
// Left pending on purpose: the Unpublished filter needs something to find,
|
||||||
|
// and every other fixture here is published.
|
||||||
|
await item(NAMES.staged, '400', true, false);
|
||||||
|
|
||||||
await api.dispose();
|
await api.dispose();
|
||||||
});
|
});
|
||||||
|
|
||||||
// antd Segmented hides the real radio input behind a styled label, so the input
|
// Toggles one status in the multi-select. Clicking an option that is already
|
||||||
// is found by role but cannot be clicked. The label carries a title attribute,
|
// selected removes it, which is what the clearing test relies on.
|
||||||
// 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
|
// Two antd details decide this locator. It renders an invisible role="listbox"
|
||||||
// does not require visibility.
|
// shim beside the real list for accessibility, so getByRole('option') finds
|
||||||
async function chooseAvailability(page: Page, label: string) {
|
// something zero-sized that cannot be clicked. And once a status is selected it
|
||||||
await page.getByTitle(label, { exact: true }).click();
|
// 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.
|
||||||
|
async function chooseStatus(page: Page, label: string) {
|
||||||
|
const option = page.locator(`.ant-select-item-option[title="${label}"]`);
|
||||||
|
if (!(await option.isVisible().catch(() => false))) {
|
||||||
|
await page.getByRole('combobox', { name: 'Filter by status' }).click();
|
||||||
|
}
|
||||||
|
await option.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 });
|
||||||
@@ -86,28 +102,61 @@ test.describe('Admin inventory filters', () => {
|
|||||||
await expect(row(page, NAMES.dear)).toHaveCount(0);
|
await expect(row(page, NAMES.dear)).toHaveCount(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
// The four-way status dropdown is gone, replaced by the Sold / Not sold / All
|
test('filters by a single status', async ({ page }) => {
|
||||||
// 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 chooseAvailability(page, 'Sold');
|
await chooseStatus(page, 'Sold');
|
||||||
|
|
||||||
// Every fixture is available, so a Sold filter must exclude them all.
|
// Every fixture is published and unsold, so a Sold filter excludes 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);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The question that prompted #132. Every item arrives pending since #90, so
|
||||||
|
// "what is waiting for me to publish" is routine, and the preset this control
|
||||||
|
// replaced could not ask it.
|
||||||
|
test('finds unpublished items, and only those', async ({ page }) => {
|
||||||
|
await page.goto('/admin');
|
||||||
|
await filterToOwnCategory(page);
|
||||||
|
|
||||||
|
await chooseStatus(page, 'Pending');
|
||||||
|
|
||||||
|
await expect(row(page, NAMES.staged)).toBeVisible();
|
||||||
|
await expect(row(page, NAMES.cheap)).toHaveCount(0);
|
||||||
|
await expect(row(page, NAMES.mid)).toHaveCount(0);
|
||||||
|
await expect(row(page, NAMES.dear)).toHaveCount(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The complement, and the case a two-way preset could not express either:
|
||||||
|
// published means three statuses at once, not one and not "everything else".
|
||||||
|
test('finds published items by selecting several statuses at once', async ({ page }) => {
|
||||||
|
await page.goto('/admin');
|
||||||
|
await filterToOwnCategory(page);
|
||||||
|
|
||||||
|
await chooseStatus(page, 'Available');
|
||||||
|
await chooseStatus(page, 'Reserved');
|
||||||
|
await chooseStatus(page, 'Sold');
|
||||||
|
|
||||||
// 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.cheap)).toBeVisible();
|
||||||
await expect(row(page, NAMES.mid)).toBeVisible();
|
await expect(row(page, NAMES.mid)).toBeVisible();
|
||||||
await expect(row(page, NAMES.dear)).toBeVisible();
|
await expect(row(page, NAMES.dear)).toBeVisible();
|
||||||
|
await expect(row(page, NAMES.staged)).toHaveCount(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Cleared back to nothing must mean "no filter" rather than "no statuses",
|
||||||
|
// or emptying the box would empty the table.
|
||||||
|
test('clearing the status shows everything again', async ({ page }) => {
|
||||||
|
await page.goto('/admin');
|
||||||
|
await filterToOwnCategory(page);
|
||||||
|
await chooseStatus(page, 'Pending');
|
||||||
|
await expect(row(page, NAMES.cheap)).toHaveCount(0);
|
||||||
|
|
||||||
|
await chooseStatus(page, 'Pending');
|
||||||
|
|
||||||
|
await expect(row(page, NAMES.cheap)).toBeVisible();
|
||||||
|
await expect(row(page, NAMES.staged)).toBeVisible();
|
||||||
});
|
});
|
||||||
|
|
||||||
test('combines filters, and clearing restores them', async ({ page }) => {
|
test('combines filters, and clearing restores them', async ({ page }) => {
|
||||||
|
|||||||
Reference in New Issue
Block a user