bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

fix(admin): offer Remove and Restore independently (#293)

Browse Source 
One button whose label flipped on "is every photo cut out?" could not serve a partly cut-out item, which is not a hypothetical state: it is what a partial removal leaves behind, and it is also what happens when REMBG_URL goes away after some photos were already done. In that state the single button read "Remove backgrounds", so the cut-out photos the item already had could never be restored from this screen. Remove and Restore are now separately gated and can appear together, which is correct — Remove finishes the job on what is left, Restore undoes what is already done.

Restore is deliberately not gated on the backgroundRemoval config flag. Gating it would strand cut-out photos with no way back in exactly the environment that most needs the undo. Remove stays gated, so an unconfigured environment shows no button rather than one that reports zero of four done every time.

The emptiness check moves from `!== null` to `!= null`: original_image_path is optional on the shared Item type because the public storefront response omits it, so a stray undefined has to count as "not cut out" — `undefined !== null` is true, which would misread a public-shaped item as fully cut out.

The modal now refreshes on a non-ok response too. A restore that fails partway can still have swapped some files back before it failed, so returning early left the thumbnails showing files that are no longer on the server. The warning text is now driven off whichever count the action reports, so a partial restore says how far it got the same way a partial removal already did.

The e2e spec seeds its item into a category of its own and filters the table down to it. The inventory table paginates at 10 and the suite runs fullyParallel, so an unfiltered page one was never a reliable place to find the fixture.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bermudalamb
2026-09-04 12:02:37 -05:00
co-authored by Claude Opus 5
parent 445c9c4a22
commit 64f8efb617
2 changed files with 74 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/admin/Admin.tsx
+58 -30
View File
@@ -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() {
</div>
))}
</Space>
{(backgroundRemoval || editingItem.images.every(img => img.original_image_path !== null)) && (
<div style={{ marginTop: 8 }}>
<Button
size="small"
loading={busyBackgrounds}
onClick={() => void handleBackgrounds(
editingItem.id,
editingItem.images.every(img => img.original_image_path !== null)
? 'restore-originals'
: 'remove-backgrounds'
)}
>
{editingItem.images.every(img => img.original_image_path !== null)
? 'Restore originals'
: 'Remove backgrounds'}
</Button>
</div>
)}
{(() => {
// `!= 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 (
<div style={{ marginTop: 8 }}>
<Space>
{showRemove && (
<Button
size="small"
loading={busyBackgrounds}
onClick={() => void handleBackgrounds(editingItem.id, 'remove-backgrounds')}
>
Remove backgrounds
</Button>
)}
{showRestore && (
<Button
size="small"
loading={busyBackgrounds}
onClick={() => void handleBackgrounds(editingItem.id, 'restore-originals')}
>
Restore originals
</Button>
)}
</Space>
</div>
);
})()}
</Form.Item>
)}
<Form.Item label={editingItem ? 'Add More Images' : 'Images (front, back, etc.)'}>
frontend/tests/e2e/admin-item-backgrounds.spec.ts
+16 -4
View File
@@ -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();
});