fix(archives): take the restored category from the manifest (#1240)
* 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]>
This commit is contained in:
Marian
@@ -226,9 +226,35 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
// Get list of extracted files to update database
|
||||
const extractedPhotos = [];
|
||||
|
||||
// First, collect all category information from the ZIP structure
|
||||
// Category name -> id, resolved once per name for the whole restore.
|
||||
const categoriesMap = new Map();
|
||||
|
||||
// Find-or-create the category by name, scoped to this event.
|
||||
const resolveCategoryId = async (categoryName) => {
|
||||
if (!categoryName) return null;
|
||||
if (categoriesMap.has(categoryName)) return categoriesMap.get(categoryName);
|
||||
|
||||
const existingCategory = await db('photo_categories')
|
||||
.where('event_id', archive.id)
|
||||
.where('name', categoryName)
|
||||
.first();
|
||||
|
||||
if (existingCategory) {
|
||||
categoriesMap.set(categoryName, existingCategory.id);
|
||||
} else {
|
||||
const insertResult = await db('photo_categories').insert({
|
||||
event_id: archive.id,
|
||||
name: categoryName,
|
||||
slug: slugify(categoryName),
|
||||
created_at: new Date()
|
||||
}).returning('id');
|
||||
|
||||
categoriesMap.set(categoryName, insertResult[0]?.id || insertResult[0]);
|
||||
}
|
||||
|
||||
return categoriesMap.get(categoryName);
|
||||
};
|
||||
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory && entry.name.match(/\.(jpg|jpeg|png|gif|webp)$/i)) {
|
||||
const filename = path.basename(entry.name);
|
||||
@@ -238,39 +264,33 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
try {
|
||||
// Check if file was extracted successfully
|
||||
const stats = await fs.stat(actualFilePath);
|
||||
|
||||
// Determine category from directory structure
|
||||
|
||||
const manifestEntry = manifestByFilename.get(filename);
|
||||
|
||||
// The manifest is the only faithful source for the category, and
|
||||
// it is authoritative INCLUDING when it says "none". A manifest
|
||||
// entry with a null category_name means the photo was genuinely
|
||||
// uncategorized, so falling through to the directory would
|
||||
// contradict the very record being restored from.
|
||||
//
|
||||
// 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/…` —
|
||||
// categories have never been directories there. Reading the first
|
||||
// path segment on a real archive therefore invents categories
|
||||
// literally named "individual" and "collages".
|
||||
//
|
||||
// So the fallback is 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 two names
|
||||
// is still better than losing every category.
|
||||
let categoryId = null;
|
||||
if (dirPath && dirPath !== '.') {
|
||||
// Get the first level directory as category
|
||||
const categoryName = dirPath.split(path.sep)[0];
|
||||
|
||||
if (!categoriesMap.has(categoryName)) {
|
||||
// Check if this category exists in the database
|
||||
const existingCategory = await db('photo_categories')
|
||||
.where('event_id', archive.id)
|
||||
.where('name', categoryName)
|
||||
.first();
|
||||
|
||||
if (existingCategory) {
|
||||
categoriesMap.set(categoryName, existingCategory.id);
|
||||
} else {
|
||||
// Create the category if it doesn't exist
|
||||
const insertResult = await db('photo_categories').insert({
|
||||
event_id: archive.id,
|
||||
name: categoryName,
|
||||
slug: slugify(categoryName),
|
||||
created_at: new Date()
|
||||
}).returning('id');
|
||||
|
||||
const newCategoryId = insertResult[0]?.id || insertResult[0];
|
||||
categoriesMap.set(categoryName, newCategoryId);
|
||||
}
|
||||
}
|
||||
|
||||
categoryId = categoriesMap.get(categoryName);
|
||||
if (manifestEntry) {
|
||||
categoryId = await resolveCategoryId(manifestEntry.category_name);
|
||||
} else if (dirPath && dirPath !== '.') {
|
||||
categoryId = await resolveCategoryId(dirPath.split(path.sep)[0]);
|
||||
}
|
||||
|
||||
|
||||
// Check if photo already exists in database
|
||||
const existingPhoto = await db('photos')
|
||||
.where('event_id', archive.id)
|
||||
@@ -280,7 +300,6 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
if (!existingPhoto) {
|
||||
// Store relative path from storage root
|
||||
const relativePath = path.relative(storagePath, actualFilePath);
|
||||
const manifestEntry = manifestByFilename.get(filename);
|
||||
extractedPhotos.push({
|
||||
event_id: archive.id,
|
||||
filename: filename,
|
||||
|
||||
Reference in New Issue
Block a user