* fix(archives): take the restored category from the manifest
The archive writer already persists `category_name` per photo in
photos_manifest.json — that is why the manifest exists, and the comment
above it says so: "(and category linkage) can't be derived from the
extracted files alone". The restore route then read only
`original_filename` out of it and kept deriving the category from the
ZIP's first path segment.
Archives store photos exactly as they sit on disk, so an event whose
photos live in the gallery root produces a FLAT zip. `path.dirname()` is
'.' for every entry, no category is resolved, and every restored photo
lands with `category_id = null` — silently, behind a 200.
Seen on a real restore: 596 photos back, 0 with a category, while the
nine category rows sat untouched in the table.
Now the manifest is the source of truth and the first path segment is
the fallback, so foldered archives and legacy archives without a
manifest behave exactly as before. The find-or-create is pulled into
`resolveCategoryId` so both paths share it and each name is resolved
once per restore.
Tests: __tests__/integration/adminArchives.restoreCategories.test.js
builds real ZIPs (flat with manifest, flat with an existing category
row, foldered without manifest) and drives POST /:id/restore. Without
this change the two manifest cases fail and the foldered one passes —
the fallback is unchanged.
* fix(archives): let the manifest be authoritative when it says "no category"
Review follow-up on #1240, pushed with the author's agreement.
The manifest won for "category X" but not for "none": an entry with a null
category_name fell through to the directory fallback, so a photo the archive
recorded as uncategorized came back filed under a category anyway.
That matters because the directory is not a category. Archive entry names are
the storage key minus `events/active/{slug}`, and that layout is
`individual/{filename}` / `collages/{filename}` — categories have never been
directories there. Reading the first path segment on a real archive invents
categories literally named "individual" and "collages", so the fallback was
overriding an accurate record with a junk one.
The fallback is now confined to photos with NO manifest entry at all: archives
written before the manifest existed, where the directory is the only signal
left and inventing those names still beats losing every category.
Tests: the legacy case now uses `individual/`, the shape a real archive
actually has, instead of a category-shaped folder no archive produces — so it
documents what the fallback really does. Plus a new case pinning that a
manifest saying uncategorized leaves the photo uncategorized and creates no
category row. It fails without this change; the legacy fallback keeps passing.
---------
Co-authored-by: Paul Nothaft <[email protected]>
Unit test for the v1 upload route's category lookup, requested in
the PR review. Mocks db (chainable, mirroring src/routes/__tests__/
adminAuth.test.js) plus apiTokenAuth/requireApiScope (pass-through)
and multer (stub req.file). Two cases:
1. The scoping clause: the andWhere callback applied to a knex
builder spy produces .where({event_id: <event.id>}).orWhere(
'is_global', true) — exactly the contract the reviewer asked
for, exercising the OR-clause rather than just asserting the
callback was passed.
2. Null lookup result yields 400 with "Unknown or out-of-scope
category_id <N>".
No v1 jest scaffolding existed before, but the project-wide harness
(backend/jest.config.js + jest.setup.js) already covers the new
file via testMatch '**/__tests__/**/*.test.js'. Happy-path tests
deferred — would require stubbing fs/sharp/imageProcessor/share
linkService and several more db chains, which the reviewer was
willing to accept as a separate follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
PR review pointed out the original lookup
db('photo_categories').where({ id: parsedCategoryId }).first()
accepted any category id — including one that belongs to a different
event. photo_categories carries both event_id (per-event) and is_global
(see backend/migrations/legacy/004_add_categories_and_cms.js); the v1
upload route should require either match.
Not a privilege issue (apiTokenAuth.js inherits the admin's powers, no
per-event scoping), but it lets a misconfigured uploader silently file
photos under a category the target event doesn't own — and the 201 echo
includes a category_id that makes no semantic sense.
Tighten to:
.where({ id: parsedCategoryId })
.andWhere(function () {
this.where({ event_id: event.id }).orWhere('is_global', true);
})
…and update the 400 message to "Unknown or out-of-scope category_id N".
OpenAPI description already documents the intended scope.
Tests deferred to a follow-up; v1 has no jest harness today, see PR
discussion.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The v1 photo upload endpoint previously ignored any caller-supplied
category and inserted photos with category_id=NULL. That meant
programmatic uploads via API tokens (e.g. a photobox sidecar) landed
in picpeak as uncategorized, forcing operators to bulk-assign category
in the admin UI after each event.
Mirror the adminPhotos.js category-handling logic on v1:
- Read optional `category_id` from the multipart form body.
- Reject unknown ids with 400 (with the id in the error) so callers
fail fast on misconfigured envs instead of silently uncategorized
uploads.
- Set photos.category_id on insert.
- Flip photos.type to 'collage' when the category's slug is
collage/collages, matching adminPhotos.
Backwards-compatible: omitting category_id keeps the prior behavior
(insert with NULL category, type='individual'). OpenAPI spec + 201
response body updated to include the new field.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
