diff --git a/backend/src/routes/adminArchives.js b/backend/src/routes/adminArchives.js index 57fdecbf..d290e26e 100644 --- a/backend/src/routes/adminArchives.js +++ b/backend/src/routes/adminArchives.js @@ -3,6 +3,7 @@ const path = require('path'); const fs = require('fs').promises; const { db } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); +const { slugify } = require('../utils/slug'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const archiver = require('archiver'); @@ -217,7 +218,7 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re const insertResult = await db('photo_categories').insert({ event_id: archive.id, name: categoryName, - slug: categoryName.toLowerCase().replace(/[^a-z0-9]/g, '-'), + slug: slugify(categoryName), created_at: new Date() }).returning('id'); diff --git a/backend/src/routes/adminEvents.js b/backend/src/routes/adminEvents.js index d1bbab3e..4ce28d14 100644 --- a/backend/src/routes/adminEvents.js +++ b/backend/src/routes/adminEvents.js @@ -2,6 +2,7 @@ const express = require('express'); const { body, validationResult } = require('express-validator'); const { db, logActivity } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); +const { slugify } = require('../utils/slug'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); const router = express.Router(); @@ -549,12 +550,10 @@ router.post('/', adminAuth, requirePermission('events.create'), [ } } - // Generate unique slug - const processedEventName = event_name - .toLowerCase() - .replace(/[^a-z0-9]/g, '-') // Replace non-alphanumeric with dash - .replace(/-+/g, '-') // Replace multiple dashes with single dash - .replace(/^-|-$/g, ''); // Remove leading/trailing dashes + // Generate unique slug. Uses the shared util so accented names + // (Família, Decoração, etc.) get transliterated instead of dropped + // — see backend/src/utils/slug.js for the why (#525). + const processedEventName = slugify(event_name); // Use event_date in slug if provided, otherwise use random suffix const slugSuffix = event_date || crypto.randomBytes(3).toString('hex'); diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 4f48016a..50496a52 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -226,15 +226,28 @@ router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), r let photoType = 'individual'; // default let categoryName = 'individual'; - // Look up the actual category from database if provided + // Look up the actual category from database if provided. Scope the + // lookup to (event_id = event.id OR is_global = true) — same contract + // the public v1 upload route enforces (#500 / #525). Without it, the + // admin upload silently accepts any category id including ones that + // belong to a different event. The v1 route rejects out-of-scope ids + // with 400; mirror that here so admin and v1 stay consistent. if (parsedCategoryId && !isNaN(parsedCategoryId)) { - const category = await db('photo_categories').where({ id: parsedCategoryId }).first(); - if (category) { - categoryName = category.slug || category.name.toLowerCase().replace(/\s+/g, '_'); - // Use category slug for type determination - if (category.slug === 'collage' || category.slug === 'collages') { - photoType = 'collage'; - } + const category = await db('photo_categories') + .where({ id: parsedCategoryId }) + .andWhere(function () { + this.where({ event_id: event.id }).orWhere('is_global', true); + }) + .first(); + if (!category) { + return res.status(400).json({ + error: `Unknown or out-of-scope category_id ${parsedCategoryId}` + }); + } + categoryName = category.slug || category.name.toLowerCase().replace(/\s+/g, '_'); + // Use category slug for type determination + if (category.slug === 'collage' || category.slug === 'collages') { + photoType = 'collage'; } } else if (category_id === 'collage') { // For backwards compatibility, accept string values diff --git a/backend/src/routes/events.js b/backend/src/routes/events.js index a42942a9..4122bd5d 100644 --- a/backend/src/routes/events.js +++ b/backend/src/routes/events.js @@ -4,6 +4,7 @@ const bcrypt = require('bcrypt'); const crypto = require('crypto'); const { db } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); +const { slugify } = require('../utils/slug'); const { validatePasswordInContext, getBcryptRounds } = require('../utils/passwordValidation'); const { adminAuth } = require('../middleware/auth'); const fs = require('fs').promises; @@ -125,8 +126,8 @@ router.post('/', adminAuth, [ } } - // Generate unique slug - const baseSlug = `${event_type}-${event_name.toLowerCase().replace(/[^a-z0-9]/g, '-')}-${event_date}`; + // Generate unique slug — slugify() handles accents (see #525). + const baseSlug = `${event_type}-${slugify(event_name)}-${event_date}`; let slug = baseSlug; let counter = 1; diff --git a/backend/src/routes/v1/__tests__/events.category.test.js b/backend/src/routes/v1/__tests__/events.category.test.js index 6ea0f7f4..872ba5fd 100644 --- a/backend/src/routes/v1/__tests__/events.category.test.js +++ b/backend/src/routes/v1/__tests__/events.category.test.js @@ -79,6 +79,32 @@ jest.mock('multer', () => { return factory; }); +// Stub sharp so the happy-path test doesn't actually decode an image +// (the temp file is a 0-byte placeholder — see the beforeAll below). +jest.mock('sharp', () => jest.fn(() => ({ + metadata: jest.fn().mockResolvedValue({ width: 1920, height: 1080 }), +}))); + +// Thumbnail + storage are network/fs-heavy; stub to constant resolves +// so the test stays a pure unit test of the route handler's contract. +jest.mock('../../../services/imageProcessor', () => ({ + generateThumbnail: jest.fn().mockResolvedValue('thumbnails/fake_thumb.jpg'), +})); + +jest.mock('../../../services/storage', () => ({ + getStorage: jest.fn(() => ({ + putFromFile: jest.fn().mockResolvedValue(undefined), + })), +})); + +// webhookService.fire is wrapped in try/catch in the route, so a +// missing mock would still let the test pass — but stubbing it +// silences the predictable failure log so the test output stays clean. +jest.mock('../../../services/webhookService', () => ({ + fire: jest.fn().mockResolvedValue(undefined), +})); + +const fsSync = require('fs'); const { db } = require('../../../database/db'); const eventsRouter = require('../events'); @@ -145,3 +171,92 @@ describe('v1 POST /events/:id/photos — category scoping', () => { }); }); }); + +describe('v1 POST /events/:id/photos — happy path (#525)', () => { + const FAKE_TMP = '/tmp/fake-v1-upload.jpg'; + + beforeEach(() => { + jest.clearAllMocks(); + // Recreate the temp file on every test — the handler calls + // fs.unlink(tempPath) after a successful upload, so a beforeAll + // would leave the second test without an inode for statSync to + // read (manifests as 500 Internal Server Error). + fsSync.writeFileSync(FAKE_TMP, ''); + }); + + afterAll(() => { + try { fsSync.unlinkSync(FAKE_TMP); } catch { /* may have been unlinked by the handler */ } + }); + + it('inserts the photo and returns 201 with the resolved category_id', async () => { + // Three db() calls in sequence on the happy path: + // 1. events lookup + // 2. photo_categories lookup (returns a valid in-scope row) + // 3. photos insert returning the new id + const eventChain = buildChain({ + firstResult: { id: 42, slug: 'wedding-2026', event_name: 'Wedding 2026' }, + }); + const categoryChain = buildChain({ + firstResult: { id: 7, slug: 'ceremony', name: 'Ceremony', event_id: 42 }, + }); + const insertChain = { + ...buildChain({ insertResult: [{ id: 101 }] }), + returning: jest.fn().mockResolvedValue([{ id: 101 }]), + }; + // Override insert so the returning() call is chainable + insertChain.insert = jest.fn(() => insertChain); + db.__setImplementations(eventChain, categoryChain, insertChain); + + const response = await request(buildApp()) + .post('/events/42/photos') + .send({ category_id: '7' }) + .expect(201); + + // Response shape pins the v1 API contract — id + category_id are + // the fields the n8n / API-token use case depends on (see #500). + expect(response.body).toMatchObject({ + id: 101, + category_id: 7, + size_bytes: 0, + thumbnail_path: 'thumbnails/fake_thumb.jpg', + }); + expect(response.body.filename).toMatch(/^\d+_[a-f0-9]+\.jpg$/); + expect(response.body.path).toMatch(/^wedding-2026\/\d+_[a-f0-9]+\.jpg$/); + + // The insert payload should carry the resolved category_id and the + // 'individual' photo type (the test category slug isn't 'collage'). + const insertedRow = insertChain.insert.mock.calls[0][0]; + expect(insertedRow).toMatchObject({ + event_id: 42, + category_id: 7, + type: 'individual', + media_type: 'image', + mime_type: 'image/jpeg', + }); + }); + + it('flips photo type to collage when the category slug is "collage"', async () => { + const eventChain = buildChain({ + firstResult: { id: 42, slug: 'wedding-2026' }, + }); + const categoryChain = buildChain({ + firstResult: { id: 9, slug: 'collage', name: 'Collage', event_id: 42 }, + }); + const insertChain = { + ...buildChain(), + returning: jest.fn().mockResolvedValue([{ id: 202 }]), + }; + insertChain.insert = jest.fn(() => insertChain); + db.__setImplementations(eventChain, categoryChain, insertChain); + + await request(buildApp()) + .post('/events/42/photos') + .send({ category_id: '9' }) + .expect(201); + + expect(insertChain.insert.mock.calls[0][0]).toMatchObject({ + category_id: 9, + type: 'collage', + }); + }); +}); diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index 78c68f94..436d8467 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -23,6 +23,7 @@ const { apiTokenAuth, requireApiScope } = require('../../middleware/apiTokenAuth const { buildShareLinkVariants } = require('../../services/shareLinkService'); const { generateThumbnail } = require('../../services/imageProcessor'); const logger = require('../../utils/logger'); +const { slugify } = require('../../utils/slug'); const router = express.Router(); @@ -51,8 +52,8 @@ const photoUpload = multer({ } }); -const slugify = (s) => - String(s).toLowerCase().replace(/[^a-z0-9]/g, '-').replace(/-+/g, '-').replace(/^-|-$/g, ''); +// slugify now imported from ../../utils/slug — shared with adminEvents +// and events.js so the diacritic fix from #502 lands here too (#525). // ────────────────────────────────────────────────────────────────────────── // POST /events — create event diff --git a/backend/src/utils/__tests__/slug.test.js b/backend/src/utils/__tests__/slug.test.js new file mode 100644 index 00000000..1e7cfa78 --- /dev/null +++ b/backend/src/utils/__tests__/slug.test.js @@ -0,0 +1,93 @@ +/** + * Tests for the shared slug util extracted in #525 from the inline + * pipelines in adminEvents.js, events.js, v1/events.js, adminArchives.js. + * + * Two contracts to pin: + * 1. ASCII inputs produce byte-identical output to the previous + * inline pipelines, so existing event/archive slugs in the DB + * keep resolving via the same lookup path after the refactor. + * 2. Accented characters (Portuguese, German, French, Spanish) are + * transliterated to their ASCII bases (Decoração → decoracao) + * instead of being dropped (Decoração → decorao) as the legacy + * pipelines did — same fix as #502 for category slugs. + */ + +const { slugify } = require('../slug'); + +describe('slugify — ASCII parity with the legacy event-style pipeline', () => { + // Replays the exact transformation used by adminEvents.js before the + // refactor: lowercase → replace [^a-z0-9] with '-' → collapse → trim. + const legacy = (s) => + String(s).toLowerCase() + .replace(/[^a-z0-9]/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + + const samples = [ + 'Wedding 2026', + ' Hello World ', + 'birthday-party-42', + 'event_with_underscores', + 'CamelCase Event Name', + '', + 'event.with.dots', + 'event!@#$%^&*()chars', + '2026-06-12', + ]; + + it.each(samples)('matches legacy output for ASCII input: %j', (input) => { + expect(slugify(input)).toBe(legacy(input)); + }); +}); + +describe('slugify — accented characters (the #502 fix, now shared)', () => { + // The legacy pipeline produced f-mlia for "Família" because the í + // got replaced with '-' rather than being NFD-normalised to 'i'. + // These tests pin the corrected behaviour across the locales the + // app already ships in (de, es, fr, nl, pt, ru). + it.each([ + ['Decoração', 'decoracao'], + ['Família', 'familia'], + ['Recepção', 'recepcao'], + ['Über uns', 'uber-uns'], + ['Niño', 'nino'], + ['Fête de famille', 'fete-de-famille'], + ['L\'Évènement', 'l-evenement'], + ['Crème Brûlée', 'creme-brulee'], + ])('transliterates %j → %j', (input, expected) => { + expect(slugify(input)).toBe(expected); + }); + + it('CJK and other scripts without NFD decompositions still strip cleanly', () => { + // NFD doesn't decompose Chinese characters to ASCII, so they get + // dropped by the [^a-z0-9]+ replace. Output is sensible if not + // perfect — the surrounding ASCII tokens survive. + expect(slugify('Photo 混合 Test')).toBe('photo-test'); + // Pure-CJK names collapse to empty after trim — caller's job to + // handle (typically by appending a uniqueness suffix). + expect(slugify('婚礼')).toBe(''); + }); +}); + +describe('slugify — input edge cases', () => { + it('returns empty string for null / undefined / empty', () => { + expect(slugify(null)).toBe(''); + expect(slugify(undefined)).toBe(''); + expect(slugify('')).toBe(''); + }); + + it('coerces non-string input to string before slugifying', () => { + expect(slugify(2026)).toBe('2026'); + expect(slugify(true)).toBe('true'); + }); + + it('collapses any run of non-alphanumeric chars into a single dash', () => { + expect(slugify('a!@#$%b')).toBe('a-b'); + expect(slugify('a b\t\nc')).toBe('a-b-c'); + }); + + it('trims leading and trailing dashes', () => { + expect(slugify('---hello---')).toBe('hello'); + expect(slugify('!!!world!!!')).toBe('world'); + }); +}); diff --git a/backend/src/utils/slug.js b/backend/src/utils/slug.js new file mode 100644 index 00000000..8799c508 --- /dev/null +++ b/backend/src/utils/slug.js @@ -0,0 +1,35 @@ +/** + * URL-safe slug generation shared across event, archive, and v1 upload + * routes (#525 follow-up to #502). Previously every caller had its own + * inline `name.toLowerCase().replace(/[^a-z0-9]/g, '-')` pipeline, each + * with the same latent bug: JS's `\w` and the ASCII alphanumeric class + * silently drop non-ASCII letters instead of transliterating them + * (`Decoração` → `decorao`, `Família` → `f-mlia`). + * + * Fix mirrors #502: NFD-normalize so accented characters split into a + * base letter + combining mark, then strip the combining-mark range + * (U+0300–U+036F) so the ASCII base survives. Single regex pass after + * that — `[^a-z0-9]+` collapses any run of non-alphanumerics into one + * dash, no separate collapse step needed. + * + * For pure-ASCII input the output is byte-identical to the previous + * inline pipelines, so existing slugs continue to round-trip cleanly + * via lookups; only new inserts with non-ASCII names start producing + * the corrected slugs. + * + * Not exported as the default category slug — `adminCategories.js` + * intentionally preserves underscores (the legacy category pipeline + * used `\w` not `[a-z0-9]`), so changing it here would silently shift + * "wedding_party" → "wedding-party" on new inserts. Categories keep + * their own pipeline as fixed in #502. + */ +function slugify(input) { + return String(input ?? '') + .normalize('NFD') + .replace(/[̀-ͯ]/g, '') + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-|-$/g, ''); +} + +module.exports = { slugify };