53a16f9f6f9b11c224b3ff5f337f8a7fe4dbfc38
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul Nothaft
|
69b5186582 |
fix(gallery): guest upload honours general_max_files_per_upload + i18n placeholder interpolates (#613)
Zszywany reported on v3.44.0 (Ubuntu, Postgres 15): with Settings →
General → "Max Files per Upload" set to 10, an event configured with
allow_user_uploads + a guest selecting 16 files in the gallery's
"Upload Photos" modal succeeded silently — admin uploads to the same
event correctly refused with "Upload limit reached". On top of that,
the gallery modal's "fileRequirements" hint literally rendered
`{{limit}}` instead of the configured number.
Two separate misses for the guest path, both fixed here:
1. **Backend enforcement** — `backend/src/routes/gallery.js:1641` had
`limits: { fileSize: 50MB, files: 10 }` and `.array('photos', 10)`
hardcoded. The admin path at adminPhotos.js:131 has always resolved
files-per-batch via `getMaxFilesPerUpload()` (cached 60s read of
`general_max_files_per_upload`); guest path just never used it.
Mirror the admin: `const maxFilesPerUpload = await getMaxFilesPerUpload()`
and feed multer both `limits.files` AND the `.array(...)` cap. The
50MB per-file size is a separate concern from this issue and stays
as-is for now.
2. **i18n interpolation missing on the guest modal** —
`UserPhotoUpload.tsx:203` called `t('upload.fileRequirements')` with
no arguments. The translation string at `en.json:160` is
"JPEG, PNG or WebP (max 50MB per file, {{limit}} files per upload)"
— `{{limit}}` is unbound, so i18next emits it literally. The admin
variant `PhotoUpload.tsx:414` correctly passes
`{ limit: maxFilesPerUpload }`.
Also wired up the same client-side count guard the admin component
uses: addFiles refuses additions past the limit (`upload.limitReached`)
and warns on partial-truncate (`upload.someFilesSkipped`). Backend
enforces too, but the client guard saves a 4MB+ multipart POST when
the user is clearly over.
To surface the setting on the guest side, `general_max_files_per_upload`
joins the public-settings whitelist + projection (publicSettings.js)
and the `PublicSettings` TS interface gets the new field. Default
fallback (500, matching `uploadSettings.js` DEFAULT_MAX_FILES_PER_UPLOAD)
in both backend projection and frontend reader so an install that's
never set the value renders a sensible number rather than "undefined".
|
||
|
Paul Nothaft
|
457c956386 |
fix(admin/events): delete cascade orphaned photo folders because it read a non-existent column (#608)
jodrmx reported on v3.44.0 (Pi Lite, Docker compose): admin-UI event
delete removes the DB row but leaves `storage/events/active/<event>/`
intact on disk.
Root cause: `deleteEventCascade` in adminEvents.js read
`event.folder_path` and gated the `fs.rm` on it. That column is NEVER
WRITTEN anywhere in the codebase — grep confirms two reads in this one
function, zero writes elsewhere. So `event.folder_path` was always
undefined, `if (event.folder_path)` always false, and the per-folder
cleanup silently no-op'd for every delete. The DB-cascade transaction
ran fine, so the symptom was always "row gone, files stay" — exactly
what jodrmx hit.
The actual on-disk location is `events/active/{slug}` everywhere else
in the codebase:
- adminPhotos.js:260 — `path.posix.join('events/active', event.slug)`
- adminEvents.js:610, events.js:155, adminThumbnails.js:153 — read
from `events/active/{slug}`
- adminArchives.js:171 — reads from same root
- photoResolver.js:14-15 — documents the layout
The delete cascade was the only path looking at the non-existent column.
Cure: drop the `if (event.folder_path)` guard, read `event.slug`
instead, and remove from both `events/active/{slug}` (active gallery
folder) and `events/archived/{slug}` (the post-archive copy that
survives the archive flow). `event.slug` is NOT NULL and slugify-
sanitized (lower-case ASCII + dashes only via utils/slug.js), so the
path is well-formed and path-traversal-safe. Best-effort `fs.rm`
semantics + try/catch unchanged — failures still log a warning rather
than unwinding the DB transaction, since orphan files are recoverable
noise compared to a half-deleted DB row.
Forward fix only — does not retroactively clean up the orphans that
have accumulated on existing installs. Admins can `rm -rf
storage/events/active/<old-slug>` manually for those; not worth a
migration script for a one-time deploy ritual.
|
||
|
Paul Nothaft
|
620163f2db |
fix(downloads): transliterate accented characters in filename via NFD instead of dropping them (#607)
patchingfailed reported on v3.44 stable: a gallery named `Ägypten` with
photo `Ägypten_individual_0050.jpg` downloads as `gypten_...` —
the leading umlaut is dropped entirely. Their hypothesis was a
Content-Disposition encoding issue, but the actual root cause sits
one layer earlier: at UPLOAD time when `generatePhotoFilename` calls
`sanitizeFilename`.
`sanitizeFilename` did:
String(str).trim()
.replace(/\s+/g, '_')
.replace(/[^a-zA-Z0-9_\-\.]/g, '') // ← drops `Ä` outright
.replace(/[_\-]{2,}/g, '_')
.replace(/^[_\-]+|[_\-]+$/g, ''); // ← would strip a leading _ too
For `Ägypten`: alphanumeric-strip → `gypten` (Ä gone, no underscore
left behind because the regex used '' as the replacement, not '_'). The
result is stored in `photos.filename` and that's what downloads serve.
By that point `buildContentDisposition` is doing the right thing
(emits both `filename="..."` ASCII fallback AND RFC 5987
`filename*=UTF-8''…` — Chrome correctly picks the UTF-8 form), but the
string it's encoding has already lost the umlaut at the DB layer.
Cure: NFD-normalize + strip combining marks BEFORE the alphanumeric
strip. Same pipeline `utils/slug.js` (#525) already uses for URL
slugs:
sanitized = sanitized
.normalize('NFD')
.replace(/[̀-ͯ]/g, '');
Now `Ägypten` → NFD-decomposed `A` + combining diaeresis → strip
combining mark → `Agypten` survives the alphanumeric pass. Filename
and URL slug stay in sync (the URL was already `Agypten`, per
patchingfailed's report — the filename now matches).
Test surface: new `filenameSanitizer.test.js` pins:
- the headline #607 contract for German / Portuguese / French / Spanish
accented inputs (with a counter-example using the pre-fix pipeline so
a future edit can't quietly regress it)
- ASCII-input parity — pre-#607 byte-identical output for every
pre-existing ASCII case
- `generatePhotoFilename` composed round-trip
- `sanitizeForContentDisposition` + `buildContentDisposition` RFC 6266
dual-form output (since the helper sits next to this function and is
the next thing to break if a refactor goes sideways)
- `sanitizeForZipEntry` path-traversal blocking
31 cases total, all pass.
Bundled into PR #609 since it's a small targeted fix and that PR is
already an admin-UI polish branch with low review weight.
|
||
|
Paul Nothaft
|
f51b9cf8df |
fix(admin): graceful logo-img fallback + show sidebar widgets during perm hydration (#523 follow-up 2)
Two issues Rekoo-PS hit immediately after upgrading to v3.60.3-beta.0:
1. **Broken logo URL rendered the browser's broken-image icon + alt
text.** Their `<img src={resolvedLogoUrl}>` had no `onError` handler,
so a 404 / slow logo URL produced the default broken-image rendering
— which uses the `alt` attribute (`companyName`) as text. Visually it
looked like the wordmark span had unexpectedly re-appeared on phone,
even though the actual `<span>` was correctly hidden by the existing
`wordmarkVisibilityClass` logic.
Fix:
- `useState` tracks `logoLoadError` (first failure) and
`fallbackLoadError` (second failure). On a configured-URL miss the
`<img>` swaps to the bundled `/picpeak-kamera-transparent.png`; on
a second miss the `<img>` is removed from the DOM entirely.
- `useEffect([resolvedLogoUrl])` resets both flags when the URL
changes, so a dark-mode toggle that flips `lightLogo ↔ darkLogo`
gets a fresh attempt instead of being permanently sad.
- `wordmarkVisibilityClass` now derives from `logoEffectivelyVisible`
(showLogo && !fallbackLoadError) — when both the configured URL
AND the bundled fallback have failed, the wordmark un-hides on <sm
so the phone header isn't completely empty.
2. **Sidebar VersionInfo + StorageInfo vanished during the
permission-hydration window.** The bottom block was gated on
`hasPermission('settings.view')` directly, which returns `false`
while `PermissionsContext.isLoading` is still resolving (a few
hundred ms right after a deploy when the auth context bootstraps).
Net effect: the whole "Version / Storage" block was absent on first
paint, then re-appeared once permissions hydrated — Rekoo-PS read
that flash as "backend version + storage missing".
Fix: gate on `permissionsLoading || hasPermission('settings.view')`.
Optimistic render during hydration; permitted users see the widgets
immediately (with each widget's own internal loading state), denied
users still see nothing once the permission state lands as `false`.
Side benefit: the `<img>` fallback chain also covers the broader "logo
hosted on a flaky CDN" case for self-hosters, not just the one-time
post-upgrade asset-cache hiccup. Pure resilience polish — no behaviour
change when everything works.
|
||
|
Paul Nothaft
|
fe10191b82 |
fix(admin-header): skeleton brand block + move LanguageSelector into profile menu on <sm (#523 follow-up)
Two complaints in Rekoo-PS's 3.60.1-beta.0 follow-up screenshots: 1. "Logo took some time to load" — header appeared empty for the ~hundreds-of-ms window between admin mount and `usePublicSettings()` resolving. The previous code rendered the static fallback `/picpeak-kamera-transparent.png` during that window, which often either 404'd or loaded after the rest of the chrome, and because the wordmark is `hidden sm:inline` whenever a logo is intended to be shown, phone-width admins saw an empty left cluster instead of anything. Cure: render a small pulsing skeleton block (h-8 w-8 on <sm, w-32 on sm+) while `brandingLoading === true`. Same h-8 footprint as the real logo image so there's no layout shift when the real payload arrives. Once the public-settings query settles, the normal brand block renders against known state. 2. "Moving the languages inside the profile tab" — Rekoo-PS argues language is set-once and shouldn't occupy permanent header real estate on mobile (4 widgets in the right cluster on phone is crowded). I agree. On <sm: header LanguageSelector is hidden (`hidden sm:block` wrapper around the existing component). A collapsible Language section is added at the top of the user-menu dropdown showing the current flag/name + chevron-down. Expanding shows the 8 supported languages as inline rows highlighting the active one. Picking a language fires i18n.changeLanguage and closes the menu. On sm+: header LanguageSelector stays where it was. The user-menu Language section is suppressed (`sm:hidden`) so the same control isn't surfaced twice. Also: `useOnClickOutside(userMenuRef, …)` and the in-menu action handlers now route through a shared `closeUserMenu()` helper that also resets the lang sub-section state, so re-opening the menu doesn't surprise the user with the language list still expanded. `SUPPORTED_LANGUAGES` re-exported from `components/common` so AdminHeader doesn't reach into `LanguageSelector.tsx` directly. No behaviour change on `sm+` — pure phone-view layout fix + loading-state polish. Locales unaffected (uses the already-existing language names from SUPPORTED_LANGUAGES). |
||
|
Paul Nothaft
|
29e63e5ce5 |
fix(notifications): restore /clear-all route the frontend already calls (#597)
The AdminHeader "Clear All" notifications button has been 404'ing for
a while: frontend `notifications.service.ts` calls
`DELETE /admin/notifications/clear-all`, backend only defined
`DELETE /admin/notifications/clear-old`.
The /clear-old route was misleadingly named anyway — it tried to
delete read OR >30-days-old rows, then had a fallback that nuked
EVERY row when nothing matched. Both the frontend and the existing
test expect a simple Clear All shape, so just rename to /clear-all,
drop the tiered logic, and return the plain
`{ message, deletedCount }` payload the test asserts on.
The test (adminNotifications.test.js) was hiding the breakage —
it was on CI's --testPathIgnorePatterns ignore list and so never
ran. Two reasons it failed locally before this fix:
1. Route path mismatch (the actual #597 bug).
2. The mock only stubbed adminAuth — requirePermission lives in
its own middleware module and ran for real, 403'ing before
the handler. Add a passthrough mock for that too.
With both fixed, the test passes. Drop adminNotifications from the
CI ignore list so future regressions in this route fail loudly
instead of going to ground.
|
||
|
Paul Nothaft
|
c246fd3cc8 |
fix(admin-header): hide wordmark on <sm when logo also shows (#523)
Rekoo-PS's v3.59.0-beta.0 screenshot showed a different shape than
the truncate fix in
|
||
|
Paul Nothaft
|
8c6525af01 |
test(v1/events): update mock chains to cover new app_settings probes
The #592 fix added a devtools-detection probe, and the #592 follow-up added a require_password probe + a branding-defaults whereIn().select(). Both shift the db() call indices the existing #550 test relied on, and the branding probe needed `.select()` to resolve to an array (the mock chain wasn't thenable, so `for..of` on the result threw → 500 on every test that hit BASE_BODY). Add `whereIn` + `selectResult` to buildChain so the branding probe yields an iterable. Factor the three pre-slug app_settings chains into a baseSettingsChains() helper and update each test's queued sequence and toHaveBeenNthCalledWith / toHaveBeenCalledTimes expectations to match the new shape. No behaviour change in v1/events.js — only the test scaffolding moves. |
||
|
Paul Nothaft
|
791e9974eb |
fix(gallery): preserve per-viewer is_liked across hard refresh (#590 follow-up)
The in-session toggle fix in
|
||
|
Paul Nothaft
|
2d44b1ab2d |
fix(api/v1/events): also honour require_password + branding defaults (#592 follow-up)
Same class of bug as the devtools-detection gap landed in
|
||
|
Paul Nothaft
|
2304b25624 |
fix(api/v1/events): honour global devtools-detection default on create (#592)
Same class of bug as #550 part 2 (feedback default ignored on API
events): the events table column default for enable_devtools_protection
is true, so an admin who disabled detection globally still got it ON
for every API-created gallery.
Mirror the feedback fallback that landed in
|
||
|
Paul Nothaft
|
c83e88348f |
fix(nginx): defensive large_client_header_buffers bump (#591)
Default nginx is 4 8k — too tight when an outer Cloudflare / corp-proxy injects long Set-Cookie / X-Forwarded-* headers, or when a power-user accumulates many per-gallery gallery_token_<slug> cookies over the 24h maxAge in tokenUtils.js. Either way users hit "400 Request Header Or Cookie Too Large" and clearing cookies is the only workaround. 4×32k is cheap RAM, matches what most reverse proxies do upstream, and means PicPeak doesn't fail the request before the upstream even sees it. |
||
|
Paul Nothaft
|
d292b9fa10 |
fix(gallery): toggle (not add) the local liked set on click (#590)
The /feedback like endpoint is a server-side toggle — the same one the lightbox uses. Every grid layout's optimistic-UI setter only ever did next.add(photoId), so click 2 on a liked tile fired a server unlike but kept the heart filled in the UI. Switch each setter to toggle (delete if present, else add). Covers Masonry (default), Grid, Justified, Timeline, Carousel, Mosaic, and Premium layouts — including their identity-modal callback paths for shape consistency. Lightbox toggle is unchanged (already correct). |
||
|
Paul Nothaft
|
e7cf834325 |
fix(admin-header): truncate long company names on narrow widths (#523 regression)
#527 hid the language *name* on <sm to free space for the title. Since then the right cluster gained dark-mode toggle, notifications, and the user avatar, and the brand block still had no truncation — so a long branding_company_name would still push past the available width into the action buttons on phones. Defensive fix: min-w-0 on the brand-block wrapper, truncate on the company-name span, flex-shrink-0 on the logo image. Long names now ellipsis within the left cluster regardless of how many widgets fill the right. |
||
|
Paul Nothaft
|
dcc629cad2 |
fix(csp): external bootstrap script to survive strict reverse-proxy CSP (#564)
demo.picpeak.app sits behind Caddy + Cloudflare; Caddy replaces the nginx CSP entirely with one that omits 'unsafe-inline' / hash / nonce, so the #358 inline theme-bootstrap was being blocked there — admin loaded a black page, the SPA bundle 404'd, link buttons did nothing. Move the bootstrap to /public/bootstrap.js served as 'self' so the script runs under every reasonable CSP without further coordination. Vite copies /public/* to the dist root at build time (same pipeline as /favicon-32x32.png), and it remains in <head> without defer/async so it still runs before <body> paints. The OS-preference @media CSS above still handles the first-frame dark/light baseline. |
||
|
Paul Nothaft
|
dfcebccee9 |
feat(admin/users): reactivate + delete actions for deactivated admin users
#574 follow-up — @blazmaric flagged that once an admin user is deactivated, the UI loses every affordance to manage that record. The deactivate button hides (rightly — they're already deactivated) but nothing replaces it, leaving the row stranded in the list with no path to either restore access or permanently remove it. ## Backend New on `userManagementService`: - **`activateAdminUser(id, activatedById)`** — symmetric to `deactivateAdminUser`. Flips `is_active` back to true, logs `admin_user_activated` activity. Idempotent: already-active target short-circuits without bumping `updated_at`. No "can't activate yourself" guard needed (actor is by definition already active). - **`deleteAdminUser(id, deletedById)`** — hard-deletes the row. Same self-action and last-super-admin guards as deactivate. Last-super-admin guard counts ACTIVE super admins excluding the target — so an already-deactivated super_admin can still be deleted when an active super_admin remains. FK ON DELETE rules in core migrations handle the cascade: SET NULL on `created_by_admin_id` everywhere (events, photos, quotes, invoices, contracts, customer_accounts, …); CASCADE on the user's own `api_tokens` + their pending admin / customer invitations. New routes on `adminUsers.js`: - `POST /api/admin/users/:id/activate` — `users.delete` permission (same tier as deactivate; reverting deactivation is the same scope of action as performing it). - `DELETE /api/admin/users/:id` — `users.delete`. ## Frontend `UserManagementPage.tsx`: - New mutation hooks: `activateUserMutation`, `deleteUserMutation`. - The row's action cell now branches on `user.isActive`: active users see Edit + Deactivate (unchanged); deactivated users see Edit + Reactivate (`UserCheck` icon, green hover) + Delete (`Trash2` icon, red hover). - The shared `ConfirmDialog` handles all four action types (deactivate / activate / delete / cancelInvitation) via per-type title / message / confirmText / variant lookup. `userManagement.service.ts`: - New `activateUser(id)` and `deleteUser(id)` methods mirroring the existing `deactivateUser` shape. i18n keys are added with English fallbacks via `t(key, fallback)` so the page works on every locale without a missing-translation warning. Native translations can be filled in via a follow-up. ## Test plan - [x] 8 new service tests pin: activate happy-path, idempotency on already-active, NotFoundError on missing target, activity log emitted, delete self-refusal, last-super-admin guard for both active and already-deactivated super_admin targets, hard-delete success, delete activity log. - [x] Frontend type-check clean. - [x] Frontend lint clean for the changed files. - [x] Backend lint clean. - [ ] Manual: deactivate a user → row now shows Reactivate + Delete → reactivate → user can log in again. Then deactivate again → delete → row vanishes, pending tokens for that user invalidated. Closes the UX gap blazmaric called out in https://github.com/the-luap/picpeak/pull/579#issuecomment-... . |
||
|
Paul Nothaft
|
5c4da1eacd |
test(crm): HTTP route tests for CRM public + admin surface (#570)
Closes #570. PR #555 shipped the CRM module with strong service-layer coverage but no HTTP-layer tests. This adds Supertest-based route coverage across the externally-reachable public routes (P0) and an auth-gate sweep of every CRM admin route (P1+P2). ## What's covered ### P0 — Public routes (49% of new tests) The three public routes are the security-sensitive surface — any IP with the raw token from a leaked email can hit them. Tests pin the publicTokenGuards.loadActionToken contract end-to-end: - **publicQuotes** (8 tests) — GET load + POST respond: 404 unknown, 400 malformed, 410 expired, 200 valid w/ sanitised payload (no customer_account_id / created_by_admin_id leakage), 429 after 20 bad attempts (IP lockout), 400 invalid action. - **publicContracts** (10 tests) — GET load + POST sign + POST upload-signed-pdf + GET pdf: same guard outcomes per endpoint, plus the pre-multer token check (malformed token rejected before multer reads the body — prevents the disk-spam attack the preMulterTokenGuard was added for). - **publicPaymentCheck** (6 tests) — different shape (no loadActionToken; service does its own validation): validator gate on token shape, all 4 canonical actions pass through the validator, negative amountMinor rejected. The NULL-expires_at defensive branch in loadActionToken is documented but not tested here — current schema declares quote/contract_action_tokens.expires_at NOT NULL, so the branch is unreachable at the route level. Worth a direct unit test on loadActionToken if anyone wants to cover it. ### P1 + P2 — Admin routes (51% of new tests, 25 cases) One consolidated `adminCrmAuth.test.js` file rather than nine per-route files — the auth-gate contract is identical for every CRM admin route, so a parametrised `describe.each` is more efficient and lands the same coverage: Per route (adminQuotes, adminContracts, adminInvoices, adminCalendar, adminDeals, adminTaxReport, adminBusinessProfile): - 401 without Authorization header (adminAuth gate) - 401 with invalid JWT signature (adminAuth signature check) - 2xx with super-admin token + CRM feature flags on (permission + feature-flag gates both pass) Plus 4 tests for the CRM additions in adminCustomers (hour-entries / bill / trigger-monthly-bill) — those endpoints are mixed in with pre-existing customer routes, so they get explicit coverage rather than bulk via the parametrised sweep. ## Harness extensions to integration/helpers/crmDb.js Three new helpers (one place for any future route test to find): - `mintAdminToken(adminId, opts)` — JWT signed with the test JWT_SECRET, shape matches what adminAuth expects. - `createPublicToken(db, tableName, opts)` — insert a row into quote/contract_action_tokens with controllable expires_at / used_at / token. Note: Date values are explicitly ISO-stringified before insert — bare Date objects round-tripped inconsistently through knex+SQLite, sometimes via .toString() → literal `"[object Object]"` which parsed back to NaN and silently defeated the expiry guard. Caught it in test bring-up. - `buildRouteApp(mount, router)` — minimal Express app (json + cookies) with a catch-all error handler that mirrors middleware/errorHandler (uses err.statusCode, not err.status — getting that wrong silently maps every 4xx to 500 in tests). - `assignAdminRole(db, adminId, roleName)` — promotes a seedMinimal admin into super_admin (or any seeded role) for happy-path tests. ## Out of scope (follow-up) Deeper integration tests for the document mint/send paths (adminQuotes.send → PDF persisted + token minted + email queued; adminInvoices.Storno → new row with shared deal_uuid + original cancelled; adminContracts.countersign → integrity_hash computed) are deferred. The service-layer behind those is already covered by the existing __tests__/services/ suites — this PR pins the HTTP-layer contract, which is what #570 actually asked for. ## Counts - 4 new test files, 49 tests total - ~860 LOC of test code + ~85 LOC of new harness in crmDb.js - All tests pass in <2.5s (no real network, no real disk except the per-test tmpdir, no email sending) |
||
|
Paul Nothaft
|
37cc3631d8 |
feat(i18n): add Slovenian (sl) language support
Closes #580. Slovenian community contribution from @blazmaric (filed as an issue with attached files rather than as a PR — files inlined here unchanged except for the migration number). ## Changes - **`frontend/src/i18n/locales/sl.json`** — full Slovenian UI translations. Covers every top-level key present in `en.json` as of pre-CRM beta. The new CRM-module keys (`bills`, `businessProfile`, `calendar`, `contracts`, `crm`, `crmDev`, `crmSettings`, `dealLineage`, `eventReminderOverride`, `hoursLogging`) are not yet translated and will fall back to English — same posture as FR / NL / PT / RU / ES currently have for the CRM module (see PR #555 description). - **`frontend/src/components/common/LanguageSelector.tsx`** — adds `SLFlag` SVG component + registers `{ code: 'sl', name: 'Slovenščina', Flag: SLFlag }` in `SUPPORTED_LANGUAGES`. Frontend i18n auto-discovers locale files via `import.meta.glob` so no separate config registration is needed. - **`backend/migrations/core/108_seed_sl_email_template_translations.js`** — contribution-author's `107_*` filename renumbered to `108_` to avoid collision with `107_crm_consolidated.js` that landed on beta in the meantime. Idempotent insert via (template_id, language) uniqueness check — re-runnable, never overwrites admin edits. Covers 17 templates: admin invitation / password reset, archive complete, backup completed / failed, customer gallery assigned, customer invitation / password reset, database backup completed / failed, expiration warning, gallery created / expired, restore completed / failed, version update available / test. - **`backend/src/services/emailProcessor.js`** — adds `.si → sl` to the email-domain → language inference map, matching the pattern for every other supported locale. A customer with `@example.si` now gets Slovenian emails automatically without needing to set their preferred_language explicitly. ## Out of scope (consistent with existing locales) - CRM email templates (quote_sent, invoice_sent, contract_sent, etc., seeded at boot by `crmEmailTemplates.ensureCrmEmailTemplatesSeeded`) will fall back to English for Slovenian customers — those seeders only emit EN + DE rows today across every locale. - CRM UI strings under the missing top-level keys listed above will fall back to English. Both gaps mirror the existing FR / NL / PT / RU / ES situation. |
||
|
Paul Nothaft
|
975a815f99 |
Merge branch 'beta' into fix/email-normalization-574
Resolves a conflict with the CRM merge (#555) that landed on beta between when this branch was cut and now. Two conflict regions in backend/src/routes/adminCustomers.js: 1. **Require block** — both branches added new requires after customerAccountsService. Kept both: this branch's emailNormalization import AND beta's customerHoursService + invoiceService imports (the CRM merge added the hours-billing + invoice-creation paths to this router). 2. **Edit-customer validators** — both branches changed the same set of body() validators in the PUT /:id handler. This branch added the IDENTITY_PRESERVING_NORMALIZE_EMAIL options arg to normalizeEmail; beta changed every body() to optional({ nullable: true }) so passive-customer records that store nulls for missing profile fields don't reject on save. Kept both: the nullable pattern from beta + the email-normalization options from this branch. Preserved beta's explanatory comment about the nullable choice. Also patched one NEW normalizeEmail site the CRM merge introduced: - backend/src/routes/adminCustomers.js:231 — POST /admin/customers now exists (CRM-era customer-create endpoint). Same options arg applied. backend/src/routes/adminBusinessProfile.js has an isEmail() WITHOUT normalizeEmail() on the issuer email — intentional (no normalization means no risk of the Gmail dot-strip bug for that field), no change needed. All 18 normalizeEmail sites now pass IDENTITY_PRESERVING_NORMALIZE_EMAIL. 7/7 regression tests still pass. Lint clean on the merged file. |
||
|
Paul Nothaft
|
2692e71297 |
docs(contributing): note rebuild-after-package.json gotcha for dev compose
When PR #555 (CRM module) added pdfkit/swissqrbill/pdf-lib/qrcode to backend/package.json, every dev with an already-built dev image hit a MODULE_NOT_FOUND restart loop on the next pull. Root cause: the dev compose bakes node_modules into the image while live-mounting src/ from disk — a dep added on disk isn't visible to the running container until the image is rebuilt. The symptom doesn't point at the cause, so this adds a short rebuild note to the Local Development section of CONTRIBUTING.md. A self-healing entrypoint (compare node_modules/.package-lock.json vs /app/package-lock.json on boot, npm ci if they differ) would fix this at the runtime layer too; tracked as a follow-up. |
||
|
Paul Nothaft
|
c2dcd9ca84 |
docs(readme): list CRM module under Beta Features with own-risk disclaimer
PR #555 shipped the CRM module on beta. The README's "Beta Features (Use at your own risk)" table is the right place to signal that the feature exists, is opt-in, and carries non-trivial legal / financial caveats — readers landing on the README should not first discover the CRM by enabling its feature flags and bumping into the seeded example contract bodies without warning. Adds one row to the Beta Features table linking to docs.picpeak.app/features/crm where the full disclaimers, sub-feature pages, and admin-settings reference live. CRM is intentionally NOT added to the top-of-README "Key Features" list — those are stable, production-ready features. Mixing the beta CRM in there would undermine the clear stable/beta distinction. |
||
|
Paul Nothaft
|
075b45f020 |
fix(email): preserve dots + subaddresses across all normalization sites (#574)
Closes #574. Reporter (@blazmaric) identified the root cause cleanly: express-validator's `.normalizeEmail()` applies provider-specific canonicalization by default — Gmail dot-stripping, +tag stripping, googlemail → gmail folding, etc. That's wrong for identity: PicPeak uses email as a login identifier, so `[email protected]` getting silently stored as `[email protected]` means the user can't log in with the address they were invited with. The bug existed at 17 call sites across the codebase (auth, admin user create/update, customer create/update, event create/update on three different routes, customer login, feedback submission). All of them are identity-bearing — none had a legitimate reason to strip dots for deduplication. Fix: introduce one shared options object in `utils/emailNormalization` disabling every provider-specific normalization (gmail_remove_dots, gmail_remove_subaddress, gmail_convert_googlemaildotcom, outlookdotcom_remove_subaddress, yahoo_remove_subaddress, icloud_remove_subaddress). The only default left enabled is `all_lowercase`, which is safe — local-parts are case-insensitive in practice on every major provider, and lowercasing keeps login lookup consistent. Every call site updated to pass the shared options. 7 unit tests pin the preserved-dots, preserved-subaddress, preserved-googlemail-domain, and still-lowercase behaviours so a future refactor can't silently regress. ## Migration note Existing accounts whose emails were already stripped before this fix remain with the stripped form in the DB. The fix takes effect for new invitations going forward. If an admin re-invites an existing user with the un-stripped address, that would create a duplicate account — out of scope here; if it becomes a real problem we can add a backward-compat login fallback (try lookup with dot-stripped form too) as a separate change. |
||
|
Paul Nothaft
|
832f7bad45 |
feat(admin): update-available modal with aggregated changelog + upgrade command (#567)
Closes #567. The sidebar already had a "vX.Y.Z available" indicator (#566 made it a link to that release's page) but there was no way to read the actual changelog inline or to grab a copy-paste upgrade command. This adds the modal the issue spec'd, layered on top of the existing updateCheckService / environmentService backend infrastructure that already shipped. ## Backend - `updateCheckService.fetchAvailableVersions` now returns full release objects (tag, name, body, publishedAt, htmlUrl) instead of just version strings — body data is what the changelog modal renders. `checkForUpdates` extracts the version strings for its existing consumers; no API change visible to callers. - New `getReleasesSince(currentVersion, channel)` returns the list of releases strictly newer than current, filtered to the user's channel. Reuses the same 1-hour cache as `checkForUpdates` so the modal opening doesn't trigger an extra GitHub round-trip. - New `GET /admin/system/updates/changelog` route in `adminSystem.js`, same auth + UPDATE_CHECK_ENABLED gating as the existing /updates and /updates/instructions endpoints. - 4 unit tests (axios mocked) pin: strictly-newer filtering, channel-scoped, empty array on GitHub fetch failure, empty array when already on latest. ## Frontend - New `UpdateAvailableModal.tsx` — opens from the sidebar chip. Two sections: 1. **How to upgrade** — fetches /updates/instructions for the environment-detected copy-paste command (Docker compose / git / standalone). Copy-to-clipboard button per step. 2. **Release notes** — fetches /updates/changelog for every version between current and latest in the user's channel. Latest is auto-expanded; older releases are collapsed by default (click to expand). Each release also has a "View on GitHub" link to the canonical release page. - Renders release body markdown through the existing safe MarkdownContent component (marked + DOMPurify allowlist). - New `updateDismissal.ts` helper — single localStorage key holds the last-dismissed version. Chip stays hidden until a STRICTLY newer version appears, using the same compare semantics as the backend (stable > beta, higher beta > lower beta, semantic numeric on major.minor.patch). 9 unit tests pin the rules. - `VersionInfo.tsx` — chip is now a button that opens the modal instead of an external link (the #566 link-to-release behaviour is preserved on the modal's per-release "View on GitHub" affordance). Dismissal triggers an immediate re-render so the chip disappears without waiting for the next route change. No new dependencies — uses `marked` + `DOMPurify` that were already present in the bundle for the contract block renderer. |
||
|
Paul Nothaft
|
ab81998996 |
docs(release): establish stable-channel cadence + promotion process (#565)
Closes #565. Beta has been the de-facto stable channel because the actual stable lagged so far behind that new users following the README ended up worse off than users who knew to switch to beta. The fix has two parts: regular stable cuts (the PR #568 promotion is the first one) and a written process so future cuts don't depend on memory. This adds: - RELEASING.md at the repo root — full operational doc with cadence target (4–6 weeks), promotion criteria (CI green + 7-day bug soak + upgrade-walk on real-shaped data + operator smoke), the actual beta→main mechanics including the conflict-resolution checklist we used in PR #568, hotfix backport path (with PR #412 as the worked example), and the project's versioning rules. - CONTRIBUTING.md — replaces the four-line "Release Process" stub (which was wrong; it described a hand-rolled flow that release-please has handled for the last several releases) with a brief summary and a pointer to RELEASING.md. - README.md — one-sentence addition to the existing "Release Channels" section pointing curious users at RELEASING.md. No code change. CHANGELOG.md and version files are intentionally untouched — release-please will catch this on the next regular cut. |
||
|
Paul Nothaft
|
d231623c59 |
feat(admin): link version numbers in sidebar to GitHub release notes (#566)
Closes #566. The admin sidebar showed the running frontend + backend versions as plain text. Wraps each version (and the "update available" indicator) in an anchor pointing at the corresponding GitHub release tag, opening in a new tab so the admin session isn't disrupted. A small githubReleaseUrl helper (extracted to its own module for testability) does the version → URL mapping. Because release-please tags every release as `vX.Y.Z[-beta.N]`, the version string already carries the channel suffix and a pure template covers both stable and beta without branching. Three unit tests pin the URL template — stable, beta-with-suffix, and a defensive check that the leading `v` isn't double-prefixed if a caller accidentally passes a tag-shaped value. |
||
|
Paul Nothaft
|
d5a37df2c4 |
fix(events): preserve branding inheritance when saving events with null color_theme
API-created events (and any event whose `color_theme` is NULL) had two visible bugs in the admin edit page (#550 follow-up — PR #552 fixed the v1 POST write path, this fixes the read/save path): 1. The theme picker initialised to the hardcoded `GALLERY_THEME_PRESETS .default.config` ("Classic Grid", green) — which had nothing to do with the admin's actual branding palette, while the gallery itself was rendering with the branding theme. Confusing visual mismatch. 2. Saving the event for ANY reason (changing the date, password, etc.) wrote `color_theme = 'default'` back to the row because the save handler always emitted the picker's initial preset name. That silently replaced "inherit from branding" with the literal Classic Grid preset, so the gallery's visuals jumped. Two fixes, both in EventDetailsPage: - Add a `themeChanged` flag, defaulted false. Flip in the picker's onChange / onPresetChange / onSyncFromBranding callbacks. The save handler now only writes `updateData.color_theme` when the flag is true, so saving without touching the picker preserves NULL. - When `event.color_theme` is null and `publicSettings.theme_config` (the site branding) is available, initialise `currentTheme` from branding instead of the Classic Grid preset, with currentPresetName set to 'custom' (since inherited branding isn't a named preset). Falls back to the Classic Grid preset only when no branding theme exists either. Combined effect: opening an API-created event shows the same palette the gallery uses, and saving without changing the theme preserves the inheritance. Existing events with a stored color_theme are unaffected (themeChanged stays false → no write, just like before for the common no-change-to-theme save). |
||
|
Paul Nothaft
|
d5823c79d9 |
feat(lightbox): multi-photo Web Share save-to-Photos on iOS (#557)
Extends #531 to the selection-based bulk-download flow. On iOS with a selection at or under MAX_WEB_SHARE_FILES (25), galleryService .downloadSelectedPhotos now routes through navigator.share({ files }) so the photos land directly in Photos via the share sheet's "Save N Images" action. Above the cap, anywhere off-iOS, or on any failure, the existing server-side zip path runs unchanged. The 25-file cap is the empirically-safe ceiling: iOS Safari's share sheet starts choking beyond ~25–30 files, and every File materialises as an in-memory Blob before share() is invoked, so a 500-photo selection would buffer multiple GB on the device. trySaveMultipleToDevice exposes three outcomes: - 'shared' — share() resolved; flow ends - 'dismissed' — user cancelled (AbortError); flow ends without zip fallback so dismissal isn't silently overridden - 'fallback' — capability missing or unexpected failure; caller takes the zip path Partial shares are deliberately avoided: a single failed photo fetch collapses the whole selection back to the zip endpoint rather than sharing only the photos that resolved. All 4 grid callers (PhotoGrid, PhotoGridWithLayouts, GalleryStoryLayout, GalleryPremiumLayout) funnel through downloadSelectedPhotos, so no caller-side changes are needed. Android, desktop, Firefox, and "Download All" are untouched. Layers on top of #556 (iOS-only gating via isIOS()). Builds against the fix/android-download-web-share-554 branch. |
||
|
Paul Nothaft
|
04795219a0 |
fix(lightbox): eliminate download lag on Android by skipping the blob round-trip
`savePhotoToDevice` previously buffered the full image through JS as a Blob on every platform before clicking <a download>. On cellular this added ~5s of dead air between the button press and the browser's download dialog, prompting users to re-click and produce duplicate downloads (#554 follow-up, post-#556). The blob round-trip is only required for the iOS Web Share path (`navigator.share({files})` needs File objects in hand). On Android and desktop the browser can fetch the download URL itself and show its own progress in the notification shade — instantly. So iOS keeps the existing flow; everywhere else gets a direct anchor navigation. The new `triggerDirectDownload` helper uses `api.getUri()` so the path also works in split-origin deployments (where the existing hardcoded `/api/...` pattern used by `downloadAllPhotos` would 404). Tests updated: Android / desktop / regular-Mac branches now assert that `fetchPhotoBlob` is NOT called and `triggerDirectDownload` is invoked with a `/gallery/{slug}/download/{id}` URL. iOS tests unchanged. |
||
|
Paul Nothaft
|
2a309c75a7 |
fix(lightbox): restrict Web Share save-to-Photos path to iOS (#554)
PR #531 routed the single-photo download through navigator.share() whenever canShare({files}) returned true, on the assumption that any mobile share sheet would expose a "Save Image" action. That holds on iOS — Safari's share sheet has a first-party "Save to Photos" entry — but on Android the system share sheet only lists installed apps that registered an image/* intent (WhatsApp, Telegram, Drive, etc.). There is no built-in save-to-Gallery action, so Android users tapping the download button got an app-picker instead of the file saved to their device. Fix: gate the Web Share branch behind a UA-based isIOS() check. Android, desktop, and everything else fall through to the existing <a download> path (file lands in Downloads, visible in the Photos / Gallery app afterwards — same behaviour as before #531). iOS — including iPadOS 13+, which reports as MacIntel + touch — keeps the share-sheet flow that drops directly into Photos. UA-sniff is the only available signal here: canShare({files}) is true on both iOS Safari and Chrome Android, so feature detection cannot distinguish them. Tests pin all six scenarios — iOS share path, Android download fallback (even with canShare=true), desktop, iPadOS-as-Mac detected as iOS, regular Mac NOT detected as iOS, AbortError dismissal preserved (no surprise fallback), and non-Abort share() rejection falls back to download. |
||
|
Paul Nothaft
|
1b521e761c |
fix(api/v1): accept color_theme + create feedback row on event create (#550)
POST /v1/events was a strict subset of the admin create path: it did not accept color_theme on the body, and it skipped the event_feedback_settings insert that adminEvents.js does. Two visible bugs followed. 1. Editing an API-created event in the admin UI snapped the theme picker to GALLERY_THEME_PRESETS.default (EventDetailsPage.tsx falls through to the default preset when event.color_theme is falsy), and saving wrote that default back. Inherited themes were silently clobbered. 2. The "Enable Guest Feedback by default" admin setting (#520) did not apply to API-created events. With no event_feedback_settings row the gallery UI reads feedback as off, regardless of event_default_feedback_enabled. Fix mirrors the admin path: - color_theme accepted on the request body (optional, persisted as-is — preset name or JSON-encoded ThemeConfig, same shape adminEvents stores). - feedback_enabled accepted on the request body; when omitted, falls back to the event_default_feedback_enabled global setting (same behaviour adminEvents.js:511-520 implements via readBooleanSetting). - event_feedback_settings row inserted when feedback resolves to true, using the same sub-flag defaults as the admin form (everything on except require_name_email). OpenAPI JSDoc updated so docs.picpeak.app picks up the new fields. Tests cover all four scenarios — explicit color_theme persisted, JSON theme persisted verbatim, explicit feedback_enabled creates the row, omitted feedback_enabled honours the global setting, and a validator regression for non-boolean feedback_enabled. |
||
|
Paul Nothaft
|
5488de3383 |
fix(nginx): honour outer X-Forwarded-Proto when behind a reverse proxy (#547)
When PicPeak runs behind NPM / Traefik / Caddy, the inner nginx receives plain HTTP from the outer proxy. The previous `X-Forwarded-Proto $scheme` therefore always forwarded "http" to the backend, even when the public URL was HTTPS. Express has `trust proxy` enabled for loopback/linklocal, so req.secure became false, the Secure cookie flag wasn't set, and generated URLs (cookies, tokens) used http://. Add a top-of-file `map` block that picks the incoming X-Forwarded-Proto when present and falls back to `$scheme` for direct access. Applied to both nginx.conf (bundled production image) and nginx.dev.conf. Validated with `nginx -t` against nginx:1.28-alpine (the same image used by Dockerfile.prod / Dockerfile). |
||
|
Paul Nothaft
|
dba98f1325 |
chore: address clawpatch review findings (test scope, deps, legal-page hardening)
- frontend: `npm test` now runs all 7 vitest suites instead of one hardcoded file; the previously skipped ProtectedImage / Skeleton / usePublicSettings / contrast / themeMigration / url suites are now active in CI - frontend: wrap ThemeCustomizerEnhanced test in QueryClientProvider so the newly-enabled run passes (component uses useQuery internally) - root: drop unused better-sqlite3 / canvas / node-fetch + their prebuild-install/tar-fs override (backend keeps its own copies); add dotenv so playwright.config.ts can load on a clean install; add name/version/private - LegalPage: scheme-validate external_url before window.location.replace so a CMS edit can't redirect visitors to javascript:/data: - LegalPage: force rel="noopener noreferrer" on target="_blank" anchors in sanitized CMS HTML to block reverse-tabnabbing |
||
|
Paul Nothaft
|
efa6b4a205 |
fix(brand-title): runtime substitution so GHCR-image users can override (#521 follow-up)
@Rekoo-PS confirmed the prior #521 fix landed on beta but reported the preview still shows the default "PicPeak" title — their brand is "arkan-studio". Root cause: that fix used Vite's build-time %VITE_DEFAULT_TITLE% substitution. Self-hosters running the pre-built ghcr.io/the-luap/picpeak/frontend image can't override at build time without rebuilding, so they were stuck with whatever the upstream build baked in. Pivot to runtime substitution: the frontend container now reads BRAND_TITLE / BRAND_DESCRIPTION env vars on startup and envsubsts them into index.html. Change the values in .env, restart the frontend service, done — no rebuild required. Mechanics: - frontend/index.html: tokens are now ${BRAND_TITLE} / ${BRAND_DESCRIPTION} (shell expansion syntax, passes through Vite unchanged into the built dist). - frontend/Dockerfile: install gettext (provides envsubst), snapshot /usr/share/nginx/html/index.html → index.html.tpl at build, install docker-entrypoint.sh, wire ENTRYPOINT to it. The .tpl is the immutable source — every container start re-renders index.html from .tpl, so restarts pick up new env values cleanly (no accidental "first-boot env stuck forever" trap). - frontend/docker-entrypoint.sh: applies defaults if env unset, runs envsubst (locked to BRAND_TITLE + BRAND_DESCRIPTION explicitly so /assets/*.js template literals aren't touched if anyone ever extends substitution to the bundle), execs nginx. - frontend/vite.config.ts: drop the htmlTitleDefaults plugin — no longer needed since substitution is fully runtime. - frontend/.env.example + .env.production.example: drop the VITE_DEFAULT_* docs (the vars no longer have effect). - docker-compose.yml + docker-compose.production.yml: pass BRAND_TITLE / BRAND_DESCRIPTION env into the frontend service with sensible defaults so unconfigured installs work unchanged. - .env.example: add BRAND_TITLE / BRAND_DESCRIPTION with comment pointing at the social-preview use case. Verified end-to-end against the built image: - BRAND_TITLE="Arkan Studio" BRAND_DESCRIPTION="Wedding photographs by Arkan Studio" → index.html serves <title>Arkan Studio</title> + og:title="Arkan Studio" + og:description correctly substituted. - .tpl preserves ${...} tokens so the next restart can re-substitute. - Bundle assets unaffected. - Defaults applied when env unset → <title>PicPeak</title>. Docs PR in picpeak-docs describes the two new env vars under "Social link preview fallback" in the environment-variables reference. Refs: #521 |
||
|
Paul Nothaft
|
53139b8cb8 |
fix(lightbox): pan zoomed image with single-finger touch on mobile (#532)
@Rekoo-PS reported zoom on mobile only shows the centre crop — the
image zooms but you can't pan around to see other parts. Single-finger
touch was being routed through the carousel-swipe branch which is
gated on zoom <= 1 (so swipe doesn't fight with pan), so when zoomed
the touch hit no handler at all.
Desktop has the equivalent path via handleMouseDown / handleMouseMove
(line 364), which is why this only manifests on mobile.
Add a single-finger pan branch to the touch handlers that mirrors the
mouse path:
- handleTouchStart: when zoom > 1 and one finger, record dragStart
relative to the existing dragOffset (so subsequent moves continue
from where the last pan left off, not from origin).
- handleTouchMove: when isDragging + zoom > 1 + one finger, update
dragOffset from touch position.
- handleTouchEnd: clear the isDragging flag (offset persists so the
image stays where the user left it).
Also fix a latent bug surfaced while reading the pinch-zoom path:
when pinch-out drops zoom back to 1.0, dragOffset wasn't reset, so the
photo sat off-centre at natural zoom. Re-centre in handleTouchMove
when newZoom drops to <=1 with a non-zero offset.
Carousel swipe stays disabled when zoomed (existing behaviour). Mouse
path untouched. Pinch-to-zoom path untouched.
Refs: #532
|
||
|
Paul Nothaft
|
b2bbf7efb5 |
feat(lightbox): save photo to Photos app on mobile via Web Share (#531)
@Jasper2213 reported non-technical clients struggle to get downloaded
photos into their Photos / Gallery app — current flow goes through the
Files folder, requires unzipping for the bulk download, and is hard to
explain over email. Browsers can't write directly to the OS Photos app
(it's a protected location), but navigator.share({ files: [...] }) opens
the native share sheet which on iOS includes "Save Image" and on Android
includes "Save to Photos" / "Save image" — exactly the affordance non-
technical users are looking for.
Plumbed through three layers:
1. galleryService — new savePhotoToDevice(slug, photoId, filename).
Fetches the photo blob, probes navigator.canShare({ files: [file] })
with a representative File (some browsers return true for empty
files arrays even when they won't accept a non-empty one), and:
- shares if supported,
- falls back to the existing <a download> path otherwise.
AbortError on share() means the user dismissed the sheet — that's
a choice, not a failure, so no fallback. Any other error falls
through to a regular download so the user still gets the file.
Refactored the existing downloadPhoto to share the fetch + trigger
helpers (no behaviour change for the other 3 callers; they keep
the regular download path).
2. useGallery — new useSavePhotoToDevice() hook next to the existing
useDownloadPhoto(). Onsuccess toast omitted because the share-sheet
path doesn't finish from this code's perspective — the OS UI takes
over and the user picks the destination, so "Photo downloaded" is
misleading. Fallback path stays silent to keep the two flows
symmetrical (the file appearing in Downloads is its own signal).
3. PhotoLightbox — swap the existing useDownloadPhoto call site to
useSavePhotoToDevice. No UI change. Desktop unchanged. Other
download buttons (PhotoGrid, PhotoGridWithLayouts, GalleryView
bulk) still use useDownloadPhoto — scoping this PR to the
lightbox download button per the discussion thread.
Browser support:
- iOS Safari 15+: Web Share Files → "Save Image" → Photos ✓
- Chrome Android: Web Share Files → "Save to Photos" / "Save" ✓
- Desktop Chrome: canShare returns false → regular download ✓
- Desktop Safari: canShare returns false → regular download ✓
- Firefox (any): no Web Share File support → regular download ✓
No new tests — the flow is browser-API-driven; jsdom doesn't model
navigator.share or canShare, so a meaningful unit test would mostly
exercise the mock rather than the contract. Verified the build is
clean (tsc --noEmit + vite build both pass).
Refs: #531
|
||
|
Paul Nothaft
|
600c29db8a |
fix(lightbox): fill the heart icon when liked (#538 follow-up)
@Tietge86 spotted that both branches of the heart-icon className were `text-white` — the conditional was a no-op, the `fill-current` class that would actually fill the icon was missing entirely. The button background was turning red on like, but the heart icon stayed as a white outline against the red, making it nearly invisible. Move text-white outside the conditional (always white against the red/dark backgrounds the button uses), and add fill-current to the liked branch so the heart fills in. Same shape as bug 2 of the original report — the like state needed to be visually unambiguous. PhotoLikes.tsx was already fixed in this PR; this catches the equivalent latent bug in the inline lightbox toolbar button. Also: bug 4 of the original report (recovery flow) turned out to be SMTP misconfig on the reporter's end (mailhog silently dropping emails), not a PicPeak bug. Confirmed in this thread; no further backend changes needed. Refs: #538 |
||
|
Paul Nothaft
|
5311588baf |
fix(feedback): three guest-mode bugs reported in #538
Picks up three of the four bugs from @Tietge86's report. Bug 4
(recovery flow) needs network-tab data from the reporter before a fix
makes sense; commented on the issue asking for the HTTP status from
POST /gallery/:slug/guest/recover.
Bug 1 — "Liked" filter empty in guest identity mode (GalleryView.tsx)
The feedback filter was scoping by `photo.like_count > 0`, which is
the global aggregate across all guests. In guest identity mode the
filter intent is "show MY picks", so a guest who'd liked photos that
nobody else had touched got an empty grid.
Fix: pull the current guest's interactions from /my-feedback (already
keyed by x-guest-token in the api interceptor) into per-type
photo-id Sets and filter against those when identity_mode === 'guest'.
Falls back to the aggregate-count check in simple mode where there's
no per-person identity to scope by. Same per-guest scoping applied to
the chip-count labels ("Liked (N)" etc.) so the chip number matches
what the filter actually surfaces — otherwise the chip says one
count globally and the filter shows a different (smaller) one, which
is the same UX cliff #538 originally surfaced.
The /my-feedback query is gated on isGuestIdentityMode (not on
filterType being feedback-related) so the chip counts are populated
on first render. One extra request per gallery load in guest mode;
payload is tiny.
Bug 2 — Liked state on PhotoLikes button invisible
bg-red-50 text-red-600 is barely visible against most themes,
especially dark + brand-coloured backgrounds. Switch to the same
filled state the lightbox toolbar already uses
(bg-red-500/80 text-white) so the like registers visually.
Heart icon's fill-current was already there for the liked state —
unchanged.
Bug 3 — Aggregate like count leaks in lightbox toolbar
PhotoLightbox.tsx rendered {likeCount} unconditionally next to the
inline heart button. When the admin has show_feedback_to_guests off,
guests still saw how many other guests had liked a photo (the count
is an admin-only metric in that mode). Gate the span on
feedbackSettings?.show_feedback_to_guests, matching how the rest of
the lightbox toolbar treats that toggle. Also added show_feedback_to_guests
to the local feedbackSettings TS type (backend already returns it).
Tests: tsc --noEmit clean. No new unit tests — bug 1 is data-flow
plumbing best validated via manual / e2e (the existing my-feedback
endpoint and feedbackService are already covered upstream); bugs 2/3
are CSS and a conditional render.
Refs: #538 (bugs 1, 2, 3 of 4)
|
||
|
Paul Nothaft
|
4d3f2470bc |
ci(schema-drift): handle absent migrations table in precondition (#530)
First CI run failed at the precondition check because the SQL `CASE WHEN to_regclass(...) IS NULL THEN 0 ELSE (SELECT count(*) FROM migrations)` expression doesn't short-circuit at parse time — Postgres parses the subquery against `migrations` even when the outer guard would skip it, fails the run with "relation 'migrations' does not exist". initializeDatabase() doesn't create the `migrations` tracking table — that's the migrate:safe runner's responsibility — so in the recovery scenario the table genuinely doesn't exist yet. Both "absent table" and "present but empty table" are valid recovery states. Split the check into two shell steps: to_regclass first, then count only if the table exists. Avoids the parse-time subquery error and accepts either state. |
||
|
Paul Nothaft
|
8f0108ce23 |
feat(install): skip legacy chain when modern bootstrap fingerprint detected (#530)
Refined from the original #530 framing after a dry-run uncovered that the "bootstrap vs migration chain" diff produces mostly noise — most of the ~200 lines of difference are expected (migrations add new tables and columns over time). initializeDatabase() isn't a parallel path that diverges from migrations; it's invoked by migration 001 itself, so every normal install/upgrade runs both. The genuine drift hazard surfaced during the dry-run: a DB with the modern bootstrap tables but an empty `migrations` table (which happens when a backup was restored that lost the migrations table, or someone invoked initializeDatabase() outside the runner, or the DB was moved between systems without copying the migrations row) fails to upgrade. Failure mode: 1. detectExistingSchema sees the bootstrap tables + empty migrations, treats it as an "existing deployment". 2. Runs the legacy chain first. 3. legacy/008 renames email_templates.subject → subject_en. 4. core/029 (later in the chain) inserts email templates referencing the pre-rename `subject` column. 5. Postgres rejects: column "subject" doesn't exist; subject_en is NOT NULL with no default. Fresh installs avoid this because they only run core/* (and core/059 handles the rename AFTER core/029 has inserted). Real legacy upgrades avoid it because their migrations table already records legacy/008–028 as applied historically. Fix in detectExistingSchema: - Detect the modern bootstrap fingerprint (photo_categories + cms_pages both present, which initializeDatabase produces as part of the consolidated post-004-era bootstrap). - When matched, enumerate every file in migrations/legacy/ and mark each as applied. This puts the recovery state on the same code path fresh installs use — only core migrations run, in core order. - Real legacy upgrades that already have entries in the migrations table hit no-op markings (markMigrationAsApplied skips duplicates), so their behaviour is unchanged. New CI workflow (`.github/workflows/schema-drift.yml`): - Boots fresh postgres. - Seeds via `node -e \"require('./src/database/db').initializeDatabase()\"` — reproduces the recovery state in one line. - Runs `npm run migrate:safe`. - Asserts: precondition (bootstrap fingerprint + empty migrations table), migrate:safe exits 0, final schema has ≥40 tables (soft floor, not exact pin so future migrations don't force workflow edits), legacy migrations marked applied (confirms the fingerprint check actually fired vs. the chain silently bailing). - Triggers only on PRs that touch backend/migrations/**, src/database/db.js, knexfile.js, or this workflow. Manually verified end-to-end before this commit: Before fix: migrate:safe dies at core/029 with NOT NULL violation on email_templates.subject_en (17/48 tables present). After fix: 82 migrations applied + 27 marked applied = 109 total, final state has all 48 tables matching fresh-install. Issue body in #530 has been updated to match this refined scope. Refs: #530, #484, #519 |
||
|
Paul Nothaft
|
e8c2212dad |
refactor(slug): extract shared slugify util + scope adminPhotos category lookup (#525)
Folds all three follow-up items tracked in #525 into one commit: 1. Mirror PR #500's category scoping on adminPhotos.js. The admin upload route at adminPhotos.js:231 still accepted any category_id without event scoping — quietly less strict than the public v1 API after #500 landed. Same one-liner fix (event_id OR is_global) with a matching 400 response shape so admin + v1 stay consistent. 2. Extract a shared slugify() in backend/src/utils/slug.js with the NFD-strip-combining-marks fix from #502, and route 5 callers through it: - adminEvents.js (event-name slug) - events.js (event-create slug) - v1/events.js (replaces local slugify helper) - adminArchives.js (archive→category slug) For pure-ASCII input the output is byte-identical to each old inline pipeline, so existing slugs in the DB keep round-tripping cleanly via lookup. Accented inputs now transliterate (Família → familia) instead of dropping the diacritic (Família → f-mlia). adminCategories.js stays with its own pipeline (underscores-as- word-chars semantics differ from the events-style transform — changing would silently shift wedding_party → wedding-party on new inserts). xmpGenerator.sanitizeKeyword stays unchanged for the same compat-cautious reason. 3. Cover the v1 upload happy path. Existing test only exercised the 400-out-of-scope branch. Add two happy-path cases that stub sharp / generateThumbnail / storage.putFromFile and pin the response shape (id, category_id, type, etc.) plus the collage- slug → type='collage' flip. Temp file recreated in beforeEach because the handler unlinks it on success. Tests: - New slug.test.js: 22 cases pinning ASCII parity with the legacy pipeline (so the refactor is provably non-breaking for existing data) and the corrected accent handling across de/es/fr/nl/pt inputs, plus CJK and edge-case behaviour. - events.category.test.js: 4 tests total (2 existing + 2 new happy path). - galleryOgService.shareImage.test.js: 11 (3 added in #521 + 8 pre- existing) still pass. 37 tests pass across the three touched files. Refs: #525, follows up #500 and #502 |
||
|
Paul Nothaft
|
4b4ecfdf71 |
fix(header): hide language name on mobile to free the title (#523)
@Rekoo-PS reported the LanguageSelector pushing into the company-name title on narrow viewports — the button always rendered Globe + flag + full language name (~120px), and on mobile that pinched the left-side title cluster in AdminHeader. Wrap the name in `hidden sm:inline` so <sm the button collapses to just Globe + flag, matching the existing "hidden xl:block" pattern on the date display in the same header. Self-explanatory at icon-only width (users see their current flag and a globe), and the dropdown still shows full names when opened. Title/aria-label keep the name discoverable for screen readers + tooltip hover on the icon-only state. Refs: #523 |
||
|
Paul Nothaft
|
b960639035 |
fix(og): brandable static title + wider crawler UA coverage (#521)
@Rekoo-PS reported that gallery URLs sent via the WhatsApp Business API render an unbranded "PicPeak - Photo Sharing Platform" preview even though manual link sends from the WhatsApp app pick up the per-event rich preview correctly. Two root causes, two fixes: 1. WhatsApp Business and 3rd-party preview services (Twilio, LinkPreview.net, etc.) don't always crawl with the recognisable "WhatsApp/X.Y.Z" UA we matched in nginx + galleryOgService. Extend the regex (both copies) to also catch WhatsAppBot, wa-bot, LinkPreview, and Slack-ImgProxy. 2. Even with broader UA coverage, some senders cache metadata with no UA at all and fetch the static SPA shell. That shell's <title> was hard-coded to "PicPeak - Photo Sharing Platform" — embarrassingly generic for any self-hosted brand. Switch to Vite's %VITE_DEFAULT_TITLE% / %VITE_DEFAULT_DESCRIPTION% HTML substitution so self-hosters can bake their brand into the fallback at build time. Defaults stay "PicPeak" so the upstream image doesn't change behaviour for anyone. The per-event rich preview path (handleGalleryOgRequest, fired on matched crawler UAs) is unchanged — this only improves the fallback for unrecognised UAs and for the SPA-shell title that humans see in their browser tab. Adds a vite.config plugin to provide the defaults when env vars aren't set, so unsubstituted "%VITE_..." literals never reach the built HTML. Adds .env.example entries explaining the override. Tests: extend galleryOgService.shareImage.test.js with an isSocialCrawler suite that pins every documented UA (incl. the new ones) plus three browser UAs (negative) and null/empty edge cases. Verified locally: `vite build` with VITE_DEFAULT_TITLE="MyBrand" produces <title>MyBrand</title> + og:title="MyBrand"; without the env var falls back to "PicPeak". Refs: #521 |
||
|
Paul Nothaft
|
3465b55abc |
feat(events): default Guest Feedback ON via admin setting (#520)
@Rekoo-PS asked for an admin-level switch so new events can have Guest Feedback enabled out of the box instead of toggling it on every time. Mirrors the existing event_default_require_password pattern (#317) — same shape end-to-end, same set of five files. - publicSettings.js: whitelist + expose event_default_feedback_enabled (defaults to false to match the prior hard-coded form default; no behaviour change for existing installs until an admin flips it). - adminEvents.js: rename `feedback_enabled = false` destructure to `feedback_enabled: feedbackEnabledInput` so we can distinguish "omitted" from "explicit false", then resolve the default from the setting only when the caller omitted it — identical to the require_password handling a few lines above. - Frontend EventSettings type + state + loader: new boolean, default false. - EventsTab: toggle UI right under "Require password by default". - CreateEventPage: one-shot useEffect that seeds feedback_settings.feedback_enabled from the public setting on first load (mirrors the require_password seed effect right above it). Sub-toggles (likes / ratings / comments) keep their hard-coded true defaults so flipping the master setting immediately gives sensible behaviour without a second admin setting to manage. Refs: #520 |
||
|
Paul Nothaft
|
d44e1adba7 |
fix(lightbox): hide comments toggle when allow_comments=false (#518)
@Rekoo-PS reported the MessageSquare comment button stayed visible in the lightbox toolbar even when guest comments were disabled. Same class of bug as #513 (per-photo Like button missing the master gate) but on a different control. The Like and Rating buttons in the lightbox toolbar gate correctly: feedbackEnabled && feedbackSettings?.allow_likes feedbackEnabled && feedbackSettings?.allow_ratings The MessageSquare button only checked feedbackEnabled. Since likes and ratings already have their own inline buttons in the same toolbar, this third button is effectively the "open comments panel" affordance — its badge counts comments, its tooltip mentions comments. When comments are off it has nothing meaningful to do. Add allow_comments to the local feedbackSettings type (the backend already returns it via galleryFeedback.js:33) and gate the button on feedbackEnabled && feedbackSettings?.allow_comments. Refs: #518 |
||
|
Paul Nothaft
|
763fd4593f |
ci(install-smoke): use BusyBox-compatible ps in node-user check
Alpine ships BusyBox ps (no -p PID, no pgrep), which failed CI on the first run of this workflow with "ps: unrecognized option: p". Replace the pgrep-then-ps chain with `ps -o user,comm | awk '$2=="node"'` which works on both BusyBox (Alpine, in the container) and procps (the GitHub runner host, though we don't use it here). |
||
|
Paul Nothaft
|
1505775678 |
fix(install): self-chowning entrypoint kills fresh-install restart loop (#484)
The fresh-install restart loop reported by @MrGabri (and confirmed by
@AloePacci with the user:0:0 workaround) had a clear root cause:
- Dockerfile pinned USER nodejs (UID 1001) before the entrypoint
ran, so the existing chown branch in init-production.sh:13 was
dead code.
- wait-for-db.sh (the actual entrypoint, not init-production.sh)
silently swallowed mkdir/EACCES on bind mounts with || true,
then a downstream migration error surfaced as the visible failure.
- Net effect on a typical Linux host where the bind-mount dir is
owned by UID 1000: container can't write, exits non-zero,
restarts forever with no clear error.
Switch to the standard Docker drop-privileges pattern:
1. Install su-exec, drop `USER nodejs` from the Dockerfile —
container now starts as root.
2. wait-for-db.sh: if running as root, chown /app/storage,
/app/data, /app/logs to nodejs and re-exec self via
su-exec nodejs:nodejs. App still ends up running as UID 1001.
3. Preflight check for non-root invocations (compose `user:`
overrides): verify the bind mounts are actually writable
before continuing. If not, exit 1 immediately with an
actionable error pointing at the docs — no more silent
restart loops.
Also:
- Delete backend/init-production.sh. It was an orphan — no caller
in the Dockerfile, compose, or anywhere else. Its chown logic
looked authoritative enough that @MrGabri ran it manually trying
to debug, which is what finally surfaced the EACCES.
- docker-compose.yml: drop user: + PUID/PGID env. The pattern-B
UID-matching workaround they implemented is obsolete now that
pattern A (root-then-drop) is in place.
- .env.example + README: drop PUID/PGID documentation.
- Add fresh-install smoke test workflow. Boots backend + postgres
against bind mounts owned by UID 1000 (the GitHub runner UID,
and the common-mismatch case on Linux hosts) and verifies:
+ container reaches healthy without restart-looping
+ chown happened (dirs now owned by 1001 inside the container)
+ node runs as nodejs, not root (su-exec drop worked)
+ /health returns status:ok
+ with --user 5005:5005 + unwritable mounts, preflight exits
loud with the expected error string
Verified locally end-to-end against a fresh Postgres + UID-501-owned
bind mount: backend reaches healthy in ~20s, chown applied, node
runs as nodejs, no restart loop. Docs in picpeak-docs cover the new
behavior + a Troubleshooting section for the install-path bugs
fixed in #484/#494/#511/#488.
Refs: #484
|
||
|
Paul Nothaft
|
51890e1aa5 |
fix(i18n): drive customer "Preferred language" select from SUPPORTED_LANGUAGES (#510)
Audit follow-up to the Spanish-locale commit: `CustomerDetailPage` had a hardcoded `<option>` list for the customer's preferred-language selector — 5 entries (en/de/nl/pt/ru) that were missing both fr (an existing gap) and es (the new one). Every other language selector in the frontend (the navbar `LanguageSelector`, the `GeneralTab` default- language dropdown, the `EmailConfigPage` per-language tabs) already reads from the shared `SUPPORTED_LANGUAGES` constant, so adding es there was enough for those. This one had drifted. Now mapped from `SUPPORTED_LANGUAGES` so future locales only need to touch one place. |
||
|
Paul Nothaft
|
061712ebf1 |
feat(i18n): add Spanish (es) locale (#510)
Contributed by @AloePacci on issue #510. Drops their es.json into the existing locale set, registers Spanish in the language selector with a flag SVG matching the inline style of the other six locales, and extends the email pipeline so es-language guests receive a localised email subject/body where available. Coverage: - frontend/src/i18n/locales/es.json — 2132 translated keys. ~824 EN keys are not yet covered; i18next's `fallbackLng: 'en'` handles those at runtime so the UI never renders a missing key. fr/nl/pt/ru have a similar (smaller) gap and ship the same way. - LanguageSelector.tsx — added the ESFlag inline SVG (red/yellow/red horizontal bands, official #AA151B + #F1BF00; no coat of arms to stay consistent with the other simple flag components) and a new entry in SUPPORTED_LANGUAGES. - emailProcessor.js — added .es to the domain-language heuristic, and an `es:` row to the three inline-translated snippets (passwordSecurityI18n / noPasswordI18n / passwordSetAtCreationI18n). - 106_seed_es_email_template_translations.js (new) — idempotent seeder for the four customer-facing templates AloePacci translated: gallery_created, expiration_warning, gallery_expired, archive_complete. Mirrors the pattern from 099. Template keys without an `es` row fall back to `en` via the existing resolution chain in emailProcessor.processTemplate — no functional gap, just untranslated copy until someone fills them in. What I deliberately did NOT take from the contribution: the proposed in-place edit of migration 075 (history mutation — won't reseed for existing installs anyway) and the whitespace/`gallery_list_html`-drop churn in emailProcessor.js (would have regressed the #354 follow-up). The semantic additions from those files are preserved via 106 and the targeted edits above. |
||
|
Paul Nothaft
|
98f3c3df41 |
fix(upload): restore configurable batch-size for reverse proxies (#509)
Regression of #208. PR #214 (commit |
||
|
Paul Nothaft
|
33de294d57 |
feat(lightbox): surface original camera filenames (#508)
Photographers running the gallery as a client-selection tool want to map a guest's picks back to source files for retouching. The `general_use_original_filenames_for_downloads` toggle (#493) already does this on the download side; this extends the same toggle to the in-lightbox view so the camera filename is visible alongside the photo while it's being looked at. Tied to the same toggle on purpose — one switch controls both surfaces. Off by default; existing galleries keep showing only the position counter. Wiring: - gallery.js serializes `photos[].original_filename` and surfaces the resolved toggle as `event.use_original_filenames` so the client can decide whether to render it. - The bespoke `PhotoLightbox` renders the original filename (falling back to the storage filename only for pre-migration-062 uploads) in a muted line under the position counter, truncated to keep the toolbar tidy. - `GalleryStoryLayout` mounts the same `PhotoLightbox`, so its rendering follows along. - `GalleryPremiumLayout` uses `yet-another-react-lightbox` instead; added the Captions plugin and a `title` field on the slides so the same name appears as a caption when the toggle is on. The remaining layouts feed back into the main `PhotoLightbox` via `PhotoGridWithLayouts`, so the prop reaches them through the layout props bag. |
||
|
Paul Nothaft
|
38343e62de |
fix(downloads): apply original-filename toggle to individual downloads too (#507)
Follow-up to #498. The toggle reached zip downloads but single-photo downloads still landed on disk with the renamed `event_individual_NNN.jpg` even when the admin had flipped the setting on. Two reasons, fixed in lockstep: - Frontend overrode the server's Content-Disposition with a hardcoded `<a download="X">` attribute (`gallery.service.ts`, `photos.service.ts`) where X was the sanitized `photo.filename` known to the client. So the backend's correctly-formed `Content-Disposition` never reached the disk write. Added `parseContentDispositionFilename` (RFC 5987 + plain `filename=` fallback) and let the server name win when present. - `secureImages.js` (enhanced/maximum protection's secure-download route) was missed in #498 and still emitted a hardcoded `filename="${photo.filename}"` regardless of the toggle. Wired it through `getUseOriginalFilenames` + `buildContentDisposition` so it matches the regular gallery download path. Also exposed `Content-Disposition` via CORS so split (cross-origin) frontend deployments can still read it from JavaScript. Same-origin Docker deploys already had access; this is a defensive addition for the split case. |
||
|
Paul Nothaft
|
9d2db9a73b |
fix(gallery): hide Like button when guest feedback is off (#506)
Four gallery layouts were rendering the per-photo Like button without gating on the master "Guest Feedback" toggle, so a guest still saw a heart icon and could submit likes on events where the host had turned feedback off. The other layouts (Grid / Justified / Masonry / Story) already gated correctly with `feedbackEnabled && allowLikes` — Rekoo-PS's note that "it's hidden in some themes" matches that split. - CarouselGalleryLayout, MosaicGalleryLayout, TimelineGalleryLayout: the existing conditional checked only `feedbackOptions?.allowLikes`, missing the `feedbackEnabled` master gate. Added it inline. - GalleryPremiumLayout: the per-card Like button rendered unconditionally because PhotoCard never received the allow-likes signal. Added an `allowLikes` prop on PhotoCardProps, plumbed `feedbackOptions?.allowLikes` down from the parent, and wrapped the button in `feedbackEnabled && allowLikes`. The follow-up "default guest-feedback ON" request from Rekoo-PS in the comments is a separate feature (admin > General > Event Creation default) and out of scope for this fix. |
||
|
Paul Nothaft
|
d2d55098d6 |
fix(lightbox): align swipe-neighbour height + stop black flash on commit (#505)
Two adjacent swipe-time defects, one diagnosis each: 1. Height differed between current and neighbouring slides during a swipe but matched when the arrow buttons advanced the carousel. Cause: neighbour slides wrap their image in a div with extra `px-2` horizontal padding while the current slide does not. `object-contain` then sees a narrower container on neighbours, so wide images cap on width first and render shorter than the same image at the current position. Removed the padding so both slots share the same container geometry. Arrow-button navigation looked fine because it never showed the neighbour layout side-by-side. 2. The image flashed black for ~100–400 ms each time a swipe committed to the next slide. Cause: the 3-slide track has no React keys, so React reconciled slides by position. After commit the photo at every position changed (`prev → current → next` shifts left), every slot's `<AuthenticatedImage>` saw a new `src` prop, and its fetch effect restarted from the placeholder state — including the slot that was the user's "next" slide a moment ago and held a fully-loaded image. Added a stable `key` derived from `photo.id` so React MOVES existing DOM nodes across slots instead of refetching. 2-photo galleries are a key-collision edge case (`prev === next`), so they fall back to slot-prefixed keys to keep siblings unique; behaviour there is no worse than today. |
||
|
Paul Nothaft
|
577c4bdf29 |
fix(upload): wire drag-and-drop on admin + user upload zones (#504)
The dashed-border upload area in `PhotoUpload` (admin) and `UserPhotoUpload` (gallery user-upload) is styled and labelled as a drop zone — every locale's `upload.clickToUpload` already reads "Click to upload or drag and drop" or its translation — but neither component had any `onDragOver` / `onDragEnter` / `onDragLeave` / `onDrop` handlers. Files dropped on the zone fell through to the browser's default behaviour (open the image in a new tab), which is what Rekoo-PS reported. Added native HTML5 drag-and-drop wiring on both components, plumbed through the same filter/limit/toast pipeline used by the click path (`addFiles` helper). Visual highlight on drag-over via an `isDragOver` flag; the listener guards against the `dragleave` strobing that fires on every child node. Also reset the `<input>` value after onChange so re-picking the same file still triggers an upload — matches the new drop-then-pick mental model. |
||
|
Paul Nothaft
|
86b33d4dda |
fix(install): silence clean-install postgres log noise (#484)
Two latent install-time issues that emitted scary postgres ERROR lines on every fresh start but didn't actually break anything. MrGabri flagged them after #494 had already cleared the FK-ordering crash. 1. Migration 035 builds three `CREATE INDEX` statements against `backup_runs(created_at, …)`, but 029 creates the table with `started_at` and no `created_at`. The wrapping try/catch silently swallowed the resulting `column "created_at" does not exist` ERROR, so the migration "succeeded" without ever creating the indexes. Switched 035 to reference `started_at` (same chronological semantics) and added migration 105 to create the same indexes idempotently for deployments whose 035 already ran and silently failed. 2. `run-migrations-safe.js` snapshots `appliedFilenames` *before* `detectExistingSchema()` runs. When `detectExistingSchema()` inserts a row for e.g. `004_add_categories_and_cms.js` (because its tables exist from a partially-completed prior install), the subsequent migration loop still doesn't know about that insert, attempts the legacy migration anyway, and its transaction-internal `insert into migrations` conflicts with the row already there. Re-query the applied set after detectExistingSchema so the loop sees the corrected snapshot. No behavioural change for healthy installs. New installs no longer log the `column "created_at" does not exist` or `duplicate key value violates unique constraint "migrations_filename_unique"` ERRORs. |
||
|
Paul Nothaft
|
7eeef2ba98 |
feat(downloads): preserve original camera filenames on download (opt-in) (#493)
New Settings → General toggle `Use original filenames on download` (off by default). When on, single-photo downloads, bulk/selection zips, and per-event archive zips surface `photos.original_filename` instead of the sanitized storage filename. Storage paths are unchanged. - Content-Disposition uses RFC 5987 (`filename=` ASCII + `filename*=UTF-8''…`) so unicode camera filenames survive while header-injection bytes are stripped. - Zip entries are deduplicated with a deterministic `_1` / `_2` suffix on collision (folder structure preserved in archive zips). - Pre-generated download-all zips and the in-memory setting cache are invalidated when the toggle flips so the next download rebuilds with the new names. - Falls back to the storage filename whenever `original_filename` is null (legacy uploads predating migration 062). |
||
|
Paul Nothaft
|
61f1d13210 |
feat(lightbox): medium-resolution preview tier (#492)
Adds an opt-in lightbox preview tier so guests open photos against an
aspect-preserved ~1920px JPEG (~200–500 KB) instead of the full original
(often 5–12 MB). Originals are still served on Download.
Backend:
- imageProcessor: generatePreviewImage / isPreviewValid / ensurePreviewImage
using fit:'inside' + withoutEnlargement (longEdge 1920, q85, mozjpeg)
- migration 104: photos.preview_path + lightbox_preview_enabled setting
(off by default, JSON-stringified for SQLite/Postgres parity)
- GET /api/gallery/:slug/preview/:photoId — gallery-auth, lazy generation,
ETag based on mtime+photoId+watermarkHash
- preview_url surfaced in the photo response only when the toggle is on
- admin /thumbnails/regenerate-previews mirrors regenerate-thumbnails,
skipping videos
- backup walk + archive cleanup + photo-delete now include previews/
Frontend:
- PhotoLightbox uses photo.preview_url ?? photo.url (null-safe fallback)
- ThumbnailsTab gets a Lightbox Preview Tier card: opt-in toggle +
Regenerate All Previews button (gated until the toggle is on)
- en/de locale strings; nl/pt/ru/fr fall back to en
Tested end-to-end: 11 MB / 4000×3000 source → 985 KB / 1920×1440 preview,
381 ms first call, 7 ms cached, ~91% byte reduction.
|
||
|
Paul Nothaft
|
87834a7fff |
fix(install): defer events.hero_photo_id FK to break circular reference (#484)
Real root cause behind MrGabri's fresh-Postgres install crash, surfaced by his second log dump after #488 silenced the FATAL noise: Initial setup failed: error: alter table "events" add constraint "events_hero_photo_id_foreign" foreign key ("hero_photo_id") references "photos" ("id") on delete SET NULL - relation "photos" does not exist initializeDatabase() in src/database/db.js declared the FK inline at events createTable (line 89), but the photos table is created later in the same function (line 203). On Postgres this is a hard error — the referenced table must exist at FK-declaration time. SQLite silently tolerated it because its FK enforcement is lazy and the inline declaration just became a column with no FK metadata. Why no existing Postgres install hit it: initializeDatabase only runs the createTable block on `if (!hasEventsTable)`. Once a deployment has the events table from any prior run, the path is skipped. So the bug only ever fires on a truly fresh Postgres install — which is exactly MrGabri's scenario, and which our smoke suite never exercises (it runs against a long-lived dev stack). Fix: - events createTable: drop the inline FK; column declared as a plain integer with an explainer comment. - After both tables exist (post photos createTable): db.schema .alterTable('events').foreign('hero_photo_id').references... Wrapped in a try/catch that swallows "already exists" so re-runs on installs that previously got into a half-state don't fail boot. Verified by docker compose down -v + up against the dev stack — no FK error, all migrations apply, FK present in pg_constraint with the expected definition. |
||
|
Paul Nothaft
|
b6b58d0659 |
fix(admin-users): normalise date fields to ISO across DB drivers (#485)
Admin > Users page crashed with "TypeError: e.split is not a function" on native installs (SQLite default). Reported by @blazmaric in #485 with a clean diagnosis: SQLite returns lastLogin / createdAt / updatedAt as integer milliseconds since epoch, while Postgres returns ISO strings via the standard JSON serialiser. The page used parseISO() on the raw value and parseISO trips on numbers. Fix at both layers — defence in depth: - backend/src/routes/adminUsers.js: new toIso() helper applied in transformUser + transformInvitation. Coerces Date / number / numeric-string / null to a single ISO 8601 string contract before the response leaves the API. Protects every consumer (frontend AND external API tokens / n8n) regardless of which DB driver is underneath. - frontend/src/services/userManagement.service.ts: same helper as defence-in-depth for stale backends mid-deploy and any cached pre-fix response shape. Also surfaced an existing transformInvitation gap — invitations endpoints were returning raw response.data.invitations without going through the transformer. 10 unit tests pin the toIso contract: all known driver shapes (Date, number, numeric-string, ISO-string, null/undefined/empty) plus the full transformer paths for transformUser and transformInvitation. Out of scope: same epoch-ms surface may exist on other admin pages that were never tested against SQLite (events list, customers, webhooks, api tokens, activity log). Worth a follow-up audit pass to apply toIso() in every snake_case→camelCase transformer the admin routes use, but the immediate Users-page crash is the only reported one and shipping that fix unblocks @blazmaric. |
||
|
Paul Nothaft
|
d4155c4611 |
fix(install): drop racy migration step + add missing frontend container (#484)
Two follow-up fixes inside the same install-experience surface as the previous commit: 1. **Removed `docker compose exec -T backend npm run migrate`** in both install_docker and update_docker_installation. The backend container's wait-for-db.sh already runs `npm run migrate:safe` on startup; the script was racing it with a separate (and non-safe) `npm run migrate`. That race is the most likely actual mechanism behind #484's "relation 'photos' does not exist" error on the second install attempt — partial schema visible to one of the two parallel migrators. Replaced with a bounded wait for the backend container to become healthy (Docker healthcheck reports green only after wait-for-db.sh finishes its migration pass). 2. **Added the missing frontend container** to the script-generated compose. The script previously generated a postgres + redis + backend stack with no frontend at all (backend on host port 3001), while the documented production install (docker-compose.production.yml) ships postgres + redis + backend + frontend (nginx /api proxy on host port 3000). That shape divergence is half of issue B in #484 — script-installed admins had no frontend container and were left wondering where the UI lived. Aligning both compose files on the same shape eliminates the divergence; the frontend uses curl in its healthcheck (frontend/Dockerfile explicitly `apk add curl`) unlike the backend. The remaining piece of issue B — picking ONE canonical install path (build-from-source script vs. prebuilt-image production compose) and deprecating the other — is a deployment-strategy call that deserves its own design pass. Both paths now produce architecturally-equivalent stacks. |
||
|
Paul Nothaft
|
0b0b1bb2d5 |
fix(install): silence pg healthcheck noise + drop legacy workers container (#484)
Three install-experience bugs that compounded into MrGabri's "fresh
install fails" report:
1. **postgres healthcheck noise.** `pg_isready -U <user>` without
-d defaults to probing a database whose name matches the user.
Since DB_NAME defaults to picpeak_prod (not picpeak), every
healthcheck interval logged
FATAL: database "picpeak" does not exist
into postgres logs even though the install was working
correctly. Reporter saw the FATAL, assumed broken, restarted
with DB_NAME=picpeak, hit a tainted-state migration error on
the second try, filed a bug. Fixed in both
docker-compose.production.yml and the inline compose generated
by scripts/picpeak-setup.sh — pin -d to ${DB_NAME} so the
probe hits the real database.
2. **backend container shows perpetually `unhealthy`.** Both
compose files used `curl -f` for the backend healthcheck, but
backend/Dockerfile only installs dumb-init + postgresql-client +
ffmpeg — no curl. Switch to wget --no-verbose --tries=1 --spider
to match what backend/Dockerfile's own HEALTHCHECK already
does. Now docker ps, docker compose ps, and the backend image's
built-in healthcheck all agree.
3. **stale separate `workers` container.** scripts/picpeak-setup.sh
still generated a second container running `npm run workers`
alongside the backend, but workers (fileWatcher,
expirationChecker, emailQueueProcessor, backgroundProcessor,
webhookWorker) have been started by server.js in-process for
a while — see the comment at line ~895 of the same script for
the systemd-side cleanup. The duplicate container caused two
file watchers and two expiration checkers to compete for the
same DB rows. Removed from the generated compose; install +
upgrade paths now stop and rm any pre-existing picpeak-workers
container.
Doesn't address issue B's bigger architecture mismatch (the script
generates a build-from-source compose with no frontend container,
while docker-compose.production.yml uses prebuilt images with a
separate frontend container). That deserves its own design pass
to pick a canonical install path and align — out of scope here.
|
||
|
Paul Nothaft
|
a803491cf4 |
fix(promo-banner): center by default + admin alignment selector (#482)
The gallery promotional banner (#440) read as visually offset from the gallery footer because: - Footer used `container text-center px-4` (full container width, centered text). - Promo block used `container py-4 sm:py-6` with an inner `max-w-3xl mx-auto` wrapper holding left-aligned text — a narrower column with left-aligned content sitting in the middle of the page. Two issues compounded: the column was narrower than the footer AND its text alignment differed. Reported by Rekoo-PS in #482 with a screenshot showing the misalignment, with a request for an admin alignment option. Fix: - Drop the inner max-w-3xl wrapper. Promo content now spans the same .container width as the footer, eliminating the narrower-column visual. - Default text alignment changed from left → center to match the footer. - New `branding_promo_alignment` setting ('left' | 'center' | 'right', default 'center'). Surfaced as a dropdown next to the existing Position dropdown on the BrandingPage. Live preview block on the BrandingPage mirrors the gallery render so admins see what guests will see. - Also replaced the no-op `prose-sm` prose-modifier with a real `prose prose-sm` outer class so the existing `prose-a:text-accent` modifier actually takes effect (it didn't before — modifiers without an outer .prose are silently ignored by Tailwind Typography). Migration 103 seeds the new setting at 'center' so existing installs that have a promo banner today see the corrected alignment immediately on next deploy. i18n: en + de hand-translated; nl/pt/ru/fr machine-translated and flagged for native review per project convention. |
||
|
Paul Nothaft
|
c3256dc6bf |
fix(ci): pin TRIVY_PLATFORM per matrix arch (post-#477 follow-up)
PR #477 moved Trivy from the merge-* job into the per-arch build-* matrix scanning by digest. The amd64 leg works; the arm64 leg crashes with: remote error: no child with platform linux/amd64 in index ghcr.io/.../<image>@sha256:<digest> Root cause: docker/build-push-action wraps every push in an OCI index — the actual image manifest sits next to a SLSA provenance attestation manifest as siblings under the digest. Trivy's remote backend defaults to linux/amd64 when resolving an index, so: - amd64 leg → looks for amd64 child → finds the amd64 image → ok. - arm64 leg → looks for amd64 child → finds NO amd64 child (the only platform child is arm64) → fails. Fix: set TRIVY_PLATFORM = ${{ matrix.platform }} on each leg's Trivy step. Each scanner then asks for its own arch and finds it. SLSA provenance attestation stays attached to the per-arch images — a real win for supply-chain visibility we'd lose if we'd disabled provenance instead. amd64 was the only thing keeping CI partly green; this restores full green across both legs without touching the build artifact shape. |
||
|
Paul Nothaft
|
40e176cb46 |
fix(ci): trivy-action tag is v0.36.0 (was 0.28.0 — does not exist)
Initial pinning shipped a tag that doesn't exist in the aquasecurity/trivy-action repo. Workflow run failed with: Unable to resolve action 'aquasecurity/[email protected].0', unable to find version '0.28.0' The repo's tags use a v prefix (v0.36.0, v0.35.0, …). Bumping both occurrences (build-backend and build-frontend matrix jobs) to v0.36.0, which is the latest stable as of 2026-04-22. |
||
|
Paul Nothaft
|
caf0d61857 |
fix(ci): scan multi-arch images per-arch by digest, pin trivy-action (#476)
Resolves the intermittent "no child with platform linux/amd64 in
index" failure on the merge-backend job — and fixes the same latent
bug on merge-frontend before it surfaces.
Two compounding root causes per Luca's diagnosis:
1. aquasecurity/trivy-action@master was unpinned, so the action and
its bundled Trivy binary float on every CI run. A green build
could flip red overnight without a single repo change.
2. Trivy was asked to scan a multi-platform OCI index by tag (the
merge-* jobs ran AFTER manifest creation). Its remote resolver
cannot reliably pick the right per-arch child out of an index
reference — it needs a single-platform reference (digest, or a
--platform flag).
Fix:
- Move the Trivy + upload-sarif steps OUT of merge-backend /
merge-frontend and INTO the per-arch build-backend / build-frontend
matrix jobs. Each leg scans the image it just pushed by its
sha256 digest (`...@${{ steps.build.outputs.digest }}`), which is
always single-platform by construction.
- Pin aquasecurity/[email protected].0 (was @master).
- Distinct SARIF category per arch
(`backend-vulnerabilities-linux-amd64`, …-arm64) so an
amd64-only finding in a base layer doesn't get masked by the
arm64 scan in the Security tab.
- Move security-events: write down to the build-* jobs (where the
scan now runs) and remove it from the merge-* jobs (which only
publish the manifest now).
Out of scope: flipping `exit-code: '1'` to actually gate CI on
findings. Worth doing as a separate follow-up after an audit pass —
landing it here would surprise beta with a red build for any
pre-existing CRITICAL/HIGH in current images. Inline TODO in the
workflow notes the deferral.
|
||
|
Paul Nothaft
|
0bc7e2af17 |
feat(og): per-event opt-in to use hero photo as social-share preview (#474)
Background: galleryOgService already serves OG/Twitter Card meta tags to social-crawler User-Agents (WhatsApp, Facebook, Slack, Telegram, Discord, ~21 in total) for /gallery/:slug URLs. Today the og:image is always the brand logo with the inline rationale "no protected photo content". #474 asked for a hero/cover photo preview. The trade-off is that any URL embedded in og:image is fetched unauthenticated by every link-preview crawler — so an opted-in image is effectively public to anyone the gallery URL is shared to. Ship as a per-event boolean, default FALSE, so existing galleries never start surfacing photos without explicit admin intent. Schema (migration 102): - events.og_image_share_enabled BOOLEAN NOT NULL DEFAULT FALSE. Backend: - galleryOgService.buildOgMetadata: when opt-in is on AND a hero_photo_id is set AND the photo has a generated thumbnail, emit og:image as /og/gallery/:slug/cover. Falls back to the brand logo on any miss (deleted hero, missing thumbnail, no opt-in) so a half-configured gallery still gets a polished preview rather than a broken-image src. - galleryOgService.handleGalleryOgCover: new public endpoint that streams the hero thumbnail. Validates slug shape, checks the opt-in flag + hero presence + thumbnail existence; returns 404 on any failure. ETag = thumbnail mtime + photo id so a regenerated thumb busts crawler caches. Cache-Control: public, max-age=300 (short — admins shouldn't wait an hour for a cover swap to land in chat previews). - server.js: mount the new GET /og/gallery/:slug/cover route. The existing nginx ^~ /og/gallery/ proxy block already covers it. - adminEvents.js: validator + persistence on POST + PUT. formatBoolean coercion so SQLite (0/1) and Postgres (boolean) both behave correctly. Frontend: - Event type + UpdateEventData carry og_image_share_enabled. - EventDetailsPage adds a checkbox under the HeroPhotoSelector, disabled when no hero photo is picked. Help text deliberately spells out the public-by-design consequence — admins shouldn't flip this on for a sensitive gallery without realising what they're sharing with link-preview crawlers. Tests: 8 new in galleryOgService.shareImage.test.js — pin the cover-vs-logo decision contract (3 cases) plus the defensive fallbacks (deleted hero, missing thumbnail) and the 404 contract on the cover endpoint (4 cases). The 404 tests assert that ensureThumbnail() is NOT called when opt-in is off, so a future refactor can't accidentally widen the unauthenticated cover endpoint to expose a hero the admin hasn't shared. i18n: en + de hand-translated; nl + pt + ru + fr machine-translated and flagged for native review per project convention. |
||
|
Paul Nothaft
|
3122dd08a8 |
fix(customer-routes): Cache-Control: no-store on customer endpoints (#470)
The trigger: PR #458 mounted requireCustomerPortalEnabled which 410'd every /api/customer/* + /api/admin/customers/* request when the master toggle was off. Some browsers cached that 410 (no Cache-Control header was set, so heuristic freshness applied — the wrong default for an authenticated/sensitive surface). PR #470 reverted the middleware, but a customer whose tab cached the 410 still saw 410s until they hard-refreshed. Add noStoreCache middleware and mount it in front of both route groups. Every response (200, 4xx, 5xx) now carries `Cache-Control: no-store, no-cache, must-revalidate, private` plus the HTTP/1.0 Pragma + Expires fallbacks. Any future transient error from these endpoints can no longer get pinned in browser or proxy caches and outlive its cause. Cost is one setHeader per request; applied per route group rather than globally so static assets + galleries keep their own caching strategy unchanged. Includes a dedicated unit test pinning the header set so a future cleanup pass can't quietly drop it and re-introduce the bug. |
||
|
Paul Nothaft
|
5e86eef4f8 |
test(gallery): verifyGalleryAccess customer-assignment revocation (#470)
4 unit tests pinning the contract of the customer-minted JWT re-check added in #470: - via='customer' + customerId, assignment present → next() runs. - via='customer' + customerId, assignment removed → 403 with CUSTOMER_ASSIGNMENT_REVOKED code. - customerId in payload but `via` claim missing → no re-check (defends against a future refactor accidentally widening the gate to match every legacy session that happens to carry a customerId field). - per-event-password JWT (no via, no customerId) → no event_customer_assignments query at all (asserted by counting db() invocations — a regression that quietly added a re-check here would 403 every guest the moment any unrelated customer was unassigned from any event). Same mock pattern as customerAuth.middleware.test.js. The re-check is the load-bearing piece behind the "Manage galleries" dialog UX promise — these tests guard it explicitly. |
||
|
Paul Nothaft
|
7a9c4ca44e |
test(customers): unit-cover setAssignmentsForCustomer (#470 follow-up)
5 new tests covering the diff math (added/removed), the archived-event filter, the no-op short-circuit when wanted equals existing, and the type-coercion of the wanted-list input. Mirrors the existing setAssignmentsForEvent suite shape so the inverse- direction service function carries equivalent regression coverage. This function is the writer behind the "Manage galleries" dialog and the verifyGalleryAccess re-check together form the access- control story for the whole feature — getting the diff math wrong here means assignments don't actually revoke, which is the entire promise of the new UI. |
||
|
Paul Nothaft
|
fad2de5abe |
fix(activity-log): smart feature_flags_updated rendering + 33 missing types
The Dashboard "Recent Activity" widget and the header notifications dropdown both rendered raw activity-type strings (e.g. the literal "feature_flags_updated") for any type missing from their lookup maps — including everything emitted by the recently-added customer portal (#354), webhooks (#327), API tokens (#322), event types, event-publish flow, admin user management (#350), and the feature-flags reorg itself. Two coordinated changes: 1. Smart formatter for feature_flags_updated. The backend writes `metadata.changed = { [flagKey]: { from, to } }` on every save. New formatFeatureFlagsChanged() helper in admin.service.ts reads that diff and renders: - 1 change → "Customer Portal enabled" - N changes → "3 features updated: Customer Portal enabled, Calendar disabled, Quotes enabled" Per-flag display labels source from `settings.features.<key>.title` so they stay in sync with the Features tab. Unknown flag keys fall through to a humanised version of the key. 2. 33 missing activity types added to BOTH renderers and to the `admin.activities.*` + `admin.notificationMessages.*` i18n namespaces across all six locales. Coverage groups: customer portal (13 types), admin user management (6), webhooks (3), API tokens (2), event types (4), event publish/logo (3), bulk delete (1), and assorted post-merge surfaces (4). The notifications.service.ts switch + admin.service.ts fallback message map are still duplicated; consolidating them into a single source of truth is a follow-up worth doing before the next significant addition. For now both stay in sync via this PR. en + de hand-translated. nl + pt + ru + fr machine-translated and flagged for native review per project convention. |
||
|
Paul Nothaft
|
dec2f5d3d2 |
fix(features): customer-portal card uses 'Clients' to match sidebar wording
Settings → Features showed the customer-portal toggle as "Accounts"
("Konten" in DE, "Comptes" in FR, etc.) — the deeper sub-nav label
inside ClientsLayout — while the prominent menu-bar entry the admin
actually clicks first reads "Clients" / "Kunden". The mismatch was
confusing on first encounter ("which one do I look for?").
Align the Features tab card title and the "Sidebar:" callout with
the menu-bar wording (`navigation.clients`) across all six locales.
The sub-nav inside ClientsLayout keeps its own "Accounts" label —
that one matches the /admin/clients/accounts URL and is correct.
|
||
|
Paul Nothaft
|
ae64a6acbc |
fix(branding): socials + promo round-trip from DB to form (#460)
formatBrandingSettings was updated when the BrandingSettings interface added the footer-overhaul fields (#441 / #440), so the admin BrandingPage initialised them as empty strings on every load. Saving any other field then sent the form's empty socials / promo_markdown / promo_position back to the backend and wiped the saved values from the DB. The public gallery footer kept rendering the old values until the next save, which is why the bug appeared asymmetric (visible to galleries, gone from the admin form). Add the missing read mappings for the seven branding_* keys so the form round-trips them correctly. Reported by @Rekoo-PS in #460 (split out of #447). |
||
|
Paul Nothaft
|
2f63188a34 |
fix(events): TDZ ReferenceError on /admin/events from #442 fix (#454)
The pagination-clamp useEffect added in #448 (commit
|
||
|
Paul Nothaft
|
49b36a0352 |
chore(migrations): renumber 090 → 096 + small notes from #403 review
Post-merge cleanups after #403 (customer portal): - Renumber 090_backfill_photo_dimensions_v2.js → 096 to follow #403's 090_add_customer_accounts ... 095_add_customer_portal_flag chain. - customerAccountsService.js: TODO note on must_change_password documenting that the column is decorative until an admin pre-loaded-password flow ships (mirrors what adminAuth does for must_change_password today). - customerAuth.js: doc-comment on the /login route explaining why the customerPortal feature flag deliberately doesn't gate it (toggle off hides UI, doesn't revoke existing-customer access; deactivate individual accounts to lock out). - 095_add_customer_portal_flag.js: header comment said "Migration 094" (copy-paste from 094) — now matches the filename. |
||
|
Paul Nothaft
|
936a277eb8 |
fix(import): capture photo dimensions in fileWatcher + s3AutoImporter (#447)
The aspect-aware gallery layouts (masonry / mosaic / justified) read photo.width and photo.height to size each card to the source's real proportions. Two import paths were inserting rows without those fields, which forced MasonryGalleryLayout to fall back to a hard-coded 800×600 default — every card came out the same shape, so users reported masonry as "always cropped to 1:1ish" no matter which thumbnail fit mode they chose. - fileWatcher.js: extract dims with sharp.metadata() before insert. - s3AutoImporter.js: same, materialising a tmp local copy via withLocalCopy so it works in S3 mode. - migration 090: backfill any pre-existing rows with NULL dims (skips videos, skips S3 deployments — those need the writer fix alone since migrations cannot reach the storage backend). - imageProcessor.js: change DEFAULT_THUMBNAIL_FIT from 'cover' to 'inside' (only kicks in when the seed setting is missing — existing installs keep their saved value). Add UI tooltip recommending 'inside' for masonry/mosaic/justified, 'cover' for uniform grids. i18n covers all six locales. |
||
|
Paul Nothaft
|
d62c529b02 |
fix(create-event): branding-default theme survives eventTypes refetch
The "apply recommended preset on event-type change" effect was firing on the initial mount AND every time the eventTypes API resolved (because availableEventTypes is recomputed when that query settles). The first fire matched the wedding default and clobbered the global Branding theme that the previous effect had just applied. Track the previous event_type in a ref and bail out when it hasn't actually changed. The Branding-default effect now wins on first paint, and the recommended-preset behaviour still kicks in when the user manually picks a different event type. Restores the green state of smoke spec 07 (#323-B regression). |
||
|
Paul Nothaft
|
3a731e7c95 |
feat(footer): hideable legal links + socials + promo banner (#441 + #440)
Combined footer overhaul: - Per-CMS-page show_in_footer toggle (#441) — admins can hide Impressum / Datenschutz from the gallery footer when an external privacy / imprint URL is enough. - Five social-media URL fields in branding settings (#441) — Facebook, Instagram, WhatsApp, X/Twitter, YouTube. Empty string hides each icon individually; the row is omitted when none are set. - Promotional banner slot above or below the gallery footer (#440) — global default authored as markdown in branding settings, plus a three-way per-event override on the Edit Event form (inherit / custom / off). Backend nulls promo_markdown automatically when mode != 'custom' so stale text never persists. Sanitization: marked with gfm/breaks → DOMPurify with a tight allowlist (no img, no tables, no inline html). Post-process forces target=_blank rel="noopener noreferrer nofollow" on every link so admin-set URLs can't tab-nap the gallery context. i18n covers all six locales (en/de/nl/pt/ru/fr). Targets the beta branch. |
||
|
Paul Nothaft
|
9c4a96fe97 |
fix(events): clamp page state when totalPages drops below current page (#442)
Bulk-deleting all events on the current page left the list empty until manual reload. After the React Query refetch returned `events: []` with a smaller `totalPages`, the page state was stuck on the old (now out-of-range) page index — the backend correctly serves an empty page for `page > totalPages`, but the UI had no logic to step back. Add a useEffect that watches `data.pagination.totalPages` against the current `page` and resets `page = max(1, totalPages)` whenever the result count shrinks. Fires after every refetch so it covers bulk delete, individual delete, archive, and any filter change that shrinks the result set — same one-line guarantee. Reported by @Rekoo-PS in #442. |
||
|
Paul Nothaft
|
e54456135c |
fix(events): admins can clear expiration on edit even when "Require expiration" is ON (#426)
iSchumi6210 reported that with the global "Require expiration date"
toggle ON, an admin couldn't clear the expiration on an existing event
via the Edit Event form. The PUT returned 400 "Expiration date is
required."
The cause was intentional in the original code: the global setting was
enforced on both create AND edit, so once flipped ON, no event could
ever be cleared of its expiration — not even by admins editing one-by-
one. Reproduced the exact scenario byte-for-byte against beta:
Toggle ON → POST /admin/events {expiration_days: 30} → 200 created
Toggle ON → PUT /admin/events/:id {expires_at: null} → 400 rejected
The setting now controls only the create-time default. On edit, an
admin can clear the field and the value persists as NULL ("never
expires"). Matches CMS-style admin tool conventions where field-
required-by-default doesn't lock the field after creation.
Backend: drop the `getEventFieldRequirements()` enforcement on the
expires_at branch in PUT /admin/events/:id. Empty/null on edit
normalizes to NULL.
Frontend: drop the matching `requireExpiration && !editForm.expires_at`
toast in EventDetailsPage. The variable is no longer referenced, so
remove its declaration too.
Verified end-to-end with toggle ON:
STEP 1: create with expiration → ok (unchanged)
STEP 2: create without expiration → backend auto-applies default 30d
(create-time enforcement intact)
STEP 3: PUT {expires_at: null} on existing → "Event updated
successfully" (was 400)
STEP 4: DB column expires_at is NULL
STEP 5: PUT {expires_at: ''} also accepted (matches what an HTML
date input sends when cleared)
Smoke 13/13 green; no regressions.
|
||
|
Paul Nothaft
|
15e333681f |
feat(settings): Features tab + sidebar reorg with feature-flag gating
Reorganises the admin sidebar around what users actually do, and adds a
single Features page that gates which feature surfaces appear in the
nav. Shrinks the main sidebar from 11 items to 4-6 (depending on
feature flags) and groups configuration screens into a single Settings
home with six logical sections.
Why
---
The current sidebar mixes three concerns: workspaces (Dashboard, Events,
Archives), feature surfaces (Analytics, Users), and configuration
screens that get touched maybe once a month (Email Settings, Branding,
Event Types, Backup, CMS Pages). That's 11 items, half of them config.
Backend
-------
- New `feature_flags` table (key, value, updated_at, updated_by).
Migration 088 detects existing-vs-fresh installs from the events
table:
* Existing install (events>0) → all 9 flags TRUE so nothing
vanishes from an admin's UI on upgrade.
* Fresh install (events=0) → spec defaults: galleries,
reminderEmails, analytics, userManagement TRUE; calendar,
calendarBooking, quotes, bills, messaging FALSE.
- New `/api/admin/feature-flags` (GET/PUT) under `settings.view` and
`settings.edit`. Server enforces the same dependency rules the
frontend does (galleries always TRUE, quotes=false → bills=false,
calendar=false → calendarBooking=false). PUT writes one
`feature_flags_updated` activity log row with the diff.
Frontend
--------
- `FeatureFlagsContext` provides `useFeatureFlags()` (with staged/save/
reset/isDirty) and `useFeatureEnabled(key)`. Mounted inside
AdminLayout so flag fetches carry the auth cookie. Source of truth
is the server response; staged is a local copy that the Features tab
edits and the Save button PUTs.
- `RequireFeature` route guard for /admin/analytics and /admin/users —
redirects to /admin/dashboard when the corresponding flag is OFF.
- AdminSidebar dropped from 11 to 6 items. Removed: Email Settings,
Branding, Event Types, Backup, CMS Pages (now Settings tabs).
Feature-gated: Analytics, Users.
- Old top-level routes (/admin/email, /admin/branding, /admin/event-
types, /admin/backup, /admin/cms) kept as <Navigate> redirects to
/admin/settings?tab=<key> so existing bookmarks don't 404.
- SettingsPage rewritten with a 6-group inner-nav (General /
Content & Appearance / Communication / Privacy & Security /
Integrations / System) and 19 tabs. New Features tab is the
default landing tab. URL ?tab=<key> roundtrips with state — deep
links and the back button work.
- FeaturesTab renders 9 cards across 5 sections. Toggles enabled for
Analytics + User Management (the two flags that gate sidebar items
in this PR). All other toggles disabled with a "Not yet available"
lockedReason — the cards still render so admins see the roadmap, but
the flag has no UI effect until the surface ships in its own PR. The
galleries card is locked TRUE per spec (foundation, can't be off).
- Live SidebarPreview reflects unsaved staged changes — admins see
what their sidebar will look like before they save.
- New i18n keys across all 5 locales (en, de, nl, pt, ru) for the
Features tab copy, the new Settings group labels, and the lifted
tab titles.
Verified end-to-end
-------------------
- Migration on this dev DB (existing install, 977 events): all 9 flags
set to TRUE.
- Migration on simulated fresh install (events table emptied): spec
defaults applied (5 OFF, 4 ON).
- Backend round-trip: GET → PUT → audit-log entry written, dependency
rule enforced (bills forced false when quotes=false even when bills=
true requested).
- UI Playwright spec: sidebar dropped 5 items, old top-level routes
redirect, Features tab is default, Galleries+Calendar+Quotes+Bills+
Messaging+ReminderEmails toggles disabled, Analytics+Users toggles
enabled, toggling Analytics off + saving updates the sidebar +
redirects /admin/analytics to /admin/dashboard.
- Smoke 13/13 still green; no regressions on existing flows.
|
||
|
Paul Nothaft
|
83d79f4d39 |
fix(gallery): serve thumbnails / photos / hero via storage abstraction (#432)
Three gallery serving routes bypassed the getStorage() abstraction and
used fs.* directly against local paths. Worked in local-fs mode, 500'd
in S3 mode because the files only exist in the bucket. Reported by
@w1ll-i-code with a precise root-cause pointer at gallery.js:1138.
The admin photo serving route (adminPhotos.js) had already been
converted to use storage.stat + storage.get; the gallery side hadn't.
This PR brings the gallery routes in line.
Changes:
- Add getRange(relPath, start, end) to the StorageBackend interface +
LocalFsStorage (fs.createReadStream with start/end) + S3StorageBackend
(downloadStream with Range header). Needed for video range requests
on S3 — previously the photo route did fs.createReadStream(filePath,
{start, end}) which is local-only.
- /:slug/thumbnail/:photoId — read mtime via storage.stat, stream bytes
via storage.get. Watermark application path materializes the source
via withLocalCopy (no-op in local mode, downloads to a tmp file then
cleans up in S3 mode) so applyWatermark's sharp + fs.readFile still
works.
- /:slug/photo/:photoId — branches on source_origin: external/reference
photos still use the local fs path (NAS mounts are local), managed
photos use the storage abstraction. Video range requests pass through
to storage.getRange. Pre-generated watermarks served via storage too.
On-the-fly watermark generation uses withLocalCopy for managed photos.
- /:slug/hero/:photoId — hero images are always managed-storage keys
(imageProcessor.generateHeroImage writes via the storage abstraction),
so this just switches to storage.stat + storage.get. Watermark via
withLocalCopy.
Verified end-to-end against minio in dev:
POST /api/admin/photos/N/upload → photo + thumbnail land in S3
GET /api/gallery/<slug>/thumbnail/<id> → 200, JPEG 300x300 ✓
GET /api/gallery/<slug>/photo/<id> → 200, JPEG 1200x800 ✓
GET /api/gallery/<slug>/hero/<id> → 200, JPEG 1920x1080 ✓
ETag round-trip (If-None-Match) → 304 ✓
Backend logs → no errors
LocalFs regression: 13/13 smoke tests pass.
Closes #432.
|
||
|
Paul Nothaft
|
5c7de96b7f |
fix(auth): default COOKIE_SECURE to 'auto' in production + first-install UX (#427)
Two intertwined bugs reported in #427 by @iSchumi6210: 1. Login silently fails over HTTP. Backend defaulted COOKIE_SECURE to true when NODE_ENV=production. Over plain HTTP the browser drops the Secure cookie → next /auth/session request returns 401 → redirect back to /admin/login → no error shown. picpeak-setup.sh writes NODE_ENV=production but never writes COOKIE_SECURE, so every first-time install without a reverse proxy hits this. 2. Admin password is generated but admins can't find it. The 001_init.js migration writes the generated password to data/ADMIN_CREDENTIALS.txt inside the backend container, but picpeak-setup.sh only copies it out when --reset-admin-password is passed. Default-path users never see it and resort to manual bcrypt updates in psql. Changes: - tokenUtils.js: production default goes from `true` to `'auto'`. On real HTTPS req.secure is true → Secure flag is still emitted (no security regression for reverse-proxy deployments). On plain HTTP req.secure is false → Secure flag omitted → login works. Users who explicitly want the strict HTTPS-only behaviour can still set COOKIE_SECURE=true. - .env.example: rewrite the COOKIE_SECURE block to make the new default obvious and explain when to override (set =true for strict, =false to skip the per-request check, leave unset for the auto behaviour). - picpeak-setup.sh (both Docker and native paths): - Write COOKIE_SECURE=auto explicitly to the generated .env (defense in depth so the right behaviour is preserved even if the backend default flips again later) - After migrations, ALWAYS copy ADMIN_CREDENTIALS.txt out of the backend container/data dir to the host data dir, chmod 600, and print the email + password to the install output. The credentials file remains as a backup record that the operator should delete after noting the password. Verified locally with all 4 permutations of NODE_ENV × COOKIE_SECURE: production, unset → HTTPS: secure=true ✓ HTTP: secure=false ✓ (was both true) production, =true → both: secure=true (strict opt-in preserved) production, =auto → HTTPS: secure=true HTTP: secure=false (already-correct) development, unset → both: secure=false (dev unchanged) |
||
|
Paul Nothaft
|
f3d0f161c9 |
fix(external-media): pre-generate thumbnails so reference-mode galleries load fast (#423)
External (source_origin='external') photos always had thumbnail_path=NULL,
so the gallery returned thumbnail_url=null and every layout fell back to
streaming the full original from the NAS via the secure-image route.
With ~100 NAS-mounted photos that meant minutes of wall-clock load time,
sequential per tile.
Two halves:
1. import-external route generates the thumbnail right after each
successful insert and writes thumbnail_path on the row. Best-effort:
a single failure logs a warning and leaves thumbnail_path=NULL —
ensureThumbnail will retry lazily on first view. Synchronous in the
loop adds ~100-300ms per image; for the worst-case 1000-photo import
that's still under the typical request timeout.
2. ensureThumbnail() in imageProcessor handles external photos too —
resolves the local NAS mount path via resolvePhotoFilePath instead of
the storage-backend key. This covers existing externals already in
the database that were imported before this fix: first gallery view
per photo regenerates the thumbnail, subsequent views are fast.
Filename-collision protection: external thumbnails use
`thumb_ext<photoId>_<basename>` so two events both referencing
e.g. `IMG_0001.jpg` on different NAS subtrees can't clobber each other's
thumbnail. generateThumbnail accepts a new options.outputBasename to
support this without changing the managed-photo behaviour.
Verified locally with a 3-photo external dir and a real NAS-style import:
POST /api/admin/external-media/events/N/import-external
→ {imported:3, thumbnailsGenerated:3, thumbnailsFailed:0}
/api/gallery/<slug>/photos returns thumbnail_url for every photo
Lazy-regen path: clearing thumbnail_path + deleting the file, then
hitting /thumbnail/N regenerates and repopulates the row in 42ms.
Closes #423.
|
||
|
Paul Nothaft
|
c2b1854df6 |
fix(admin): test email always sends, regardless of update availability (#418)
The "Send Test Email" button on the Update Notifications settings page called sendUpdateNotificationNow() — which bailed out with "No updates available" when the instance was already on the latest version. Admins on a current install had no way to verify their SMTP / recipient list was working until an update happened to be pending. Reported in #418 by @Rekoo-PS. Changes: - Add migration 087: insert a dedicated `version_update_test` email template (EN + DE, matching the existing version_update_available convention) with copy that reads as a config-check rather than as a real update notice. Subject prefixed with [TEST] so it's unambiguous in the inbox. Variables: current_version, channel, recipient_email. - Replace sendUpdateNotificationNow() with sendTestUpdateNotification() in updateNotificationService.js. The new path: - Always sends — no updateAvailable bail-out. - Uses the version_update_test template. - Falls back gracefully if checkForUpdates fails (so a transient GitHub API hiccup doesn't block a config-check email). - Does NOT update last_notified_version — that field stays owned by the real-update path so a test send doesn't shadow a future genuine notification for the same version. - Wire /admin/system/updates/notifications/send to the renamed function. No frontend change needed (the button already calls this endpoint). Verified locally with the dev mailhog: clicking Send Test Email on a 3.42.3-beta.0 instance (which has no pending update) delivers 4 emails to all admin recipients with subject "[TEST] PicPeak Update Notification — configuration check" and body interpolated correctly. Returns {success: true, successCount: 4, ...} — previously would have returned {success: false, message: "No updates available"}. |
||
|
Paul Nothaft
|
99e420b1b9 |
fix(events): typed-DELETE confirmation for bulk delete (#417)
The bulk-delete modal previously used a password input as a confirmation gate, with an Enter-to-submit handler. Windows Hello / passkey flows that target password fields were able to autofill and synthesise an Enter keystroke, which submitted the form and triggered the destructive delete without an explicit click on the red Delete button (Rekoo's report in #417). Replace the password gate with a GitHub-style typed-literal pattern: the user types the literal "DELETE" (English, case-sensitive) into a plain text input. The Delete button stays disabled until the input matches, and there is no Enter-to-submit handler — only an explicit click on the red button proceeds. Plain text inputs aren't subject to password autofill or passkey ceremony so the auto-submit class of bug is gone. Server side, drop the bcrypt password verify on /admin/events/bulk-delete and the related INVALID_PASSWORD response. The server's auth boundary remains adminAuth + requirePermission('events.delete'); this matches DELETE /admin/events/:id which has never required a re-entered password. The client-side typed gate is the safeguard against accidental clicks. i18n: drop password-related keys, add confirmLabel + confirmHelp across en, de, nl, pt, ru. The literal "DELETE" stays English in all locales to keep the gesture immune to translation drift and unambiguous. Verified locally: typed-DELETE sanity spec covers the gate (wrong case disabled, correct enables, Enter-on-input no-ops, click submits, events deleted). Existing 03-bulk-archive smoke remains green. |
||
|
Paul Nothaft
|
401abf7a27 |
fix(create-event): re-apply Branding theme on stale→fresh settings (#323-B)
CreateEventPage's branding-default effect used a boolean ref guard that locked in whichever theme_config arrived first. React Query can hand the observer a cached (stale) copy on initial render and then push fresh data once the network call resolves — the boolean ref meant the form kept the stale theme and ignored the fresh one. Replace the ref with a stringified-hash check: re-apply when the source actually changes (including stale → fresh) but skip when nothing has. User edits via the customizer aren't disturbed because settings.theme_config only refreshes on a real Branding save, not on form state. This unblocks the local pre-push smoke gate's 07-branding-default test, which was test.fixme'd against this exact React Query staleness. |
||
|
Paul Nothaft
|
6b6191a426 |
fix(security): scan triage cleanup — drop dead deps, harden Docker/nginx/postMessage
Triage of an external SAST/SCA scan run on 2026-05-06. Most loud findings were already resolved by PR #412 (the 18-CVE backport); this PR addresses the residual real items: * Drop unused `handlebars` from backend deps. The runtime require was removed in PR #367 (#367) but the package.json line stayed. handlebars was the source of two flagged criticals (CVE-2026-33937 RCE, GHSA-2w6w-674q-4c4q AST injection) plus 8 highs — all now gone. * `npm audit fix` on backend + frontend. Bumps transitive picomatch, flatted, postcss, brace-expansion via lockfile, and direct dompurify, lodash, vite, i18next-http-backend within their existing semver ranges. Both audits now report 0 vulnerabilities. * Add `event.origin === window.location.origin` check to the THEME_PREVIEW message listener in PreviewPage. The branding page posts from the same origin, so nothing legitimate is rejected; without the check, any third party that window.open()'d the preview could push arbitrary branding/theme payloads (semgrep insufficient-postmessage-origin-validation). * nginx: `proxy_hide_header` for X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Content-Security-Policy, Permissions-Policy, Strict-Transport-Security at server level. nginx adds these itself, but helmet on the backend was also emitting them — clients were seeing duplicates (testssl flagged "Multiple X-Frame-Options / CSP / Permissions-Policy / Referrer-Policy headers" on the live origin). Single source of truth now. * Dockerfile hardening (checkov): - HEALTHCHECK on backend/Dockerfile, backend/Dockerfile.dev, frontend/Dockerfile.dev. Frontend production Dockerfile already had one. - USER node in frontend/Dockerfile.dev (was running as root). * GitHub Actions docker-build.yml: explicit top-level `permissions: contents: read`. Per-job blocks already declare `packages: write` where needed; this stops future steps from inheriting unintended privileges (CKV2_GHA_1). Backend npm audit: 4 vulns -> 0. Frontend npm audit: 6 vulns -> 0. Backend unit tests: 13 suites, 131/132 passing (1 pre-existing skip). Frontend type-check + lint: clean. The pre-existing integration-test failures (live DB / S3 required) and the ThemeCustomizerEnhanced QueryClientProvider failures are unrelated and reproduce on origin/beta without these changes. |
||
|
Paul Nothaft
|
b7d6ca0b65 |
fix(security): patch 18 dependency CVEs (axios + transitives + nodemailer + i18next-http-backend)
Closes the open Trivy code-scanning alerts for app-side dependencies. The npm-bundled CVEs in /usr/local/lib/node_modules/npm (picomatch, brace-expansion, ip-address inside the Node image itself) are deferred to a separate Node-base-image PR — they're build-environment-side and need their own compatibility testing. ## Direct dependency bumps | Package | From | To | CVEs cleared | |---|---|---|---| | axios (backend + frontend) | 1.14.0 | 1.15.2 | CVE-2026-42264 (HIGH), CVE-2026-42043 (HIGH), CVE-2026-42035 (HIGH), CVE-2026-42033 (HIGH), CVE-2026-42044, CVE-2026-42042, CVE-2026-42041, CVE-2026-42040, CVE-2026-42039, CVE-2026-42038, CVE-2026-42037, CVE-2026-42036, CVE-2026-42034, CVE-2026-40175, CVE-2025-62718 | | nodemailer (backend) | ^7.0.13 | ^8.0.5 | GHSA-vvjj-xcjg-gr5g, GHSA-c7w3-x93f-qmm8 | | i18next-http-backend (frontend) | ^3.0.2 | ^3.0.5 | CVE-2026-41691 | | uuid (backend) | ^11.1.0 | ^11.1.1 | CVE-2026-41907 | | postcss (frontend, devDep) | ^8.4.21 | ^8.5.10 | CVE-2026-41305 | ## Transitive bumps (npm overrides) For transitives whose direct parents haven't released a version that picks up the patched range, pinned via npm overrides: | Package | Min | CVE | |---|---|---| | follow-redirects (backend + frontend) | >=1.16.0 | GHSA-r4q5-vmmm-2653 | | fast-xml-parser (backend) | >=5.7.0 | CVE-2026-41650 | | @tootallnate/once (backend) | >=3.0.1 | CVE-2026-3449 | | ip-address (backend) | >=10.1.1 | CVE-2026-42338 | ## Why axios is now safe to bump past 1.14.0 PR #268 originally pinned axios to 1.14.0 to avoid a supply-chain attack on a specific compromised version range. The 1.15.x series are post-incident upstream releases — clean. Confirmed with the maintainer before bumping. ## Verified * `npx tsc --noEmit` (frontend) — clean * `npx vite build` (frontend) — clean (~4s, existing bundle-size warning, not new) * Backend module-load smoke test — all critical modules load (`auth`, `adminAuth` middleware, `emailProcessor`, `recaptcha`, `storage`) with the new axios + nodemailer * Lockfile re-verification — every targeted CVE now resolves to the patched version range ## Remaining out of scope * npm-bundled CVEs inside `/usr/local/lib/node_modules/npm/` — picomatch CVE-2026-33671 (HIGH), CVE-2026-33672, brace-expansion CVE-2026-33750, ip-address (npm-internal) CVE-2026-42338. These live in the Node base image and require a Node base image bump with its own compatibility testing — separate PR. Targeting `beta` so the bumps go through the normal release-please flow before promotion to `main`. |
||
|
Paul Nothaft
|
0c80abd57b |
fix(gallery): WCAG-safe Download button text + extract HeaderDownloadButton (#401 follow-ups)
Two follow-ups from PR #401's review: 1. Download button text was hardcoded `color: '#ffffff'`. Once admins start picking palettes via #400's expanded customizer, a pale accent (yellow, pastel blue, etc.) leaves the button unreadable — white text on near-white background. Fix: derive the foreground colour from the accent's WCAG relative luminance and expose it as the new `--color-accent-fg` CSS variable in ThemeContext.applyTheme. Light backgrounds (L >= 0.5) get black text; dark backgrounds get white. Same treatment applied to `--color-accent-dark-fg` for the filled-CTA token. The Download button now reads `var(--color-accent-fg, #ffffff)` so any future component that paints on accent gets the same treatment for free, and legacy deployments before the variable is set fall back to the previous hardcoded white. Threshold-based (rather than "highest contrast ratio") to preserve how saturated mid-tone accents have always rendered. The Picpeak default green (#5C8762, L≈0.20) keeps white text — same visual identity as before. Only genuinely pale accents flip to black, which is the actual scenario the review flagged. 2. The Download button JSX was duplicated three times in GalleryLayout.tsx (standard/banner, minimal, hero — ~15 lines each). Extracted into a small inline `HeaderDownloadButton` component above the GalleryLayout export. Three call sites now collapse to a 5-line component invocation each. Markup, accessibility, and styling live in one place — future tweaks only need to happen once. ## Files - `frontend/src/utils/contrast.ts` — new helper module: `relativeLuminance(hex)` (WCAG 2.x sRGB luminance) and `getReadableForeground(hex)` (white-or-black picker). - `frontend/src/utils/__tests__/contrast.test.ts` — 10 cases: fallbacks, saturated mid-tones, pale accents, near-black, shorthand `#RGB`, no-leading-`#`, case-insensitive, anchors (black/white luminance). - `frontend/src/contexts/ThemeContext.tsx` — wire the helper into `applyTheme`: set `--color-accent-fg` from `accentColor` and `--color-accent-dark-fg` from `accentDarkColor`/`primaryColor`. - `frontend/src/components/gallery/GalleryLayout.tsx` — extract `HeaderDownloadButton` component above `GalleryLayout`, replace three inline button blocks with the component, update its inline style to read `--color-accent-fg` (with the legacy `#ffffff` as the CSS-variable fallback). ## Verified - `npx vitest run src/utils/__tests__/contrast.test.ts` — 10/10 pass - `npx tsc --noEmit` — clean - `npx eslint` clean on every touched file - Default PicPeak green still renders white text (no regression) - Pale accent (#fef9c3 yellow-100) now correctly renders black text |
||
|
Paul Nothaft
|
b106da1ede |
fix(auth): /auth/session must enforce session timeout symmetrically (#350 recurrence)
Third loop fix in the same /admin/login → /admin/dashboard → /admin/login pattern as #355 and #363. Reported on v3.39.1-beta.0 — the loop returns after a server restart or after an idle gap longer than the configured session timeout. ## Root cause (server) `sessionTimeoutMiddleware` is mounted on `/api/admin` (server.js:411). It rejects with `401 SESSION_TIMEOUT` when either: - the in-memory `lastActivity` for the token is older than the timeout, or - this is the first request with this token AND the token's `iat` is older than the timeout (post-restart guard). `/auth/session` lives under `/api/auth/session`, NOT under `/api/admin`, so the middleware never runs for it. Result: an idle/old-iat admin token returns `valid: true` from `/auth/session` while every protected endpoint immediately rejects it with `401 SESSION_TIMEOUT`. Frontend's 401 interceptor hard-redirects to `/admin/login`, `/auth/session` says valid again, loop closes — exact same shape as the previous two asymmetries the symmetry pass missed. Fix: add a non-mutating `isSessionExpired(token, decoded)` helper to `middleware/sessionTimeout.js` that reads the same in-memory map and applies the same lastActivity / iat-vs-timeout logic as the middleware, without updating the map (the middleware is the only place that records activity; `/auth/session` is read-only by design). `/auth/session` calls the helper for `decoded.type === 'admin'` after the existing admin-existence and password-change checks. Same try/catch fall-through pattern as the prior fixes so a missing/broken helper doesn't fail-closed during early bootstrap or in test stubs. ## Root cause (client race amplifying the loop) Even with the server fix, the previous `useSessionTimeout` hook called `AdminAuthContext.logout()` which dispatches `POST /auth/logout` fire-and-forget AND has its own `finally { window.location.href }`, then immediately set `window.location.href = '/admin/login?session=expired'` on top. Two consequences: - The cookie wasn't reliably cleared before the new page loaded — if any /auth/session asymmetry slipped through, the loop replayed inside the same tab. New-tab and "refresh several times" "fixes" were just the logout request eventually completing. - Two redirects raced; sometimes the `?session=expired` query was dropped, breaking the login-page toast. Fix: rewrite the hook to (a) await `POST /auth/logout` so the cookie is guaranteed cleared, (b) clear `sessionStorage.admin_user` directly instead of going through AdminAuthContext.logout (which has the side-effect redirect we don't want), and (c) navigate exactly once with the `?session=expired` query. ## Tests - `__tests__/routes/authSession.symmetry.test.js` — 4 new cases under a `session-timeout symmetry` describe block: helper says expired → valid:false; helper says active → valid:true; helper not called for gallery tokens; helper throws → fall through to valid:true (defensive). Existing 9 tests still pass (mock now includes `isSessionExpired: jest.fn(() => Promise.resolve(false))` as the default). - `__tests__/middleware/sessionTimeout.isSessionExpired.test.js` — 7 new unit tests for the helper itself: fresh token / old-iat / recently-active / null-input / no-mutation / 60-min default boundary cases. 20 cases total, all green. Lint clean on every touched file. |
||
|
Paul Nothaft
|
51f28d7330 |
test(fonts): fix mock bypass and case-insensitive FS skip (#390 follow-up)
Two issues in the fonts service test suite added by #390 — the behaviour assertions all passed, but 5 of 24 tests had assertions that silently no-op'd, so any regression in those code paths would not have been caught. ## Issue 1: jest.resetModules() bypassed the logger mock `beforeEach` called `jest.resetModules()` then re-required `fontsService`. After resetModules, the `jest.mock('../../src/utils/logger', ...)` factory at the top of the file no longer applied to subsequent requires — so the freshly-required `fontsService` captured the REAL logger while the test file's `logger` variable still pointed at the mocked one. The 4 "warning logged" / "info logged" assertions resolved as 0 calls and silently passed-as-noop. The resetModules call wasn't necessary in the first place — module-level state in fontsService is just the cache, which clearFontsCache() already resets. And both getBundledFontsRoot() and getUserFontsRoot() read process.env at call-time, not at module load, so the env vars set in beforeEach are picked up without needing a fresh require. Fix: require fontsService once at module top (inside the jest.mock hoisting scope) and drop resetModules + the per-test re-require. ## Issue 2: case-insensitive filesystem (macOS / Windows) The "case-insensitive duplicate within the same root" test created `Inter/` and `INTER/` to trigger the dedup warning. On a case-sensitive FS (Linux ext4) both directory entries exist and the dedup branch fires; on macOS APFS or Windows NTFS the second mkdir resolves to the same folder as the first, so only one ever exists and the dedup is unreachable from this test setup. Test failed on macOS dev, passed on Linux CI. Fix: probe at load time by creating a lowercase file and checking if its uppercase variant resolves to the same inode, then conditionally test.skip the affected test on case-insensitive hosts. Comment in the test body explains why. ## Result 23 of 24 tests now pass on macOS; the case-sensitive-only test runs on Linux CI. All previously-no-op'd assertions now exercise their code paths. |
||
|
Paul Nothaft
|
dbe0a3055b |
docs(readme): add Contributors section with @Luca-Timo and @Rekoo-PS
The Acknowledgments block had a generic "thanks to all contributors" line but no actual recognition by name. Two people in particular have moved the project meaningfully forward and should be called out: - @Luca-Timo — code contributor across multi-arch Docker, the external- URL CMS toggle, folder tree picker, admin email picker, self-hosted webfonts, the gallery header/banner decoupling, and typed-API refactors. Consistent quality. - @Rekoo-PS — bug reporter and feedback loop. Filed the issues that drove the login-loop fix, gallery loading skeleton, redirection cleanup, mobile lightbox overhaul, admin events search-counter fix, photo-count column, and bulk-delete workflow. Also a BuyMeACoffee supporter. Closes the implicit recognition gap and sets up the section so future contributors can be added with a one-line PR. |
||
|
Paul Nothaft
|
48d538f94f |
feat(events): bulk delete with password confirmation (#384)
Adds the bulk-delete half of #384 — admins can select multiple events from the list and delete them in one batch, gated by re-entering their password. ## Why password confirmation Bulk delete is destructive and irreversible (cascades across 5 DB tables and 3 filesystem paths per event). Re-entering the password matches the pattern already used by /auth/admin/change-password and makes accidental clicks much harder than a plain "type DELETE to confirm" — the muscle-memory required to type your real password is a stronger gate than typing a literal word. ## Changes ### Backend (adminEvents.js) - Extracted the per-event cascade-delete logic into a module-private `deleteEventCascade(eventId, adminContext)` helper. The DELETE /:id route now calls it instead of inlining 60 lines of cascade — same behaviour, no drift between the per-event and bulk paths. - New `POST /admin/events/bulk-delete`. Body: `{ eventIds, password }`. Permission: `events.delete`. - Validates `eventIds` array length (1–100) and that each id is an integer. The 100-cap keeps request time bounded; the per-event cascade touches DB + filesystem so 1000 events at once would risk timing out the request. - Verifies `password` against the calling admin's bcrypt hash via `bcrypt.compare()` (same as /auth/admin/change-password). Wrong password → 401 `{ error, code: 'INVALID_PASSWORD' }` and no events are touched. - Loops via `deleteEventCascade`, returns `{ results: { successful, failed } }` with the same shape as /bulk-archive so the frontend can show partial-failure feedback. - Logs `bulk_delete_completed` activity with totals. ### Frontend - `events.service.ts`: `bulkDeleteEvents(eventIds, password)`. - New `BulkDeleteModal.tsx`. Red/destructive variant of the bulk-archive modal: - Lists the events to be deleted (so the admin can verify). - Password input with show/hide toggle, autofocus, Enter-to-submit. - Inline `passwordError` prop surfaces the 401 INVALID_PASSWORD response without losing the modal state — admin can retry without re-typing the event list. - "Processing" state replaces the form with a spinner + "Deleting N events. This may take a few minutes — please don't close this window." (i18n) so admins know not to abandon the page during a slow operation. - `EventsListPage.tsx`: "Delete Selected" button next to "Archive Selected" in the bulk-actions bar (red-styled to signal danger), bulkDeleteMutation that maps the 401 to the modal's inline error and any other failure to a generic toast. ### i18n 12 new keys under `events.bulkDelete.*` in all 5 locales (en/de/nl/pt/ru): title, warning, password label/placeholder/help, submit, processing, incorrectPassword, successAll, successPartial, errorGeneric, plus `events.deleteSelected` for the button. Hand- written for de; nl/pt/ru should get a native-speaker pass at some point but read naturally. ### Verified - `npx tsc --noEmit` clean - `npx eslint` clean on every touched file (4 pre-existing errors in adminEvents.js for unused vars unrelated to this PR) - All 5 locale JSON files parse cleanly - `node -e "require('./src/routes/adminEvents')"` loads the module Closes the bulk-delete half of #384. The Photos-column half lands separately in PR #387. |
||
|
Paul Nothaft
|
ffb4318a1f |
feat(events): add Photos column to admin events list (#384)
The admin events table didn't surface how many photos each event
contained — admins had to click into the event to find out. The
backend already computes `photo_count` for every row in the
GET /admin/events list response (adminEvents.js:794-796), so this
is a frontend-only display change.
- Insert a "Photos" column between Date and Status — groups with
the "what's in this event" info.
- Right-aligned, tabular-nums for clean numeric alignment in the
column.
- Reuses the existing `events.photos` i18n key already shipped in
all 5 locales for the EventDetailsPage tab list ("Fotos" / etc.) —
no new translations needed.
- Updates the empty-state colSpan from 7 to 8.
Closes the column-add half of #384. The bulk-delete request from
the same issue lands separately.
|
||
|
Paul Nothaft
|
98c6f6cf06 |
chore(events): type FolderTreeNode entries with ExternalEntry
Follow-up to PR #378 — drops the (e: any) / (d: any) casts in the external-folder-tree picker. ExternalEntry is already exported from externalMedia.service.ts; the call site just wasn't using it. - Import the type alongside the service. - Annotate the dirs filter callback so `e.type` is the union 'dir' | 'file' instead of any. - Drop the (d: any) annotation from the map — TypeScript infers ExternalEntry from the typed `dirs` array. No behaviour change, no test impact. `npx tsc --noEmit` clean, `npx eslint` clean. |
||
|
Paul Nothaft
|
96818c7ae8 |
fix(docker): install system ffmpeg on Alpine, drop broken bundled binary
Video uploads on production fail with "missing ffmpeg" because the
backend container ships nothing usable for the video pipeline.
Two compounding causes:
1. **Alpine + glibc mismatch.** The npm `@ffmpeg-installer/ffmpeg`
dependency added with the video-support PR (commit
|
||
|
Paul Nothaft
|
bce5c1f725 |
fix(cms): nl/pt/ru i18n + gate external_url in public response
Two follow-ups to PR #372 (external-URL toggle for imprint / privacy CMS pages): 1. **i18n.** PR #372 added 6 new `cms.*` keys to the en + de locales but the project ships 5 locales total. Adds the missing nl / pt / ru translations so the admin CMS page renders in the active language for those users instead of falling back to English literals next to the German/Dutch/Portuguese/Russian surrounding strings. 2. **API shape.** `publicCMS.js` returned `external_url` unconditionally — even when `use_external_url` is false the URL value was still emitted in the public response. The frontend correctly gated on both flags so it worked, but the API surface was leaking a value the admin had explicitly disabled. The value still lives in the DB (so the toggle can be flipped back on without losing it), but the public endpoint now returns `null` whenever the toggle is off. Note: kept the existing `logo_url` shape unchanged. Its semantics are different — null means "fall back to global branding" and consumers rely on always having the field, so emitting it unconditionally is intentional there. No frontend change needed: both `GalleryLayout` and `LegalPage` already gate on `use_external_url && external_url`, so the short-circuit handles `external_url: null` correctly. |
||
|
Paul Nothaft
|
c270bcfc9f |
i18n(events): translate PasswordResetModal across 5 locales
The rebuilt modal in this PR shipped with hard-coded English strings.
That made the reset flow untranslated for German/Dutch/Portuguese/
Russian customers — toasts, confirm dialog, success screen all
fell back to English regardless of the active locale.
- New `events.passwordReset.*` namespace in en/de/nl/pt/ru with 22
keys covering both modal screens, the warning banner, validation
errors, and the toast messages.
- Modal uses `useTranslation()` for every previously hard-coded
string. Reuses `common.cancel`, `events.copy`, `events.copied`
where they already exist across all locales.
- The {{eventName}} interpolation uses i18next's standard variable
syntax so the description line reads naturally in each language.
No behaviour change. TypeScript clean (`npx tsc --noEmit`), ESLint
clean. JSON validity checked for all 5 locale files.
|
||
|
Paul Nothaft
|
ff50c74e19 |
fix(events): admin-set password on reset, full-URL gallery_link in all emails
Two related defects on the same gallery-email surface that PR #367 opened, addressed together: 1. Reset-password endpoint was a one-way auto-generate. `POST /admin/events/:id/reset-password` always called `generateReadablePassword()` and ignored any client-supplied value; the modal only offered a confirm + a forced auto-generated result. Admins who wanted to set a memorable customer-supplied password had no way to do it. Backend: route now reads optional `password` from the body. If present, validates with `validatePasswordInContext('gallery', …)` (same rules as create-event) and uses it; if absent, falls back to the existing generator, so old callers / cron stay functional. Switched the bcrypt rounds from a hard-coded `10` to `getBcryptRounds()` to match the create flow. Frontend: rebuilt `PasswordResetModal.tsx`. Typed input with show/hide, confirm-password field that appears on type, the same `<PasswordGenerator>` used by `CreateEventPage` (event-context- aware, fills both fields when used), send-email checkbox, client-side validation, server-side validation feedback inline. Submit empty → server auto-generates and the success screen shows the value with a copy button (legacy one-click flow preserved); submit with a typed password → success toast + close (no need to re-show what the admin already typed). Service layer: `events.service.resetPassword(id, sendEmail, password?)` only sends `password` in the body when set. Caller: `EventDetailsPage` now passes `eventDate` + `eventType` into the modal so the generator has event context. 2. `gallery_link` was the path-only `event.share_link` in three email-queue sites, so customer mail showed `/gallery/<slug>/<token>` instead of the full `https://example.com/gallery/<slug>/<token>` URL. - `adminEvents.js` reset-password queue (#1437) - `adminEvents.js` resend-creation-email queue (#1502) - `expirationChecker.js` expiration_warning queue (#82) All three now derive `shareUrl` from `buildShareLinkVariants` (the same helper already used by create-event, publish-from- draft, and event-rename). The other 4 callers (`adminEvents.js:651/913`, `events.js:187`, `eventRenameService.js:231`) already used the full URL — this closes the gap. Verified: TypeScript clean (`npx tsc --noEmit`), ESLint clean on every touched file (the 4 lint errors that remain in `adminEvents.js` are pre-existing and predate this branch). |
||
|
Paul Nothaft
|
e8052adf1d |
fix(email): render conditionals, localise password placeholders, fix caller/template variable drift
Bundle of email-renderer and email-caller fixes triggered by a
reproducer on picpeak.nothaft.cloud (gallery_created mail showing
literal `{{#if welcome_message}}` markers and `Passwort: (set at
creation)`). The audit that followed surfaced six more user-visible
defects in the same surface; all are fixed here so customer-facing
mail renders cleanly.
Renderer (`backend/src/services/emailProcessor.js`)
- `safeTemplateReplace` now resolves `{{#if VAR}}…{{/if}}` blocks
before flat `{{var}}` substitution. The shipped templates have used
Handlebars-style conditionals since migration 026; the renderer
ignored them, so the markers leaked verbatim into every mail with
an empty welcome_message. Lifted to module scope and exported so
the conditional contract is unit-testable. Single-pass, non-nested
(commented).
- Added `passwordSetAtCreationI18n` next to the existing two i18n
password sentinels so `(set at creation)` (sent by the publish-
from-draft flow when only the bcrypt hash remains) is localised
to "Das bei der Erstellung der Galerie gesetzte Passwort" /
equivalent in EN/DE/NL/PT/RU instead of the raw English string.
- Added an opt-in `{ escapeHtml: true }` mode to `safeTemplateReplace`
so admin-supplied free text (`event_name`, `host_name`, …) is
HTML-escaped on substitution into the HTML body. Allowlist of
passthrough keys (`welcome_message` already-HTML, server-generated
URLs `gallery_link` / `client_link`). Subject and text body keep
the legacy unescaped behaviour. `formatWelcomeMessage` now escapes
before nl2br so the welcome_message allowlist is safe.
- New `htmlToText()` strips `<style>` and `<script>` blocks (and
their content) before tag-stripping, decodes common entities, and
collapses whitespace. Used by the textBody fallback in
`sendTemplateEmail` — without this, every template missing a
`body_text` produced a "plain-text" mail starting with the 100+
lines of CSS embedded by `wrapEmailHtml()`.
- The client-access section (#172) now mirrors its HTML block into
`textBody` using the same per-language strings, so plain-text
recipients see the link / PIN / warning. `pinLabel = 'PIN'` moved
into `clientAccessI18n` (RU uses ПИН-код).
- Added `getSupportEmail()` exported helper that reads
`branding_support_email` from `app_settings` (JSON-decoded), with
the SMTP from-address as fallback. Used by the gallery_expired and
archive_complete callers below.
- Removed dead `require('handlebars')` (unused since the regex
renderer landed; pre-existing lint error in this file).
Callers (data the templates already reference)
- `expirationChecker.js queueExpirationWarning`: send `expiry_date`
(templates use this, the old code sent `expiration_date` —
typo'd key, never read), drop the hard-coded `.de`/`en` sniff
(the processor formats with the recipient's resolved language),
add the `{{password_security_message}}` sentinel for
`gallery_password` (plaintext is gone by warning time, so
customers used to see literal `{{gallery_password}}` in the mail).
- `expirationChecker.js handleExpiredEvent`: both queueEmail calls
now supply `host_name`, `event_date`, `expiry_date`,
`support_email` so the EN/DE/NL/PT/RU `gallery_expired` template
doesn't render literal `{{host_name}}, your gallery expired on
{{expiry_date}}`. Skip the duplicate admin send when
admin_email == customer_email.
- `archiveService.js`: `archive_complete` queue now supplies
`host_name`, `photo_count` (from `photoEntries.length`),
`archive_date`, `support_email` — the previous payload had only
`event_name` and `archive_size`, so most of the mail was
unfilled placeholders.
Tests
- `__tests__/services/emailProcessor.safeTemplateReplace.test.js`:
16 cases — flat substitution, conditional truthy/falsy/missing/
multi-line/sibling/numeric-0, plus 5 cases for the new
`escapeHtml` option (default off, escape on, allowlist
passthrough for welcome_message and gallery_link).
- `__tests__/services/emailProcessor.htmlToText.test.js`: 7 cases —
the regression scenario (full wrapped body with embedded `<style>`
block), tag-stripping, entity decoding, paragraph spacing.
- `__tests__/utils/formatters.test.js`: 12 cases for `escapeHtml`,
`nl2br`, and the now-escaping `formatWelcomeMessage`.
35 cases total, all green. Lint clean on every touched file
(also fixes a pre-existing `no-prototype-builtins` warning in the
process). Pre-existing failures in
`__tests__/services/backupService.enhanced.test.js` are unrelated
and pre-date this branch.
|