backend/src/imageRotation.ts

@@ -0,0 +1,81 @@
+import path from 'path';
+import { pool } from './db';
+import { typeForExtension } from './uploadTypes';
+import { rotateInPlace, RotateDirection } from './imageProcessing';
+
+/**
+ * Turning one stored photo, and everything that photo is stored alongside.
+ *
+ * Its own module rather than another export on backgroundRemoval.ts: the two
+ * features share a table and nothing else. Rotation touches no sidecar, records
+ * no provenance, and is not idempotent — bundling them would put a function
+ * with none of that module's invariants under its documentation.
+ */
+
+interface ImageRow {
+  image_path: string;
+  original_image_path: string | null;
+}
+
+/**
+ * Thrown when the photo is not on that item.
+ *
+ * A named class for the same reason NoOriginalToRestoreError is one: the route
+ * has to tell "no such photo" apart from "the application is in trouble", and
+ * the two need opposite replies. Everything else stays loud.
+ */
+export class ImageNotOnItemError extends Error {}
+
+/**
+ * basename only. `image_path` is stored as `/uploads/<name>` and the directory
+ * it lives in is a server constant — the same rule readPhotos and
+ * removeImageBackground both follow.
+ */
+async function rotateStoredFile(storedPath: string, direction: RotateDirection): Promise<void> {
+  const name = path.basename(storedPath);
+  const mediaType = typeForExtension(path.extname(name));
+  if (mediaType === null) {
+    throw new Error(`cannot rotate ${name}: unrecognised extension`);
+  }
+  await rotateInPlace(path.join(process.env.UPLOADS_DIR ?? '', name), mediaType, direction);
+}
+
+/**
+ * Turns one photo of one item, and its pristine original when it has one.
+ *
+ * Both files or neither is not achievable here and is not claimed. What matters
+ * is that they cannot end up disagreeing silently: an image that has been
+ * through #281 has a displayed cut-out and a recorded original, and rotating
+ * only the first would leave Restore original quietly un-rotating the photo —
+ * the undo of one feature becoming a regression of another.
+ *
+ * The displayed file goes first, so a failure on the original leaves the admin
+ * looking at a photo that visibly moved, with the failure reported. A retry
+ * would turn the displayed file a second time: rotation is not idempotent the
+ * way removeImageBackground is, because nothing records how far the last
+ * attempt got. That is accepted rather than engineered around — it takes the
+ * disk failing between two writes, and the remedy is one press in the other
+ * direction.
+ *
+ * No column is written. The paths are the same afterwards; only the bytes
+ * differ.
+ */
+export async function rotateItemImage(
+  itemId: number,
+  imageId: number,
+  direction: RotateDirection
+): Promise<void> {
+  const { rows } = await pool.query<ImageRow>(
+    `SELECT image_path, original_image_path FROM item_images WHERE id = $1 AND item_id = $2`,
+    [imageId, itemId]
+  );
+  const row = rows[0];
+  if (!row) {
+    throw new ImageNotOnItemError(`no image ${imageId} on item ${itemId}`);
+  }
+
+  await rotateStoredFile(row.image_path, direction);
+  if (row.original_image_path !== null) {
+    await rotateStoredFile(row.original_image_path, direction);
+  }
+}

backend/src/routes/admin.ts

@@ -8,6 +8,8 @@ import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilter
 import { readId, tagColorFor } from '../utils';
 import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecipients } from '../favoriteAlerts';
 import { removeBackgroundsForItem, restoreOriginalsForItem } from '../intake/backgroundRemoval';
+import { rotateItemImage, ImageNotOnItemError } from '../imageRotation';
+import { RotateDirection } from '../imageProcessing';
 // 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 —
@@ -411,4 +413,61 @@ router.post('/items/:id/restore-originals', asyncRoute(async (req: Request, res:
   res.json(await restoreOriginalsForItem(itemId));
 }));
 
