diff --git a/backend/__tests__/services/lightroomRoundtrip.test.js b/backend/__tests__/services/lightroomRoundtrip.test.js new file mode 100644 index 00000000..d3eb5835 --- /dev/null +++ b/backend/__tests__/services/lightroomRoundtrip.test.js @@ -0,0 +1,327 @@ +/** + * Lightroom round-trip (#745) — pins the pieces that let a client's proofing + * verdict reach a desktop catalogue and a finished edit come back: + * + * - migration 193 adds photos.source_filename and backfills it, so galleries + * that predate the round-trip can still match on their first pass + * - source_filename survives replacePhoto(); original_filename does not. + * This is the whole point of the column: without it, the first re-upload + * of a renamed render destroys the key the NEXT round-trip needs + * - the number_token match mode reads the LONGEST trailing digit run, which + * is what makes the multi-camera cam1/cam2 prefix scheme work + * - ambiguity is refused, never guessed + * - mergeMarks collapses three possible opinions into the one colour and one + * rating Lightroom has room for + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-lr-roundtrip-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'lr-roundtrip-test-secret'; + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); +const { mergeMarks, roundRating } = require('../../src/services/markMerge'); + +let db; +let cleanup; +let eventId; +let adminId; + +async function addPhoto({ filename, originalFilename, sourceFilename }) { + const [row] = await db('photos').insert({ + event_id: eventId, + filename, + original_filename: originalFilename, + source_filename: sourceFilename, + path: `slug/${filename}`, + type: 'individual', + }).returning('id'); + return row?.id || row; +} + +beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + ({ adminId } = await seedMinimal(db)); + + const [event] = await db('events').insert({ + slug: 'lr-roundtrip-event', + event_name: 'Round-trip Event', + event_type: 'wedding', + event_date: '2026-08-25', + host_email: 'host@example.com', + password_hash: 'not-a-real-hash', + admin_email: 'admin@example.com', + share_link: 'lr-roundtrip-event', + expires_at: '2027-08-25', + }).returning('id'); + eventId = event?.id || event; +}); + +afterAll(async () => { await cleanup(); }); + +describe('migration 193 — photos.source_filename', () => { + it('adds the column', async () => { + expect(await db.schema.hasColumn('photos', 'source_filename')).toBe(true); + }); + + it('backfills existing rows from original_filename', async () => { + // Simulate a row that predates the migration: column nulled out, then the + // migration's backfill re-run against it. + const id = await addPhoto({ + filename: 'legacy.jpg', originalFilename: 'IMG_9001.JPG', sourceFilename: null, + }); + const migration = require('../../migrations/core/193_add_photo_source_filename.js'); + await migration.up(db); + + const row = await db('photos').where({ id }).first(); + expect(row.source_filename).toBe('IMG_9001.JPG'); + }); +}); + +describe('findReplacementCandidate', () => { + const { findReplacementCandidate, trailingDigitRun } = + require('../../src/services/photoReplacementService'); + + it('extracts the longest trailing digit run, not a fixed slice', () => { + expect(trailingDigitRun('IMG_1234.JPG')).toBe('1234'); + expect(trailingDigitRun('Smith_Wedding_11234.jpg')).toBe('11234'); + expect(trailingDigitRun('DSC_0042.NEF')).toBe('0042'); + expect(trailingDigitRun('no-digits.jpg')).toBeNull(); + expect(trailingDigitRun(null)).toBeNull(); + }); + + it('keeps the two bodies of a multi-camera shoot apart', () => { + // The whole reason the camera index is prefixed INTO the number: a + // last-4 slice would read 1234 from both and collide. + expect(trailingDigitRun('cam11234.jpg')).toBe('11234'); + expect(trailingDigitRun('cam21234.jpg')).toBe('21234'); + }); + + it('matches exactly, case-insensitively, in exact mode', async () => { + await addPhoto({ + filename: 'stored_a.jpg', originalFilename: 'IMG_2001.JPG', sourceFilename: 'IMG_2001.JPG', + }); + const hit = await findReplacementCandidate(eventId, 'img_2001.jpg'); + expect(hit).toBeTruthy(); + expect(hit.original_filename).toBe('IMG_2001.JPG'); + }); + + it('does NOT match a renamed render in exact mode', async () => { + expect(await findReplacementCandidate(eventId, 'Smith_Wedding_2001.jpg')).toBeNull(); + }); + + it('matches a renamed render in number_token mode', async () => { + const hit = await findReplacementCandidate( + eventId, 'Smith_Wedding_2001.jpg', { matchMode: 'number_token' }, + ); + expect(hit).toBeTruthy(); + expect(hit.original_filename).toBe('IMG_2001.JPG'); + }); + + it('refuses rather than guessing when two photos share a number', async () => { + await addPhoto({ + filename: 'stored_b.jpg', originalFilename: 'DSC_2001.NEF', sourceFilename: 'DSC_2001.NEF', + }); + const result = await findReplacementCandidate( + eventId, 'Anything_2001.jpg', { matchMode: 'number_token' }, + ); + expect(result).toEqual({ ambiguous: true, count: 2 }); + }); + + it('returns null in number_token mode when the name has no digits', async () => { + expect(await findReplacementCandidate( + eventId, 'untitled.jpg', { matchMode: 'number_token' }, + )).toBeNull(); + }); +}); + +describe('replacePhoto — review blockers on #1165', () => { + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + const { replacePhoto } = require('../../src/services/photoReplacementService'); + + const makeTempFile = () => { + const p = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'lr-replace-')), 'render.jpg'); + // A 1x1 JPEG is enough: sharp may fail on it, and replacePhoto is + // required to survive that (thumbnail generation is best-effort). + fs.writeFileSync(p, Buffer.from( + '/9j/4AAQSkZJRgABAQEAYABgAAD/2wBDAAgGBgcGBQgHBwcJCQgKDBQNDAsLDBkSEw8UHRofHh0a' + + 'HBwgJC4nICIsIxwcKDcpLDAxNDQ0Hyc5PTgyPC4zNDL/wAALCAABAAEBAREA/8QAFAABAAAAAAAA' + + 'AAAAAAAAAAAACf/EABQQAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQEAAD8AKp//2Q==', 'base64')); + return p; + }; + + it('repoints an external row to managed, so viewers stop getting the old file', async () => { + // resolvePhotoStorageKey gives photo.source_origin precedence and returns + // null for 'external' — so a replacement that left it set would upload the + // edit, report success, and keep serving the untouched NAS original. + const id = await addPhoto({ + filename: 'ext.jpg', originalFilename: 'IMG_7001.JPG', sourceFilename: 'IMG_7001.JPG', + }); + await db('photos').where({ id }).update({ + source_origin: 'external', external_relpath: 'nas/sub/IMG_7001.JPG', + }); + + const existing = await db('photos').where({ id }).first(); + const event = await db('events').where({ id: eventId }).first(); + const result = await replacePhoto(existing, makeTempFile(), { + originalFilename: 'Edited_7001.jpg', mimeType: 'image/jpeg', event, + }); + + expect(result.success).toBe(true); + const row = await db('photos').where({ id }).first(); + expect(row.source_origin).toBe('managed'); + // Kept, not cleared: adminExternalMedia dedupes a re-scan on + // (event_id, external_relpath). Clearing it would make the next scan + // re-import the NAS original as a duplicate of the photo that just + // replaced it. + expect(row.external_relpath).toBe('nas/sub/IMG_7001.JPG'); + }); + + it('deletes the temp file it was handed', async () => { + // putFromFile copies rather than moves, and the v1 route disables its own + // cleanup — so leaving this behind stranded up to 100 MB per replacement. + const id = await addPhoto({ + filename: 'leak.jpg', originalFilename: 'IMG_7002.JPG', sourceFilename: 'IMG_7002.JPG', + }); + const existing = await db('photos').where({ id }).first(); + const event = await db('events').where({ id: eventId }).first(); + const tempPath = makeTempFile(); + + const result = await replacePhoto(existing, tempPath, { + originalFilename: 'Edited_7002.jpg', mimeType: 'image/jpeg', event, + }); + + expect(result.success).toBe(true); + expect(fs.existsSync(tempPath)).toBe(false); + }); +}); + +describe('migration 193 backfill reaches watcher and external rows', () => { + it('falls back to filename when original_filename was never set', async () => { + // fileWatcher and adminExternalMedia insert `filename` only. Copying + // original_filename alone left those galleries with a NULL match key. + const [row] = await db('photos').insert({ + event_id: eventId, filename: 'IMG_8001.JPG', original_filename: null, + source_filename: null, path: 'slug/IMG_8001.JPG', type: 'individual', + }).returning('id'); + const id = row?.id || row; + + const migration = require('../../migrations/core/193_add_photo_source_filename.js'); + await migration.up(db); + + const after = await db('photos').where({ id }).first(); + expect(after.source_filename).toBe('IMG_8001.JPG'); + }); +}); + +describe('externalRelpathFold — a delivered edit must survive a collision', () => { + it('claims managed rows first, so they win and the external row loses', () => { + // The fold DELETES collision losers, and the survivor used to be + // whichever row was claimed first. A replaced photo keeps its + // external_relpath (so re-scans still dedupe) but holds the edit the + // photographer delivered — losing that to the untouched camera original + // is unrecoverable, where losing the external row is not. + const placements = [ + [{ id: 1, source_origin: 'external', external_relpath: 'shoot/IMG_1.jpg' }, 'base'], + [{ id: 2, source_origin: 'managed', external_relpath: 'shoot/IMG_1.jpg' }, 'base'], + ]; + const claimOrder = placements.slice().sort((a, b) => { + const aManaged = a[0].source_origin === 'managed' ? 0 : 1; + const bManaged = b[0].source_origin === 'managed' ? 0 : 1; + return aManaged - bManaged; + }); + + const claimed = new Map(); + const losers = new Map(); + for (const [row, chosen] of claimOrder) { + const next = chosen ? `${chosen}/${row.external_relpath}` : row.external_relpath; + const winner = claimed.get(next); + if (winner != null) { losers.set(row.id, winner); continue; } + claimed.set(next, row.id); + } + + expect([...losers.keys()]).toEqual([1]); + expect([...claimed.values()]).toEqual([2]); + }); +}); + +describe('mergeMarks', () => { + const photo = { + dominant_color_label: 'green', + average_rating: 4.6, + my_color_label: 'red', + my_rating: 2, + }; + + it('reads only the client verdict for mark_source=client', () => { + expect(mergeMarks(photo, 'client')).toEqual({ color_label: 'green', rating: 5 }); + }); + + it('reads only the photographer verdict for mark_source=mine', () => { + expect(mergeMarks(photo, 'mine')).toEqual({ color_label: 'red', rating: 2 }); + }); + + it('lets the photographer win the colour but keeps the higher rating', () => { + // Colour is a category — one deliberate choice beats an aggregate a + // tie-break already had to guess at. Rating is a magnitude, so taking the + // max avoids quietly demoting a photo somebody rated highly. + expect(mergeMarks(photo, 'either')).toEqual({ color_label: 'red', rating: 5 }); + }); + + it('falls back to the client colour when the photographer set none', () => { + expect(mergeMarks({ ...photo, my_color_label: null }, 'either').color_label).toBe('green'); + }); + + it('reports no marks as null rather than 0/empty string', () => { + expect(mergeMarks({}, 'either')).toEqual({ color_label: null, rating: null }); + expect(mergeMarks(null, 'either')).toEqual({ color_label: null, rating: null }); + }); + + it('keeps "somebody rated this" distinguishable from "nobody did"', () => { + // Matches XmpGenerator.mapRating: any non-zero average is at least 1 star. + expect(roundRating(0)).toBe(0); + expect(roundRating(0.4)).toBe(1); + expect(roundRating(4.5)).toBe(5); + }); +}); + +describe('PhotoFilterBuilder — marked_only', () => { + const { PhotoFilterBuilder } = require('../../src/utils/photoFilterBuilder'); + + const build = (filters) => { + const b = new PhotoFilterBuilder(db('photos').select('photos.id'), eventId); + return b.applyFilters(filters).getQuery(); + }; + + it('matches nothing when mark_source=mine has no admin_id', async () => { + // Must not silently widen to the whole event. + const rows = await build({ marked_only: true, mark_source: 'mine' }); + expect(rows).toHaveLength(0); + }); + + it('finds photos carrying the photographer own mark', async () => { + const id = await addPhoto({ + filename: 'marked.jpg', originalFilename: 'IMG_3001.JPG', sourceFilename: 'IMG_3001.JPG', + }); + await db('photo_admin_marks').insert({ + photo_id: id, event_id: eventId, admin_id: adminId, color_label: 'green', + }); + + const rows = await build({ marked_only: true, mark_source: 'mine', admin_id: adminId }); + expect(rows.map(r => r.id)).toContain(id); + }); + + it('ignores another admin marks', async () => { + const rows = await build({ + marked_only: true, mark_source: 'mine', admin_id: adminId + 999, + }); + expect(rows).toHaveLength(0); + }); +}); diff --git a/backend/migrations/core/193_add_photo_source_filename.js b/backend/migrations/core/193_add_photo_source_filename.js new file mode 100644 index 00000000..efb1ea5e --- /dev/null +++ b/backend/migrations/core/193_add_photo_source_filename.js @@ -0,0 +1,74 @@ +/** + * Preserve the camera-original filename across a replace (Lightroom + * round-trip, #745). + * + * `photos.original_filename` is the only carrier of the camera name + * (`IMG_1234.JPG`) — the stored `filename` is rewritten by + * `generatePhotoFilename` to `__.jpg`. The round-trip + * matches renders back to their proof on that camera name. + * + * The problem: `photoReplacementService.replacePhoto()` overwrites + * `original_filename` with the incoming name. So the moment an editor uploads + * `Smith_Wedding_11234.jpg` over the proof, the camera name is gone and a + * SECOND round-trip on the same photo has nothing left to match on. The bug + * is invisible on the first pass, which is exactly why it needs a column + * rather than a convention. + * + * `source_filename` is written once at ingest and never touched by a replace. + * Existing rows are backfilled from `original_filename`, which for any photo + * that has not yet been replaced IS the camera name — so galleries that + * predate this migration can still round-trip on their first pass. + */ + +exports.up = async function (knex) { + const hasColumn = await knex.schema.hasColumn('photos', 'source_filename'); + if (!hasColumn) { + await knex.schema.alterTable('photos', (table) => { + table.string('source_filename', 255); + }); + } + + // Backfill deliberately OUTSIDE the column guard, for the same reason as + // the index below: a run that died after the alterTable but partway through + // the update would leave the column present and half the rows unfilled, and + // the re-run would skip both. `whereNull` makes this idempotent and + // self-healing — it only ever touches rows that still have nothing. + // + // For a never-replaced photo original_filename IS the camera name. A photo + // replaced before this migration existed has already lost it and cannot be + // recovered; it gets the current name, which is the best available answer + // and no worse than the NULL it would otherwise keep. + // + // COALESCE, not a plain copy: fileWatcher auto-imports and external-media + // scans never set original_filename at all — for those rows the camera name + // only ever lived in `filename`. Copying original_filename alone left every + // NAS-mounted and auto-imported gallery with a NULL match key, which is to + // say the round-trip could not see the galleries most likely to be driven + // from Lightroom. + await knex('photos') + .whereNull('source_filename') + .update({ + source_filename: knex.raw('COALESCE(original_filename, filename)'), + }); + + // Outside the column guard, for the same reason as migration 182: a run + // that died between the two statements would leave the column present and + // the index missing, and the re-run would skip both. IF NOT EXISTS is + // supported by Postgres and SQLite alike. + await knex.raw( + 'CREATE INDEX IF NOT EXISTS photos_source_filename_idx ' + + 'ON photos (event_id, source_filename)' + ); +}; + +exports.down = async function (knex) { + // Drop the index first and unconditionally: SQLite rebuilds the table on + // dropColumn, and a lingering index over the dropped column makes that + // rebuild fail. + await knex.raw('DROP INDEX IF EXISTS photos_source_filename_idx'); + if (await knex.schema.hasColumn('photos', 'source_filename')) { + await knex.schema.alterTable('photos', (table) => { + table.dropColumn('source_filename'); + }); + } +}; diff --git a/backend/src/routes/adminExternalMedia.js b/backend/src/routes/adminExternalMedia.js index f6517ef2..bb63f745 100644 --- a/backend/src/routes/adminExternalMedia.js +++ b/backend/src/routes/adminExternalMedia.js @@ -237,6 +237,11 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos. .insert({ event_id: eventId, filename: f.name, + // The camera-original name (#745). External ingest never sets + // original_filename, and NAS-mounted galleries are among the + // most likely to be driven from Lightroom — without this the + // round-trip has nothing to match a RAW against. + source_filename: f.name, // Keep path as a hint for legacy code but not used for resolution in external mode path: path.join(event.slug, f.name), thumbnail_path: null, diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index d381b813..4c31ef1c 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -178,8 +178,14 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r try { const { eventId } = req.params; - const { category_id, replace_by_name } = req.body; + const { category_id, replace_by_name, match_mode } = req.body; const replaceByName = replace_by_name === 'true' || replace_by_name === true; + // How replace_by_name finds its target (#745). 'exact' is the historical + // behaviour and stays the default; 'number_token' matches on the trailing + // digit run so a render renamed in Lightroom still lands on its proof. + // Anything else falls back to 'exact' rather than erroring — an unknown + // mode must not silently widen the match. + const matchMode = match_mode === 'number_token' ? 'number_token' : 'exact'; logger.info('Upload request received for event:', eventId); logger.info('Body:', req.body); @@ -205,7 +211,9 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r // Subtract likely replacements from cap calculation if (replaceByName && req.files) { for (const file of req.files) { - const candidate = await findReplacementCandidate(parseInt(eventId), file.originalname); + const candidate = await findReplacementCandidate( + parseInt(eventId), file.originalname, { matchMode } + ); if (candidate && !candidate.ambiguous) newFilesCount--; } } @@ -273,7 +281,9 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r if (replaceByName && req.files.length > 0) { const newFiles = []; for (const file of req.files) { - const candidate = await findReplacementCandidate(parseInt(eventId), file.originalname); + const candidate = await findReplacementCandidate( + parseInt(eventId), file.originalname, { matchMode } + ); if (candidate && !candidate.ambiguous) { // Replace existing photo const result = await replacePhoto(candidate, file.path, { @@ -294,7 +304,12 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r } else if (candidate && candidate.ambiguous) { skippedReplacements.push({ filename: file.originalname, - reason: `${candidate.count} photos share this name — uploaded as new`, + reason: matchMode === 'number_token' + ? `${candidate.count} photos share this number — uploaded as new. ` + + 'Multi-camera shoots should prefix the camera index into the ' + + 'filename (cam11234.jpg / cam21234.jpg) and keep it in the ' + + 'delivery name.' + : `${candidate.count} photos share this name — uploaded as new`, }); newFiles.push(file); } else { @@ -373,6 +388,10 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r event_id: parseInt(eventId, 10), filename: newFilename, original_filename: file.originalname, + // Camera-original name, kept separate so a later replace can + // overwrite original_filename without losing the Lightroom + // round-trip's match key (migration 193, #745). + source_filename: file.originalname, path: relativePath, thumbnail_path: null, type: photoType, diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index ca2a6523..bb07a3ab 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -37,9 +37,18 @@ const { slugify } = require('../../utils/slug'); const { formatBoolean } = require('../../utils/dbCompat'); const { parseBooleanInput } = require('../../utils/parsers'); const { isValidEventType } = require('../../services/eventTypeService'); +const { replacePhoto } = require('../../services/photoReplacementService'); +const downloadZipService = require('../../services/downloadZipService'); +const { PhotoFilterBuilder } = require('../../utils/photoFilterBuilder'); +const { PhotoExportService } = require('../../services/photoExportService'); +const { mergeMarks } = require('../../services/markMerge'); const router = express.Router(); +// Reused for its getPhotosWithFeedback() enrichment (colour tallies + the +// caller's own marks); the v1 surface exposes no export formats. +const photoExportService = new PhotoExportService(); + const getStoragePath = () => process.env.STORAGE_PATH || path.join(__dirname, '../../../../storage'); // ────────────────────────────────────────────────────────────────────────── @@ -599,7 +608,7 @@ router.get('/events/:id', apiTokenAuth, requireApiScope('read'), requirePermissi * size_bytes: { type: integer } * category_id: { type: integer, nullable: true } * 400: { description: No file or invalid type } - * 404: { description: Event not found } + * 404: { description: Event not found, or replaces_photo_id not in this event } */ router.post( '/events/:id/photos', @@ -649,6 +658,82 @@ router.post( } } + // Replacement (#745). The Lightroom plugin stores the picpeak photo id + // on the catalogue photo, so the id rides along even after the editor + // renames the render — which makes the id, not the filename, the + // reliable key for putting a finished edit back over its proof. + // + // Scoped to this event on purpose: a token inherits its owner's powers + // across every event they can see, so an id from another gallery would + // otherwise overwrite a photo the caller never named in the URL. + const rawReplacesId = req.body?.replaces_photo_id; + if (rawReplacesId !== undefined && rawReplacesId !== null && rawReplacesId !== '') { + const replacesId = parseInt(rawReplacesId, 10); + if (Number.isNaN(replacesId)) { + // Cleanup is in this route's catch block, so an early return has to + // drop the multer temp file itself or it leaks. + await fs.unlink(tempPath).catch(() => {}); + tempPath = null; + return res.status(400).json({ error: 'replaces_photo_id must be an integer' }); + } + const target = await db('photos') + .where({ id: replacesId, event_id: event.id }) + .first(); + if (!target) { + await fs.unlink(tempPath).catch(() => {}); + tempPath = null; + return res.status(404).json({ + error: `No photo ${replacesId} in event ${event.id}`, + }); + } + + const result = await replacePhoto(target, tempPath, { + originalFilename: req.file.originalname, + mimeType: req.file.mimetype, + event, + }); + // replacePhoto unlinks the temp file on success. Unlink again anyway: + // a FAILED replacement returns before doing so, and this route only + // cleans up in its catch block, so the failure path would otherwise + // strand the upload. Already-gone is not an error here. + await fs.unlink(tempPath).catch(() => {}); + tempPath = null; + if (!result.success) { + return res.status(500).json({ error: `Replacement failed: ${result.error}` }); + } + + // Guests are served a cached ZIP of the whole gallery. Without this + // they keep downloading the pre-edit photo indefinitely, which + // defeats the point of putting the edit back. adminPhotos.js does the + // same after its replacements. + downloadZipService.invalidate(event.id); + + // event.id, not null: the dashboard feed excludes NULL-event rows for + // scoped callers (GHSA-jhcf), so a system-level entry would vanish + // from the audit trail of the photographer who owns the event. + await logActivity('photo_replaced', { + photoId: result.photo.id, + originalFilename: req.file.originalname, + previousFilename: result.previousFilename, + eventName: event.event_name, + via: 'v1_api', + }, event.id, { type: 'admin', id: req.admin.id, name: req.admin.username }); + + return res.status(200).json({ + replaced: true, + photo: { + id: result.photo.id, + filename: result.photo.filename, + original_filename: result.photo.original_filename, + source_filename: result.photo.source_filename, + previous_filename: result.previousFilename, + size_bytes: result.photo.size_bytes, + width: result.photo.width, + height: result.photo.height, + }, + }); + } + const ext = path.extname(req.file.originalname); const finalName = `${Date.now()}_${crypto.randomBytes(4).toString('hex')}${ext}`; // photo.path is stored relative to events/active so resolvePhotoStorageKey @@ -687,6 +772,10 @@ router.post( event_id: event.id, filename: finalName, original_filename: req.file.originalname, + // The camera-original name, kept separate so a later replace can + // overwrite original_filename without losing the round-trip's match + // key (migration 193, #745). + source_filename: req.file.originalname, path: relPath, thumbnail_path: thumbRel, type: photoType, @@ -771,4 +860,228 @@ router.get('/events/:id/share-link', apiTokenAuth, requireApiScope('read'), requ } }); +/** + * @openapi + * /events/{id}/photos: + * get: + * summary: List an event's photos with their proofing marks + * description: > + * Feeds the Lightroom round-trip (#745): the plugin fetches the photos a + * client (or the photographer) marked while proofing, matches them to + * local RAW files by `source_filename`, and applies the stars and colour + * labels in the catalogue. Also usable for any automation that needs to + * know what was picked. + * tags: [Photos] + * security: [{ bearerAuth: [] }] + * parameters: + * - in: path + * name: id + * required: true + * schema: { type: integer } + * - in: query + * name: page + * schema: { type: integer, minimum: 1, default: 1 } + * - in: query + * name: limit + * schema: { type: integer, minimum: 1, maximum: 100, default: 50 } + * - in: query + * name: marked_only + * schema: { type: boolean } + * description: Only photos carrying a star rating or colour label from `mark_source`. + * - in: query + * name: mark_source + * schema: { type: string, enum: [client, mine, either], default: either } + * description: > + * Whose marks `marked_only` and the merged `label`/`rating` fields + * reflect. `mine` is the calling token owner's own triage. + * - in: query + * name: color_labels + * schema: { type: string } + * description: Comma-separated client colours, e.g. `green,yellow`. + * - in: query + * name: my_color_labels + * schema: { type: string } + * description: Comma-separated colours from the token owner's own marks. + * - in: query + * name: min_rating + * schema: { type: number, minimum: 0, maximum: 5 } + * - in: query + * name: my_min_rating + * schema: { type: integer, minimum: 1, maximum: 5 } + * - in: query + * name: logic + * schema: { type: string, enum: [AND, OR], default: AND } + * responses: + * 200: + * description: Photos with feedback + * content: + * application/json: + * schema: + * type: object + * properties: + * photos: + * type: array + * items: + * type: object + * properties: + * id: { type: integer } + * filename: { type: string } + * original_filename: { type: string, nullable: true } + * source_filename: + * type: string + * nullable: true + * description: Camera-original name, preserved across replaces. Match on this. + * average_rating: { type: number } + * feedback_count: { type: integer } + * like_count: { type: integer } + * favorite_count: { type: integer } + * comment_count: { type: integer } + * color_labels: + * type: object + * description: > + * Per-colour tallies across guests, keyed by colour — + * for example a green count of 2 and a red count of 1. + * Braces are spelled out here on purpose: an inline + * JSON example in an unquoted YAML scalar parses as a + * flow mapping and swagger-jsdoc drops the whole route. + * dominant_color_label: { type: string, nullable: true } + * my_rating: { type: integer, nullable: true } + * my_color_label: { type: string, nullable: true } + * color_label: + * type: string + * nullable: true + * description: Merged colour for `mark_source`. What a client should apply. + * rating: + * type: integer + * nullable: true + * description: Merged 0-5 rating for `mark_source`. + * pagination: + * type: object + * properties: + * page: { type: integer } + * limit: { type: integer } + * total: { type: integer } + * filtered: { type: integer } + * pages: { type: integer } + * 403: { description: Token lacks scope or permission } + * 404: { description: Event not found } + */ +router.get( + '/events/:id/photos', + apiTokenAuth, + requireApiScope('read'), + requirePermission('photos.view'), + requireEventOwnership, + [ + query('page').optional().isInt({ min: 1 }).toInt(), + query('limit').optional().isInt({ min: 1, max: 100 }).toInt(), + query('marked_only').optional().isBoolean(), + query('mark_source').optional().isIn(['client', 'mine', 'either']), + query('color_labels').optional().isString(), + query('my_color_labels').optional().isString(), + query('min_rating').optional().isFloat({ min: 0, max: 5 }).toFloat(), + query('my_min_rating').optional().isInt({ min: 1, max: 5 }).toInt(), + query('logic').optional().isIn(['AND', 'OR']) + ], + async (req, res) => { + try { + const errors = validationResult(req); + if (!errors.isEmpty()) { + return res.status(400).json({ errors: errors.array() }); + } + + const eventId = parseInt(req.params.id, 10); + const event = await db('events').where({ id: eventId }).first(); + if (!event) return res.status(404).json({ error: 'Event not found' }); + + const page = req.query.page || 1; + const limit = req.query.limit || 50; + const markSource = req.query.mark_source || 'either'; + + const filters = { + min_rating: req.query.min_rating, + my_min_rating: req.query.my_min_rating, + color_labels: req.query.color_labels, + my_color_labels: req.query.my_color_labels, + marked_only: req.query.marked_only, + mark_source: markSource, + // The token's owning admin. `my_*` filters and marks are per-admin + // (migration 183 is unique on photo_id + admin_id), so a second + // admin's triage is deliberately invisible here. + admin_id: req.admin.id, + logic: req.query.logic || 'AND' + }; + + // Two-step on purpose: PhotoFilterBuilder knows how to FILTER on marks + // but its select list carries none of them, while + // photoExportService.getPhotosWithFeedback knows how to ENRICH but does + // not filter. Filter to a page of ids first, then enrich just those — + // which also keeps the per-colour tally query bounded by page size. + const filterBuilder = new PhotoFilterBuilder( + db('photos').select('photos.id'), + eventId + ); + filterBuilder + .applyFilters(filters) + .applySorting('filename', 'asc') + .applyPagination(page, limit); + + const [idRows, countResult, summary] = await Promise.all([ + filterBuilder.getQuery(), + PhotoFilterBuilder.buildCountQuery(db, eventId, filters), + PhotoFilterBuilder.getSummary(db, eventId) + ]); + + const pageIds = idRows.map(r => r.id); + const photos = pageIds.length + ? await photoExportService.getPhotosWithFeedback(eventId, pageIds, req.admin.id) + : []; + + const filtered = parseInt(countResult[0]?.count, 10) || 0; + + res.json({ + photos: photos.map(photo => { + const merged = mergeMarks(photo, markSource); + return { + id: photo.id, + filename: photo.filename, + original_filename: photo.original_filename || null, + // What the round-trip matches on. Null only for rows predating + // migration 193 that had no original_filename either. + // filename is the last fallback on purpose: fileWatcher and + // external-media ingest never set original_filename, so for NAS + // and auto-import galleries the camera name lives only there. + source_filename: photo.source_filename || photo.original_filename || photo.filename || null, + category: photo.category_name || null, + average_rating: photo.average_rating ? parseFloat(photo.average_rating) : 0, + feedback_count: photo.feedback_count || 0, + like_count: photo.like_count || 0, + favorite_count: photo.favorite_count || 0, + comment_count: photo.comment_count || 0, + color_labels: photo.color_labels || {}, + dominant_color_label: photo.dominant_color_label || null, + my_rating: photo.my_rating ?? null, + my_color_label: photo.my_color_label || null, + color_label: merged.color_label, + rating: merged.rating, + width: photo.width || null, + height: photo.height || null, + uploaded_at: photo.uploaded_at || null + }; + }), + pagination: { + page, + limit, + total: summary.total, + filtered, + pages: Math.ceil(filtered / limit) || 0 + } + }); + } catch (error) { + logger.error('v1 GET /events/:id/photos failed', { error: error.message }); + res.status(500).json({ error: 'Failed to list photos' }); + } + } +); + module.exports = router; diff --git a/backend/src/services/externalRelpathFold.js b/backend/src/services/externalRelpathFold.js index 12fb43e3..d7eae2b4 100644 --- a/backend/src/services/externalRelpathFold.js +++ b/backend/src/services/externalRelpathFold.js @@ -169,7 +169,7 @@ async function foldExternalRelpaths(knex, log = () => {}) { const rows = await knex('photos') .where('event_id', eventId) .whereNotNull('external_relpath') - .select('id', 'external_relpath', 'size_bytes'); + .select('id', 'external_relpath', 'size_bytes', 'source_origin'); // Deciding health from a SAMPLE was the tempting shortcut and it is not // safe: a rebased event whose first few rows happen to come from the most @@ -227,7 +227,24 @@ async function foldExternalRelpaths(knex, log = () => {}) { const claimed = new Map(); const resolved = []; const losers = new Map(); - for (const [row, chosen] of placements) { + + // A replaced photo keeps its external_relpath so the folder re-scan can + // still dedupe on it (#745), but its source_origin is now 'managed' and + // its file is the edit the photographer delivered. Collisions here DELETE + // the loser, and the survivor was whichever row happened to be claimed + // first — so a delivered edit could be destroyed in favour of the + // untouched camera original sitting next to it on the share. + // + // Managed rows claim first, which makes them the survivor in any + // collision. The external row that loses is the recoverable one: it is + // still on the share and a re-scan re-imports it. The edit is not. + const claimOrder = placements.slice().sort((a, b) => { + const aManaged = a[0].source_origin === 'managed' ? 0 : 1; + const bManaged = b[0].source_origin === 'managed' ? 0 : 1; + return aManaged - bManaged; + }); + + for (const [row, chosen] of claimOrder) { const next = chosen ? `${chosen}/${row.external_relpath}` : row.external_relpath; const winner = claimed.get(next); if (winner != null) { losers.set(row.id, winner); collided++; continue; } diff --git a/backend/src/services/fileWatcher.js b/backend/src/services/fileWatcher.js index 46bf33df..cd699da5 100644 --- a/backend/src/services/fileWatcher.js +++ b/backend/src/services/fileWatcher.js @@ -136,6 +136,10 @@ async function processNewPhoto(filePath) { const insertResult = await db('photos').insert({ event_id: event.id, filename: path.basename(filePath), + // The camera-original name. This path never sets original_filename, so + // without this the Lightroom round-trip (#745) has nothing to match a + // RAW against for auto-imported galleries. + source_filename: path.basename(filePath), path: relativePath, thumbnail_path: relativeThumbPath, type: isVideo ? 'video' : photoType, diff --git a/backend/src/services/imageProcessor.js b/backend/src/services/imageProcessor.js index 2e9bdb3b..471e0587 100644 --- a/backend/src/services/imageProcessor.js +++ b/backend/src/services/imageProcessor.js @@ -60,9 +60,31 @@ async function extractRawPreview(rawPath) { } } catch (err) { lastErr = err; + // exiftool missing is a DEPLOYMENT fault, not a bad file, and it fails + // identically for every tag — so stop rather than retrying the same + // spawn twice more and reporting the last one as if it described the + // photo. + if (err && err.code === 'ENOENT') break; } } await fsp.rm(outDir, { recursive: true, force: true }).catch(() => {}); + + // Distinguish "the tool isn't installed" from "this file has no preview". + // Both used to surface as `No usable embedded preview in RAW file X: + // spawn exiftool ENOENT`, which reads as a corrupt photo and sends people + // hunting through their RAWs instead of installing a package. RAW upload is + // the only feature that needs exiftool, so an install can be missing it and + // not find out until someone uploads a CR3. + if (lastErr && lastErr.code === 'ENOENT') { + throw new Error( + 'exiftool is not installed on the server, and it is required to read ' + + `RAW files (${path.basename(rawPath)}). Install it (Debian/Ubuntu: ` + + 'apt-get install libimage-exiftool-perl, Alpine: apk add exiftool, ' + + 'macOS: brew install exiftool) and retry. JPEG and other ordinary ' + + 'images do not need it.' + ); + } + throw new Error(`No usable embedded preview in RAW file ${path.basename(rawPath)}: ${lastErr ? lastErr.message : 'no preview tag returned data'}`); } diff --git a/backend/src/services/markMerge.js b/backend/src/services/markMerge.js new file mode 100644 index 00000000..c097baa1 --- /dev/null +++ b/backend/src/services/markMerge.js @@ -0,0 +1,78 @@ +/** + * Merging several proofing verdicts into the single value a desktop + * cataloguer can apply (Lightroom round-trip, #745). + * + * A photo can carry three different opinions at once: the guest colour + * tallies in `photo_feedback`, the guest star average denormalized onto + * `photos.average_rating`, and the photographer's own triage in + * `photo_admin_marks`. Lightroom has room for exactly one colour label and + * one star rating per photo, so something has to decide. + * + * This is that decision, in one place, so the API response, the XMP export + * and the plugin can never drift apart on it. + */ + + +/** + * Collapse a guest star average to Lightroom's 0-5 integer scale. + * + * Deliberately identical to XmpGenerator.mapRating so a photo exported as an + * XMP sidecar and the same photo fetched through the v1 API never disagree + * about how many stars it has. Note the last branch: any non-zero average + * below 1.5 becomes 1 star, not 0 — "somebody rated this" and "nobody rated + * this" must stay distinguishable. + */ +function roundRating(avgRating) { + const value = parseFloat(avgRating); + if (!value || Number.isNaN(value)) return 0; + if (value >= 4.5) return 5; + if (value >= 3.5) return 4; + if (value >= 2.5) return 3; + if (value >= 1.5) return 2; + return 1; +} + +/** + * Resolve one photo's marks down to a single colour and rating. + * + * @param {Object} photo - row enriched by photoExportService.getPhotosWithFeedback + * @param {'client'|'mine'|'either'} markSource + * @returns {{ color_label: string|null, rating: number|null }} + */ +function mergeMarks(photo, markSource = 'either') { + if (!photo) return { color_label: null, rating: null }; + + const clientColor = photo.dominant_color_label || null; + const clientRating = roundRating(photo.average_rating); + const myColor = photo.my_color_label || null; + const myRating = Number(photo.my_rating) || 0; + + if (markSource === 'client') { + return { + color_label: clientColor, + rating: clientRating || null, + }; + } + + if (markSource === 'mine') { + return { + color_label: myColor, + rating: myRating || null, + }; + } + + // 'either' — the photographer's own mark wins the colour. It is one + // person's deliberate triage rather than an aggregate that a tie-break + // already had to guess at, so it is the stronger signal. Stars take the + // max instead: a rating is a magnitude, and losing the higher of the two + // would quietly demote a photo somebody rated highly. + return { + color_label: myColor || clientColor, + rating: Math.max(myRating, clientRating) || null, + }; +} + +module.exports = { + mergeMarks, + roundRating, +}; diff --git a/backend/src/services/photoExportService.js b/backend/src/services/photoExportService.js index 4d97d391..6ae26321 100644 --- a/backend/src/services/photoExportService.js +++ b/backend/src/services/photoExportService.js @@ -33,6 +33,9 @@ class PhotoExportService { 'photos.id', 'photos.filename', 'photos.original_filename', + // Camera-original name, preserved across replaces (migration 193) so + // the Lightroom round-trip can still match after a re-upload (#745). + 'photos.source_filename', 'photos.path', 'photos.average_rating', 'photos.feedback_count', diff --git a/backend/src/services/photoReplacementService.js b/backend/src/services/photoReplacementService.js index bcb4e20b..ffce78d4 100644 --- a/backend/src/services/photoReplacementService.js +++ b/backend/src/services/photoReplacementService.js @@ -21,15 +21,102 @@ const { resolvePhotoStorageKey } = require('./photoResolver'); const logger = require('../utils/logger'); /** - * Find a replacement candidate by matching original_filename (case-insensitive). - * Returns the photo row if exactly one match, { ambiguous: true, count } if multiple, or null. + * The trailing digit run of a filename stem, e.g. `Smith_Wedding_11234.jpg` + * -> `11234`. Used by the `number_token` match mode below. + * + * Deliberately the LONGEST trailing run and never a fixed last-N slice. + * Multi-camera shoots disambiguate by prefixing the camera index into the + * number (`cam11234.jpg`, `cam21234.jpg`); a last-4 slice reads `1234` from + * both bodies and reintroduces exactly the collision the prefix removes. + * + * @param {string} filename + * @returns {string|null} the digit run, or null when there is none */ -async function findReplacementCandidate(eventId, originalFilename) { +function trailingDigitRun(filename) { + if (!filename) return null; + const stem = String(filename).replace(/\.[^.]+$/, ''); + const match = stem.match(/(\d+)$/); + return match ? match[1] : null; +} + +/** + * Find a replacement candidate. + * + * Two modes: + * + * - `exact` (default, unchanged behaviour): case-insensitive match on the + * name the photo was uploaded under. + * - `number_token` (#745): match on the trailing digit run instead, so an + * editor who renamed `IMG_1234.JPG` to `Smith_Wedding_1234.jpg` in + * Lightroom still lands on the right photo. Opt-in, because a digit run is + * a much weaker key than a filename. + * + * Both modes prefer `source_filename` (the camera-original name, preserved + * across replaces by migration 193) and fall back to `original_filename` for + * rows that predate it. + * + * Returns the photo row on exactly one match, `{ ambiguous: true, count }` + * when several match, or null. Ambiguity is never resolved by guessing — the + * caller uploads the file as new rather than overwriting the wrong photo. + * + * @param {number} eventId + * @param {string} originalFilename + * @param {Object} [opts] + * @param {'exact'|'number_token'} [opts.matchMode='exact'] + */ +async function findReplacementCandidate(eventId, originalFilename, opts = {}) { if (!originalFilename) return null; + const { matchMode = 'exact' } = opts; + + if (matchMode === 'number_token') { + const token = trailingDigitRun(originalFilename); + if (!token) return null; + + // The token has to be compared against the STEM of the stored name, not + // the whole string, so `IMG_1234.JPG` yields `1234` on both sides. That + // comparison stays in JS — expressing it in SQL across two engines is + // more trouble than it is worth — but the CANDIDATE SET is narrowed in + // SQL first. + // + // Without the LIKE this read every photo row in the event, once per + // uploaded file, and twice per file when a photo cap is configured. At + // the 2000-file upload limit against a 5000-photo event that is up to 20M + // rows before any image processing starts. The token is a digit run + // extracted by regex, so it is safe to interpolate and cannot carry a + // LIKE wildcard. + // + // The LIKE over-matches on purpose (it ignores position and extension); + // the exact trailing-run check below is still what decides, so semantics + // are unchanged and only the row count drops. + const pattern = '%' + token + '%'; + const rows = await db('photos') + .where({ event_id: eventId }) + .where(function () { + this.where('source_filename', 'like', pattern) + .orWhere('original_filename', 'like', pattern); + }) + .select('id', 'source_filename', 'original_filename'); + + const matches = rows.filter((row) => { + const stored = row.source_filename || row.original_filename; + return stored && trailingDigitRun(stored) === token; + }); + + if (matches.length > 1) return { ambiguous: true, count: matches.length }; + if (matches.length === 1) { + // Re-read the full row: replacePhoto() needs the columns the narrowed + // select above deliberately skipped (type, filename, source_filename). + return db('photos').where({ id: matches[0].id }).first(); + } + return null; + } const matches = await db('photos') .where({ event_id: eventId }) - .whereRaw('LOWER(original_filename) = LOWER(?)', [originalFilename]); + .where(function () { + this.whereRaw('LOWER(original_filename) = LOWER(?)', [originalFilename]) + .orWhereRaw('LOWER(source_filename) = LOWER(?)', [originalFilename]); + }); if (matches.length === 1) return matches[0]; if (matches.length > 1) return { ambiguous: true, count: matches.length }; @@ -113,14 +200,29 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename, // Ignore — watermark may not exist } - // Upload the new original. + // Upload the new original. `putFromFile` COPIES (LocalFsStorage) or + // uploads (S3) — neither consumes the source, and this function used to + // leave it behind. The v1 route additionally stops its own cleanup on the + // (wrong) assumption that this moved the file, so every replacement + // stranded up to 100 MB in storage/temp. Cleaning up here closes the v1 + // and the admin path at once: adminPhotos only unlinks in its + // new-files branch, so replaced files leaked there too. await storage.putFromFile(finalKey, newFileTempPath, { contentType: mimeType }); + await fsp.unlink(newFileTempPath).catch(() => {}); // Update DB record — preserve id, event_id, category_id, type, visibility, // uploaded_at, sort_order, feedback counts, view/download counts const updates = { filename: newFilename, original_filename: originalFilename, + // source_filename is deliberately NOT in this list. It holds the + // camera-original name and must survive a replace, otherwise the + // Lightroom round-trip (#745) loses its match key the first time an + // editor uploads a renamed render over the proof. Backfilled here only + // when the row predates migration 193 and has nothing stored yet. + ...(existingPhoto.source_filename + ? {} + : { source_filename: existingPhoto.original_filename || originalFilename }), path: relativePath, thumbnail_path: thumbnailPath, size_bytes: stats.size, @@ -129,6 +231,25 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename, captured_at: capturedAt, mime_type: mimeType, media_type: mimeType?.startsWith('video/') ? 'video' : 'image', + // The replacement lives in the managed backend, so the row has to say + // so. resolvePhotoStorageKey gives photo.source_origin precedence over + // everything and returns null for 'reference'/'external' — so leaving + // this set meant the new file was stored and recorded while every + // viewer kept being served the untouched NAS original, with the upload + // orphaned and the API reporting success. + // + // external_relpath is deliberately KEPT. It is no longer used to + // resolve the photo — source_origin decides that, and every other + // consumer reads the two together — but it is still the key + // adminExternalMedia dedupes on when the folder is re-scanned + // (`where({ event_id, external_relpath })`) and the column the unique + // index from migration 186 covers. Clearing it would make the next scan + // treat the NAS original as a new file and import a duplicate + // alongside the photo that just replaced it. + // + // The external file itself is never touched: this repoints the row, it + // does not delete or move anything on the share. + source_origin: 'managed', }; // Face data (#1074): the row keeps its id but now points at a DIFFERENT @@ -173,4 +294,4 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename, } } -module.exports = { findReplacementCandidate, replacePhoto }; +module.exports = { trailingDigitRun, findReplacementCandidate, replacePhoto }; diff --git a/backend/src/services/xmpGenerator.js b/backend/src/services/xmpGenerator.js index f0ce7d95..e939e091 100644 --- a/backend/src/services/xmpGenerator.js +++ b/backend/src/services/xmpGenerator.js @@ -4,6 +4,7 @@ */ const { COLOR_LABEL_TO_XMP, dominantColorLabel } = require('../constants/colorLabels'); +const { roundRating } = require('./markMerge'); class XmpGenerator { /** @@ -50,12 +51,10 @@ class XmpGenerator { * @returns {number} XMP rating (0-5, integer) */ mapRating(avgRating) { - if (!avgRating || avgRating === 0) return 0; - if (avgRating >= 4.5) return 5; - if (avgRating >= 3.5) return 4; - if (avgRating >= 2.5) return 3; - if (avgRating >= 1.5) return 2; - return 1; + // Delegates to markMerge.roundRating (#745) so the sidecar and the v1 API + // can never disagree about how many stars a photo has. The thresholds + // used to live here; they moved rather than being copied. + return roundRating(avgRating); } /** diff --git a/backend/src/utils/photoFilterBuilder.js b/backend/src/utils/photoFilterBuilder.js index 956ca326..e2f1cf63 100644 --- a/backend/src/utils/photoFilterBuilder.js +++ b/backend/src/utils/photoFilterBuilder.js @@ -57,6 +57,9 @@ class PhotoFilterBuilder { has_comments, color_labels, my_color_labels, + my_min_rating, + marked_only, + mark_source = 'either', admin_id, category_id, logic = 'AND' @@ -130,6 +133,56 @@ class PhotoFilterBuilder { })); } + // The caller's own star rating (#745). Same admin_id requirement as the + // colour filter above, and skipped rather than widened without one. + if (my_min_rating !== undefined && my_min_rating !== null && admin_id) { + conditions.push(builder => builder.whereExists(function () { + this.select('*') + .from('photo_admin_marks') + .whereRaw('photo_admin_marks.photo_id = photos.id') + .where('photo_admin_marks.admin_id', admin_id) + .where('photo_admin_marks.rating', '>=', my_min_rating); + })); + } + + // "Only the photos somebody actually marked" — the Lightroom round-trip's + // import scope (#745). This is ONE condition that ORs internally rather + // than several pushed conditions, so it still behaves as a single clause + // when the caller asked for `logic: 'AND'` alongside other filters. + // + // `mark_source` decides whose marks count: the client's proofing verdict, + // the photographer's own triage, or either. 'mine' and 'either' need an + // admin_id for the same reason the filters above do; without one the + // admin half is dropped instead of matching every admin's marks. + if (marked_only === true || marked_only === 'true') { + const wantsClient = mark_source === 'client' || mark_source === 'either'; + const wantsMine = (mark_source === 'mine' || mark_source === 'either') && Boolean(admin_id); + + conditions.push(builder => builder.where(function () { + if (wantsClient) { + this.orWhere('photos.color_label_count', '>', 0); + this.orWhere('photos.average_rating', '>', 0); + } + if (wantsMine) { + this.orWhereExists(function () { + this.select('*') + .from('photo_admin_marks') + .whereRaw('photo_admin_marks.photo_id = photos.id') + .where('photo_admin_marks.admin_id', admin_id) + .where(function () { + this.whereNotNull('photo_admin_marks.color_label') + .orWhereNotNull('photo_admin_marks.rating'); + }); + }); + } + // Neither half available (mark_source 'mine' with no admin_id) — + // match nothing rather than silently returning the whole event. + if (!wantsClient && !wantsMine) { + this.whereRaw('1 = 0'); + } + })); + } + if (category_id) { conditions.push(builder => builder.where('photos.category_id', category_id)); }