a1a73bf75ff3fcd0833fdf7922a35ad09f19439b
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul Nothaft
|
a40ab6a9b1 |
ci: required-check workflows now fire on every PR (no paths filter)
Branch protection on `main` + `stable` lists `upgrade-from-bootstrap` and `fresh-install` as REQUIRED checks. The producing workflows had `paths:` filters in their `pull_request` triggers, so they correctly skipped on PRs that didn't touch migrations / package.json. But a skipped workflow doesn't satisfy a required check — it leaves the status "missing", which blocks merge on every unrelated PR. Concretely surfaced on PR #692 (security bumps): all 12 visible checks were green, but the merge button was blocked because the two path-filtered workflows skipped and their required-check names never reported. This PR drops the `paths:` filter from both workflows so they always fire on PRs against `main` + `stable`. Costs: - `schema-drift` (`upgrade-from-bootstrap`): ~75 s per PR (Postgres service boot + migrate:safe run + schema assertion). - `install-smoke` (`fresh-install`): ~2 min per PR (full Docker Compose boot + login). Both are buying unconditional safety nets on the install + migration paths, which is what the required-check gate is supposed to model. Also fixes the trigger branch list while in the file: `[main, beta]` → `[main, stable]`, completing the post-#669 rename for these two workflows that were missed in PR #686. ## What this does NOT fix `GitGuardian Security Checks` is the third required check that's currently missing on PRs — but that's a separate problem. The GitGuardian GitHub App was installed at the user-account level (`the-luap`) before the org transfer and didn't move with the repo. Re-installing it on the org via the GitHub Marketplace is a UI step the maintainer needs to do; can't be done via API. |
||
|
Paul Nothaft
|
7546f104a3 |
chore(security): close 27 code-scanning alerts via dep + base-image bumps
Single PR closing every open code-scanning alert at https://github.com/PicPeak/picpeak/security/code-scanning. Both repos go from 27 open alerts → 0 across direct deps, transitive deps, and build- time bundled deps. ## Backend (`backend/package.json` + overrides) Direct dep bumps: - axios 1.15.2 → 1.16.0 (closes 9 alerts: 7 high + 1 med + 1 low) - nodemailer 8.0.10 → ^9.0.1 (closes 1 high — SSRF + file-read via raw option) - multer 2.1.1 → 2.2.0 (closes 2 alerts: 1 high + 1 med) - form-data 4.0.5 → 4.0.6 (closes 1 high) - tar ≥7.5.13 → ≥7.5.16 (closes 1 med) - postcss 8.5.6 → 8.5.10 (closes 1 med) - i18next-http-backend 3.0.2 → 3.0.5 (closes 1 med — backend lagged frontend) - js-yaml 4.1.1 → ^4.2.0 (closes 1 med) - joi 17.13.3 → ^17.13.4 (closes 1 med) Overrides updated to match deps (npm rejected the install otherwise) + nodemailer ^9.0.1 added as override so imapflow + mailparser transitive bundling of older nodemailer is also fixed. Babel devDep auto-bumped via `npm audit fix` (low-severity arbitrary file read). Backend npm audit: 0 vulnerabilities. ## Frontend (`frontend/package.json`) Direct dep bumps: - axios 1.15.2 → 1.16.0 - postcss 8.5.6 → 8.5.10 - i18next-http-backend 3.0.5 → 3.0.5 (already current — kept for parity) `npm audit fix` swept up 12 transitive issues at the same time: - vitest (1 critical — file read on UI server) - vite (2 high — fs.deny bypass, NTLM hash via launch-editor) - ws (2 high — uninitialized memory + DoS) - dompurify (8 mod — multiple IN_PLACE / hook-pollution XSS vectors) - react-router-dom + react-router (1 mod transitive) - esbuild (1 mod — dev server file read) - @babel/core (1 low) Frontend npm audit: 0 vulnerabilities. ## Frontend Dockerfile - Build stage: `node:20-alpine` → `node:22-alpine` Closes the npm-bundled CVE class (picomatch, ip-address, brace-expansion, @sigstore/core, tar) that came from Node 20's older bundled npm. Matches the backend Dockerfile base. The nginx serving stage stays at `nginx:1.28-alpine` — that tag is rolling, so the next build picks up the fixed 1.28.3-r4 layer that closes the 4 nginx CVEs. ## Verification - Backend: `npm audit` → 0 vulnerabilities ✅ - Frontend: `npm audit` → 0 vulnerabilities ✅ - Backend Jest (workflow engine, rounding, WhatsApp): 47/47 pass ✅ - Frontend Vitest: 84/84 pass ✅ - `frontend npm run build`: succeeds ✅ - nodemailer 9 sanity check: our usage is `createTransport({host,port,secure,auth})` + `sendMail({from,to,subject,html,text})` — we don't touch the `raw` option that 9.x tightened, so the major bump is API-compatible. |
||
|
Paul Nothaft
|
806b1ac921 |
ci: bypass size gate — cap self-merge PR size for review-bypass users
@Luca-Timo is on main's review-bypass list so he can self-merge small
bugfixes without waiting for a maintainer review. The bypass list alone
is binary (he can merge anything), so this adds a complementary required
status check that fails when a bypass user's PR exceeds a configured
line-count threshold — blocking merge for genuine features while leaving
small bugfixes flowing.
How it works:
- Trigger: pull_request_target (so the workflow runs in the base repo's
context with permissions to write a check status — script never
executes PR code, so fork-PR-attack-safe).
- For PRs authored by a bypass user (default: @Luca-Timo):
- linesChanged = additions + deletions
- If ≤ LINE_LIMIT (300): check = success → bypass works → self-merge OK
- If > LINE_LIMIT: check = failure → required-check gate blocks merge
regardless of bypass; needs a maintainer review.
- For everyone else: check = success ("not applicable"). They go through
the normal review path and are unaffected.
Both constants (LINE_LIMIT, BYPASS_USERS) are at the top of the workflow
for easy tuning.
After this lands on main, a separate API step adds 'bypass-size-gate' to
the main branch's required_status_checks list so the gate is actually
enforced. Until that's in place the check runs but doesn't block.
|
||
|
Paul Nothaft
|
5839bba72a |
docs: prominent migration banner at the top of README (#669)
GitHub-flavored `> [!IMPORTANT]` callout right below the title, before the badges/hero block, so it's the first thing a visitor or repo browser sees in the rendered README. Mirrors the in-app banner (#687) so an operator gets the same message whether they're browsing the repo or logged into the admin dashboard. Body covers: - Image-path change with the literal new path - Branch rename (beta → main, main → stable) with auto-redirect note - Link to docs/migration-to-org.md for the exact compose-file edit Remove (or downgrade to a regular note) after the migration window settles, same lifecycle as the in-app banner constant. |
||
|
Paul Nothaft
|
2a4bf3b868 |
feat(admin): in-app migration banner for the org rename (#669)
One-time banner shown at the top of the admin layout to surface the org
rename + GHCR registry change for operators who haven't read the release
notes. Sits right below the existing maintenance banner — same pattern.
## What it looks like
Blue, dismissible banner with a short body:
> PicPeak's image registry has moved
> Update your docker-compose.yml to pull from
> ghcr.io/picpeak/picpeak/{backend,frontend} — the old path is no longer
> being updated. [See migration notes]
The link goes to `docs/migration-to-org.md` on the new org repo.
## Design choices
- **No backend feature flag.** A hard-coded `MIGRATION_BANNER_ENABLED`
constant in the component file (1 line) gates global display. After
~1 quarter, flip it to false (or drop the mount in `AdminLayout.tsx`)
in a small follow-up PR. A backend `app_settings` row + Settings UI
toggle would be overkill for a one-time migration event.
- **Per-admin dismissal via localStorage.** Key is `picpeak:migration-banner:v1`
(versioned so a future "we've moved AGAIN" banner can show without
inheriting earlier dismissal). Wrapped in try/catch so private-mode
browsers + storage-quota-exceeded errors don't crash the layout.
- **EN + DE strings** under a new top-level `migrationBanner` namespace.
Other locales (fr, nl, pt, ru) fall through to EN — `migrationBanner.*`
keys aren't translated there yet, deliberate (per #669 the ops
banner is operator-facing and admins reading EN/DE is the majority).
- **Reuses `common.dismiss`** for the close-button aria-label.
## Test plan
- [ ] Frontend `npm run build:check` passes (TS + build)
- [ ] Open admin dashboard in EN → banner shows at top, below header,
above main content
- [ ] Switch to DE → banner shows German strings
- [ ] Click dismiss → banner hides, doesn't re-appear on hard refresh
- [ ] Clear localStorage `picpeak:migration-banner:v1` → banner returns
- [ ] Flip `MIGRATION_BANNER_ENABLED` to false → banner doesn't render
for anyone, regardless of dismissal state
Refs #669.
|
||
|
Paul Nothaft
|
3ca6378bd7 |
chore: workflows + RELEASING.md for the post-rename branch model
After the org move + branch rename (#669): beta → main (active development) main → stable (curated release channel) This PR rewires the workflows that referenced the old branch names so release-please and the Docker build target the right channels. ## Workflow changes ### `.github/workflows/docker-build.yml` - **Push triggers**: `[main, beta]` → `[main, stable]` (both `push.branches` and `pull_request.branches`). `beta` no longer exists; `stable` is the curated channel that should also produce builds. - **`is_prerelease` detection**: pre-release context was decided by `refs/heads/beta`; now decided by `refs/heads/main` (active dev → prerelease, `-beta.N` version suffix unchanged). - **`:latest` + `:stable` tagging**: were gated on `{{is_default_branch}}` (which used to be `main` = stable channel). Default branch is now `main` = active dev, so the implicit gate would have aliased `:latest` to dev. Both tags now explicitly gate on `refs/heads/stable` OR a non-prerelease release tag. - **`:beta` tag**: REMOVED. Active-dev pulls are `:main` (auto-generated by `type=ref,event=branch`). The pre-rename `:beta` tag remains frozen at its last build under Option B / #669 — operators are expected to update to `:main` or pin to a versioned tag. ### `.github/workflows/release-please.yml` - `branches: [main]` → `branches: [stable]`. This is the **stable** release-please workflow (uses `release-please-config.json`); after the rename, the stable channel lives on the `stable` branch. ### `.github/workflows/release-please-beta.yml` - `branches: [beta]` → `branches: [main]`. - `target-branch: beta` → `target-branch: main`. - This is the **pre-release** release-please workflow (uses `release-please-config-beta.json`, `prerelease: true`); after the rename, pre-releases are cut from the new `main` (active dev). The version-suffix scheme stays `-beta.N` so existing operator pins keep working. ## RELEASING.md Rewrote the TL;DR, "How a stable release is cut", and hotfix path to reference the new branch names. Added a one-line "branch model background" note pointing at #669 so future maintainers know why `main` means active dev (the opposite of what some projects use). Filename conventions: `release/X.Y.Z-merge-from-main` (was `…-from-beta`); promotion PR title `promote main → stable as vX.Y.Z` (was `promote beta → main`). ## Why combined with PR A's content as a single PR Originally planned as two PRs (B = workflow triggers, C = release-please reconfigure). Splitting wasn't worth it: the configs are branch-agnostic (`release-please-config.json` and `release-please-config-beta.json` don't mention branch names internally), and not bundling them meant a window where the stable release-please workflow would fire on pushes to the new `main` (active dev) — exactly the wrong place. Single PR closes that gap. ## Versioning scheme — kept No version-scheme decision needed. The `-beta.N` suffix on pre-release versions is preserved (existing operator pins like `v3.71.3-beta.0` keep working). If a `v4.0.0-pre.N`-style reset is desired later, that's a separate PR with explicit operator-comms attached. |
||
|
Paul Nothaft
|
d606fcd5a4 |
docs: branch model + migration-to-org guide + PR-template target hint
Operator + contributor docs for the post-org-move world. None of these files reference the legacy branch names (`beta` / old `main` meaning) — they describe the new shape (`main` = active dev, `stable` = curated release channel), so they're correct from the moment the rename happens. Three additions/edits: 1. `docs/migration-to-org.md` (new) — operator-facing one-pager that the in-app migration banner + the OLD GHCR package URLs (now 404) can point at. Walks through the single `docker-compose.yml` edit needed. 2. `CONTRIBUTING.md` — new "Branch model" section explaining which branch to target (`main` for features + most fixes; `stable` only for small, surgical bugfix backports). Updates the "fork from beta" step to "fork from main". Updates the release-process paragraph to describe the two-channel model instead of the old beta→main promote. 3. `.github/PULL_REQUEST_TEMPLATE.md` — adds a target-branch hint at the top of the template (HTML comment so it shows during PR composition but doesn't render in the merged PR body). |
||
|
Paul Nothaft
|
0205c7dcce |
chore: migrate Docker registry + GitHub URLs to PicPeak org
Repo transferred from the-luap/picpeak → PicPeak/picpeak. Docker images
publish to ghcr.io/picpeak/picpeak/{backend,frontend} (lowercase, per the
GHCR canonical form computed by docker-build.yml's `${GITHUB_REPOSITORY,,}`).
Sweep covers:
- docker-compose.production.yml + Dockerfiles → new image registry path
- README, CONTRIBUTING, SECURITY, SIMPLE_SETUP, scripts/picpeak-setup.sh
→ new GitHub URLs
- Update-check / release-notes services (updateCheckService,
environmentService, updateNotificationService, adminSystem,
UpdateNotification, githubReleaseUrl) → GitHub API + tag URLs use the
canonical PicPeak/picpeak path
- Issue templates + README-DOCKER + workflow README → updated package URLs
- One commit-context comment in migrations/090 + customerAccountsService
CHANGELOG.md is intentionally untouched (historical release entries are
immutable; GitHub auto-redirects the old URLs indefinitely).
CLAUDE.md keeps the bare `(the-luap)` reference — that's the maintainer's
personal handle, not a repo URL.
22 files, 48/48 line swaps (every change is a 1:1 URL replacement).
|
||
|
Paul Nothaft
|
511d647eec |
fix(events): wire customer notifications into both public API entry points (#647)
Two entry points for event creation were missing customer notifications, both discovered while triaging @Rekoo-PS's report that "API created events" don't send WhatsApp after #649/#650 landed. POST /api/v1/events (the OpenAPI-spec'd bearer-token API at v1/events.js): - gallery_created email was NEVER queued — only the webhook fired. - WhatsApp was NEVER queued either. POST /api/events (legacy admin-auth route at routes/events.js): - gallery_created email was queued, but WhatsApp was not. - customer_phone wasn't read from the body at all. Both routes now mirror the adminEvents.js create-and-publish path: best-effort queues that never block the API response, gated on customer_email / customer_phone presence and the global event_phone_field_enabled toggle for the phone field. The webhook subject from POST /api/events now also includes customer_phone, so downstream integrations get the same shape as the v1 API. No schema change. No migration. customer_phone column already exists on events (migration 080). WhatsApp config + template_language + template_params resolve through the existing queue processor. |
||
|
Paul Nothaft
|
ea2852dcb0 |
fix(admin): stack publish-gallery dialog CTAs so the German label fits (#670)
Reporter @the-luap hit the German `Veröffentlichen & Kunden benachrichtigen` button overflowing the modal footer in the publish dialog. Two failure modes chained: 1. The footer was a `flex` row with two `flex-1` buttons inside a `max-w-md` (448 px) modal. Default `min-width: auto` on flex children meant the primary button kept its content width (~340 px including the paper-airplane icon + padding) and pushed the row past the modal frame. 2. Adding `min-w-0 whitespace-normal` doesn't help — the base `.btn` class has `@apply ... whitespace-nowrap` (`index.css:149`) which wins over a utility className via the Tailwind CSS cascade order. So the text won't wrap, the button silently extends past the modal frame, no overflow indicator. Verified with `getComputedStyle().whiteSpace = 'normal'` and the button still rendering as one ~340 px wide line at ~224 px allocated space. Fix: stack both buttons vertically (`flex flex-col-reverse gap-3`). Primary appears on top visually (col-reverse), cancel below — standard confirmation-dialog pattern (Material, Headless UI, Radix all do this for single-action dialogs). Works in every locale and viewport regardless of label length. No side-by-side row to overflow. Tried two prior shapes that didn't hold: - `flex-col-reverse sm:flex-row` with `sm:flex-1 min-w-0 whitespace-normal` on the primary: still overflowed silently because of the whitespace-nowrap cascade above. - `flex-col-reverse sm:flex-row sm:justify-end` with content-width buttons: `justify-end` doesn't constrain a row whose content sum exceeds the container; row just pushes left of the modal. Bumping the modal to `max-w-lg` (or wider) was also considered and rejected: matching modal width is asymmetric (every other admin dialog stays at `max-w-md`), and any locale longer than German would re-hit the wall. Stack-always is the only shape that handles every locale + every viewport without per-language tuning. Verified end-to-end against a dockerised dev backend: - DE + EN × desktop (1280px) + mobile (375px) — all four show primary on top, cancel below, both inside the modal frame, no overflow. Lint + tsc + full vitest suite (84/84) clean. Closes #670. |
||
|
Paul Nothaft
|
ab501459a4 |
feat(analytics): pluggable trackers — Umami + Rybbit + Custom (#663 Phase 1)
Implements the hybrid scope agreed on in #663: two native adapters (Umami + Rybbit) for trackers we'd keep maintained, plus a Custom script-paste mode for everyone else (Plausible, Matomo, Pirsch, GA4, GoatCounter, Fathom, Cloudflare Web Analytics). Phase 2 (Plausible native, deeper metrics) explicitly deferred until someone asks. ## Architecture **Backend `services/trackers/`**: - `TrackerAdapter` shape (single method): `fetchDeviceBreakdown` → `{ desktop, mobile, tablet } | null`. Null = route falls back to access_logs heuristic. - `umamiAdapter.js` — extracted from the `services/umamiClient.js` that landed in #662. Same 10 test contract preserved. - `rybbitAdapter.js` — new. Hits `/api/site/{id}/breakdown?dimension= device` with Bearer auth, accepts both bare-array and `{data:[...]}` envelope variants, tolerates `sessions`/`visitors`/`value`/`count` metric keys. - `customScriptSanitiser.js` — sanitize-html with a tracker-tight allowlist (`<script>` / `<noscript>` / `<link rel=preconnect| dns-prefetch>` / `<meta>`). Strips event-handler attributes, `javascript:` and `data:` URLs. - `index.js` factory: `resolveAdapter()` reads `analytics_tracker_provider` setting → dispatches. Back-compat: when provider is unset, infers `umami` from the legacy `analytics_umami_enabled` flag so #662 installs keep working without an admin touching settings. **Backend routes**: - `adminDashboard.js /analytics`: now goes through `resolveAdapter()`. Old `fetchUmamiDeviceBreakdown` direct import removed; both `umamiClient.js` and its test file deleted (replaced by the adapter shape). - `adminSettings.js PUT /analytics`: validates the new `analytics_tracker_provider` enum, sanitises any incoming `analytics_custom_head_html` on save via the sanitiser. Masks the new `analytics_rybbit_api_key` on every GET — same pattern as Umami's API key and recaptcha secret. - `publicSettings.js`: emits `analytics_tracker_provider`, `rybbit_url`/`rybbit_website_id` (only when provider=rybbit), and the pre-sanitised `analytics_custom_head_html` (only when provider=custom). Legacy `umami_*` fields stay for back-compat. **Frontend**: - `analytics.service.ts` reworked into a provider-aware shape. `initialize({provider, ...config})` dispatches to Umami / Rybbit / Custom / None. `track()` calls dispatch to `window.umami.track` / `window.rybbit.event` / no-op based on the loaded provider. - `App.tsx` `AnalyticsBootstrap` reads `analytics_tracker_provider` from public-settings and routes to the right `initialize` call. Legacy `umami_enabled`-based path preserved as fallback when the new field is missing. - `AnalyticsTab.tsx` (Settings → Analytics) reworked with a "Provider" dropdown switching between None / Umami / Rybbit / Custom panels. Each panel renders its own config fields; Custom panel surfaces an explicit CSP-reminder banner. - `useSettingsState.ts` shape extended with `tracker_provider`, `rybbit_url`/`rybbit_website_id`/`rybbit_api_key`, `custom_head_html`. Save mutation keeps `umami_enabled` in sync with `tracker_provider==='umami'` for back-compat with downstream consumers (publicSettings shape, embedded iframe). - `publicSettings.service.ts` type extended. **i18n**: EN + DE for the provider heading + description + dropdown options + Rybbit fields + Custom HTML field + CSP warning. ## Custom mode — script execution caveat When the gallery `<head>` receives the custom HTML, simply assigning innerHTML to a container element wouldn't execute the embedded `<script>` tags (per the HTML spec, dynamically-inserted scripts via innerHTML are non-running). `analytics.service.ts:120-130` re-creates each `<script>` element manually so the browser actually evaluates it. Non-script nodes (link, meta, noscript) move in directly. ## Tests **Backend** (42 cases, all pass locally): - `umamiAdapter.test.js` (10) — pinned from the original `umamiClient.test.js`: missing-config / URL shape / encoding / payload normalisation / `laptop`→`desktop` / unknown buckets / empty / non-2xx / invalid JSON / network error. - `rybbitAdapter.test.js` (9) — same shape adapted for Rybbit: bare-array + envelope payload, `sessions`/`visitors`/`dimension` key tolerance, encoding, failure modes. - `trackerFactory.test.js` (6) — resolves null for `none`/`custom`, correct adapter for `umami`/`rybbit`, back-compat path via legacy `analytics_umami_enabled`, garbage-provider defensive null. - `customScriptSanitiser.test.js` (12) — Plausible-style passthrough, Umami-style passthrough, inline body passthrough, `<noscript>` allowed, `<link rel="preconnect|dns-prefetch">` allowed, `<link rel="stylesheet">` stripped, disallowed tags stripped, `javascript:`/`data:` URLs stripped, `on*` event handlers stripped, defensive on malformed input. - `analyticsDateMerge.test.js` (5) — preserved from #662. **Frontend**: full 84-case vitest suite green; tsc + eslint clean on changed files. Adapter changes are narrow refactors of code covered by backend tests; no new analytics-page unit test added. ## End-to-end smoke (dockerised backend + my changes mounted) ``` test 1 (back-compat: no provider, umami_enabled=true) → factory returns umami adapter, /analytics returns devicesSource:access_logs (umami fetch to fake host fails gracefully). ✓ test 2 (invalid provider value) → 400 "analytics_tracker_provider must be one of: none, umami, rybbit, custom" ✓ test 3 (save custom HTML with XSS payload) → stored sanitised: `<script>alert(1)</script>evil<script async defer data-domain="x.com" src="https://plausible.io/js/script.js"></script>` (<div> stripped; script tags survive but CSP `script-src 'self'` still blocks inline + non-allowlisted external at runtime) ✓ test 4 (public-settings exposes the provider switch) → `analytics_tracker_provider: 'custom'`, `analytics_custom_head_html: '<sanitised>'` ✓ ``` ## Out of scope (next discussions) - **Plausible native** — covered via Custom mode for now; native is Phase 2 if someone explicitly asks. - **CSP "trusted domains" admin input** — Phase 1.5. For now operators add their tracker domain to nginx/proxy CSP manually; the new CSP-reminder banner in the Custom panel makes that clear. - **Refactor `(window as any).umami.track(...)` direct calls** in PhotoLightbox/PhotoGrid to go through `analyticsService.track()` so events fire on the right tracker. Currently a no-op when Umami isn't loaded; functional but not optimal. Closes #663 Phase 1. |
||
|
Paul Nothaft
|
7534447b6c |
fix(analytics): admin dashboard reads correct fields + Umami device API (#661)
Reporter @alexvaltchev hit three independent bugs on the Analytics
Dashboard. All three fixed in one PR; pluggable-tracker support
(Rybbit, Plausible, etc.) left for a separate discussion.
## Bug A — Summary cards showed 0
Two layers, both fixed.
**Frontend** (`AnalyticsPage.tsx:142-149`): the cards summed
`chartData[].views/uniqueVisitors/downloads`. The backend now (and
already) emits a dedicated `totals` object computed via separate
COUNT queries, which is what the cards should read. Postgres returns
counts as strings, so coerce via `Number()`.
**Backend** (`adminDashboard.js:268-282`): the chartData merge used
`dateObj.date === row.date`. On Postgres, pg's driver auto-converts
`DATE(timestamp)` to a JS Date object — the string-equality match
failed silently and `chartData` stayed all-zero on every Postgres
install with traffic. Added a `normaliseDateKey()` helper that
returns YYYY-MM-DD regardless of driver shape, plus `Number()`
coercion on the counts. SQLite path unchanged.
## Bug B — "Umami Not Configured" banner despite valid config
`AnalyticsPage.tsx:90` did `settings.reduce(...)` on the
`/admin/settings` response. That endpoint returns a
key/value **object** (verified at `adminSettings.js:108-149`), not
an array, so `.reduce` threw `data.reduce is not a function` and
the catch silently rendered the "Not Configured" banner even on
perfectly-configured installs. Read the umami keys directly off the
response object.
## Bug C — Device breakdown 0/0/0
Two-pronged fix.
**Primary path — Umami device API** (`services/umamiClient.js`,
wired into `adminDashboard.js`). When the admin provides an Umami
v2 API key (new setting `analytics_umami_api_key`), the backend
fetches the per-period device breakdown from Umami's
`/api/websites/:id/metrics?type=device` endpoint. Umami tracks
devices natively — far more accurate than our coarse user-agent
heuristic. The new `devicesSource` field in the response lets the
UI hint at where the numbers came from.
**Fallback hardening — local heuristic** (`adminDashboard.js:296-320`).
The existing access_logs `LIKE '%Mobile%' / '%Tablet%'` query stays
in place as a fallback for installs without Umami. Hardened with:
`whereNotNull('user_agent')` skips rows we never captured a UA on,
`Number()` coercion on COUNT results (pg returns strings), and a
guard against divide-by-zero when access_logs is empty.
## API key handling
Mirrors the existing recaptcha-secret pattern: stored plaintext in
`app_settings`, masked as `••••••••` on every GET via the existing
`adminSettings.js` GET handlers, and the frontend save mutation
silently drops the masked sentinel so re-saving without typing a
new key preserves the stored value.
## End-to-end smoke (dockerised backend with my fixes applied)
```
chartData total views: 27 ← previously 0 (date merge broken on PG)
totals: {'views': '27', 'downloads': '3', 'uniqueVisitors': '1'}
devices: {'desktop': 100, 'mobile': 0, 'tablet': 0} ← was 0/0/0
devicesSource: access_logs ← falls back correctly
analytics_umami_api_key (GET /settings/analytics): ••••••••
```
## Tests
**Backend** (15 new cases):
- `umamiClient.test.js` (10): missing-config → null, URL shape +
`x-umami-api-key` header, websiteId URL-encoding, `{x,y}` →
percentages, `laptop` → `desktop` mapping, unknown buckets
dropped, empty payload → null, non-2xx → null, invalid JSON →
null, network error → null.
- `analyticsDateMerge.test.js` (5): YYYY-MM-DD string pass-through,
ISO timestamp slice, JS Date (pg shape) → YYYY-MM-DD, null/empty
→ null, coercion for unexpected types.
**Frontend**: full 84-case vitest suite still green (no analytics
unit tests existed before; not adding any here — the changes are
narrow and the unit-level confidence comes from the type system +
the backend smoke above).
Closes #661 (bugs A + B + C). Rybbit / pluggable tracker support is
the next conversation per the issue author's follow-up.
|
||
|
Paul Nothaft
|
98e97e3cf2 |
fix(i18n): replace ASCII quote with U+201D in DE perGuestLimitsDesc
CI's frontend test job failed with "Failed to parse JSON file, invalid
JSON syntax found at position 163854" on de.json:3041. The German
description used „…" — the opening „ (U+201E) was correct, but the
closing was an ASCII " (U+0022) which the JSON parser treated as the
string terminator, leaving "-Abläufe..." as garbage outside the string.
Replace with the proper German closing quote " (U+201D). 84/84 vitest
suite now passes locally. End-to-end smoke against a dev backend with
migration 141 applied confirms the modal renders correctly on desktop
(centered card) + mobile (bottom slide-up) and the backend returns the
structured 403 on the 11th-click cap hit.
Also flagging adjacent: origin/beta has a pre-existing duplicate `Mail`
import in frontend/src/pages/admin/SettingsPage.tsx (lines 20 + 58 from
commit
|
||
|
Paul Nothaft
|
f2814e4a4c |
feat(feedback): per-guest favorite + like caps with mobile-friendly limit modal (#655)
Reporter @Duecki1 wants to stop telling guests "pick only 5 photos" by
hand. Per-event cap, enforced server-side, with a clear popup when the
11th click would exceed the limit. Per-guest scope matches the "every
couple picks their top 10" mental model; per-gallery aggregate is
explicitly NOT in scope (creates weird "first 10 visitors use up all
slots" race conditions).
## Schema (migration 141)
Two nullable columns on `event_feedback_settings`:
- `max_favorites_per_guest`
- `max_likes_per_guest`
null / 0 = unlimited (preserves current behaviour for every existing
install — operator must opt in). Both shipped together because the
code path is identical; photographers can cap either, both, or neither.
## Backend
- `feedbackService.submitFeedback` cap check on the INSERT branch only.
Toggle-off (un-favoriting) is always allowed, so a guest at 10/10
can free a slot by un-clicking an existing favorite.
- New `countGuestFeedback(eventId, type, guestId, guestIdentifier)` —
matches the exact same guest-key shape the existing duplicate-check
uses (guest_id when present, fallback to guest_identifier in simple
identity mode).
- Limit reduction grandfathers: admin lowering 20→10 keeps existing
rows in place; new adds blocked until the guest removes some.
- Route layer (`galleryFeedback.js` POST) translates a `limit_reached`
service-return into a structured 403 with `code:
'FAVORITE_LIMIT_REACHED'` / `'LIKE_LIMIT_REACHED'`, `limit`, and
`current_count`. Stable UI contract.
- `feedback-settings` GET exposes the caps so the gallery UI can
optionally render a counter near the heart icon (UI extension TBD;
the modal alone is the contract this PR commits to).
- `feedbackValidation`: range guard `0..10000`, null allowed,
per-field error messages.
## Frontend — the popup
New `FeedbackLimitReachedModal` component renders via a `createPortal`
to `document.body` so it escapes any lightbox / sticky parent stacking
context and reliably sits above everything else.
Mobile-first responsive:
- `items-end sm:items-center` — slides up from the bottom on phones
(native action-sheet feel), centers on desktop (familiar modal).
- `w-full sm:max-w-md` — full-width on phones, clamps to 420px on
desktop.
- `rounded-2xl sm:rounded-xl` — more rounded on phones for the
sheet feel.
- `pb-[env(safe-area-inset-bottom)]` — respects the iOS home indicator
and Android gesture bar.
- `z-[60]` — above the lightbox's z-50.
Title + body + "8 of 10 used" pill + "Got it" button. Backdrop click +
Escape both dismiss. Focus management lands on the OK button so
keyboard / screen-reader users can dismiss immediately.
New `useFeedbackLimitModal()` hook is the shared API: components
on every submit-feedback site wire `onError: (err) => handleError(err)`
and render `{limitModal}` in their JSX. Returns `true` from
`handleError` when the error is a structured cap-reached 403 (so the
caller can skip its generic error toast). PhotoFavorites + PhotoLikes
+ PhotoLightbox all wire through the hook — every favorite/like submit
path is covered, including the lightbox's three different submit
sites (guest mode, simple mode, post-identity-modal-confirm).
## Admin UI
`FeedbackSettings` card gets a new "Per-guest limits" section that
only renders when at least one of `allow_favorites` / `allow_likes` is
on. Two numeric inputs (0 / empty = unlimited) side-by-side on
desktop, stacked on mobile. Hint text covers the limit-reduction
grandfathering semantics so admins aren't surprised.
## i18n
EN + DE for:
- Modal title + body (parameterized with `{{limit}}`)
- Counter pill (parameterized with `{{current}}` / `{{limit}}`)
- OK button label
- Admin field labels + hints + section header + grandfathering note
## Tests
**Backend** (`__tests__/utils/feedbackPerGuestLimit.test.js`, 8 cases):
- null cap → unlimited (back-compat)
- 0 cap → unlimited (UI convenience)
- cap=10: rows 1-10 succeed, 11 returns limit_reached
- toggle-off frees a slot at the cap
- limit reduction grandfathers existing rows
- per-guest scope: guest A's cap doesn't affect guest B
- favorite cap doesn't block likes (per-type)
- like cap returns LIKE_LIMIT_REACHED-shaped payload
**Frontend** (`__tests__/useFeedbackLimitModal.test.ts`, 7 cases):
- Non-axios errors → null
- Non-403 axios errors → null
- 403 with wrong code → null
- FAVORITE_LIMIT_REACHED parsed
- LIKE_LIMIT_REACHED parsed
- Falls back to code-implied type when feedback_type missing
- Missing numeric fields → 0 (not NaN)
All 15 pass. tsc --noEmit clean. eslint clean on changed files.
Closes #655.
|
||
|
Paul Nothaft
|
f4b6b8941a |
fix(test): raise bootCrmDb beforeAll timeout on slideshow suites
CI runners hit Jest's default 5s `beforeAll` timeout on slideshowPublic.test.js's bootCrmDb call (~5.4s observed vs ~2s local — runner-to-runner I/O variance, not a regression). Same hook shape on slideshowAdmin.test.js is one slow runner away from the same failure. Raise both to 30s so this stops blocking unrelated PRs branched off beta. Adjacent to #654 — not strictly part of that fix but the only blocker between #656 and a green CI right now. |
||
|
Paul Nothaft
|
b1bfd4838e |
fix(gallery): unbreak password entry in Instagram in-app browser (#654)
Reporter @Duecki1 hit "Incorrect Password" on byte-correct input from
Instagram's iOS/Android IAB. Backend bcrypt compare is fine — the
frontend was handing it a mangled byte sequence because the password
Input lacked the autocaps/autocorrect/spellcheck/autocomplete defenses
Instagram's WKWebView keyboard bridge needs (the standard `type="password"`
WebKit defaults that suppress autocaps get overridden inside the IAB).
Three layers of defense:
1. **Explicit input attributes** on the gallery password field —
`autoCapitalize="none"`, `autoCorrect="off"`, `spellCheck={false}`,
`autoComplete="current-password"`. Stops iOS autocaps turning
`wedding2026` into `Wedding2026`, stops predictive-text rewrites,
nudges password managers to autofill the right credential rather
than the IAB's stale saved-password store.
2. **Silent `.trim()` on submit** — Android Instagram IAB's predictive
keyboard often appends a trailing space when the user taps the
submit button. Event-gallery passwords don't legitimately carry
leading/trailing whitespace (they're set by photographers, usually
generated short strings), so trimming here is safe.
3. **Instagram IAB detection banner** — `frontend/src/utils/inAppBrowser.ts`
detects the `Instagram` UA tag and surfaces a one-time advisory at
the top of the password card with the right platform-specific
"Open in external browser" instructions (⋯ menu copy for iOS,
⋮ for Android). Self-rescue path for users who hit it before we
can close every keyboard mangling vector.
Scope is strictly Instagram per #654. Facebook IAB (`FBAV`/`FBAN`)
behaves identically and would benefit, but expanding the matcher is
a separate scope decision — the detector + i18n shape leaves room for
it without further refactor.
EN + DE i18n for the banner; 8 vitest cases on `detectInAppBrowser`
(iOS / Android Instagram UAs, plain Safari / Chrome / desktop UAs,
case-insensitive match, word-boundary defense against substring
collisions, SSR-safety when `navigator` is undefined). Lint + tsc
clean; pre-push Playwright smoke still expected green.
Closes #654.
|
||
|
Paul Nothaft
|
1f46a241d2 |
chore(whatsapp): renumber migration 138 → 140 (after PR #646's 138+139)
PR #646's review-round renumbered its slideshow migrations to 138 + 139 to slot in after PR #649's 137 (whatsapp_template_language). That now collides with this PR's 138. Slide ours to 140 so all three land in strict order: #649 (137) → #646 (138, 139) → this PR (140). Content unchanged; pure rename + a one-line docstring tweak noting the slot. |
||
|
Paul Nothaft
|
16055cdc41 |
feat(whatsapp): admin-selectable template parameters + reorder (#647 follow-up)
Reporter @Rekoo-PS confirmed the language fix unblocked sending, then
hit a second gap: their template uses only `{{1}} = event_name` +
`{{2}} = gallery_link`, but the legacy `buildComponents` hardcoded all
5 positional values from the `gallery_ready` shape (customer_name,
event_name, gallery_link, password_line, expiry_date). Meta rejected
with a parameter-count mismatch even after the language matched.
This adds a per-config slot list — which built-in values to send, and
in what positional order — so admins can match templates of any shape
without code changes.
## Schema (migration 138)
Additive `template_params` TEXT column on `whatsapp_configs` (default
empty string = legacy 5-slot behaviour for existing installs). Stored
as a JSON-serialized array of slot keys: `customer_name`, `event_name`,
`gallery_link`, `password_line`, `expiry_date`. Unknown / duplicate /
non-string entries are sanitized out at read time.
## Processor
- `parseTemplateParams(raw)` — defensive parser; falls back to the
5-slot default on empty / malformed / all-invalid input.
- `buildComponents(data, metaLang, params)` — emits ONLY the listed
slots in the listed order, computed via a small switch on slot key.
The password line still receives the locale-specific 🔒 label and
the empty-when-no-real-password sentinel handling.
- Processor reads `config.template_params` once per cycle and passes
the parsed array to `buildComponents` per message.
## Admin route
- GET surfaces `template_params` as the parsed array (default 5-slot
when null/empty).
- PUT round-trips the incoming array through `parseTemplateParams`
before persisting, so the stored value is always the canonical
sanitized JSON.
- Test send rebuilt to use the same `buildComponents` path so the
admin's test message matches their configured slot shape — a
reporter who configures 2 slots gets a 2-parameter test send, not
the legacy 5-parameter payload.
## UI
- `WhatsAppTab` gets a checkbox + up/down list under the Template
language field. Each slot shows its current `{{N}}` position when
checked, an em-dash when unchecked. Live preview below the list:
"Your template will receive: {{1}} = event_name, {{2}} = gallery_link".
- EN + DE i18n for the field labels, hint, preview, and per-slot
human-readable names.
## Tests
- 17 unit tests in `__tests__/utils/whatsappBuildComponents.test.js`
covering: parseTemplateParams sanitization (unknown keys, duplicates,
non-strings, malformed JSON, all-invalid fallback, pre-parsed array
acceptance) and buildComponents shape (reporter's 2-slot case,
reorder, empty list, locale-specific password label, password
sentinel handling, expiry omission).
- All 17 + the 34 existing networkValidation tests pass.
## Migration numbering
Sits at 138 on top of PR #649's migration 137. If #646 (Live Slideshow)
merges before this, #646's own 137 + 138 take precedence and this
needs renumbering to 139. Coordinated via PR #646's review thread.
## Honest caveat
Still no Meta Business API account on my side. Spec-built, sanitizer +
shape unit-tested, lint + tsc clean. End-to-end against Meta needs the
reporter (or a maintainer with an account) to verify. If a real
round-trip surfaces a mismatch, drop it in #647 and I'll iterate.
|
||
|
Paul Nothaft
|
4fd7709596 |
fix(whatsapp): admin-pinned template language + Arabic locale support (#647)
Reporter @Rekoo-PS hit three independent gaps trying to deliver an Arabic Meta template. Bundled here because they fan out from the same root cause (no first-class language config on the WhatsApp tab) and the review surfaces are tightly coupled. **1. Test send hardcoded `en_US` (`adminWhatsapp.js:141`).** Smoking gun for "I can't make it work" — Meta returned template_not_found_in_language (132001) on every test send for non-English templates, no matter what else the admin configured. Replaced with `config.template_language || 'en_US'`. **2. No `template_language` field on `whatsapp_configs`.** The only priors were per-message `data.language` (always null from our callers in `adminEvents.js:854,1188`) and `app_settings.general_default_language` (the *system UI* language, not the *template's* language registered with Meta). Migration 137 adds the column; GET + PUT surface it; the processor uses it as the highest-priority default when message_data doesn't override. Resolution order in `whatsappProcessor.processWhatsAppQueue` is now: 1. message_data.language (per-event override — caller path TBD) 2. config.template_language (admin-pinned template language) 3. app_settings.general_default_language (system fallback) 4. en_US (hardcoded last resort) **3. `LANGUAGE_MAP` + `PASSWORD_LABELS` didn't cover Arabic.** Added `ar` (Meta's single-code form per RFC; no region variant). For any language we don't enumerate (e.g. Turkish `tr_TR`, Chinese `zh_CN`, Hebrew `he_IL`), `resolveLanguageCode` now pass-throughs valid-shape codes (lowercase-language + optional underscore + uppercase-region) and forwards them to Meta as-is. If they don't match a registered template Meta returns 132001, which the test route already surfaces back to the admin via `error.message` — fail-loud, no silent fallback. Validation: - Unit smoke on `resolveLanguageCode` across 18 representative inputs (in-map, pass-through, canonicalization, rejection) — all behaviours correct. - Lint clean on all 7 changed files. - Frontend `tsc --noEmit` clean. - Migration `node -c` syntax-checked; additive + `hasColumn`-guarded so re-running is safe. Frontend: free-text input on the WhatsApp tab with EN + DE i18n. Pointing at Meta's supported-languages docs via the hint text — Meta's list grows; a hardcoded dropdown would rot. Closes #647. |
||
|
Paul Nothaft
|
7cf26795ec |
fix(branding): preserve customCss through preset switches + theme changes (#645)
Reporter @aemisrogers nailed the root cause: same #317 class of bug as logoUrl. None of `GALLERY_THEME_PRESETS` (`theme.types.ts:125`) include `customCss` in their `config` object, so any path that REPLACES `currentTheme` with `preset.config` (or with a sparse `newTheme` that came from `preset.config` upstream) silently dropped `customCss` from React state. The persisted value in `theme_config` stayed correct (the public gallery still rendered it), but the admin textarea showed empty on reload — admin-UI display drift, not data loss. Three surgical fixes, mirroring the #317 logoUrl pattern: 1. `BrandingPage.tsx` `handleThemeChange` — `customCss: newTheme.customCss ?? currentTheme.customCss` alongside the existing `logoUrl` fallback. Closes the propagation hole where the customizer's `handlePresetSelect` fires `onChange(preset.config)` (no customCss) and the parent wipes it from currentTheme. 2. `BrandingPage.tsx` `handlePresetChange` — preserve `customCss` from prev/currentTheme on preset switch, same shape as the existing `logoUrl: prev.logoUrl` preservation. Touches both the `setCurrentTheme` and the preview-mode `setTheme` paths. 3. `ThemeCustomizerEnhanced.tsx` `handlePresetSelect` — remove the `setCustomCss('')` that wiped the local textarea state on preset pick. The previous comment ("Clear custom CSS when selecting a preset") described the original intent but produced data drift across the preset round-trip. The sibling `ThemeCustomizer.tsx` already never cleared it; this aligns the two. Verified against `v3.44.0` and `origin/beta`: identical code on both branches, so the bug exists on stable + beta. Lint + tsc clean on the two changed files. Closes #645. |
||
|
Paul Nothaft
|
d705059d3c |
fix(deps): bump qs/brace-expansion overrides + add uuid override for node-cron
Code-scanning Trivy alerts on the open beta (PR #641). Of the 10 open alerts, 6 are stale (lockfile already past the fix) or live in floating-tag base images (`nginx:1.28-alpine`, `node:22-alpine`) which auto-update on the next CI rebuild — no code change needed for those. The 3 actually present in the current `backend/package-lock.json`: - `qs 6.15.0 → 6.15.2` (CVE-2026-8723, alert #266). Bump override from `>=6.14.2` to `>=6.15.2`. - `brace-expansion 5.0.5 → 5.0.6` (CVE-2026-45149, alert #264). Bump override from `>=5.0.5` to `>=5.0.6`. - `uuid 8.3.2` transitively via `[email protected]` (CVE-2026-41907, alert #265). Add top-level `uuid: ^11.1.1` override so node-cron's nested resolution collapses into our root uuid version. node-cron uses only `uuid.v4()` — API-stable across v8 → v11. Verified the scheduler still constructs tasks under the override. Lockfile regenerated; net -9 lines (one fewer uuid copy). Stale alerts that will close on next code-scan rebuild: - #205 postcss (frontend lockfile already at 8.5.14) - #221 i18next-http-backend (backend lockfile already at 3.0.6) Auto-resolved on next image rebuild (no Dockerfile change — floating tags): - #267 nginx (frontend `nginx:1.28-alpine`) - #223 ip-address, #156/#155 picomatch, #140 brace-expansion (all in the npm CLI shipped inside `node:22-alpine`) Refs: code-scanning alerts #264, #265, #266 |
||
|
Paul Nothaft
|
b8211e9944 |
fix(security): close BOLA on photo-export + NAT64 SSRF in URL guard
Two security advisories landed against the open #641 branch — bundling both because they touch independent surfaces and PR #641 is the next beta ship vehicle. **GHSA-9v4w-jrhx-g5wr (BOLA on /admin/photo-export/:eventId/*)** — the three /:eventId-scoped routes in `adminPhotoExport.js` (filtered, filter-summary, export) ran `adminAuth + requirePermission(...)` but not `requireEventOwnership`, so any non-super-admin admin/editor with photos.view (or photos.download) could enumerate + export the photos of events created by other admins — leaking `original_filename`, which routinely encodes client identity. Sibling `adminPhotos.js` applies the middleware on every :eventId route; this file was the single drift. Reporter: Wernerina. **GHSA-wmjx-pc37-272r (NAT64 SSRF in `isPrivateIPv6`)** — the old implementation did naive string-prefix checks (`startsWith('fc')`, `startsWith('fe80')`) and had zero coverage for NAT64 (`64:ff9b::/96` per RFC 6052, `64:ff9b:1::/48` per RFC 8215). On instances with NAT64/DNS64 egress, a webhook URL like `http://[64:ff9b:1::a9fe:a9fe]/` translated through the gateway and reached 169.254.169.254 — exfiltrating cloud metadata (IAM creds) into `webhook_deliveries.response_body`. Rewrote `isPrivateIPv6` to expand the address to its canonical 8-group form, block both NAT64 prefixes, decode embedded IPv4 from IPv4-mapped (`::ffff:0:0/96`) and deprecated IPv4-compatible (`::/96`) forms and re-check via `isPrivateIPv4`, and fail closed on any parse failure. Reporter: tonghuaroot. Added 34 unit tests covering: both NAT64 prefixes in hex + mixed dotted-quad notation, IPv4-mapped IPv6 hex + mixed, deprecated ::IPv4 form, legacy fc00::/fd00::/fe80::/::1/:: cases stay blocked, and public IPv6 (Google/Cloudflare/Google IPv6) negative controls stay allowed. Refs: GHSA-9v4w-jrhx-g5wr, GHSA-wmjx-pc37-272r |
||
|
Paul Nothaft
|
a8bb7b439f |
fix(i18n): wrap WhatsApp token show/hide aria-label through t()
i18n audit caught one straggler — the eye-icon toggle on the access-token
input had a bare `aria-label={showToken ? 'Hide' : 'Show'}` that wouldn't
translate for screen readers on non-English locales. Switched to
`t('common.hide')` / `t('common.show')`; added the matching `common.show`
key in EN + DE (common.hide already existed).
The two remaining `placeholder=` literals in the WhatsApp tab are sample
ID strings (`123456789012345`, `gallery_ready`, `+49123456789`) — those
are identifier/value examples, not translatable English.
Other PR-touched UI surfaces passed the audit clean: 30 new i18n keys
across categories (5), settings.whatsapp (16), settings.features.whatsapp
(2), feedback (3), and the activity-log + bell entries (4) all exist in
both EN and DE.
|
||
|
Paul Nothaft
|
49bfb45332 |
fix(settings): hoist tab-visibility useEffect above isLoading early return
Surfaced while exercising Part D (WhatsApp) end-to-end. Navigating to Settings → WhatsApp triggered React error #310 ("Rendered more hooks than during the previous render"). Root cause is pre-existing: the SettingsPage redirect-to-visible-tab `useEffect` lived AFTER the `if (isLoading) return <Loading />` early return, so on the isLoading=true→false transition the hook count grew by one and React's rules-of-hooks invariant blew up. Move the effect above the early return so the hook count is stable across renders. While here, switch the gating logic from "is the key in the currently-visible nav list" (which the bundle couldn't reference yet because the nav array is built lower down) to a small lookup keyed by activeTab → matching dependency flag. That's an equivalent decision for the four tabs we already gated (crm, contracts, reminderTemplates, accounting) plus the new whatsapp tab. Add `flagsLoading` from the FeatureFlags context to the deps so the snap-back only fires once the server's actual flag values have arrived. Without this, the initial render with the placeholder DEFAULT_FLAGS would falsely snap away from any tab whose flag is "on" on the server but absent from the placeholder. Also add `whatsapp: false` to `DEFAULT_FLAGS` in FeatureFlagsContext (was missing — TypeScript should have caught the Record<FeatureKey, boolean> violation but the build pipeline didn't surface it). Without this, `flags.whatsapp` is undefined on the placeholder, which had secondary effects on tab visibility and the snap-back logic. Verified via Chrome DevTools: Settings → WhatsApp now loads cleanly with all 5 form fields, the saved config values prefilled, the Save button, and the Send-test card. |
||
|
Paul Nothaft
|
fabd67aecd |
feat(feedback): export shape toggle — per-action vs per-guest pivot (#640 part E)
Ports 8digit/picpeak@ed7943b as a TOGGLE rather than a replacement. The current per-action shape (one row per favourite/like/rating/comment) stays the default for backward compat with any external scripts consuming the export; the new pivot shape (one row per (photo, guest_identifier) with boolean is_favorited/is_liked + star_rating + comment) is opt-in via a ?shape=pivot query param and a dropdown in the admin feedback page. Pivot wins for "which guests engaged with which photos" analysis in Sheets / Excel pivot tables. Long wins for engagement timeline analysis and re-importing into another tool. Different products, both valid. ### Backend - `feedbackService.exportEventFeedbackPivoted(eventId)`: new method. LEFT-of-Map approach, pure JS pivot so PG / SQLite behave identically. Key is `(filename, guest_identifier)` — anonymous guests with no identifier get a synthetic per-row key so two anonymous comments on the same photo don't collapse. Comments: most recent wins (history dropped in exchange for "current state" semantics). Hidden-by-moderator rows excluded — the pivot represents what we want to surface, not the raw event log. - `adminFeedback.js` export route: accepts `?shape=pivot|long` (default `long`). CSV filename now carries the shape (e.g. `feedback-pivot-{id}.csv`) so repeated exports don't overwrite. - `convertToCSV` helper in `adminFeedback.js` gains the three escaping improvements that 8digit's commit also shipped: booleans → `yes`/`no`, null/undefined → empty, escape strings containing newlines (\n/\r) as well as commas/quotes. Comments with line breaks were silently breaking CSV row counts before this. Improvements are pure wins regardless of shape; archives' own `convertToCSV` copy left untouched (separate surface, no behaviour drift risk). ### Frontend - `feedback.service.ts` `exportEventFeedback()` gains optional `shape` parameter, default 'long'. - `EventFeedbackPage.tsx`: new shape dropdown next to the CSV / JSON buttons (defaults to 'long'). Selected shape flows through to the API request AND the downloaded filename. ### i18n 3 new EN + DE entries (`feedback.exportShapeLabel`, `feedback.exportShapeLong`, `feedback.exportShapePivot`). ### Notes - Pivot shape is **per-guest current state**, not history. A guest who rated a photo, then changed their mind and removed the rating, would show the final state in the pivot but BOTH actions in the long form. Acceptable trade-off: pivot users care about the snapshot, long users want the trail. - `latest_at` column in pivot gives a "most recent activity" timestamp per row, useful for sorting/filtering recent engagement. ### Test plan - [x] Backend syntax + TS check + lint clean (no new warnings; existing `catch (error)` warning was pre-existing) - [ ] Manual: feedback page → select Per-guest (pivot) → Export CSV → verify one row per (filename, guest) with is_favorited='yes'/'no', latest_at column populated - [ ] Manual: long shape default still produces the same per-action output as before (no regression for existing consumers) - [ ] Manual: comment containing a newline → pivot CSV escapes correctly, row count matches data length + 1 header - [ ] Manual: archive a published event with feedback → archive's `feedback_data.csv` still uses the long shape (archive surface unchanged on purpose) |
||
|
Paul Nothaft
|
78c8e9d9f9 |
feat(whatsapp): WhatsApp Business API notification channel (#640 part D)
Ports filpgame/picpeak's WhatsApp integration with substantial adaptation
to fit our codebase patterns. Deliver the gallery-ready notification via
Meta Graph API in addition to (or instead of) email — useful where the
customer base expects WhatsApp by default. Strictly opt-in behind the new
`whatsapp` feature flag.
### Backend
- **Migration 136** (`whatsapp_configs` + `whatsapp_queue`). Loose-FK on
`event_id` matching our `inbound_documents` / `expenses` pattern (NOT
filpgame's hard FK — deleting an event shouldn't RESTRICT on stale queue
rows). Composite index on `(status, retry_count, created_at)` covers the
poll path.
- **`whatsappService.js`**: thin Meta Graph client. Meta API version bumped
v19 → v20 (filpgame's v19 deprecates Q3 2026); configurable via
`WHATSAPP_META_API_VERSION` env var. Timeout dropped 10s → 8s for
processor budget. Errors surface the Meta `error.code` so the processor
can tell retryable from permanent.
- **`whatsappProcessor.js`**: queue processor polling every 30s (configurable
via `WHATSAPP_QUEUE_POLL_MS`), 10 messages per cycle, 3 retries before
marking `failed`. Default language sourced from
`app_settings.general_default_language` (matches our email-language
resolution pattern); replaces filpgame's hardcoded `pt_BR` fallback.
Falls back to `en_US` if nothing is configured. No-ops gracefully when
the `whatsapp` flag is off, the config row is missing, or the access
token isn't set.
- **`adminWhatsapp.js`**: three routes (GET/PUT config, POST test). Gated
by `requireFeatureFlag('whatsapp')` so operators who haven't enabled it
can't see the surface. Access token masked as `'********'` on GET;
masked values silently preserve the stored token on PUT. Enabling with
no Phone Number ID, template name, or token (and none stored) fails at
the validator.
- **Two hook points** in `adminEvents.js`:
- **Create-and-publish-in-one-step**: queues immediately after the
`gallery_created` email when `!isDraft && customerPhone &&
waConfig.enabled`. Password from `req.body` is still in scope.
- **Publish-from-draft** (`POST /:id/publish`): queues with the password
the admin re-typed via PR #627's `PublishGalleryDialog`. When no
password was typed (legacy API consumers without dialog), passes empty
string so the password line renders blank rather than leaking the
`(set at creation)` sentinel.
- **`server.js`**: starts `whatsappQueueProcessor` at boot. Non-fatal if it
fails to start (logged as warning).
- **`feature_flags`**: new `whatsapp` flag in `KNOWN_FLAGS` and
`DEFAULT_FLAGS` (default false).
### Frontend
- **`featureFlags.service.ts`**: `'whatsapp'` added to `FeatureKey` union.
- **`FeaturesTab.tsx`**: WhatsApp card in the Communication section
(between Incoming mail and Messaging). Smartphone icon, "new" status,
sidebar-hidden (no sidebar entry — config lives under Settings).
- **`whatsapp.service.ts`** (new): typed client for the three admin routes.
- **`WhatsAppTab.tsx`** (new): Settings tab. Form for Phone Number ID,
WABA ID, access token (masked toggle), template name, and enabled flag.
Separate card below for a static test send. Token masking matches the
server's `'********'` sentinel — admin can edit other fields without
re-entering the token.
- **`SettingsPage.tsx`**: WhatsApp tab nav item gated on `flags.whatsapp`
(so it shows only when the feature is enabled); render block wires
`<WhatsAppTab />`.
### i18n
22 new EN + 22 new DE entries covering the Settings tab form, the
Features-tab card, plus `admin.activities.whatsapp_config_updated` +
`admin.notificationMessages.whatsappConfigUpdated` for the bell /
dashboard surfaces from PR #637.
### Deliberately NOT included
- filpgame's **password-encryption-at-rest** layer
(`password_encrypted`/`password_iv`/`password_key_version` columns).
Our publish-from-draft password recovery uses the admin re-type flow
from #627 (PublishGalleryDialog) — no plaintext at rest.
### Setup notes for operators
1. Create a Meta Business Account + WhatsApp Business App.
2. Register a phone number and obtain `phone_number_id` + `waba_id`.
3. Create a system-user access token (long-lived recommended).
4. Submit a message template for approval. The default `gallery_ready`
expects 5 body parameters: customer name, event name, gallery link,
password line, expiry date.
5. Enable the `whatsapp` feature flag.
6. Enter credentials under Settings → WhatsApp, send a test, then enable
delivery.
### Test plan
- [x] Backend `node -c` on all new/changed files clean
- [x] `tsc --noEmit` on frontend clean
- [x] Backend dev container restart picks up new files, /health OK
- [ ] Manual: enable `whatsapp` flag → Settings → WhatsApp tab appears
- [ ] Manual: save config with masked-only token (existing token preserved)
- [ ] Manual: enable=true without phone_number_id rejected at PUT
- [ ] Manual: enable=true without stored or new token rejected at PUT
- [ ] Manual: create-and-publish event with customer_phone → queue row
inserts with message_type='gallery_created'
- [ ] Manual: publish-from-draft via PublishGalleryDialog with password →
queue row uses the admin-typed password in the {{4}} line
- [ ] Manual: test send to a real phone with valid Meta config + approved
template → Meta returns messages[0].id, toast shows the id
- [ ] Manual: bell renders "WhatsApp configuration updated" in DE when
the config_updated activity fires (via PR #637 smart default)
|
||
|
Paul Nothaft
|
a3fcb5bc9e |
feat(common): generic Promise-based ConfirmDialog primitive (#640 part C)
Ports 8digit/picpeak@88bfde1 — replaces `window.confirm()` with a styled, themed, accessible in-app modal. Usage: const confirm = useConfirm(); const ok = await confirm({ title: 'Delete event?', message: 'This will permanently remove the gallery and all photos.', variant: 'danger', confirmLabel: 'Delete', }); if (ok) doDelete(); Three variants: 'primary' (default, no icon), 'danger' (red AlertCircle + red confirm button), 'warning' (amber AlertTriangle). Keyboard support: Escape cancels, Enter confirms (unless focus is in an input/textarea/select so an open form doesn't get hijacked), backdrop click cancels. Cancel button is focused by default — a stray Enter cannot accidentally confirm a destructive action. Wraps at App.tsx level, inside GlobalThemeProvider so the modal respects the theme tokens, above the toast container so a confirm appearing under a toast still gets the click. Provider exports through components/common alongside the rest of the shared primitives. This PR only lands the primitive. Existing window.confirm() call-sites are left untouched — sweeping them is follow-up work that can land in any cadence (each sweep is one component, no architectural risk). Existing structured-input flows (PublishGalleryDialog, DuplicateEventDialog, PasswordResetModal, etc.) stay as-is — they collect data, not yes/no. No new i18n entries — uses common.cancel / common.confirm / common.close which already exist in EN + DE. ### Test plan - [x] tsc --noEmit clean - [x] eslint clean on changed files - [ ] Manual: pick any existing window.confirm() site (e.g. EventDetailsPage delete button), swap to useConfirm(), verify the modal renders with theme tokens, Escape cancels, Enter confirms, backdrop click cancels, focus lands on Cancel - [ ] Manual: variant='danger' renders red confirm button + AlertCircle icon - [ ] Manual: open the dialog from inside another modal (e.g. a settings panel) — z-[9999] keeps the confirm on top of any other overlay |
||
|
Paul Nothaft
|
820f4835f1 |
feat(categories): per-category download permissions (#640 part B)
Adds an `allow_downloads` boolean to `photo_categories` so admins can have different download policies per category — e.g. preview categories public, originals client-only. AND's with the event-level `allow_downloads`, so disabling at either level blocks downloads for that category's photos. Defaults to true so categories created before migration 135 keep working without admin intervention. Credit: 8digit/picpeak@928164b + @751ec75. ### Backend - **Migration 135**: additive `allow_downloads BOOLEAN NOT NULL DEFAULT true` on `photo_categories`, hasColumn-guarded + sane down. - **`adminCategories.js`**: PUT /:id accepts optional `allow_downloads` patch. - **`gallery.js`**: - `GET /:slug/photos` returns `allow_downloads` per category AND `category_allow_downloads` per photo. - `GET /:slug/download/:photoId` returns 403 when the photo's category disables downloads. - `GET /:slug/download-all` LEFT JOINs `photo_categories` and filters `whereNull(category_id) OR allow_downloads=true OR allow_downloads IS NULL`. The null check covers pre-migration-135 rows during the upgrade window. - `POST /:slug/download-selected` same filter pattern. ### Frontend - **`categories.service.ts`**: `updateCategory()` gains an optional `patch` argument carrying `{ allow_downloads }`. PhotoCategory interface gains the optional field. - **`EventCategoryManager.tsx`**: new toggle button next to the delete X. Green DownloadCloud icon when downloads are on, plain Download icon when off. Click toggles via the new mutation; toast confirms. - **`PhotoLightbox.tsx`**: `photoAllowsDownload = allowDownloads && currentPhoto?.category_allow_downloads !== false`. Hides the download button + blocks the 'D' keyboard shortcut + early-returns from handleDownload. - **Types**: Photo interface gains `category_allow_downloads`. - **i18n**: 5 new EN + DE entries for the toggle button toast + tooltip. No global-category surface change yet — global categories don't currently have a UI for the toggle. Admins can still flip the column directly via SQL or via a future global-categories editor. ### Test plan - [x] Backend syntax + TS check clean - [x] ESLint: no new warnings - [ ] Manual: admin → event detail → categories panel → click DownloadCloud icon → category flips, toast confirms - [ ] Manual: gallery (guest) → photo in disabled category → lightbox shows no download button, 'D' shortcut is a no-op - [ ] Manual: download-all on a gallery with one disabled category → ZIP excludes that category's photos - [ ] Manual: download-selected including a disabled-category photo → 404 (filtered out) and the response carries only the allowed selection - [ ] Manual: pre-migration-135 category (legacy row with NULL allow_downloads) → downloads still work (defaults true via fallback) |
||
|
Paul Nothaft
|
e4e79a0b3a |
fix(archives): stream-extract restore for >2 GiB + preserve original_filename via manifest (#640)
Two related backup-integrity fixes from 8digit's fork (issue #640 items #3 + #4), bundled because they touch the same two files and ship better together than apart. ### Stream-extract restore for >2 GiB archives `adminArchives.js:170` was using `adm-zip`, which loads the entire ZIP into a Node Buffer before extracting. Node has a hard 2 GiB Buffer cap, so any restore over that limit fails with `ERR_FS_FILE_TOO_LARGE` — and since the frontend `onError` toast is the generic "Something went wrong", the cause stays invisible. Real-world wedding archives routinely cross 2 GiB; affected restores have likely been silent failures. Swapped `adm-zip` for `node-stream-zip` which streams each entry to disk as it's processed — no full-file Buffer, no 2 GiB ceiling. API shape: ```js const zip = new StreamZip.async({ file: archivePath }); const entries = Object.values(await zip.entries()); await zip.extract(null, eventDir); await zip.close(); ``` Re-import logic (photos, categories, sizes) unchanged; only field rename `entry.entryName` → `entry.name`. Credit: 8digit/picpeak@69033c6. ### Preserve `original_filename` via photos manifest Archive → restore round-trip currently loses `original_filename` (the post-#508 column tracking the camera-side name) because the gallery filenames are renamed on upload and can't be derived from the extracted files. This matters now that the Lightroom export (#623) depends on `original_filename` — a restored event lost that signal. - **`archiveService.js`**: writes `photos_manifest.json` into the archive containing per-photo `{filename, original_filename, type, uploaded_at, category_name}`. Non-fatal: a manifest write failure falls through to legacy behaviour (filename used as original_filename, same as before). - **`adminArchives.js`**: reads the manifest on restore, builds a `Map<filename → manifest>`, and assigns `original_filename = manifest?.original_filename || filename`. Archives produced before this lands have no manifest — restore logs a one-shot notice and falls back to filename, preserving backward compat. Credit: 8digit/picpeak@eb018aa. ### Deps - Removed `adm-zip ^0.5.16` - Added `node-stream-zip ^1.15.0` ### What's NOT in this PR 8digit's commit also fixed the production compose healthcheck (`curl` isn't in our Alpine image); that's already been addressed upstream in the meantime. The frontend `onError` swallow on the restore toast is a separate small follow-up. ### Test plan - [x] `node -c` on both files clean - [x] `node-stream-zip` async API verified at load time - [ ] Manual: archive a multi-GB event → restore → confirm photos re-import with original_filename preserved - [ ] Manual: restore an archive produced before this lands → confirm fallback to filename works (no manifest path crashes) - [ ] Manual: confirm the new photos_manifest.json is inside the generated archive (`unzip -l <archive>.zip | grep manifest`) |
||
|
Paul Nothaft
|
eaed00fcca |
Merge remote-tracking branch 'origin/beta' into fix/i18n-activity-types-comprehensive
# Conflicts: # frontend/src/i18n/locales/de.json # frontend/src/i18n/locales/en.json |
||
|
Paul Nothaft
|
997a41293e |
fix(i18n): sweep Events / API Tokens / Webhooks settings tabs
Continuing the activity-type i18n sweep from this PR: three settings
tabs still had hardcoded English strings (or referenced i18n keys that
didn't exist in either locale).
EventsTab (Settings → Event Creation):
- defaultFeedbackEnabled + defaultFeedbackEnabledHelp were referenced
by the component but missing from both locales. The inline-default
English text leaked through to German users.
ApiTokensTab (Settings → API Tokens):
- "Preview" table-header column was a bare string literal; now wraps
through t('settings.apiTokens.preview').
- confirmRevoke called t() with a backtick template-literal default
("Revoke \"${token.name}\"…"). The interpolation happened at the
default-string level, so the actual translated string never received
the name and shipped without it. Switched to the i18next {{name}}
parameter pattern with the matching value in en+de.
WebhooksTab (Settings → Webhooks):
- Half the tab was still hardcoded English. Wired everything through
t(): toast messages (createError, updateError, deletedToast,
deleteError, copied, copyFailed), Just-Created Secret card buttons
(Copy, Dismiss), form placeholders (name, URL, template), advanced
toggle label, filter and template help paragraphs, the filterError
setter, all six table headers, the eventsSubscribed count (with
proper {{count}} pluralisation), the status badge (Active/Disabled),
the active/inactive title tooltips, the Deliveries link, the Delete
button, and the delete-confirm dialog (proper {{name}} interpolation
instead of the broken template-literal-in-default-string pattern).
Added 34 new key/value pairs to each locale; counts now symmetric at
events=28, apiTokens=23, webhooks=43 in both EN and DE.
DE wording authored natively; tone matches the existing maintainer-
voice style.
|
||
|
Paul Nothaft
|
bc8d3330bb |
fix(i18n): sweep activity-type translations + smart notification fallback
The admin notification bell and dashboard "Recent Activities" panel were
showing raw snake_case keys ("event_published") or the generic
"Systemaktivität: <type>" fallback for ~65 activity types — most of them
from the CRM and Accounting modules added since #555. Users with German
locale saw the gap most visibly because the English placeholder leaked
through.
Three pieces:
1. notifications.service.ts — smart `default:` branch. Instead of falling
straight to the systemActivity template, derive the camelCase i18n key
from the snake_case type, try resolving `admin.notificationMessages.<camelCase>`
directly with the full metadata spread as params, and only drop to the
legacy template when no specific translation exists. This means every
future activity type just needs an i18n entry — no per-type switch
case to add.
2. en.json + de.json — added 65 missing `admin.notificationMessages.*`
bell entries and 58 missing `admin.activities.*` dashboard entries
across both locales. Covers Contracts (13), Quotes (7), Invoices /
Storno (12), Monthly billing (5), Expenses (4), Hours (5), Incoming
invoices (6), Customers (1), Admin user mgmt (3), and 9 misc /
legacy types (bulk_archive_completed, email_resent, email_queue_flushed,
email_template_created, event_duplicated, feedback_deleted,
feedback_moderated, feedback_settings_updated, word_filter_added).
Both locales finish symmetrical (149 activities / 136 notifications
each, vs. 91 / 71 before).
3. admin.service.ts `formatActivityMessage` messages dict — added the
same 58 English-only entries as a last-resort fallback for the
dashboard when i18n itself fails to load. Keeps the surface
resilient against bundle-load issues.
Metadata field names in the new translations match what the backend
writes via `logActivity()` — `{{contractNumber}}`, `{{quoteNumber}}`,
`{{invoiceNumber}}`, `{{username}}`, `{{template_key}}`,
`{{source_event_name}}`, `{{word}}` — verified against the call sites
in contractService, quoteService, invoiceService, userManagementService,
expenseService, adminEvents, adminEmail, adminFeedback.
DE wording authored natively; tone matches the existing terse,
maintainer-voice style of the rest of the file.
|
||
|
Paul Nothaft
|
27b5f7e4b6 |
feat(admin/exports): inline preview modal with copy-to-clipboard (#631)
Follow-up to #623. The Lightroom TXT export now shows the filename list in a modal with a "Copy to clipboard" button instead of triggering a .txt file download — saves the "open file → select all → copy" dance admins were doing anyway. CSV export takes the same path (paste straight into Sheets / Excel). The modal keeps a "Download as file" button so admins who want the file (sharing with colleagues, archiving, post-processing tooling) aren't worse off than before — fully additive. XMP (ZIP archive) and JSON exports keep their direct download path. A textarea preview is the wrong UI for a binary archive, and JSON is structured tool input where the file form is the natural mode. Implementation: - ExportPreviewModal — readonly textarea, copy + download buttons, monospace font for filename lists, click-to-select-all on the textarea for browsers that block clipboard writes (older Safari, hardened sandboxes — the catch falls through to a "select and copy manually" toast instead of silent failure). - photosService.exportPhotosAsText — same backend endpoint as exportPhotos but resolves the blob.text() and returns { content, filename } instead of triggering a download. Preserves the existing exportPhotos for the XMP / JSON paths. - PhotoExportMenu — PREVIEW_FORMATS = ['txt', 'csv']; non-preview formats keep the direct-download flow unchanged. - EN + DE i18n entries. No backend changes. No new endpoints. No breaking changes for callers of photosService.exportPhotos. |
||
|
Paul Nothaft
|
e985d25207 |
feat(events): duplicate-gallery action (#626)
Daniel asked for a way to re-use a good gallery configuration without re-entering every setting. Two of his three suggested workflows are covered by this PR; the third (per-event-type behaviour defaults) is partially shipped already via event_types.theme_preset + theme_config and is left as a follow-up if the duplicate workflow doesn't cover it. Backend — POST /admin/events/:id/duplicate. Validates a new event_name (required) + event_date (optional) + customer_name/email (optional); copies branding (color_theme, css_template_id, header/hero/divider/anchor), behaviour toggles (allow_downloads, watermark_*, allow_user_uploads, require_password, etc.), photo_cap, welcome_message, default_photo_sort, admin_email, and feedback settings + per-event photo categories. Mints a fresh slug + share_token + random-placeholder password_hash (admin sets the real one via the publish dialog shipped in #627). Recomputes expires_at = new_event_date + (source.expires_at - source.event_date) so the duplicate keeps the same active window; defaults to 30 days if either source field was null. is_draft is always true. Deliberately NOT carried over: photos, hero_photo_id, client_access secrets, og_image_share opt-in, customer_phone, sent_at flags, archive state, customer-account assignments. Frontend — new DuplicateEventDialog (matches the PublishGalleryDialog pattern), wired into the Actions card on EventDetailsPage. Visible in both draft and live mode since admins typically duplicate from a published gallery. On success the page navigates to the new draft so the admin can finish customising + publish. I18n: EN + DE entries for the dialog + button label. Backend logs an event_duplicated activity with the source event id/name so the trail is auditable. Frontend service: eventsService.duplicateEvent(eventId, data). |
||
|
Paul Nothaft
|
714a9f6fb1 |
fix(upload): auto-throttle on low-memory hosts + correct documented RAM minimum (#628)
The README claimed 2GB RAM as the minimum, but two background-processor worker loops × sharp.concurrency(2) means up to four libvips threads can decode full-resolution images in parallel — peak RSS lands at 1.5GB+ on a batch of 20MP+ photos. Add Postgres + Redis + Node baseline and one heavy batch on a 2GB VPS OOM-kills the backend, surfacing as 503s on thumbnails until restart:unless-stopped brings it back. Reported in #602, filed as #628. Three changes, smallest-surface-area each: 1. backgroundProcessor.js — on startup, when UPLOAD_PROCESSOR_CONCURRENCY is NOT set and os.totalmem() reports < 3GB, default to 1 instead of 2 and log a one-shot warning naming the override env var. Explicit env-var setters keep their value. os.totalmem() reports container memory under cgroup v2 so this works in Docker / k8s as well as bare metal. 2. README.md — bumped the documented minimum from 2GB to 4GB, kept 2GB only as a "Low-memory hosts" recipe pointing at UPLOAD_PROCESSOR_CONCURRENCY=1 with the throughput trade-off spelled out. Added the 503-on-OOM symptom so the next reporter finds it via search. 3. docker-compose.production.yml — commented mem_limit / memswap_limit example on the backend service. Off by default (don't surprise existing deployments) but visible to operators thinking about shared/multi-tenant hosts. restart:unless-stopped already on every service. No code path for memory-aware runtime throttling (Luca's option 4) — out of scope for a bug fix; tracked separately if #1-#3 don't close the case. |
||
|
Paul Nothaft
|
83b568ee2d |
fix(events): publish-from-draft email carries the real password (#627)
Previously, publishing a password-protected DRAFT gallery sent the gallery_created email with the literal sentinel "(set at creation)", which the email processor localised to "The password you set when creating the gallery" / "Das bei der Erstellung der Galerie gesetzte Passwort". Root cause: at draft creation only the bcrypt hash is stored (no plaintext column, by design); the publish endpoint had nowhere to pull the actual password from. Create-and-publish-in-one-step worked because the plaintext is still in memory at email-queue time. Fix: the Publish action now opens a small PublishGalleryDialog that prompts the admin to (re-)type the gallery password. The publish endpoint accepts an optional `password` body, re-hashes + writes `password_hash` so the stored hash matches what was just emailed (admins who mistype at creation get a self-healing publish flow), and puts the plaintext into the gallery_password email field. When the publish call is made without a password (API-only consumers), behaviour falls back to the legacy sentinel — no breaking change. The window.confirm() publish flow is gone; the dialog handles the no- password case too (plain confirm + Publish button). I18n: EN + DE entries for the dialog. Other locales fall through to the EN defaults via the t() default-value pattern. No schema changes. No plaintext at rest. |
||
|
Paul Nothaft
|
ea6245cfde |
fix(gallery): admin edits to welcome_message land for returning guests (#625)
GalleryAuthContext cached the event in sessionStorage on first visit and then SKIPPED the server fetch on returning visits (`if (!storedEvent)`), so a guest who'd already opened the gallery would never see admin edits to welcome_message / event_name / hero_logo / colour theme — sessionStorage survives Cmd+Shift+R, so the only escape was closing the tab or wiping site data manually. The cached event is still shown above as an instant placeholder for perceived perf, but the server fetch is no longer gated: on every mount the fresh row overwrites both React state and the sessionStorage entry. Cost is one extra /gallery/:slug/photos request per gallery navigation when the session is already authenticated; benefit is admin edits propagating on next page load for everyone. |
||
|
Paul Nothaft
|
178d6dafb1 |
fix(gallery): leave a visible gap between filter bar and hero header (#624)
When a gallery uses the 'hero' header_style AND the admin enables the filter bar (search + sort), the search/sort row glued itself to the top of the hero image. Root cause: HeroHeader carries a decorative `-mt-6` on its outer div (so it can bleed flush against the page header when nothing else is above), and that exactly cancelled the wrapper's `mt-6` between PhotoFilterBar and PhotoGridWithLayouts. Fix: when the filter bar is shown above a hero header, the grid wrapper uses `mt-12` instead of `mt-6` so the hero's bleed leaves a 24px net gap rather than zero. The no-filter-bar case keeps the original flush bleed. Also tidied up: extract the filter-bar-shown predicate to a named const so the two reads (conditional render + wrapper class) can't drift apart. |
||
|
Paul Nothaft
|
a239fec9d7 |
fix(admin/exports): Lightroom TXT export joins with comma + drops extension (#623)
The PhotoExportMenu's TXT format advertises "Simple text list for Lightroom search" but emitted newline-separated filenames WITH `.jpg`. Lightroom's filename search wants a comma-separated one-liner, and the gallery JPEGs may correspond to RAW files in the catalog — so the search has to match on the stem only. The frontend now passes `separator: 'comma'` + `include_extension: false` for the TXT format specifically. The backend gains an `include_extension` option (defaulting to true so direct API consumers don't break), and the comma case joins without a trailing space (the form Lightroom expects). Unit test pins the Lightroom-mode output AND the backward-compatible default for any direct API caller. CSV / XMP / JSON exports are unchanged. |
||
|
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.
|