+/**
+ * Turn one photo a quarter turn.
+ *
+ * On the item rather than on the draft, which is the decision that makes the
+ * inventory editor free later: an image belongs to an item whether or not a
+ * draft row exists, so the second screen to want this is the same call from a
+ * different place, with no new backend at all.
+ *
+ * Unlike the per-item background endpoints, this acts on exactly one file, so
+ * it can honestly answer whether it worked. 204 rather than 200 because
+ * rotation changes no column — the paths are identical afterwards and only the
+ * bytes differ, so there is no row worth returning, which is also why
+ * DELETE /items/:id/images/:imageId is a 204.
+ *
+ * A factory rather than two copied handlers: the direction is the only thing
+ * that differs. Two paths rather than one endpoint taking a direction in the
+ * body matches how remove-background and restore-original are already spelled.
+ */
+function rotationRoute(direction: RotateDirection) {
+  return asyncRoute(async (req: Request, res: Response) => {
+    // Both ids, not just the first. A route carrying two of them can guard one
+    // and forget the other, and the forgotten one fails as a 500 rather than
+    // the 404 that "no such photo" actually means (#207).
+    const itemId = readId(req.params.id);
+    const imageId = readId(req.params.imageId);
+    if (itemId === null || imageId === null) {
+      return res.status(404).json({ error: 'no such photo on this item' });
+    }
+
+    try {
+      await rotateItemImage(itemId, imageId, direction);
+    } catch (err) {
+      // Only "not on this item" is a 404, and it is indistinguishable from an
+      // absent one on purpose: an image id is a serial, and confirming which
+      // ids exist is not something this endpoint should do. Everything else is
+      // a real fault and stays loud — the file is untouched in every one of
+      // those cases, because rotateInPlace renames over the original only once
+      // the new file has been written successfully.
+      if (err instanceof ImageNotOnItemError) {
+        return res.status(404).json({ error: 'no such photo on this item' });
+      }
+      console.error(`[rotation] item ${itemId}, image ${imageId}:`, err);
+      return res.status(500).json({
+        error:
+          err instanceof Error
+            ? `this photo could not be rotated: ${err.message}`
+            : 'this photo could not be rotated'
+      });
+    }
+
+    res.status(204).end();
+  });
+}
+
+router.post('/items/:id/images/:imageId/rotate-left', rotationRoute('left'));
+router.post('/items/:id/images/:imageId/rotate-right', rotationRoute('right'));
+
 export default router;

backend/tests/integration/itemImageRotation.integration.test.ts

