feat(admin): remove or restore every background on an item (#293)

Two routes on the item, and a small admin config route so the inventory screen can know whether to offer them.

Both answer 200 once the id is valid, even when the sidecar fails, and that is a deliberate departure from the per-photo endpoints in #281. Those act on one image, so the request either worked or it did not and 502 says which. These act on several, so "did it work" has no single answer — two of four is the normal shape of a bad day here, not an exception — and a 502 would throw away the count that is the only thing making the outcome actionable. Non-200 is reserved for not being able to try at all, which here means an unreadable or absent id.

No status check on either. A sold item's photos are still the shop's photos and improving them changes nothing about the sale; the guards on unpublish protect a checkout in progress and a completed sale, neither of which is at stake in a photograph's background.

The config route follows adminVersion's precedent rather than extending the public /api/config: admin-only, one purpose, and the reason written down. The inventory screen had no other way to learn the feature exists, because GET /api/admin/items answers a bare array with several consumers and reshaping it for one boolean is the worse trade.

Also extends the admin item select to carry original_image_path on each image, behind a new ADMIN_IMAGES_SUBQUERY kept separate from the shared IMAGES_SUBQUERY the public select uses. Task 3 needs to derive its restore-button label from that field, and the server never sent it for items before this — only the drafts endpoint carried it, added by #281 for the review queue. It stays admin-only for the same reason itemSelect.ts already names PUBLIC_ITEM_SELECT's columns explicitly: an internal original filename is nobody's business on the storefront, and sharing one subquery would put it in every public item response.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-04 10:26:19 -05:00
co-authored by Claude Opus 5
parent 6f3e77fa88
commit 8f35204995
5 changed files with 312 additions and 2 deletions
+2
View File
@@ -14,6 +14,7 @@ import adminItemDraftsRouter from './routes/adminItemDrafts';
import intakeActionsRouter from './routes/intakeActions';
import intakeRouter from './routes/intake';
import adminVersionRouter from './routes/adminVersion';
import adminConfigRouter from './routes/adminConfig';
import filtersRouter from './routes/filters';
import customersRouter from './routes/customers';
import publicRouter from './routes/public';
@@ -84,6 +85,7 @@ 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/config', requireAdminGate, adminConfigRouter);
app.use('/api/admin', requireAdminGate, adminRouter);
app.use('/api/customers/me/addresses', shippingAddressesRouter);
app.use('/api/customers', customersRouter);
+34 -2
View File
@@ -30,6 +30,26 @@ const IMAGES_SUBQUERY = `
WHERE img.item_id = i.id
), '[]') AS images`;
/**
* Admin-only images, carrying `original_image_path` alongside the public
* fields — the field the inventory screen needs to know whether a photo has a
* cut-out to restore (#293).
*
* A separate subquery rather than adding the column to `IMAGES_SUBQUERY`
* itself, for the same reason `PUBLIC_ITEM_SELECT` names its columns instead
* of using `i.*`: an original filename is internal — nobody's business on the
* storefront — and folding it into the one subquery both selects share would
* put it in every public item response too.
*/
const ADMIN_IMAGES_SUBQUERY = `
COALESCE((
SELECT json_agg(json_build_object('id', img.id, 'image_path', img.image_path, 'sort_order', img.sort_order,
'original_image_path', img.original_image_path)
ORDER BY img.sort_order)
FROM item_images img
WHERE img.item_id = i.id
), '[]') AS images`;
const TAGS_SUBQUERY = `
COALESCE((
SELECT json_agg(json_build_object('id', t.id, 'name', t.name, 'color', t.color) ORDER BY t.name)
@@ -54,7 +74,7 @@ export const PUBLIC_ITEM_SELECT = `
export const ADMIN_ITEM_SELECT = `
SELECT i.*,
c.name AS category_name,
${IMAGES_SUBQUERY},
${ADMIN_IMAGES_SUBQUERY},
${TAGS_SUBQUERY}
${FROM_CLAUSE}`;
@@ -89,16 +109,28 @@ interface ItemRowBase {
/** What PUBLIC_ITEM_SELECT returns. Deliberately no payment or reservation columns. */
export type PublicItemRow = ItemRowBase;
/**
* An admin item's image: everything `ItemImage` has, plus where the
* background-removed photo's original went. `null` for a photo that was never
* cut out.
*/
export interface AdminItemImage extends ItemImage {
original_image_path: string | null;
}
/**
* What ADMIN_ITEM_SELECT returns: `i.*`, so every column on the table.
*
* The extra fields are the ones the storefront is not allowed to see, which is
* the whole reason the two selects differ.
* the whole reason the two selects differ. `images` is narrowed rather than
* inherited as-is, to match `ADMIN_IMAGES_SUBQUERY` carrying
* `original_image_path` where the public select's images do not.
*/
export interface AdminItemRow extends ItemRowBase {
reserved_until: Date | null;
sold_at: Date | null;
paypal_order_id: string | null;
images: AdminItemImage[];
}
/**
+56
View File
@@ -7,6 +7,7 @@ import { asyncRoute } from '../asyncRoute';
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
import { readId, tagColorFor } from '../utils';
import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecipients } from '../favoriteAlerts';
import { removeBackgroundsForItem, restoreOriginalsForItem } from '../intake/backgroundRemoval';
// The upload pipeline moved to src/imageUpload.ts when #222's public intake
// endpoint became a second caller. Mounting uploadImages gets the type
// allowlist, the magic-byte check, and the EXIF-stripping re-encode together —
@@ -350,4 +351,59 @@ router.post('/items/:id/mark-available', asyncRoute(async (req: Request, res: Re
res.json(available);
}));
/**
* One item's images, or null when the item does not exist.
*
* Checked before acting so an absent item is a 404 rather than a cheerful
* summary of nothing. `removeBackgroundsForItem` would happily report
* `total: 0` for an id that was never an item, which is true and useless.
*/
async function itemExists(itemId: number): Promise<boolean> {
const { rows } = await pool.query(`SELECT 1 FROM items WHERE id = $1`, [itemId]);
return rows.length > 0;
}
/**
* Remove the background from every photo of one item.
*
* Per item rather than per photo because an upload is one item: the front, the
* back and the chipped base are three views of one thing, not three things to
* cut out separately (#293).
*
* Answers 200 once the id is valid, even when the sidecar fails. Unlike the
* per-photo endpoints in #281, this acts on several images, so "did it work"
* has no single answer — two of four is the normal shape of a bad day here.
* A 502 would throw away the count, which is the only thing that makes the
* outcome actionable. Non-200 is reserved for not being able to try at all.
*
* No status check. A sold item's photos are still the shop's photos, and
* improving them changes nothing about the sale — the guards on `unpublish`
* protect a checkout in progress and a completed sale, neither of which is at
* stake in a photograph's background.
*/
router.post('/items/:id/remove-backgrounds', asyncRoute(async (req: Request, res: Response) => {
const itemId = readId(req.params.id);
if (itemId === null || !(await itemExists(itemId))) {
return res.status(404).json({ error: 'not found' });
}
res.json(await removeBackgroundsForItem(itemId));
}));
/**
* Put every original back.
*
* The reason removing is safe to try. Photos that were never cut out are
* skipped rather than refused, so a half-done item — what a partial failure
* leaves behind — is restorable too.
*/
router.post('/items/:id/restore-originals', asyncRoute(async (req: Request, res: Response) => {
const itemId = readId(req.params.id);
if (itemId === null || !(await itemExists(itemId))) {
return res.status(404).json({ error: 'not found' });
}
res.json(await restoreOriginalsForItem(itemId));
}));
export default router;
+27
View File
@@ -0,0 +1,27 @@
import { Router, Request, Response } from 'express';
import { isRembgConfigured } from '../intake/rembgClient';
const router = Router();
/**
* What the admin screens can offer in this environment.
*
* Behind `requireAdminGate` like every other admin router, and deliberately not
* folded into `/api/config` — the same reasoning `adminVersion.ts` records.
* That endpoint is public and the storefront fetches it on every load; nothing
* here is any of a customer's business.
*
* It exists because the inventory screen has no other way to learn this.
* `GET /api/admin/item-drafts` carries the flag for the review queue, but
* `GET /api/admin/items` answers a bare array with several consumers, and
* changing its shape for one boolean would be a worse trade than one small
* route.
*
* Not wrapped in `asyncRoute` because the handler is synchronous: it reads an
* environment variable, so there is no promise to reject.
*/
router.get('/', (_req: Request, res: Response) => {
res.json({ backgroundRemoval: isRembgConfigured() });
});
export default router;
@@ -0,0 +1,193 @@
import http from 'http';
import { AddressInfo } from 'net';
import { promises as fsp } from 'fs';
import os from 'os';
import path from 'path';
import request from 'supertest';
import app from '../../src/app';
import { pool } from '../../src/db';
import { resetDb, closeDb } from './setup/testDb';
const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0]);
const JPEG_BYTES = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0, 0, 0, 0, 0, 0, 0, 0]);
let uploads = '';
let previousUploadsDir: string | undefined;
let stub: http.Server | null = null;
/**
* A stub sidecar on an ephemeral port, and a temporary uploads directory.
*
* Port 0 rather than a fixed number: several ports in the 55000s are
* Hyper-V-reserved on this machine. No test here contacts a real rembg — it
* takes forty seconds to start, and a suite depending on that is broken by
* construction.
*/
async function startStub(
handler: (req: http.IncomingMessage, res: http.ServerResponse) => void
): Promise<void> {
uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'itembg-'));
previousUploadsDir = process.env.UPLOADS_DIR;
process.env.UPLOADS_DIR = uploads;
stub = http.createServer((req, res) => {
req.on('data', () => undefined);
req.on('end', () => handler(req, res));
});
await new Promise<void>((resolve) => stub!.listen(0, '127.0.0.1', resolve));
process.env.REMBG_URL = `http://127.0.0.1:${(stub!.address() as AddressInfo).port}`;
}
function answerWithPng(_req: http.IncomingMessage, res: http.ServerResponse): void {
res.writeHead(200, { 'Content-Type': 'image/png' });
res.end(PNG_BYTES);
}
beforeEach(async () => {
await resetDb();
});
afterEach(async () => {
delete process.env.REMBG_URL;
if (previousUploadsDir === undefined) delete process.env.UPLOADS_DIR;
else process.env.UPLOADS_DIR = previousUploadsDir;
if (uploads) await fsp.rm(uploads, { recursive: true, force: true });
uploads = '';
if (stub) {
await new Promise<void>((resolve) => stub!.close(() => resolve()));
stub = null;
}
});
afterAll(async () => {
await pool.end();
await closeDb();
});
/** An available item with two photos on disk. */
async function seedItem(): Promise<number> {
const { rows } = await pool.query<{ id: number }>(
`INSERT INTO items (name, status) VALUES ('Blue vase', 'available') RETURNING id`
);
const itemId = rows[0]!.id;
for (const [index, name] of ['front.jpg', 'back.jpg'].entries()) {
await pool.query(
`INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`,
[itemId, `/uploads/${name}`, index]
);
await fsp.writeFile(path.join(uploads, name), JPEG_BYTES);
}
return itemId;
}
describe('removing every background on an item', () => {
it('answers with the summary', async () => {
await startStub(answerWithPng);
const itemId = await seedItem();
const res = await request(app).post(`/api/admin/items/${itemId}/remove-backgrounds`);
expect(res.status).toBe(200);
expect(res.body).toEqual({ total: 2, removed: 2, failed: false });
});
// Every status, with no carve-out. A sold item's photos are still the shop's
// photos, and improving them changes nothing about the sale.
it.each(['sold', 'reserved', 'pending'])('works on a %s item', async (status) => {
await startStub(answerWithPng);
const itemId = await seedItem();
await pool.query(`UPDATE items SET status = $2 WHERE id = $1`, [itemId, status]);
const res = await request(app).post(`/api/admin/items/${itemId}/remove-backgrounds`);
expect(res.status).toBe(200);
expect(res.body.removed).toBe(2);
});
// The departure from #281's per-photo endpoints, and the reason for it: this
// acts on several images, so "did it work" has no single answer and a 502
// would throw away the count that makes the outcome actionable.
it('answers 200 with the count when the sidecar fails, not 502', async () => {
await startStub((_req, res) => {
res.writeHead(500);
res.end('boom');
});
const itemId = await seedItem();
const res = await request(app).post(`/api/admin/items/${itemId}/remove-backgrounds`);
expect(res.status).toBe(200);
expect(res.body).toEqual({ total: 2, removed: 0, failed: true });
});
it('answers 404 for an id that cannot be read', async () => {
await startStub(answerWithPng);
expect((await request(app).post('/api/admin/items/abc/remove-backgrounds')).status).toBe(404);
});
it('answers 404 for an item that does not exist', async () => {
await startStub(answerWithPng);
expect((await request(app).post('/api/admin/items/999999/remove-backgrounds')).status).toBe(404);
});
});
describe('restoring every original on an item', () => {
it('puts them back and says how many', async () => {
await startStub(answerWithPng);
const itemId = await seedItem();
await request(app).post(`/api/admin/items/${itemId}/remove-backgrounds`);
const res = await request(app).post(`/api/admin/items/${itemId}/restore-originals`);
expect(res.status).toBe(200);
expect(res.body).toEqual({ total: 2, restored: 2 });
});
it('answers 404 for an id that cannot be read', async () => {
await startStub(answerWithPng);
expect((await request(app).post('/api/admin/items/abc/restore-originals')).status).toBe(404);
});
});
// Extra scope beyond the endpoints themselves: Task 3 derives its button label
// from `original_image_path`, which the admin item select did not carry before
// this change. Asserted here because it is this task's select that changed.
describe('what item images carry in each list', () => {
it('gives original_image_path to admin, not the storefront', async () => {
await startStub(answerWithPng);
const itemId = await seedItem();
await request(app).post(`/api/admin/items/${itemId}/remove-backgrounds`);
const adminRes = await request(app).get('/api/admin/items');
expect(adminRes.status).toBe(200);
const adminItem = adminRes.body.find((item: { id: number }) => item.id === itemId);
expect(adminItem.images[0]).toHaveProperty('original_image_path', '/uploads/front.jpg');
const publicRes = await request(app).get('/api/items');
expect(publicRes.status).toBe(200);
const publicItem = publicRes.body.find((item: { id: number }) => item.id === itemId);
expect(publicItem.images[0]).not.toHaveProperty('original_image_path');
});
});
describe('what the admin screen is told about the feature', () => {
it('says it is on when a sidecar is configured', async () => {
process.env.REMBG_URL = 'http://rembg-syn:7000';
const res = await request(app).get('/api/admin/config');
expect(res.status).toBe(200);
expect(res.body.backgroundRemoval).toBe(true);
});
it('says it is off when none is', async () => {
delete process.env.REMBG_URL;
const res = await request(app).get('/api/admin/config');
expect(res.body.backgroundRemoval).toBe(false);
});
});