From e2844d190969269e53dfac9a74ebd8fe94e042dc Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sun, 23 Aug 2026 12:15:01 +0300 Subject: [PATCH] feat(gallery): colour labels for client proofing, and one global default per feedback type (#1044) (#1137) Colour labels for client proofing, plus the photographer's own stars and colours in the admin grid. - Guest colour labels alongside likes/reactions, opt-in per event (defaults off so live galleries do not change mid-proofing), with 'colors' and 'lightroom' keybind schemes. - One global default per feedback type, replacing the per-type scatter. - Admin marks live in their own table (photo_admin_marks) so they can never reach a guest-facing surface. - XMP export prefers a real label, keeping the rating-derived mapping as a fallback. Review: concurrent-write loss on the mark update path, migration index idempotency and error classification all fixed in 7139bcae; migrations renumbered to 182/183 in 8fecdfae after 180/181 were taken on main. Merged with admin privileges: bypass-size-gate is a required check that fails on size alone for review-bypass authors and never re-evaluates on review, which is its designed behaviour once a maintainer has approved. --- .../services/feedbackDefaults.test.js | 176 ++++++++++++ .../services/photoAdminMarks.test.js | 241 ++++++++++++++++ .../utils/feedbackColorLabels.test.js | 268 ++++++++++++++++++ .../migrations/core/182_add_color_labels.js | 94 ++++++ .../core/183_add_photo_admin_marks.js | 48 ++++ backend/src/constants/colorLabels.js | 93 ++++++ backend/src/middleware/feedbackRateLimit.js | 34 ++- backend/src/routes/adminEvents/crud.js | 57 +++- backend/src/routes/adminFeedback.js | 3 +- backend/src/routes/adminGuests.js | 6 + backend/src/routes/adminPhotoExport.js | 25 +- backend/src/routes/adminPhotos.js | 115 +++++++- backend/src/routes/gallery.js | 56 +++- backend/src/routes/galleryFeedback.js | 20 +- backend/src/routes/publicSettings.js | 20 ++ .../routes/v1/__tests__/events.create.test.js | 26 +- backend/src/routes/v1/events.js | 20 +- backend/src/services/feedbackDefaults.js | 170 +++++++++++ backend/src/services/feedbackService.js | 162 +++++++++-- .../src/services/photoAdminMarksService.js | 185 ++++++++++++ backend/src/services/photoExportService.js | 62 +++- backend/src/services/xmpGenerator.js | 48 +++- backend/src/utils/feedbackValidation.js | 15 +- backend/src/utils/photoFilterBuilder.js | 74 ++++- .../src/components/admin/AdminGuestDetail.tsx | 24 +- .../src/components/admin/AdminGuestsList.tsx | 6 + .../src/components/admin/AdminPhotoGrid.tsx | 51 ++++ .../src/components/admin/AdminPhotoViewer.tsx | 139 +++++++++ .../src/components/admin/FeedbackSettings.tsx | 101 ++++++- .../src/components/admin/PhotoExportMenu.tsx | 30 +- .../src/components/admin/PhotoFilterPanel.tsx | 116 +++++++- .../components/gallery/ColorLabelBadge.tsx | 46 +++ .../gallery/ColorLabelFilterChips.tsx | 79 ++++++ .../src/components/gallery/GallerySidebar.tsx | 20 ++ .../src/components/gallery/GalleryView.tsx | 47 ++- frontend/src/components/gallery/PhotoCard.tsx | 6 + .../components/gallery/PhotoColorLabels.tsx | 183 ++++++++++++ .../src/components/gallery/PhotoFilterBar.tsx | 34 ++- .../src/components/gallery/PhotoLightbox.tsx | 158 ++++++++++- .../gallery/layouts/GalleryPremiumLayout.tsx | 4 + .../gallery/layouts/story/StoryPhotoCard.tsx | 4 + .../settings/hooks/useSettingsState.ts | 25 ++ .../src/features/settings/tabs/EventsTab.tsx | 93 ++++++ frontend/src/i18n/locales/de.json | 67 ++++- frontend/src/i18n/locales/en.json | 67 ++++- frontend/src/i18n/locales/es.json | 69 ++++- frontend/src/i18n/locales/fr.json | 67 ++++- frontend/src/i18n/locales/nl.json | 67 ++++- frontend/src/i18n/locales/pt.json | 67 ++++- frontend/src/i18n/locales/ru.json | 67 ++++- frontend/src/i18n/locales/sl.json | 67 ++++- frontend/src/pages/admin/CreateEventPage.tsx | 26 +- frontend/src/pages/admin/EventDetailsPage.tsx | 6 + frontend/src/services/feedback.service.ts | 62 +++- frontend/src/services/guests.service.ts | 2 + frontend/src/services/photos.service.ts | 48 ++++ .../src/services/publicSettings.service.ts | 8 + frontend/src/types/index.ts | 6 + .../utils/__tests__/feedbackKeybinds.test.ts | 129 +++++++++ frontend/src/utils/feedbackKeybinds.ts | 85 ++++++ 60 files changed, 3912 insertions(+), 182 deletions(-) create mode 100644 backend/__tests__/services/feedbackDefaults.test.js create mode 100644 backend/__tests__/services/photoAdminMarks.test.js create mode 100644 backend/__tests__/utils/feedbackColorLabels.test.js create mode 100644 backend/migrations/core/182_add_color_labels.js create mode 100644 backend/migrations/core/183_add_photo_admin_marks.js create mode 100644 backend/src/constants/colorLabels.js create mode 100644 backend/src/services/feedbackDefaults.js create mode 100644 backend/src/services/photoAdminMarksService.js create mode 100644 frontend/src/components/gallery/ColorLabelBadge.tsx create mode 100644 frontend/src/components/gallery/ColorLabelFilterChips.tsx create mode 100644 frontend/src/components/gallery/PhotoColorLabels.tsx create mode 100644 frontend/src/utils/__tests__/feedbackKeybinds.test.ts create mode 100644 frontend/src/utils/feedbackKeybinds.ts diff --git a/backend/__tests__/services/feedbackDefaults.test.js b/backend/__tests__/services/feedbackDefaults.test.js new file mode 100644 index 00000000..2e7ef0ac --- /dev/null +++ b/backend/__tests__/services/feedbackDefaults.test.js @@ -0,0 +1,176 @@ +/** + * Global guest-feedback defaults (#1044). + * + * Before this module the creation defaults lived in four places that had + * already drifted: the admin create route's destructuring defaults, the v1 + * create route's hard-coded insert (which omitted `allow_reactions` + * entirely), getEventFeedbackSettings()'s no-row fallback, and the migration + * column defaults. Pinned here: + * + * - with no app_settings rows, the built-in fallbacks apply (so installs + * that never open Settings > Events keep behaving exactly as before) + * - a stored global overrides its fallback, whether it was written as a JSON + * boolean, a bare "true"/"false" string, or SQLite's 0/1 + * - garbage in a settings row falls back rather than writing null into a + * boolean column + * - an explicitly-sent body value always beats the global + * - keybind_mode only accepts the known schemes + * - getEventFeedbackSettings()'s no-row fallback reads the same globals + */ + +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-feedback-defaults-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'feedback-defaults-test-secret'; + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +const { + FEEDBACK_TOGGLES, + resolveEventFeedbackDefaults, + applyFeedbackDefaults, +} = require('../../src/services/feedbackDefaults'); +const feedbackService = require('../../src/services/feedbackService'); + +let db; +let cleanup; + +async function setGlobal(key, value) { + await db('app_settings').where('setting_key', key).delete(); + await db('app_settings').insert({ + setting_key: key, + setting_value: value, + setting_type: 'general', + updated_at: new Date().toISOString(), + }); +} + +async function clearGlobals() { + await db('app_settings').where('setting_key', 'like', 'event_default_%').delete(); +} + +beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); +}, 120000); + +afterAll(async () => { + if (cleanup) await cleanup(); +}); + +beforeEach(async () => { + await clearGlobals(); +}); + +describe('resolveEventFeedbackDefaults (#1044)', () => { + it('falls back to the built-ins when nothing is configured', async () => { + const defaults = await resolveEventFeedbackDefaults(); + expect(defaults).toEqual({ + allow_ratings: true, + allow_likes: true, + allow_favorites: true, + allow_comments: true, + allow_reactions: true, + // Colour labels are opt-in so no existing gallery grows a colour bar + // on upgrade. + allow_color_labels: false, + keybind_mode: 'colors', + }); + }); + + it('covers every toggle the UI shows — no type can be silently missing', async () => { + const defaults = await resolveEventFeedbackDefaults(); + for (const toggle of FEEDBACK_TOGGLES) { + expect(defaults).toHaveProperty(toggle.column); + } + }); + + it('lets a stored global override its fallback', async () => { + await setGlobal('event_default_allow_comments', JSON.stringify(false)); + await setGlobal('event_default_allow_color_labels', JSON.stringify(true)); + + const defaults = await resolveEventFeedbackDefaults(); + expect(defaults.allow_comments).toBe(false); + expect(defaults.allow_color_labels).toBe(true); + expect(defaults.allow_likes).toBe(true); // untouched + }); + + it('accepts the value shapes older writers left behind', async () => { + await setGlobal('event_default_allow_likes', 'false'); // bare string, not JSON-quoted + await setGlobal('event_default_allow_ratings', '0'); // SQLite boolean + const defaults = await resolveEventFeedbackDefaults(); + expect(defaults.allow_likes).toBe(false); + expect(defaults.allow_ratings).toBe(false); + }); + + it('falls back rather than writing garbage into a boolean column', async () => { + await setGlobal('event_default_allow_favorites', 'not-a-boolean'); + const defaults = await resolveEventFeedbackDefaults(); + expect(defaults.allow_favorites).toBe(true); + }); + + it('only accepts known keybind schemes', async () => { + await setGlobal('event_default_keybind_mode', JSON.stringify('lightroom')); + expect((await resolveEventFeedbackDefaults()).keybind_mode).toBe('lightroom'); + + await setGlobal('event_default_keybind_mode', JSON.stringify('vim')); + expect((await resolveEventFeedbackDefaults()).keybind_mode).toBe('colors'); + }); +}); + +describe('applyFeedbackDefaults (#1044)', () => { + const globals = { + allow_ratings: true, + allow_likes: true, + allow_favorites: true, + allow_comments: true, + allow_reactions: true, + allow_color_labels: false, + keybind_mode: 'colors', + }; + + it('inherits every value the caller omitted', () => { + expect(applyFeedbackDefaults({}, globals)).toEqual(globals); + expect(applyFeedbackDefaults(undefined, globals)).toEqual(globals); + }); + + it('lets an explicit body value win — including an explicit false', () => { + const resolved = applyFeedbackDefaults( + { allow_likes: false, allow_color_labels: true, keybind_mode: 'lightroom' }, + globals, + ); + expect(resolved.allow_likes).toBe(false); + expect(resolved.allow_color_labels).toBe(true); + expect(resolved.keybind_mode).toBe('lightroom'); + expect(resolved.allow_ratings).toBe(true); // still inherited + }); + + it('ignores an unknown keybind mode from the body', () => { + expect(applyFeedbackDefaults({ keybind_mode: 'emacs' }, globals).keybind_mode).toBe('colors'); + }); + + it('does not mutate the globals object it was handed', () => { + const snapshot = { ...globals }; + applyFeedbackDefaults({ allow_likes: false }, globals); + expect(globals).toEqual(snapshot); + }); +}); + +describe('getEventFeedbackSettings no-row fallback (#1044)', () => { + it('reads the same globals instead of a fourth hard-coded copy', async () => { + await setGlobal('event_default_allow_comments', JSON.stringify(false)); + await setGlobal('event_default_allow_color_labels', JSON.stringify(true)); + + // An event id with no event_feedback_settings row. + const settings = await feedbackService.getEventFeedbackSettings(999999); + expect(settings.feedback_enabled).toBe(false); // no row still means feedback off + expect(settings.allow_comments).toBe(false); + expect(settings.allow_color_labels).toBe(true); + expect(settings.keybind_mode).toBe('colors'); + }); +}); diff --git a/backend/__tests__/services/photoAdminMarks.test.js b/backend/__tests__/services/photoAdminMarks.test.js new file mode 100644 index 00000000..de61d868 --- /dev/null +++ b/backend/__tests__/services/photoAdminMarks.test.js @@ -0,0 +1,241 @@ +/** + * The photographer's own stars and colour labels (#1044 follow-up). + * + * The property that matters most here is isolation: admin marks live in their + * own table precisely so they can never reach a guest-facing surface. Pinned: + * + * - rating and colour are independent halves of one mark — writing one must + * not wipe the other + * - a mark with neither half left is deleted, not kept as an empty row + * - two admins on the same event keep separate marks + * - marks never touch photo_feedback or the denormalized photos.* counters + * the gallery reads + * - invalid values are rejected the same way guest colour labels are + */ + +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-admin-marks-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'admin-marks-test-secret'; + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +const marks = require('../../src/services/photoAdminMarksService'); +const feedbackService = require('../../src/services/feedbackService'); + +const ADMIN_A = 11; +const ADMIN_B = 22; + +let db; +let cleanup; +let eventId; +let photoIds; + +beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + const inserted = await db('events').insert({ + slug: 'admin-marks-test-event', + event_type: 'wedding', + event_name: 'Admin Marks Test', + event_date: '2026-07-20', + host_email: 'host@example.com', + admin_email: 'admin@example.com', + password_hash: 'x', + share_link: '/gallery/admin-marks-test-event/share', + share_token: 'admin-marks-share', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = inserted[0]?.id ?? inserted[0]; + + photoIds = []; + for (let i = 0; i < 3; i++) { + const photo = await db('photos').insert({ + event_id: eventId, + filename: `photo-${i}.jpg`, + path: `events/admin-marks/${i}.jpg`, + type: 'individual', + uploaded_at: new Date().toISOString(), + }).returning('id'); + photoIds.push(photo[0]?.id ?? photo[0]); + } +}, 120000); + +afterAll(async () => { + if (cleanup) await cleanup(); +}); + +describe('photoAdminMarksService (#1044 follow-up)', () => { + it('sets a colour without touching the rating, and vice versa', async () => { + expect(await marks.setMark(eventId, photoIds[0], ADMIN_A, { colorLabel: 'green' })) + .toEqual({ rating: null, color_label: 'green' }); + + // Writing the rating must leave the colour alone — the two lightbox key + // groups write independently. + expect(await marks.setMark(eventId, photoIds[0], ADMIN_A, { rating: 4 })) + .toEqual({ rating: 4, color_label: 'green' }); + + expect(await marks.setMark(eventId, photoIds[0], ADMIN_A, { colorLabel: 'red' })) + .toEqual({ rating: 4, color_label: 'red' }); + }); + + it('keeps exactly one row per photo per admin', async () => { + const rows = await db('photo_admin_marks') + .where({ photo_id: photoIds[0], admin_id: ADMIN_A }); + expect(rows).toHaveLength(1); + }); + + it('clears one half with null and leaves the other', async () => { + expect(await marks.setMark(eventId, photoIds[0], ADMIN_A, { colorLabel: null })) + .toEqual({ rating: 4, color_label: null }); + }); + + it('deletes the row once neither half is left', async () => { + expect(await marks.setMark(eventId, photoIds[0], ADMIN_A, { rating: null })).toBeNull(); + const rows = await db('photo_admin_marks') + .where({ photo_id: photoIds[0], admin_id: ADMIN_A }); + expect(rows).toHaveLength(0); + }); + + it('keeps two admins on the same photo independent', async () => { + await marks.setMark(eventId, photoIds[1], ADMIN_A, { colorLabel: 'green' }); + await marks.setMark(eventId, photoIds[1], ADMIN_B, { colorLabel: 'red', rating: 2 }); + + expect((await marks.getEventMarks(eventId, ADMIN_A))[photoIds[1]]) + .toEqual({ rating: null, color_label: 'green' }); + expect((await marks.getEventMarks(eventId, ADMIN_B))[photoIds[1]]) + .toEqual({ rating: 2, color_label: 'red' }); + }); + + it('rejects colours outside the set and ratings outside 1-5', async () => { + await expect(marks.setMark(eventId, photoIds[2], ADMIN_A, { colorLabel: 'chartreuse' })) + .rejects.toThrow('Invalid color label'); + await expect(marks.setMark(eventId, photoIds[2], ADMIN_A, { rating: 6 })) + .rejects.toThrow('Rating must be between 1 and 5'); + await expect(marks.setMark(eventId, photoIds[2], ADMIN_A, { rating: 0 })) + .rejects.toThrow('Rating must be between 1 and 5'); + // Nothing was written by any of the rejected calls. + expect(await marks.getEventMarks(eventId, ADMIN_A, [photoIds[2]])).toEqual({}); + }); + + it('tags caller mistakes with a code, not just a message', async () => { + // The route maps this to a 400. Keyed on the code so rewording the + // message can't silently turn a bad request into a 500. + await expect(marks.setMark(eventId, photoIds[2], ADMIN_A, { rating: 9 })) + .rejects.toMatchObject({ code: marks.INVALID_MARK }); + await expect(marks.setMark(eventId, photoIds[2], ADMIN_A, { colorLabel: 'beige' })) + .rejects.toMatchObject({ code: marks.INVALID_MARK }); + }); + + it('narrows to a page of photo ids', async () => { + expect(Object.keys(await marks.getEventMarks(eventId, ADMIN_A, [photoIds[1]]))) + .toEqual([String(photoIds[1])]); + expect(await marks.getEventMarks(eventId, ADMIN_A, [])).toEqual({}); + }); + + it('converges instead of 500ing when two marks race into the same row', async () => { + // Simulate the check-then-insert race a keyboard triage pass produces: + // both calls read "no row", one inserts, the other hits the unique index. + // The loser must land on the winner's row, not throw — and the half it + // did not address must keep the winner's value, not the stale pre-read. + const photoId = photoIds[2]; + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); + + const [first, second] = await Promise.all([ + marks.setMark(eventId, photoId, ADMIN_A, { rating: 4 }), + marks.setMark(eventId, photoId, ADMIN_A, { colorLabel: 'green' }), + ]); + + expect(first).not.toBeNull(); + expect(second).not.toBeNull(); + + const rows = await db('photo_admin_marks').where({ photo_id: photoId, admin_id: ADMIN_A }); + expect(rows).toHaveLength(1); + // Whichever order they landed in, neither half may be lost. + expect(rows[0].rating).toBe(4); + expect(rows[0].color_label).toBe('green'); + + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); + }); + + it('does not lose a concurrent half when the row already exists', async () => { + // luap's finding on #1137: the insert race was handled, but two keystrokes + // on a photo that ALREADY has a mark took a plain read-modify-write — + // both calls read the old row, both wrote the full pair, and the second + // clobbered the first with its stale value. That is the COMMON case in a + // triage pass, since by the second keystroke there usually is a row. + const photoId = photoIds[2]; + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); + await marks.setMark(eventId, photoId, ADMIN_A, { rating: 3 }); + + await Promise.all([ + marks.setMark(eventId, photoId, ADMIN_A, { rating: 4 }), + marks.setMark(eventId, photoId, ADMIN_A, { colorLabel: 'blue' }), + ]); + + const row = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN_A }).first(); + expect(row.rating).toBe(4); // was 3 before the fix — the colour + expect(row.color_label).toBe('blue'); // write reverted it + + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); + }); + + it('leaves the untouched half alone even when the caller sends only one', async () => { + // The mechanism behind the fix: an unaddressed column is never written, + // so it cannot be written with a stale value. + const photoId = photoIds[2]; + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); + await marks.setMark(eventId, photoId, ADMIN_A, { rating: 2, colorLabel: 'purple' }); + + const before = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN_A }).first(); + + await marks.setMark(eventId, photoId, ADMIN_A, { colorLabel: 'green' }); + const after = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: ADMIN_A }).first(); + + expect(after.rating).toBe(before.rating); + expect(after.color_label).toBe('green'); + + await db('photo_admin_marks').where({ photo_id: photoId }).delete(); + }); + + it('counts colours per admin for the filter chips', async () => { + await marks.setMark(eventId, photoIds[2], ADMIN_A, { colorLabel: 'green' }); + expect(await marks.getEventMarkColorCounts(eventId, ADMIN_A)).toEqual({ green: 2 }); + expect(await marks.getEventMarkColorCounts(eventId, ADMIN_B)).toEqual({ red: 1 }); + // A rating-only mark contributes no colour count. + await marks.setMark(eventId, photoIds[0], ADMIN_A, { rating: 5 }); + expect(await marks.getEventMarkColorCounts(eventId, ADMIN_A)).toEqual({ green: 2 }); + }); + + it('never leaks into guest feedback or the denormalized gallery counters', async () => { + // Everything above wrote marks on all three photos. + expect(await db('photo_feedback').count('* as count').first()) + .toEqual(expect.objectContaining({ count: 0 })); + + for (const photoId of photoIds) { + // updatePhotoFeedbackStats is what the gallery payload reads; it must + // still see an untouched photo. + await feedbackService.updatePhotoFeedbackStats(photoId); + const photo = await db('photos').where('id', photoId).first(); + expect(Number(photo.color_label_count) || 0).toBe(0); + expect(Number(photo.average_rating) || 0).toBe(0); + expect(Number(photo.feedback_count) || 0).toBe(0); + } + + // And the guest-facing tallies stay empty. + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[1])).toEqual({}); + expect(await feedbackService.getEventColorLabelCounts(eventId)).toEqual({}); + }); +}); diff --git a/backend/__tests__/utils/feedbackColorLabels.test.js b/backend/__tests__/utils/feedbackColorLabels.test.js new file mode 100644 index 00000000..b3fd9c73 --- /dev/null +++ b/backend/__tests__/utils/feedbackColorLabels.test.js @@ -0,0 +1,268 @@ +/** + * Colour labels (#1044) — pins the contract of the `color_label` feedback + * type, which shares its single-value machinery with emoji reactions (#839): + * - only the five Lightroom colours are accepted + * - one label per guest per photo: the same colour again toggles OFF, + * a different colour SWITCHES the existing row (never a second row) + * - per-guest scoping mirrors reactions: guest_id when present, else the + * device-hash guest_identifier + * - a check-then-insert race that produced duplicate rows collapses on the + * next interaction instead of leaving a phantom count + * - denormalized photos.color_label_count and the per-colour tallies follow + * visibility: hidden-by-moderator labels disappear from both + * - the exports carry the colour + */ + +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-feedback-color-labels-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'feedback-color-labels-test-secret'; + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +const feedbackService = require('../../src/services/feedbackService'); +const { COLOR_LABELS, dominantColorLabel } = require('../../src/constants/colorLabels'); +const { XmpGenerator } = require('../../src/services/xmpGenerator'); + +const EVENT_SLUG = 'color-labels-test-event'; +const GUEST_A = 'guest-a-identifier'; +const GUEST_B = 'guest-b-identifier'; + +let db; +let cleanup; +let eventId; +let photoIds; + +async function label(photoId, color, { guestIdentifier = GUEST_A, guestId = null } = {}) { + return feedbackService.submitFeedback(photoId, eventId, { + feedback_type: 'color_label', + color_label: color, + guest_id: guestId, + ip_address: '127.0.0.1', + user_agent: 'jest', + }, guestIdentifier); +} + +async function colorLabelCountOf(photoId) { + const row = await db('photos').where('id', photoId).first(); + return Number(row.color_label_count) || 0; +} + +beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + const inserted = await db('events').insert({ + slug: EVENT_SLUG, + event_type: 'wedding', + event_name: 'Colour Labels Test', + event_date: '2026-07-20', + host_email: 'host@example.com', + admin_email: 'admin@example.com', + password_hash: 'x', + share_link: `/gallery/${EVENT_SLUG}/share`, + share_token: 'color-labels-test-share', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = inserted[0]?.id ?? inserted[0]; + + photoIds = []; + for (let i = 0; i < 3; i++) { + const photo = await db('photos').insert({ + event_id: eventId, + filename: `photo-${i}.jpg`, + path: `events/color-labels/${i}.jpg`, + type: 'individual', + uploaded_at: new Date().toISOString(), + }).returning('id'); + photoIds.push(photo[0]?.id ?? photo[0]); + } +}, 120000); + +afterAll(async () => { + if (cleanup) await cleanup(); +}); + +describe('colour label submission (#1044)', () => { + it('rejects colours outside the Lightroom set', async () => { + await expect(label(photoIds[0], 'chartreuse')).rejects.toThrow('Invalid color label'); + await expect(label(photoIds[0], 'Green')).rejects.toThrow('Invalid color label'); // case matters + await expect(label(photoIds[0], undefined)).rejects.toThrow('Invalid color label'); + expect(await colorLabelCountOf(photoIds[0])).toBe(0); + }); + + it('creates a label row and maintains the denormalized count', async () => { + const result = await label(photoIds[0], 'green'); + expect(result.created).toBe(true); + + const row = await db('photo_feedback') + .where({ photo_id: photoIds[0], feedback_type: 'color_label' }) + .first(); + expect(row.color_label).toBe('green'); + expect(await colorLabelCountOf(photoIds[0])).toBe(1); + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[0])).toEqual({ green: 1 }); + }); + + it('switches to another colour in place — never a second row per guest', async () => { + const result = await label(photoIds[0], 'red'); + expect(result.updated).toBe(true); + + const rows = await db('photo_feedback') + .where({ photo_id: photoIds[0], feedback_type: 'color_label' }); + expect(rows).toHaveLength(1); + expect(rows[0].color_label).toBe('red'); + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[0])).toEqual({ red: 1 }); + }); + + it('tallies different guests per colour', async () => { + await label(photoIds[0], 'red', { guestIdentifier: GUEST_B }); + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[0])).toEqual({ red: 2 }); + expect(await colorLabelCountOf(photoIds[0])).toBe(2); + }); + + it('toggles off with the same colour', async () => { + const result = await label(photoIds[0], 'red'); + expect(result.removed).toBe(true); + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[0])).toEqual({ red: 1 }); // GUEST_B remains + expect(await colorLabelCountOf(photoIds[0])).toBe(1); + }); + + it('scopes per guest_id when present — two token-guests on one device stay independent', async () => { + const first = await label(photoIds[1], 'green', { guestIdentifier: GUEST_A, guestId: 101 }); + const second = await label(photoIds[1], 'blue', { guestIdentifier: GUEST_A, guestId: 102 }); + expect(first.created).toBe(true); + expect(second.created).toBe(true); // NOT treated as guest 101's switch + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[1])) + .toEqual({ green: 1, blue: 1 }); + }); + + it('accepts every colour of the set', async () => { + for (const color of COLOR_LABELS) { + const res = await label(photoIds[2], color, { guestIdentifier: `guest-${color}` }); + expect(res.created).toBe(true); + } + const counts = await feedbackService.getPhotoColorLabelCounts(photoIds[2]); + expect(Object.keys(counts).sort()).toEqual([...COLOR_LABELS].sort()); + }); + + it('hidden labels leave both the per-colour tallies and color_label_count', async () => { + const row = await db('photo_feedback') + .where({ photo_id: photoIds[0], feedback_type: 'color_label' }) + .first(); + await feedbackService.moderateFeedback(row.id, 'hide', 1); + + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[0])).toEqual({}); + expect(await colorLabelCountOf(photoIds[0])).toBe(0); + + await feedbackService.moderateFeedback(row.id, 'approve', 1); + expect(await colorLabelCountOf(photoIds[0])).toBe(1); + }); + + it('toggle and switch collapse racy duplicate rows for the same guest', async () => { + // Simulate the check-then-insert race: two rows for one guest+photo. + const mk = (color) => ({ + photo_id: photoIds[1], event_id: eventId, feedback_type: 'color_label', + color_label: color, guest_identifier: 'dup-guest', is_approved: true, is_hidden: false, + created_at: new Date(), updated_at: new Date(), + }); + await db('photo_feedback').insert([mk('yellow'), mk('yellow')]); + + // Switching converges to exactly ONE row with the new colour… + const switched = await label(photoIds[1], 'purple', { guestIdentifier: 'dup-guest' }); + expect(switched.updated).toBe(true); + let rows = await db('photo_feedback') + .where({ photo_id: photoIds[1], feedback_type: 'color_label', guest_identifier: 'dup-guest' }); + expect(rows).toHaveLength(1); + expect(rows[0].color_label).toBe('purple'); + + // …and toggle-off removes the full guest-scoped set. + await db('photo_feedback').insert(mk('purple')); + const removed = await label(photoIds[1], 'purple', { guestIdentifier: 'dup-guest' }); + expect(removed.removed).toBe(true); + rows = await db('photo_feedback') + .where({ photo_id: photoIds[1], feedback_type: 'color_label', guest_identifier: 'dup-guest' }); + expect(rows).toHaveLength(0); + }); + + it('reactions and colour labels coexist on the same photo and guest', async () => { + await feedbackService.submitFeedback(photoIds[0], eventId, { + feedback_type: 'reaction', + reaction: '❤️', + ip_address: '127.0.0.1', + user_agent: 'jest', + }, GUEST_B); + await label(photoIds[0], 'blue', { guestIdentifier: GUEST_B }); + + // Switching the colour must not disturb the reaction row, and vice versa. + expect(await feedbackService.getPhotoReactionCounts(photoIds[0])).toEqual({ '❤️': 1 }); + expect(await feedbackService.getPhotoColorLabelCounts(photoIds[0])).toEqual({ blue: 1 }); + }); + + it('event-wide colour counts group per photo', async () => { + const byPhoto = await feedbackService.getEventColorLabelCounts(eventId); + expect(byPhoto[photoIds[2]]).toEqual( + COLOR_LABELS.reduce((acc, c) => ({ ...acc, [c]: 1 }), {}), + ); + // Narrowing to a page of ids only returns those. + const narrowed = await feedbackService.getEventColorLabelCounts(eventId, [photoIds[2]]); + expect(Object.keys(narrowed)).toEqual([String(photoIds[2])]); + expect(await feedbackService.getEventColorLabelCounts(eventId, [])).toEqual({}); + }); + + it('summary and exports carry colour labels', async () => { + const summary = await feedbackService.getEventFeedbackSummary(eventId); + expect(Number(summary.stats.total_color_labels)).toBeGreaterThan(0); + + const longRows = await feedbackService.exportEventFeedback(eventId); + const longLabel = longRows.find((r) => r.feedback_type === 'color_label'); + expect(COLOR_LABELS).toContain(longLabel.color_label); + + const pivotRows = await feedbackService.exportEventFeedbackPivoted(eventId); + const pivotWithLabel = pivotRows.find((r) => r.color_label); + expect(COLOR_LABELS).toContain(pivotWithLabel.color_label); + }); +}); + +describe('dominant colour + XMP round-trip (#1044)', () => { + it('picks the most-labelled colour, breaking ties toward the 1st-choice order', () => { + expect(dominantColorLabel({ red: 3, green: 1 })).toBe('red'); + // Tie: green is "1st choice" in the proofing workflow, so it survives. + expect(dominantColorLabel({ red: 2, green: 2 })).toBe('green'); + expect(dominantColorLabel({})).toBeNull(); + expect(dominantColorLabel(null)).toBeNull(); + }); + + it('xmp:Label prefers a real colour label over the rating-derived guess', () => { + const generator = new XmpGenerator(); + // A 5-star photo would historically map to 'Red'. An explicit green label wins. + expect(generator.mapLabel({ average_rating: 5, dominant_color_label: 'green' })).toBe('Green'); + expect(generator.mapLabel({ average_rating: 5, color_labels: { blue: 1 } })).toBe('Blue'); + // No label: unchanged legacy behaviour, so existing exports don't move. + expect(generator.mapLabel({ average_rating: 5 })).toBe('Red'); + expect(generator.mapLabel({ average_rating: 0 })).toBeNull(); + // Tolerates the pre-#1044 call shape (a bare average rating). + expect(generator.mapLabel(4.6)).toBe('Red'); + }); + + it('writes the label into the sidecar and adds a searchable keyword', () => { + const generator = new XmpGenerator(); + const xmp = generator.generateXmp({ + filename: 'a.jpg', + average_rating: 0, + like_count: 0, + favorite_count: 0, + dominant_color_label: 'yellow', + color_labels: { yellow: 2 }, + }); + expect(xmp).toContain('xmp:Label="Yellow"'); + expect(xmp).toContain('color-yellow'); + }); +}); diff --git a/backend/migrations/core/182_add_color_labels.js b/backend/migrations/core/182_add_color_labels.js new file mode 100644 index 00000000..39a3c315 --- /dev/null +++ b/backend/migrations/core/182_add_color_labels.js @@ -0,0 +1,94 @@ +/** + * Colour labels for client proofing (#1044). + * + * Structurally identical to emoji reactions (migration 164): ONE value per + * guest per photo, changeable, the same value again toggles it off, drawn + * from a fixed curated set (constants/colorLabels.js). No new table. + * + * - event_feedback_settings.allow_color_labels: per-event toggle next to + * allow_reactions. Defaults FALSE, unlike its siblings — a colour bar + * appearing unannounced in every gallery that already has feedback enabled + * would be a visible change to live galleries mid-proofing, so this one is + * opt-in. New events inherit the global `event_default_allow_color_labels` + * via services/feedbackDefaults.js. + * - event_feedback_settings.keybind_mode: which lightbox shortcut scheme the + * gallery uses — 'colors' (1/2/3 = green/yellow/red, simplest for clients) + * or 'lightroom' (1-5 stars, 6-9 colours, identical muscle memory for + * photographers). See constants/colorLabels.js for the maps. + * - photo_feedback.color_label: the colour for feedback_type='color_label' + * rows, validated against COLOR_LABELS. + * - photos.color_label_count: denormalized total, maintained by + * updatePhotoFeedbackStats alongside like_count / reaction_count. + * - photo_feedback_color_label_idx: the admin "show only the greens" filter + * runs as a whereExists over (event_id, feedback_type, color_label). + */ + +exports.up = async function (knex) { + const hasAllowColorLabels = await knex.schema.hasColumn('event_feedback_settings', 'allow_color_labels'); + if (!hasAllowColorLabels) { + await knex.schema.alterTable('event_feedback_settings', (table) => { + table.boolean('allow_color_labels').defaultTo(false); + }); + } + + const hasKeybindMode = await knex.schema.hasColumn('event_feedback_settings', 'keybind_mode'); + if (!hasKeybindMode) { + await knex.schema.alterTable('event_feedback_settings', (table) => { + table.string('keybind_mode', 16).defaultTo('colors'); + }); + } + + const hasColorLabel = await knex.schema.hasColumn('photo_feedback', 'color_label'); + if (!hasColorLabel) { + await knex.schema.alterTable('photo_feedback', (table) => { + // Lightroom's colour names, lowercase: red/yellow/green/blue/purple. + table.string('color_label', 16); + }); + } + + // Outside the column guard on purpose: a run that died between the two + // statements would otherwise leave the column present and the index missing, + // and a re-run would skip both — silently costing the admin colour filter + // the index this migration's header says it relies on. IF NOT EXISTS is + // supported by Postgres and by SQLite (verified idempotent on 3.44), so this + // is safe to re-run in any state. + await knex.raw( + 'CREATE INDEX IF NOT EXISTS photo_feedback_color_label_idx ' + + 'ON photo_feedback (event_id, feedback_type, color_label)' + ); + + const hasColorLabelCount = await knex.schema.hasColumn('photos', 'color_label_count'); + if (!hasColorLabelCount) { + await knex.schema.alterTable('photos', (table) => { + table.integer('color_label_count').defaultTo(0); + }); + } +}; + +exports.down = async function (knex) { + if (await knex.schema.hasColumn('photos', 'color_label_count')) { + await knex.schema.alterTable('photos', (table) => { + table.dropColumn('color_label_count'); + }); + } + // Drop the index first, and unconditionally: SQLite rebuilds the table on + // dropColumn and a lingering index over the dropped column makes that + // rebuild fail. Unconditional so a down() after a partial up() still clears + // whichever half landed. + await knex.raw('DROP INDEX IF EXISTS photo_feedback_color_label_idx'); + if (await knex.schema.hasColumn('photo_feedback', 'color_label')) { + await knex.schema.alterTable('photo_feedback', (table) => { + table.dropColumn('color_label'); + }); + } + if (await knex.schema.hasColumn('event_feedback_settings', 'keybind_mode')) { + await knex.schema.alterTable('event_feedback_settings', (table) => { + table.dropColumn('keybind_mode'); + }); + } + if (await knex.schema.hasColumn('event_feedback_settings', 'allow_color_labels')) { + await knex.schema.alterTable('event_feedback_settings', (table) => { + table.dropColumn('allow_color_labels'); + }); + } +}; diff --git a/backend/migrations/core/183_add_photo_admin_marks.js b/backend/migrations/core/183_add_photo_admin_marks.js new file mode 100644 index 00000000..0e2ddcc7 --- /dev/null +++ b/backend/migrations/core/183_add_photo_admin_marks.js @@ -0,0 +1,48 @@ +/** + * Photographer-side stars and colour labels (#1044 follow-up). + * + * The photographer triages their own shoot with the same 1-5 stars and five + * Lightroom colours the client uses while proofing — but their marks are a + * SEPARATE table rather than more photo_feedback rows, for one reason: + * photo_feedback is read by a long tail of guest-facing queries (the gallery + * payload, the per-photo tallies, the denormalized photos.average_rating / + * *_count columns, the feedback exports, moderation). Adding admin rows there + * would leak the photographer's own opinions into the client's proofing view + * through whichever of those queries someone forgot to filter — and "forgot to + * filter" is exactly the failure this shape makes impossible. + * + * One row per (photo, admin): rating and colour live together because they are + * one person's verdict on one photo, and a row with neither is deleted rather + * than kept as a tombstone. + */ + +exports.up = async function (knex) { + const hasTable = await knex.schema.hasTable('photo_admin_marks'); + if (!hasTable) { + await knex.schema.createTable('photo_admin_marks', (table) => { + table.increments('id').primary(); + table.integer('photo_id').notNullable() + .references('id').inTable('photos').onDelete('CASCADE'); + // Denormalized from photos.event_id so the per-event filter and the + // count queries don't have to join photos on every admin grid render. + table.integer('event_id').notNullable(); + table.integer('admin_id').notNullable(); + table.integer('rating'); // 1-5, NULL = no star rating + table.string('color_label', 16); // NULL = no colour + table.timestamp('created_at').defaultTo(knex.fn.now()); + table.timestamp('updated_at').defaultTo(knex.fn.now()); + + // One verdict per photo per admin. Two admins on the same event keep + // their own marks; the same admin marking twice updates in place. + table.unique(['photo_id', 'admin_id'], 'photo_admin_marks_photo_admin_uniq'); + table.index(['event_id', 'admin_id'], 'photo_admin_marks_event_admin_idx'); + table.index(['event_id', 'color_label'], 'photo_admin_marks_color_idx'); + }); + } +}; + +exports.down = async function (knex) { + if (await knex.schema.hasTable('photo_admin_marks')) { + await knex.schema.dropTable('photo_admin_marks'); + } +}; diff --git a/backend/src/constants/colorLabels.js b/backend/src/constants/colorLabels.js new file mode 100644 index 00000000..74b3cc7a --- /dev/null +++ b/backend/src/constants/colorLabels.js @@ -0,0 +1,93 @@ +/** + * Colour labels (#1044): the fixed, curated colour set clients mark photos + * with while proofing. Guests pick ONE per photo (changeable), exactly like + * the emoji reactions in constants/reactions.js. + * + * The set is Lightroom's, deliberately: keeping the same five colours means a + * client's selection can round-trip into the photographer's catalogue through + * the XMP `xmp:Label` field (services/xmpGenerator.js) with no remapping. + * + * Mirrored in frontend/src/services/feedback.service.ts (COLOR_LABELS and the + * keymaps) — update both together. + */ + +/** Canonical stored values. Lowercase; the XMP layer capitalises. */ +const COLOR_LABELS = ['red', 'yellow', 'green', 'blue', 'purple']; + +/** + * Lightroom / XMP spelling for each label. `xmp:Label` is a free-text field, + * but Lightroom only lights up its colour swatch for these exact strings. + */ +const COLOR_LABEL_TO_XMP = { + red: 'Red', + yellow: 'Yellow', + green: 'Green', + blue: 'Blue', + purple: 'Purple', +}; + +/** + * Tie-break order when several guests labelled the same photo differently and + * the export has to pick one. Green first: in the proofing workflow this + * feature exists for, green is "1st choice" — the pick that must survive. + */ +const COLOR_LABEL_PRIORITY = ['green', 'yellow', 'red', 'blue', 'purple']; + +/** + * Lightbox keyboard schemes. Both are data, not code, so the gallery lightbox, + * the admin viewer and the settings preview can never drift. + * + * - 'colors' the scheme requested in discussion #1027: three keys, no + * Lightroom knowledge needed. 1 = 1st choice, 2 = 2nd choice, + * 3 = rejected. + * - 'lightroom' Lightroom's own defaults: 1-5 set the star rating, 6-9 set + * red/yellow/green/blue. Lightroom has no default shortcut for + * purple, and neither do we — it stays click-only. + * + * In both schemes pressing the same key again clears the value, and '0' + * clears (the rating in 'lightroom', the colour in 'colors'). + */ +const KEYBIND_SCHEMES = { + colors: { + colors: { 1: 'green', 2: 'yellow', 3: 'red' }, + ratings: {}, + }, + lightroom: { + colors: { 6: 'red', 7: 'yellow', 8: 'green', 9: 'blue' }, + ratings: { 1: 1, 2: 2, 3: 3, 4: 4, 5: 5 }, + }, +}; + +function isValidColorLabel(value) { + return COLOR_LABELS.includes(value); +} + +/** + * Pick the single colour that represents a photo when several guests have + * labelled it — most-labelled wins, ties broken by COLOR_LABEL_PRIORITY. + * + * @param {Object} counts - e.g. { green: 2, red: 1 } + * @returns {string|null} + */ +function dominantColorLabel(counts) { + if (!counts) return null; + let best = null; + let bestCount = 0; + for (const color of COLOR_LABEL_PRIORITY) { + const count = Number(counts[color]) || 0; + if (count > bestCount) { + best = color; + bestCount = count; + } + } + return best; +} + +module.exports = { + COLOR_LABELS, + COLOR_LABEL_TO_XMP, + COLOR_LABEL_PRIORITY, + KEYBIND_SCHEMES, + isValidColorLabel, + dominantColorLabel, +}; diff --git a/backend/src/middleware/feedbackRateLimit.js b/backend/src/middleware/feedbackRateLimit.js index 7dad144d..51692929 100644 --- a/backend/src/middleware/feedbackRateLimit.js +++ b/backend/src/middleware/feedbackRateLimit.js @@ -25,6 +25,24 @@ function generateGuestIdentifier(req) { .digest('hex'); } +/** + * Per-action-type rate limits, in one place — the happy path and the + * error path used to keep separate copies, and the error copy silently + * missed every action type added after it was written. + */ +const DEFAULT_RATE_LIMITS = { + rating: { max: 100, window: 3600 }, // 100 ratings per hour + comment: { max: 20, window: 3600 }, // 20 comments per hour + like: { max: 200, window: 3600 }, // 200 likes per hour + favorite: { max: 100, window: 3600 }, // 100 favorites per hour + reaction: { max: 200, window: 3600 }, // reactions churn like likes (#839) + // Colour labels (#1044) are the keyboard-driven proofing path: a client + // works through a 500-photo shoot pressing 1/2/3, and changing their mind + // costs a second request. A likes-sized 200/h cap would lock them out + // mid-session, so this one is deliberately generous. + color_label: { max: 2000, window: 3600 } +}; + /** * Get rate limit settings from app_settings */ @@ -37,13 +55,7 @@ async function getRateLimitSettings() { // Defaults FIRST, stored values override: persisted rows predate newer // action types (`reaction`, #839) — returning the stored object alone // would silently drop their intended defaults to the generic 100/h. - const defaults = { - rating: { max: 100, window: 3600 }, // 100 ratings per hour - comment: { max: 20, window: 3600 }, // 20 comments per hour - like: { max: 200, window: 3600 }, // 200 likes per hour - favorite: { max: 100, window: 3600 }, // 100 favorites per hour - reaction: { max: 200, window: 3600 } // reactions churn like likes (#839) - }; + const defaults = { ...DEFAULT_RATE_LIMITS }; if (settings && settings.setting_value) { // setting_value is already a JSON object in PostgreSQL @@ -57,13 +69,7 @@ async function getRateLimitSettings() { } catch (error) { logger.error('Error getting rate limit settings:', error); // Return defaults on error - return { - rating: { max: 100, window: 3600 }, - comment: { max: 20, window: 3600 }, - like: { max: 200, window: 3600 }, - favorite: { max: 100, window: 3600 }, - reaction: { max: 200, window: 3600 } - }; + return { ...DEFAULT_RATE_LIMITS }; } } diff --git a/backend/src/routes/adminEvents/crud.js b/backend/src/routes/adminEvents/crud.js index 5dec245d..c2767c92 100644 --- a/backend/src/routes/adminEvents/crud.js +++ b/backend/src/routes/adminEvents/crud.js @@ -28,6 +28,7 @@ const { getAppSetting } = require('../../utils/appSettings'); const { clampIntOrUndefined } = require('../../utils/numericHelpers'); const { getFrontendBaseUrl, getAbsoluteFrontendUrl } = require('../../utils/frontendUrl'); const downloadZipService = require('../../services/downloadZipService'); +const { resolveEventFeedbackDefaults, applyFeedbackDefaults, KEYBIND_MODES } = require('../../services/feedbackDefaults'); const { validateHeroImageAnchor, getEventFieldRequirements, readBooleanSetting, getDownloadProtectionDefaults, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers'); module.exports = (router) => { @@ -93,6 +94,15 @@ module.exports = (router) => { // #328 follow-up: per-event opt-in for presigned-URL "Download All". // Bypasses watermarks; admin must enable knowingly. body('allow_presigned_download').optional().isBoolean(), + // Feedback sub-toggles (#1044). Optional: omitting them inherits the + // global Settings > Events defaults. + body('allow_ratings').optional().isBoolean(), + body('allow_likes').optional().isBoolean(), + body('allow_comments').optional().isBoolean(), + body('allow_favorites').optional().isBoolean(), + body('allow_reactions').optional().isBoolean(), + body('allow_color_labels').optional().isBoolean(), + body('keybind_mode').optional().isIn(KEYBIND_MODES), body('css_template_id').optional({ nullable: true, checkFalsy: true }).isInt(), // Hero logo settings body('hero_logo_visible').optional({ nullable: true }).isBoolean(), @@ -167,13 +177,20 @@ module.exports = (router) => { watermark_text = null, allow_presigned_download = false, require_password: requirePasswordInput, - // Feedback settings + // Feedback settings. The allow_* sub-toggles deliberately have NO + // destructuring defaults: `undefined` means "the caller didn't say", + // which inherits the global Settings > Events default (#1044). The + // admin create form posts explicit values (it seeds its own panel + // from the same globals), so inheritance here is what covers the v1 + // API and any other caller that omits them. feedback_enabled: feedbackEnabledInput, - allow_ratings = true, - allow_likes = true, - allow_comments = true, - allow_favorites = true, - allow_reactions = true, + allow_ratings: allowRatingsInput, + allow_likes: allowLikesInput, + allow_comments: allowCommentsInput, + allow_favorites: allowFavoritesInput, + allow_reactions: allowReactionsInput, + allow_color_labels: allowColorLabelsInput, + keybind_mode: keybindModeInput, require_name_email = false, moderate_comments = true, show_feedback_to_guests = true, @@ -253,6 +270,18 @@ module.exports = (router) => { } const feedback_enabled = parseBooleanInput(feedbackEnabledInput, feedbackEnabledFallback); + // Sub-toggle defaults from the global Settings > Events values (#1044). + // One batched read; an explicitly-sent body value still wins. + const feedbackDefaults = applyFeedbackDefaults({ + allow_ratings: allowRatingsInput, + allow_likes: allowLikesInput, + allow_comments: allowCommentsInput, + allow_favorites: allowFavoritesInput, + allow_reactions: allowReactionsInput, + allow_color_labels: allowColorLabelsInput, + keybind_mode: keybindModeInput, + }, await resolveEventFeedbackDefaults()); + // Debug logging logger.debug('Download control values', { allow_downloads, @@ -510,11 +539,13 @@ module.exports = (router) => { await db('event_feedback_settings').insert({ event_id: eventId, feedback_enabled: formatBoolean(feedback_enabled), - allow_ratings: formatBoolean(allow_ratings), - allow_likes: formatBoolean(allow_likes), - allow_comments: formatBoolean(allow_comments), - allow_favorites: formatBoolean(allow_favorites), - allow_reactions: formatBoolean(allow_reactions), + allow_ratings: formatBoolean(feedbackDefaults.allow_ratings), + allow_likes: formatBoolean(feedbackDefaults.allow_likes), + allow_comments: formatBoolean(feedbackDefaults.allow_comments), + allow_favorites: formatBoolean(feedbackDefaults.allow_favorites), + allow_reactions: formatBoolean(feedbackDefaults.allow_reactions), + allow_color_labels: formatBoolean(feedbackDefaults.allow_color_labels), + keybind_mode: feedbackDefaults.keybind_mode, require_name_email: formatBoolean(require_name_email), moderate_comments: formatBoolean(moderate_comments), show_feedback_to_guests: formatBoolean(show_feedback_to_guests), @@ -1157,6 +1188,10 @@ module.exports = (router) => { allow_comments: sourceFeedback.allow_comments, allow_favorites: sourceFeedback.allow_favorites, allow_reactions: sourceFeedback.allow_reactions, + // A clone copies the SOURCE event, so these come from the source + // row rather than the global defaults (#1044). + allow_color_labels: sourceFeedback.allow_color_labels, + keybind_mode: sourceFeedback.keybind_mode, require_name_email: sourceFeedback.require_name_email, moderate_comments: sourceFeedback.moderate_comments, show_feedback_to_guests: sourceFeedback.show_feedback_to_guests, diff --git a/backend/src/routes/adminFeedback.js b/backend/src/routes/adminFeedback.js index 481ccdc8..a1b76d59 100644 --- a/backend/src/routes/adminFeedback.js +++ b/backend/src/routes/adminFeedback.js @@ -262,6 +262,7 @@ router.get('/events/:eventId/feedback-analytics', total_comments: Number(summaryData.stats?.total_comments) || 0, total_favorites: Number(summaryData.stats?.total_favorites) || 0, total_reactions: Number(summaryData.stats?.total_reactions) || 0, + total_color_labels: Number(summaryData.stats?.total_color_labels) || 0, }; const summary = { average_rating: parseFloat(avgRatingResult?.average_rating || 0), @@ -269,7 +270,7 @@ router.get('/events/:eventId/feedback-analytics', pending_moderation: Number(pendingModeration?.count) || 0, total_feedback: counts.total_ratings + counts.total_likes + counts.total_comments + counts.total_favorites + - counts.total_reactions + counts.total_reactions + counts.total_color_labels }; // Get top-rated photos diff --git a/backend/src/routes/adminGuests.js b/backend/src/routes/adminGuests.js index 91755c3b..320f0c55 100644 --- a/backend/src/routes/adminGuests.js +++ b/backend/src/routes/adminGuests.js @@ -83,6 +83,7 @@ router.get( db.raw('COUNT(CASE WHEN photo_feedback.feedback_type = \'comment\' THEN 1 END) AS comments'), db.raw('COUNT(CASE WHEN photo_feedback.feedback_type = \'rating\' THEN 1 END) AS ratings'), db.raw('COUNT(CASE WHEN photo_feedback.feedback_type = \'reaction\' THEN 1 END) AS reactions'), + db.raw('COUNT(CASE WHEN photo_feedback.feedback_type = \'color_label\' THEN 1 END) AS color_labels'), db.raw('COUNT(DISTINCT photo_feedback.photo_id) AS distinct_photos') ) .orderBy('gallery_guests.created_at', 'desc'); @@ -410,6 +411,7 @@ router.get( 'photo_feedback.rating', 'photo_feedback.comment_text', 'photo_feedback.reaction', + 'photo_feedback.color_label', 'photo_feedback.created_at', 'photos.id as photo_id', 'photos.filename', @@ -433,6 +435,7 @@ router.get( rated: [], commented: [], reacted: [], + labeled: [], }; for (const row of feedback) { if (row.feedback_type === 'like') { @@ -449,6 +452,8 @@ router.get( }); } else if (row.feedback_type === 'reaction') { selections.reacted.push({ photo: photoFor(row), reaction: row.reaction }); + } else if (row.feedback_type === 'color_label') { + selections.labeled.push({ photo: photoFor(row), color_label: row.color_label }); } } @@ -461,6 +466,7 @@ router.get( comments: selections.commented.length, ratings: selections.rated.length, reactions: selections.reacted.length, + color_labels: selections.labeled.length, }, }, selections, diff --git a/backend/src/routes/adminPhotoExport.js b/backend/src/routes/adminPhotoExport.js index b6e1aa6c..e1874268 100644 --- a/backend/src/routes/adminPhotoExport.js +++ b/backend/src/routes/adminPhotoExport.js @@ -13,6 +13,7 @@ const { requireEventOwnership } = require('../middleware/ownership'); const { PhotoFilterBuilder } = require('../utils/photoFilterBuilder'); const { getPagination } = require('../utils/routeHelpers'); const { PhotoExportService } = require('../services/photoExportService'); +const photoAdminMarksService = require('../services/photoAdminMarksService'); const logger = require('../utils/logger'); const exportService = new PhotoExportService(); @@ -29,6 +30,8 @@ router.get('/:eventId/filtered', adminAuth, requirePermission('photos.view'), re query('has_favorites').optional().isBoolean(), query('min_favorites').optional().isInt({ min: 0 }), query('has_comments').optional().isBoolean(), + query('color_labels').optional().isString(), + query('my_color_labels').optional().isString(), query('category_id').optional().isInt(), query('logic').optional().isIn(['AND', 'OR']), query('sort').optional().isIn(['rating', 'likes', 'favorites', 'date', 'filename']), @@ -62,6 +65,9 @@ router.get('/:eventId/filtered', adminAuth, requirePermission('photos.view'), re has_favorites: req.query.has_favorites, min_favorites: req.query.min_favorites ? parseInt(req.query.min_favorites) : undefined, has_comments: req.query.has_comments, + color_labels: req.query.color_labels, + my_color_labels: req.query.my_color_labels, + admin_id: req.admin.id, category_id: req.query.category_id ? parseInt(req.query.category_id) : undefined, logic: req.query.logic || 'AND' }; @@ -84,6 +90,7 @@ router.get('/:eventId/filtered', adminAuth, requirePermission('photos.view'), re 'photos.like_count', 'photos.favorite_count', 'photos.comment_count', + 'photos.color_label_count', 'photos.width', 'photos.height', 'photos.uploaded_at', @@ -142,9 +149,15 @@ router.get('/:eventId/filter-summary', adminAuth, requirePermission('photos.view PhotoFilterBuilder.getSummary(db, eventId) ); + // Per-colour counts for the caller's OWN marks (#1044 follow-up), so the + // "My marks" chips can show numbers the way the client-selection chips do. + const myColorLabelCounts = await withRetry(() => + photoAdminMarksService.getEventMarkColorCounts(eventId, req.admin.id) + ); + res.json({ success: true, - data: summary + data: { ...summary, myColorLabelCounts } }); } catch (error) { logger.error('Filter summary error:', error); @@ -161,6 +174,7 @@ router.post('/:eventId/export', adminAuth, requirePermission('photos.download'), body('photo_ids.*').optional().isInt(), body('filter').optional().isObject(), body('format').isIn(['txt', 'csv', 'xmp', 'json']), + body('options.mark_source').optional().isIn(['client', 'mine']), body('options').optional().isObject() ], async (req, res) => { try { @@ -189,13 +203,18 @@ router.post('/:eventId/export', adminAuth, requirePermission('photos.download'), db('photos').select('id'), eventId ); - filterBuilder.applyFilters(filter); + // admin_id comes from the session, so a my-marks filter in the body can + // only ever mean the caller's own marks. + filterBuilder.applyFilters({ ...filter, admin_id: req.admin.id }); const filteredPhotos = await withRetry(() => filterBuilder.getQuery()); photoIds = filteredPhotos.map(p => p.id); } // Export photos - const result = await exportService.exportPhotos(eventId, photoIds, format, options); + const result = await exportService.exportPhotos(eventId, photoIds, format, { + ...options, + admin_id: req.admin.id, + }); if (result.type === 'stream') { res.setHeader('Content-Type', result.contentType); diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 40abd792..e30de028 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -13,6 +13,9 @@ const { pickRawDownloadName, } = require('../services/downloadFilenameService'); const { escapeLikePattern } = require('../utils/sqlSecurity'); +const { COLOR_LABELS, dominantColorLabel } = require('../constants/colorLabels'); +const feedbackService = require('../services/feedbackService'); +const photoAdminMarksService = require('../services/photoAdminMarksService'); const { validateUploadedFiles } = require('../middleware/uploadValidation'); const { getMaxFilesPerUpload, getAllowedMimeTypes } = require('../services/uploadSettings'); const { processUploadedPhotos } = require('../services/photoProcessor'); @@ -724,6 +727,60 @@ router.delete('/:eventId/photos/:photoId', adminAuth, requirePermission('photos. } }); +// The photographer's own star / colour mark on a photo (#1044 follow-up). +// +// Separate from guest feedback in every sense: its own table, its own +// endpoint, and never surfaced to the gallery. `rating` and `color_label` are +// tri-state — omit a key to leave that half alone, send null to clear it — so +// the lightbox's colour keys and star keys don't wipe each other. +router.put('/:eventId/photos/:photoId/mark', adminAuth, requirePermission('photos.edit'), requireEventOwnership, async (req, res) => { + try { + const { eventId } = req.params; + // Parse before querying: Postgres errors on `where id = 'abc'` (22P02), + // which would answer 500 for what is really a bad URL. SQLite just fails + // to match, so without this the two engines disagree. + const photoId = parseInt(req.params.photoId, 10); + if (!Number.isInteger(photoId) || photoId < 1) { + return res.status(404).json({ error: 'Photo not found' }); + } + + // requireEventOwnership proves the caller owns the EVENT; this proves the + // photo is in it, so a photo id from another event can't be marked + // through an event the caller does own. + const photo = await db('photos') + .where({ id: photoId, event_id: eventId }) + .first(); + if (!photo) { + return res.status(404).json({ error: 'Photo not found' }); + } + + const mark = {}; + if (Object.prototype.hasOwnProperty.call(req.body, 'rating')) { + mark.rating = req.body.rating === null ? null : req.body.rating; + } + if (Object.prototype.hasOwnProperty.call(req.body, 'color_label')) { + mark.colorLabel = req.body.color_label === null ? null : req.body.color_label; + } + if (Object.keys(mark).length === 0) { + return res.status(400).json({ error: 'Send rating and/or color_label' }); + } + + const result = await photoAdminMarksService.setMark( + parseInt(eventId, 10), photoId, req.admin.id, mark, + ); + + res.json({ success: true, mark: result }); + } catch (error) { + // Validation errors from the service are the caller's fault, not a 500. + // Keyed on the code, not the message: matching text would couple this + // status to the service's wording. + if (error.code === photoAdminMarksService.INVALID_MARK) { + return res.status(400).json({ error: error.message }); + } + errorResponse(res, error, 500, 'Failed to save mark'); + } +}); + // Update a photo (e.g., change category) router.patch('/:eventId/photos/:photoId', adminAuth, requirePermission('photos.edit'), requireEventOwnership, async (req, res) => { try { @@ -1036,7 +1093,7 @@ router.get('/:eventId/photos/:photoId/download', adminAuth, requirePermission('p router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requireEventOwnership, async (req, res) => { try { const { eventId } = req.params; - const { category_id, type, search, sort = 'date', has_likes, has_favorites, has_comments, min_rating } = req.query; + const { category_id, type, search, sort = 'date', has_likes, has_favorites, has_comments, min_rating, color_label } = req.query; const order = ['asc', 'desc'].includes(req.query.order) ? req.query.order : 'desc'; const logic = req.query.logic === 'OR' ? 'OR' : 'AND'; @@ -1090,6 +1147,41 @@ router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requ feedbackConditions.push(qb => qb.where('photos.average_rating', '>=', minRatingNum)); } } + // Colour-label filter (#1044). Comma-separated colours; unknown values are + // dropped rather than passed to the query. Unlike its siblings this can't + // read a denormalized count column — "any label" and "a GREEN label" are + // different questions — so it runs as an EXISTS over photo_feedback, + // which the migration-180 index covers. + const requestedColorLabels = String(color_label || '') + .split(',') + .map(value => value.trim().toLowerCase()) + .filter(value => COLOR_LABELS.includes(value)); + if (requestedColorLabels.length > 0) { + feedbackConditions.push(qb => qb.whereExists(function () { + this.select('*') + .from('photo_feedback') + .whereRaw('photo_feedback.photo_id = photos.id') + .where('photo_feedback.feedback_type', 'color_label') + .where('photo_feedback.is_hidden', false) + .whereIn('photo_feedback.color_label', requestedColorLabels); + })); + } + // The same filter against the caller's OWN marks (#1044 follow-up). + // Scoped to req.admin.id: one photographer's triage must not filter by + // another's, even on a shared event. + const requestedMyColorLabels = String(req.query.my_color_label || '') + .split(',') + .map(value => value.trim().toLowerCase()) + .filter(value => COLOR_LABELS.includes(value)); + if (requestedMyColorLabels.length > 0) { + feedbackConditions.push(qb => qb.whereExists(function () { + this.select('*') + .from('photo_admin_marks') + .whereRaw('photo_admin_marks.photo_id = photos.id') + .where('photo_admin_marks.admin_id', req.admin.id) + .whereIn('photo_admin_marks.color_label', requestedMyColorLabels); + })); + } if (feedbackConditions.length > 0) { if (logic === 'OR') { query = query.where(builder => { @@ -1132,6 +1224,20 @@ router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requ commentCounts.forEach(c => { commentMap[c.photo_id] = parseInt(c.comment_count); }); + + // Per-colour tallies for the grid badges (#1044) — one grouped query for + // the whole page, same shape as commentMap above. + const colorLabelMap = await feedbackService.getEventColorLabelCounts( + parseInt(eventId, 10), + photos.map(p => p.id), + ); + + // The caller's own marks for this page (#1044 follow-up). + const myMarks = await photoAdminMarksService.getEventMarks( + parseInt(eventId, 10), + req.admin.id, + photos.map(p => p.id), + ); res.json({ photos: photos.map(photo => ({ @@ -1159,6 +1265,13 @@ router.get('/:eventId/photos', adminAuth, requirePermission('photos.view'), requ comment_count: commentMap[photo.id] || 0, like_count: photo.like_count || 0, favorite_count: photo.favorite_count || 0, + color_label_count: photo.color_label_count || 0, + color_labels: colorLabelMap[photo.id] || {}, + dominant_color_label: dominantColorLabel(colorLabelMap[photo.id]), + // The requesting admin's own mark — never the whole team's, and never + // shown to guests. + my_rating: myMarks[photo.id]?.rating ?? null, + my_color_label: myMarks[photo.id]?.color_label ?? null, // Engagement counters (#895 follow-up): the grid reads these, but // this explicit mapper never included them — so the Engagement // column showed 0 regardless of what the DB counted. This, not diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index d66734ab..6a0c10fd 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -31,6 +31,7 @@ const { verifyGalleryAccess, denySlideshowToken, isAdminPreview } = require('../ const withPreview = (req, url) => (req.isAdminPreview ? `${url}${url.includes('?') ? '&' : '?'}admin_preview=1` : url); const { resolveGuest } = require('../middleware/guestAuth'); const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit'); +const { COLOR_LABELS } = require('../constants/colorLabels'); const secureImageService = require('../services/secureImageService'); const logger = require('../utils/logger'); const { pipeStreamToResponse } = require('../utils/streamResponse'); @@ -721,10 +722,11 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) }; let guestFeedbackByType = null; + let guestColorLabels = null; if (guest_id) { const guestFeedbackRows = await db('photo_feedback') .where({ event_id: req.event.id, guest_identifier: guest_id }) - .select('photo_id', 'feedback_type'); + .select('photo_id', 'feedback_type', 'color_label'); guestFeedbackByType = guestFeedbackRows.reduce((acc, row) => { if (!acc[row.feedback_type]) { @@ -733,6 +735,15 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) acc[row.feedback_type].add(row.photo_id); return acc; }, {}); + + // Colour filters are per-COLOUR, not just per-type (#1044) — "show + // me my greens" needs the value, which the type map above discards. + guestColorLabels = guestFeedbackRows.reduce((acc, row) => { + if (row.feedback_type !== 'color_label' || !row.color_label) return acc; + if (!acc[row.color_label]) acc[row.color_label] = new Set(); + acc[row.color_label].add(row.photo_id); + return acc; + }, {}); } const includeGuestMatches = (type) => { @@ -757,6 +768,23 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) includeBy(photo => (photo.average_rating || 0) > 0); } + // Colour-label filters (#1044), one token per colour: `color:green`. + // Same OR-of-guest-and-aggregate shape as the sibling tokens above: + // the guest's own labels of that colour, plus anyone's. (The public + // gallery narrows to "my greens" client-side from `my_color_label`, + // which is per-viewer by construction.) + const requestedColors = COLOR_LABELS.filter(color => filterTokens.has(`color:${color}`)); + if (requestedColors.length > 0) { + for (const color of requestedColors) { + guestColorLabels?.[color]?.forEach(id => include.add(id)); + } + const colorRows = await db('photo_feedback') + .where({ event_id: req.event.id, feedback_type: 'color_label', is_hidden: false }) + .whereIn('color_label', requestedColors) + .select('photo_id'); + colorRows.forEach(row => include.add(row.photo_id)); + } + if (filterTokens.has('commented')) { includeGuestMatches('comment'); const commentedRows = await db('photo_feedback') @@ -813,6 +841,26 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) likedRows.forEach(row => likedPhotoIds.add(row.photo_id)); } + // Per-viewer colour label (#1044), same identity resolution as the likes + // above. NOT gated on showFeedbackToGuests: a guest's own label is their + // own selection, not shared aggregate data, and hiding it would blank the + // grid badges on every refresh in a gallery with sharing switched off. + const myColorLabelByPhoto = {}; + if (photos.length > 0) { + const colorQuery = db('photo_feedback') + .where({ event_id: req.event.id, feedback_type: 'color_label' }) + .whereIn('photo_id', photos.map(p => p.id)); + if (req.guest?.id) { + colorQuery.where('guest_id', req.guest.id); + } else { + colorQuery.where('guest_identifier', generateGuestIdentifier(req)); + } + const colorRows = await colorQuery.select('photo_id', 'color_label'); + colorRows.forEach(row => { + if (row.color_label) myColorLabelByPhoto[row.photo_id] = row.color_label; + }); + } + // People in each photo (#1074). Two independent gates: the feature must // be on for this event AND, for a plain guest, the photographer must have // left the strip visible. A client (PIN access) is the photographer's own @@ -1087,6 +1135,12 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) // grid seed its lifted likedPhotoIds correctly on hard refresh. is_liked: showFeedbackToGuests ? likedPhotoIds.has(photo.id) : false, favorite_count: showFeedbackToGuests ? (photo.favorite_count || 0) : 0, + // Colour labels (#1044). The COUNT is aggregate data and follows + // show_feedback_to_guests like its siblings; the viewer's OWN label + // is not aggregate and must survive with sharing off, otherwise the + // grid badge disappears on refresh for the very guest who set it. + color_label_count: showFeedbackToGuests ? (photo.color_label_count || 0) : 0, + my_color_label: myColorLabelByPhoto[photo.id] || null, // People in this photo (#1074). Empty array when the feature is // off for this event or hidden from guests, so the frontend has // one shape to handle. Riding along on this payload is what keeps diff --git a/backend/src/routes/galleryFeedback.js b/backend/src/routes/galleryFeedback.js index c249a159..6e5105ba 100644 --- a/backend/src/routes/galleryFeedback.js +++ b/backend/src/routes/galleryFeedback.js @@ -33,6 +33,9 @@ router.get('/:slug/feedback-settings', allow_comments: Boolean(settings.allow_comments), allow_favorites: Boolean(settings.allow_favorites), allow_reactions: Boolean(settings.allow_reactions), + allow_color_labels: Boolean(settings.allow_color_labels), + // Which lightbox shortcut scheme this gallery uses (#1044). + keybind_mode: settings.keybind_mode || 'colors', require_name_email: Boolean(settings.require_name_email), show_feedback_to_guests: Boolean(settings.show_feedback_to_guests), identity_mode: settings.identity_mode || 'simple', @@ -125,6 +128,7 @@ router.get('/:slug/photos/:photoId/feedback', // Gated like the per-emoji map below — aggregate reaction data is // a new surface, kept fully hidden while sharing is off. reaction_count: settings.show_feedback_to_guests ? (photo.reaction_count || 0) : 0, + color_label_count: settings.show_feedback_to_guests ? (photo.color_label_count || 0) : 0, comment_count: await db('photo_feedback') .where({ photo_id: photoId, @@ -142,11 +146,17 @@ router.get('/:slug/photos/:photoId/feedback', reactions: settings.show_feedback_to_guests ? await feedbackService.getPhotoReactionCounts(photoId) : {}, + // Per-colour tallies (#1044), gated exactly like `reactions` above: + // with sharing off the guest sees only their own label. + color_labels: settings.show_feedback_to_guests + ? await feedbackService.getPhotoColorLabelCounts(photoId) + : {}, my_feedback: { rating: guestFeedback.find(f => f.feedback_type === 'rating')?.rating, liked: !!guestFeedback.find(f => f.feedback_type === 'like'), favorited: !!guestFeedback.find(f => f.feedback_type === 'favorite'), - reaction: guestFeedback.find(f => f.feedback_type === 'reaction')?.reaction || null + reaction: guestFeedback.find(f => f.feedback_type === 'reaction')?.reaction || null, + color_label: guestFeedback.find(f => f.feedback_type === 'color_label')?.color_label || null } }); } catch (error) { @@ -199,7 +209,8 @@ router.post('/:slug/photos/:photoId/feedback', like: settings.allow_likes, comment: settings.allow_comments, favorite: settings.allow_favorites, - reaction: settings.allow_reactions + reaction: settings.allow_reactions, + color_label: settings.allow_color_labels }; if (!typeAllowed[feedbackType]) { @@ -246,6 +257,7 @@ router.post('/:slug/photos/:photoId/feedback', rating: req.body.rating, comment_text: req.body.comment_text, reaction: req.body.reaction, + color_label: req.body.color_label, guest_name: req.guest?.name ?? req.body.guest_name, guest_email: req.guest?.email ?? req.body.guest_email, guest_id: req.guest?.id ?? null, @@ -372,7 +384,9 @@ router.get('/:slug/feedback-summary', allow_likes: settings.allow_likes, allow_comments: settings.allow_comments, allow_favorites: settings.allow_favorites, - allow_reactions: settings.allow_reactions + allow_reactions: settings.allow_reactions, + allow_color_labels: settings.allow_color_labels, + keybind_mode: settings.keybind_mode || 'colors' }, summary: guestSummary }); diff --git a/backend/src/routes/publicSettings.js b/backend/src/routes/publicSettings.js index db9b15eb..2d591c0a 100644 --- a/backend/src/routes/publicSettings.js +++ b/backend/src/routes/publicSettings.js @@ -18,6 +18,16 @@ router.get('/', async (req, res) => { 'seo_meta_noindex', 'seo_meta_nofollow', 'seo_meta_noai', 'event_default_require_password', 'event_default_feedback_enabled', + // Per-type feedback defaults (#1044) — the create form seeds + // its feedback panel from these, the same way it seeds the + // master toggle above. + 'event_default_allow_ratings', + 'event_default_allow_likes', + 'event_default_allow_favorites', + 'event_default_allow_comments', + 'event_default_allow_reactions', + 'event_default_allow_color_labels', + 'event_default_keybind_mode', 'gallery_show_filter_bar', 'event_phone_field_enabled', // #613 — guest upload UI needs to know the per-batch file @@ -206,6 +216,16 @@ router.get('/', async (req, res) => { // Defaults to false (matches the prior hard-coded form default), so // existing installs see no behaviour change until an admin flips it. event_default_feedback_enabled: settingsObject.event_default_feedback_enabled === true, + // Per-type feedback defaults (#1044). The fallbacks mirror + // FEEDBACK_TOGGLES in services/feedbackDefaults.js — the backend is + // still the authority; these only pre-fill the create form. + event_default_allow_ratings: settingsObject.event_default_allow_ratings !== false, + event_default_allow_likes: settingsObject.event_default_allow_likes !== false, + event_default_allow_favorites: settingsObject.event_default_allow_favorites !== false, + event_default_allow_comments: settingsObject.event_default_allow_comments !== false, + event_default_allow_reactions: settingsObject.event_default_allow_reactions !== false, + event_default_allow_color_labels: settingsObject.event_default_allow_color_labels === true, + event_default_keybind_mode: settingsObject.event_default_keybind_mode === 'lightroom' ? 'lightroom' : 'colors', // Phone-number field on events is opt-in (#322). event_phone_field_enabled: settingsObject.event_phone_field_enabled === true, // Whether to show the search/sort filter bar in public galleries (default: true) diff --git a/backend/src/routes/v1/__tests__/events.create.test.js b/backend/src/routes/v1/__tests__/events.create.test.js index 406e6046..1e973a86 100644 --- a/backend/src/routes/v1/__tests__/events.create.test.js +++ b/backend/src/routes/v1/__tests__/events.create.test.js @@ -172,20 +172,25 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { // 2. branding probe (whereIn → select) // 3. slug probe // 4. events insert - // 5. event_feedback_settings insert + // 5. feedback sub-toggle defaults probe (whereIn → select, #1044) + // 6. event_feedback_settings insert const devtoolsChain = buildChain({ firstResult: null }); const brandingChain = buildChain({ selectResult: [] }); const slugChain = buildChain({ firstResult: null }); const insertChain = buildChain({ returningResult: [{ id: 50 }] }); + const feedbackDefaultsChain = buildChain({ selectResult: [] }); const feedbackInsertChain = buildChain(); - db.__setImplementations(devtoolsChain, brandingChain, slugChain, insertChain, feedbackInsertChain); + db.__setImplementations( + devtoolsChain, brandingChain, slugChain, insertChain, + feedbackDefaultsChain, feedbackInsertChain, + ); await request(buildApp()) .post('/events') .send({ ...BASE_BODY, feedback_enabled: true }) .expect(201); - expect(db).toHaveBeenNthCalledWith(5, 'event_feedback_settings'); + expect(db).toHaveBeenNthCalledWith(6, 'event_feedback_settings'); const feedbackRow = feedbackInsertChain.insert.mock.calls[0][0]; expect(feedbackRow).toMatchObject({ event_id: 50 }); @@ -197,6 +202,12 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { expect(Boolean(feedbackRow.allow_likes)).toBe(true); expect(Boolean(feedbackRow.allow_comments)).toBe(true); expect(Boolean(feedbackRow.allow_favorites)).toBe(true); + // #1044: this insert used to omit allow_reactions entirely, so v1-created + // events only got reactions by accident of the column default. + expect(Boolean(feedbackRow.allow_reactions)).toBe(true); + // Colour labels are opt-in, so they stay off until the global is flipped. + expect(Boolean(feedbackRow.allow_color_labels)).toBe(false); + expect(feedbackRow.keybind_mode).toBe('colors'); expect(Boolean(feedbackRow.require_name_email)).toBe(false); expect(Boolean(feedbackRow.moderate_comments)).toBe(true); expect(Boolean(feedbackRow.show_feedback_to_guests)).toBe(true); @@ -205,7 +216,8 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { it('honours the event_default_feedback_enabled global when body omits feedback_enabled', async () => { // Feedback probe returns serialized "true" → fallback kicks in and // the feedback insert runs. Sequence: feedback probe, devtools probe, - // branding probe, slug, insert, feedback insert (6 calls total). + // branding probe, slug, insert, sub-toggle defaults probe (#1044), + // feedback insert (7 calls total). const feedbackProbe = buildChain({ firstResult: { setting_key: 'event_default_feedback_enabled', setting_value: 'true' }, }); @@ -213,9 +225,11 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { const brandingChain = buildChain({ selectResult: [] }); const slugChain = buildChain({ firstResult: null }); const insertChain = buildChain({ returningResult: [{ id: 51 }] }); + const feedbackDefaultsChain = buildChain({ selectResult: [] }); const feedbackInsertChain = buildChain(); db.__setImplementations( - feedbackProbe, devtoolsChain, brandingChain, slugChain, insertChain, feedbackInsertChain + feedbackProbe, devtoolsChain, brandingChain, slugChain, insertChain, + feedbackDefaultsChain, feedbackInsertChain, ); await request(buildApp()) @@ -223,7 +237,7 @@ describe('v1 POST /events — issue #550 (color_theme + feedback row)', () => { .send(BASE_BODY) .expect(201); - expect(db).toHaveBeenNthCalledWith(6, 'event_feedback_settings'); + expect(db).toHaveBeenNthCalledWith(7, 'event_feedback_settings'); expect(feedbackInsertChain.insert).toHaveBeenCalledTimes(1); }); diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index 9284b3ca..bbc23b36 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -29,6 +29,7 @@ const { requireEventOwnership, scopeEventsQuery } = require('../../middleware/ow // requirePermission gates supply the missing half; they key on req.admin.id, // which apiTokenAuth populates. const { requirePermission } = require('../../middleware/permissions'); +const { resolveEventFeedbackDefaults } = require('../../services/feedbackDefaults'); const { buildShareLinkVariants } = require('../../services/shareLinkService'); const { generateThumbnail } = require('../../services/imageProcessor'); const logger = require('../../utils/logger'); @@ -303,17 +304,22 @@ router.post( const id = insertResult[0]?.id || insertResult[0]; // Issue #550 — mirror adminEvents.js: create event_feedback_settings - // row when feedback is enabled, so the gallery actually shows - // feedback UI. Sub-flags default to the same values the admin form - // ships with (everything on except require_name_email). + // row when feedback is enabled, so the gallery actually shows feedback + // UI. The sub-flags come from the shared global defaults (#1044) rather + // than a hard-coded list, which is how this path silently shipped + // without allow_reactions for two releases. if (feedback_enabled) { + const feedbackDefaults = await resolveEventFeedbackDefaults(); await db('event_feedback_settings').insert({ event_id: id, feedback_enabled: formatBoolean(true), - allow_ratings: formatBoolean(true), - allow_likes: formatBoolean(true), - allow_comments: formatBoolean(true), - allow_favorites: formatBoolean(true), + allow_ratings: formatBoolean(feedbackDefaults.allow_ratings), + allow_likes: formatBoolean(feedbackDefaults.allow_likes), + allow_comments: formatBoolean(feedbackDefaults.allow_comments), + allow_favorites: formatBoolean(feedbackDefaults.allow_favorites), + allow_reactions: formatBoolean(feedbackDefaults.allow_reactions), + allow_color_labels: formatBoolean(feedbackDefaults.allow_color_labels), + keybind_mode: feedbackDefaults.keybind_mode, require_name_email: formatBoolean(false), moderate_comments: formatBoolean(true), show_feedback_to_guests: formatBoolean(true), diff --git a/backend/src/services/feedbackDefaults.js b/backend/src/services/feedbackDefaults.js new file mode 100644 index 00000000..058973f8 --- /dev/null +++ b/backend/src/services/feedbackDefaults.js @@ -0,0 +1,170 @@ +/** + * Global defaults for the per-event guest-feedback toggles (#1044). + * + * Before this module the creation defaults lived in four places that had + * already drifted apart: the admin create route's destructuring defaults, the + * v1 create route's hard-coded insert (which forgot `allow_reactions` + * entirely), `feedbackService.getEventFeedbackSettings()`'s no-row fallback + * and the migration column defaults. The same install answered "are comments + * on by default?" differently depending on which code path created the event. + * + * Everything now resolves through FEEDBACK_TOGGLES: + * + * app_settings row -> per-event column default -> the built-in fallback + * + * The `event_default_*` keys extend the family `event_default_require_password` + * (#317) and `event_default_feedback_enabled` (#520) already established: they + * are the value a NEW event starts from, not a retroactive master switch. + * Flipping one never changes a gallery that already exists — a switch that + * silently strips the rating stars off a gallery a client is mid-way through + * proofing would be a different, much riskier feature. + * + * Adding a seventh feedback type means adding one row to FEEDBACK_TOGGLES + * (plus its column in the migration and its UI) — no sweep across the create + * routes. + */ + +const { db } = require('../database/db'); +const logger = require('../utils/logger'); + +/** + * Every guest-feedback toggle, in the order the admin UI shows them. + * + * `fallback` is what applies when the admin has never touched the global + * setting. These match the values the admin create route has shipped with, so + * installs that never open Settings > Events keep creating events exactly as + * they did before — except `allow_color_labels`, which is new in #1044 and + * opt-in so no existing gallery grows a colour bar on upgrade. + */ +const FEEDBACK_TOGGLES = [ + { column: 'allow_ratings', settingKey: 'event_default_allow_ratings', fallback: true }, + { column: 'allow_likes', settingKey: 'event_default_allow_likes', fallback: true }, + { column: 'allow_favorites', settingKey: 'event_default_allow_favorites', fallback: true }, + { column: 'allow_comments', settingKey: 'event_default_allow_comments', fallback: true }, + { column: 'allow_reactions', settingKey: 'event_default_allow_reactions', fallback: true }, + { column: 'allow_color_labels', settingKey: 'event_default_allow_color_labels', fallback: false }, +]; + +/** Non-boolean feedback defaults that follow the same global -> event flow. */ +const KEYBIND_MODES = ['colors', 'lightroom']; +const DEFAULT_KEYBIND_MODE = 'colors'; +const KEYBIND_MODE_SETTING_KEY = 'event_default_keybind_mode'; + +/** Every app_settings key this module owns — for the settings API surface. */ +const FEEDBACK_DEFAULT_SETTING_KEYS = [ + ...FEEDBACK_TOGGLES.map((t) => t.settingKey), + KEYBIND_MODE_SETTING_KEY, +]; + +/** + * app_settings stores JSON-encoded values, but rows written by older code + * paths (and by SQLite's looser typing) can arrive as raw strings. Accept + * both, and return undefined for anything that isn't recognisably a boolean + * so the caller falls back rather than persisting `null` into a NOT NULL-ish + * boolean column. + */ +function parseBooleanSetting(rawValue) { + let value = rawValue; + if (typeof value === 'string') { + try { + value = JSON.parse(value); + } catch { + const normalized = value.trim().toLowerCase(); + if (normalized === 'true') return true; + if (normalized === 'false') return false; + return undefined; + } + } + if (typeof value === 'boolean') return value; + // SQLite hands back 0/1 for booleans written through formatBoolean. + if (value === 1 || value === 0) return value === 1; + return undefined; +} + +function parseKeybindModeSetting(rawValue) { + let value = rawValue; + if (typeof value === 'string') { + try { + const parsed = JSON.parse(value); + if (typeof parsed === 'string') value = parsed; + } catch { + /* keep raw */ + } + } + return KEYBIND_MODES.includes(value) ? value : undefined; +} + +/** + * Resolve the global feedback defaults in ONE query (mirrors + * getBrandingDefaults' batched whereIn rather than firing seven `.first()` + * calls on every event creation). + * + * Never throws: a settings-table hiccup must not fail event creation, so the + * built-in fallbacks stand in. + * + * @returns {Promise<{allow_ratings: boolean, allow_likes: boolean, + * allow_favorites: boolean, allow_comments: boolean, allow_reactions: boolean, + * allow_color_labels: boolean, keybind_mode: string}>} + */ +async function resolveEventFeedbackDefaults() { + const defaults = {}; + for (const toggle of FEEDBACK_TOGGLES) { + defaults[toggle.column] = toggle.fallback; + } + defaults.keybind_mode = DEFAULT_KEYBIND_MODE; + + try { + const rows = await db('app_settings') + .whereIn('setting_key', FEEDBACK_DEFAULT_SETTING_KEYS) + .select('setting_key', 'setting_value'); + + const bySettingKey = new Map(rows.map((row) => [row.setting_key, row.setting_value])); + + for (const toggle of FEEDBACK_TOGGLES) { + if (!bySettingKey.has(toggle.settingKey)) continue; + const parsed = parseBooleanSetting(bySettingKey.get(toggle.settingKey)); + if (parsed !== undefined) defaults[toggle.column] = parsed; + } + + if (bySettingKey.has(KEYBIND_MODE_SETTING_KEY)) { + const parsed = parseKeybindModeSetting(bySettingKey.get(KEYBIND_MODE_SETTING_KEY)); + if (parsed !== undefined) defaults.keybind_mode = parsed; + } + } catch (error) { + logger.error('Failed to read global feedback defaults, using built-ins', { + error: error.message, + }); + } + + return defaults; +} + +/** + * Merge an explicit request body over the resolved globals. A value the caller + * actually sent always wins; `undefined` (the field was omitted) inherits. + * + * @param {Object} body - the create-request payload + * @param {Object} globals - output of resolveEventFeedbackDefaults() + */ +function applyFeedbackDefaults(body = {}, globals) { + const resolved = { ...globals }; + for (const toggle of FEEDBACK_TOGGLES) { + if (body[toggle.column] !== undefined) { + resolved[toggle.column] = body[toggle.column]; + } + } + if (KEYBIND_MODES.includes(body.keybind_mode)) { + resolved.keybind_mode = body.keybind_mode; + } + return resolved; +} + +module.exports = { + FEEDBACK_TOGGLES, + FEEDBACK_DEFAULT_SETTING_KEYS, + KEYBIND_MODES, + KEYBIND_MODE_SETTING_KEY, + DEFAULT_KEYBIND_MODE, + resolveEventFeedbackDefaults, + applyFeedbackDefaults, +}; diff --git a/backend/src/services/feedbackService.js b/backend/src/services/feedbackService.js index d1cb7d9e..565c4374 100644 --- a/backend/src/services/feedbackService.js +++ b/backend/src/services/feedbackService.js @@ -2,6 +2,8 @@ const { db, logActivity } = require('../database/db'); const logger = require('../utils/logger'); const { formatBoolean } = require('../utils/dbCompat'); const { REACTION_EMOJIS } = require('../constants/reactions'); +const { isValidColorLabel } = require('../constants/colorLabels'); +const { resolveEventFeedbackDefaults, DEFAULT_KEYBIND_MODE, KEYBIND_MODES } = require('./feedbackDefaults'); // Every writable column on event_feedback_settings (#1030). The admin form // posts its whole client-side state back, including UI-only keys that were @@ -17,6 +19,8 @@ const FEEDBACK_SETTINGS_COLUMNS = [ 'allow_comments', 'allow_favorites', 'allow_reactions', + 'allow_color_labels', + 'keybind_mode', 'require_name_email', 'moderate_comments', 'require_moderation', @@ -26,6 +30,16 @@ const FEEDBACK_SETTINGS_COLUMNS = [ 'max_likes_per_guest' ]; +/** + * Feedback types that store exactly ONE value per guest per photo, and the + * column each keeps it in. Both share the toggle-off / switch semantics in + * submitFeedback below. + */ +const SINGLE_VALUE_COLUMNS = { + reaction: 'reaction', + color_label: 'color_label', +}; + function pickSettingsColumns(settings) { const picked = {}; for (const column of FEEDBACK_SETTINGS_COLUMNS) { @@ -47,15 +61,15 @@ class FeedbackService { .first(); if (!settings) { - // Return default settings if none exist + // No row = feedback was never enabled for this event. The sub-toggles + // still matter: they are the state the admin's feedback panel opens + // with. Read them from the same global defaults the create routes use + // (#1044) instead of a fourth hard-coded copy that drifts from them. + const globals = await resolveEventFeedbackDefaults(); return { event_id: eventId, feedback_enabled: false, - allow_ratings: true, - allow_likes: true, - allow_comments: false, - allow_favorites: true, - allow_reactions: true, + ...globals, require_name_email: false, moderate_comments: true, show_feedback_to_guests: true, @@ -69,6 +83,12 @@ class FeedbackService { if (!settings.identity_mode) { settings.identity_mode = 'simple'; } + // Rows created before migration 180 have NULL keybind_mode. An + // unrecognised value gets the same treatment — the lightbox switches on + // this string and must never receive something it has no scheme for. + if (!KEYBIND_MODES.includes(settings.keybind_mode)) { + settings.keybind_mode = DEFAULT_KEYBIND_MODE; + } // Per-guest caps (#655). NULL on existing rows = unlimited; the route // layer treats null/0/missing identically. settings.max_favorites_per_guest = settings.max_favorites_per_guest ?? null; @@ -139,10 +159,10 @@ class FeedbackService { async submitFeedback(photoId, eventId, feedbackData, guestIdentifier) { try { - const { feedback_type, rating, comment_text, reaction, guest_name, guest_email, ip_address, user_agent, guest_id } = feedbackData; + const { feedback_type, rating, comment_text, reaction, color_label, guest_name, guest_email, ip_address, user_agent, guest_id } = feedbackData; // Validate feedback type - if (!['rating', 'like', 'comment', 'favorite', 'reaction'].includes(feedback_type)) { + if (!['rating', 'like', 'comment', 'favorite', 'reaction', 'color_label'].includes(feedback_type)) { throw new Error('Invalid feedback type'); } @@ -152,6 +172,11 @@ class FeedbackService { throw new Error('Invalid reaction'); } + // Same contract for colour labels (#1044). + if (feedback_type === 'color_label' && !isValidColorLabel(color_label)) { + throw new Error('Invalid color label'); + } + // Rating 0 clears the guest's rating (#884). Only the explicit zero // sentinel (0, or "0" from callers that skip the route validator's // toInt) triggers the destructive path — malformed input (undefined, @@ -209,35 +234,41 @@ class FeedbackService { return { id: existing.id, updated: true }; } - // One reaction per guest per photo, changeable (#839): the same - // emoji again toggles it off; a different one switches the row. + // Single-value types — one reaction (#839) and one colour label + // (#1044) per guest per photo, both changeable: submitting the + // current value again toggles it off, a different one switches the + // row. Shared so the two paths can't drift. + // // Toggle/switch act on the full guest-scoped QUERY, not existing.id: // like the sibling like path, the check-then-insert above can race // into duplicate rows — operating on the set makes the next // interaction collapse them instead of leaving a phantom count. - const reactionScope = () => { - const q = db('photo_feedback').where({ - photo_id: photoId, - event_id: eventId, - feedback_type: 'reaction', - }); - if (guest_id) q.where('guest_id', guest_id); - else q.where('guest_identifier', guestIdentifier); - return q; - }; - if (feedback_type === 'reaction') { - if (existing.reaction === reaction) { - await reactionScope().delete(); + const singleValueColumn = SINGLE_VALUE_COLUMNS[feedback_type]; + if (singleValueColumn) { + const submittedValue = feedback_type === 'reaction' ? reaction : color_label; + const singleValueScope = () => { + const q = db('photo_feedback').where({ + photo_id: photoId, + event_id: eventId, + feedback_type, + }); + if (guest_id) q.where('guest_id', guest_id); + else q.where('guest_identifier', guestIdentifier); + return q; + }; + + if (existing[singleValueColumn] === submittedValue) { + await singleValueScope().delete(); await this.updatePhotoFeedbackStats(photoId); return { removed: true }; } // Converge to exactly one row: drop any racy duplicates, then - // switch the surviving row's emoji. - await reactionScope().whereNot('id', existing.id).delete(); + // switch the surviving row's value. + await singleValueScope().whereNot('id', existing.id).delete(); await db('photo_feedback') .where('id', existing.id) .update({ - reaction, + [singleValueColumn]: submittedValue, updated_at: new Date() }); await this.updatePhotoFeedbackStats(photoId); @@ -300,6 +331,7 @@ class FeedbackService { rating: feedback_type === 'rating' ? rating : null, comment_text: feedback_type === 'comment' ? comment_text : null, reaction: feedback_type === 'reaction' ? reaction : null, + color_label: feedback_type === 'color_label' ? color_label : null, guest_name, guest_email, guest_identifier: guestIdentifier, @@ -352,7 +384,7 @@ class FeedbackService { const feedback = await query .orderBy('created_at', 'desc') - .select('id', 'feedback_type', 'rating', 'comment_text', 'reaction', 'guest_name', 'created_at', 'is_approved', 'is_hidden'); + .select('id', 'feedback_type', 'rating', 'comment_text', 'reaction', 'color_label', 'guest_name', 'created_at', 'is_approved', 'is_hidden'); return feedback; } catch (error) { @@ -368,7 +400,7 @@ class FeedbackService { try { const photos = await db('photos') .where('event_id', eventId) - .select('id', 'filename', 'feedback_count', 'like_count', 'average_rating', 'favorite_count', 'reaction_count') + .select('id', 'filename', 'feedback_count', 'like_count', 'average_rating', 'favorite_count', 'reaction_count', 'color_label_count') .orderBy('average_rating', 'desc') .orderBy('like_count', 'desc'); @@ -380,7 +412,8 @@ class FeedbackService { db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_likes', ['like']), db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_comments', ['comment']), db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_favorites', ['favorite']), - db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_reactions', ['reaction']) + db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_reactions', ['reaction']), + db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_color_labels', ['color_label']) ) .first(); @@ -419,6 +452,67 @@ class FeedbackService { } } + /** + * Per-colour tallies for one photo (#1044) — the colour-label sibling of + * getPhotoReactionCounts. + */ + async getPhotoColorLabelCounts(photoId) { + try { + const rows = await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'color_label' }) + .where('is_hidden', false) + .groupBy('color_label') + .select('color_label') + .count('id as count'); + + const counts = {}; + for (const row of rows) { + if (row.color_label) counts[row.color_label] = Number(row.count) || 0; + } + return counts; + } catch (error) { + logger.error('Error getting photo color label counts:', error); + throw error; + } + } + + /** + * Per-colour tallies for every labelled photo in an event, keyed by photo + * id (#1044). One grouped query — the admin grid needs this for a whole + * page of photos at once, and the per-photo helper above would be N+1. + * + * @param {number} eventId + * @param {number[]} [photoIds] - optional narrowing to the visible page + * @returns {Promise} { [photoId]: { green: 2, red: 1 } } + */ + async getEventColorLabelCounts(eventId, photoIds = null) { + try { + const query = db('photo_feedback') + .where({ event_id: eventId, feedback_type: 'color_label' }) + .where('is_hidden', false) + .groupBy('photo_id', 'color_label') + .select('photo_id', 'color_label') + .count('id as count'); + + if (Array.isArray(photoIds)) { + if (photoIds.length === 0) return {}; + query.whereIn('photo_id', photoIds); + } + + const rows = await query; + const byPhoto = {}; + for (const row of rows) { + if (!row.color_label) continue; + if (!byPhoto[row.photo_id]) byPhoto[row.photo_id] = {}; + byPhoto[row.photo_id][row.color_label] = Number(row.count) || 0; + } + return byPhoto; + } catch (error) { + logger.error('Error getting event color label counts:', error); + return {}; + } + } + /** * Update photo feedback statistics */ @@ -433,6 +527,7 @@ class FeedbackService { db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as like_count', ['like']), db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as favorite_count', ['favorite']), db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as reaction_count', ['reaction']), + db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as color_label_count', ['color_label']), db.raw('AVG(CASE WHEN feedback_type = ? THEN rating END) as average_rating', ['rating']), db.raw('COUNT(DISTINCT COALESCE(CAST(guest_id AS VARCHAR), guest_identifier)) as feedback_count') ) @@ -446,7 +541,8 @@ class FeedbackService { like_count: stats.like_count || 0, average_rating: stats.average_rating || 0, favorite_count: stats.favorite_count || 0, - reaction_count: stats.reaction_count || 0 + reaction_count: stats.reaction_count || 0, + color_label_count: stats.color_label_count || 0 }); } catch (error) { logger.error('Error updating photo feedback stats:', error); @@ -587,6 +683,7 @@ class FeedbackService { 'photo_feedback.rating', 'photo_feedback.comment_text', 'photo_feedback.reaction', + 'photo_feedback.color_label', 'photo_feedback.guest_name', 'photo_feedback.guest_email', 'photo_feedback.created_at' @@ -625,6 +722,7 @@ class FeedbackService { 'photo_feedback.rating', 'photo_feedback.comment_text', 'photo_feedback.reaction', + 'photo_feedback.color_label', 'photo_feedback.guest_name', 'photo_feedback.guest_email', 'photo_feedback.guest_identifier', @@ -651,6 +749,7 @@ class FeedbackService { star_rating: '', comment: '', reaction: '', + color_label: '', latest_at: row.created_at, }; byKey.set(key, entry); @@ -680,6 +779,9 @@ class FeedbackService { case 'reaction': if (row.reaction) entry.reaction = row.reaction; break; + case 'color_label': + if (row.color_label) entry.color_label = row.color_label; + break; default: // Unknown feedback type — ignore so a future type doesn't break the export. break; diff --git a/backend/src/services/photoAdminMarksService.js b/backend/src/services/photoAdminMarksService.js new file mode 100644 index 00000000..2512945d --- /dev/null +++ b/backend/src/services/photoAdminMarksService.js @@ -0,0 +1,185 @@ +/** + * The photographer's own stars and colour labels (#1044 follow-up). + * + * Deliberately separate from photo_feedback — see the migration-181 header for + * why. Nothing in here is ever read by a guest-facing route: admin marks show + * on admin surfaces and in exports, and the client's proofing view never sees + * what the photographer thought. + */ + +const { db } = require('../database/db'); +const logger = require('../utils/logger'); +const { isValidColorLabel } = require('../constants/colorLabels'); + +/** + * A mark the caller got wrong, as opposed to something that went wrong. + * + * Carries a `code` so the route can map it to a 400 without matching on the + * message text — message-matching couples the status code to this file's copy, + * and a reworded string would silently turn a 400 into a 500. + */ +const INVALID_MARK = 'INVALID_MARK'; +function invalidMark(message) { + return Object.assign(new Error(message), { code: INVALID_MARK }); +} + +/** + * Set, change or clear an admin's mark on one photo. + * + * `rating` and `colorLabel` are tri-state: `undefined` leaves that half of the + * mark alone, `null` clears it, a value sets it. That is what lets the + * lightbox's colour keys and star keys write independently without each + * wiping the other. + * + * @param {number} eventId + * @param {number} photoId + * @param {number} adminId + * @param {{rating?: number|null, colorLabel?: string|null}} mark + * @returns {Promise<{rating: number|null, color_label: string|null}|null>} + * the resulting mark, or null when it was cleared entirely + */ +async function setMark(eventId, photoId, adminId, { rating, colorLabel } = {}) { + if (rating !== undefined && rating !== null) { + const asInt = Number(rating); + if (!Number.isInteger(asInt) || asInt < 1 || asInt > 5) { + throw invalidMark('Rating must be between 1 and 5'); + } + } + if (colorLabel !== undefined && colorLabel !== null && !isValidColorLabel(colorLabel)) { + throw invalidMark('Invalid color label'); + } + + /** + * Write ONLY the halves this call was asked about, onto the row as it + * stands right now. + * + * The obvious version — recompute both halves from a pre-read and write the + * pair — loses a concurrent write: press 4 then 9 on a photo that already + * has a mark and both calls read the old row, both write {rating, + * color_label}, and the second silently clobbers the first with its stale + * value. Not writing the untouched column at all means there is nothing to + * lose, and no race machinery is needed to achieve it. + */ + const applyToRow = async (row) => { + const now = new Date().toISOString(); + const patch = { updated_at: now }; + if (rating !== undefined) patch.rating = rating === null ? null : Number(rating); + if (colorLabel !== undefined) patch.color_label = colorLabel; + + await db('photo_admin_marks').where('id', row.id).update(patch); + + // A mark with neither half left is deleted, not kept as an empty row — an + // empty row would still count as "marked" to anything testing existence. + // + // The emptiness test is a predicate ON the delete rather than a re-read + // followed by a delete: a concurrent call that just filled a half back in + // must not have its value dropped by our stale view of the row. + const removed = await db('photo_admin_marks') + .where('id', row.id) + .whereNull('rating') + .whereNull('color_label') + .delete(); + if (removed) return null; + + const after = await db('photo_admin_marks').where('id', row.id).first(); + if (!after) return null; + return { rating: after.rating ?? null, color_label: after.color_label ?? null }; + }; + + const existing = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: adminId }) + .first(); + + if (existing) return applyToRow(existing); + + // No row yet, so there is nothing for a clear-only call to clear. + const fresh = { + rating: rating === undefined || rating === null ? null : Number(rating), + color_label: colorLabel === undefined || colorLabel === null ? null : colorLabel, + }; + if (fresh.rating === null && fresh.color_label === null) return null; + + const now = new Date().toISOString(); + try { + await db('photo_admin_marks').insert({ + photo_id: photoId, + event_id: eventId, + admin_id: adminId, + ...fresh, + created_at: now, + updated_at: now, + }); + } catch (error) { + // The read above and this insert are not atomic, and a triage pass fires + // them back to back — press 4 then 9 on an UNMARKED photo and both + // requests can find no row and both try to insert. The unique index stops + // the duplicate, which would otherwise surface as a 500 and a lost + // keystroke; converge onto the row the winner created, through the same + // write-only-what-you-addressed path as any other update. + const raced = await db('photo_admin_marks') + .where({ photo_id: photoId, admin_id: adminId }) + .first(); + if (!raced) throw error; + return applyToRow(raced); + } + + return fresh; +} + +/** + * One admin's marks across an event, keyed by photo id. Optionally narrowed to + * the ids on the current page. + * + * @returns {Promise} { [photoId]: { rating, color_label } } + */ +async function getEventMarks(eventId, adminId, photoIds = null) { + try { + const query = db('photo_admin_marks') + .where({ event_id: eventId, admin_id: adminId }) + .select('photo_id', 'rating', 'color_label'); + + if (Array.isArray(photoIds)) { + if (photoIds.length === 0) return {}; + query.whereIn('photo_id', photoIds); + } + + const rows = await query; + const byPhoto = {}; + for (const row of rows) { + byPhoto[row.photo_id] = { + rating: row.rating ?? null, + color_label: row.color_label ?? null, + }; + } + return byPhoto; + } catch (error) { + logger.error('Error reading admin photo marks:', error); + return {}; + } +} + +/** + * Per-colour photo counts for one admin's marks — drives the counts on the + * "My marks" filter chips. + */ +async function getEventMarkColorCounts(eventId, adminId) { + try { + const rows = await db('photo_admin_marks') + .where({ event_id: eventId, admin_id: adminId }) + .whereNotNull('color_label') + .groupBy('color_label') + .select('color_label') + .count('id as count'); + + const counts = {}; + for (const row of rows) { + counts[row.color_label] = parseInt(row.count, 10) || 0; + } + return counts; + } catch (error) { + logger.error('Error counting admin photo marks:', error); + return {}; + } +} + +module.exports = { setMark, getEventMarks, getEventMarkColorCounts, INVALID_MARK }; diff --git a/backend/src/services/photoExportService.js b/backend/src/services/photoExportService.js index 530b59c2..4d97d391 100644 --- a/backend/src/services/photoExportService.js +++ b/backend/src/services/photoExportService.js @@ -6,6 +6,9 @@ const archiver = require('archiver'); const { PassThrough } = require('stream'); const { XmpGenerator } = require('./xmpGenerator'); +const { dominantColorLabel } = require('../constants/colorLabels'); +const photoAdminMarksService = require('./photoAdminMarksService'); +const feedbackService = require('./feedbackService'); const { neutralizeSpreadsheetFormula } = require('../utils/spreadsheetSafe'); const { db } = require('../database/db'); const path = require('path'); @@ -22,7 +25,7 @@ class PhotoExportService { * @param {number[]} photoIds - Photo IDs to export (optional, exports all if not provided) * @returns {Promise} Photos with feedback */ - async getPhotosWithFeedback(eventId, photoIds = null) { + async getPhotosWithFeedback(eventId, photoIds = null, adminId = null) { let query = db('photos') .leftJoin('photo_categories', 'photos.category_id', 'photo_categories.id') .where('photos.event_id', eventId) @@ -36,6 +39,7 @@ class PhotoExportService { 'photos.like_count', 'photos.favorite_count', 'photos.comment_count', + 'photos.color_label_count', 'photos.width', 'photos.height', 'photos.size_bytes', @@ -48,7 +52,33 @@ class PhotoExportService { query = query.whereIn('photos.id', photoIds); } - return await query; + const photos = await query; + + // Colour labels (#1044). One grouped query for the whole export, then + // each row carries both the per-colour tallies and the single colour the + // XMP sidecar should claim. + const colorCounts = await feedbackService.getEventColorLabelCounts( + eventId, + photos.map(p => p.id), + ); + for (const photo of photos) { + photo.color_labels = colorCounts[photo.id] || {}; + photo.dominant_color_label = dominantColorLabel(photo.color_labels); + } + + // The exporting photographer's own marks (#1044 follow-up), so a triage + // pass can leave the app as XMP the same way a client selection can. + if (adminId) { + const marks = await photoAdminMarksService.getEventMarks( + eventId, adminId, photos.map(p => p.id), + ); + for (const photo of photos) { + photo.my_rating = marks[photo.id]?.rating ?? null; + photo.my_color_label = marks[photo.id]?.color_label ?? null; + } + } + + return photos; } /** @@ -60,7 +90,9 @@ class PhotoExportService { * @returns {Promise} Export result with stream/content */ async exportPhotos(eventId, photoIds, format, options = {}) { - const photos = await this.getPhotosWithFeedback(eventId, photoIds); + // admin_id is set by the route from the session, never taken from the + // request body — it decides whose marks the export carries. + const photos = await this.getPhotosWithFeedback(eventId, photoIds, options.admin_id || null); if (photos.length === 0) { throw new Error('No photos to export'); @@ -139,6 +171,9 @@ class PhotoExportService { 'likes', 'favorites', 'comments', + 'color_label', + 'my_rating', + 'my_color_label', 'category', 'width', 'height', @@ -154,6 +189,9 @@ class PhotoExportService { photo.like_count || 0, photo.favorite_count || 0, photo.comment_count || 0, + photo.dominant_color_label || '', + photo.my_rating ?? '', + photo.my_color_label || '', photo.category_name || '', photo.width || '', photo.height || '', @@ -181,7 +219,17 @@ class PhotoExportService { * Export as XMP sidecar files in a ZIP archive */ async exportAsXmpZip(photos, options = {}) { - const { filename_format = 'original' } = options; + const { filename_format = 'original', mark_source = 'client' } = options; + + // Whose verdict the sidecar carries (#1044 follow-up). Default 'client' + // keeps existing exports identical; 'mine' writes the photographer's own + // triage instead, which is the point of being able to mark at all. + const project = (photo) => (mark_source !== 'mine' ? photo : { + ...photo, + average_rating: photo.my_rating || 0, + dominant_color_label: photo.my_color_label || null, + color_labels: {}, + }); const archive = archiver('zip', { zlib: { level: 9 } }); const passthrough = new PassThrough(); @@ -192,7 +240,7 @@ class PhotoExportService { ? (photo.original_filename || photo.filename) : photo.filename; const xmpFilename = this.xmpGenerator.getXmpFilename(baseFilename); - const xmpContent = this.xmpGenerator.generateXmp(photo, options); + const xmpContent = this.xmpGenerator.generateXmp(project(photo), options); archive.append(xmpContent, { name: xmpFilename }); } @@ -237,6 +285,10 @@ class PhotoExportService { likes: photo.like_count || 0, favorites: photo.favorite_count || 0, comments: photo.comment_count || 0, + color_label: photo.dominant_color_label || null, + color_labels: photo.color_labels || {}, + my_rating: photo.my_rating ?? null, + my_color_label: photo.my_color_label || null, dimensions: { width: photo.width || null, height: photo.height || null diff --git a/backend/src/services/xmpGenerator.js b/backend/src/services/xmpGenerator.js index 5a70989e..f0ce7d95 100644 --- a/backend/src/services/xmpGenerator.js +++ b/backend/src/services/xmpGenerator.js @@ -3,6 +3,8 @@ * Generates Adobe XMP metadata files for photos with guest feedback */ +const { COLOR_LABEL_TO_XMP, dominantColorLabel } = require('../constants/colorLabels'); + class XmpGenerator { /** * Generate XMP sidecar content for a photo @@ -19,7 +21,7 @@ class XmpGenerator { } = options; const rating = include_rating ? this.mapRating(photo.average_rating) : 0; - const label = include_label ? this.mapLabel(photo.average_rating) : null; + const label = include_label ? this.mapLabel(photo) : null; const descriptionXml = include_description ? this.generateDescription(photo) : ''; const keywordsXml = include_keywords ? this.generateKeywords(photo) : ''; @@ -57,11 +59,39 @@ class XmpGenerator { } /** - * Map PicPeak rating to XMP color label + * Resolve the photo's XMP colour label. + * + * A real colour label the client set while proofing (#1044) always wins: + * that is the whole point of using Lightroom's colour set, and it is an + * explicit choice rather than something inferred. Only when a photo has no + * label does this fall back to the historical rating-derived mapping, so + * exports for events that never enabled colour labels are unchanged. + * + * @param {Object} photo - photo row, may carry dominant_color_label + * @returns {string|null} XMP label colour + */ + mapLabel(photo) { + // Tolerate being handed a bare rating: this used to take one, and + // callers outside the export service may still do so. + if (typeof photo === 'number' || photo === null || photo === undefined) { + return this.mapRatingToLabel(photo); + } + + const colorLabel = photo.dominant_color_label + || dominantColorLabel(photo.color_labels); + if (colorLabel && COLOR_LABEL_TO_XMP[colorLabel]) { + return COLOR_LABEL_TO_XMP[colorLabel]; + } + + return this.mapRatingToLabel(photo.average_rating); + } + + /** + * The pre-#1044 mapping: infer a colour from the average star rating. * @param {number} avgRating - Average rating * @returns {string|null} XMP label color */ - mapLabel(avgRating) { + mapRatingToLabel(avgRating) { if (!avgRating || avgRating === 0) return null; if (avgRating >= 4.5) return 'Red'; // Top picks if (avgRating >= 3.5) return 'Yellow'; // Good @@ -80,7 +110,9 @@ class XmpGenerator { const likes = photo.like_count || 0; const favorites = photo.favorite_count || 0; - const desc = `PicPeak Guest Feedback: ${rating} stars, ${likes} likes, ${favorites} favorites`; + const colorLabel = photo.dominant_color_label || dominantColorLabel(photo.color_labels); + const colorPart = colorLabel ? `, ${colorLabel} label` : ''; + const desc = `PicPeak Guest Feedback: ${rating} stars, ${likes} likes, ${favorites} favorites${colorPart}`; return ` @@ -113,6 +145,14 @@ class XmpGenerator { keywords.push('favorited'); } + // A searchable keyword for the client's colour choice (#1044) — Lightroom + // can filter on xmp:Label directly, Bridge and Capture One users often + // find keywords easier. + const colorLabel = photo.dominant_color_label || dominantColorLabel(photo.color_labels); + if (colorLabel) { + keywords.push(`color-${colorLabel}`); + } + if (photo.category_name) { keywords.push(this.sanitizeKeyword(photo.category_name)); } diff --git a/backend/src/utils/feedbackValidation.js b/backend/src/utils/feedbackValidation.js index bfcd6163..65c828c3 100644 --- a/backend/src/utils/feedbackValidation.js +++ b/backend/src/utils/feedbackValidation.js @@ -2,6 +2,8 @@ const { body, param, validationResult } = require('express-validator'); const validator = require('validator'); const { IDENTITY_PRESERVING_NORMALIZE_EMAIL } = require('./emailNormalization'); const { REACTION_EMOJIS } = require('../constants/reactions'); +const { COLOR_LABELS } = require('../constants/colorLabels'); +const { KEYBIND_MODES } = require('../services/feedbackDefaults'); /** * Validation rules for feedback submission @@ -134,7 +136,7 @@ function getValidationRules(feedbackType) { */ const validateFeedbackSubmission = [ body('feedback_type') - .isIn(['rating', 'like', 'comment', 'favorite', 'reaction']) + .isIn(['rating', 'like', 'comment', 'favorite', 'reaction', 'color_label']) .withMessage('Invalid feedback type'), // Conditional validation based on feedback type. 0 clears the guest's @@ -151,6 +153,14 @@ const validateFeedbackSubmission = [ .if(body('feedback_type').equals('reaction')) .custom((value) => REACTION_EMOJIS.includes(value)) .withMessage('Invalid reaction'), + + // Colour labels (#1044): Lightroom's five colours only — the value ends up + // in an XMP field Lightroom parses, so free-form strings are rejected here + // rather than sanitised later. + body('color_label') + .if(body('feedback_type').equals('color_label')) + .custom((value) => COLOR_LABELS.includes(value)) + .withMessage('Invalid color label'), body('comment_text') .if(body('feedback_type').equals('comment')) @@ -194,6 +204,9 @@ const validateFeedbackSettings = [ body('allow_comments').optional().isBoolean(), body('allow_favorites').optional().isBoolean(), body('allow_reactions').optional().isBoolean(), + body('allow_color_labels').optional().isBoolean(), + body('keybind_mode').optional().isIn(KEYBIND_MODES) + .withMessage(`keybind_mode must be one of: ${KEYBIND_MODES.join(', ')}`), body('require_name_email').optional().isBoolean(), body('moderate_comments').optional().isBoolean(), body('show_feedback_to_guests').optional().isBoolean(), diff --git a/backend/src/utils/photoFilterBuilder.js b/backend/src/utils/photoFilterBuilder.js index deb829cf..d772e0ea 100644 --- a/backend/src/utils/photoFilterBuilder.js +++ b/backend/src/utils/photoFilterBuilder.js @@ -3,6 +3,23 @@ * Builds Knex queries for filtering photos by feedback metrics */ +const { COLOR_LABELS } = require('../constants/colorLabels'); + +/** + * Accept a colour filter as an array or a comma-separated string, drop + * anything that isn't one of the five known colours, and de-duplicate. + */ +function normalizeColorLabels(value) { + if (!value) return []; + const raw = Array.isArray(value) ? value : String(value).split(','); + const seen = new Set(); + for (const entry of raw) { + const color = String(entry).trim().toLowerCase(); + if (COLOR_LABELS.includes(color)) seen.add(color); + } + return [...seen]; +} + class PhotoFilterBuilder { constructor(queryBuilder, eventId) { this.query = queryBuilder; @@ -21,6 +38,9 @@ class PhotoFilterBuilder { has_favorites, min_favorites, has_comments, + color_labels, + my_color_labels, + admin_id, category_id, logic = 'AND' } = filters; @@ -59,6 +79,36 @@ class PhotoFilterBuilder { conditions.push(builder => builder.where('photos.comment_count', '>', 0)); } + // Colour labels (#1044): "only the greens". Can't read a denormalized + // count — "has any label" and "has a GREEN label" are different questions + // — so this is an EXISTS over photo_feedback, covered by the + // photo_feedback_color_label_idx index from migration 180. + const requestedColors = normalizeColorLabels(color_labels); + if (requestedColors.length > 0) { + conditions.push(builder => builder.whereExists(function () { + this.select('*') + .from('photo_feedback') + .whereRaw('photo_feedback.photo_id = photos.id') + .where('photo_feedback.feedback_type', 'color_label') + .where('photo_feedback.is_hidden', false) + .whereIn('photo_feedback.color_label', requestedColors); + })); + } + + // The same question against the caller's own marks (#1044 follow-up). + // Requires admin_id: without one this would filter by every admin's marks + // at once, so it is skipped rather than silently widened. + const requestedMyColors = normalizeColorLabels(my_color_labels); + if (requestedMyColors.length > 0 && 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) + .whereIn('photo_admin_marks.color_label', requestedMyColors); + })); + } + if (category_id) { conditions.push(builder => builder.where('photos.category_id', category_id)); } @@ -143,18 +193,36 @@ class PhotoFilterBuilder { db.raw('COUNT(CASE WHEN average_rating > 0 THEN 1 END) as with_ratings'), db.raw('COUNT(CASE WHEN like_count > 0 THEN 1 END) as with_likes'), db.raw('COUNT(CASE WHEN favorite_count > 0 THEN 1 END) as with_favorites'), - db.raw('COUNT(CASE WHEN comment_count > 0 THEN 1 END) as with_comments') + db.raw('COUNT(CASE WHEN comment_count > 0 THEN 1 END) as with_comments'), + db.raw('COUNT(CASE WHEN color_label_count > 0 THEN 1 END) as with_color_labels') ) .first(); + // Per-colour totals for the filter chips (#1044) — the swatch row shows + // "Green 42" so the photographer knows which colours are worth filtering. + const colorRows = await db('photo_feedback') + .where({ event_id: eventId, feedback_type: 'color_label' }) + .where('is_hidden', false) + .groupBy('color_label') + .select('color_label') + .countDistinct('photo_id as count'); + + const colorLabelCounts = {}; + for (const color of COLOR_LABELS) colorLabelCounts[color] = 0; + for (const row of colorRows) { + if (row.color_label) colorLabelCounts[row.color_label] = parseInt(row.count) || 0; + } + return { total: parseInt(result.total) || 0, withRatings: parseInt(result.with_ratings) || 0, withLikes: parseInt(result.with_likes) || 0, withFavorites: parseInt(result.with_favorites) || 0, - withComments: parseInt(result.with_comments) || 0 + withComments: parseInt(result.with_comments) || 0, + withColorLabels: parseInt(result.with_color_labels) || 0, + colorLabelCounts }; } } -module.exports = { PhotoFilterBuilder }; +module.exports = { PhotoFilterBuilder, normalizeColorLabels }; diff --git a/frontend/src/components/admin/AdminGuestDetail.tsx b/frontend/src/components/admin/AdminGuestDetail.tsx index 58594f3b..37c9cbab 100644 --- a/frontend/src/components/admin/AdminGuestDetail.tsx +++ b/frontend/src/components/admin/AdminGuestDetail.tsx @@ -1,7 +1,7 @@ import React, { useState } from 'react'; import { useQuery } from '@tanstack/react-query'; import { useTranslation } from 'react-i18next'; -import { X, Heart, Bookmark, Star, MessageCircle, Smile } from 'lucide-react'; +import { X, Heart, Bookmark, Star, MessageCircle, Smile, Palette } from 'lucide-react'; import { Loading } from '../common'; import { guestsService, AdminGuest } from '../../services/guests.service'; import { AuthenticatedImage } from '../common/AuthenticatedImage'; @@ -14,13 +14,18 @@ interface AdminGuestDetailProps { onClose: () => void; } -type Tab = 'all' | 'liked' | 'favorited' | 'rated' | 'commented' | 'reacted'; +type Tab = 'all' | 'liked' | 'favorited' | 'rated' | 'commented' | 'reacted' | 'labeled'; export const AdminGuestDetail: React.FC = ({ eventId, guest, onClose }) => { const { t } = useTranslation(); const { formatDateTime: fmtDateTime } = useLocalizedDate(); const [tab, setTab] = useState('all'); + // Badges are plain strings in this grid, so a colour label shows as its + // name rather than a swatch — which also keeps it readable for anyone who + // can't tell the five colours apart. + const colorBadge = (color: string) => t(`feedback.colorLabels.${color}`, color); + const { data, isLoading } = useQuery({ queryKey: ['admin-guest-detail', eventId, guest.id], queryFn: () => guestsService.getGuestDetail(eventId, guest.id), @@ -32,6 +37,7 @@ export const AdminGuestDetail: React.FC = ({ eventId, gue const rated = selections?.rated || []; const commented = selections?.commented || []; const reacted = selections?.reacted || []; + const labeled = selections?.labeled || []; // "all" view combines the three visual selection types. type GridItem = { photo: { id: number; filename: string; thumbnail_url: string }; badges: string[] }; @@ -50,6 +56,7 @@ export const AdminGuestDetail: React.FC = ({ eventId, gue favorited.forEach((p) => add(p, 'favorite')); rated.forEach((r) => add(r.photo, 'rating')); reacted.forEach((r) => add(r.photo, r.reaction)); + labeled.forEach((r) => add(r.photo, colorBadge(r.color_label))); const visibleItems: GridItem[] = tab === 'all' @@ -62,6 +69,8 @@ export const AdminGuestDetail: React.FC = ({ eventId, gue ? rated.map((r) => ({ photo: r.photo, badges: [`${r.rating}★`] })) : tab === 'reacted' ? reacted.map((r) => ({ photo: r.photo, badges: [r.reaction] })) + : tab === 'labeled' + ? labeled.map((r) => ({ photo: r.photo, badges: [colorBadge(r.color_label)] })) : []; return ( @@ -137,11 +146,20 @@ export const AdminGuestDetail: React.FC = ({ eventId, gue {t('admin.guests.columns.reactions', 'Reactions')} +
+
+ {labeled.length} +
+
+ + {t('admin.guests.columns.colorLabels', 'Color labels')} +
+
{/* Tabs */}
- {(['all', 'liked', 'favorited', 'rated', 'commented', 'reacted'] as const).map((k) => ( + {(['all', 'liked', 'favorited', 'rated', 'commented', 'reacted', 'labeled'] as const).map((k) => (
)} + {/* Color label (#1044). Bottom-left, opposite the rating/comment + indicators, so a labelled photo reads at a glance in the + admin grid the same way it does in the client's gallery. */} + {photo.dominant_color_label && COLOR_LABEL_SWATCHES[photo.dominant_color_label as ColorLabel] && ( +
+ +
+ )} + + {/* The admin's OWN mark (#1044 follow-up), next to the client's + dot but visually distinct — a white ring and a star count — + so a triage pass is never confused with what the client + chose. */} + {(photo.my_color_label || photo.my_rating) && ( +
+ {photo.my_color_label && COLOR_LABEL_SWATCHES[photo.my_color_label as ColorLabel] && ( + + )} + {!!photo.my_rating && ( + + + {photo.my_rating} + + )} +
+ )} + {/* Feedback Indicators (moved to bottom-right to avoid covering category) */} {(commentCount > 0 || averageRating > 0 || likeCount > 0) && (
diff --git a/frontend/src/components/admin/AdminPhotoViewer.tsx b/frontend/src/components/admin/AdminPhotoViewer.tsx index e36b8ba8..4e8da529 100644 --- a/frontend/src/components/admin/AdminPhotoViewer.tsx +++ b/frontend/src/components/admin/AdminPhotoViewer.tsx @@ -10,6 +10,9 @@ import { Button } from '../common'; import { AdminAuthenticatedImage } from './AdminAuthenticatedImage'; import { AdminAuthenticatedVideo } from './AdminAuthenticatedVideo'; import { useLocalizedDate } from '../../hooks/useLocalizedDate'; +import { COLOR_LABELS, COLOR_LABEL_SWATCHES, type ColorLabel, type KeybindMode } from '../../services/feedback.service'; +import { resolveFeedbackKey, colorShortcutHints } from '../../utils/feedbackKeybinds'; +import { useTranslation } from 'react-i18next'; import { useMutationWithToast, useModal } from '../../hooks'; type AdminFeedbackResponse = { @@ -36,6 +39,11 @@ export const AdminPhotoViewer: React.FC = ({ }) => { const [currentIndex, setCurrentIndex] = useState(initialIndex); const [isDeleting, setIsDeleting] = useState(false); + const { t } = useTranslation(); + // The photographer's own triage mark (#1044 follow-up). Held locally and + // seeded from the row so the star/colour UI responds instantly; the grid + // picks it up when its query is invalidated. + const [myMarks, setMyMarks] = useState>({}); const categoryMenuModal = useModal(); const commentsModal = useModal(); const queryClient = useQueryClient(); @@ -143,6 +151,52 @@ export const AdminPhotoViewer: React.FC = ({ errorMessage: () => 'Failed to delete feedback' }); + // The mark shown for a photo: the local edit if there is one, otherwise + // whatever the list query loaded. + const markFor = (photo: AdminPhoto) => myMarks[photo.id] ?? { + rating: photo.my_rating ?? null, + color_label: (photo.my_color_label as ColorLabel) ?? null, + }; + const currentMark = markFor(currentPhoto); + + const saveMark = async (patch: { rating?: number | null; color_label?: ColorLabel | null }) => { + const photoId = currentPhoto.id; + const previous = markFor(currentPhoto); + const optimistic = { + rating: patch.rating === undefined ? previous.rating : patch.rating, + color_label: patch.color_label === undefined ? previous.color_label : patch.color_label, + }; + setMyMarks((prev) => ({ ...prev, [photoId]: optimistic })); + try { + await photosService.setPhotoMark(eventId, photoId, patch); + // The grid reads my_rating / my_color_label off the photo rows. + queryClient.invalidateQueries({ queryKey: ['admin-event-photos', String(eventId)] }); + } catch { + setMyMarks((prev) => ({ ...prev, [photoId]: previous })); + toast.error(t('admin.photos.markError', 'Failed to save your mark')); + } + }; + + // Pressing the same value again clears it, matching the gallery lightbox. + // + // 0 is the clear sentinel — Lightroom's own binding, and what + // resolveFeedbackKey returns for the '0' key. It must become `null` here: + // the mark service stores 1-5 only and rejects a literal 0, so passing it + // straight through turned "clear my rating" into an error toast. + const toggleMarkRating = (value: number) => + saveMark({ rating: value === 0 || currentMark.rating === value ? null : value }); + const toggleMarkColor = (color: ColorLabel) => + saveMark({ color_label: currentMark.color_label === color ? null : color }); + + // The admin viewer always uses the Lightroom bindings — 1-5 stars, 6-9 + // colours — regardless of the scheme chosen for the gallery. That scheme is + // a choice made FOR the client; this surface belongs to the photographer, + // who came from Lightroom and needs both halves on the keyboard. Resolved + // through the shared helper so the two viewers can't drift. + const keybindMode: KeybindMode = 'lightroom'; + const markRef = React.useRef({ currentMark, toggleMarkRating, toggleMarkColor, saveMark }); + markRef.current = { currentMark, toggleMarkRating, toggleMarkColor, saveMark }; + React.useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { switch (e.key) { @@ -155,6 +209,23 @@ export const AdminPhotoViewer: React.FC = ({ case 'ArrowRight': goToNext(); break; + default: { + // Proofing shortcuts for the photographer's own marks (#1044 + // follow-up). Read through a ref: this effect is keyed on + // currentIndex, so the closure would otherwise mark the photo that + // was open when it was registered. + const action = resolveFeedbackKey(e, { + mode: keybindMode, + allowColorLabels: true, + allowRatings: true, + }); + if (!action) break; + e.preventDefault(); + if (action.type === 'color') void markRef.current.toggleMarkColor(action.color); + else if (action.type === 'rating') void markRef.current.toggleMarkRating(action.value); + else void markRef.current.saveMark({ color_label: null }); + break; + } } }; @@ -331,6 +402,74 @@ export const AdminPhotoViewer: React.FC = ({ )}
+ {/* The photographer's own marks (#1044 follow-up). Above the guest + feedback block on purpose: this is the surface being used during + a triage pass, and it is explicitly labelled as private so + nobody mistakes it for what the client chose. */} +
+

+ + {t('admin.photos.myMarks', 'Your marks')} +

+

+ {t('admin.photos.myMarksHelp', 'Only you see these. They never appear in the client gallery, and they export to Lightroom as XMP.')} +

+ +
+ {[1, 2, 3, 4, 5].map((value) => ( + + ))} + 1–5 +
+ +
+ {COLOR_LABELS.map((color) => { + const swatch = COLOR_LABEL_SWATCHES[color]; + const isMine = currentMark.color_label === color; + const shortcut = colorShortcutHints(keybindMode)[color]; + const name = t(`feedback.colorLabels.${color}`, color); + return ( + + ); + })} +
+
+ {/* Feedback Section */} {feedbackData && (
diff --git a/frontend/src/components/admin/FeedbackSettings.tsx b/frontend/src/components/admin/FeedbackSettings.tsx index 1d05c2cf..269ac6f9 100644 --- a/frontend/src/components/admin/FeedbackSettings.tsx +++ b/frontend/src/components/admin/FeedbackSettings.tsx @@ -1,7 +1,8 @@ import React from 'react'; -import { MessageSquare, Star, Heart, Bookmark, Shield, Eye, User, Users, Smile } from 'lucide-react'; +import { MessageSquare, Star, Heart, Bookmark, Shield, Eye, User, Users, Smile, Palette, Keyboard } from 'lucide-react'; import { Card } from '../common'; import { useTranslation } from 'react-i18next'; +import { COLOR_LABELS, COLOR_LABEL_SWATCHES, KEYBIND_SCHEMES, type KeybindMode } from '../../services/feedback.service'; interface FeedbackSettingsProps { settings: FeedbackSettings; @@ -16,6 +17,8 @@ interface FeedbackSettings { allow_comments: boolean; allow_favorites: boolean; allow_reactions: boolean; + allow_color_labels: boolean; + keybind_mode?: KeybindMode; require_name_email: boolean; moderate_comments: boolean; show_feedback_to_guests: boolean; @@ -238,9 +241,105 @@ export const FeedbackSettings: React.FC = ({
+ + {/* Color labels (#1044) */} + + {/* Keyboard scheme for the lightbox (#1044). Only meaningful once + color labels are on — stars alone already use 1-5. */} + {settings.allow_color_labels && ( +
+

+ + {t('feedback.settings.keybindMode', 'Keyboard shortcuts')} +

+
+ {(['colors', 'lightroom'] as KeybindMode[]).map((mode) => ( + + ))} +
+
+ )} + {/* Per-guest caps (#655). Two numeric inputs; 0 / empty = unlimited. Only renders when the matching toggle is on — the cap is meaningless if the type itself is disabled. */} diff --git a/frontend/src/components/admin/PhotoExportMenu.tsx b/frontend/src/components/admin/PhotoExportMenu.tsx index bb1bd267..e7d8f3fb 100644 --- a/frontend/src/components/admin/PhotoExportMenu.tsx +++ b/frontend/src/components/admin/PhotoExportMenu.tsx @@ -85,6 +85,10 @@ export const PhotoExportMenu: React.FC = ({ }, }); + // Whose verdict the XMP sidecars carry (#1044 follow-up). Defaults to the + // client's selections, so existing exports are unchanged. + const [markSource, setMarkSource] = useState<'client' | 'mine'>('client'); + const handleExport = (format: 'txt' | 'csv' | 'xmp' | 'json') => { const options: ExportOptions = { format, @@ -98,7 +102,8 @@ export const PhotoExportMenu: React.FC = ({ include_rating: true, include_label: true, include_description: true, - include_keywords: true + include_keywords: true, + mark_source: markSource } }; @@ -115,6 +120,9 @@ export const PhotoExportMenu: React.FC = ({ has_favorites: filters.hasFavorites, min_favorites: filters.minFavorites, has_comments: filters.hasComments, + // #1044 — lets "export only the greens" round-trip to Lightroom. + color_labels: filters.colorLabels?.length ? filters.colorLabels : undefined, + my_color_labels: filters.myColorLabels?.length ? filters.myColorLabels : undefined, category_id: filters.categoryId, logic: filters.logic, sort: filters.sort, @@ -134,7 +142,9 @@ export const PhotoExportMenu: React.FC = ({ filters.minRating !== null || filters.hasLikes || filters.hasFavorites || - filters.hasComments + filters.hasComments || + (filters.colorLabels?.length || 0) > 0 || + (filters.myColorLabels?.length || 0) > 0 ); const isDisabled = disabled || (!hasSelection && !hasFilters); @@ -187,6 +197,22 @@ export const PhotoExportMenu: React.FC = ({ }

+ {/* Whose stars/colours the XMP sidecars carry (#1044 + follow-up). Only affects XMP — the CSV and JSON exports + carry both columns regardless. */} + + {EXPORT_FORMATS.map((format) => { const Icon = format.icon; return ( diff --git a/frontend/src/components/admin/PhotoFilterPanel.tsx b/frontend/src/components/admin/PhotoFilterPanel.tsx index 0b5f5122..3b43ec44 100644 --- a/frontend/src/components/admin/PhotoFilterPanel.tsx +++ b/frontend/src/components/admin/PhotoFilterPanel.tsx @@ -1,5 +1,6 @@ import React from 'react'; import { Star, Heart, Bookmark, MessageCircle, Filter, X } from 'lucide-react'; +import { COLOR_LABELS, COLOR_LABEL_SWATCHES, type ColorLabel } from '../../services/feedback.service'; import { useTranslation } from 'react-i18next'; import { Button } from '../common'; import { FeedbackFilters, FilterSummary } from '../../services/photos.service'; @@ -37,6 +38,31 @@ export const PhotoFilterPanel: React.FC = ({ onChange({ ...filters, [field]: !filters[field] }); }; + // Colour labels are multi-select (#1044): each swatch toggles its colour, + // an empty list means no colour filtering. + const toggleColorLabel = (color: ColorLabel) => { + const active = filters.colorLabels || []; + onChange({ + ...filters, + colorLabels: active.includes(color) + ? active.filter(c => c !== color) + : [...active, color], + }); + }; + + // The same row against the admin's own marks (#1044 follow-up), kept as a + // separate filter rather than merged with the client's — "the client's + // greens" and "my greens" are different questions during a cull. + const toggleMyColorLabel = (color: ColorLabel) => { + const active = filters.myColorLabels || []; + onChange({ + ...filters, + myColorLabels: active.includes(color) + ? active.filter(c => c !== color) + : [...active, color], + }); + }; + const handleLogicChange = (logic: 'AND' | 'OR') => { onChange({ ...filters, logic }); }; @@ -47,6 +73,8 @@ export const PhotoFilterPanel: React.FC = ({ hasLikes: false, hasFavorites: false, hasComments: false, + colorLabels: [], + myColorLabels: [], logic: 'AND' }); }; @@ -54,7 +82,9 @@ export const PhotoFilterPanel: React.FC = ({ const hasActiveFilters = filters.minRating !== null || filters.hasLikes || filters.hasFavorites || - filters.hasComments; + filters.hasComments || + (filters.colorLabels?.length || 0) > 0 || + (filters.myColorLabels?.length || 0) > 0; return (
@@ -150,6 +180,90 @@ export const PhotoFilterPanel: React.FC = ({
+ {/* Color labels (#1044). Rendered only when someone has actually + labelled something — an always-visible swatch row would be dead + UI in the many galleries that never turn the feature on. */} + {(summary?.withColorLabels || 0) > 0 && ( +
+ + {t('filter.colorLabels', 'Color labels')} + +
+ {COLOR_LABELS.map((color) => { + const count = summary?.colorLabelCounts?.[color] || 0; + const isActive = (filters.colorLabels || []).includes(color); + const swatch = COLOR_LABEL_SWATCHES[color]; + const name = t(`feedback.colorLabels.${color}`, color); + return ( + + ); + })} +
+
+ )} + + {/* The admin's own marks (#1044 follow-up). Same shape as the row + above, labelled so the two are never confused. */} + {Object.keys(summary?.myColorLabelCounts || {}).length > 0 && ( +
+ + {t('filter.myColorLabels', 'Your marks')} + +
+ {COLOR_LABELS.map((color) => { + const count = summary?.myColorLabelCounts?.[color] || 0; + if (count === 0 && !(filters.myColorLabels || []).includes(color)) return null; + const isActive = (filters.myColorLabels || []).includes(color); + const swatch = COLOR_LABEL_SWATCHES[color]; + const name = t(`feedback.colorLabels.${color}`, color); + return ( + + ); + })} +
+
+ )} + {/* Logic Toggle */} {(filters.hasLikes || filters.hasFavorites || filters.hasComments) && (
diff --git a/frontend/src/components/gallery/ColorLabelBadge.tsx b/frontend/src/components/gallery/ColorLabelBadge.tsx new file mode 100644 index 00000000..c8b3df86 --- /dev/null +++ b/frontend/src/components/gallery/ColorLabelBadge.tsx @@ -0,0 +1,46 @@ +import React from 'react'; +import { useTranslation } from 'react-i18next'; +import { COLOR_LABEL_SWATCHES, type ColorLabel } from '../../services/feedback.service'; + +interface ColorLabelBadgeProps { + colorLabel?: string | null; + /** Extra classes for positioning inside the tile. */ + className?: string; +} + +/** + * The colour a guest gave a photo, shown on the thumbnail (#1044). + * + * The whole point of the feature is that a client can see their selection + * progress across the grid without reopening anything, so this is deliberately + * loud: an inset ring around the tile plus a corner dot. Both are + * pointer-events-none so they never swallow a click meant for the tile. + */ +export const ColorLabelBadge: React.FC = ({ colorLabel, className = '' }) => { + const { t } = useTranslation(); + + if (!colorLabel || !(colorLabel in COLOR_LABEL_SWATCHES)) return null; + const swatch = COLOR_LABEL_SWATCHES[colorLabel as ColorLabel]; + const name = t(`feedback.colorLabels.${colorLabel}`, colorLabel); + + return ( + <> +
)} diff --git a/frontend/src/components/gallery/GalleryView.tsx b/frontend/src/components/gallery/GalleryView.tsx index 3bb6eff2..585a5a6e 100644 --- a/frontend/src/components/gallery/GalleryView.tsx +++ b/frontend/src/components/gallery/GalleryView.tsx @@ -1,4 +1,4 @@ -import React, { useState, useMemo, useEffect } from 'react'; +import React, { useState, useMemo, useEffect, useCallback } from 'react'; import { differenceInDays, parseISO } from 'date-fns'; import { useQuery } from '@tanstack/react-query'; import { useTranslation } from 'react-i18next'; @@ -26,7 +26,7 @@ import { useDevToolsProtection } from '../../hooks/useDevToolsProtection'; import { api } from '../../config/api'; import { Upload, Menu, Eye, EyeOff, Shield, X, Download } from 'lucide-react'; import { galleryService } from '../../services/gallery.service'; -import { feedbackService } from '../../services/feedback.service'; +import { feedbackService, type ColorLabel } from '../../services/feedback.service'; import { useWatermarkSettings } from '../../hooks/useWatermarkSettings'; import { useGalleryCustomCss } from '../../hooks/useGalleryCustomCss'; import { usePublicSettings } from '../../hooks/usePublicSettings'; @@ -106,6 +106,11 @@ export const GalleryView: React.FC = ({ slug, event }) => { // Multi-select feedback filters (#889): OR-combined; empty = "All". // Clicking a filter toggles it, clicking "All" clears the set. const [activeFilters, setActiveFilters] = useState([]); + // Colour-label filters (#1044) live in their own slice rather than as five + // more members of FeedbackFilterType: every exhaustive + // Record in this file and the chip components would + // otherwise have to grow to ten keys. + const [activeColorFilters, setActiveColorFilters] = useState([]); // People filter (#1074). Multi-select, AND by default — see the filter // block below. `peopleMatchAny` only becomes reachable once a second @@ -639,6 +644,16 @@ export const GalleryView: React.FC = ({ slug, event }) => { }); } + // Apply colour-label filters (#1044). Guest-scoped by construction: + // `my_color_label` is the requesting viewer's own label, which is what a + // proofing client means by "show me my greens". Composes with (ANDs + // against) every filter above, like the people filter. + if (activeColorFilters.length > 0) { + photos = photos.filter(photo => + !!photo.my_color_label && activeColorFilters.includes(photo.my_color_label as ColorLabel) + ); + } + // Apply sorting // Each comparator defaults to its natural order (desc for dates/size/rating, asc for name). // The flip multiplier reverses that when sortDesc differs from the natural order. @@ -679,7 +694,7 @@ export const GalleryView: React.FC = ({ slug, event }) => { } return photos; - }, [data?.photos, selectedCategoryId, searchTerm, sortBy, sortDesc, watermarkEnabled, slug, activeFilters, mediaFilter, isGuestIdentityMode, myFeedbackPhotoIds, selectedPersonIds, peopleMatchAny]); + }, [data?.photos, selectedCategoryId, searchTerm, sortBy, sortDesc, watermarkEnabled, slug, activeFilters, activeColorFilters, mediaFilter, isGuestIdentityMode, myFeedbackPhotoIds, selectedPersonIds, peopleMatchAny]); // Counts shown in the filter chips ("Liked (N)", etc.). In guest // mode these need to mirror the per-guest filter behaviour above — @@ -703,6 +718,24 @@ export const GalleryView: React.FC = ({ slug, event }) => { return data?.photos?.filter(p => (p.total_ratings || 0) > 0 || (p.average_rating || 0) > 0).length || 0; }, [data?.photos, isGuestIdentityMode, myFeedbackPhotoIds]); + // Per-colour chip counts (#1044) — the viewer's own labels, matching what + // the filter actually selects. + const colorLabelCounts = useMemo(() => { + const counts: Partial> = {}; + for (const photo of data?.photos || []) { + const label = photo.my_color_label as ColorLabel | null | undefined; + if (!label) continue; + counts[label] = (counts[label] || 0) + 1; + } + return counts; + }, [data?.photos]); + + const handleColorFilterToggle = useCallback((color: ColorLabel) => { + setActiveColorFilters(prev => + prev.includes(color) ? prev.filter(c => c !== color) : [...prev, color] + ); + }, []); + // Check if downloads are allowed (both event setting and not expired) const allowDownloads = !isExpired && (data?.event?.allow_downloads === true); @@ -1090,6 +1123,10 @@ export const GalleryView: React.FC = ({ slug, event }) => { likeCount={likeCount} favoriteCount={favoriteCount} ratedCount={ratedCount} + colorLabelsEnabled={!!feedbackSettings?.allow_color_labels} + activeColorFilters={activeColorFilters} + onColorFilterChange={handleColorFilterToggle} + colorLabelCounts={colorLabelCounts} /> ) : null} @@ -1237,6 +1274,10 @@ export const GalleryView: React.FC = ({ slug, event }) => { mediaFilter={mediaFilter} onMediaFilterChange={setMediaFilter} showMediaFilter={showMediaFilter} + colorLabelsEnabled={!!feedbackSettings?.allow_color_labels} + activeColorFilters={activeColorFilters} + onColorFilterChange={handleColorFilterToggle} + colorLabelCounts={colorLabelCounts} /> ) : null} diff --git a/frontend/src/components/gallery/PhotoCard.tsx b/frontend/src/components/gallery/PhotoCard.tsx index acd09954..412ef6f6 100644 --- a/frontend/src/components/gallery/PhotoCard.tsx +++ b/frontend/src/components/gallery/PhotoCard.tsx @@ -5,6 +5,7 @@ import { AuthenticatedImage } from '../common'; import { thumbnailUrlForTile } from './imageTiers'; import { FeedbackIdentityModal } from './FeedbackIdentityModal'; import { feedbackService } from '../../services/feedback.service'; +import { ColorLabelBadge } from './ColorLabelBadge'; import { useGuestIdentityOptional } from '../../contexts/GuestIdentityContext'; import type { Photo } from '../../types'; @@ -378,6 +379,11 @@ export const PhotoCard: React.FC = ({ <> + {/* The guest's own colour label (#1044) — visible without hovering + or opening anything, which is the point: the client watches + their selection progress across the grid. */} + + {beforeOverlay} {/* Hover Overlay */} diff --git a/frontend/src/components/gallery/PhotoColorLabels.tsx b/frontend/src/components/gallery/PhotoColorLabels.tsx new file mode 100644 index 00000000..25952c92 --- /dev/null +++ b/frontend/src/components/gallery/PhotoColorLabels.tsx @@ -0,0 +1,183 @@ +import React, { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { useMutation, useQueryClient } from '@tanstack/react-query'; +import { + feedbackService, + COLOR_LABELS, + COLOR_LABEL_SWATCHES, + type ColorLabel, +} from '../../services/feedback.service'; +import { toast } from 'react-toastify'; +import { FeedbackIdentityModal } from './FeedbackIdentityModal'; +import { useGuestIdentityOptional } from '../../contexts/GuestIdentityContext'; + +interface PhotoColorLabelsProps { + photoId: string; + gallerySlug: string; + /** The guest's current colour label, or null. */ + myColorLabel: ColorLabel | null; + /** Per-colour visible counts, e.g. { green: 3 }. */ + colorLabelCounts?: Partial>; + isEnabled: boolean; + requireNameEmail?: boolean; + /** Keyboard hint to show under each swatch, e.g. { green: '1' }. */ + shortcutHints?: Partial>; + onColorLabelChange?: (label: ColorLabel | null) => void; +} + +/** + * Colour-label picker (#1044): one label per guest per photo, changeable — + * tapping the current colour removes it, tapping another switches. Same + * contract and identity handling as PhotoReactions; the labels are + * Lightroom's five colours so a selection round-trips into the catalogue. + */ +export const PhotoColorLabels: React.FC = ({ + photoId, + gallerySlug, + myColorLabel, + colorLabelCounts = {}, + isEnabled, + requireNameEmail = false, + shortcutHints = {}, + onColorLabelChange +}) => { + const { t } = useTranslation(); + const queryClient = useQueryClient(); + const guestIdentity = useGuestIdentityOptional(); + const [isSubmitting, setIsSubmitting] = useState(false); + const [showIdentityModal, setShowIdentityModal] = useState(false); + const [savedIdentity, setSavedIdentity] = useState<{ name: string; email: string } | null>(null); + const [pendingColor, setPendingColor] = useState(null); + + const colorName = (color: ColorLabel) => t(`feedback.colorLabels.${color}`, color); + + const submitColorLabelMutation = useMutation({ + mutationFn: (data: { color: ColorLabel; guest_name?: string; guest_email?: string }) => + feedbackService.submitFeedback(gallerySlug, photoId, { + feedback_type: 'color_label', + color_label: data.color, + guest_name: data.guest_name || undefined, + guest_email: data.guest_email || undefined + }), + onMutate: async (data) => { + setIsSubmitting(true); + // Optimistic update: the same colour toggles off, another switches. The + // PRE-mutation value travels via the mutation context — the onError + // closure sees the post-optimistic render, so reading `myColorLabel` + // there would "revert" to the already-wrong state. + const previousColor = myColorLabel; + if (onColorLabelChange) { + onColorLabelChange(data.color === previousColor ? null : data.color); + } + return { previousColor }; + }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['photo-feedback', gallerySlug, photoId] }); + }, + onError: (_error, _data, context) => { + if (onColorLabelChange) { + onColorLabelChange(context?.previousColor ?? null); // revert optimistic update + } + toast.error(t('feedback.colorLabelError', 'Failed to update color label')); + }, + onSettled: () => { + setIsSubmitting(false); + } + }); + + const handleColorClick = async (color: ColorLabel) => { + if (!isEnabled || isSubmitting) return; + + // Guest identity mode: ensure a per-person guest token; the server reads + // name/email from the token — body values are ignored. + if (guestIdentity?.identityMode === 'guest') { + try { + await guestIdentity.ensureIdentity(); + } catch { + return; // user cancelled the prompt + } + submitColorLabelMutation.mutate({ color }); + return; + } + + // Simple mode: legacy inline prompt flow. + if (requireNameEmail && !savedIdentity) { + setPendingColor(color); + setShowIdentityModal(true); + } else { + submitColorLabelMutation.mutate({ + color, + ...(savedIdentity ? { guest_name: savedIdentity.name, guest_email: savedIdentity.email } : {}) + }); + } + }; + + const handleIdentitySubmit = (name: string, email: string) => { + setSavedIdentity({ name, email }); + setShowIdentityModal(false); + if (pendingColor) { + submitColorLabelMutation.mutate({ color: pendingColor, guest_name: name, guest_email: email }); + setPendingColor(null); + } + }; + + if (!isEnabled) return null; + + return ( + <> +
+ {COLOR_LABELS.map((color) => { + const count = colorLabelCounts[color] || 0; + const isMine = myColorLabel === color; + const swatch = COLOR_LABEL_SWATCHES[color]; + const shortcut = shortcutHints[color]; + return ( + + ); + })} +
+ { setShowIdentityModal(false); setPendingColor(null); }} + onSubmit={handleIdentitySubmit} + feedbackType={t('feedback.colorLabel', 'color label')} + /> + + ); +}; diff --git a/frontend/src/components/gallery/PhotoFilterBar.tsx b/frontend/src/components/gallery/PhotoFilterBar.tsx index c329c355..01b4e182 100644 --- a/frontend/src/components/gallery/PhotoFilterBar.tsx +++ b/frontend/src/components/gallery/PhotoFilterBar.tsx @@ -3,6 +3,8 @@ import { Search, SortAsc, SortDesc, Grid, Heart, Star, MessageSquare, Bookmark } import { useTranslation } from 'react-i18next'; import { Button, Input } from '../common'; import type { FilterType, FeedbackFilterType } from './GalleryFilter'; +import { ColorLabelFilterChips } from './ColorLabelFilterChips'; +import type { ColorLabel } from '../../services/feedback.service'; interface PhotoCategory { id: number | string; @@ -40,6 +42,12 @@ interface PhotoFilterBarProps { mediaFilter?: 'all' | 'photo' | 'video'; onMediaFilterChange?: (filter: 'all' | 'photo' | 'video') => void; showMediaFilter?: boolean; + // Colour-label filters (#1044). Their own slice rather than more members + // of FilterType — see the note in GalleryView. + colorLabelsEnabled?: boolean; + activeColorFilters?: ColorLabel[]; + onColorFilterChange?: (color: ColorLabel) => void; + colorLabelCounts?: Partial>; } export const PhotoFilterBar: React.FC = ({ @@ -59,7 +67,11 @@ export const PhotoFilterBar: React.FC = ({ onFilterChange, mediaFilter = 'all', onMediaFilterChange, - showMediaFilter = false + showMediaFilter = false, + colorLabelsEnabled = false, + activeColorFilters = [], + onColorFilterChange, + colorLabelCounts = {} }) => { const { t } = useTranslation(); const [showSortMenu, setShowSortMenu] = useState(false); @@ -291,6 +303,16 @@ export const PhotoFilterBar: React.FC = ({ )} + {/* Colour filter (#1044), desktop */} + {feedbackEnabled && colorLabelsEnabled && onColorFilterChange && ( + + )} + {/* Without categories this row only carries desktop content (the chips are lg-only; mobile has its own block below), so hide the count below lg to keep the mobile layout unchanged. */} @@ -389,6 +411,16 @@ export const PhotoFilterBar: React.FC = ({ )} + + {/* Colour filter (#1044), mobile/tablet */} + {feedbackEnabled && colorLabelsEnabled && onColorFilterChange && ( + + )} ); diff --git a/frontend/src/components/gallery/PhotoLightbox.tsx b/frontend/src/components/gallery/PhotoLightbox.tsx index f40b6e9b..2c0cc569 100644 --- a/frontend/src/components/gallery/PhotoLightbox.tsx +++ b/frontend/src/components/gallery/PhotoLightbox.tsx @@ -7,7 +7,9 @@ import { useSavePhotoToDevice } from '../../hooks/useGallery'; import { AuthenticatedImage } from '../common'; import { PhotoFeedback } from './PhotoFeedback'; import { previewUrlForViewport } from './imageTiers'; -import { feedbackService } from '../../services/feedback.service'; +import { feedbackService, type ColorLabel, type KeybindMode } from '../../services/feedback.service'; +import { PhotoColorLabels } from './PhotoColorLabels'; +import { resolveFeedbackKey, colorShortcutHints } from '../../utils/feedbackKeybinds'; import { galleryService } from '../../services/gallery.service'; import { FeedbackIdentityModal } from './FeedbackIdentityModal'; import { VideoPlayer } from './VideoPlayer'; @@ -93,19 +95,25 @@ export const PhotoLightbox: React.FC = ({ allow_ratings?: boolean; allow_comments?: boolean; allow_reactions?: boolean; + allow_color_labels?: boolean; + keybind_mode?: KeybindMode; show_feedback_to_guests?: boolean; require_name_email?: boolean; } | null>(null); const [myLiked, setMyLiked] = useState(false); const [myRating, setMyRating] = useState(0); + const [myColorLabel, setMyColorLabel] = useState(null); + const [colorLabelCounts, setColorLabelCounts] = useState>>({}); const [likeCount, setLikeCount] = useState(0); const [avgRating, setAvgRating] = useState(0); const [totalRatings, setTotalRatings] = useState(0); const [savedIdentity, setSavedIdentity] = useState<{ name: string; email: string } | null>(null); const [showIdentityModal, setShowIdentityModal] = useState(false); - const [pendingAction, setPendingAction] = useState(null); + const [pendingAction, setPendingAction] = useState(null); const guestIdentity = useGuestIdentityOptional(); const isGuestMode = guestIdentity?.identityMode === 'guest'; + // Which shortcut scheme this gallery uses (#1044). + const keybindMode: KeybindMode = feedbackSettings?.keybind_mode || 'colors'; // Per-guest cap modal (#655) — shared across every submitFeedback call site // in the lightbox (guest mode, simple mode, identity-modal-confirm path). const { modal: limitModal, handleError: handleLimitError } = useFeedbackLimitModal(); @@ -224,6 +232,21 @@ export const PhotoLightbox: React.FC = ({ }; }, [disableRightClick]); + // The keydown effect below is registered with [currentIndex] deps, so its + // closure would still hold the settings from the moment the lightbox + // opened — i.e. `null`, since they load asynchronously, leaving every + // proofing shortcut dead until the user changed photo. A ref refreshed on + // every render keeps the handler reading current state without + // re-registering the listener on each keystroke's worth of state change. + const proofingRef = useRef({ + feedbackEnabled: false, + allowColorLabels: false, + allowRatings: false, + keybindMode: 'colors' as KeybindMode, + myRating: 0, + submitColorLabel: (async () => {}) as (color: ColorLabel | null) => Promise, + submitRating: (async () => {}) as (value: number) => Promise, + }); useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { switch (e.key) { @@ -250,6 +273,30 @@ export const PhotoLightbox: React.FC = ({ handleDownload(); } break; + default: { + // Proofing shortcuts (#1044). Resolved from the event's keybind + // scheme so 1/2/3 mean colours in colour-only mode and stars in + // Lightroom mode; the helper ignores modified keys and anything + // typed into a field. + const proofing = proofingRef.current; + if (!proofing.feedbackEnabled) break; + const action = resolveFeedbackKey(e, { + mode: proofing.keybindMode, + allowColorLabels: proofing.allowColorLabels, + allowRatings: proofing.allowRatings, + }); + if (!action) break; + e.preventDefault(); + if (action.type === 'color') { + void proofing.submitColorLabel(action.color); + } else if (action.type === 'rating') { + // Pressing the current rating again clears it (#884). + void proofing.submitRating(action.value === proofing.myRating ? 0 : action.value); + } else { + void proofing.submitColorLabel(null); + } + break; + } } }; @@ -296,6 +343,8 @@ export const PhotoLightbox: React.FC = ({ if (!mounted) return; setMyLiked(!!data.my_feedback.liked); setMyRating(data.my_feedback.rating || 0); + setMyColorLabel((data.my_feedback.color_label as ColorLabel) || null); + setColorLabelCounts(data.color_labels || {}); setLikeCount(Number(data.summary?.like_count) || 0); setAvgRating(Number(data.summary?.average_rating) || 0); setTotalRatings(Number(data.summary?.total_ratings) || 0); @@ -417,6 +466,72 @@ export const PhotoLightbox: React.FC = ({ if (onFeedbackChange) onFeedbackChange(); }; + /** + * Set / switch / clear the guest's colour label (#1044). Same identity + * handling as submitLike above; `null` means "clear", which the backend + * expresses as submitting the current colour again. + */ + const submitColorLabel = async (color: ColorLabel | null) => { + if (!feedbackSettings?.allow_color_labels) return; + // Clearing means re-submitting the current colour — the backend toggles + // a repeat submission off. With nothing set there is nothing to clear. + const value = color ?? myColorLabel; + if (!value) return; + const willBeSet = value === myColorLabel ? null : value; + + if (isGuestMode && guestIdentity) { + try { + await guestIdentity.ensureIdentity(); + } catch { + return; + } + try { + await feedbackService.submitFeedback(slug, String(currentPhoto.id), { + feedback_type: 'color_label', + color_label: value, + }); + setMyColorLabel(willBeSet); + if (onFeedbackChange) onFeedbackChange(); + } catch (err) { + if (handleLimitError(err)) return; + console.warn('Color label submit failed', err); + } + return; + } + + const needIdentity = feedbackSettings?.require_name_email && !savedIdentity; + if (needIdentity) { + setPendingAction({ type: 'color_label', color: value }); + setShowIdentityModal(true); + return; + } + try { + await feedbackService.submitFeedback(slug, String(currentPhoto.id), { + feedback_type: 'color_label', + color_label: value, + guest_name: savedIdentity?.name, + guest_email: savedIdentity?.email, + }); + setMyColorLabel(willBeSet); + if (onFeedbackChange) onFeedbackChange(); + } catch (err) { + if (handleLimitError(err)) return; + console.warn('Color label submit failed', err); + } + }; + + // Refreshed on every render (see the ref's declaration above): the keydown + // listener reads current settings and handlers without being re-registered. + proofingRef.current = { + feedbackEnabled: !!feedbackEnabled && !!feedbackSettings?.feedback_enabled, + allowColorLabels: !!feedbackSettings?.allow_color_labels, + allowRatings: !!feedbackSettings?.allow_ratings, + keybindMode, + myRating, + submitColorLabel, + submitRating, + }; + const goToPrevious = () => { setCurrentIndex((prev) => (prev > 0 ? prev - 1 : photos.length - 1)); resetZoom(); @@ -927,6 +1042,27 @@ export const PhotoLightbox: React.FC = ({ )} + {/* Inline color labels (#1044). In the toolbar rather than the + feedback panel: the whole point is a fast keyboard/click + proofing pass, which a panel toggle would interrupt. */} + {feedbackEnabled && feedbackSettings?.allow_color_labels && ( +
+ { + setMyColorLabel(label); + if (onFeedbackChange) onFeedbackChange(); + }} + /> +
+ )} + {/* Feedback button with indicator. Likes/ratings have their own dedicated toolbar buttons above, so this panel toggle only has work to do when comments (#518) or the emoji @@ -1197,13 +1333,29 @@ export const PhotoLightbox: React.FC = ({ setAvgRating(Number(fresh.summary?.average_rating) || 0); setTotalRatings(Number(fresh.summary?.total_ratings) || 0); } catch {} + } else if (pendingAction?.type === 'color_label' && pendingAction.color) { + try { + await feedbackService.submitFeedback(slug, String(currentPhoto.id), { + feedback_type: 'color_label', + color_label: pendingAction.color, + guest_name: name, + guest_email: email, + }); + setMyColorLabel(pendingAction.color === myColorLabel ? null : pendingAction.color); + } catch (err) { + if (!handleLimitError(err)) throw err; + } } // Sync gallery photo list (feedback filter chips) — parity with // the direct submit paths. if (onFeedbackChange) onFeedbackChange(); setPendingAction(null); }} - feedbackType={pendingAction?.type === 'rating' ? 'rating' : 'like'} + feedbackType={ + pendingAction?.type === 'rating' ? 'rating' + : pendingAction?.type === 'color_label' ? 'color label' + : 'like' + } /> {/* Per-guest cap modal (#655). Single instance fires for any of the lightbox's submitFeedback paths via the shared hook. */} diff --git a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx index 743e2c9e..5699dbf8 100644 --- a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx +++ b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx @@ -10,6 +10,7 @@ import Download from 'yet-another-react-lightbox/plugins/download'; import Captions from 'yet-another-react-lightbox/plugins/captions'; import 'yet-another-react-lightbox/styles.css'; import 'yet-another-react-lightbox/plugins/thumbnails.css'; +import { ColorLabelBadge } from '../ColorLabelBadge'; import 'yet-another-react-lightbox/plugins/captions.css'; import { motion, AnimatePresence } from 'framer-motion'; import { Download as DownloadIcon, Heart, Check, Star, MessageSquare, Package, LogOut } from 'lucide-react'; @@ -130,6 +131,9 @@ const PhotoCard: React.FC = ({ useCanvasRendering={useCanvasRendering || protectionLevel === 'maximum'} /> + {/* Colour label (#1044) — same badge every layout uses. */} + + {/* Overlay Gradient */}
diff --git a/frontend/src/components/gallery/layouts/story/StoryPhotoCard.tsx b/frontend/src/components/gallery/layouts/story/StoryPhotoCard.tsx index 2bd4e0ca..dd3374bd 100644 --- a/frontend/src/components/gallery/layouts/story/StoryPhotoCard.tsx +++ b/frontend/src/components/gallery/layouts/story/StoryPhotoCard.tsx @@ -2,6 +2,7 @@ import React, { useState } from 'react'; import { motion } from 'framer-motion'; import { Heart } from 'lucide-react'; import { AuthenticatedImage } from '../../../common'; +import { ColorLabelBadge } from '../../ColorLabelBadge'; import type { Photo } from '../../../../types'; interface StoryPhotoCardProps { @@ -78,6 +79,9 @@ export const StoryPhotoCard: React.FC = ({ /> + {/* Colour label (#1044) — same badge every layout uses. */} + + {/* Overlay */}
diff --git a/frontend/src/features/settings/hooks/useSettingsState.ts b/frontend/src/features/settings/hooks/useSettingsState.ts index 37ad7f1d..3d45d4e9 100644 --- a/frontend/src/features/settings/hooks/useSettingsState.ts +++ b/frontend/src/features/settings/hooks/useSettingsState.ts @@ -89,6 +89,15 @@ export interface EventSettings { event_require_expiration: boolean; event_default_require_password: boolean; event_default_feedback_enabled: boolean; + // Per-type guest-feedback defaults for new galleries (#1044). These are + // DEFAULTS: changing one never touches a gallery that already exists. + event_default_allow_ratings: boolean; + event_default_allow_likes: boolean; + event_default_allow_favorites: boolean; + event_default_allow_comments: boolean; + event_default_allow_reactions: boolean; + event_default_allow_color_labels: boolean; + event_default_keybind_mode: 'colors' | 'lightroom'; gallery_show_filter_bar: boolean; event_phone_field_enabled: boolean; } @@ -177,6 +186,13 @@ export function useSettingsState() { event_require_expiration: true, event_default_require_password: true, event_default_feedback_enabled: false, + event_default_allow_ratings: true, + event_default_allow_likes: true, + event_default_allow_favorites: true, + event_default_allow_comments: true, + event_default_allow_reactions: true, + event_default_allow_color_labels: false, + event_default_keybind_mode: 'colors', gallery_show_filter_bar: true, event_phone_field_enabled: false }); @@ -285,6 +301,15 @@ export function useSettingsState() { event_require_expiration: toBoolean(settings.event_require_expiration, true), event_default_require_password: toBoolean(settings.event_default_require_password, true), event_default_feedback_enabled: toBoolean(settings.event_default_feedback_enabled, false), + // Fallbacks mirror FEEDBACK_TOGGLES in backend + // services/feedbackDefaults.js — keep the two in step. + event_default_allow_ratings: toBoolean(settings.event_default_allow_ratings, true), + event_default_allow_likes: toBoolean(settings.event_default_allow_likes, true), + event_default_allow_favorites: toBoolean(settings.event_default_allow_favorites, true), + event_default_allow_comments: toBoolean(settings.event_default_allow_comments, true), + event_default_allow_reactions: toBoolean(settings.event_default_allow_reactions, true), + event_default_allow_color_labels: toBoolean(settings.event_default_allow_color_labels, false), + event_default_keybind_mode: settings.event_default_keybind_mode === 'lightroom' ? 'lightroom' : 'colors', gallery_show_filter_bar: toBoolean(settings.gallery_show_filter_bar, true), event_phone_field_enabled: toBoolean(settings.event_phone_field_enabled, false) }); diff --git a/frontend/src/features/settings/tabs/EventsTab.tsx b/frontend/src/features/settings/tabs/EventsTab.tsx index 5907c724..7a81ea60 100644 --- a/frontend/src/features/settings/tabs/EventsTab.tsx +++ b/frontend/src/features/settings/tabs/EventsTab.tsx @@ -3,6 +3,7 @@ import { Save, AlertCircle } from 'lucide-react'; import { Button, Card } from '../../../components/common'; import { useTranslation } from 'react-i18next'; import type { EventSettings } from '../hooks/useSettingsState'; +import { COLOR_LABEL_SWATCHES, COLOR_LABELS } from '../../../services/feedback.service'; interface EventsTabProps { eventSettings: EventSettings; @@ -13,6 +14,25 @@ interface EventsTabProps { }; } +/** + * The per-type feedback defaults, in the order the per-event panel shows + * them. Mirrors FEEDBACK_TOGGLES in backend services/feedbackDefaults.js. + */ +const FEEDBACK_TYPE_DEFAULTS: Array<{ + key: 'event_default_allow_ratings' | 'event_default_allow_likes' + | 'event_default_allow_favorites' | 'event_default_allow_comments' + | 'event_default_allow_reactions' | 'event_default_allow_color_labels'; + label: string; + fallback: string; +}> = [ + { key: 'event_default_allow_ratings', label: 'settings.events.defaultAllowRatings', fallback: 'Star ratings' }, + { key: 'event_default_allow_likes', label: 'settings.events.defaultAllowLikes', fallback: 'Likes' }, + { key: 'event_default_allow_favorites', label: 'settings.events.defaultAllowFavorites', fallback: 'Favourites' }, + { key: 'event_default_allow_comments', label: 'settings.events.defaultAllowComments', fallback: 'Comments' }, + { key: 'event_default_allow_reactions', label: 'settings.events.defaultAllowReactions', fallback: 'Emoji reactions' }, + { key: 'event_default_allow_color_labels', label: 'settings.events.defaultAllowColorLabels', fallback: 'Color labels' }, +]; + export const EventsTab: React.FC = ({ eventSettings, setEventSettings, @@ -188,6 +208,79 @@ export const EventsTab: React.FC = ({
+ {/* Per-type guest-feedback defaults (#1044). Defaults for NEW + galleries — existing galleries keep whatever they were created + with, so flipping one here can never change a gallery a client + is in the middle of. Greyed out rather than hidden while the + master default is off, so the options stay discoverable. */} +
+

+ {t( + 'settings.events.feedbackTypeDefaultsHelp', + 'Which feedback types new galleries start with. Existing galleries are not affected — each gallery can still be changed individually.' + )} +

+ + {FEEDBACK_TYPE_DEFAULTS.map(({ key, label, fallback }) => ( + + ))} + +
+ + +
+
+