diff --git a/backend/src/app.ts b/backend/src/app.ts index bf9da94..c254692 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -10,6 +10,7 @@ import adminEmailTemplatesRouter from './routes/adminEmailTemplates'; import adminCategoriesRouter from './routes/adminCategories'; import adminTagsRouter from './routes/adminTags'; import adminUploadLinksRouter from './routes/adminUploadLinks'; +import adminItemDraftsRouter from './routes/adminItemDrafts'; import intakeRouter from './routes/intake'; import adminVersionRouter from './routes/adminVersion'; import filtersRouter from './routes/filters'; @@ -79,6 +80,7 @@ app.use('/api/admin/email-templates', requireAdminGate, adminEmailTemplatesRoute app.use('/api/admin/categories', requireAdminGate, adminCategoriesRouter); app.use('/api/admin/tags', requireAdminGate, adminTagsRouter); app.use('/api/admin/upload-links', requireAdminGate, adminUploadLinksRouter); +app.use('/api/admin/item-drafts', requireAdminGate, adminItemDraftsRouter); app.use('/api/admin/version', requireAdminGate, adminVersionRouter); app.use('/api/admin', requireAdminGate, adminRouter); app.use('/api/customers/me/addresses', shippingAddressesRouter); diff --git a/backend/src/intake/priceSource.ts b/backend/src/intake/priceSource.ts new file mode 100644 index 0000000..ff87238 --- /dev/null +++ b/backend/src/intake/priceSource.ts @@ -0,0 +1,34 @@ +/** + * Where an item's price came from, and when that changes. + * + * This is the protection that used to live in the schema. Items are priced on + * arrival — the model's suggestion, or the 80.00 default — so nothing stops a + * number nobody chose from reaching the storefront except the review queue + * showing that it was never chosen. + * + * Pure and separately tested because the failure is silent. An item that sells + * at a default price looks exactly like one that sells at a chosen price; + * 80.00 was picked precisely because it reads as a decision rather than as an + * obvious sentinel the way 0.00 would. + */ +export type PriceSource = 'default' | 'ai' | 'admin'; + +/** + * Editing the number is the admin taking responsibility for it, and it is the + * only thing that can. Publishing without touching the field deliberately does + * NOT confirm it — that would turn "I did not look at this" into "I approved + * this", which is the exact misrecording the review queue exists to prevent. + */ +export function nextPriceSource( + current: PriceSource, + submittedCents: number, + storedCents: number +): PriceSource { + if (current === 'admin') return 'admin'; + return submittedCents === storedCents ? current : 'admin'; +} + +/** Anything a person did not choose, which the screen marks visibly. */ +export function isUnconfirmed(source: PriceSource): boolean { + return source !== 'admin'; +} diff --git a/backend/src/routes/adminItemDrafts.ts b/backend/src/routes/adminItemDrafts.ts new file mode 100644 index 0000000..833c4a2 --- /dev/null +++ b/backend/src/routes/adminItemDrafts.ts @@ -0,0 +1,213 @@ +import { Router, Request, Response } from 'express'; +import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { nextPriceSource, PriceSource } from '../intake/priceSource'; + +const router = Router(); + +/** + * The review queue: everything waiting for a person, with what a person needs + * in order to decide. + * + * Columns are spelled out rather than `d.*, i.*` so that a column added later — + * a cost, a token count, an internal error — does not silently start being sent + * to the browser. That matters most for the join to upload_links, which carries + * the token digest: only the label is taken. + * + * Images come back as an aggregate rather than a second round trip, matching + * how itemSelect.ts builds them. + */ +const DRAFT_SELECT = ` + SELECT d.item_id, d.state, d.attempts, d.submitter_note, d.ai_error, + d.ai_name, d.ai_description, d.ai_category_id, d.ai_tag_names, + d.ai_suggested_price_cents, d.price_source, d.model, d.drafted_at, + d.created_at, + i.name AS item_name, i.description AS item_description, + i.price_cents, i.status, + l.label AS upload_link_label, + COALESCE(( + SELECT json_agg(json_build_object('id', img.id, 'image_path', img.image_path) + ORDER BY img.sort_order) + FROM item_images img WHERE img.item_id = d.item_id + ), '[]'::json) AS images + FROM item_drafts d + JOIN items i ON i.id = d.item_id + LEFT JOIN upload_links l ON l.id = d.upload_link_id +`; + +/** + * Discarded rows are excluded by default rather than deleted. + * + * Discard has to be recoverable, because it is one click away in what amounts + * to an inbox — but a discarded row left in the default view would compete for + * attention with work that still needs doing. + */ +router.get( + '/', + asyncRoute(async (req: Request, res: Response) => { + const state = typeof req.query.state === 'string' ? req.query.state : null; + + const { rows } = state + ? await pool.query(`${DRAFT_SELECT} WHERE d.state = $1 ORDER BY d.created_at DESC`, [state]) + : await pool.query(`${DRAFT_SELECT} WHERE d.state <> 'discarded' ORDER BY d.created_at DESC`); + + res.json({ drafts: rows }); + }) +); + +interface DraftPriceRow { + price_source: PriceSource; + price_cents: number; +} + +/** + * Publish: the edited copy goes onto the item, and the item goes live. + * + * The only path from an intake submission to the storefront. It performs what + * mark-available performs — the status, and clearing the sale and reservation + * fields — rather than calling that route, because both halves have to be one + * transaction. An item published carrying the previous draft's name would be a + * worse outcome than one not published at all. + */ +router.post( + '/:itemId/publish', + asyncRoute(async (req: Request, res: Response) => { + const name = typeof req.body?.name === 'string' ? req.body.name.trim() : ''; + const description = typeof req.body?.description === 'string' ? req.body.description.trim() : ''; + const priceCents = Number(req.body?.priceCents); + + if (name === '') { + return res.status(400).json({ error: 'a name is required' }); + } + // Integer because the column is cents. A fractional value would round + // somewhere nobody is looking and sell the item at a price no one entered. + if (!Number.isInteger(priceCents) || priceCents < 0) { + return res.status(400).json({ error: 'a price in whole cents is required' }); + } + + const client = await pool.connect(); + try { + await client.query('BEGIN'); + + // Locked for the length of the transaction, so two admins publishing the + // same submission cannot interleave one's price decision with another's + // name. + const { rows } = await client.query( + `SELECT d.price_source, i.price_cents + FROM item_drafts d JOIN items i ON i.id = d.item_id + WHERE d.item_id = $1 + FOR UPDATE OF d, i`, + [req.params.itemId] + ); + const existing = rows[0]; + if (!existing) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'no draft for this item' }); + } + + const priceSource = nextPriceSource(existing.price_source, priceCents, existing.price_cents); + + await client.query( + `UPDATE items + SET name = $2, description = $3, price_cents = $4, + status = 'available', sold_at = NULL, reserved_until = NULL, paypal_order_id = NULL + WHERE id = $1`, + [req.params.itemId, name, description === '' ? null : description, priceCents] + ); + + await client.query(`UPDATE item_drafts SET price_source = $2 WHERE item_id = $1`, [ + req.params.itemId, + priceSource + ]); + + await client.query('COMMIT'); + res.json({ published: true, priceSource }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } + }) +); + +/** + * Regenerate: hand it back to the worker. + * + * attempts is reset along with the state. The worker only picks up rows below + * the attempt cap, so re-queueing a draft that has already failed three times + * without clearing them produces a button that appears to work, does nothing, + * and leaves nothing anywhere to say why. + */ +router.post( + '/:itemId/regenerate', + asyncRoute(async (req: Request, res: Response) => { + const { rowCount } = await pool.query( + `UPDATE item_drafts SET state = 'queued', attempts = 0, ai_error = NULL WHERE item_id = $1`, + [req.params.itemId] + ); + if (rowCount === 0) return res.status(404).json({ error: 'no draft for this item' }); + res.json({ state: 'queued' }); + }) +); + +/** + * Discard: out of the queue, off the storefront, and entirely recoverable. + * + * Nothing is deleted — not the item, not the photographs. This is one click + * away in what amounts to an inbox, and the photos are often the only copy of + * something no longer in the sender's hands, so the destructive reading of + * "discard" is deliberately not available here. The item returns to pending + * because a discarded submission must not stay on sale. + */ +router.post( + '/:itemId/discard', + asyncRoute(async (req: Request, res: Response) => { + const client = await pool.connect(); + try { + await client.query('BEGIN'); + const { rowCount } = await client.query( + `UPDATE item_drafts SET state = 'discarded' WHERE item_id = $1`, + [req.params.itemId] + ); + if (rowCount === 0) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'no draft for this item' }); + } + await client.query(`UPDATE items SET status = 'pending' WHERE id = $1`, [req.params.itemId]); + await client.query('COMMIT'); + res.json({ state: 'discarded' }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } + }) +); + +/** + * Restore: back into the queue, at the state the draft's own contents justify. + * + * Not unconditionally 'ready'. A submission discarded before it was ever + * drafted has no copy, and returning it as ready would present an empty draft + * as a finished one. Judged on whether a name was ever written, because the + * state it held before being discarded is not stored anywhere. + */ +router.post( + '/:itemId/restore', + asyncRoute(async (req: Request, res: Response) => { + const { rowCount } = await pool.query( + `UPDATE item_drafts + SET state = CASE WHEN ai_name IS NULL THEN 'failed' ELSE 'ready' END + WHERE item_id = $1`, + [req.params.itemId] + ); + if (rowCount === 0) return res.status(404).json({ error: 'no draft for this item' }); + res.json({ restored: true }); + }) +); + +export default router; diff --git a/backend/tests/integration/adminItemDrafts.integration.test.ts b/backend/tests/integration/adminItemDrafts.integration.test.ts new file mode 100644 index 0000000..3c76836 --- /dev/null +++ b/backend/tests/integration/adminItemDrafts.integration.test.ts @@ -0,0 +1,306 @@ +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(); +}); + +interface SeedOptions { + state?: string; + aiName?: string | null; + priceSource?: string; +} + +/** + * A submission as the intake route leaves it: a pending item priced at the + * migration's 8000 default, a draft beside it, and one photo. + */ +async function seedDraft(overrides: SeedOptions = {}): Promise { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Submission 2026-09-01', 'pending') RETURNING id` + ); + const itemId = rows[0]!.id; + + await pool.query( + `INSERT INTO item_drafts (item_id, submitter_note, state, ai_name, ai_description, price_source) + VALUES ($1, 'found in a loft', $2, $3, 'A blue vase.', $4)`, + [ + itemId, + overrides.state ?? 'ready', + overrides.aiName === undefined ? 'Blue vase' : overrides.aiName, + overrides.priceSource ?? 'ai' + ] + ); + await pool.query( + `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, '/uploads/a.jpg', 0)`, + [itemId] + ); + return itemId; +} + +const itemIds = (body: { drafts: { item_id: number }[] }): number[] => + body.drafts.map((draft) => draft.item_id); + +describe('GET /api/admin/item-drafts', () => { + it('returns the draft with its item, photos and note', async () => { + const itemId = await seedDraft(); + + const res = await request(app).get('/api/admin/item-drafts'); + + expect(res.status).toBe(200); + const draft = res.body.drafts.find((d: { item_id: number }) => d.item_id === itemId); + expect(draft).toBeDefined(); + expect(draft.submitter_note).toBe('found in a loft'); + expect(draft.ai_name).toBe('Blue vase'); + expect(draft.price_cents).toBe(8000); + expect(draft.price_source).toBe('ai'); + expect(draft.images).toHaveLength(1); + }); + + // The token digest lives on upload_links and must never be selected into a + // response. Spelling the columns is what prevents that; this is the assertion + // that keeps it spelled. + it('never sends the upload link token digest', async () => { + await seedDraft(); + + const res = await request(app).get('/api/admin/item-drafts'); + + expect(JSON.stringify(res.body)).not.toContain('token_hash'); + }); + + it('filters by state', async () => { + const ready = await seedDraft({ state: 'ready' }); + const failed = await seedDraft({ state: 'failed' }); + + const res = await request(app).get('/api/admin/item-drafts?state=failed'); + + expect(itemIds(res.body)).toContain(failed); + expect(itemIds(res.body)).not.toContain(ready); + }); + + // Discarded is recoverable, so it has to be reachable — but it must not sit + // in the default view competing with work that still needs doing. + it('hides discarded drafts unless they are asked for', async () => { + const discarded = await seedDraft({ state: 'discarded' }); + + const def = await request(app).get('/api/admin/item-drafts'); + expect(itemIds(def.body)).not.toContain(discarded); + + const asked = await request(app).get('/api/admin/item-drafts?state=discarded'); + expect(itemIds(asked.body)).toContain(discarded); + }); + + /** + * The gate is disabled when ADMIN_GATE_SECRET is unset, which is how the rest + * of this suite runs, so it is set here for the length of this test alone. + * Worth asserting: the gate goes on the mount in app.ts rather than inside the + * router, and leaving it off a new mount is a silent hole. + */ + describe('with the admin gate configured', () => { + const original = process.env.ADMIN_GATE_SECRET; + + beforeAll(() => { + process.env.ADMIN_GATE_SECRET = 'integration-secret'; + }); + + afterAll(() => { + if (original === undefined) delete process.env.ADMIN_GATE_SECRET; + else process.env.ADMIN_GATE_SECRET = original; + }); + + it('refuses a request with no gate header', async () => { + await seedDraft(); + const res = await request(app).get('/api/admin/item-drafts'); + expect(res.status).toBe(403); + }); + + it('allows a request carrying the secret', async () => { + await seedDraft(); + const res = await request(app) + .get('/api/admin/item-drafts') + .set('X-Admin-Gate', 'integration-secret'); + expect(res.status).toBe(200); + }); + }); +}); + +describe('POST /api/admin/item-drafts/:itemId/publish', () => { + const body = { name: 'Blue stoneware vase', description: 'Chipped base.', priceCents: 9500 }; + + it('writes the edited copy onto the item and publishes it', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/publish`).send(body); + + expect(res.status).toBe(200); + const { rows } = await pool.query( + `SELECT name, description, price_cents, status FROM items WHERE id = $1`, + [itemId] + ); + expect(rows[0]).toMatchObject({ + name: 'Blue stoneware vase', + description: 'Chipped base.', + price_cents: 9500, + status: 'available' + }); + }); + + // The transition priceSource.ts defines, asserted end to end: a changed + // number is now the admin's responsibility. + it('records an edited price as the admin choice', async () => { + const itemId = await seedDraft(); + + await request(app).post(`/api/admin/item-drafts/${itemId}/publish`).send(body); + + const { rows } = await pool.query(`SELECT price_source FROM item_drafts WHERE item_id = $1`, [ + itemId + ]); + expect(rows[0]?.price_source).toBe('admin'); + }); + + // And the case that matters more: publishing without touching the number must + // leave it recorded as unconfirmed rather than quietly claiming it was chosen. + it('leaves an untouched price unconfirmed', async () => { + const itemId = await seedDraft(); + + await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .send({ ...body, priceCents: 8000 }); + + const { rows } = await pool.query(`SELECT price_source FROM item_drafts WHERE item_id = $1`, [ + itemId + ]); + expect(rows[0]?.price_source).toBe('ai'); + }); + + it('refuses a publish with no name, and leaves the item unpublished', async () => { + const itemId = await seedDraft(); + + const res = await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .send({ ...body, name: ' ' }); + + expect(res.status).toBe(400); + const { rows } = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(rows[0]?.status).toBe('pending'); + }); + + it('refuses a negative price', async () => { + const itemId = await seedDraft(); + const res = await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .send({ ...body, priceCents: -1 }); + expect(res.status).toBe(400); + }); + + it('refuses a fractional price', async () => { + const itemId = await seedDraft(); + const res = await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .send({ ...body, priceCents: 95.5 }); + expect(res.status).toBe(400); + }); + + it('404s for an item with no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary item') RETURNING id` + ); + const res = await request(app) + .post(`/api/admin/item-drafts/${rows[0]!.id}/publish`) + .send(body); + expect(res.status).toBe(404); + }); +}); + +describe('the other three actions', () => { + // Back to queued, and attempts cleared — otherwise a draft that already failed + // three times is re-queued into a state the worker will not pick up, and the + // button does nothing with nothing anywhere to say why. + it('regenerate re-queues a failed draft and clears its attempts', async () => { + const itemId = await seedDraft({ state: 'failed' }); + await pool.query(`UPDATE item_drafts SET attempts = 3, ai_error = 'boom' WHERE item_id = $1`, [ + itemId + ]); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/regenerate`); + + expect(res.status).toBe(200); + const { rows } = await pool.query( + `SELECT state, attempts, ai_error FROM item_drafts WHERE item_id = $1`, + [itemId] + ); + expect(rows[0]).toMatchObject({ state: 'queued', attempts: 0, ai_error: null }); + }); + + it('discard marks the draft and leaves the item unpublished', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/discard`); + + expect(res.status).toBe(200); + const draft = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(draft.rows[0]?.state).toBe('discarded'); + const item = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(item.rows[0]?.status).toBe('pending'); + }); + + // The reason discard is allowed to be a single click. + it('discard does not delete the item or its photos', async () => { + const itemId = await seedDraft(); + + await request(app).post(`/api/admin/item-drafts/${itemId}/discard`); + + const item = await pool.query(`SELECT id FROM items WHERE id = $1`, [itemId]); + expect(item.rows).toHaveLength(1); + const images = await pool.query(`SELECT id FROM item_images WHERE item_id = $1`, [itemId]); + expect(images.rows).toHaveLength(1); + }); + + it('discard unpublishes an item that had already been published', async () => { + const itemId = await seedDraft(); + await pool.query(`UPDATE items SET status = 'available' WHERE id = $1`, [itemId]); + + await request(app).post(`/api/admin/item-drafts/${itemId}/discard`); + + const item = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(item.rows[0]?.status).toBe('pending'); + }); + + it('restore brings a discarded draft back as ready', async () => { + const itemId = await seedDraft({ state: 'discarded' }); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/restore`); + + expect(res.status).toBe(200); + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('ready'); + }); + + // A submission discarded before it was ever drafted has no copy, and must not + // return claiming to have one. + it('restore returns an undrafted submission to failed, not ready', async () => { + const itemId = await seedDraft({ state: 'discarded', aiName: null }); + + await request(app).post(`/api/admin/item-drafts/${itemId}/restore`); + + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('failed'); + }); + + it('404s each action for an item with no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary item') RETURNING id` + ); + for (const action of ['regenerate', 'discard', 'restore']) { + const res = await request(app).post(`/api/admin/item-drafts/${rows[0]!.id}/${action}`); + expect(res.status).toBe(404); + } + }); +}); diff --git a/backend/tests/unit/priceSource.test.ts b/backend/tests/unit/priceSource.test.ts new file mode 100644 index 0000000..d967d35 --- /dev/null +++ b/backend/tests/unit/priceSource.test.ts @@ -0,0 +1,32 @@ +import { nextPriceSource, isUnconfirmed } from '../../src/intake/priceSource'; + +describe('nextPriceSource', () => { + // Touching the number is the admin taking responsibility for it. That is the + // only event that can confirm a price. + it('becomes admin when the number changes', () => { + expect(nextPriceSource('default', 9500, 8000)).toBe('admin'); + expect(nextPriceSource('ai', 4000, 4500)).toBe('admin'); + }); + + // Publishing without touching the field must NOT silently confirm it. That + // is the whole failure this screen exists to prevent: an item selling at a + // number nobody chose, with nothing recording that. + it('leaves an untouched price unconfirmed', () => { + expect(nextPriceSource('default', 8000, 8000)).toBe('default'); + expect(nextPriceSource('ai', 4500, 4500)).toBe('ai'); + }); + + // Already confirmed stays confirmed, including when re-submitted unchanged. + it('keeps admin once set', () => { + expect(nextPriceSource('admin', 9500, 9500)).toBe('admin'); + expect(nextPriceSource('admin', 7000, 9500)).toBe('admin'); + }); +}); + +describe('isUnconfirmed', () => { + it('treats anything but admin as unconfirmed', () => { + expect(isUnconfirmed('default')).toBe(true); + expect(isUnconfirmed('ai')).toBe(true); + expect(isUnconfirmed('admin')).toBe(false); + }); +}); diff --git a/docs/superpowers/plans/2026-09-01-intake-notification.md b/docs/superpowers/plans/2026-09-01-intake-notification.md new file mode 100644 index 0000000..417d144 --- /dev/null +++ b/docs/superpowers/plans/2026-09-01-intake-notification.md @@ -0,0 +1,941 @@ +# Intake Notification Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** When a draft is ready, the admin gets an email with the drafted copy, a link into the review queue, and two signed links that can regenerate or discard it without signing in. + +**Architecture:** A pure HMAC signer, a new editable email template, a notification sent by the drafting worker after it writes a draft, and a small public router that verifies a signature and performs one of two state changes. Nothing in the email can publish. + +**Tech Stack:** Express 4, TypeScript, `crypto` (HMAC), nodemailer via the existing mailer, Jest + supertest. + +**Spec:** `docs/superpowers/specs/2026-08-29-intake-pipeline-design.md` (issue #224, slice 3 of #220) + +**Depends on #225**, which is on `feature/225-review-queue` and not yet merged. The review link has nowhere to land without it, and the two actions perform the same transitions its admin routes already perform. Branch from `feature/225-review-queue`, not `main`. + +**Already verified against the tree, so no need to re-check:** `TemplateKey` is a union in `src/emailTemplates.ts:14`; each entry of `TEMPLATES` has `label`, `required`, `available`, `defaultSubject`, `defaultBody`, and an optional `footer`. `SAMPLE_VALUES` must gain an entry for every new `available` name — a unit test asserts this. `renderTemplate(key, stored, values)` returns `{ subject, html }`. Callers send fire-and-forget: `sendMail(to, subject, html).catch(err => console.error(...))`. Admin settings are rows in the `DEFINITIONS` array in `src/adminSettings.ts:23` typed `hours | text | choice`; adding one means adding a row and nothing else. There is no HMAC anywhere yet; `src/middleware/adminGate.ts:48` shows the timing-safe comparison idiom — hash both sides first, because `timingSafeEqual` throws on buffers of different length. `PUBLIC_URL` is read as `process.env.PUBLIC_URL ?? ''` (`favoriteAlerts.ts:58`). + +## Global Constraints + +- **The email cannot publish.** The two signed actions are exactly the ones whose worst case is a wasted API call or a recoverable hide. Publishing stays a deliberate act on a screen showing the price — that is what bounds the risk taken by pricing items on arrival. +- **SMTP being down must not strand a draft.** The queue is the source of truth: a `ready` draft is visible and actionable whether or not its notification ever sent. Send fire-and-forget with a logged catch, exactly like `customers.ts:54`. +- **`INTAKE_ACTION_SECRET` is optional.** Absent, the notification still sends with its review link and simply omits the two signed links, and boot warns. A missing secret must not stop the admin being told an item arrived. +- **Signatures are timing-safe and expire in 30 days.** Signed over `(itemId, action, expiry)`. +- **`reviewUrl` is a required placeholder.** A notification with no link in it still sends, still looks fine in the log, and is useless to whoever receives it — which is what the required-placeholder validation exists to catch. +- **Every route handler wrapped in `asyncRoute`** — `tests/unit/routesAreWrapped.test.ts` enforces it. +- **QA silently drops mail to unlisted addresses.** `MAIL_ALLOWLIST=thomlamb@gmail.com` is hardcoded in `docker-compose.qa.yml` and is the entire safety property there. Testing this in QA against any other address looks like a silent failure — the flow succeeds and no mail arrives, with a `[mail-blocked]` line naming the address. +- **Commit style:** Conventional Commits, subject ending `(#224)`, no hard wrapping in bodies. + +## The decision this plan makes that the issue does not + +**Email clients and security scanners prefetch links.** Outlook Safe Links, Gmail's scanners, and most corporate mail gateways issue a GET against every URL in a message before a human sees it. A `GET /intake-actions/discard?...` would therefore fire itself on delivery, and the admin would find drafts discarded that nobody touched — with a valid signature in the logs saying it was legitimate. + +So the signed link is a **GET that renders a confirmation page, and a POST that performs the action**. The GET is safe and idempotent, which is what makes it survive a prefetch; the POST carries the same signature and is what actually changes state. + +This costs one extra click. It is worth it: the alternative is a destructive action that a mail scanner can trigger, which no amount of recoverability makes acceptable, because nobody would know to go and recover it. It is also cheap to reverse if you would rather have one click — the verification and the transition are unchanged, only the handler that performs them moves. + +## One wording difference from the issue + +The issue says the signature covers `(draftId, action, expiry)`. This signs `(itemId, action, expiry)` instead. `item_drafts.item_id` is `UNIQUE` and is the key everything else addresses a draft by — the #225 routes are all `/:itemId/...` — so there is no separate draft id in circulation, and introducing one only for the signature would mean two ways to name the same row. Same property, same guarantees. + +## File Structure + +**Created:** +- `backend/src/intake/actionLinks.ts` — sign and verify. Pure. +- `backend/src/intake/notifyDraft.ts` — build the values and send. +- `backend/src/routes/intakeActions.ts` — the public GET/POST pair. +- `backend/tests/unit/actionLinks.test.ts` +- `backend/tests/integration/intakeActions.integration.test.ts` + +**Modified:** +- `backend/src/emailTemplates.ts` — the `intakeDraft` template and its samples +- `backend/src/adminSettings.ts` — where the notification goes +- `backend/src/intake/draftingWorker.ts` — send after a successful draft +- `backend/src/envValidation.ts` — warn when the secret is absent +- `backend/src/app.ts` — mount the public router + +--- + +### Task 1: Signing and verifying + +**Files:** +- Create: `backend/src/intake/actionLinks.ts`, `backend/tests/unit/actionLinks.test.ts` + +**Interfaces:** +- Produces: `type IntakeAction = 'regenerate' | 'discard'`, `signAction(itemId, action, expiresAt): string`, `verifyAction(itemId, action, expiresAt, signature, now?): boolean`, `actionUrl(itemId, action): string | null`, `ACTION_TTL_MS` + +- [ ] **Step 1: Write the failing test** + +```ts +import { + signAction, + verifyAction, + actionUrl, + ACTION_TTL_MS +} from '../../src/intake/actionLinks'; + +const SECRET = 'test-intake-secret'; +const NOW = 1_800_000_000_000; +const EXPIRY = NOW + ACTION_TTL_MS; + +beforeEach(() => { + process.env.INTAKE_ACTION_SECRET = SECRET; + process.env.PUBLIC_URL = 'https://shop.example.com'; +}); + +describe('signAction / verifyAction', () => { + it('accepts a signature it produced', () => { + const sig = signAction(7, 'discard', EXPIRY); + expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(true); + }); + + // Each of these is a different link. A signature that survives any of these + // swaps is a signature that authorises more than it names. + it('refuses a signature reused for another item', () => { + const sig = signAction(7, 'discard', EXPIRY); + expect(verifyAction(8, 'discard', EXPIRY, sig, NOW)).toBe(false); + }); + + it('refuses a signature reused for another action', () => { + const sig = signAction(7, 'discard', EXPIRY); + expect(verifyAction(7, 'regenerate', EXPIRY, sig, NOW)).toBe(false); + }); + + // Otherwise the expiry is decoration: anyone holding an expired link could + // extend it themselves by editing the timestamp. + it('refuses a signature whose expiry was altered', () => { + const sig = signAction(7, 'discard', EXPIRY); + expect(verifyAction(7, 'discard', EXPIRY + 1000, sig, NOW)).toBe(false); + }); + + it('refuses an expired link even with a valid signature', () => { + const sig = signAction(7, 'discard', EXPIRY); + expect(verifyAction(7, 'discard', EXPIRY, sig, EXPIRY + 1)).toBe(false); + }); + + it('refuses a malformed signature without throwing', () => { + expect(verifyAction(7, 'discard', EXPIRY, 'not-a-signature', NOW)).toBe(false); + expect(verifyAction(7, 'discard', EXPIRY, '', NOW)).toBe(false); + }); + + // A different secret must not validate. This is what makes rotating the + // secret revoke every outstanding link. + it('refuses a signature made with a different secret', () => { + const sig = signAction(7, 'discard', EXPIRY); + process.env.INTAKE_ACTION_SECRET = 'a-different-secret'; + expect(verifyAction(7, 'discard', EXPIRY, sig, NOW)).toBe(false); + }); +}); + +describe('actionUrl', () => { + it('builds an absolute url carrying the expiry and signature', () => { + const url = actionUrl(7, 'discard'); + expect(url).toContain('https://shop.example.com/api/intake-actions/7/discard'); + expect(url).toMatch(/expires=\d+/); + expect(url).toMatch(/sig=[A-Za-z0-9_-]+/); + }); + + // Absent secret is a working configuration: the notification still sends with + // its review link. A link that cannot be verified must never be offered. + it('returns null when there is no secret', () => { + delete process.env.INTAKE_ACTION_SECRET; + expect(actionUrl(7, 'discard')).toBeNull(); + }); + + it('returns null when there is no public url to build against', () => { + delete process.env.PUBLIC_URL; + expect(actionUrl(7, 'discard')).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.unit.config.js actionLinks +``` + +Expected: FAIL — module not found. + +- [ ] **Step 3: Write it** + +```ts +import crypto from 'crypto'; + +/** + * Links in the notification email that act without a login. + * + * Only two actions are signable, and neither can publish. The worst case of a + * leaked link is a wasted API call or a hide that the review queue can undo — + * which is what makes it acceptable to put them in an inbox at all. + * + * Signed over the item, the action and the expiry together. Signing any subset + * would let a link be replayed against a different item or upgraded to a + * different action, and leaving the expiry out of the payload would let anyone + * holding an expired link extend it by editing the timestamp. + */ +export type IntakeAction = 'regenerate' | 'discard'; + +/** Thirty days. Long enough to survive a holiday, short enough to lapse. */ +export const ACTION_TTL_MS = 30 * 24 * 60 * 60 * 1000; + +function secret(): string | null { + const value = process.env.INTAKE_ACTION_SECRET; + return value && value.trim() !== '' ? value : null; +} + +export function signAction(itemId: number, action: IntakeAction, expiresAt: number): string { + const key = secret(); + if (!key) throw new Error('INTAKE_ACTION_SECRET is not set'); + return crypto + .createHmac('sha256', key) + .update(`${itemId}:${action}:${expiresAt}`) + .digest('base64url'); +} + +/** + * Compared through a second digest rather than directly, because + * timingSafeEqual throws when the two buffers differ in length — and a + * malformed signature from a truncated link is an ordinary thing to receive, + * not an exception. Same idiom as middleware/adminGate.ts. + */ +function digest(value: string): Buffer { + return crypto.createHash('sha256').update(value).digest(); +} + +export function verifyAction( + itemId: number, + action: IntakeAction, + expiresAt: number, + signature: string, + now: number = Date.now() +): boolean { + if (!secret()) return false; + if (!Number.isFinite(expiresAt) || now > expiresAt) return false; + + const expected = signAction(itemId, action, expiresAt); + return crypto.timingSafeEqual(digest(expected), digest(signature)); +} + +/** + * The absolute link, or null when one cannot be made. + * + * Null rather than a throw or a relative path. An unconfigured environment + * still sends the notification with its review link — being told an item + * arrived matters more than the shortcuts — and a link that could not be + * verified must never be offered in the first place. + */ +export function actionUrl(itemId: number, action: IntakeAction): string | null { + const base = process.env.PUBLIC_URL; + if (!secret() || !base || base.trim() === '') return null; + + const expiresAt = Date.now() + ACTION_TTL_MS; + const sig = signAction(itemId, action, expiresAt); + const origin = base.replace(/\/+$/, ''); + return `${origin}/api/intake-actions/${itemId}/${action}?expires=${expiresAt}&sig=${sig}`; +} +``` + +- [ ] **Step 4: Run it to verify it passes** + +```bash +cd backend && npx jest -c jest.unit.config.js actionLinks +``` + +Expected: PASS, 10 tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/intake/actionLinks.ts backend/tests/unit/actionLinks.test.ts +git commit -m "feat(intake): sign the two actions an email may take (#224)" +``` + +--- + +### Task 2: The template and the recipient + +**Files:** +- Modify: `backend/src/emailTemplates.ts`, `backend/src/adminSettings.ts`, `backend/src/envValidation.ts` +- Test: `backend/tests/unit/emailTemplates.test.ts`, `backend/tests/unit/envValidation.test.ts` + +**Interfaces:** +- Produces: `TemplateKey` gains `'intakeDraft'`; `SettingName` gains `'intakeNotifyEmail'` + +- [ ] **Step 1: Write the failing test** + +Add to `backend/tests/unit/emailTemplates.test.ts`: + +```ts +describe('the intake notification template', () => { + // Without the review link the email is a notification you cannot act on. + it('requires the review url', () => { + expect(missingPlaceholders('intakeDraft', 'An item arrived.')).toContain('reviewUrl'); + }); + + it('accepts a body carrying the review url', () => { + expect(missingPlaceholders('intakeDraft', 'Review it: {{reviewUrl}}')).toEqual([]); + }); + + // The signed links are optional in the body: they are absent whenever + // INTAKE_ACTION_SECRET is unset, and a template that demanded them would make + // an unconfigured environment unable to send at all. + it('does not require the signed action links', () => { + expect(missingPlaceholders('intakeDraft', '{{reviewUrl}}')).not.toContain('discardUrl'); + expect(missingPlaceholders('intakeDraft', '{{reviewUrl}}')).not.toContain('regenerateUrl'); + }); + + it('offers the drafted copy to the template author', () => { + const available = TEMPLATES.intakeDraft.available; + for (const name of ['itemName', 'draftName', 'draftDescription', 'price', 'submitterNote']) { + expect(available).toContain(name); + } + }); +}); +``` + +The existing test that every `available` name has a `SAMPLE_VALUES` entry will fail until the samples are added — that is the point of it. + +Add to `backend/tests/unit/envValidation.test.ts`: + +```ts +describe('the intake action secret', () => { + it('is not required', () => { + expect(validateEnv(MINIMAL).errors).toEqual([]); + }); + + it('warns when it is absent', () => { + expect(validateEnv(MINIMAL).warnings.join(' ')).toMatch(/INTAKE_ACTION_SECRET/); + }); + + it('says nothing when it is set', () => { + const { warnings } = validateEnv(withEnv({ INTAKE_ACTION_SECRET: 'a-secret' })); + expect(warnings.join(' ')).not.toMatch(/INTAKE_ACTION_SECRET/); + }); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +```bash +cd backend && npx jest -c jest.unit.config.js emailTemplates envValidation +``` + +Expected: FAIL — `intakeDraft` is not a `TemplateKey`, and no such warning. + +- [ ] **Step 3: Add the template** + +In `src/emailTemplates.ts`, extend the union: + +```ts +export type TemplateKey = + | 'verification' + | 'passwordReset' + | 'favoriteSold' + | 'favoriteWithdrawn' + | 'cartReminder' + | 'emailChanged' + | 'intakeDraft'; +``` + +and add to `TEMPLATES`: + +```ts + intakeDraft: { + label: 'Item submitted for review', + // Only the review link. The signed shortcuts are absent whenever + // INTAKE_ACTION_SECRET is unset, and requiring them would make an + // unconfigured environment unable to send this at all. + required: ['reviewUrl'], + available: [ + 'itemName', + 'draftName', + 'draftDescription', + 'price', + 'submitterNote', + 'linkLabel', + 'reviewUrl', + 'regenerateUrl', + 'discardUrl' + ], + defaultSubject: 'An item was submitted: {{draftName}}', + defaultBody: + 'Someone sent in an item through {{linkLabel}}.\n\n' + + '**{{draftName}}**\n\n' + + '{{draftDescription}}\n\n' + + 'Suggested price: {{price}}\n\n' + + "The sender's note: {{submitterNote}}\n\n" + + '[Review and publish it]({{reviewUrl}})\n\n' + + 'Nothing is listed until you publish it from that screen, and the price ' + + 'above is a suggestion rather than a decision.\n\n' + + '[Ask for another draft]({{regenerateUrl}}) — [Discard it]({{discardUrl}})' + } +``` + +and to `SAMPLE_VALUES`: + +```ts + draftName: 'Blue stoneware vase', + draftDescription: 'A hand-thrown vase with a chipped base.', + price: '$80.00', + submitterNote: 'Found in a loft clearance.', + linkLabel: 'Autumn drop-off', + reviewUrl: 'https://example.com/admin?tab=review-queue', + regenerateUrl: 'https://example.com/api/intake-actions/1/regenerate?expires=0&sig=sample', + discardUrl: 'https://example.com/api/intake-actions/1/discard?expires=0&sig=sample', +``` + +- [ ] **Step 4: Add the recipient setting** + +In `src/adminSettings.ts`, add a row to `DEFINITIONS`: + +```ts + // Where the intake notification goes (#224). A setting rather than an + // environment variable, for the same reason drafting_model is one: it is + // changed by the person running the shop, not by whoever deploys it, and a + // redeploy to change an address would be absurd. Empty means do not notify, + // which is a working configuration and the default. + { key: 'intake_notify_email', name: 'intakeNotifyEmail', type: 'text', fallback: '' } +``` + +- [ ] **Step 5: Warn about the missing secret** + +In `src/envValidation.ts`, beside `checkDraftingKey`: + +```ts +// Optional, like the drafting key. Absent, the notification still sends with +// its review link and simply carries no shortcuts — being told an item arrived +// matters far more than being able to discard it in one click. +function checkIntakeActionSecret(env: NodeJS.ProcessEnv): string[] { + if (isPresent(env, 'INTAKE_ACTION_SECRET')) return []; + return [ + 'INTAKE_ACTION_SECRET is not set — intake notifications will link to the review queue ' + + 'but carry no regenerate or discard shortcuts.' + ]; +} +``` + +and add `...checkIntakeActionSecret(env)` to the `warnings` array in `validateEnv`. + +- [ ] **Step 6: Verify** + +```bash +cd backend && npm run test:unit && npm run lint && npm run build +``` + +Expected: PASS. If the `SAMPLE_VALUES` completeness test fails, a name in `available` has no sample — add it rather than removing the name. + +- [ ] **Step 7: Commit** + +```bash +git add backend/src/emailTemplates.ts backend/src/adminSettings.ts backend/src/envValidation.ts backend/tests/unit +git commit -m "feat(intake): add the submission notification template (#224)" +``` + +--- + +### Task 3: Sending it + +**Files:** +- Create: `backend/src/intake/notifyDraft.ts` +- Modify: `backend/src/intake/draftingWorker.ts` + +**Interfaces:** +- Consumes: `actionUrl` (Task 1), the `intakeDraft` template (Task 2) +- Produces: `notifyDraftReady(itemId: number): Promise` + +- [ ] **Step 1: Write it** + +```ts +import { pool } from '../db'; +import { sendMail } from '../mailer'; +import { renderTemplate } from '../emailTemplates'; +import { loadStoredTemplate } from '../routes/adminEmailTemplates'; +import { getSettings } from '../adminSettings'; +import { actionUrl } from './actionLinks'; + +interface NotifyRow { + item_name: string; + price_cents: number; + ai_name: string | null; + ai_description: string | null; + submitter_note: string | null; + link_label: string | null; +} + +/** + * Tells the admin an item arrived and is drafted. + * + * Everything here is best-effort by design. The review queue is the source of + * truth: a ready draft is visible and actionable whether or not this ever sent, + * so a missing recipient, an SMTP outage or a template that will not render + * must all end in a log line rather than an exception that reaches the worker + * and marks a perfectly good draft as failed. + */ +export async function notifyDraftReady(itemId: number): Promise { + const { intakeNotifyEmail } = await getSettings(); + const to = intakeNotifyEmail?.trim(); + if (!to) { + // Not an error. Nobody has said where to send it, and the draft is waiting + // in the queue regardless. + return; + } + + const { rows } = await pool.query( + `SELECT i.name AS item_name, i.price_cents, + d.ai_name, d.ai_description, d.submitter_note, + l.label AS link_label + FROM item_drafts d + JOIN items i ON i.id = d.item_id + LEFT JOIN upload_links l ON l.id = d.upload_link_id + WHERE d.item_id = $1`, + [itemId] + ); + const row = rows[0]; + if (!row) return; + + const base = (process.env.PUBLIC_URL ?? '').replace(/\/+$/, ''); + const regenerate = actionUrl(itemId, 'regenerate'); + const discard = actionUrl(itemId, 'discard'); + + const template = renderTemplate('intakeDraft', await loadStoredTemplate('intakeDraft'), { + itemName: row.item_name, + draftName: row.ai_name ?? row.item_name, + // Said plainly rather than left blank. An empty description in a + // notification reads as a bug; "not drafted" reads as the fact it is. + draftDescription: row.ai_description ?? 'No description was drafted for this item.', + price: `$${(row.price_cents / 100).toFixed(2)}`, + submitterNote: row.submitter_note ?? 'The sender left no note.', + linkLabel: row.link_label ?? 'an upload link', + reviewUrl: `${base}/admin`, + // Empty rather than a broken link when there is no secret to sign with. + regenerateUrl: regenerate ?? '', + discardUrl: discard ?? '' + }); + + await sendMail(to, template.subject, template.html); +} +``` + +`loadStoredTemplate` is exported from `src/routes/adminEmailTemplates.ts` and is how `favoriteAlerts.ts:57` and `customers.ts` already load a stored template. Use it rather than querying a table directly — it is the only place that knows where stored overrides live. + +- [ ] **Step 2: Call it from the worker** + +In `src/intake/draftingWorker.ts`, immediately after the transaction that applies a draft commits and `drafted++` runs: + +```ts + // Fire and forget, and deliberately outside the transaction. A mail + // failure must never roll back a draft that was written correctly, and + // the queue is what the admin actually works from — the email is a + // convenience on top of it. + void notifyDraftReady(row.item_id).catch((err) => + console.error(`[drafting] notifying for item ${row.item_id}:`, err) + ); +``` + +with the import at the top: + +```ts +import { notifyDraftReady } from './notifyDraft'; +``` + +- [ ] **Step 3: Verify** + +```bash +cd backend && npm run build && npm run lint && npm run test:unit +npx jest -c jest.integration.config.js --runInBand drafting +``` + +Expected: all pass. The drafting integration tests run with no recipient configured, so `notifyDraftReady` returns before touching the mailer — which is the path that must not break them. + +- [ ] **Step 4: Commit** + +```bash +git add backend/src/intake/notifyDraft.ts backend/src/intake/draftingWorker.ts +git commit -m "feat(intake): tell the admin when a draft is ready (#224)" +``` + +--- + +### Task 4: Acting on a signed link + +**Files:** +- Create: `backend/src/routes/intakeActions.ts`, `backend/tests/integration/intakeActions.integration.test.ts` +- Modify: `backend/src/app.ts` + +**Interfaces:** +- Consumes: `verifyAction`, `IntakeAction` (Task 1) +- Produces: `GET /api/intake-actions/:itemId/:action` (confirmation page), `POST /api/intake-actions/:itemId/:action` (performs it) + +- [ ] **Step 1: Write the failing test** + +```ts +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; +import { signAction, ACTION_TTL_MS } from '../../src/intake/actionLinks'; + +const SECRET = 'integration-intake-secret'; +const original = process.env.INTAKE_ACTION_SECRET; + +beforeAll(() => { + process.env.INTAKE_ACTION_SECRET = SECRET; +}); + +afterAll(async () => { + if (original === undefined) delete process.env.INTAKE_ACTION_SECRET; + else process.env.INTAKE_ACTION_SECRET = original; + await pool.end(); + await closeDb(); +}); + +beforeEach(async () => { + await resetDb(); +}); + +async function seedDraft(): Promise { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Submission', 'pending') RETURNING id` + ); + const itemId = rows[0]!.id; + await pool.query( + `INSERT INTO item_drafts (item_id, state, ai_name) VALUES ($1, 'ready', 'Blue vase')`, + [itemId] + ); + return itemId; +} + +function link(itemId: number, action: 'regenerate' | 'discard', expiresAt: number): string { + const sig = signAction(itemId, action, expiresAt); + return `/api/intake-actions/${itemId}/${action}?expires=${expiresAt}&sig=${sig}`; +} + +const soon = () => Date.now() + ACTION_TTL_MS; + +describe('the signed action links', () => { + /** + * The reason GET does not act. Mail scanners and Safe Links issue a GET + * against every URL in a message before a human sees it, so a GET that + * discarded a draft would fire itself on delivery — with a valid signature, + * looking entirely legitimate in the log. + */ + it('GET confirms without changing anything', async () => { + const itemId = await seedDraft(); + + const res = await request(app).get(link(itemId, 'discard', soon())); + + expect(res.status).toBe(200); + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('ready'); + }); + + it('POST discards', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post(link(itemId, 'discard', soon())); + + expect(res.status).toBe(200); + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('discarded'); + const item = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(item.rows[0]?.status).toBe('pending'); + }); + + it('POST regenerates', async () => { + const itemId = await seedDraft(); + await pool.query(`UPDATE item_drafts SET attempts = 3 WHERE item_id = $1`, [itemId]); + + await request(app).post(link(itemId, 'regenerate', soon())); + + const { rows } = await pool.query( + `SELECT state, attempts FROM item_drafts WHERE item_id = $1`, + [itemId] + ); + expect(rows[0]).toMatchObject({ state: 'queued', attempts: 0 }); + }); + + it('refuses a tampered signature', async () => { + const itemId = await seedDraft(); + const expires = soon(); + + const res = await request(app).post( + `/api/intake-actions/${itemId}/discard?expires=${expires}&sig=forged` + ); + + expect(res.status).toBe(403); + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('ready'); + }); + + // The signature names the item, so one link must not act on another. + it('refuses a signature minted for a different item', async () => { + const mine = await seedDraft(); + const other = await seedDraft(); + const expires = soon(); + const sig = signAction(other, 'discard', expires); + + const res = await request(app).post( + `/api/intake-actions/${mine}/discard?expires=${expires}&sig=${sig}` + ); + + expect(res.status).toBe(403); + }); + + it('refuses an expired link', async () => { + const itemId = await seedDraft(); + const expired = Date.now() - 1000; + + const res = await request(app).post(link(itemId, 'discard', expired)); + + expect(res.status).toBe(403); + }); + + it('refuses an action it does not recognise', async () => { + const itemId = await seedDraft(); + const expires = soon(); + + const res = await request(app).post( + `/api/intake-actions/${itemId}/publish?expires=${expires}&sig=anything` + ); + + expect(res.status).toBe(404); + }); + + it('404s for an item with no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary') RETURNING id` + ); + const res = await request(app).post(link(rows[0]!.id, 'discard', soon())); + expect(res.status).toBe(404); + }); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand intakeActions +``` + +Expected: FAIL — 404 everywhere, the router does not exist. + +- [ ] **Step 3: Write the router** + +```ts +import { Router, Request, Response } from 'express'; +import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; +import { IntakeAction, verifyAction } from '../intake/actionLinks'; + +const router = Router(); + +const ACTIONS: readonly IntakeAction[] = ['regenerate', 'discard']; + +function isAction(value: string): value is IntakeAction { + return (ACTIONS as readonly string[]).includes(value); +} + +/** + * Public, and protected by the signature rather than by the admin gate. + * + * These are clicked from an inbox by someone who is not signed in, which is the + * whole point. Neither action can publish: the worst outcome of a leaked link + * is a wasted API call or a hide the review queue can undo, and that is exactly + * what makes putting them in an email acceptable. + */ +interface Checked { + itemId: number; + action: IntakeAction; +} + +function check(req: Request, res: Response): Checked | null { + const action = req.params.action ?? ''; + if (!isAction(action)) { + res.status(404).json({ error: 'unknown action' }); + return null; + } + + const itemId = Number(req.params.itemId); + const expiresAt = Number(req.query.expires); + const sig = typeof req.query.sig === 'string' ? req.query.sig : ''; + + if (!Number.isInteger(itemId) || !verifyAction(itemId, action, expiresAt, sig)) { + // One response for a forged signature, an expired link and a missing + // secret alike. Distinguishing them would tell someone probing which of + // those they had achieved. + res.status(403).json({ error: 'this link is not valid, or has expired' }); + return null; + } + + return { itemId, action }; +} + +/** + * Confirms, and changes nothing. + * + * Mail scanners and corporate link-rewriting gateways issue a GET against every + * URL in a message before a human ever sees it. A GET that discarded a draft + * would therefore fire itself on delivery, with a valid signature, looking + * entirely legitimate. So the state change lives on POST and this page exists + * only to let a person confirm it. + */ +router.get( + '/:itemId/:action', + asyncRoute(async (req: Request, res: Response) => { + const checked = check(req, res); + if (!checked) return; + + const { rows } = await pool.query<{ item_name: string }>( + `SELECT i.name AS item_name FROM item_drafts d JOIN items i ON i.id = d.item_id + WHERE d.item_id = $1`, + [checked.itemId] + ); + if (!rows[0]) return res.status(404).json({ error: 'no draft for this item' }); + + res.json({ + itemId: checked.itemId, + action: checked.action, + itemName: rows[0].item_name, + confirmWith: 'POST to this same url' + }); + }) +); + +router.post( + '/:itemId/:action', + asyncRoute(async (req: Request, res: Response) => { + const checked = check(req, res); + if (!checked) return; + + if (checked.action === 'regenerate') { + const { rowCount } = await pool.query( + `UPDATE item_drafts SET state = 'queued', attempts = 0, ai_error = NULL WHERE item_id = $1`, + [checked.itemId] + ); + if (rowCount === 0) return res.status(404).json({ error: 'no draft for this item' }); + return res.json({ state: 'queued' }); + } + + const client = await pool.connect(); + try { + await client.query('BEGIN'); + const { rowCount } = await client.query( + `UPDATE item_drafts SET state = 'discarded' WHERE item_id = $1`, + [checked.itemId] + ); + if (rowCount === 0) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'no draft for this item' }); + } + await client.query(`UPDATE items SET status = 'pending' WHERE id = $1`, [checked.itemId]); + await client.query('COMMIT'); + res.json({ state: 'discarded' }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } + }) +); + +export default router; +``` + +- [ ] **Step 4: Mount it, publicly** + +In `src/app.ts`, beside the other public routers — **not** behind `requireAdminGate`, which would defeat the purpose: + +```ts +import intakeActionsRouter from './routes/intakeActions'; +``` + +```ts +app.use('/api/intake-actions', intakeActionsRouter); +``` + +Put it next to `app.use('/api/intake', intakeRouter);` so the two public intake surfaces sit together. + +- [ ] **Step 5: Run to verify it passes** + +```bash +cd backend +npx jest -c jest.integration.config.js --runInBand intakeActions +npx jest -c jest.unit.config.js routesAreWrapped +npm run build && npm run lint +``` + +Expected: PASS, 8 integration tests, the wrapper guard green. + +- [ ] **Step 6: Commit** + +```bash +git add backend/src/routes/intakeActions.ts backend/src/app.ts backend/tests/integration/intakeActions.integration.test.ts +git commit -m "feat(intake): act on a signed link from the notification (#224)" +``` + +--- + +### Task 5: The whole path, once + +**Files:** +- Modify: `backend/tests/integration/intakeActions.integration.test.ts` + +- [ ] **Step 1: Add the end-to-end integration test** + +```ts +import { notifyDraftReady } from '../../src/intake/notifyDraft'; + +describe('the notification itself', () => { + // Nowhere to send it is a working configuration, and must not throw into the + // worker and fail a draft that was written correctly. + it('does nothing when no recipient is configured', async () => { + const itemId = await seedDraft(); + await expect(notifyDraftReady(itemId)).resolves.toBeUndefined(); + }); + + it('does not throw when the item has no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary') RETURNING id` + ); + await expect(notifyDraftReady(rows[0]!.id)).resolves.toBeUndefined(); + }); +}); +``` + +- [ ] **Step 2: Run the whole backend** + +```bash +cd backend +npm run test:unit +npx jest -c jest.integration.config.js --runInBand +npm run lint && npm run build +``` + +Expected: everything passes. + +- [ ] **Step 3: Commit** + +```bash +git add backend/tests/integration/intakeActions.integration.test.ts +git commit -m "test(intake): cover the notification's quiet paths (#224)" +``` + +--- + +## Done when + +- A draft becoming `ready` sends one email to the configured address, carrying the drafted name, description, suggested price and the sender's note. +- The email links into the review queue, and offers regenerate and discard as signed links. +- The email contains no way to publish. +- A signed link cannot be replayed against a different item, upgraded to a different action, extended past its expiry, or forged. +- A GET on a signed link changes nothing, so a mail scanner cannot act on the admin's behalf. +- With no recipient configured, or no `INTAKE_ACTION_SECRET`, or SMTP down, the draft is still `ready` and actionable in the queue. +- Unit, integration, lint and build all pass. + +## Not in this plan + +A rendered confirmation page. The GET returns JSON describing what the link would do; making it a styled page that POSTs on a button press is frontend work worth its own issue, and the security property — that GET does not act — is already in place without it. + +Manual verification against real SMTP. Worth doing once in QA, remembering that `MAIL_ALLOWLIST` there silently drops anything but the allowlisted address. diff --git a/docs/superpowers/plans/2026-09-01-intake-review-queue.md b/docs/superpowers/plans/2026-09-01-intake-review-queue.md new file mode 100644 index 0000000..afb1dc1 --- /dev/null +++ b/docs/superpowers/plans/2026-09-01-intake-review-queue.md @@ -0,0 +1,1198 @@ +# Intake Review Queue Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** An admin can see every submitted item with its photos, note and drafted copy, edit it, and publish it — or send it back for another draft, or discard it. + +**Architecture:** One new admin router at `/api/admin/item-drafts` exposing a list and four actions, and one new admin tab that renders it. Publishing writes the edited fields onto the item and sets it available; nothing else in the app moves an intake item to the storefront. + +**Tech Stack:** Express 4, TypeScript, `pg`, Jest + supertest, React, antd, Playwright. + +**Spec:** `docs/superpowers/specs/2026-08-29-intake-pipeline-design.md` (issue #225, slice 4 of #220) + +**Already verified against the tree, so no need to re-check:** `item_drafts` has `item_id` (UNIQUE, FK CASCADE), `upload_link_id`, `submitter_note`, `state`, `attempts`, `model`, `ai_name`, `ai_description`, `ai_category_id`, `ai_tag_names`, `ai_suggested_price_cents`, `price_source`, `ai_error`, `input_tokens`, `output_tokens`, `cost_micros`, `drafted_at`, `created_at`. `items` is `(id, name, description, price_cents NOT NULL DEFAULT 8000, status DEFAULT 'pending', …)` with no CHECK constraint on status. `item_images` is `(item_id, image_path, sort_order)`. Admin routers mount as `app.use('/api/admin/', requireAdminGate, router)` — see `app.ts:81` for upload links. `POST /api/admin/items/:id/mark-available` (`routes/admin.ts:304`) sets `status='available'` and clears `sold_at`, `reserved_until`, `paypal_order_id`. Admin tabs are registered in `frontend/src/admin/Admin.tsx:392-398`. + +## Global Constraints + +- **Nothing reaches the storefront except by publishing from this queue.** The item arrives `pending`, which every public query already excludes. This screen is the only place `mark-available` is reached for an intake item. +- **The price field is the point of this screen.** Items are priced on arrival, so the schema no longer prevents an unchosen price reaching the storefront — the protection lives here now. The field is labelled with where the number came from, anything that is not `price_source='admin'` is visibly unconfirmed, editing sets `'admin'`, and publishing an unconfirmed price is allowed but says so *before* rather than after. +- **80.00 is a plausible price, not an obvious sentinel.** `0` would render "$0.00" and read as a bug; 80.00 reads as a decision. That is the entire argument for surfacing provenance loudly. +- **Discard is recoverable.** A one-click destructive action reachable from an inbox must not be final. +- **States are `queued | drafting | ready | failed | discarded`.** Note the worker never sets `drafting` today — it goes `queued → ready|failed` — so that filter will legitimately be empty. Do not "fix" that here. +- **Every route handler wrapped in `asyncRoute`** — `tests/unit/routesAreWrapped.test.ts` enforces it. +- **antd deep imports from `antd/es/...`**, never the barrel. +- **E2E assertions scoped to the item under test**, never the whole grid — the dev database never truncates. Use `findOrFail` from `./fixtures` for collection lookups, and `exact: true` on any short accessible name (#253). +- **Playwright and the integration suite need Node 20+**; the machine default is 18.16.1. Put a newer Node first on `PATH` for the command — never `nvm use`. +- **Commit style:** Conventional Commits, subject ending `(#225)`, no hard wrapping in bodies. + +## File Structure + +**Created:** +- `backend/src/intake/priceSource.ts` — the transition rule. Pure. +- `backend/src/routes/adminItemDrafts.ts` — list plus four actions. +- `backend/tests/unit/priceSource.test.ts` +- `backend/tests/integration/adminItemDrafts.integration.test.ts` +- `frontend/src/admin/draftsApi.ts` — typed client. +- `frontend/src/admin/DraftQueue.tsx` — the screen. +- `frontend/tests/e2e/admin-draft-queue.spec.ts` + +**Modified:** +- `backend/src/app.ts` — mount the router +- `frontend/src/admin/Admin.tsx` — register the tab + +--- + +### Task 1: The price provenance rule + +The one piece of logic on this screen that must not be wrong, so it is pure and tested on its own rather than buried in a route. + +**Files:** +- Create: `backend/src/intake/priceSource.ts`, `backend/tests/unit/priceSource.test.ts` + +**Interfaces:** +- Produces: `type PriceSource = 'default' | 'ai' | 'admin'`, `nextPriceSource(current: PriceSource, submittedCents: number, storedCents: number): PriceSource`, `isUnconfirmed(source: PriceSource): boolean` + +- [ ] **Step 1: Write the failing test** + +```ts +import { nextPriceSource, isUnconfirmed } from '../../src/intake/priceSource'; + +describe('nextPriceSource', () => { + // Touching the number is the admin taking responsibility for it. That is the + // only event that can confirm a price. + it('becomes admin when the number changes', () => { + expect(nextPriceSource('default', 9500, 8000)).toBe('admin'); + expect(nextPriceSource('ai', 4000, 4500)).toBe('admin'); + }); + + // Publishing without touching the field must NOT silently confirm it. That + // is the whole failure this screen exists to prevent: an item selling at a + // number nobody chose, with nothing recording that. + it('leaves an untouched price unconfirmed', () => { + expect(nextPriceSource('default', 8000, 8000)).toBe('default'); + expect(nextPriceSource('ai', 4500, 4500)).toBe('ai'); + }); + + // Already confirmed stays confirmed, including when re-submitted unchanged. + it('keeps admin once set', () => { + expect(nextPriceSource('admin', 9500, 9500)).toBe('admin'); + expect(nextPriceSource('admin', 7000, 9500)).toBe('admin'); + }); +}); + +describe('isUnconfirmed', () => { + it('treats anything but admin as unconfirmed', () => { + expect(isUnconfirmed('default')).toBe(true); + expect(isUnconfirmed('ai')).toBe(true); + expect(isUnconfirmed('admin')).toBe(false); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.unit.config.js priceSource +``` + +Expected: FAIL — module not found. + +- [ ] **Step 3: Write it** + +```ts +/** + * Where an item's price came from, and when that changes. + * + * This is the protection that used to live in the schema. Items are priced on + * arrival — the model's suggestion, or the 80.00 default — so nothing stops a + * number nobody chose from reaching the storefront except this screen showing + * that it was never chosen. + * + * Pure and separately tested because the failure is silent. An item that sells + * at a default price looks exactly like an item that sells at a chosen one; + * 80.00 was picked precisely because it reads as a decision rather than as an + * obvious sentinel like 0.00 would. + */ +export type PriceSource = 'default' | 'ai' | 'admin'; + +/** + * Editing the number is the admin taking responsibility for it, and it is the + * only thing that can. Publishing without touching the field deliberately does + * NOT confirm it — that would turn "I did not look at this" into "I approved + * this", which is the exact misrecording this screen exists to prevent. + */ +export function nextPriceSource( + current: PriceSource, + submittedCents: number, + storedCents: number +): PriceSource { + if (current === 'admin') return 'admin'; + return submittedCents === storedCents ? current : 'admin'; +} + +/** Anything a person did not choose, which the screen marks visibly. */ +export function isUnconfirmed(source: PriceSource): boolean { + return source !== 'admin'; +} +``` + +- [ ] **Step 4: Run it to verify it passes** + +```bash +cd backend && npx jest -c jest.unit.config.js priceSource +``` + +Expected: PASS, 6 tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/intake/priceSource.ts backend/tests/unit/priceSource.test.ts +git commit -m "feat(intake): record who chose an item's price (#225)" +``` + +--- + +### Task 2: Listing the queue + +**Files:** +- Create: `backend/src/routes/adminItemDrafts.ts`, `backend/tests/integration/adminItemDrafts.integration.test.ts` +- Modify: `backend/src/app.ts` + +**Interfaces:** +- Consumes: `PriceSource` from Task 1 +- Produces: `GET /api/admin/item-drafts?state=` returning `{ drafts: DraftRow[] }` + +- [ ] **Step 1: Write the failing test** + +```ts +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +const GATE = { 'x-admin-gate': process.env.ADMIN_GATE_SECRET ?? '' }; + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +async function seedDraft(overrides: { state?: string; aiName?: string } = {}): Promise { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Submission 2026-09-01', 'pending') RETURNING id` + ); + const itemId = rows[0]!.id; + await pool.query( + `INSERT INTO item_drafts (item_id, submitter_note, state, ai_name, ai_description, price_source) + VALUES ($1, 'found in a loft', $2, $3, 'A blue vase.', 'ai')`, + [itemId, overrides.state ?? 'ready', overrides.aiName ?? 'Blue vase'] + ); + await pool.query( + `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, '/uploads/a.jpg', 0)`, + [itemId] + ); + return itemId; +} + +describe('GET /api/admin/item-drafts', () => { + it('returns the draft with its item, photos and note', async () => { + const itemId = await seedDraft(); + + const res = await request(app).get('/api/admin/item-drafts').set(GATE); + + expect(res.status).toBe(200); + const draft = res.body.drafts.find((d: { item_id: number }) => d.item_id === itemId); + expect(draft).toBeDefined(); + expect(draft.submitter_note).toBe('found in a loft'); + expect(draft.ai_name).toBe('Blue vase'); + expect(draft.price_cents).toBe(8000); + expect(draft.price_source).toBe('ai'); + expect(draft.images).toHaveLength(1); + }); + + it('filters by state', async () => { + const ready = await seedDraft({ state: 'ready' }); + const failed = await seedDraft({ state: 'failed' }); + + const res = await request(app).get('/api/admin/item-drafts?state=failed').set(GATE); + + const ids = res.body.drafts.map((d: { item_id: number }) => d.item_id); + expect(ids).toContain(failed); + expect(ids).not.toContain(ready); + }); + + // Discarded is recoverable, so it has to be reachable — but it must not sit + // in the default view competing with work that still needs doing. + it('hides discarded drafts unless they are asked for', async () => { + const discarded = await seedDraft({ state: 'discarded' }); + + const def = await request(app).get('/api/admin/item-drafts').set(GATE); + expect(def.body.drafts.map((d: { item_id: number }) => d.item_id)).not.toContain(discarded); + + const asked = await request(app).get('/api/admin/item-drafts?state=discarded').set(GATE); + expect(asked.body.drafts.map((d: { item_id: number }) => d.item_id)).toContain(discarded); + }); + + it('refuses without the admin gate', async () => { + await seedDraft(); + const res = await request(app).get('/api/admin/item-drafts'); + expect(res.status).toBe(403); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +Bring up a database (see Global Constraints), then: + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand adminItemDrafts +``` + +Expected: FAIL — 404, the route does not exist. + +- [ ] **Step 3: Write the router** + +Create `backend/src/routes/adminItemDrafts.ts`: + +```ts +import { Router, Request, Response } from 'express'; +import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; + +const router = Router(); + +/** + * The review queue: everything waiting for a person, with what a person needs + * to decide. + * + * Columns are spelled out rather than `d.*` so a column added to item_drafts + * later — a cost, a token count, an error — does not silently start being sent + * to the browser. The images come back as an aggregate rather than a second + * round trip, matching itemSelect.ts. + */ +const DRAFT_SELECT = ` + SELECT d.item_id, d.state, d.attempts, d.submitter_note, d.ai_error, + d.ai_name, d.ai_description, d.ai_category_id, d.ai_tag_names, + d.ai_suggested_price_cents, d.price_source, d.model, d.drafted_at, + d.created_at, + i.name AS item_name, i.description AS item_description, + i.price_cents, i.status, + l.label AS upload_link_label, + COALESCE(( + SELECT json_agg(json_build_object('id', img.id, 'image_path', img.image_path) + ORDER BY img.sort_order) + FROM item_images img WHERE img.item_id = d.item_id + ), '[]'::json) AS images + FROM item_drafts d + JOIN items i ON i.id = d.item_id + LEFT JOIN upload_links l ON l.id = d.upload_link_id +`; + +/** + * Discarded is excluded by default rather than deleted. Discard has to be + * recoverable — it is one click from an inbox — but a discarded row sitting in + * the default view would compete with work that still needs doing. + */ +router.get('/', asyncRoute(async (req: Request, res: Response) => { + const state = typeof req.query.state === 'string' ? req.query.state : null; + + const { rows } = state + ? await pool.query(`${DRAFT_SELECT} WHERE d.state = $1 ORDER BY d.created_at DESC`, [state]) + : await pool.query(`${DRAFT_SELECT} WHERE d.state <> 'discarded' ORDER BY d.created_at DESC`); + + res.json({ drafts: rows }); +})); + +export default router; +``` + +- [ ] **Step 4: Mount it** + +In `backend/src/app.ts`, beside the other admin routers: + +```ts +import adminItemDraftsRouter from './routes/adminItemDrafts'; +``` + +and, next to the `upload-links` line: + +```ts +app.use('/api/admin/item-drafts', requireAdminGate, adminItemDraftsRouter); +``` + +The gate goes on the mount, not inside the router — that is the pattern every other admin router follows, and it is what makes the "refuses without the admin gate" test above pass without a line of code in the handler. + +- [ ] **Step 5: Run it to verify it passes** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand adminItemDrafts +npx jest -c jest.unit.config.js routesAreWrapped +``` + +Expected: PASS, 4 integration tests, and the wrapper guard still green. + +- [ ] **Step 6: Commit** + +```bash +git add backend/src/routes/adminItemDrafts.ts backend/src/app.ts backend/tests/integration/adminItemDrafts.integration.test.ts +git commit -m "feat(intake): list submitted items waiting for review (#225)" +``` + +--- + +### Task 3: Publishing + +**Files:** +- Modify: `backend/src/routes/adminItemDrafts.ts`, `backend/tests/integration/adminItemDrafts.integration.test.ts` + +**Interfaces:** +- Consumes: `nextPriceSource` from Task 1 +- Produces: `POST /api/admin/item-drafts/:itemId/publish` taking `{ name, description, priceCents, categoryId?, tagNames? }` + +- [ ] **Step 1: Write the failing test** + +Append to the integration test: + +```ts +describe('POST /api/admin/item-drafts/:itemId/publish', () => { + const body = { name: 'Blue stoneware vase', description: 'Chipped base.', priceCents: 9500 }; + + it('writes the edited copy onto the item and publishes it', async () => { + const itemId = await seedDraft(); + + const res = await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .set(GATE) + .send(body); + + expect(res.status).toBe(200); + const { rows } = await pool.query( + `SELECT name, description, price_cents, status FROM items WHERE id = $1`, + [itemId] + ); + expect(rows[0]).toMatchObject({ + name: 'Blue stoneware vase', + description: 'Chipped base.', + price_cents: 9500, + status: 'available' + }); + }); + + // The transition Task 1 defines, asserted end to end: a changed number is + // now the admin's. + it('records an edited price as the admin choice', async () => { + const itemId = await seedDraft(); + + await request(app).post(`/api/admin/item-drafts/${itemId}/publish`).set(GATE).send(body); + + const { rows } = await pool.query(`SELECT price_source FROM item_drafts WHERE item_id = $1`, [ + itemId + ]); + expect(rows[0]?.price_source).toBe('admin'); + }); + + // And the case that matters more: publishing without touching the number + // must leave it recorded as unconfirmed rather than quietly claiming the + // admin chose it. + it('leaves an untouched price unconfirmed', async () => { + const itemId = await seedDraft(); + + await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .set(GATE) + .send({ ...body, priceCents: 8000 }); + + const { rows } = await pool.query(`SELECT price_source FROM item_drafts WHERE item_id = $1`, [ + itemId + ]); + expect(rows[0]?.price_source).toBe('ai'); + }); + + it('refuses a publish with no name', async () => { + const itemId = await seedDraft(); + const res = await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .set(GATE) + .send({ ...body, name: ' ' }); + + expect(res.status).toBe(400); + const { rows } = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(rows[0]?.status).toBe('pending'); + }); + + it('refuses a negative price', async () => { + const itemId = await seedDraft(); + const res = await request(app) + .post(`/api/admin/item-drafts/${itemId}/publish`) + .set(GATE) + .send({ ...body, priceCents: -1 }); + expect(res.status).toBe(400); + }); + + it('404s for an item with no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary item') RETURNING id` + ); + const res = await request(app) + .post(`/api/admin/item-drafts/${rows[0]!.id}/publish`) + .set(GATE) + .send(body); + expect(res.status).toBe(404); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand adminItemDrafts +``` + +Expected: FAIL — 404 on the publish route. + +- [ ] **Step 3: Write it** + +Add to `adminItemDrafts.ts`, above `export default router`: + +```ts +import { nextPriceSource, PriceSource } from '../intake/priceSource'; + +interface DraftPriceRow { + price_source: PriceSource; + price_cents: number; +} + +/** + * Publish: the edited copy goes onto the item, and the item goes live. + * + * The only path from an intake submission to the storefront. It does what + * mark-available does — status, and clearing the sale and reservation fields — + * rather than calling that route, because both halves must be one transaction: + * an item published with the previous draft's name would be worse than one not + * published at all. + */ +router.post('/:itemId/publish', asyncRoute(async (req: Request, res: Response) => { + const name = typeof req.body?.name === 'string' ? req.body.name.trim() : ''; + const description = + typeof req.body?.description === 'string' ? req.body.description.trim() : ''; + const priceCents = Number(req.body?.priceCents); + + if (name === '') { + return res.status(400).json({ error: 'a name is required' }); + } + if (!Number.isInteger(priceCents) || priceCents < 0) { + return res.status(400).json({ error: 'a price in whole cents is required' }); + } + + const client = await pool.connect(); + try { + await client.query('BEGIN'); + + // Locked for the length of the transaction so two admins publishing the + // same submission cannot interleave a price decision with a name. + const { rows } = await client.query( + `SELECT d.price_source, i.price_cents + FROM item_drafts d JOIN items i ON i.id = d.item_id + WHERE d.item_id = $1 + FOR UPDATE OF d, i`, + [req.params.itemId] + ); + const existing = rows[0]; + if (!existing) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'no draft for this item' }); + } + + const priceSource = nextPriceSource(existing.price_source, priceCents, existing.price_cents); + + await client.query( + `UPDATE items + SET name = $2, description = $3, price_cents = $4, + status = 'available', sold_at = NULL, reserved_until = NULL, paypal_order_id = NULL + WHERE id = $1`, + [req.params.itemId, name, description === '' ? null : description, priceCents] + ); + + await client.query( + `UPDATE item_drafts SET price_source = $2 WHERE item_id = $1`, + [req.params.itemId, priceSource] + ); + + await client.query('COMMIT'); + res.json({ published: true, priceSource }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } +})); +``` + +- [ ] **Step 4: Run it to verify it passes** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand adminItemDrafts +``` + +Expected: PASS, 10 integration tests. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/routes/adminItemDrafts.ts backend/tests/integration/adminItemDrafts.integration.test.ts +git commit -m "feat(intake): publish a reviewed item to the storefront (#225)" +``` + +--- + +### Task 4: Regenerate, discard and restore + +**Files:** +- Modify: `backend/src/routes/adminItemDrafts.ts`, `backend/tests/integration/adminItemDrafts.integration.test.ts` + +**Interfaces:** +- Produces: `POST /:itemId/regenerate`, `POST /:itemId/discard`, `POST /:itemId/restore` + +- [ ] **Step 1: Write the failing test** + +```ts +describe('the other three actions', () => { + // Back to queued, and attempts cleared — otherwise a draft that already + // failed three times is regenerated into a state the worker will not pick up, + // and the button does nothing with no way to tell. + it('regenerate re-queues a failed draft and clears its attempts', async () => { + const itemId = await seedDraft({ state: 'failed' }); + await pool.query(`UPDATE item_drafts SET attempts = 3, ai_error = 'boom' WHERE item_id = $1`, [ + itemId + ]); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/regenerate`).set(GATE); + + expect(res.status).toBe(200); + const { rows } = await pool.query( + `SELECT state, attempts, ai_error FROM item_drafts WHERE item_id = $1`, + [itemId] + ); + expect(rows[0]).toMatchObject({ state: 'queued', attempts: 0, ai_error: null }); + }); + + it('discard marks the draft and leaves the item unpublished', async () => { + const itemId = await seedDraft(); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/discard`).set(GATE); + + expect(res.status).toBe(200); + const draft = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(draft.rows[0]?.state).toBe('discarded'); + const item = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(item.rows[0]?.status).toBe('pending'); + }); + + // The reason discard is allowed to be one click. + it('discard does not delete the item or its photos', async () => { + const itemId = await seedDraft(); + await request(app).post(`/api/admin/item-drafts/${itemId}/discard`).set(GATE); + + const item = await pool.query(`SELECT id FROM items WHERE id = $1`, [itemId]); + expect(item.rows).toHaveLength(1); + const images = await pool.query(`SELECT id FROM item_images WHERE item_id = $1`, [itemId]); + expect(images.rows).toHaveLength(1); + }); + + it('discard unpublishes an item that had been published', async () => { + const itemId = await seedDraft(); + await pool.query(`UPDATE items SET status = 'available' WHERE id = $1`, [itemId]); + + await request(app).post(`/api/admin/item-drafts/${itemId}/discard`).set(GATE); + + const item = await pool.query(`SELECT status FROM items WHERE id = $1`, [itemId]); + expect(item.rows[0]?.status).toBe('pending'); + }); + + it('restore brings a discarded draft back', async () => { + const itemId = await seedDraft({ state: 'discarded' }); + + const res = await request(app).post(`/api/admin/item-drafts/${itemId}/restore`).set(GATE); + + expect(res.status).toBe(200); + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('ready'); + }); + + // A draft that never drafted must not come back claiming it did. + it('restore returns an undrafted submission to failed, not ready', async () => { + const itemId = await seedDraft({ state: 'discarded' }); + await pool.query(`UPDATE item_drafts SET ai_name = NULL WHERE item_id = $1`, [itemId]); + + await request(app).post(`/api/admin/item-drafts/${itemId}/restore`).set(GATE); + + const { rows } = await pool.query(`SELECT state FROM item_drafts WHERE item_id = $1`, [itemId]); + expect(rows[0]?.state).toBe('failed'); + }); + + it('404s each action for an item with no draft', async () => { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name) VALUES ('ordinary item') RETURNING id` + ); + for (const action of ['regenerate', 'discard', 'restore']) { + const res = await request(app) + .post(`/api/admin/item-drafts/${rows[0]!.id}/${action}`) + .set(GATE); + expect(res.status).toBe(404); + } + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd backend && npx jest -c jest.integration.config.js --runInBand adminItemDrafts +``` + +Expected: FAIL — 404 on all three routes. + +- [ ] **Step 3: Write them** + +```ts +/** + * Regenerate: hand it back to the worker. + * + * attempts is reset along with the state. The worker only picks up rows below + * the attempt cap, so re-queueing a draft that already failed three times + * without clearing them produces a button that appears to work and does + * nothing — and nothing anywhere would say why. + */ +router.post('/:itemId/regenerate', asyncRoute(async (req: Request, res: Response) => { + const { rowCount } = await pool.query( + `UPDATE item_drafts + SET state = 'queued', attempts = 0, ai_error = NULL + WHERE item_id = $1`, + [req.params.itemId] + ); + if (rowCount === 0) return res.status(404).json({ error: 'no draft for this item' }); + res.json({ state: 'queued' }); +})); + +/** + * Discard: out of the queue, off the storefront, and still entirely recoverable. + * + * Nothing is deleted — not the item, not the photos. This is one click from an + * inbox, and the photos are often the only copy of something no longer in the + * sender's hands, so the destructive reading of "discard" is not available here. + * The item is returned to pending because a discarded submission must not stay + * on sale. + */ +router.post('/:itemId/discard', asyncRoute(async (req: Request, res: Response) => { + const client = await pool.connect(); + try { + await client.query('BEGIN'); + const { rowCount } = await client.query( + `UPDATE item_drafts SET state = 'discarded' WHERE item_id = $1`, + [req.params.itemId] + ); + if (rowCount === 0) { + await client.query('ROLLBACK'); + return res.status(404).json({ error: 'no draft for this item' }); + } + await client.query(`UPDATE items SET status = 'pending' WHERE id = $1`, [req.params.itemId]); + await client.query('COMMIT'); + res.json({ state: 'discarded' }); + } catch (err) { + await client.query('ROLLBACK'); + console.error(err); + res.status(500).json({ error: 'internal error' }); + } finally { + client.release(); + } +})); + +/** + * Restore: back into the queue, at the state the draft's own contents justify. + * + * Not unconditionally 'ready'. A submission discarded before it was ever + * drafted has no copy, and returning it as ready would present an empty draft + * as a finished one. Judged on whether a name was ever written rather than on + * remembered history, because the previous state is not stored. + */ +router.post('/:itemId/restore', asyncRoute(async (req: Request, res: Response) => { + const { rowCount } = await pool.query( + `UPDATE item_drafts + SET state = CASE WHEN ai_name IS NULL THEN 'failed' ELSE 'ready' END + WHERE item_id = $1`, + [req.params.itemId] + ); + if (rowCount === 0) return res.status(404).json({ error: 'no draft for this item' }); + res.json({ restored: true }); +})); +``` + +- [ ] **Step 4: Run it and the guards** + +```bash +cd backend +npx jest -c jest.integration.config.js --runInBand adminItemDrafts +npm run test:unit && npm run lint && npm run build +``` + +Expected: PASS, 17 integration tests; unit, lint and build clean. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/routes/adminItemDrafts.ts backend/tests/integration/adminItemDrafts.integration.test.ts +git commit -m "feat(intake): regenerate, discard and restore a draft (#225)" +``` + +--- + +### Task 5: The screen + +**Files:** +- Create: `frontend/src/admin/draftsApi.ts`, `frontend/src/admin/DraftQueue.tsx` +- Modify: `frontend/src/admin/Admin.tsx` + +**Interfaces:** +- Consumes: the four routes from Tasks 2-4 +- Produces: a `Review queue` tab + +- [ ] **Step 1: Write the client** + +Create `frontend/src/admin/draftsApi.ts`: + +```ts +export type PriceSource = 'default' | 'ai' | 'admin'; + +export interface DraftImage { + id: number; + image_path: string; +} + +export interface Draft { + item_id: number; + state: string; + attempts: number; + submitter_note: string | null; + ai_error: string | null; + ai_name: string | null; + ai_description: string | null; + ai_suggested_price_cents: number | null; + price_source: PriceSource; + model: string | null; + item_name: string; + item_description: string | null; + price_cents: number; + status: string; + upload_link_label: string | null; + images: DraftImage[]; +} + +async function send(path: string, init?: RequestInit): Promise { + return fetch(`/api/admin/item-drafts${path}`, { + ...init, + headers: { 'Content-Type': 'application/json', ...(init?.headers ?? {}) } + }); +} + +export async function fetchDrafts(state?: string): Promise { + const query = state ? `?state=${encodeURIComponent(state)}` : ''; + const res = await send(query); + if (!res.ok) throw new Error('could not load the review queue'); + return (await res.json()).drafts; +} + +export interface PublishInput { + name: string; + description: string; + priceCents: number; +} + +export async function publishDraft(itemId: number, input: PublishInput): Promise { + const res = await send(`/${itemId}/publish`, { method: 'POST', body: JSON.stringify(input) }); + if (!res.ok) throw new Error((await res.json()).error ?? 'could not publish'); +} + +export async function actOnDraft( + itemId: number, + action: 'regenerate' | 'discard' | 'restore' +): Promise { + const res = await send(`/${itemId}/${action}`, { method: 'POST' }); + if (!res.ok) throw new Error(`could not ${action}`); +} +``` + +- [ ] **Step 2: Write the screen** + +Create `frontend/src/admin/DraftQueue.tsx`: + +```tsx +import { useCallback, useEffect, useState } from 'react'; +import Card from 'antd/es/card'; +import Button from 'antd/es/button'; +import Input from 'antd/es/input'; +import InputNumber from 'antd/es/input-number'; +import Space from 'antd/es/space'; +import Tag from 'antd/es/tag'; +import Select from 'antd/es/select'; +import Empty from 'antd/es/empty'; +import Alert from 'antd/es/alert'; +import Modal from 'antd/es/modal'; +import message from 'antd/es/message'; +import { Draft, PriceSource, actOnDraft, fetchDrafts, publishDraft } from './draftsApi'; + +const { TextArea } = Input; + +/** Mirrors isUnconfirmed on the server. Anything a person did not choose. */ +function isUnconfirmed(source: PriceSource): boolean { + return source !== 'admin'; +} + +function priceLabel(source: PriceSource): string { + if (source === 'admin') return 'you set this price'; + if (source === 'ai') return 'suggested by the model — not confirmed'; + return 'default price — nobody chose this'; +} + +/** + * One submission, with everything needed to judge it. + * + * The price is the reason this screen exists. Items are priced on arrival, so + * nothing stops a number nobody chose from reaching the storefront except this + * saying so — and 80.00 is a plausible price rather than an obvious sentinel, + * which is exactly why it has to be called out rather than left to be noticed. + */ +function DraftCard({ draft, onChanged }: { draft: Draft; onChanged: () => void }) { + const [name, setName] = useState(draft.ai_name ?? draft.item_name); + const [description, setDescription] = useState(draft.ai_description ?? draft.item_description ?? ''); + const [priceCents, setPriceCents] = useState(draft.price_cents); + const [busy, setBusy] = useState(false); + + // Unconfirmed until the number is actually changed. Opening the field and + // leaving it alone is not a decision, and must not be recorded as one. + const unconfirmed = isUnconfirmed(draft.price_source) && priceCents === draft.price_cents; + + const run = async (work: () => Promise) => { + setBusy(true); + try { + await work(); + onChanged(); + } catch (err) { + message.error(err instanceof Error ? err.message : 'that did not work'); + } finally { + setBusy(false); + } + }; + + const publish = () => { + const go = () => run(() => publishDraft(draft.item_id, { name, description, priceCents })); + if (!unconfirmed) return go(); + // Said before, not after. Publishing an unconfirmed price is allowed — + // it is a decision someone is entitled to make — but not by accident. + Modal.confirm({ + title: 'Publish at a price nobody chose?', + content: `This will go on sale at $${(priceCents / 100).toFixed(2)}, which is ${ + draft.price_source === 'ai' ? "the model's suggestion" : 'the default' + } rather than a price you set.`, + okText: 'Publish anyway', + onOk: go + }); + }; + + return ( + {draft.state}} + style={{ marginBottom: 16 }} + data-testid={`draft-${draft.item_id}`} + > + + {draft.ai_error && } + + + {draft.images.map((image) => ( + + ))} + + + {draft.submitter_note && ( + + )} + {draft.upload_link_label && via {draft.upload_link_label}} + + setName(e.target.value)} aria-label="Name" /> +