Merge pull request 'feat(admin): filter inventory by status directly, so Published and Unpublished are reachable (#132)' (#134) from feature/132-admin-status-multiselect into main

Reviewed-on: #134

frontend/src/admin/InventoryFilters.tsx

@@ -1,20 +1,27 @@
 import { useMemo } from 'react';
 import TreeSelect from 'antd/es/tree-select';
 import Select from 'antd/es/select';
-import Segmented from 'antd/es/segmented';
 import InputNumber from 'antd/es/input-number';
 import Button from 'antd/es/button';
 import type { Category, Tag } from '../api';
 import {
   ItemFilters,
-  SaleState,
-  ADMIN_SALE_STATUSES,
+  ItemStatus,
   buildCategoryTree,
   CategoryNode,
-  hasActiveFilters,
-  saleStateFromStatuses
+  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;
@@ -93,32 +100,34 @@ export default function InventoryFilters({ categories, tags, filters, onChange,
         onChange={(value) => onChange({ ...filters, maxPriceCents: dollarsToCents(value) })}
       />
 
-      {/* Replaces the four-way status dropdown that used to sit here. One
-          control instead of two overlapping ways to say the same thing.
-          Note what it costs: a single status can no longer be isolated, so
-          there is no longer a way to view only Reserved, or only Pending.
-          Not Sold folds pending in with available and reserved. If isolating
-          one status turns out to matter — the pending workflow from #90 is the
-          likeliest candidate — the fix is to put that back alongside this
-          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' }
-        ]}
+      {/* 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>}

frontend/src/filters.ts

@@ -38,14 +38,12 @@ export const STOREFRONT_SALE_STATUSES: Record<SaleState, ItemStatus[]> = {
   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']
-};
+// The admin had a table of its own here until #132, where the preset was
+// replaced by a multi-select of the statuses themselves. Presets could not
+// express Published or Unpublished and could not isolate a single status, and
+// the admin is where those questions get asked. The storefront keeps its
+// preset: pending never reaches a customer, so the distinction does not exist
+// for them.
 
 function isPublicStatus(value: string): value is ItemStatus {
   return value === 'available' || value === 'reserved' || value === 'sold';

frontend/tests/e2e/admin-inventory-filters.spec.ts

@@ -7,7 +7,8 @@ const NAMES = {
   tag: `filt-${RUN}`,
   cheap: `Cheap item ${RUN}`,
   mid: `Mid item ${RUN}`,
-  dear: `Dear item ${RUN}`
+  dear: `Dear item ${RUN}`,
+  staged: `Staged item ${RUN}`
 };
 
 test.beforeAll(async ({ playwright }) => {
@@ -20,7 +21,7 @@ test.beforeAll(async ({ playwright }) => {
 
   // Published after creation: new items are pending, and these fixtures stand
   // 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', {
       multipart: {
         name,
@@ -30,23 +31,38 @@ test.beforeAll(async ({ playwright }) => {
         tags: JSON.stringify(inCategory ? [NAMES.tag] : [])
       }
     });
+    if (publish) {
       await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`);
+    }
   };
 
   await item(NAMES.cheap, '50', true);
   await item(NAMES.mid, '150', 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();
 });
 
-// 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();
+// Toggles one status in the multi-select. Clicking an option that is already
+// selected 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.
+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 });
@@ -86,28 +102,61 @@ test.describe('Admin inventory filters', () => {
     await expect(row(page, NAMES.dear)).toHaveCount(0);
   });
 
-  // 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 }) => {
+  test('filters by a single status', async ({ page }) => {
     await page.goto('/admin');
     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.mid)).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.mid)).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 }) => {