fix(analytics): make per-photo view/download counters actually count (#895) (stable) (#905)

* fix(analytics): make per-photo view/download counters actually count (#895) (stable) Three stacked defects behind 'per-image stats stay 0': - photos.view_count had NO writer anywhere — the admin IMAGES table and photo viewer display it, so it was permanently 0. It now increments when the full-size photo or its preview tier is served, excluding the slideshow kiosk (migration 138 design) and follow-up video Range requests (seeks are not views). Fire-and-forget so analytics can never fail the byte-serving path. - Zip downloads (download-all, presigned download-all, download-selected) never incremented per-photo download_count — only single-photo downloads did, so zip-heavy galleries showed 0 forever. The zip routes now bump exactly the photos that went into the archive (the prebuilt-zip path mirrors the archive builders' category filter). - Every admin surface used a different definition of 'downloads', which is the reporter's 46 vs 45 vs 0: event details counted only action='download' (no zips at all), the dashboard counted download+download_all but silently EXCLUDED download_selected and download_all_presigned. All queries now share one action set: download, download_all, download_all_presigned, download_selected. New photoEngagementCounters suite pins all of it (7 tests). * fix(analytics): count views via an explicit lightbox beacon (#895 review round) External review flagged that request-level view counting is wrong in both directions: the lightbox preloads prev/next neighbours (3 fetches per open) while a preloaded neighbour promoted by a swipe is never re-fetched (#505 keeps the DOM node), and enhanced/maximum galleries never hit /photo at all (bytes come from /api/secure-images). - Views now count via POST /:slug/photo/:photoId/view, fired by the lightbox exactly when a photo becomes the visible slide; the serving-route increments are removed. Covers protected galleries and the preview tier uniformly; slideshow kiosk stays excluded. - bumpEventDownloadCounts mirrors downloadZipService._build (ALL event photos) — the category filter mismatched the prebuilt zip's actual contents. (That the builder ignores per-category allow_downloads is a separate pre-existing issue.) - Zip loops count only successfully appended entries, with a pre-append storage stat: a lazy stream's async error bypassed the per-photo catch and hung the whole response — pre-existing bug, now fixed. Suite extended to 9 tests (beacon semantics, serve-does-not-count, skipped-entry exclusion). * fix(analytics): fire the view beacon from the premium lightbox too (#895 review round 2) gallery-premium events use yet-another-react-lightbox inside GalleryPremiumLayout instead of PhotoLightbox, so the layout never counted views. yarl's on.view fires on open and on every slide change — identical semantics to the PhotoLightbox beacon. Also documents the accepted prebuilt-zip approximation: _build can skip entries whose watermark step fails and still publish the archive; counting those exactly would need a persisted zip manifest. * perf(analytics): skip the per-entry zip preflight on S3 (#895 review round 3) The pre-append source check exists for LocalFs's lazy createReadStream (async error would kill the whole zip response). S3's get() awaits GetObject and rejects inside the loop's try/catch on a missing key, so a HEAD per entry was a redundant serial round trip — 500 extra HEADs on a 500-photo zip. --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
2026-07-29 17:59:06 +02:00
parent 1ad8ad5b68
commit 90b589a88e
7 changed files with 413 additions and 8 deletions
@@ -0,0 +1,277 @@
+/**
+ * Per-photo engagement counters (#895).
+ *
+ * Pins the contract that the admin EVENT > IMAGES table depends on:
+ *  - photos.view_count increments when the full-size photo is served
+ *    (it existed in the schema + admin UI but had NO writer at all)
+ *  - the slideshow kiosk never increments views (migration 138 design)
+ *  - single-photo downloads increment download_count (regression pin)
+ *  - zip downloads (download-all, download-selected) increment
+ *    download_count for the contained photos — previously they didn't,
+ *    so zip-heavy galleries showed 0 per-photo downloads forever
+ *  - the admin event-detail total_downloads counts singles AND zips
+ *    (it counted action='download' only, disagreeing with the dashboard)
+ */
+
+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-engagement-')), 'db.sqlite',
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'engagement-test-secret';
+// Real files on disk so /photo and the zip routes actually stream bytes.
+process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-engagement-storage-'));
+
+const request = require('supertest');
+const express = require('express');
+const cookieParser = require('cookie-parser');
+const bcrypt = require('bcrypt');
+const jwt = require('jsonwebtoken');
+
+const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
+
+const SLUG = 'engagement-test-event';
+
+describe('photo engagement counters (#895)', () => {
+  let db;
+  let cleanup;
+  let app;
+  let eventId;
+  let photoIds;
+  let adminToken;
+
+  const galleryToken = (extra = {}) => jwt.sign(
+    { eventId, eventSlug: SLUG, type: 'gallery', ...extra },
+    process.env.JWT_SECRET,
+    { expiresIn: '1h', issuer: 'picpeak-auth' }
+  );
+
+  const getPhoto = async (id) => db('photos').where('id', id).first();
+  // The counter writes are fire-and-forget on purpose — give the event
+  // loop a beat before asserting.
+  const settle = () => new Promise((r) => setTimeout(r, 100));
+
+  beforeAll(async () => {
+    ({ db, cleanup } = await bootCrmDb());
+    await seedMinimal(db);
+
+    const inserted = await db('events').insert({
+      slug: SLUG,
+      event_type: 'wedding',
+      event_name: 'Engagement Test',
+      event_date: '2026-08-01',
+      host_email: 'host@example.com',
+      admin_email: 'admin@example.com',
+      password_hash: 'x',
+      share_link: `/gallery/${SLUG}/share`,
+      share_token: 'engagement-test-share',
+      expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
+      is_active: 1,
+      is_archived: 0,
+      is_draft: 0,
+      allow_downloads: 1,
+      created_at: new Date().toISOString(),
+    }).returning('id');
+    eventId = inserted[0]?.id ?? inserted[0];
+
+    const photoDir = path.join(process.env.STORAGE_PATH, 'events/active', SLUG);
+    fs.mkdirSync(photoDir, { recursive: true });
+
+    photoIds = [];
+    for (let i = 0; i < 3; i++) {
+      const filename = `photo-${i}.jpg`;
+      fs.writeFileSync(path.join(photoDir, filename), Buffer.from(`fake-jpeg-bytes-${i}`));
+      const p = await db('photos').insert({
+        event_id: eventId,
+        filename,
+        path: `${SLUG}/${filename}`,
+        type: 'individual',
+        uploaded_at: new Date().toISOString(),
+      }).returning('id');
+      photoIds.push(p[0]?.id ?? p[0]);
+    }
+
+    const superRole = await db('roles').where({ name: 'super_admin' }).first();
+    const [rootId] = await db('admin_users').insert({
+      username: 'engagement-admin',
+      email: 'engagement-admin@example.com',
+      password_hash: await bcrypt.hash('EngagementAdmin123', 4),
+      role_id: superRole.id,
+      is_active: 1,
+      created_at: new Date(),
+      updated_at: new Date(),
+    }).returning('id').then((r) => [r[0]?.id || r[0]]);
+    adminToken = jwt.sign(
+      { id: rootId, username: 'engagement-admin', type: 'admin', role: 'super_admin', loginTime: Date.now() },
+      process.env.JWT_SECRET,
+      { expiresIn: '1h', issuer: 'picpeak-auth' }
+    );
+
+    app = express();
+    app.use(express.json());
+    app.use(cookieParser());
+    app.use('/api/gallery', require('../../src/routes/gallery'));
+    app.use('/api/admin/events', require('../../src/routes/adminEvents'));
+  }, 120000);
+
+  afterAll(async () => {
+    if (cleanup) await cleanup();
+  });
+
+  beforeEach(async () => {
+    await db('photos').where('event_id', eventId).update({ view_count: 0, download_count: 0 });
+    await db('access_logs').where('event_id', eventId).del();
+  });
+
+  describe('view_count via the view beacon (#895 — previously never written)', () => {
+    const beacon = (photoId, token = galleryToken()) => request(app)
+      .post(`/api/gallery/${SLUG}/photo/${photoId}/view`)
+      .set('Authorization', `Bearer ${token}`);
+
+    it('increments exactly the beaconed photo', async () => {
+      expect((await beacon(photoIds[0])).status).toBe(204);
+      expect((await getPhoto(photoIds[0])).view_count).toBe(1);
+
+      expect((await beacon(photoIds[0])).status).toBe(204);
+      expect((await getPhoto(photoIds[0])).view_count).toBe(2);
+      // Other photos untouched
+      expect((await getPhoto(photoIds[1])).view_count).toBe(0);
+    });
+
+    it('serving the image bytes does NOT count (preloads must not inflate)', async () => {
+      const res = await request(app)
+        .get(`/api/gallery/${SLUG}/photo/${photoIds[0]}`)
+        .set('Authorization', `Bearer ${galleryToken()}`);
+      expect(res.status).toBe(200);
+      await settle();
+      expect((await getPhoto(photoIds[0])).view_count).toBe(0);
+    });
+
+    it('rejects the slideshow kiosk (migration 138 design)', async () => {
+      const res = await beacon(photoIds[0], galleryToken({ accessLevel: 'slideshow' }));
+      expect(res.status).toBeGreaterThanOrEqual(400);
+      expect((await getPhoto(photoIds[0])).view_count).toBe(0);
+    });
+
+    it("404s a photo that isn't in the event", async () => {
+      const res = await beacon(999999);
+      expect(res.status).toBe(404);
+    });
+  });
+
+  describe('download_count', () => {
+    it('single-photo download increments (regression pin)', async () => {
+      const res = await request(app)
+        .get(`/api/gallery/${SLUG}/download/${photoIds[0]}`)
+        .set('Authorization', `Bearer ${galleryToken()}`);
+      expect(res.status).toBe(200);
+      await settle();
+      expect((await getPhoto(photoIds[0])).download_count).toBe(1);
+      expect((await getPhoto(photoIds[1])).download_count).toBe(0);
+    });
+
+    it('download-selected increments exactly the selected photos (#895)', async () => {
+      const res = await request(app)
+        .post(`/api/gallery/${SLUG}/download-selected`)
+        .set('Authorization', `Bearer ${galleryToken()}`)
+        .send({ photo_ids: [photoIds[0], photoIds[1]] });
+      expect(res.status).toBe(200);
+      await settle();
+      expect((await getPhoto(photoIds[0])).download_count).toBe(1);
+      expect((await getPhoto(photoIds[1])).download_count).toBe(1);
+      expect((await getPhoto(photoIds[2])).download_count).toBe(0);
+    });
+
+    it('download-all increments every downloadable photo (#895)', async () => {
+      const res = await request(app)
+        .get(`/api/gallery/${SLUG}/download-all`)
+        .set('Authorization', `Bearer ${galleryToken()}`);
+      expect(res.status).toBe(200);
+      await settle();
+      for (const id of photoIds) {
+        expect((await getPhoto(id)).download_count).toBe(1);
+      }
+    });
+
+    it('skipped archive entries do not count (missing source file)', async () => {
+      // Own event so the on-the-fly archiver path is guaranteed — the
+      // main event may have a cached zip from the previous test's
+      // background generation, and racing its build/invalidate hangs.
+      const slug2 = `${SLUG}-skip`;
+      const ev = await db('events').insert({
+        slug: slug2,
+        event_type: 'wedding',
+        event_name: 'Engagement Skip Test',
+        event_date: '2026-08-01',
+        host_email: 'host@example.com',
+        admin_email: 'admin@example.com',
+        password_hash: 'x',
+        share_link: `/gallery/${slug2}/share`,
+        share_token: 'engagement-skip-share',
+        expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
+        is_active: 1,
+        is_archived: 0,
+        is_draft: 0,
+        allow_downloads: 1,
+        created_at: new Date().toISOString(),
+      }).returning('id');
+      const eventId2 = ev[0]?.id ?? ev[0];
+      const dir2 = path.join(process.env.STORAGE_PATH, 'events/active', slug2);
+      fs.mkdirSync(dir2, { recursive: true });
+      const ids2 = [];
+      for (let i = 0; i < 2; i++) {
+        // Only photo 0 gets a real file — photo 1's source is missing.
+        if (i === 0) fs.writeFileSync(path.join(dir2, `photo-${i}.jpg`), Buffer.from('skip-test-bytes'));
+        const p = await db('photos').insert({
+          event_id: eventId2,
+          filename: `photo-${i}.jpg`,
+          path: `${slug2}/photo-${i}.jpg`,
+          type: 'individual',
+          uploaded_at: new Date().toISOString(),
+        }).returning('id');
+        ids2.push(p[0]?.id ?? p[0]);
+      }
+      const token2 = jwt.sign(
+        { eventId: eventId2, eventSlug: slug2, type: 'gallery' },
+        process.env.JWT_SECRET,
+        { expiresIn: '1h', issuer: 'picpeak-auth' }
+      );
+
+      const res = await request(app)
+        .get(`/api/gallery/${slug2}/download-all`)
+        .set('Authorization', `Bearer ${token2}`);
+      expect(res.status).toBe(200);
+      await settle();
+      expect((await db('photos').where('id', ids2[0]).first()).download_count).toBe(1);
+      // photo-1's source was missing → skipped from the zip → not counted
+      expect((await db('photos').where('id', ids2[1]).first()).download_count).toBe(0);
+    });
+  });
+
+  describe('admin event-detail total_downloads (#895 — one definition everywhere)', () => {
+    it('counts singles and every zip variant, one row each', async () => {
+      const row = (action) => ({
+        event_id: eventId,
+        ip_address: '127.0.0.1',
+        user_agent: 'jest',
+        action,
+      });
+      await db('access_logs').insert([
+        row('download'),
+        row('download_all'),
+        row('download_all_presigned'),
+        row('download_selected'),
+        row('view'), // not a download
+      ]);
+
+      const res = await request(app)
+        .get(`/api/admin/events/${eventId}`)
+        .set('Authorization', `Bearer ${adminToken}`);
+      expect(res.status).toBe(200);
+      expect(res.body.total_downloads).toBe(4);
+    });
+  });
+});
@@ -69,7 +69,7 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,

    // Get total downloads (last 30 days) - include both single and bulk downloads
    const totalDownloads = await db('access_logs')
-      .whereIn('action', ['download', 'download_all'])
+      .whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
      .where('timestamp', '>=', thirtyDaysAgo.toISOString())
      .count('id as count')
      .first();
@@ -99,7 +99,7 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,
      .first();

    const previousDownloads = await db('access_logs')
-      .whereIn('action', ['download', 'download_all'])
+      .whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
      .where('timestamp', '>=', sixtyDaysAgo.toISOString())
      .where('timestamp', '<', thirtyDaysAgo.toISOString())
      .count('id as count')
@@ -271,7 +271,7 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
    // Get downloads per day - include both single and bulk downloads
    const downloadsData = await db('access_logs')
      .select(db.raw('DATE(timestamp) as date'), db.raw('COUNT(*) as count'))
-      .whereIn('action', ['download', 'download_all'])
+      .whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
      .where('timestamp', '>=', startDateStr)
      .groupByRaw('DATE(timestamp)');

@@ -307,7 +307,7 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
      .select('events.id', 'events.event_name', 'events.slug')
      .select(db.raw('COUNT(CASE WHEN action = \'view\' THEN 1 END) as views'))
      .select(db.raw('COUNT(DISTINCT CASE WHEN action = \'view\' THEN ip_address END) as uniqueVisitors'))
-      .select(db.raw('COUNT(CASE WHEN action IN (\'download\', \'download_all\') THEN 1 END) as downloads'))
+      .select(db.raw('COUNT(CASE WHEN action IN (\'download\', \'download_all\', \'download_all_presigned\', \'download_selected\') THEN 1 END) as downloads'))
      .join('events', 'access_logs.event_id', 'events.id')
      .where('access_logs.timestamp', '>=', startDateStr)
      .groupBy('events.id', 'events.event_name', 'events.slug')
@@ -377,7 +377,7 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
      .first();

    const totalDownloadsCount = await db('access_logs')
-      .whereIn('action', ['download', 'download_all'])
+      .whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
      .where('timestamp', '>=', startDateStr)
      .count('id as count')
      .first();
@@ -771,9 +771,11 @@ module.exports = (router) => {
        .where('action', 'view')
        .count('* as totalViews');

+      // One row per download event: singles AND zips (#895). Must stay in
+      // sync with adminDashboard's definition or the two surfaces disagree.
      const [{ totalDownloads }] = await db('access_logs')
        .where('event_id', id)
-        .where('action', 'download')
+        .whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
        .count('* as totalDownloads');

      const [{ uniqueVisitors }] = await db('access_logs')
@@ -933,6 +933,22 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
 });

 // Download all photos as ZIP
+// Zip downloads count toward each contained photo's download_count (#895)
+// — previously only single-photo downloads did, so galleries whose guests
+// grab the zip showed 0 per-photo downloads forever. Used by the
+// pre-generated-zip branches only: it mirrors downloadZipService._build,
+// which zips EVERY event photo with no per-category allow_downloads
+// filter — the counter has to reflect what actually shipped. (That the
+// prebuilt zip ignores per-category download opt-outs is a separate,
+// pre-existing issue.) Known approximation: _build skips entries whose
+// WATERMARK step fails and still publishes the zip; counting those
+// would need a persisted archive manifest, which isn't worth it for
+// that tail case. Fire-and-forget at the call sites: counters must
+// never fail a download.
+async function bumpEventDownloadCounts(eventId) {
+  await db('photos').where('event_id', eventId).increment('download_count', 1);
+}
+
 router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async (req, res) => {
  try {
    // Check if downloads are allowed for this event
@@ -962,6 +978,7 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
            user_agent: req.headers['user-agent'],
            action: 'download_all_presigned'
          }).catch(() => {});
+          bumpEventDownloadCounts(req.event.id).catch(() => {});
          res.redirect(302, url);
          return;
        } catch (err) {
@@ -985,6 +1002,7 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
        user_agent: req.headers['user-agent'],
        action: 'download_all'
      }).catch(() => {});
