Merge pull request 'Feature/224 intake notification' (#263) from feature/224-intake-notification into main
Reviewed-on: #263
This commit was merged in pull request #263.
This commit is contained in:
@@ -10,6 +10,7 @@ import adminEmailTemplatesRouter from './routes/adminEmailTemplates';
|
|||||||
import adminCategoriesRouter from './routes/adminCategories';
|
import adminCategoriesRouter from './routes/adminCategories';
|
||||||
import adminTagsRouter from './routes/adminTags';
|
import adminTagsRouter from './routes/adminTags';
|
||||||
import adminUploadLinksRouter from './routes/adminUploadLinks';
|
import adminUploadLinksRouter from './routes/adminUploadLinks';
|
||||||
|
import adminItemDraftsRouter from './routes/adminItemDrafts';
|
||||||
import intakeRouter from './routes/intake';
|
import intakeRouter from './routes/intake';
|
||||||
import adminVersionRouter from './routes/adminVersion';
|
import adminVersionRouter from './routes/adminVersion';
|
||||||
import filtersRouter from './routes/filters';
|
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/categories', requireAdminGate, adminCategoriesRouter);
|
||||||
app.use('/api/admin/tags', requireAdminGate, adminTagsRouter);
|
app.use('/api/admin/tags', requireAdminGate, adminTagsRouter);
|
||||||
app.use('/api/admin/upload-links', requireAdminGate, adminUploadLinksRouter);
|
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/version', requireAdminGate, adminVersionRouter);
|
||||||
app.use('/api/admin', requireAdminGate, adminRouter);
|
app.use('/api/admin', requireAdminGate, adminRouter);
|
||||||
app.use('/api/customers/me/addresses', shippingAddressesRouter);
|
app.use('/api/customers/me/addresses', shippingAddressesRouter);
|
||||||
|
|||||||
@@ -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';
|
||||||
|
}
|
||||||
@@ -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<DraftPriceRow>(
|
||||||
|
`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;
|
||||||
@@ -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<number> {
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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<void>`
|
||||||
|
|
||||||
|
- [ ] **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<void> {
|
||||||
|
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<NotifyRow>(
|
||||||
|
`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<number> {
|
||||||
|
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.
|
||||||
File diff suppressed because it is too large
Load Diff
@@ -34,6 +34,7 @@ import Settings from './Settings';
|
|||||||
import Categories from './Categories';
|
import Categories from './Categories';
|
||||||
import Tags from './Tags';
|
import Tags from './Tags';
|
||||||
import UploadLinks from './UploadLinks';
|
import UploadLinks from './UploadLinks';
|
||||||
|
import DraftQueue from './DraftQueue';
|
||||||
import BuildStamp from './BuildStamp';
|
import BuildStamp from './BuildStamp';
|
||||||
import CategoryTreeSelect from './CategoryTreeSelect';
|
import CategoryTreeSelect from './CategoryTreeSelect';
|
||||||
import ItemCard from '../components/ItemCard';
|
import ItemCard from '../components/ItemCard';
|
||||||
@@ -393,6 +394,7 @@ export default function Admin() {
|
|||||||
{ key: 'categories', label: 'Categories', children: <Categories /> },
|
{ key: 'categories', label: 'Categories', children: <Categories /> },
|
||||||
{ key: 'tags', label: 'Tags', children: <Tags /> },
|
{ key: 'tags', label: 'Tags', children: <Tags /> },
|
||||||
{ key: 'upload-links', label: 'Upload links', children: <UploadLinks /> },
|
{ key: 'upload-links', label: 'Upload links', children: <UploadLinks /> },
|
||||||
|
{ key: 'review-queue', label: 'Review queue', children: <DraftQueue /> },
|
||||||
{ key: 'customers', label: 'Customers', children: <Customers /> },
|
{ key: 'customers', label: 'Customers', children: <Customers /> },
|
||||||
{ key: 'emails', label: 'Emails', children: <Emails /> },
|
{ key: 'emails', label: 'Emails', children: <Emails /> },
|
||||||
{ key: 'settings', label: 'Settings', children: <Settings /> }
|
{ key: 'settings', label: 'Settings', children: <Settings /> }
|
||||||
|
|||||||
@@ -0,0 +1,195 @@
|
|||||||
|
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 — nobody chose this';
|
||||||
|
return 'default price — nobody chose this';
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* One submission, with everything needed to judge it.
|
||||||
|
*
|
||||||
|
* The price is why 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 }: Readonly<{ 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 — the
|
||||||
|
// server applies the same rule, this only has to agree with it.
|
||||||
|
const unconfirmed = isUnconfirmed(draft.price_source) && priceCents === draft.price_cents;
|
||||||
|
|
||||||
|
const run = async (work: () => Promise<void>) => {
|
||||||
|
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) {
|
||||||
|
void go();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Said before, not after. Publishing at 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',
|
||||||
|
okButtonProps: { danger: true },
|
||||||
|
onOk: go
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
return (
|
||||||
|
<Card title={draft.item_name} extra={<Tag>{draft.state}</Tag>} style={{ marginBottom: 16 }}>
|
||||||
|
<Space direction="vertical" style={{ width: '100%' }} size="middle">
|
||||||
|
{draft.ai_error && <Alert type="warning" message={`Drafting failed: ${draft.ai_error}`} />}
|
||||||
|
|
||||||
|
<Space wrap>
|
||||||
|
{draft.images.map((image) => (
|
||||||
|
<img
|
||||||
|
key={image.id}
|
||||||
|
src={image.image_path}
|
||||||
|
alt=""
|
||||||
|
style={{ width: 120, height: 120, objectFit: 'cover' }}
|
||||||
|
/>
|
||||||
|
))}
|
||||||
|
</Space>
|
||||||
|
|
||||||
|
{draft.submitter_note && (
|
||||||
|
<Alert type="info" message={`Sender's note: ${draft.submitter_note}`} />
|
||||||
|
)}
|
||||||
|
{draft.upload_link_label && <Tag>via {draft.upload_link_label}</Tag>}
|
||||||
|
|
||||||
|
<Input value={name} onChange={(e) => setName(e.target.value)} aria-label="Name" />
|
||||||
|
<TextArea
|
||||||
|
value={description}
|
||||||
|
onChange={(e) => setDescription(e.target.value)}
|
||||||
|
rows={4}
|
||||||
|
aria-label="Description"
|
||||||
|
/>
|
||||||
|
|
||||||
|
<Space direction="vertical" size={4}>
|
||||||
|
<InputNumber
|
||||||
|
value={priceCents / 100}
|
||||||
|
min={0}
|
||||||
|
step={0.01}
|
||||||
|
prefix="$"
|
||||||
|
aria-label="Price"
|
||||||
|
onChange={(value) => setPriceCents(Math.round((value ?? 0) * 100))}
|
||||||
|
/>
|
||||||
|
<Tag color={unconfirmed ? 'orange' : 'green'}>
|
||||||
|
{unconfirmed ? priceLabel(draft.price_source) : 'you set this price'}
|
||||||
|
</Tag>
|
||||||
|
</Space>
|
||||||
|
|
||||||
|
<Space>
|
||||||
|
<Button type="primary" loading={busy} onClick={publish}>
|
||||||
|
Publish
|
||||||
|
</Button>
|
||||||
|
<Button loading={busy} onClick={() => void run(() => actOnDraft(draft.item_id, 'regenerate'))}>
|
||||||
|
Regenerate
|
||||||
|
</Button>
|
||||||
|
{draft.state === 'discarded' ? (
|
||||||
|
<Button loading={busy} onClick={() => void run(() => actOnDraft(draft.item_id, 'restore'))}>
|
||||||
|
Restore
|
||||||
|
</Button>
|
||||||
|
) : (
|
||||||
|
<Button
|
||||||
|
danger
|
||||||
|
loading={busy}
|
||||||
|
onClick={() => void run(() => actOnDraft(draft.item_id, 'discard'))}
|
||||||
|
>
|
||||||
|
Discard
|
||||||
|
</Button>
|
||||||
|
)}
|
||||||
|
</Space>
|
||||||
|
</Space>
|
||||||
|
</Card>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
export default function DraftQueue() {
|
||||||
|
const [drafts, setDrafts] = useState<Draft[]>([]);
|
||||||
|
const [state, setState] = useState<string | undefined>(undefined);
|
||||||
|
const [error, setError] = useState<string | null>(null);
|
||||||
|
|
||||||
|
const load = useCallback(async () => {
|
||||||
|
try {
|
||||||
|
setDrafts(await fetchDrafts(state));
|
||||||
|
setError(null);
|
||||||
|
} catch {
|
||||||
|
setError('Could not load the review queue.');
|
||||||
|
}
|
||||||
|
}, [state]);
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
// eslint-disable-next-line react-hooks/set-state-in-effect
|
||||||
|
void load();
|
||||||
|
}, [load]);
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div>
|
||||||
|
<Space style={{ marginBottom: 16 }}>
|
||||||
|
<Select
|
||||||
|
value={state}
|
||||||
|
onChange={setState}
|
||||||
|
style={{ width: 220 }}
|
||||||
|
placeholder="All except discarded"
|
||||||
|
allowClear
|
||||||
|
aria-label="State"
|
||||||
|
options={[
|
||||||
|
{ value: 'queued', label: 'Queued' },
|
||||||
|
{ value: 'ready', label: 'Ready' },
|
||||||
|
{ value: 'failed', label: 'Failed' },
|
||||||
|
{ value: 'discarded', label: 'Discarded' }
|
||||||
|
]}
|
||||||
|
/>
|
||||||
|
<Button onClick={() => void load()}>Refresh</Button>
|
||||||
|
</Space>
|
||||||
|
|
||||||
|
{error && <Alert type="error" message={error} />}
|
||||||
|
{!error && drafts.length === 0 && <Empty description="Nothing waiting for review" />}
|
||||||
|
{drafts.map((draft) => (
|
||||||
|
<DraftCard key={draft.item_id} draft={draft} onChanged={() => void load()} />
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,73 @@
|
|||||||
|
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<Response> {
|
||||||
|
return fetch(`/api/admin/item-drafts${path}`, {
|
||||||
|
...init,
|
||||||
|
headers: { 'Content-Type': 'application/json', ...(init?.headers ?? {}) }
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function fetchDrafts(state?: string): Promise<Draft[]> {
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The server's message is preferred over a generic one because its refusals are
|
||||||
|
* specific and actionable — a missing name, a fractional price — and replacing
|
||||||
|
* them with "could not publish" would throw away the only thing that says how
|
||||||
|
* to fix it.
|
||||||
|
*/
|
||||||
|
export async function publishDraft(itemId: number, input: PublishInput): Promise<void> {
|
||||||
|
const res = await send(`/${itemId}/publish`, { method: 'POST', body: JSON.stringify(input) });
|
||||||
|
if (res.ok) return;
|
||||||
|
|
||||||
|
let message = 'could not publish';
|
||||||
|
try {
|
||||||
|
message = (await res.json()).error ?? message;
|
||||||
|
} catch {
|
||||||
|
// A non-JSON body is a proxy or gateway error rather than the app refusing.
|
||||||
|
// The generic message above is the honest thing to show in that case.
|
||||||
|
}
|
||||||
|
throw new Error(message);
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function actOnDraft(
|
||||||
|
itemId: number,
|
||||||
|
action: 'regenerate' | 'discard' | 'restore'
|
||||||
|
): Promise<void> {
|
||||||
|
const res = await send(`/${itemId}/${action}`, { method: 'POST' });
|
||||||
|
if (!res.ok) throw new Error(`could not ${action} this draft`);
|
||||||
|
}
|
||||||
@@ -0,0 +1,101 @@
|
|||||||
|
import { test, expect, uniqueSuffix, findOrFail, createAdminContext } from './fixtures';
|
||||||
|
|
||||||
|
const RUN = uniqueSuffix();
|
||||||
|
|
||||||
|
// The 1x1 PNG the other upload specs use, so this exercises the real validated
|
||||||
|
// upload path rather than a buffer that merely starts correctly.
|
||||||
|
const PNG = Buffer.from(
|
||||||
|
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==',
|
||||||
|
'base64'
|
||||||
|
);
|
||||||
|
|
||||||
|
let token: string;
|
||||||
|
|
||||||
|
test.beforeAll(async ({ playwright }) => {
|
||||||
|
const api = await createAdminContext(playwright);
|
||||||
|
const res = await api.post('/api/admin/upload-links', {
|
||||||
|
data: { label: `Review queue spec ${RUN}` }
|
||||||
|
});
|
||||||
|
expect(res.status(), 'creating the upload link').toBe(201);
|
||||||
|
token = (await res.json()).token;
|
||||||
|
await api.dispose();
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Seeded through the intake route rather than POST /api/admin/items, which
|
||||||
|
* writes no item_drafts row — an item created that way would never appear in a
|
||||||
|
* queue that joins that table. This is also the path a real submission takes.
|
||||||
|
*/
|
||||||
|
async function submitAnItem(page: import('@playwright/test').Page, note: string): Promise<void> {
|
||||||
|
await page.goto(`/submit/${token}`);
|
||||||
|
await page.setInputFiles('input[type="file"]', {
|
||||||
|
name: `${RUN}.png`,
|
||||||
|
mimeType: 'image/png',
|
||||||
|
buffer: PNG
|
||||||
|
});
|
||||||
|
await page.getByLabel('Anything you know about this item').fill(note);
|
||||||
|
await page.getByRole('button', { name: 'Send' }).click();
|
||||||
|
await expect(page.getByRole('heading', { name: 'Thank you — it arrived' })).toBeVisible();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Every assertion is scoped to the card this test created. The dev database
|
||||||
|
// never truncates, so a queue-wide assertion outruns its timeout and fails for
|
||||||
|
// reasons unrelated to the behaviour under test (#241).
|
||||||
|
test.describe('The review queue', () => {
|
||||||
|
test('publishes a submitted item at an edited price', async ({ page, admin }) => {
|
||||||
|
const note = `Stoneware ${RUN}`;
|
||||||
|
await submitAnItem(page, note);
|
||||||
|
|
||||||
|
await admin.open('Review queue');
|
||||||
|
|
||||||
|
// Located by the sender's note, which carries this run's id. The item's own
|
||||||
|
// name is a submission timestamp and is not unique to this test.
|
||||||
|
const card = page.locator('.ant-card').filter({ hasText: note });
|
||||||
|
await expect(card).toBeVisible();
|
||||||
|
|
||||||
|
// The provenance warning is the entire point of the screen. With no API key
|
||||||
|
// configured the draft stays queued at the 8000 default, so this is the
|
||||||
|
// default wording rather than the model's.
|
||||||
|
await expect(card.getByText(/nobody chose this/)).toBeVisible();
|
||||||
|
|
||||||
|
await card.getByLabel('Name').fill(`Blue vase ${RUN}`);
|
||||||
|
await card.getByLabel('Price').fill('95');
|
||||||
|
await card.getByRole('button', { name: 'Publish', exact: true }).click();
|
||||||
|
|
||||||
|
// Editing the price confirms it, so no confirmation dialog appears and the
|
||||||
|
// item simply publishes.
|
||||||
|
await expect(card.getByText('you set this price')).toBeVisible();
|
||||||
|
|
||||||
|
const items = await (await page.request.get('/api/admin/items')).json();
|
||||||
|
const published = findOrFail(
|
||||||
|
items as { id: number; name: string; status: string; price_cents: number }[],
|
||||||
|
(item) => item.name === `Blue vase ${RUN}`,
|
||||||
|
`the published item Blue vase ${RUN}`
|
||||||
|
);
|
||||||
|
expect(published.status).toBe('available');
|
||||||
|
expect(published.price_cents).toBe(9500);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The protection this screen exists to provide. Publishing at a price nobody
|
||||||
|
// chose is allowed — but it must be a decision, not an accident.
|
||||||
|
test('asks before publishing at a price nobody chose', async ({ page, admin }) => {
|
||||||
|
const note = `Unpriced ${RUN}`;
|
||||||
|
await submitAnItem(page, note);
|
||||||
|
|
||||||
|
await admin.open('Review queue');
|
||||||
|
const card = page.locator('.ant-card').filter({ hasText: note });
|
||||||
|
await expect(card).toBeVisible();
|
||||||
|
|
||||||
|
await card.getByRole('button', { name: 'Publish', exact: true }).click();
|
||||||
|
|
||||||
|
// Matched on the dialog's accessible name rather than its text: antd nests
|
||||||
|
// the confirm title in two elements, so getByText resolves to both.
|
||||||
|
const dialog = page.getByRole('dialog', { name: 'Publish at a price nobody chose?' });
|
||||||
|
await expect(dialog).toBeVisible();
|
||||||
|
await expect(dialog.getByText('$80.00')).toBeVisible();
|
||||||
|
|
||||||
|
// Backing out must leave it unpublished.
|
||||||
|
await dialog.getByRole('button', { name: 'Cancel', exact: true }).click();
|
||||||
|
await expect(card.getByText(/nobody chose this/)).toBeVisible();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -7,7 +7,8 @@ export type AdminTab =
|
|||||||
| 'Tags'
|
| 'Tags'
|
||||||
| 'Customers'
|
| 'Customers'
|
||||||
| 'Emails'
|
| 'Emails'
|
||||||
| 'Settings';
|
| 'Settings'
|
||||||
|
| 'Review queue';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The admin shell: the tab strip and the panel it swaps.
|
* The admin shell: the tab strip and the panel it swaps.
|
||||||
|
|||||||
Reference in New Issue
Block a user