feat(admin): rotate a photo from the review queue (#301)

Two icon buttons under every thumbnail, and the client for the item-scoped endpoints behind them. Icon-only with an aria-label rather than visible text, because three labelled buttons under a 120px thumbnail is more furniture than the photo — and a button with no text has no accessible name at all without one.

They are not gated on the background-removal flag. That flag is about the sidecar, and rotation has nothing to do with it: turning a photo is a local file operation that works in every environment, including one where REMBG_URL was never set.

The cache-busting src is the part most likely to have shipped broken. Rotation does not change image_path, so after a successful turn the src is byte-for-byte the string the browser already holds a copy for, and the photo would appear not to have moved. express.static is mounted with no maxAge and would serve the new bytes on a full page reload, but nothing in a session asks it to. A version held in component state is what makes the button visibly do something, and it needs no column and no server change, because the file's identity has not changed — only this page's need to see it again.

The client lives in its own module rather than in draftsApi, whose send() hardcodes the item-drafts prefix these routes deliberately do not use. The inventory editor imports this same module unchanged when it follows, which is the whole reason the endpoints went on the item.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-04 13:52:20 -05:00
co-authored by Claude Opus 5
parent 3891f4fd75
commit dd533ffd63
3 changed files with 120 additions and 1 deletions
+53 -1
View File
@@ -10,6 +10,7 @@ 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 { RotateLeftOutlined, RotateRightOutlined } from '@ant-design/icons';
import {
Draft,
DraftImage,
@@ -19,6 +20,7 @@ import {
publishDraft,
setImageBackground
} from './draftsApi';
import { rotateImage, RotateDirection } from './imagesApi';
const { TextArea } = Input;
@@ -52,6 +54,15 @@ function priceLabel(source: PriceSource): string {
* already cut out — leaving a cut-out photo with no control and no way back to
* the original short of hand-editing the database. That breaks the invariant
* this whole feature rests on: the original is always restorable.
*
* The rotation buttons are not gated on `enabled`. That flag is about the
* background-removal sidecar, and rotation has nothing to do with it — turning
* a photo is a local file operation that works in every environment, including
* one where REMBG_URL was never set.
*
* Icon-only, with an aria-label rather than visible text: three labelled
* buttons under a 120px thumbnail is more furniture than the photo, and a
* button with no text has no accessible name at all without one.
*/
function DraftPhoto({
image,
@@ -65,8 +76,19 @@ function DraftPhoto({
onChanged: () => void;
}>) {
const [busy, setBusy] = useState(false);
const [turning, setTurning] = useState(false);
const cutOut = image.original_image_path !== null;
// Rotation does not change image_path, so after a successful turn the src is
// byte-for-byte the string the browser already holds a copy for, and the
// photo appears not to have moved. express.static is mounted with no maxAge
// and would serve the new bytes on a full page reload — but nothing in a
// session asks it to. This is what makes the button visibly do something. No
// column and no server change: the file's identity has not changed, only this
// page's need to see it again.
const [version, setVersion] = useState(0);
const src = version === 0 ? image.image_path : `${image.image_path}?v=${version}`;
const act = async () => {
setBusy(true);
try {
@@ -79,9 +101,39 @@ function DraftPhoto({
}
};
// No onChanged(): rotation changes nothing in the queue payload, so
// refetching it would be a request that returns exactly what is on screen.
const turn = async (direction: RotateDirection) => {
setTurning(true);
try {
await rotateImage(itemId, image.id, direction);
setVersion(Date.now());
} catch (err) {
message.error(err instanceof Error ? err.message : 'that did not work');
} finally {
setTurning(false);
}
};
return (
<Space direction="vertical" size={4} align="center">
<img src={image.image_path} alt="" style={{ width: 120, height: 120, objectFit: 'cover' }} />
<img src={src} alt="" style={{ width: 120, height: 120, objectFit: 'cover' }} />
<Space size={4}>
<Button
size="small"
icon={<RotateLeftOutlined />}
aria-label="Rotate left"
loading={turning}
onClick={() => void turn('left')}
/>
<Button
size="small"
icon={<RotateRightOutlined />}
aria-label="Rotate right"
loading={turning}
onClick={() => void turn('right')}
/>
</Space>
{enabled && (
<Button size="small" loading={busy} onClick={() => void act()}>
{cutOut ? 'Restore original' : 'Remove background'}
+40
View File
@@ -0,0 +1,40 @@
/**
* The client for the item-scoped image endpoints.
*
* Separate from draftsApi.ts, whose `send()` hardcodes the
* `/api/admin/item-drafts` prefix these routes deliberately do not use. The
* review queue is only the first screen to want rotation; the inventory editor
* imports this same module when it follows, which is the whole reason the
* endpoints were put on the item.
*/
export type RotateDirection = 'left' | 'right';
/**
* Turn one photo a quarter turn.
*
* The server's message is preferred over a generic one for the same reason
* publishDraft prefers it: its refusals name the actual problem — a file
* missing from the uploads volume, an animated image that cannot be turned a
* quarter turn — and replacing those with "could not rotate" throws away the
* only thing that says what to do next.
*/
export async function rotateImage(
itemId: number,
imageId: number,
direction: RotateDirection
): Promise<void> {
const res = await fetch(`/api/admin/items/${itemId}/images/${imageId}/rotate-${direction}`, {
method: 'POST'
});
if (res.ok) return;
let message = 'could not rotate this photo';
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 is the honest thing to show in that case.
}
throw new Error(message);
}
@@ -117,4 +117,31 @@ test.describe('The review queue', () => {
const card = page.locator('.ant-card').filter({ hasText: note });
await expect(card.getByRole('button', { name: 'Remove background' })).toBeVisible();
});
// Rotation is what repairs the photos uploaded before #300 taught the
// re-encode to apply EXIF orientation rather than discard it. The 1x1 PNG
// this spec uploads is square, so there is nothing visual to assert — what is
// being proved is that the control is there and the whole path answers
// without an error.
test('offers to rotate each photo', async ({ page, admin }) => {
const note = `Sideways ${RUN}`;
await submitAnItem(page, note);
await admin.open('Review queue');
const card = page.locator('.ant-card').filter({ hasText: note });
await expect(card.getByRole('button', { name: 'Rotate left' })).toBeVisible();
await expect(card.getByRole('button', { name: 'Rotate right' })).toBeVisible();
// Asserted on the response rather than on the absence of an error toast.
// toHaveCount(0) passes the instant it is evaluated, before a failure has
// had time to appear, so it would report success on a broken round trip —
// which is barely an assertion at all. Waiting for the POST proves the
// whole path: the button is wired, the route exists, and it answered 204.
const rotated = page.waitForResponse(
(res) => res.url().includes('/rotate-right') && res.request().method() === 'POST'
);
await card.getByRole('button', { name: 'Rotate right' }).click();
expect((await rotated).status()).toBe(204);
});
});