diff --git a/backend/__tests__/integration/hiddenOwnFeedback.test.js b/backend/__tests__/integration/hiddenOwnFeedback.test.js new file mode 100644 index 00000000..9150d135 --- /dev/null +++ b/backend/__tests__/integration/hiddenOwnFeedback.test.js @@ -0,0 +1,283 @@ +/** + * Hidden feedback, seen from the guest who left it (#1150). + * + * Everything in the system treats a hidden row as absent: getPhotoFeedback + * drops it even for the guest's own feedback, the /photos filters drop it, and + * updatePhotoFeedbackStats does not count it. Two places disagreed — the + * per-viewer `is_liked` heart and the `my_color_label` badge — so a like the + * photographer had hidden still showed as liked on a photo whose like_count + * was zero. + * + * Making those two agree exposes the second half: the duplicate check that + * powers like/favorite toggling did NOT skip hidden rows, so the now-empty + * heart, when clicked, found the hidden row and toggled it OFF. The click + * appeared to do nothing and it took two more to get back to a filled heart. + * + * Hiding a non-comment is deliberate, not an accident of the raw route: #839 + * and #1044 both ship it, with tests asserting that a hidden reaction or + * colour label stops counting. So the fix is to make hidden mean absent + * consistently — not to stop admins hiding these. + */ + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const jwt = require('jsonwebtoken'); + +const { bootCrmDb, seedMinimal } = require('./helpers/crmDb'); + +process.env.JWT_SECRET = process.env.JWT_SECRET || 'hidden-feedback-secret'; + +const SLUG = 'hidden-own-feedback'; +const ME = 'guest-me-identifier'; + +describe('a guest\'s own hidden feedback (#1150)', () => { + let db; let cleanup; let app; let feedbackService; + let eventId; let photoId; let myGuestRowId; + + const galleryToken = () => jwt.sign( + { eventId, eventSlug: SLUG, type: 'gallery' }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); + const guestToken = () => jwt.sign( + { type: 'guest', guestId: myGuestRowId, eventId }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); + + const getPhoto = async () => { + const res = await request(app) + .get(`/api/gallery/${SLUG}/photos`) + .set('Authorization', `Bearer ${galleryToken()}`) + .set('x-guest-token', guestToken()); + expect(res.status).toBe(200); + const photos = Array.isArray(res.body) ? res.body : res.body.photos; + return (photos || []).find((p) => p.id === photoId); + }; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + feedbackService = require('../../src/services/feedbackService'); + + const [ev] = await db('events').insert({ + slug: SLUG, + event_type: 'wedding', + event_name: 'Hidden Own Feedback', + event_date: '2026-08-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + share_link: `/gallery/${SLUG}/share`, + share_token: 'hidden-own-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 = typeof ev === 'object' ? ev.id : ev; + + const [p] = await db('photos').insert({ + event_id: eventId, filename: 'shot.jpg', path: 'events/hidden/shot.jpg', + type: 'individual', uploaded_at: new Date().toISOString(), + }).returning('id'); + photoId = typeof p === 'object' ? p.id : p; + + const [g] = await db('gallery_guests').insert({ + event_id: eventId, name: 'Me', identifier: ME, + created_at: new Date().toISOString(), last_seen_at: new Date().toISOString(), + is_deleted: false, + }).returning('id'); + myGuestRowId = typeof g === 'object' ? g.id : g; + + await db('event_feedback_settings').insert({ + event_id: eventId, feedback_enabled: true, allow_likes: true, + allow_color_labels: true, moderate_comments: false, + show_feedback_to_guests: true, + }); + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/gallery')); + app.use('/api/gallery', require('../../src/routes/galleryFeedback')); + }, 180000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + const like = () => db('photo_feedback').insert({ + photo_id: photoId, event_id: eventId, guest_identifier: ME, + guest_id: myGuestRowId, feedback_type: 'like', + is_approved: true, is_hidden: false, created_at: new Date().toISOString(), + }); + + beforeEach(async () => { + await db('photo_feedback').where({ photo_id: photoId }).del(); + await db('photos').where('id', photoId).update({ like_count: 0, color_label_count: 0 }); + }); + + describe('the read surfaces agree with each other', () => { + it('un-fills the heart once the like is hidden', async () => { + await like(); + await feedbackService.updatePhotoFeedbackStats(photoId); + expect((await getPhoto()).is_liked).toBe(true); + + await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like' }) + .update({ is_hidden: true }); + await feedbackService.updatePhotoFeedbackStats(photoId); + + const photo = await getPhoto(); + // like_count already ignored hidden rows, so the heart was the only + // thing still claiming this photo was liked. + expect(photo.like_count).toBe(0); + expect(photo.is_liked).toBe(false); + }); + + it('drops a hidden colour label from the badge', async () => { + await db('photo_feedback').insert({ + photo_id: photoId, event_id: eventId, guest_identifier: ME, + guest_id: myGuestRowId, feedback_type: 'color_label', color_label: 'green', + is_approved: true, is_hidden: true, created_at: new Date().toISOString(), + }); + expect((await getPhoto()).my_color_label).toBeFalsy(); + }); + }); + + describe('and every other surface agrees', () => { + it('keeps a hidden like out of /my-feedback', async () => { + await like(); + await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like' }) + .update({ is_hidden: true }); + + const res = await request(app) + .get(`/api/gallery/${SLUG}/my-feedback`) + .set('Authorization', `Bearer ${galleryToken()}`) + .set('x-guest-token', guestToken()); + expect(res.status).toBe(200); + + // In guest identity mode the Liked/Favorited/Rated chips and their + // filters are built from THIS array, not from is_liked — so a hidden + // like left an empty heart while the chip still counted it. + expect(res.body.filter((f) => f.feedback_type === 'like')).toHaveLength(0); + }); + + it('does not count a hidden row against the guest cap', async () => { + await db('event_feedback_settings') + .where({ event_id: eventId }).update({ max_likes_per_guest: 1 }); + await like(); + await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like' }) + .update({ is_hidden: true }); + + // The hidden row is room, not an occupant: the guest sees an empty + // heart, and meeting that click with limit_reached leaves the control + // dead until they un-like something they can still see. + const result = await feedbackService.submitFeedback(photoId, eventId, { + feedback_type: 'like', guest_identifier: ME, guest_id: myGuestRowId, + }); + expect(result.limit_reached).toBeUndefined(); + + await db('event_feedback_settings') + .where({ event_id: eventId }).update({ max_likes_per_guest: null }); + }); + + it('keeps the hidden record when the guest changes their replacement', async () => { + // A hidden colour label and a visible replacement now coexist. The + // toggle/switch and rating-clear paths DELETE over the guest-scoped set, + // so an unfiltered scope took the admin's record with it — leaving + // nothing to review or unhide. + const [orig] = await db('photo_feedback').insert({ + photo_id: photoId, event_id: eventId, guest_identifier: ME, + guest_id: myGuestRowId, feedback_type: 'color_label', color_label: 'red', + is_approved: true, is_hidden: true, created_at: new Date().toISOString(), + }).returning('id'); + const hiddenId = typeof orig === 'object' ? orig.id : orig; + + // The guest, seeing no label, picks green, then switches to blue, then + // toggles blue off — every mutation the single-value path offers. + const opts = { feedback_type: 'color_label', guest_identifier: ME, guest_id: myGuestRowId }; + await feedbackService.submitFeedback(photoId, eventId, { ...opts, color_label: 'green' }); + await feedbackService.submitFeedback(photoId, eventId, { ...opts, color_label: 'blue' }); + await feedbackService.submitFeedback(photoId, eventId, { ...opts, color_label: 'blue' }); + + const survivor = await db('photo_feedback').where('id', hiddenId).first(); + expect(survivor).toBeTruthy(); + expect(survivor.is_hidden).toBeTruthy(); + expect(survivor.color_label).toBe('red'); + }); + + it('leaves other anonymous rows alone when there is no identity to scope by', async () => { + // With neither guest_id nor guest_identifier the collapse scope degrades + // to `guest_identifier IS NULL` — every identifier-less row on the + // photo, i.e. other people's. Verified: knex renders that as `is null`. + const anon = (extra) => ({ + photo_id: photoId, event_id: eventId, feedback_type: 'like', + is_approved: true, created_at: new Date().toISOString(), ...extra, + }); + const [h] = await db('photo_feedback').insert(anon({ is_hidden: true })).returning('id'); + const hiddenId = typeof h === 'object' ? h.id : h; + await db('photo_feedback').insert(anon({ is_hidden: false })); + await db('photo_feedback').insert(anon({ is_hidden: false })); + + await feedbackService.moderateFeedback(hiddenId, 'approve', 1); + + // All three survive: two unrelated visitors plus the unhidden one. + expect(await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like', is_hidden: false })) + .toHaveLength(3); + }); + + it('collapses the replacement when an admin unhides the original', async () => { + await like(); + const original = await db('photo_feedback').where({ photo_id: photoId }).first(); + await db('photo_feedback').where('id', original.id).update({ is_hidden: true }); + + // The guest, seeing an empty heart, likes again — a second row. + await feedbackService.submitFeedback(photoId, eventId, { + feedback_type: 'like', guest_identifier: ME, guest_id: myGuestRowId, + }); + expect(await db('photo_feedback').where({ photo_id: photoId })).toHaveLength(2); + + await feedbackService.moderateFeedback(original.id, 'approve', 1); + + // Two visible rows for one guest would double-count in the tallies and + // need two toggles to clear, since each deletes a single row. + const visible = await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like', is_hidden: false }); + expect(visible).toHaveLength(1); + expect(visible[0].id).toBe(original.id); + + await feedbackService.updatePhotoFeedbackStats(photoId); + expect((await db('photos').where('id', photoId).first()).like_count).toBe(1); + }); + }); + + describe('and clicking still works afterwards', () => { + it('re-liking creates a fresh row instead of toggling the hidden one off', async () => { + await like(); + await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like' }) + .update({ is_hidden: true }); + + // What the guest sees is an empty heart, so this is an ADD. + const result = await feedbackService.submitFeedback(photoId, eventId, { + feedback_type: 'like', + guest_identifier: ME, + guest_id: myGuestRowId, + }); + + // Before this, the duplicate check found the hidden row and deleted it — + // `removed: true` — so the click did nothing visible and the moderation + // was silently undone. + expect(result.removed).toBeUndefined(); + + const visible = await db('photo_feedback') + .where({ photo_id: photoId, feedback_type: 'like', is_hidden: false }); + expect(visible).toHaveLength(1); + expect((await getPhoto()).is_liked).toBe(true); + }); + }); + +}); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 4158605d..9ece85cd 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -881,7 +881,12 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) const likedPhotoIds = new Set(); if (showFeedbackToGuests && photos.length > 0) { const likeQuery = db('photo_feedback') - .where({ event_id: req.event.id, feedback_type: 'like' }) + // Hidden rows are not there, for the viewer's OWN feedback as much as + // anyone's (#1150). getPhotoFeedback drops them, the filter drops them + // and updatePhotoFeedbackStats does not count them — leaving the heart + // filled was the one place that disagreed, so a like the photographer + // had hidden still showed as liked on a photo whose like_count was 0. + .where({ event_id: req.event.id, feedback_type: 'like', is_hidden: false }) .whereIn('photo_id', photos.map(p => p.id)); if (req.guest?.id) { likeQuery.where('guest_id', req.guest.id); @@ -899,7 +904,8 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) const myColorLabelByPhoto = {}; if (photos.length > 0) { const colorQuery = db('photo_feedback') - .where({ event_id: req.event.id, feedback_type: 'color_label' }) + // Same rule as the heart above (#1150). + .where({ event_id: req.event.id, feedback_type: 'color_label', is_hidden: false }) .whereIn('photo_id', photos.map(p => p.id)); if (req.guest?.id) { colorQuery.where('guest_id', req.guest.id); diff --git a/backend/src/routes/galleryFeedback.js b/backend/src/routes/galleryFeedback.js index 6e5105ba..db5b6338 100644 --- a/backend/src/routes/galleryFeedback.js +++ b/backend/src/routes/galleryFeedback.js @@ -414,7 +414,14 @@ router.get('/:slug/my-feedback', const query = db('photo_feedback') .join('photos', 'photo_feedback.photo_id', 'photos.id') - .where('photo_feedback.event_id', event.id); + .where('photo_feedback.event_id', event.id) + // Hidden rows are absent for the guest who left them too (#1150). In + // guest identity mode GalleryView builds its Liked/Favorited/Rated + // chips and their filters from THIS array rather than from is_liked, + // so without this a hidden like left an empty heart while the Liked + // chip still counted it and still surfaced the photo. Unapproved rows + // stay: a comment in the moderation queue is still the guest's own. + .where('photo_feedback.is_hidden', false); // Prefer guest_id lookup when a verified guest token is present // (per-person identity). Fall back to the device hash otherwise. diff --git a/backend/src/services/feedbackService.js b/backend/src/services/feedbackService.js index 565c4374..e983cf37 100644 --- a/backend/src/services/feedbackService.js +++ b/backend/src/services/feedbackService.js @@ -147,7 +147,12 @@ class FeedbackService { */ async countGuestFeedback(eventId, feedbackType, guestId, guestIdentifier) { const query = db('photo_feedback') - .where({ event_id: eventId, feedback_type: feedbackType }); + // Hidden rows do not count against the guest's cap (#1150). They are + // absent everywhere else — the heart is empty, the tallies skip them, + // and submitFeedback now treats one as room for a fresh row. Counting + // them here would meet that fresh row with limit_reached and leave the + // control dead until the guest un-likes something they can still see. + .where({ event_id: eventId, feedback_type: feedbackType, is_hidden: false }); if (guestId) { query.where('guest_id', guestId); } else { @@ -192,6 +197,13 @@ class FeedbackService { photo_id: photoId, event_id: eventId, feedback_type, + // A hidden row is not there (#1150). Without this the guest saw an + // empty heart — every read surface treats hidden as absent — and + // clicking it found the hidden row and TOGGLED IT OFF, so the + // click appeared to do nothing and it took two more to get back to + // a filled heart. Skipping it makes the click create a fresh, + // visible row, which is what the guest is asking for. + is_hidden: false, }); if (guest_id) { duplicateQuery.where('guest_id', guest_id); @@ -208,10 +220,15 @@ class FeedbackService { // into duplicate rows (same defense as the reaction path), and // clearing must not leave a stray duplicate in the average. if (isRatingClear) { + // Visible rows only (#1150). A hidden row can now sit alongside + // the guest's replacement, and it is the admin's moderation + // record — clearing a rating must not destroy it, or there is + // nothing left to review or unhide. const clearScope = db('photo_feedback').where({ photo_id: photoId, event_id: eventId, feedback_type: 'rating', + is_hidden: false, }); if (guest_id) clearScope.where('guest_id', guest_id); else clearScope.where('guest_identifier', guestIdentifier); @@ -246,11 +263,16 @@ class FeedbackService { const singleValueColumn = SINGLE_VALUE_COLUMNS[feedback_type]; if (singleValueColumn) { const submittedValue = feedback_type === 'reaction' ? reaction : color_label; + // Visible rows only, same reason as the rating clear above: the + // toggle-off and the duplicate collapse below both DELETE over + // this scope, and a hidden original is the admin's record rather + // than a racy duplicate of the guest's own. const singleValueScope = () => { const q = db('photo_feedback').where({ photo_id: photoId, event_id: eventId, feedback_type, + is_hidden: false, }); if (guest_id) q.where('guest_id', guest_id); else q.where('guest_identifier', guestIdentifier); @@ -406,6 +428,11 @@ class FeedbackService { const totalStats = await db('photo_feedback') .where('event_id', eventId) + // Hidden rows do not count, the same rule the photo counters above + // already apply — without this the two halves of THIS response + // disagreed, and a hidden row preserved beside its replacement (#1150) + // is counted twice. + .where('is_hidden', false) .select( db.raw('COUNT(DISTINCT CASE WHEN feedback_type = ? THEN guest_identifier END) as unique_raters', ['rating']), db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_ratings', ['rating']), @@ -555,6 +582,11 @@ class FeedbackService { */ async moderateFeedback(feedbackId, action, adminId) { try { + const target = await db('photo_feedback').where('id', feedbackId).first(); + if (!target) { + throw new Error('Feedback not found'); + } + const updates = { updated_at: new Date() }; @@ -569,17 +601,40 @@ class FeedbackService { updates.is_hidden = true; } - const feedback = await db('photo_feedback') - .where('id', feedbackId) - .first(); - - if (!feedback) { - throw new Error('Feedback not found'); - } - + const feedback = target; + await db('photo_feedback') .where('id', feedbackId) .update(updates); + + // Unhiding can collide with a replacement (#1150). A hidden row reads as + // absent, so the guest may well have re-added the same feedback in the + // meantime; making the original visible again would leave TWO visible + // rows for one guest on one photo — double-counted in the tallies, and + // needing two toggles to clear because each one deletes a single row. + // + // Converge on the row the admin acted on, the same way the submit path + // collapses racy duplicates. Comments are exempt: several from one guest + // on one photo is normal. + // Needs a stable identity to scope the collapse to ONE guest. With + // neither id nor identifier the fallback degrades to + // `guest_identifier IS NULL`, which is every identifier-less row on the + // photo — other people's, deleted. Nothing to converge in that case, so + // leave it alone. + const collapseIdentity = feedback.guest_id || feedback.guest_identifier; + if (updates.is_hidden === false && feedback.feedback_type !== 'comment' && collapseIdentity) { + const superseded = db('photo_feedback') + .where({ + photo_id: feedback.photo_id, + event_id: feedback.event_id, + feedback_type: feedback.feedback_type, + is_hidden: false, + }) + .whereNot('id', feedbackId); + if (feedback.guest_id) superseded.where('guest_id', feedback.guest_id); + else superseded.where('guest_identifier', feedback.guest_identifier); + await superseded.delete(); + } // Update photo stats if visibility changed await this.updatePhotoFeedbackStats(feedback.photo_id);