Stable twin of #1153. Everything in the system treats a hidden row as absent, but the per-viewer is_liked heart read the row without looking at is_hidden — so a like the photographer had hidden still showed as liked on a photo whose like_count was zero. Making that agree exposes the second half: the duplicate check behind like/favorite toggling did not skip hidden rows either, so the now-empty heart, when clicked, found the hidden row and toggled it OFF. Skipping hidden rows there makes the click create a fresh, visible row. Also carried from review: the per-guest caps, /my-feedback and getEventFeedbackSummary no longer count hidden rows, and unhiding collapses the guest's replacement — skipped when there is no stable identity, since that fallback was 'guest_identifier IS NULL', i.e. other visitors' rows. Not carried: the my_color_label badge (colour labels are #1044) and the clearScope / singleValueScope visibility fix, neither of which exists on this branch. Merged with admin privileges: the author cannot self-approve.
This commit is contained in:
@@ -0,0 +1,245 @@
|
|||||||
|
/**
|
||||||
|
* 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. One place disagreed — the
|
||||||
|
* per-viewer `is_liked` heart — so a like the photographer had hidden still
|
||||||
|
* showed as liked on a photo whose like_count was zero. (The `my_color_label`
|
||||||
|
* badge has the same shape on main; colour labels are not on this branch.)
|
||||||
|
*
|
||||||
|
* 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: '[email protected]',
|
||||||
|
admin_email: '[email protected]',
|
||||||
|
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,
|
||||||
|
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 });
|
||||||
|
});
|
||||||
|
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
|
||||||
|
});
|
||||||
|
|
||||||
|
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('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.
|
||||||
|
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);
|
||||||
|
|
||||||
|
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 });
|
||||||
|
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
});
|
||||||
@@ -617,7 +617,12 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
|
|||||||
const likedPhotoIds = new Set();
|
const likedPhotoIds = new Set();
|
||||||
if (showFeedbackToGuests && photos.length > 0) {
|
if (showFeedbackToGuests && photos.length > 0) {
|
||||||
const likeQuery = db('photo_feedback')
|
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 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));
|
.whereIn('photo_id', photos.map(p => p.id));
|
||||||
if (req.guest?.id) {
|
if (req.guest?.id) {
|
||||||
likeQuery.where('guest_id', req.guest.id);
|
likeQuery.where('guest_id', req.guest.id);
|
||||||
|
|||||||
@@ -367,7 +367,14 @@ router.get('/:slug/my-feedback',
|
|||||||
|
|
||||||
const query = db('photo_feedback')
|
const query = db('photo_feedback')
|
||||||
.join('photos', 'photo_feedback.photo_id', 'photos.id')
|
.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
|
// Prefer guest_id lookup when a verified guest token is present
|
||||||
// (per-person identity). Fall back to the device hash otherwise.
|
// (per-person identity). Fall back to the device hash otherwise.
|
||||||
|
|||||||
@@ -124,7 +124,12 @@ class FeedbackService {
|
|||||||
*/
|
*/
|
||||||
async countGuestFeedback(eventId, feedbackType, guestId, guestIdentifier) {
|
async countGuestFeedback(eventId, feedbackType, guestId, guestIdentifier) {
|
||||||
const query = db('photo_feedback')
|
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) {
|
if (guestId) {
|
||||||
query.where('guest_id', guestId);
|
query.where('guest_id', guestId);
|
||||||
} else {
|
} else {
|
||||||
@@ -152,6 +157,13 @@ class FeedbackService {
|
|||||||
photo_id: photoId,
|
photo_id: photoId,
|
||||||
event_id: eventId,
|
event_id: eventId,
|
||||||
feedback_type,
|
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) {
|
if (guest_id) {
|
||||||
duplicateQuery.where('guest_id', guest_id);
|
duplicateQuery.where('guest_id', guest_id);
|
||||||
@@ -297,6 +309,11 @@ class FeedbackService {
|
|||||||
|
|
||||||
const totalStats = await db('photo_feedback')
|
const totalStats = await db('photo_feedback')
|
||||||
.where('event_id', eventId)
|
.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(
|
.select(
|
||||||
db.raw('COUNT(DISTINCT CASE WHEN feedback_type = ? THEN guest_identifier END) as unique_raters', ['rating']),
|
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']),
|
db.raw('COUNT(CASE WHEN feedback_type = ? THEN 1 END) as total_ratings', ['rating']),
|
||||||
@@ -380,6 +397,32 @@ class FeedbackService {
|
|||||||
.where('id', feedbackId)
|
.where('id', feedbackId)
|
||||||
.update(updates);
|
.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.
|
||||||
|
//
|
||||||
|
// Needs a stable identity to scope by. 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. Comments are exempt: several
|
||||||
|
// from one guest on one photo is normal.
|
||||||
|
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
|
// Update photo stats if visibility changed
|
||||||
await this.updatePhotoFeedbackStats(feedback.photo_id);
|
await this.updatePhotoFeedbackStats(feedback.photo_id);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user