feat: stage new items as pending until an admin publishes them (#90) #93
@@ -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';
|
||||
`);
|
||||
};
|
||||
@@ -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[];
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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'`
|
||||
)
|
||||
]);
|
||||
|
||||
|
||||
@@ -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]);
|
||||
}));
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
|
||||
@@ -30,8 +30,11 @@ async function createItem(
|
||||
categoryId: number | null = null,
|
||||
tagIds: number[] = []
|
||||
): Promise<number> {
|
||||
// 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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<number> {
|
||||
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<number> {
|
||||
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']);
|
||||
});
|
||||
});
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<string, string> = { 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<string, string> = { sold: 'red', reserved: 'orange', pending: 'default' };
|
||||
|
||||
function Inventory() {
|
||||
const [items, setItems] = useState<Item[]>([]);
|
||||
@@ -227,6 +229,20 @@ function Inventory() {
|
||||
{item.status !== 'sold'
|
||||
? <Button size="small" onClick={() => handleStatusChange(markSold, item.id, 'mark sold')}>Mark Sold</Button>
|
||||
: <Button size="small" onClick={() => handleStatusChange(markAvailable, item.id, 'mark available')}>Mark Available</Button>}
|
||||
{/* 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' && (
|
||||
<Button size="small" type="primary" onClick={() => handleStatusChange(markAvailable, item.id, 'publish')}>
|
||||
Publish
|
||||
</Button>
|
||||
)}
|
||||
{item.status === 'available' && (
|
||||
<Button size="small" onClick={() => handleStatusChange(unpublishItem, item.id, 'unpublish')}>
|
||||
Unpublish
|
||||
</Button>
|
||||
)}
|
||||
</Space>
|
||||
)
|
||||
}
|
||||
@@ -314,7 +330,15 @@ function Inventory() {
|
||||
<div style={{ maxWidth: 340, margin: '0 auto' }}>
|
||||
{/* onChanged never fires: every handler that would call it is
|
||||
short-circuited by `preview`. */}
|
||||
<ItemCard item={previewItem} onChanged={() => 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". */}
|
||||
<ItemCard
|
||||
item={previewItem.status === 'pending' ? { ...previewItem, status: 'available' } : previewItem}
|
||||
onChanged={() => undefined}
|
||||
preview
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</Drawer>
|
||||
|
||||
@@ -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' }
|
||||
|
||||
+16
-1
@@ -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<Item> {
|
||||
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<Item> {
|
||||
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<Item> {
|
||||
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<Item> {
|
||||
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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
|
||||
@@ -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 }) => {
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user