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.
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
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
@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
@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
@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
@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
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).
Unit test for the v1 upload route's category lookup, requested in
the PR review. Mocks db (chainable, mirroring src/routes/__tests__/
adminAuth.test.js) plus apiTokenAuth/requireApiScope (pass-through)
and multer (stub req.file). Two cases:
1. The scoping clause: the andWhere callback applied to a knex
builder spy produces .where({event_id: <event.id>}).orWhere(
'is_global', true) — exactly the contract the reviewer asked
for, exercising the OR-clause rather than just asserting the
callback was passed.
2. Null lookup result yields 400 with "Unknown or out-of-scope
category_id <N>".
No v1 jest scaffolding existed before, but the project-wide harness
(backend/jest.config.js + jest.setup.js) already covers the new
file via testMatch '**/__tests__/**/*.test.js'. Happy-path tests
deferred — would require stubbing fs/sharp/imageProcessor/share
linkService and several more db chains, which the reviewer was
willing to accept as a separate follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR review pointed out the original lookup
db('photo_categories').where({ id: parsedCategoryId }).first()
accepted any category id — including one that belongs to a different
event. photo_categories carries both event_id (per-event) and is_global
(see backend/migrations/legacy/004_add_categories_and_cms.js); the v1
upload route should require either match.
Not a privilege issue (apiTokenAuth.js inherits the admin's powers, no
per-event scoping), but it lets a misconfigured uploader silently file
photos under a category the target event doesn't own — and the 201 echo
includes a category_id that makes no semantic sense.
Tighten to:
.where({ id: parsedCategoryId })
.andWhere(function () {
this.where({ event_id: event.id }).orWhere('is_global', true);
})
…and update the 400 message to "Unknown or out-of-scope category_id N".
OpenAPI description already documents the intended scope.
Tests deferred to a follow-up; v1 has no jest harness today, see PR
discussion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The v1 photo upload endpoint previously ignored any caller-supplied
category and inserted photos with category_id=NULL. That meant
programmatic uploads via API tokens (e.g. a photobox sidecar) landed
in picpeak as uncategorized, forcing operators to bulk-assign category
in the admin UI after each event.
Mirror the adminPhotos.js category-handling logic on v1:
- Read optional `category_id` from the multipart form body.
- Reject unknown ids with 400 (with the id in the error) so callers
fail fast on misconfigured envs instead of silently uncategorized
uploads.
- Set photos.category_id on insert.
- Flip photos.type to 'collage' when the category's slug is
collage/collages, matching adminPhotos.
Backwards-compatible: omitting category_id keeps the prior behavior
(insert with NULL category, type='individual'). OpenAPI spec + 201
response body updated to include the new field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
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.
Regression of #208. PR #214 (commit 02a46e0, re-merged at 9b7495e)
shipped the configurable `general_max_upload_batch_size_mb` setting so
users behind Cloudflare Tunnel and other reverse proxies with
per-request size caps could lower the chunked-upload size below their
proxy's limit. Six days later the "Merge main into beta for
release/beta-to-main" commit (28793bb) resolved its conflict by
keeping main's older tree — which silently deleted the migration
(072), the setting input on Settings → General, the i18n strings, the
`useSettingsState` field, and the read in PhotoUpload.tsx, putting the
hardcoded 500MB chunk back. Galleries fronted by Cloudflare have
quietly been broken on batch uploads since then.
Re-applying exactly the same change set:
- `backend/migrations/core/072_add_max_upload_batch_size.js`
recreated, with a comment pointing at the regression in case the
same merge accident happens again.
- `frontend/src/components/admin/PhotoUpload.tsx` line 168 now reads
the setting from query cache and falls back to 95MB (Cloudflare-safe
headroom under 100MB).
- `useSettingsState.ts`, `GeneralTab.tsx`, `en.json`, `de.json` —
added the field to the state type + defaults + load path + the
Site-Configuration input.
Existing installs are safe either way:
- Ran original 072 then lost the file: migrations table still has the
filename, so the runner skips re-applying. The setting row in
`app_settings` is also untouched (the deletion was source-only, no
down migration ran). Now the new code starts reading it again.
- Installed after the regression: migrations runner picks up the new
072 normally and seeds the setting at 95.
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.
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.
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.
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.
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.
github-actions[bot]
paul