From 15c844db067de7bc04a88ddd407f3ed8f5df0fe2 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:05:21 +0200 Subject: [PATCH] fix(admin): the "Uncategorized" photo filter returns every photo (#1211) (#1215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stable twin of the same fix on main. The dropdown rendered as `value="0"` and adminPhotos.js:1001 skips '0', so no category condition was applied and the whole event came back. The branch that does the work sits four lines below, keyed on the literal 'uncategorized' that nothing was sending. Silent by nature — a full list reads as 'nothing to narrow' rather than 'the filter did not run' — which is why it survived this long. The reporter in #1209 is on 3.46.4, so this is the branch that reaches them. Tests both ends of the contract, since the bug was the pairing rather than either half. Co-authored-by: Paul Nothaft --- .../integration/uncategorizedFilter.test.js | 107 ++++++++++++++++++ .../src/components/admin/PhotoFilters.tsx | 7 +- .../PhotoFilters.uncategorized.test.tsx | 84 ++++++++++++++ 3 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 backend/__tests__/integration/uncategorizedFilter.test.js create mode 100644 frontend/src/components/admin/__tests__/PhotoFilters.uncategorized.test.tsx diff --git a/backend/__tests__/integration/uncategorizedFilter.test.js b/backend/__tests__/integration/uncategorizedFilter.test.js new file mode 100644 index 00000000..2d6eecbf --- /dev/null +++ b/backend/__tests__/integration/uncategorizedFilter.test.js @@ -0,0 +1,107 @@ +/** + * The admin photo list's category filter, and the value it answers to (#1211). + * + * The frontend used to send `category_id=0` for "Uncategorized". This route + * skips `'0'` outright — the guard reads `category_id !== '0'` — so no + * condition was applied and the whole event came back. Four lines below that + * guard sits the branch that does the work, keyed on the literal + * `uncategorized`, which nothing was sending. + * + * Reported in #1209 by someone trying to isolate a few thousand uncategorised + * imports. The frontend half is fixed in PhotoFilters; this pins the backend + * half of the same contract, because the failure mode was the two ends + * disagreeing about a string and neither one being wrong on its own. + */ + +const request = require('supertest'); +const express = require('express'); + +const { bootCrmDb, seedMinimal } = require('./helpers/crmDb'); + +describe('admin photo list — uncategorized filter (#1211)', () => { + let db; let cleanup; let app; + let eventId; let categoryId; + let uncategorisedIds; let categorisedId; + + const list = async (query = '') => { + const res = await request(app).get(`/api/admin/events/${eventId}/photos${query}`); + expect(res.status).toBe(200); + const photos = Array.isArray(res.body) ? res.body : res.body.photos; + return (photos || []).map((p) => p.id).sort((a, b) => a - b); + }; + + beforeAll(async () => { + jest.resetModules(); + jest.doMock('../../src/middleware/auth', () => ({ + adminAuth: (req, _res, next) => { req.admin = { id: 1, username: 'tester' }; next(); }, + })); + jest.doMock('../../src/middleware/permissions', () => ({ + requirePermission: () => (_req, _res, next) => next(), + })); + jest.doMock('../../src/middleware/ownership', () => ({ + requireEventOwnership: (_req, _res, next) => next(), + })); + jest.doMock('../../src/utils/logger', () => ({ + debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn(), + })); + + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + const [ev] = await db('events').insert({ + slug: 'uncat-filter', event_type: 'wedding', event_name: 'Uncat Filter', + event_date: '2026-08-01', host_email: 'h@example.com', admin_email: 'a@example.com', + password_hash: 'x', share_link: '/gallery/uncat-filter/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 [cat] = await db('photo_categories') + .insert({ name: 'Ceremony', slug: 'ceremony', event_id: eventId }) + .returning('id'); + categoryId = typeof cat === 'object' ? cat.id : cat; + + const insertPhoto = async (filename, category) => { + const [p] = await db('photos').insert({ + event_id: eventId, filename, path: `events/uncat/${filename}`, + type: 'individual', category_id: category, + uploaded_at: new Date().toISOString(), + }).returning('id'); + return typeof p === 'object' ? p.id : p; + }; + + // Two with no category — the shape a plugin upload leaves behind — and one + // filed properly, so a filter that does nothing is visibly different from + // a filter that works. + uncategorisedIds = [await insertPhoto('a.jpg', null), await insertPhoto('b.jpg', null)]; + categorisedId = await insertPhoto('c.jpg', categoryId); + uncategorisedIds.sort((a, b) => a - b); + + app = express(); + app.use(express.json()); + app.use('/api/admin/events', require('../../src/routes/adminPhotos')); + }, 180000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + it('returns only the photos with no category', async () => { + expect(await list('?category_id=uncategorized')).toEqual(uncategorisedIds); + }); + + it('returns everything when no category filter is given', async () => { + expect(await list()).toEqual([...uncategorisedIds, categorisedId].sort((a, b) => a - b)); + }); + + it('still filters by a real category id', async () => { + expect(await list(`?category_id=${categoryId}`)).toEqual([categorisedId]); + }); + + it('treats 0 as no filter at all', async () => { + // Pinning the behaviour that made the bug silent rather than loud: '0' is + // not "uncategorized" and never was, it simply falls through the guard. A + // future change that made 0 mean uncategorized here would be fine too — + // but it must be a decision, not an accident, and this test forces it. + expect(await list('?category_id=0')).toEqual([...uncategorisedIds, categorisedId].sort((a, b) => a - b)); + }); +}); diff --git a/frontend/src/components/admin/PhotoFilters.tsx b/frontend/src/components/admin/PhotoFilters.tsx index 74c7535e..b1679755 100644 --- a/frontend/src/components/admin/PhotoFilters.tsx +++ b/frontend/src/components/admin/PhotoFilters.tsx @@ -63,7 +63,12 @@ export const PhotoFilters: React.FC = ({ className="px-3 py-2 border border-neutral-300 dark:border-neutral-600 rounded-lg bg-white dark:bg-neutral-800 text-neutral-900 dark:text-neutral-100 focus:ring-2 focus:ring-primary-500 focus:border-accent-dark" > - + {/* The literal the backend understands, not 0 (#1211). It skips + '0' outright — `category_id !== '0'` — so this filter used to + apply no condition at all and quietly returned the whole event. + The onChange below passes non-numeric values through unchanged, + so the string arrives intact. */} + {categories.map(cat => (