diff --git a/backend/migrations/1787300000000_default-items-to-pending.js b/backend/migrations/1787300000000_default-items-to-pending.js new file mode 100644 index 0000000..ec734d9 --- /dev/null +++ b/backend/migrations/1787300000000_default-items-to-pending.js @@ -0,0 +1,18 @@ +exports.up = (pgm) => { + pgm.sql(` + -- New items are staged, not published. Before this an item was live on the + -- storefront the instant it was created, with no way to add something, look + -- at it, and then decide it was ready. + -- + -- Only the default changes. Existing rows keep whatever status they have — + -- backfilling would un-publish the entire live catalogue, which is the one + -- thing this migration must not do. + ALTER TABLE items ALTER COLUMN status SET DEFAULT 'pending'; + `); +}; + +exports.down = (pgm) => { + pgm.sql(` + ALTER TABLE items ALTER COLUMN status SET DEFAULT 'available'; + `); +}; diff --git a/backend/src/itemFilters.ts b/backend/src/itemFilters.ts index 0965610..44f2ebc 100644 --- a/backend/src/itemFilters.ts +++ b/backend/src/itemFilters.ts @@ -16,12 +16,17 @@ export interface ItemFilters { favoritesOnly: boolean; } -export type ItemStatus = 'available' | 'reserved' | 'sold'; +export type ItemStatus = 'pending' | 'available' | 'reserved' | 'sold'; // Matched exactly, not case-insensitively: `items.status` only ever holds these // lowercase values, so accepting 'Reserved' would quietly return nothing rather // than reporting that the filter was wrong. -const ITEM_STATUSES: readonly string[] = ['available', 'reserved', 'sold']; +const ITEM_STATUSES: readonly string[] = ['pending', 'available', 'reserved', 'sold']; + +// Storefront-invalid statuses. This parser is shared with the admin routes, +// where filtering by 'pending' is exactly the point, so the public routes have +// to refuse it themselves rather than the parser refusing it for everyone. +export const NON_PUBLIC_STATUSES: readonly ItemStatus[] = ['pending']; export interface BuiltFilter { clauses: string[]; diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index fda676a..c410dc4 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -267,6 +267,40 @@ router.post('/items/:id/mark-sold', asyncRoute(async (req: Request, res: Respons res.json(rows[0]); })); +// Publishing is the existing mark-available: it already sets status='available' +// and clears sold_at, reserved_until and paypal_order_id, all of which are +// no-ops on a pending item. A second endpoint running the same UPDATE would be +// duplication, so the admin UI labels that button "Publish" when the item is +// pending. This is the reverse, and it is not symmetrical — see the guard. +router.post('/items/:id/unpublish', asyncRoute(async (req: Request, res: Response) => { + const { rows } = await pool.query(`SELECT status FROM items WHERE id = $1`, [req.params.id]); + if (!rows.length) { + return res.status(404).json({ error: 'not found' }); + } + + const status = rows[0].status; + if (status === 'pending') { + return res.status(400).json({ error: 'this item is already pending' }); + } + // Reserved and sold are not drafts. A reserved item is in someone's cart + // right now and hiding it would strand them mid-checkout; a sold item is a + // record of something that happened, and pulling it back would quietly + // rewrite that. Both are refused by name so the reason is on screen rather + // than left to be guessed from a generic error. + if (status === 'reserved') { + return res.status(400).json({ error: 'a customer is holding this item — it cannot be unpublished' }); + } + if (status === 'sold') { + return res.status(400).json({ error: 'a sold item cannot be unpublished' }); + } + + const { rows: updated } = await pool.query( + `UPDATE items SET status='pending' WHERE id=$1 RETURNING *`, + [req.params.id] + ); + res.json(updated[0]); +})); + router.post('/items/:id/mark-available', asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query( `UPDATE items SET status='available', sold_at=NULL, reserved_until=NULL, paypal_order_id=NULL diff --git a/backend/src/routes/filters.ts b/backend/src/routes/filters.ts index b6bf4ad..1f263be 100644 --- a/backend/src/routes/filters.ts +++ b/backend/src/routes/filters.ts @@ -12,18 +12,26 @@ router.get('/', asyncRoute(async (_req: Request, res: Response) => { pool.query( `SELECT id, name, parent_id, sort_order FROM categories ORDER BY sort_order, lower(name)` ), + // Pending items are excluded from the count, not just from the catalogue. + // Counting them would show a customer a tag reading "Rare (1)", and + // filtering by it would then report that nothing matches. pool.query( - `SELECT t.id, t.name, t.color, COUNT(it.item_id)::int AS item_count + `SELECT t.id, t.name, t.color, COUNT(i.id)::int AS item_count FROM tags t LEFT JOIN item_tags it ON it.tag_id = t.id + LEFT JOIN items i ON i.id = it.item_id AND i.status <> 'pending' GROUP BY t.id ORDER BY lower(t.name)` ), // An empty catalogue would otherwise hand the slider a null range. + // + // Pending items are excluded, or a staged item priced far above or below + // everything on sale would stretch the slider to a range no visible item + // occupies — the customer drags to the end and finds nothing there. pool.query( `SELECT COALESCE(MIN(price_cents), 0)::int AS min_cents, COALESCE(MAX(price_cents), 0)::int AS max_cents - FROM items` + FROM items WHERE status <> 'pending'` ) ]); diff --git a/backend/src/routes/items.ts b/backend/src/routes/items.ts index 9958c5d..c7f673c 100755 --- a/backend/src/routes/items.ts +++ b/backend/src/routes/items.ts @@ -2,7 +2,13 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; import { asyncRoute } from '../asyncRoute'; import { PUBLIC_ITEM_SELECT } from '../itemSelect'; -import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters'; +import { parseItemFilters, buildItemFilterSql, FilterError, NON_PUBLIC_STATUSES } from '../itemFilters'; + +// Applied to every public read, unconditionally. This route has never had a +// status filter of its own — sold items are listed and rendered with a Sold +// badge on purpose — so hiding pending items cannot be expressed as one more +// optional filter. It has to be a clause the caller cannot opt out of. +const EXCLUDE_PENDING = `i.status <> 'pending'`; const router = Router(); @@ -28,14 +34,31 @@ router.get('/', asyncRoute(async (req: Request, res: Response) => { return res.status(401).json({ error: 'sign in to filter by favorites' }); } + // Refused rather than quietly answered. The filter parser is shared with the + // admin routes, where 'pending' is valid, so it parses here too — and with + // the exclusion below it would return an empty list, which reads as "no items + // match" rather than "you may not ask that". + if (filters.status && NON_PUBLIC_STATUSES.includes(filters.status)) { + return res.status(400).json({ error: 'invalid status' }); + } + const { clauses, params } = buildItemFilterSql(filters, 1, req.customerId ?? null); - const where = clauses.length ? `WHERE ${clauses.join(' AND ')}` : ''; - const { rows } = await pool.query(`${PUBLIC_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`, params); + const where = [EXCLUDE_PENDING, ...clauses].join(' AND '); + const { rows } = await pool.query( + `${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC`, + params + ); res.json(rows); })); router.get('/:id', asyncRoute(async (req: Request, res: Response) => { - const { rows } = await pool.query(`${PUBLIC_ITEM_SELECT} WHERE i.id = $1`, [req.params.id]); + // Excluded here too, not only from the list. A pending item that stayed + // fetchable by id would be hidden from the catalogue and still reachable by + // anyone who guessed or kept a link. + const { rows } = await pool.query( + `${PUBLIC_ITEM_SELECT} WHERE i.id = $1 AND ${EXCLUDE_PENDING}`, + [req.params.id] + ); if (!rows.length) return res.status(404).json({ error: 'not found' }); res.json(rows[0]); })); diff --git a/backend/tests/integration/adminInventory.integration.test.ts b/backend/tests/integration/adminInventory.integration.test.ts index 5ed452d..030a36b 100644 --- a/backend/tests/integration/adminInventory.integration.test.ts +++ b/backend/tests/integration/adminInventory.integration.test.ts @@ -118,7 +118,9 @@ describe('GET /api/admin/items filtering', () => { it('rejects an unknown status rather than returning everything', async () => { await createItem('A', 1000); - const res = await request(app).get('/api/admin/items?status=pending'); + // Not 'pending': that is a real status now, and deliberately valid on the + // admin route — filtering for staged items is the point of it. + const res = await request(app).get('/api/admin/items?status=archived'); expect(res.status).toBe(400); }); diff --git a/backend/tests/integration/cart.integration.test.ts b/backend/tests/integration/cart.integration.test.ts index 19a69c8..d591be1 100644 --- a/backend/tests/integration/cart.integration.test.ts +++ b/backend/tests/integration/cart.integration.test.ts @@ -20,7 +20,7 @@ async function registerAndGetAgent(email: string) { describe('cart', () => { it('adding an item to cart marks it reserved', async () => { - const { rows } = await pool.query(`INSERT INTO items (name, price_cents) VALUES ('Test Item', 1000) RETURNING id`); + const { rows } = await pool.query(`INSERT INTO items (name, price_cents, status) VALUES ('Test Item', 1000, 'available') RETURNING id`); const itemId = rows[0].id; const agent = await registerAndGetAgent('cart1@example.com'); @@ -36,7 +36,7 @@ describe('cart', () => { }); it('refuses to add an item that is already reserved', async () => { - const { rows } = await pool.query(`INSERT INTO items (name, price_cents) VALUES ('Test Item', 1000) RETURNING id`); + const { rows } = await pool.query(`INSERT INTO items (name, price_cents, status) VALUES ('Test Item', 1000, 'available') RETURNING id`); const itemId = rows[0].id; const agentA = await registerAndGetAgent('cartA@example.com'); const agentB = await registerAndGetAgent('cartB@example.com'); @@ -47,7 +47,7 @@ describe('cart', () => { }); it('removing an item from cart releases it back to available', async () => { - const { rows } = await pool.query(`INSERT INTO items (name, price_cents) VALUES ('Test Item', 1000) RETURNING id`); + const { rows } = await pool.query(`INSERT INTO items (name, price_cents, status) VALUES ('Test Item', 1000, 'available') RETURNING id`); const itemId = rows[0].id; const agent = await registerAndGetAgent('cart2@example.com'); @@ -74,7 +74,7 @@ describe('cart demo checkout', () => { it('completes a multi-item cart purchase and marks all items sold', async () => { const { rows } = await pool.query( - `INSERT INTO items (name, price_cents) VALUES ('Item A', 1000), ('Item B', 2000) RETURNING id` + `INSERT INTO items (name, price_cents, status) VALUES ('Item A', 1000, 'available'), ('Item B', 2000, 'available') RETURNING id` ); const [itemA, itemB] = rows; const agent = await registerAndGetAgent('checkout1@example.com'); @@ -100,7 +100,7 @@ describe('cart demo checkout', () => { }); it('refuses checkout without a shipping address', async () => { - const { rows } = await pool.query(`INSERT INTO items (name, price_cents) VALUES ('Item A', 1000) RETURNING id`); + const { rows } = await pool.query(`INSERT INTO items (name, price_cents, status) VALUES ('Item A', 1000, 'available') RETURNING id`); const agent = await registerAndGetAgent('checkout2@example.com'); await agent.post(`/api/cart/items/${rows[0].id}`); diff --git a/backend/tests/integration/categoriesTags.integration.test.ts b/backend/tests/integration/categoriesTags.integration.test.ts index 2cd42e9..bbbd16f 100644 --- a/backend/tests/integration/categoriesTags.integration.test.ts +++ b/backend/tests/integration/categoriesTags.integration.test.ts @@ -30,8 +30,11 @@ async function createItem( categoryId: number | null = null, tagIds: number[] = [] ): Promise { + // status is explicit rather than left to the column default. These tests are + // about items a customer can see, and the default is 'pending' — an item is + // staged until an admin publishes it. const { rows } = await pool.query( - `INSERT INTO items (name, price_cents, category_id) VALUES ($1, $2, $3) RETURNING id`, + `INSERT INTO items (name, price_cents, category_id, status) VALUES ($1, $2, $3, 'available') RETURNING id`, [name, priceCents, categoryId] ); const itemId = rows[0].id; @@ -311,6 +314,11 @@ describe('admin item form', () => { [create.body.id] ); + // Published first: this asserts against PUBLIC_ITEM_SELECT specifically, + // which is the shape the images-times-tags join bug would show up in, and + // a new item is pending and therefore not publicly fetchable. + await request(app).post(`/api/admin/items/${create.body.id}/mark-available`); + const res = await request(app).get(`/api/items/${create.body.id}`); expect(res.body.images).toHaveLength(2); expect(res.body.tags).toHaveLength(3); diff --git a/backend/tests/integration/disableCustomer.integration.test.ts b/backend/tests/integration/disableCustomer.integration.test.ts index 1ebb958..e4582b3 100644 --- a/backend/tests/integration/disableCustomer.integration.test.ts +++ b/backend/tests/integration/disableCustomer.integration.test.ts @@ -24,7 +24,7 @@ async function register(email: string) { async function createItem(name: string) { const { rows } = await pool.query( - `INSERT INTO items (name, price_cents) VALUES ($1, 1000) RETURNING id`, + `INSERT INTO items (name, price_cents, status) VALUES ($1, 1000, 'available') RETURNING id`, [name] ); return rows[0].id as number; diff --git a/backend/tests/integration/favorites.integration.test.ts b/backend/tests/integration/favorites.integration.test.ts index 127e71a..3dab2d6 100644 --- a/backend/tests/integration/favorites.integration.test.ts +++ b/backend/tests/integration/favorites.integration.test.ts @@ -30,7 +30,7 @@ async function register(email: string) { async function createItem(name: string) { const { rows } = await pool.query( - `INSERT INTO items (name, price_cents) VALUES ($1, 1000) RETURNING id`, + `INSERT INTO items (name, price_cents, status) VALUES ($1, 1000, 'available') RETURNING id`, [name] ); return rows[0].id as number; diff --git a/backend/tests/integration/pendingStatus.integration.test.ts b/backend/tests/integration/pendingStatus.integration.test.ts new file mode 100644 index 0000000..548784b --- /dev/null +++ b/backend/tests/integration/pendingStatus.integration.test.ts @@ -0,0 +1,225 @@ +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +async function createTag(name: string): Promise { + const res = await request(app).post('/api/admin/tags').send({ name }); + expect(res.status).toBe(201); + return res.body.id; +} + +// Direct insert so a test can put an item in a specific state without going +// through the transitions being tested. +async function insertItem( + name: string, + priceCents: number, + status: string, + tagIds: number[] = [] +): Promise { + const { rows } = await pool.query( + `INSERT INTO items (name, price_cents, status) VALUES ($1, $2, $3) RETURNING id`, + [name, priceCents, status] + ); + const id = rows[0].id; + for (const tagId of tagIds) { + await pool.query(`INSERT INTO item_tags (item_id, tag_id) VALUES ($1, $2)`, [id, tagId]); + } + return id; +} + +describe('a new item is staged rather than published', () => { + it('arrives pending when created through the admin API', async () => { + const res = await request(app) + .post('/api/admin/items') + .field('name', 'Fresh') + .field('description', '') + .field('price', '25'); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('pending'); + }); +}); + +// Each of these is a separate query, so fixing one proves nothing about the +// others. They are tested separately for that reason. +describe('a pending item does not reach the storefront', () => { + it('is absent from the catalogue listing', async () => { + await insertItem('Staged', 1000, 'pending'); + await insertItem('Live', 2000, 'available'); + + const res = await request(app).get('/api/items'); + + expect(res.status).toBe(200); + expect(res.body.map((i: { name: string }) => i.name)).toEqual(['Live']); + }); + + // Hiding it from the list but serving it by id would leave it reachable to + // anyone who guessed the id or kept an old link. + it('is not fetchable by direct id', async () => { + const id = await insertItem('Staged', 1000, 'pending'); + + const res = await request(app).get(`/api/items/${id}`); + + expect(res.status).toBe(404); + }); + + it('is still fetchable by id once published', async () => { + const id = await insertItem('Staged', 1000, 'pending'); + await request(app).post(`/api/admin/items/${id}/mark-available`); + + const res = await request(app).get(`/api/items/${id}`); + + expect(res.status).toBe(200); + expect(res.body.name).toBe('Staged'); + }); + + // A count including pending items would show a customer "Rare (1)", and + // filtering by it would then report that nothing matches. + it('is not counted in the filter drawer tag counts', async () => { + const tag = await createTag('rare'); + await insertItem('Staged', 1000, 'pending', [tag]); + await insertItem('Live', 2000, 'available', [tag]); + + const res = await request(app).get('/api/filters'); + + const rare = res.body.tags.find((t: { name: string }) => t.name === 'rare'); + expect(rare.item_count).toBe(1); + }); + + // The tag must still be listed, at zero. Excluding pending items with a WHERE + // rather than in the count would drop the tag's row entirely and make the tag + // vanish from the drawer. + it('leaves a tag whose only item is pending listed with a count of zero', async () => { + const tag = await createTag('unreleased'); + await insertItem('Staged', 1000, 'pending', [tag]); + + const res = await request(app).get('/api/filters'); + + const unreleased = res.body.tags.find((t: { name: string }) => t.name === 'unreleased'); + expect(unreleased).toBeDefined(); + expect(unreleased.item_count).toBe(0); + }); + + // A staged item priced far outside the live range would stretch the slider to + // a range no visible item occupies. + it('does not stretch the price slider bounds', async () => { + await insertItem('Cheap live', 1000, 'available'); + await insertItem('Dear live', 5000, 'available'); + await insertItem('Absurd staged', 999999, 'pending'); + + const res = await request(app).get('/api/filters'); + + expect(res.body.priceRange.min_cents).toBe(1000); + expect(res.body.priceRange.max_cents).toBe(5000); + }); +}); + +describe('asking the public API for pending items', () => { + // Refused rather than answered with an empty list, which would read as "no + // items match" instead of "you may not ask that". + it('is refused on the storefront route', async () => { + await insertItem('Staged', 1000, 'pending'); + + const res = await request(app).get('/api/items?status=pending'); + + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid status'); + }); + + it('is still allowed on the admin route, which is the point of it', async () => { + await insertItem('Staged', 1000, 'pending'); + await insertItem('Live', 2000, 'available'); + + const res = await request(app).get('/api/admin/items?status=pending'); + + expect(res.status).toBe(200); + expect(res.body.map((i: { name: string }) => i.name)).toEqual(['Staged']); + }); + + it('leaves the other statuses working on the storefront', async () => { + await insertItem('Live', 1000, 'available'); + await insertItem('Gone', 2000, 'sold'); + + const res = await request(app).get('/api/items?status=sold'); + + expect(res.status).toBe(200); + expect(res.body.map((i: { name: string }) => i.name)).toEqual(['Gone']); + }); +}); + +describe('unpublishing', () => { + it('returns an available item to pending and removes it from the catalogue', async () => { + const id = await insertItem('Live', 1000, 'available'); + + const res = await request(app).post(`/api/admin/items/${id}/unpublish`); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('pending'); + const listing = await request(app).get('/api/items'); + expect(listing.body).toHaveLength(0); + }); + + // Not a draft: someone is holding it in their cart right now, and hiding it + // would strand them mid-checkout. + it('refuses a reserved item and says why', async () => { + const id = await insertItem('Held', 1000, 'reserved'); + + const res = await request(app).post(`/api/admin/items/${id}/unpublish`); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('holding this item'); + const { rows } = await pool.query(`SELECT status FROM items WHERE id = $1`, [id]); + expect(rows[0].status).toBe('reserved'); + }); + + // Not a draft either: a sold item is a record of something that happened. + it('refuses a sold item and says why', async () => { + const id = await insertItem('Gone', 1000, 'sold'); + + const res = await request(app).post(`/api/admin/items/${id}/unpublish`); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('sold item'); + const { rows } = await pool.query(`SELECT status FROM items WHERE id = $1`, [id]); + expect(rows[0].status).toBe('sold'); + }); + + it('refuses an item that is already pending', async () => { + const id = await insertItem('Staged', 1000, 'pending'); + + const res = await request(app).post(`/api/admin/items/${id}/unpublish`); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('already pending'); + }); + + it('reports a missing item as not found rather than as a bad request', async () => { + const res = await request(app).post('/api/admin/items/999999/unpublish'); + + expect(res.status).toBe(404); + }); +}); + +describe('publishing', () => { + it('puts a pending item into the catalogue', async () => { + const id = await insertItem('Staged', 1000, 'pending'); + expect((await request(app).get('/api/items')).body).toHaveLength(0); + + const res = await request(app).post(`/api/admin/items/${id}/mark-available`); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('available'); + const listing = await request(app).get('/api/items'); + expect(listing.body.map((i: { name: string }) => i.name)).toEqual(['Staged']); + }); +}); diff --git a/backend/tests/unit/itemFilters.test.ts b/backend/tests/unit/itemFilters.test.ts index 3f2a3f3..0f98cbe 100644 --- a/backend/tests/unit/itemFilters.test.ts +++ b/backend/tests/unit/itemFilters.test.ts @@ -107,7 +107,9 @@ describe('parseItemFilters', () => { }); it('rejects a status outside the known set', () => { - expect(() => parseItemFilters({ status: 'pending' })).toThrow(FilterError); + // 'pending' used to be the example here and is now a real status, which is + // exactly the sort of thing that quietly turns a test into a tautology. + expect(() => parseItemFilters({ status: 'archived' })).toThrow(FilterError); }); it('rejects a status differing only by case, rather than silently coercing it', () => { diff --git a/frontend/src/admin/Admin.tsx b/frontend/src/admin/Admin.tsx index 5c41af9..f967ef9 100755 --- a/frontend/src/admin/Admin.tsx +++ b/frontend/src/admin/Admin.tsx @@ -11,7 +11,7 @@ import '@uiw/react-md-editor/markdown-editor.css'; import '@uiw/react-markdown-preview/markdown.css'; import { Item, Category, Tag as TagRecord, - fetchAdminItems, saveItem, deleteItem, deleteItemImage, markSold, markAvailable, + fetchAdminItems, saveItem, deleteItem, deleteItemImage, markSold, markAvailable, unpublishItem, fetchAdminCategories, fetchAdminTags } from '../api'; import { useThemeMode } from '../theme/ThemeContext'; @@ -29,7 +29,9 @@ const { Title } = Typography; // Anything not sold or reserved is available, so green is the default rather // than a third entry — a new status shows up green instead of crashing. -const STATUS_TAG_COLORS: Record = { sold: 'red', reserved: 'orange' }; +// Pending is grey rather than a colour: it is the absence of being published, +// not a state of its own worth drawing the eye to. +const STATUS_TAG_COLORS: Record = { sold: 'red', reserved: 'orange', pending: 'default' }; function Inventory() { const [items, setItems] = useState([]); @@ -227,6 +229,20 @@ function Inventory() { {item.status !== 'sold' ? : } + {/* Publishing is mark-available under a name that says what it means + here. Unpublish is offered only from available — the server + refuses reserved and sold and says why, and hiding the button in + those states keeps the refusal from being the way you find out. */} + {item.status === 'pending' && ( + + )} + {item.status === 'available' && ( + + )} ) } @@ -314,7 +330,15 @@ function Inventory() {
{/* onChanged never fires: every handler that would call it is short-circuited by `preview`. */} - undefined} preview /> + {/* A pending item is previewed as it will look once published. + No customer ever sees a pending item, so rendering that state + would answer a question nobody is asking — what is wanted here + is "how will this look when it is live". */} + undefined} + preview + />
)} diff --git a/frontend/src/admin/InventoryFilters.tsx b/frontend/src/admin/InventoryFilters.tsx index 9f1a36c..d4533af 100644 --- a/frontend/src/admin/InventoryFilters.tsx +++ b/frontend/src/admin/InventoryFilters.tsx @@ -21,6 +21,7 @@ function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] { } const STATUS_OPTIONS: { value: ItemStatus; label: string }[] = [ + { value: 'pending', label: 'Pending' }, { value: 'available', label: 'Available' }, { value: 'reserved', label: 'Reserved' }, { value: 'sold', label: 'Sold' } diff --git a/frontend/src/api.ts b/frontend/src/api.ts index d6569d1..ff4916f 100755 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -13,7 +13,7 @@ export interface Item { description: string | null; price_cents: number; images: { id: number; image_path: string; sort_order: number }[]; - status: 'available' | 'reserved' | 'sold'; + status: 'pending' | 'available' | 'reserved' | 'sold'; category_id: number | null; category_name: string | null; tags: ItemTag[]; @@ -114,6 +114,10 @@ export async function markSold(id: number): Promise { return res.json(); } +// Publishing a pending item is mark-available: it is the same transition and +// the same UPDATE, so the admin UI simply labels the button "Publish" when the +// item is pending rather than calling a second endpoint that does the same +// thing. export async function markAvailable(id: number): Promise { const res = await expectOk( await fetch(`/api/admin/items/${id}/mark-available`, { method: 'POST' }), @@ -122,6 +126,17 @@ export async function markAvailable(id: number): Promise { return res.json(); } +// Not symmetrical with the above: the server refuses to unpublish a reserved or +// sold item and says which, so the message it returns is worth surfacing rather +// than replacing with a generic one. +export async function unpublishItem(id: number): Promise { + const res = await expectOk( + await fetch(`/api/admin/items/${id}/unpublish`, { method: 'POST' }), + 'failed to unpublish' + ); + return res.json(); +} + // The admin endpoints return a JSON error body on 4xx; surfacing its message // lets the UI say "that name is already used here" instead of a generic // failure. diff --git a/frontend/src/filters.ts b/frontend/src/filters.ts index eb24c42..4d2d3fe 100644 --- a/frontend/src/filters.ts +++ b/frontend/src/filters.ts @@ -1,6 +1,6 @@ import type { Category } from './api'; -export type ItemStatus = 'available' | 'reserved' | 'sold'; +export type ItemStatus = 'pending' | 'available' | 'reserved' | 'sold'; export interface ItemFilters { categoryId: number | null; @@ -51,6 +51,13 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters { .map((part) => readInt(part)) .filter((id): id is number => id !== null && id > 0); + // Deliberately does NOT accept 'pending', even though it is a valid + // ItemStatus. This reader exists for the storefront's URL, where filtering by + // pending is not a thing a customer may ask for — the public API refuses it + // outright, so parsing it here would only produce a request guaranteed to + // fail. The admin's status filter holds its value in React state and never + // round-trips through this function, so it is unaffected. Do not "complete" + // this list to match the type. const rawStatus = params.get('status'); const status = rawStatus === 'available' || rawStatus === 'reserved' || rawStatus === 'sold' ? rawStatus diff --git a/frontend/tests/e2e/admin-disable-customer.spec.ts b/frontend/tests/e2e/admin-disable-customer.spec.ts index 87d3fc5..09b2378 100644 --- a/frontend/tests/e2e/admin-disable-customer.spec.ts +++ b/frontend/tests/e2e/admin-disable-customer.spec.ts @@ -96,6 +96,8 @@ test.describe('Disabling a customer account', () => { multipart: { name: itemName, description: '', price: '40', category_id: '', tags: '[]' } }); const itemId = (await created.json()).id as number; + // New items are pending, and a pending item cannot be added to a cart. + expect((await request.post(`/api/admin/items/${itemId}/mark-available`)).ok()).toBeTruthy(); await register(page, email); expect((await page.request.post(`/api/cart/items/${itemId}`)).status()).toBe(201); diff --git a/frontend/tests/e2e/admin-inventory-filters.spec.ts b/frontend/tests/e2e/admin-inventory-filters.spec.ts index 34e9318..c42aa8c 100644 --- a/frontend/tests/e2e/admin-inventory-filters.spec.ts +++ b/frontend/tests/e2e/admin-inventory-filters.spec.ts @@ -18,8 +18,10 @@ test.beforeAll(async ({ playwright }) => { })).json()).id; await api.post('/api/admin/tags', { data: { name: NAMES.tag } }); - const item = (name: string, price: string, inCategory: boolean) => - api.post('/api/admin/items', { + // 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 res = await api.post('/api/admin/items', { multipart: { name, description: '', @@ -28,6 +30,8 @@ test.beforeAll(async ({ playwright }) => { tags: JSON.stringify(inCategory ? [NAMES.tag] : []) } }); + await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`); + }; await item(NAMES.cheap, '50', true); await item(NAMES.mid, '150', true); diff --git a/frontend/tests/e2e/admin-item-preview.spec.ts b/frontend/tests/e2e/admin-item-preview.spec.ts index dec1fa1..f14a6fa 100644 --- a/frontend/tests/e2e/admin-item-preview.spec.ts +++ b/frontend/tests/e2e/admin-item-preview.spec.ts @@ -7,7 +7,12 @@ async function createItem(page: import('@playwright/test').Page, name: string, p multipart: { name, description: 'A preview subject', price, category_id: '', tags: '[]' } }); expect(created.ok()).toBeTruthy(); - return (await created.json()).id as number; + const id = (await created.json()).id as number; + // New items are pending. Published here so the card renders the same states + // a customer would see; the pending case is covered in the pending spec. + const published = await page.request.post(`/api/admin/items/${id}/mark-available`); + expect(published.ok()).toBeTruthy(); + return id; } test.describe('Admin item preview', () => { diff --git a/frontend/tests/e2e/admin-reserved-items.spec.ts b/frontend/tests/e2e/admin-reserved-items.spec.ts index c03ef3f..0eee153 100644 --- a/frontend/tests/e2e/admin-reserved-items.spec.ts +++ b/frontend/tests/e2e/admin-reserved-items.spec.ts @@ -12,6 +12,8 @@ async function reserveItem(page: import('@playwright/test').Page, itemName: stri }); expect(created.ok()).toBeTruthy(); const itemId = (await created.json()).id as number; + // New items are pending, and a pending item cannot be reserved. + expect((await page.request.post(`/api/admin/items/${itemId}/mark-available`)).ok()).toBeTruthy(); await page.goto('/register'); await page.getByRole('textbox', { name: 'Email' }).fill(email); diff --git a/frontend/tests/e2e/favorites-filter.spec.ts b/frontend/tests/e2e/favorites-filter.spec.ts index f87a023..f4e1678 100644 --- a/frontend/tests/e2e/favorites-filter.spec.ts +++ b/frontend/tests/e2e/favorites-filter.spec.ts @@ -21,6 +21,8 @@ test.beforeAll(async ({ playwright }) => { multipart: { name, description: '', price, category_id: '', tags: '[]' } }); expect(res.ok()).toBeTruthy(); + // New items are pending; the storefront only lists published ones. + expect((await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`)).ok()).toBeTruthy(); } await api.dispose(); }); diff --git a/frontend/tests/e2e/favorites.spec.ts b/frontend/tests/e2e/favorites.spec.ts index 5018c31..c553def 100644 --- a/frontend/tests/e2e/favorites.spec.ts +++ b/frontend/tests/e2e/favorites.spec.ts @@ -12,6 +12,8 @@ test.beforeAll(async ({ playwright }) => { multipart: { name: ITEM, description: '', price: '60', category_id: '', tags: '[]' } }); expect(res.ok()).toBeTruthy(); + // New items are pending; the storefront only lists published ones. + expect((await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`)).ok()).toBeTruthy(); await api.dispose(); }); diff --git a/frontend/tests/e2e/filters.spec.ts b/frontend/tests/e2e/filters.spec.ts index 6c47726..f497ce1 100644 --- a/frontend/tests/e2e/filters.spec.ts +++ b/frontend/tests/e2e/filters.spec.ts @@ -49,6 +49,8 @@ async function createItem( } }); expect(res.ok()).toBeTruthy(); + // New items are pending; the storefront only lists published ones. + expect((await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`)).ok()).toBeTruthy(); } test.beforeAll(async ({ playwright }) => { diff --git a/frontend/tests/e2e/pending-publish.spec.ts b/frontend/tests/e2e/pending-publish.spec.ts new file mode 100644 index 0000000..2035cd9 --- /dev/null +++ b/frontend/tests/e2e/pending-publish.spec.ts @@ -0,0 +1,78 @@ +import { test, expect } from './fixtures'; + +const suffix = () => `s${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`; + +// Creates an item and leaves it as it arrives — pending. Deliberately does not +// publish, unlike the other specs' fixtures, because the staged state is what +// is being tested here. +async function createStagedItem(page: import('@playwright/test').Page, name: string) { + const created = await page.request.post('/api/admin/items', { + multipart: { name, description: '', price: '55', category_id: '', tags: '[]' } + }); + expect(created.ok()).toBeTruthy(); + const body = await created.json(); + // The whole premise: creating something does not publish it. + expect(body.status).toBe('pending'); + return body.id as number; +} + +const rowFor = (page: import('@playwright/test').Page, name: string) => + page.getByRole('row').filter({ hasText: name }); + +test.describe('Staging an item until it is published', () => { + test('a new item is held back from the storefront until published', async ({ page }) => { + const name = `Staged ${suffix()}`; + await createStagedItem(page, name); + + // Not in the catalogue while pending. + await page.goto('/'); + await expect(page.getByText(name)).toHaveCount(0); + + // It is in the admin, marked as pending. + await page.goto('/admin'); + const row = rowFor(page, name); + await expect(row).toBeVisible(); + await expect(row.getByText('PENDING')).toBeVisible(); + + await row.getByRole('button', { name: 'Publish' }).click(); + await expect(row.getByText('AVAILABLE')).toBeVisible(); + + // And now a customer can see it. + await page.goto('/'); + await expect(page.getByText(name)).toBeVisible(); + }); + + test('publishing can be undone while nobody is holding the item', async ({ page }) => { + const name = `Staged ${suffix()}`; + const id = await createStagedItem(page, name); + expect((await page.request.post(`/api/admin/items/${id}/mark-available`)).ok()).toBeTruthy(); + + await page.goto('/'); + await expect(page.getByText(name)).toBeVisible(); + + await page.goto('/admin'); + const row = rowFor(page, name); + await row.getByRole('button', { name: 'Unpublish' }).click(); + await expect(row.getByText('PENDING')).toBeVisible(); + + await page.goto('/'); + await expect(page.getByText(name)).toHaveCount(0); + }); + + // The two features together, which is the reason for wanting both: a staged + // item is previewed as it will look once live, because a customer never sees + // the pending state and "how will this look" is the question being asked. + test('a pending item previews as it will look once published', async ({ page }) => { + const name = `Staged ${suffix()}`; + await createStagedItem(page, name); + + await page.goto('/admin'); + await page.getByRole('button', { name }).click(); + + const drawer = page.getByRole('dialog', { name: `Preview: ${name}` }); + await expect(drawer).toBeVisible(); + await expect(drawer.getByText('$55.00')).toBeVisible(); + // The live card's action, not a pending placeholder. + await expect(drawer.getByRole('button', { name: 'Add to Cart' })).toBeVisible(); + }); +});