fix(gallery): show a guest their own likes when feedback sharing is off (#1286)
show_feedback_to_guests means "don't show guests OTHER PEOPLE's feedback". The per-viewer is_liked flag was gated on it anyway, so turning sharing off emptied every heart the guest had set themselves, on every page load, while the photo_feedback rows sat there intact. The query behind the flag is filtered to the viewer (by guest_id, or by their own IP+UA identifier), so what it returns was never aggregate data. The colour-label block twelve lines below already documents this exact reasoning and is correctly ungated. Scope is just that flag — the counts beside it stay gated, with a test pinning that the fix does not leak them back. The #1150 contract still holds: an admin-hidden like does not read as liked. Note for the reporter: the FILTER path was already correct (includeGuestMatches is ungated, /my-feedback carries no gate). The empty Likes chip was downstream of the same falsified flag, not a second bug. Closes #1286
This commit is contained in:
@@ -53,6 +53,19 @@ describe('guest filters and show_feedback_to_guests (#1044)', () => {
|
|||||||
{ expiresIn: '1h', issuer: 'picpeak-auth' }
|
{ expiresIn: '1h', issuer: 'picpeak-auth' }
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// The photo payload itself, not just the filtered id list — `is_liked` and
|
||||||
|
// the aggregate counts live here (#1286).
|
||||||
|
const payload = async ({ as = 'me' } = {}) => {
|
||||||
|
const req = request(app)
|
||||||
|
.get(`/api/gallery/${SLUG}/photos`)
|
||||||
|
.set('Authorization', `Bearer ${galleryToken()}`);
|
||||||
|
if (as === 'me') req.set('x-guest-token', guestToken());
|
||||||
|
const res = await req;
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
const photos = Array.isArray(res.body) ? res.body : res.body.photos;
|
||||||
|
return Object.fromEntries((photos || []).map((p) => [p.id, p]));
|
||||||
|
};
|
||||||
|
|
||||||
const filter = async (token, { as = 'me', claimGuestId } = {}) => {
|
const filter = async (token, { as = 'me', claimGuestId } = {}) => {
|
||||||
const req = request(app)
|
const req = request(app)
|
||||||
.get(`/api/gallery/${SLUG}/photos`)
|
.get(`/api/gallery/${SLUG}/photos`)
|
||||||
@@ -218,4 +231,70 @@ describe('guest filters and show_feedback_to_guests (#1044)', () => {
|
|||||||
expect(await filter('liked', { as: 'anon', claimGuestId: ME })).toEqual([]);
|
expect(await filter('liked', { as: 'anon', claimGuestId: ME })).toEqual([]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
// #1286 — the viewer's OWN like is not other people's feedback.
|
||||||
|
describe("a guest's own likes with feedback hidden (#1286)", () => {
|
||||||
|
beforeAll(() => setVisibility(false));
|
||||||
|
|
||||||
|
it('still reports is_liked on the photo the viewer liked', async () => {
|
||||||
|
// The regression: every heart came back empty on a gallery with
|
||||||
|
// sharing off, so the grid looked like it had discarded the guest's
|
||||||
|
// choices on every reload.
|
||||||
|
const photos = await payload();
|
||||||
|
expect(photos[mine].is_liked).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not report is_liked for someone else's like", async () => {
|
||||||
|
const photos = await payload();
|
||||||
|
expect(photos[theirs].is_liked).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps the aggregate like_count hidden', async () => {
|
||||||
|
// The count IS other people's feedback and must stay gated — the fix
|
||||||
|
// must not leak it back through the same payload.
|
||||||
|
const photos = await payload();
|
||||||
|
expect(photos[mine].like_count).toBe(0);
|
||||||
|
expect(photos[theirs].like_count).toBe(0);
|
||||||
|
expect(photos[theirs].has_feedback).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('reports nothing as liked for a viewer who liked nothing', async () => {
|
||||||
|
const photos = await payload({ as: 'anon' });
|
||||||
|
expect(photos[mine].is_liked).toBe(false);
|
||||||
|
expect(photos[theirs].is_liked).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still respects an admin hiding the viewer's own like (#1150)", async () => {
|
||||||
|
await db('photo_feedback')
|
||||||
|
.where({ photo_id: mine, guest_id: myGuestRowId, feedback_type: 'like' })
|
||||||
|
.update({ is_hidden: true });
|
||||||
|
|
||||||
|
const photos = await payload();
|
||||||
|
expect(photos[mine].is_liked).toBe(false);
|
||||||
|
|
||||||
|
await db('photo_feedback')
|
||||||
|
.where({ photo_id: mine, guest_id: myGuestRowId, feedback_type: 'like' })
|
||||||
|
.update({ is_hidden: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('matches what the liked filter already returned', async () => {
|
||||||
|
// The filter half was never gated; the payload flag was. After the fix
|
||||||
|
// the two agree, which is what makes the grid and the Likes chip show
|
||||||
|
// the same set.
|
||||||
|
expect(await filter('liked')).toEqual([mine]);
|
||||||
|
const photos = await payload();
|
||||||
|
const flagged = Object.values(photos).filter((p) => p.is_liked).map((p) => p.id);
|
||||||
|
expect(flagged).toEqual([mine]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('with feedback visible again (#1286 regression guard)', () => {
|
||||||
|
beforeAll(() => setVisibility(true));
|
||||||
|
|
||||||
|
it('is_liked and the counts both come back', async () => {
|
||||||
|
const photos = await payload();
|
||||||
|
expect(photos[mine].is_liked).toBe(true);
|
||||||
|
expect(photos[theirs].is_liked).toBe(false);
|
||||||
|
expect(photos[theirs].like_count).toBe(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1022,10 +1022,17 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, noStoreCache, asy
|
|||||||
// frontend can seed correctly. Prefers req.guest.id when a verified
|
// frontend can seed correctly. Prefers req.guest.id when a verified
|
||||||
// guest token is present (per-person identity), falls back to the
|
// guest token is present (per-person identity), falls back to the
|
||||||
// IP+UA hash that the original like was recorded under — same model
|
// IP+UA hash that the original like was recorded under — same model
|
||||||
// the /my-feedback endpoint uses. Skipped when feedback is hidden
|
// the /my-feedback endpoint uses.
|
||||||
// from guests.
|
//
|
||||||
|
// NOT gated on showFeedbackToGuests (#1286). This query is filtered to
|
||||||
|
// the VIEWER — by guest_id or by their own identifier — so what it
|
||||||
|
// returns is their own selection, not shared aggregate data. Gating it
|
||||||
|
// emptied every heart the guest had set themselves on a gallery with
|
||||||
|
// sharing off, which reads as the gallery silently discarding their
|
||||||
|
// choices. Same reasoning the colour-label block below already applies;
|
||||||
|
// this was the one per-viewer field that disagreed with it.
|
||||||
const likedPhotoIds = new Set();
|
const likedPhotoIds = new Set();
|
||||||
if (showFeedbackToGuests && photos.length > 0) {
|
if (photos.length > 0) {
|
||||||
const likeQuery = db('photo_feedback')
|
const likeQuery = db('photo_feedback')
|
||||||
// Hidden rows are not there, for the viewer's OWN feedback as much as
|
// Hidden rows are not there, for the viewer's OWN feedback as much as
|
||||||
// anyone's (#1150). getPhotoFeedback drops them, the filter drops them
|
// anyone's (#1150). getPhotoFeedback drops them, the filter drops them
|
||||||
@@ -1405,7 +1412,9 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, noStoreCache, asy
|
|||||||
// Per-viewer flag (#590 follow-up) — true when this viewer has
|
// Per-viewer flag (#590 follow-up) — true when this viewer has
|
||||||
// an active like row for this photo, false otherwise. Lets the
|
// an active like row for this photo, false otherwise. Lets the
|
||||||
// grid seed its lifted likedPhotoIds correctly on hard refresh.
|
// grid seed its lifted likedPhotoIds correctly on hard refresh.
|
||||||
is_liked: showFeedbackToGuests ? likedPhotoIds.has(photo.id) : false,
|
// Survives show_feedback_to_guests being off (#1286): the viewer's
|
||||||
|
// own heart is theirs, and the like_count beside it stays hidden.
|
||||||
|
is_liked: likedPhotoIds.has(photo.id),
|
||||||
favorite_count: showFeedbackToGuests ? (photo.favorite_count || 0) : 0,
|
favorite_count: showFeedbackToGuests ? (photo.favorite_count || 0) : 0,
|
||||||
// Colour labels (#1044). The COUNT is aggregate data and follows
|
// Colour labels (#1044). The COUNT is aggregate data and follows
|
||||||
// show_feedback_to_guests like its siblings; the viewer's OWN label
|
// show_feedback_to_guests like its siblings; the viewer's OWN label
|
||||||
|
|||||||
Reference in New Issue
Block a user