diff --git a/backend/src/app.ts b/backend/src/app.ts index b0177dd..e4acc46 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -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); diff --git a/backend/src/itemSelect.ts b/backend/src/itemSelect.ts index 28afc76..4ef8a7a 100644 --- a/backend/src/itemSelect.ts +++ b/backend/src/itemSelect.ts @@ -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[]; } /** diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index e522607..6e6c1cd 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -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 { + 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; diff --git a/backend/src/routes/adminConfig.ts b/backend/src/routes/adminConfig.ts new file mode 100644 index 0000000..15cb12a --- /dev/null +++ b/backend/src/routes/adminConfig.ts @@ -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; diff --git a/backend/tests/integration/adminItemBackgrounds.integration.test.ts b/backend/tests/integration/adminItemBackgrounds.integration.test.ts new file mode 100644 index 0000000..5093f4b --- /dev/null +++ b/backend/tests/integration/adminItemBackgrounds.integration.test.ts @@ -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 { + 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((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((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 { + 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); + }); +});