diff --git a/frontend/src/admin/Admin.tsx b/frontend/src/admin/Admin.tsx index bee99d4..ab5daef 100755 --- a/frontend/src/admin/Admin.tsx +++ b/frontend/src/admin/Admin.tsx @@ -214,7 +214,10 @@ function Inventory() { * The response replaces the open modal's images rather than being trusted to * have changed nothing else. The editor is a modal and this changes files on * the server while it is open, so without a refresh the thumbnails keep - * showing the previous files and the button looks like it did nothing. + * showing the previous files and the button looks like it did nothing. That + * holds even when the server answers something other than 200: a restore + * that fails partway can still have swapped some files back before it did, + * so the refresh below runs whether the call succeeded or not. */ async function handleBackgrounds(itemId: number, action: 'remove-backgrounds' | 'restore-originals') { const label = action === 'remove-backgrounds' ? 'remove backgrounds' : 'restore originals'; @@ -223,24 +226,26 @@ function Inventory() { const res = await fetch(`/api/admin/items/${itemId}/${action}`, { method: 'POST' }); if (!res.ok) { message.error('That did not work.'); - return; - } - const summary = await res.json(); - - if (action === 'remove-backgrounds' && summary.failed) { - // Said plainly rather than as a generic failure. How far it got is what - // decides whether pressing it again is worth anything, and it is — - // a retry skips the ones that already worked. - message.warning(`${summary.removed} of ${summary.total} photos done. Try again to finish.`); } else { - message.success('Done.'); + const summary = await res.json(); + const done = action === 'remove-backgrounds' ? summary.removed : summary.restored; + + if (summary.failed) { + // Said plainly rather than as a generic failure. How far it got is + // what decides whether pressing it again is worth anything, and it + // is — a retry skips the ones that already worked. + message.warning(`${done} of ${summary.total} photos done. Try again to finish.`); + } else { + message.success('Done.'); + } } // Re-read the list through load() — same as every other mutation here — // so a filtered view survives this, then pick this item's fresh images // back out of it for the open modal. The item's own filtered fields // (category, tags, status, search) are untouched by a background swap, - // so it stays in the result whenever it was in it before. + // so it stays in the result whenever it was in it before. Run for a + // failed response too — see the doc comment above. const rows = await load(); const updated = rows?.find(candidate => candidate.id === itemId); if (updated) setEditingItem(prev => (prev && prev.id === itemId ? updated : prev)); @@ -381,24 +386,47 @@ function Inventory() { ))} - {(backgroundRemoval || editingItem.images.every(img => img.original_image_path !== null)) && ( -
- -
- )} + {(() => { + // `!= null` rather than `!== null`: original_image_path is + // optional on the shared Item type (it is absent from the + // public storefront response), so a stray `undefined` has to + // count as "not cut out" too — `undefined !== null` is `true`, + // which would misread a public-shaped item as fully cut out. + const allCutOut = editingItem.images.every(img => img.original_image_path != null); + const someCutOut = editingItem.images.some(img => img.original_image_path != null); + // Remove and Restore are independent, not two labels for one + // button: a partly cut-out item — a partial removal's normal + // result, or REMBG_URL going away after the fact — needs both, + // or the cut-out photos it already has can never be restored + // from this screen (#293). + const showRemove = backgroundRemoval && !allCutOut; + const showRestore = someCutOut; + if (!showRemove && !showRestore) return null; + return ( +
+ + {showRemove && ( + + )} + {showRestore && ( + + )} + +
+ ); + })()} )} diff --git a/frontend/tests/e2e/admin-item-backgrounds.spec.ts b/frontend/tests/e2e/admin-item-backgrounds.spec.ts index 7f374b8..b5a4070 100644 --- a/frontend/tests/e2e/admin-item-backgrounds.spec.ts +++ b/frontend/tests/e2e/admin-item-backgrounds.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, createAdminContext, uniqueSuffix } from './fixtures'; +import { test, expect, createAdminContext, createCategory, uniqueSuffix } from './fixtures'; /** * The per-item background control in the inventory editor (#293). @@ -17,6 +17,10 @@ import { test, expect, createAdminContext, uniqueSuffix } from './fixtures'; */ const RUN = uniqueSuffix(); const NAME = `Vase ${RUN}`; +// A category of its own, not shared: it exists only so filterByCategory below +// has something unique to narrow the table down to, the same way +// admin-inventory-filters.spec.ts uses one per run. +const CATEGORY = `Backgrounds ${RUN}`; // The 1x1 PNG the other upload specs use, so this goes through the real // validated upload path rather than a buffer that merely starts correctly. @@ -27,6 +31,7 @@ const PNG = Buffer.from( test.beforeAll(async ({ playwright }) => { const api = await createAdminContext(playwright); + const categoryId = await createCategory(api, CATEGORY); // Seeded with an image directly: createItem cannot attach one, and the // control does not render for an item with no photos. const res = await api.post('/api/admin/items', { @@ -34,7 +39,7 @@ test.beforeAll(async ({ playwright }) => { name: NAME, description: '', price: '50', - category_id: '', + category_id: String(categoryId), tags: '[]', images: { name: `${RUN}.png`, mimeType: 'image/png', buffer: PNG } } @@ -44,10 +49,17 @@ test.beforeAll(async ({ playwright }) => { }); test.describe('Removing backgrounds from the item editor', () => { - test('offers the control on an item that has photos', async ({ page, admin }) => { + test('offers the control on an item that has photos', async ({ page, admin, adminInventory }) => { await admin.open('Inventory'); - await page.getByRole('row', { name: new RegExp(NAME) }).getByRole('button', { name: 'Edit' }).click(); + // The table paginates at 10 and this suite runs fullyParallel, so an + // unfiltered page 1 is not a reliable place to find this fixture — see + // AdminInventory.filterByCategory. Filtering to this item's own category + // narrows the table down to just it, the way admin-inventory-filters.spec.ts + // does. + await adminInventory.filterByCategory(CATEGORY, NAME); + + await adminInventory.row(NAME).getByRole('button', { name: 'Edit' }).click(); await expect(page.getByRole('button', { name: 'Remove backgrounds' })).toBeVisible(); });