@@ -0,0 +1,141 @@
+import request from 'supertest';
+import sharp from 'sharp';
+import { promises as fs } from 'fs';
+import path from 'path';
+import app from '../../src/app';
+import { pool } from '../../src/db';
+import { resetDb, closeDb } from './setup/testDb';
+
+const UPLOADS_DIR = process.env.UPLOADS_DIR as string;
+
+beforeAll(async () => {
+  await fs.mkdir(UPLOADS_DIR, { recursive: true });
+});
+
+beforeEach(async () => {
+  await resetDb();
+});
+
+afterAll(async () => {
+  await pool.end();
+  await closeDb();
+});
+
+/** A 400x200 landscape JPEG — wide enough that a quarter turn is unmistakable. */
+async function landscapeJpeg(): Promise<Buffer> {
+  return sharp({
+    create: { width: 400, height: 200, channels: 3, background: { r: 20, g: 60, b: 120 } }
+  })
+    .jpeg()
+    .toBuffer();
+}
+
+async function writeUpload(name: string): Promise<string> {
+  await fs.writeFile(path.join(UPLOADS_DIR, name), await landscapeJpeg());
+  return `/uploads/${name}`;
+}
+
+async function sizeOf(storedPath: string): Promise<string> {
+  const meta = await sharp(path.join(UPLOADS_DIR, path.basename(storedPath))).metadata();
+  return `${meta.width}x${meta.height}`;
+}
+
+/** An item with one photo on disk. The prefix keeps each test's file its own. */
+async function seedItem(prefix: string): Promise<{ itemId: number; imageId: number }> {
+  const { rows } = await pool.query<{ id: number }>(
+    `INSERT INTO items (name, status) VALUES ('Blue vase', 'available') RETURNING id`
+  );
+  const itemId = rows[0]!.id;
+  const imagePath = await writeUpload(`${prefix}-front.jpg`);
+  const image = await pool.query<{ id: number }>(
+    `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, 0) RETURNING id`,
+    [itemId, imagePath]
+  );
+  return { itemId, imageId: image.rows[0]!.id };
+}
+
+describe('rotating one photo of an item', () => {
+  it('turns the file and answers 204', async () => {
+    const { itemId, imageId } = await seedItem('turns');
+
+    const res = await request(app).post(
+      `/api/admin/items/${itemId}/images/${imageId}/rotate-right`
+    );
+
+    expect(res.status).toBe(204);
+    expect(await sizeOf('/uploads/turns-front.jpg')).toBe('200x400');
+  });
+
+  // The undo story. One press back is why the control offers both directions
+  // rather than making somebody turn the same button three times, each turn a
+  // further generation of JPEG loss.
+  it('turns back the other way', async () => {
+    const { itemId, imageId } = await seedItem('back');
+
+    await request(app).post(`/api/admin/items/${itemId}/images/${imageId}/rotate-right`);
+    await request(app).post(`/api/admin/items/${itemId}/images/${imageId}/rotate-left`);
+
+    expect(await sizeOf('/uploads/back-front.jpg')).toBe('400x200');
+  });
+
+  // Without this, Restore original would silently un-rotate the photo, turning
+  // the undo of #281 into a regression of #301.
+  it('turns the pristine original too', async () => {
+    const { itemId, imageId } = await seedItem('cutout');
+    const originalPath = await writeUpload('cutout-original.jpg');
+    await pool.query(`UPDATE item_images SET original_image_path = $2 WHERE id = $1`, [
+      imageId,
+      originalPath
+    ]);
+
+    const res = await request(app).post(
+      `/api/admin/items/${itemId}/images/${imageId}/rotate-right`
+    );
+
+    expect(res.status).toBe(204);
+    expect(await sizeOf('/uploads/cutout-front.jpg')).toBe('200x400');
+    expect(await sizeOf(originalPath)).toBe('200x400');
+  });
+
+  // An image id is a serial, so guessing one is easy — the photo has to belong
+  // to the item named in the path (#207).
+  it('refuses a photo belonging to another item', async () => {
+    const mine = await seedItem('mine');
+    const theirs = await seedItem('theirs');
+
+    const res = await request(app).post(
+      `/api/admin/items/${mine.itemId}/images/${theirs.imageId}/rotate-right`
+    );
+
+    expect(res.status).toBe(404);
+    expect(await sizeOf('/uploads/theirs-front.jpg')).toBe('400x200');
+  });
+
+  it('answers 404 for an unreadable id', async () => {
+    const { itemId } = await seedItem('unreadable');
+
+    const res = await request(app).post(`/api/admin/items/${itemId}/images/abc/rotate-right`);
+
+    expect(res.status).toBe(404);
+  });
+
+  it('answers 404 for an absent item', async () => {
+    const res = await request(app).post('/api/admin/items/999999/images/1/rotate-left');
+
+    expect(res.status).toBe(404);
+  });
+
+  // Reported rather than swallowed: a row pointing at a file that is not there
+  // is a real fault, and answering "done" would hide it.
+  it('answers 500 when the file is missing from disk', async () => {
+    const { itemId, imageId } = await seedItem('missing');
+    await fs.rm(path.join(UPLOADS_DIR, 'missing-front.jpg'));
+
+    const res = await request(app).post(
+      `/api/admin/items/${itemId}/images/${imageId}/rotate-right`
+    );
+
+    expect(res.status).toBe(500);
+    expect(res.body.error).toMatch(/could not be rotated/);
+  });
+});