feat(feedback): let guests remove their star rating (#884) (#893)

* feat(feedback): let guests remove their star rating (#884)

Clicking your current rating again clears it. rating: 0 is the wire
contract: the validator now accepts 0, and the service deletes the
guest's rating row (instead of storing a 0 that would drag the photo
average down) and recalculates photo stats. The lightbox stars send 0
on a same-star click; PhotoRating already did, but the backend rejected
it with a 400 until now.

* fix(feedback): harden the rating-clear path (#884 review round)

External review follow-ups: numerically normalize the clear sentinel so
a numeric-string "0" can't slip into the update/insert paths (validator
now also toInt()s), delete the full guest-scoped rating set on clear so
racy duplicate rows can't survive in the average (same defense as the
reaction path), and refresh the visible average/count after the
identity-modal submit path like the direct paths do.

* fix(feedback): round-2 review fixes for rating clear (#884)

- Clear sentinel matches only an explicit 0 / "0" — malformed input
  (undefined, NaN, garbage strings) can no longer delete a rating.
- Lightbox survives the photo list shrinking while open (clearing your
  rating under the Rated filter drops the photo on refetch): index is
  re-anchored and the lightbox closes when the list empties, instead of
  crashing on an out-of-range index.
- Story layout gets the same same-star-to-clear behavior, keyed off the
  session-local my-rating map, and an explicit 0 no longer falls back to
  displaying the photo average.

* fix(feedback): refresh guest-scoped caches after rating changes (#884 review round 3)

- GalleryView's onFeedbackChange now also invalidates ['my-feedback',
  slug]: in guest identity mode the Rated/Liked filter membership and
  chip counts come from that query (#538), so a cleared rating never
  left the Rated filter until the 30s staleTime lapsed.
- PhotoRating invalidates gallery-photos + my-feedback on success: the
  parent refetch fires optimistically in onMutate and could capture
  pre-mutation state, with nothing refreshing after the server accepted.

---------

Co-authored-by: Paul Nothaft <[email protected]>

backend/__tests__/utils/feedbackRatingRemoval.test.js

@@ -0,0 +1,177 @@
+/**
+ * Unit tests for rating removal (#884).
+ *
+ * Pins the contract of `feedbackService.submitFeedback` for
+ * `feedback_type: 'rating'` with `rating: 0` ("clear my rating"):
+ *  - An existing rating row is DELETED (not updated to 0 — a stored 0
+ *    would drag the photo's average down and still count in totals).
+ *  - Photo stats (average_rating) are recalculated after the delete.
+ *  - Rating 0 with no existing rating is a no-op that never inserts a row.
+ *  - Removal is guest-scoped: clearing guest A's rating leaves guest B's
+ *    rating (and the resulting average) intact.
+ *  - Regular re-rating (3 → 5) still updates in place.
+ */
+
+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-rating-removal-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'rating-removal-test-secret';
+
+const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
+
+const feedbackService = require('../../src/services/feedbackService');
+
+const EVENT_SLUG = 'rating-removal-event';
+const GUEST_A = 'guest-a-identifier';
+const GUEST_B = 'guest-b-identifier';
+
+let db;
+let cleanup;
+let eventId;
+let photoId;
+
+async function rate(rating, guestIdentifier = GUEST_A) {
+  return feedbackService.submitFeedback(photoId, eventId, {
+    feedback_type: 'rating',
+    rating,
+    ip_address: '127.0.0.1',
+    user_agent: 'jest',
+  }, guestIdentifier);
+}
+
+async function ratingRows(guestIdentifier) {
+  const q = db('photo_feedback').where({
+    photo_id: photoId,
+    feedback_type: 'rating',
+  });
+  if (guestIdentifier) q.where('guest_identifier', guestIdentifier);
+  return q.select('*');
+}
+
+async function photoAverage() {
+  const photo = await db('photos').where('id', photoId).first();
+  return Number(photo.average_rating);
+}
+
+beforeAll(async () => {
+  ({ db, cleanup } = await bootCrmDb());
+  await seedMinimal(db);
+  const inserted = await db('events').insert({
+    slug: EVENT_SLUG,
+    event_type: 'wedding',
+    event_name: 'Rating Removal Test',
+    event_date: '2026-06-22',
+    host_email: '[email protected]',
+    admin_email: '[email protected]',
+    password_hash: 'x',
+    share_link: `/gallery/${EVENT_SLUG}/share`,
+    share_token: 'rating-removal-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];
+
+  const r = await db('photos').insert({
+    event_id: eventId,
+    filename: 'photo-1.jpg',
+    path: 'events/rating-removal/1.jpg',
+    type: 'individual',
+    uploaded_at: new Date().toISOString(),
+  }).returning('id');
+  photoId = r[0]?.id ?? r[0];
+}, 30000);
+
+afterAll(async () => { if (cleanup) await cleanup(); });
+
+beforeEach(async () => {
+  await db('photo_feedback').where('event_id', eventId).del();
+  await db('photos').where('id', photoId).update({ average_rating: 0, feedback_count: 0 });
+});
+
+describe('rating removal (#884)', () => {
+  test('rating 0 deletes the existing rating row and resets the average', async () => {
+    const created = await rate(4);
+    expect(created.created).toBe(true);
+    expect(await photoAverage()).toBe(4);
+
+    const removed = await rate(0);
+    expect(removed.removed).toBe(true);
+    expect(await ratingRows(GUEST_A)).toHaveLength(0);
+    expect(await photoAverage()).toBe(0);
+  });
+
+  test('rating 0 without an existing rating is a no-op (no 0-row inserted)', async () => {
+    const r = await rate(0);
+    expect(r.removed).toBe(true);
+    expect(await ratingRows()).toHaveLength(0);
+    expect(await photoAverage()).toBe(0);
+  });
+
+  test('removal is guest-scoped: guest B keeps their rating and the average', async () => {
+    await rate(2, GUEST_A);
+    await rate(4, GUEST_B);
+    expect(await photoAverage()).toBe(3);
+
+    const removed = await rate(0, GUEST_A);
+    expect(removed.removed).toBe(true);
+    expect(await ratingRows(GUEST_A)).toHaveLength(0);
+    expect(await ratingRows(GUEST_B)).toHaveLength(1);
+    expect(await photoAverage()).toBe(4);
+  });
+
+  test('numeric string "0" also clears (truthy-string bypass guard)', async () => {
+    await rate(4);
+    const removed = await rate('0');
+    expect(removed.removed).toBe(true);
+    expect(await ratingRows(GUEST_A)).toHaveLength(0);
+    expect(await photoAverage()).toBe(0);
+  });
+
+  test('malformed rating input never clears an existing rating', async () => {
+    await rate(4);
+    for (const bad of [undefined, null, 'bad', NaN]) {
+      const r = await rate(bad);
+      expect(r.removed).toBeFalsy();
+    }
+    expect(await ratingRows(GUEST_A)).toHaveLength(1);
+  });
+
+  test('clearing deletes racy duplicate rating rows, not just the first', async () => {
+    // Simulate the check-then-insert race: two rating rows for one guest.
+    const row = {
+      photo_id: photoId,
+      event_id: eventId,
+      feedback_type: 'rating',
+      guest_identifier: GUEST_A,
+      is_approved: 1,
+      created_at: new Date().toISOString(),
+      updated_at: new Date().toISOString(),
+    };
+    await db('photo_feedback').insert({ ...row, rating: 3 });
+    await db('photo_feedback').insert({ ...row, rating: 5 });
+    expect(await ratingRows(GUEST_A)).toHaveLength(2);
+
+    const removed = await rate(0);
+    expect(removed.removed).toBe(true);
+    expect(await ratingRows(GUEST_A)).toHaveLength(0);
+    expect(await photoAverage()).toBe(0);
+  });
+
+  test('re-rating with a different value still updates in place', async () => {
+    await rate(3);
+    const updated = await rate(5);
+    expect(updated.updated).toBe(true);
+    const rows = await ratingRows(GUEST_A);
+    expect(rows).toHaveLength(1);
+    expect(rows[0].rating).toBe(5);
+    expect(await photoAverage()).toBe(5);
+  });
+});

backend/src/services/feedbackService.js

@@ -117,6 +117,12 @@ class FeedbackService {
         throw new Error('Invalid reaction');
       }
 
+      // 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,
+      // NaN, null) must never delete an existing rating.
+      const isRatingClear = feedback_type === 'rating' && (rating === 0 || rating === '0');
+
       // Check if similar feedback already exists (prevent duplicates).
       // When a per-person guest_id is present, scope the check to that guest
       // so two guests on the same device can independently like a photo.
@@ -135,6 +141,26 @@ class FeedbackService {
         const existing = await duplicateQuery.first();
 
         if (existing) {
+          // Rating 0 clears the guest's rating (#884) — delete rather
+          // than store 0, which would drag the photo's average down and
+          // still count in total_ratings. Delete the full guest-scoped
+          // set, not existing.id: the check-then-insert above can race
+          // into duplicate rows (same defense as the reaction path), and
+          // clearing must not leave a stray duplicate in the average.
+          if (isRatingClear) {
+            const clearScope = db('photo_feedback').where({
+              photo_id: photoId,
+              event_id: eventId,
+              feedback_type: 'rating',
+            });
+            if (guest_id) clearScope.where('guest_id', guest_id);
+            else clearScope.where('guest_identifier', guestIdentifier);
+            await clearScope.delete();
+
+            await this.updatePhotoFeedbackStats(photoId);
+            return { removed: true };
+          }
+
           if (feedback_type === 'rating' && rating !== existing.rating) {
             // Update existing rating
             await db('photo_feedback')
@@ -199,6 +225,12 @@ class FeedbackService {
         }
       }
 
+      // Rating 0 with no existing rating: nothing to clear — never insert a
+      // 0-rating row (#884).
+      if (isRatingClear) {
+        return { removed: true };
+      }
+
       // Per-guest cap enforcement (#655). Only checked on ADD; toggle-off is
       // always allowed. NULL or 0 stored in the column means "unlimited" —
       // the photographer hasn't opted in to a cap for this event.

backend/src/utils/feedbackValidation.js

@@ -137,11 +137,14 @@ const validateFeedbackSubmission = [
     .isIn(['rating', 'like', 'comment', 'favorite', 'reaction'])
     .withMessage('Invalid feedback type'),
 
-  // Conditional validation based on feedback type
+  // Conditional validation based on feedback type. 0 clears the guest's
+  // existing rating (#884). toInt so a numeric string "0" reaches the
+  // service as a real 0 and hits the removal path.
   body('rating')
     .if(body('feedback_type').equals('rating'))
-    .isInt({ min: 1, max: 5 })
-    .withMessage('Rating must be between 1 and 5'),
+    .isInt({ min: 0, max: 5 })
+    .withMessage('Rating must be between 0 and 5')
+    .toInt(),
 
   // Reactions (#839): fixed curated set only — no free-form emoji.
   body('reaction')

frontend/src/components/gallery/GalleryView.tsx

@@ -783,7 +783,13 @@ export const GalleryView: React.FC<GalleryViewProps> = ({ slug, event }) => {
           photos={filteredPhotos}
           slug={slug}
           categoryId={selectedCategoryId}
-          onFeedbackChange={() => refetch()}
+          onFeedbackChange={() => {
+            refetch();
+            // Guest-mode Rated/Liked filter membership + chip counts come
+            // from my-feedback, not the photo rows (#538) — refresh it too
+            // so e.g. a cleared rating (#884) leaves the Rated filter.
+            queryClient.invalidateQueries({ queryKey: ['my-feedback', slug] });
+          }}
           heroPhotoOverride={staticHeroPhoto}
           feedbackEnabled={feedbackEnabled}
           feedbackOptions={{
@@ -1030,7 +1036,13 @@ export const GalleryView: React.FC<GalleryViewProps> = ({ slug, event }) => {
             photos={filteredPhotos} 
             slug={slug} 
             categoryId={selectedCategoryId}
-            onFeedbackChange={() => refetch()}
+            onFeedbackChange={() => {
+              refetch();
+              // Guest-mode Rated/Liked filter membership + chip counts come
+              // from my-feedback, not the photo rows (#538) — refresh it too
+              // so e.g. a cleared rating (#884) leaves the Rated filter.
+              queryClient.invalidateQueries({ queryKey: ['my-feedback', slug] });
+            }}
             heroPhotoOverride={staticHeroPhoto}
             feedbackEnabled={feedbackEnabled}
             feedbackOptions={{

frontend/src/components/gallery/PhotoLightbox.tsx

@@ -118,6 +118,16 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
     return () => observer.disconnect();
   }, []);
 
+  // Keep the index valid when the photo list shrinks while open (see
+  // currentPhoto fallback below).
+  useEffect(() => {
+    if (photos.length === 0) {
+      onClose();
+    } else if (currentIndex > photos.length - 1) {
+      setCurrentIndex(photos.length - 1);
+    }
+  }, [photos.length, currentIndex]);
+
 
   // Save-aware download. On mobile (where Web Share + files is supported)
   // this opens the OS share sheet so "Save to Photos" actually lands in
@@ -125,7 +135,12 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
   // otherwise have to chain Files → unzip → save (#531). Desktop and
   // unsupported browsers fall through to a regular <a download>.
   const downloadPhotoMutation = useSavePhotoToDevice();
-  const currentPhoto = photos[currentIndex];
+  // Fall back to the last photo when the list shrinks under us: clearing
+  // your rating under the "Rated" feedback filter (#884) — like unliking
+  // under "Likes" — refetches the gallery and can drop the current photo,
+  // leaving currentIndex past the end. The effect below re-syncs the
+  // index (or closes the lightbox when nothing is left).
+  const currentPhoto = photos[currentIndex] ?? photos[photos.length - 1];
   // Per-category download permission (#640). AND'd with the event-level
   // allowDownloads — disabling at either level hides the download button.
   // Defaults true for uncategorised photos and pre-migration-135 categories.
@@ -241,7 +256,7 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
     let mounted = true;
     (async () => {
       try {
-        if (!feedbackSettings?.feedback_enabled) return;
+        if (!feedbackSettings?.feedback_enabled || !currentPhoto) return;
         const data = await feedbackService.getPhotoFeedback(slug, String(currentPhoto.id));
         if (!mounted) return;
         setMyLiked(!!data.my_feedback.liked);
@@ -254,7 +269,7 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
       }
     })();
     return () => { mounted = false; };
-  }, [slug, currentPhoto.id, feedbackSettings?.feedback_enabled]);
+  }, [slug, currentPhoto?.id, feedbackSettings?.feedback_enabled]);
 
   const submitLike = async () => {
     // Guest identity mode: ensure we have a per-person guest token. The
@@ -629,6 +644,12 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
     `fixed inset-0 bg-black z-50 flex items-center justify-center protected-image protection-${protectionLevel}` :
     'fixed inset-0 bg-black z-50 flex items-center justify-center';
 
+  // Empty list (last photo dropped out of the current filter): the effect
+  // above is about to close the lightbox — render nothing meanwhile.
+  if (!currentPhoto) {
+    return null;
+  }
+
   const desktopFeedbackWidth = 416; // 26rem; keep in sync with panel width
   const isDesktopFeedback = showFeedback && !isSmallScreen;
 
@@ -777,10 +798,12 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
                 {[1,2,3,4,5].map((i) => (
                   <button
                     key={i}
-                    onClick={() => submitRating(i)}
+                    // Clicking the current rating again clears it (#884) —
+                    // 0 tells the backend to delete the guest's rating.
+                    onClick={() => submitRating(i === myRating ? 0 : i)}
                     className="p-1"
-                    aria-label={`Rate ${i} star${i>1?'s':''}`}
-                    title={`Rate ${i}`}
+                    aria-label={i === myRating ? 'Remove rating' : `Rate ${i} star${i>1?'s':''}`}
+                    title={i === myRating ? 'Remove rating' : `Rate ${i}`}
                   >
                     <Star className={`w-5 h-5 ${myRating >= i ? 'text-yellow-400 fill-yellow-400' : 'text-white/70'}`} />
                   </button>
@@ -1041,7 +1064,9 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
               // Per-guest cap reached (#655) on the post-identity-modal submit.
               if (!handleLimitError(err)) throw err;
             }
-          } else if (pendingAction?.type === 'rating' && pendingAction.rating) {
+          } else if (pendingAction?.type === 'rating' && typeof pendingAction.rating === 'number') {
+            // Explicit number check above: a pending rating of 0 (= clear
+            // my rating, #884) must still be submitted.
             await feedbackService.submitFeedback(slug, String(currentPhoto.id), {
               feedback_type: 'rating',
               rating: pendingAction.rating,
@@ -1049,6 +1074,14 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
               guest_email: email,
             });
             setMyRating(pendingAction.rating);
+            // Refresh the visible average/count — parity with the direct
+            // submit paths, and required for a clear (#884) so the old
+            // average doesn't linger until the photo is reopened.
+            try {
+              const fresh = await feedbackService.getPhotoFeedback(slug, String(currentPhoto.id));
+              setAvgRating(Number(fresh.summary?.average_rating) || 0);
+              setTotalRatings(Number(fresh.summary?.total_ratings) || 0);
+            } catch {}
           }
           // Sync gallery photo list (feedback filter chips) — parity with
           // the direct submit paths.

frontend/src/components/gallery/PhotoRating.tsx

@@ -56,6 +56,12 @@ export const PhotoRating: React.FC<PhotoRatingProps> = ({
     },
     onSuccess: () => {
       queryClient.invalidateQueries({ queryKey: ['photo-feedback', gallerySlug, photoId] });
+      // The parent refetch fired optimistically in onMutate, i.e. possibly
+      // before the server applied the change — refresh gallery-level state
+      // again now that it has, so the average/Rated-filter membership
+      // reflect the accepted value (matters most for a clear, #884).
+      queryClient.invalidateQueries({ queryKey: ['gallery-photos', gallerySlug] });
+      queryClient.invalidateQueries({ queryKey: ['my-feedback', gallerySlug] });
       toast.success(t('feedback.ratingSubmitted', 'Rating submitted'));
     },
     onError: (error: any) => {

frontend/src/components/gallery/layouts/GalleryStoryLayout.tsx

@@ -207,15 +207,21 @@ export const GalleryStoryLayout: React.FC<GalleryStoryLayoutProps> = ({
   const handleRate = useCallback(async (rating: number) => {
     if (!selectedPhotoForFeedback) return;
 
+    // Clicking the star you already gave clears the rating (#884) —
+    // 0 tells the backend to delete it. Session-local `ratings` is the
+    // source of truth for "my rating" here, never the photo's average.
+    const current = ratings[selectedPhotoForFeedback.id] || 0;
+    const next = rating === current ? 0 : rating;
+
     setRatings(prev => ({
       ...prev,
-      [selectedPhotoForFeedback.id]: rating
+      [selectedPhotoForFeedback.id]: next
     }));
 
     try {
       await feedbackService.submitFeedback(slug, String(selectedPhotoForFeedback.id), {
         feedback_type: 'rating',
-        rating: rating,
+        rating: next,
         guest_name: savedIdentity?.name,
         guest_email: savedIdentity?.email,
       });
@@ -223,7 +229,7 @@ export const GalleryStoryLayout: React.FC<GalleryStoryLayoutProps> = ({
     } catch (err) {
       console.warn('Rating submit failed', err);
     }
-  }, [selectedPhotoForFeedback, slug, savedIdentity, onFeedbackChange]);
+  }, [selectedPhotoForFeedback, ratings, slug, savedIdentity, onFeedbackChange]);
 
   const handleDownloadAll = useCallback(async () => {
     toast.info(t('gallery.downloading', { count: photos.length }));
@@ -388,7 +394,7 @@ export const GalleryStoryLayout: React.FC<GalleryStoryLayoutProps> = ({
           onClose={handleCloseFeedback}
           photo={selectedPhotoForFeedback}
           comments={selectedPhotoForFeedback ? (comments[selectedPhotoForFeedback.id] || []) : []}
-          rating={selectedPhotoForFeedback ? (ratings[selectedPhotoForFeedback.id] || selectedPhotoForFeedback.average_rating || 0) : 0}
+          rating={selectedPhotoForFeedback ? (ratings[selectedPhotoForFeedback.id] ?? (selectedPhotoForFeedback.average_rating || 0)) : 0}
           onAddComment={handleAddComment}
           onRate={handleRate}
           requireNameEmail={feedbackOptions?.requireNameEmail}