+      bumpEventDownloadCounts(req.event.id).catch(() => {});
      return;
    }

@@ -1044,6 +1062,10 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
    // get a deterministic `_1` suffix before the entries hit the archive.
    const useOriginalBulk = await getUseOriginalFilenames();
    const bulkEntryNames = getZipEntryNames(photos, useOriginalBulk);
+    // Only photos whose append succeeded count as downloaded (#895) — the
+    // catch below deliberately skips missing/corrupt sources, and those
+    // never make it into the archive.
+    const appendedIds = [];
    for (let i = 0; i < photos.length; i += 1) {
      const photo = photos[i];
      const storageKey = resolvePhotoStorageKey(req.event, photo);
@@ -1057,6 +1079,22 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
      }

      try {
+        // Verify the source exists BEFORE appending — but only for local
+        // sources: fs.createReadStream is lazy, so its error fires outside
+        // this try/catch and the archive 'error' handler then kills the
+        // whole response instead of skipping one photo (#895 review). S3's
+        // get() awaits GetObject and rejects right here on a missing key,
+        // so a preflight HEAD per entry would just be a redundant serial
+        // round trip (500-photo zip = 500 extra HEADs).
+        if (storageKey && storage.kind() === 'local') {
+          const srcStat = await storage.stat(storageKey);
+          if (!srcStat) {
+            throw new Error(`Photo missing in storage: ${storageKey}`);
+          }
+        } else if (!storageKey && !fs.existsSync(resolvePhotoFilePath(req.event, photo))) {
+          throw new Error('Photo file missing on disk');
+        }
+
        if (shouldApplyWatermark && effectiveSettings) {
          // Watermark service operates on a local path. For managed photos in
          // S3 mode, materialize a tmp local copy first.
@@ -1076,9 +1114,9 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
          const stream = await storage.get(storageKey);
          archive.append(stream, { name: archiveName });
        } else {
-          const filePath = resolvePhotoFilePath(req.event, photo);
-          archive.file(filePath, { name: archiveName });
+          archive.file(resolvePhotoFilePath(req.event, photo), { name: archiveName });
        }
+        appendedIds.push(photo.id);
      } catch (err) {
        logger.warn('Skipping photo in bulk download due to error', {
          slug: req.params.slug,
@@ -1098,6 +1136,12 @@ router.get('/:slug/download-all', verifyGalleryAccess, denySlideshowToken, async
      user_agent: req.headers['user-agent'],
      action: 'download_all'
    });
+    // Exactly the photos that made it into this archive (#895) — skipped
+    // (missing/corrupt) sources don't count.
+    if (appendedIds.length > 0) {
+      db('photos').whereIn('id', appendedIds)
+        .increment('download_count', 1).catch(() => {});
+    }
  } catch (error) {
    errorResponse(res, error, 500, 'Failed to create download archive');
  }
@@ -1179,11 +1223,26 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
    // #493: same display-name resolution as bulk download, with dedup.
    const useOriginalSelected = await getUseOriginalFilenames();
    const selectedEntryNames = getZipEntryNames(photos, useOriginalSelected);
+    // Only photos whose append succeeded count as downloaded (#895).
+    const appendedIds = [];
    for (let i = 0; i < photos.length; i += 1) {
      const photo = photos[i];
      const name = selectedEntryNames[i] || `photo-${photo.id}.jpg`;
      const storageKey = resolveSelectedKey(req.event, photo);
      try {
+        // Same pre-append source check as download-all (#895 review),
+        // local backend only: a lazy fs stream's async error would kill
+        // the response instead of skipping the photo; S3's get() rejects
+        // at the await below, so no redundant per-entry HEAD there.
+        if (storageKey && selectedStorage.kind() === 'local') {
+          const srcStat = await selectedStorage.stat(storageKey);
+          if (!srcStat) {
+            throw new Error(`Photo missing in storage: ${storageKey}`);
+          }
+        } else if (!storageKey && !fs.existsSync(resolvePhotoFilePath(req.event, photo))) {
+          throw new Error('Photo file missing on disk');
+        }
+
        if (shouldApplyWatermark && effectiveSettings) {
          const buf = storageKey
            ? await withSelectedLocalCopy(storageKey, (lp) =>
@@ -1197,6 +1256,7 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
        } else {
          archive.file(resolvePhotoFilePath(req.event, photo), { name });
        }
+        appendedIds.push(photo.id);
      } catch (err) {
        logger.warn('Skipping selected photo due to error', {
          slug: req.params.slug,
@@ -1215,12 +1275,49 @@ router.post('/:slug/download-selected', verifyGalleryAccess, denySlideshowToken,
      user_agent: req.headers['user-agent'],
      action: 'download_selected'
    });
+    // Exactly the photos that made it into this archive (#895) — skipped
+    // (missing/corrupt) sources don't count.
+    if (appendedIds.length > 0) {
+      db('photos').whereIn('id', appendedIds)
+        .increment('download_count', 1).catch(() => {});
+    }
  } catch (error) {
    errorResponse(res, error, 500, 'Failed to download selected photos');
  }
 });


+// Explicit per-photo view beacon (#895). Counting views on the image-
+// serving routes is wrong in both directions: the lightbox preloads the
+// prev/next neighbours (three fetches per open), while a preloaded
+// neighbour that becomes the current slide is never re-fetched (#505
+// keeps the DOM node alive across the swipe) — so request-level counters
+// overcount preloads AND undercount swipe-throughs. Instead the lightbox
+// pings this endpoint exactly when a photo becomes the visible slide.
+// This also covers enhanced/maximum-protection galleries, whose bytes
+// are served by /api/secure-images and never pass the routes below.
+// The slideshow kiosk is excluded (denySlideshowToken; migration 138).
+router.post('/:slug/photo/:photoId/view',
+  verifyGalleryAccess,
+  denySlideshowToken,
+  async (req, res) => {
+    try {
+      const photo = await db('photos')
+        .where({ id: req.params.photoId, event_id: req.event.id })
+        .first('id', 'visibility');
+      if (!photo) {
+        return res.status(404).json({ error: 'Photo not found' });
+      }
+      if (photo.visibility === 'hidden' && req.accessLevel !== 'client') {
+        return res.status(403).json({ error: 'Photo not available' });
+      }
+      await db('photos').where('id', photo.id).increment('view_count', 1);
+      res.status(204).end();
+    } catch (error) {
+      errorResponse(res, error, 500, 'Failed to record view');
+    }
+  });
+
 // View single photo (with watermark if enabled)
 router.get('/:slug/photo/:photoId',
  verifyGalleryAccess,
@@ -6,6 +6,7 @@ import { useSavePhotoToDevice } from '../../hooks/useGallery';
 import { AuthenticatedImage } from '../common';
 import { PhotoFeedback } from './PhotoFeedback';
 import { feedbackService } from '../../services/feedback.service';
+import { galleryService } from '../../services/gallery.service';
 import { FeedbackIdentityModal } from './FeedbackIdentityModal';
 import { VideoPlayer } from './VideoPlayer';
 import { useGuestIdentityOptional } from '../../contexts/GuestIdentityContext';
@@ -102,6 +103,17 @@ export const PhotoLightbox: React.FC<PhotoLightboxProps> = ({
    return () => window.removeEventListener('resize', onResize);
  }, []);

+  // View beacon (#895): count exactly the photo that became the visible
+  // slide. The image fetches themselves can't be counted — preloaded
+  // neighbours would inflate, and a neighbour promoted by a swipe is
+  // never re-fetched (#505).
+  const currentPhotoId = photos[currentIndex]?.id;
+  useEffect(() => {
+    if (currentPhotoId !== undefined) {
+      galleryService.trackPhotoView(slug, currentPhotoId);
+    }
+  }, [slug, currentPhotoId]);
+

  // Save-aware download. On mobile (where Web Share + files is supported)
  // this opens the OS share sheet so "Save to Photos" actually lands in
@@ -563,6 +563,14 @@ export const GalleryPremiumLayout: React.FC<GalleryPremiumLayoutProps> = ({
        close={() => setLightboxIndex(-1)}
        index={lightboxIndex}
        slides={slides}
+        // View beacon (#895): yarl fires `view` on open and on every
+        // slide change — same semantics as PhotoLightbox's beacon.
+        on={{
+          view: ({ index }) => {
+            const photo = filteredPhotos[index];
+            if (photo) galleryService.trackPhotoView(slug, photo.id);
+          },
+        }}
        plugins={[
          Thumbnails,
          Zoom,
@@ -198,6 +198,15 @@ export const galleryService = {
    this.triggerBrowserDownload(fetched.blob, fetched.serverFilename || filename);
  },

+  // Per-photo view beacon (#895). Fired by the lightbox when a photo
+  // becomes the visible slide — request-level counting on the image
+  // endpoints can't tell the current slide from its preloaded
+  // neighbours. Fire-and-forget: view counting must never surface an
+  // error to the guest.
+  trackPhotoView(slug: string, photoId: number): void {
+    api.post(`/gallery/${slug}/photo/${photoId}/view`).catch(() => {});
+  },
+
  // Download all photos as ZIP
  // When a pre-generated zip is available, use native browser download (Content-Length → progress bar).
  // Otherwise fall back to blob download.