Compare commits

...

2 Commits

Author SHA1 Message Date
Paul Nothaft f99357460f chore(stable): release 3.45.9 (#907)
Build and Push Docker Images / build-backend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-backend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-backend (push) Blocked by required conditions
Build and Push Docker Images / build-frontend (linux/amd64, ubuntu-latest) (push) Waiting to run
Build and Push Docker Images / build-frontend (linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Build and Push Docker Images / merge-frontend (push) Blocked by required conditions
Build and Push Docker Images / summary (push) Blocked by required conditions
2026-07-29 16:02:51 +00:00
Paul Nothaft 90b589a88e 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
11 changed files with 423 additions and 11 deletions
+1 -1
View File
@@ -1 +1 @@
{".":"3.45.8"}
{".":"3.45.9"}
+7
View File
@@ -5,6 +5,13 @@ All notable changes to PicPeak will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [3.45.9](https://github.com/PicPeak/picpeak/compare/v3.45.8...v3.45.9) (2026-07-29)
### Bug Fixes
* **analytics:** make per-photo view/download counters actually count ([#895](https://github.com/PicPeak/picpeak/issues/895)) (stable) ([#905](https://github.com/PicPeak/picpeak/issues/905)) ([90b589a](https://github.com/PicPeak/picpeak/commit/90b589a88e4c56ccac6dba86c48a604f2abcc008))
## [3.45.8](https://github.com/PicPeak/picpeak/compare/v3.45.7...v3.45.8) (2026-07-29)
@@ -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);
});
});
});
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "picpeak-backend",
"version": "3.45.8",
"version": "3.45.9",
"description": "Backend for PicPeak event photo sharing platform",
"main": "server.js",
"engines": {
+5 -5
View File
@@ -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();
+3 -1
View File
@@ -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')
+99 -2
View File
@@ -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,
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "picpeak-frontend",
"private": true,
"version": "3.45.8",
"version": "3.45.9",
"type": "module",
"scripts": {
"dev": "vite",
@@ -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,
+9
View File
@@ -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.