64e4925fbb60052eb3bc29d3774a04414d249120
471
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Luca
|
09f6a1af6a |
fix(backup-ui): respect general_date_format + general_time_format
The four backup admin panes (BackupHistory, BackupDashboard, BackupCoverageCard, BackupIntegrityCard) used raw date-fns `format()` with hard-coded tokens like 'p' (12-hour AM/PM), 'PP', 'PPP', 'PPp', and 'yyyy-MM-dd HH:mm:ss' — ignoring the admin's configured `general_date_format` and `general_time_format` settings. Net effect on a 24h-configured install: backup History row showed "11:25 PM" instead of "23:25", and the Coverage tab's "Last dump" + "Coverage generated" timestamps were stuck on yyyy-MM-dd HH:mm:ss regardless of the admin's date-format choice. All four panes now route through `useLocalizedDate()` which honors both settings + the active i18n locale (per the existing [[feedback_respect_general_format_settings]] pattern). Tokens replaced: format(date, 'p') → formatTime(date) format(date, 'PP') → format(date) format(date, 'PPP') → format(date) format(date, 'PPp') → formatDateTime(date) format(date, 'yyyy-MM-dd HH:mm:ss') → formatDateTime(date) format(date, 'yyyy-MM-dd HH:mm') → formatDateTime(date) No backend changes — settings already shipped via /admin/settings; this just makes the consumers actually read them. |
||
|
Luca
|
83fdb47fbf |
feat(installer): install picpeak directly from a backup via trigger file
Closes the six-step DR dance ("onboard throwaway admin → restore via
wizard → log out → log back in with originals") by letting admins
recover an install with zero clicks past `docker compose up`.
Convention: drop a file named `RESTORE_ON_INSTALL` (no extension OR
.txt) into the existing `/backup` bind mount. On next container
start, the new boot hook detects it, runs the restore, and starts
the server with the restored state. Admin opens the browser, login
works first try.
Payload variants:
- empty file → auto-picks newest backup-manifest-*.json from
/backup/manifests/. Useful for "restore the latest".
- path inside the file → uses that specific manifest. Useful for
"I want this older backup, not the most recent".
Safety gates (three layers):
1. Trigger file must exist — no auto-magic, admin signals intent
2. DB must be empty (no events, ≤1 admin) — refuses to clobber
production data
3. Restore failure leaves the trigger file in place for retry on
next container start. Success deletes it so subsequent boots
don't redo the work.
Override hook: INSTALL_FROM_BACKUP_FORCE=true skips guard #2 for the
"I know what I'm doing" edge case (dev env rebuilds, etc).
No docker-compose changes required — uses the bind mount picpeak
already has, env vars are optional. The minimal admin workflow now
matches the bare-minimum mental model: "copy my backup files,
restart the container, log in with original credentials."
Tests: 7 scenarios covering trigger detection, payload variants,
safety gates, success/failure trigger-file lifecycle.
|
||
|
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. |
||
|
Luca
|
e7dffa656b |
feat(backup-stats): per-Stage-B-path counters in backup statistics
Closes the last gap from tonight's backup-hardening: backup_runs.
statistics now carries a `per_path` map keyed by backup_paths.path
(e.g. `events/active`, `business-docs`), with per-bucket count + size.
Backend (backupService.js):
- new `computePerPathStats(backedUpFiles, allFiles)` helper that
bucket-sorts each backed-up file into its owning backup_paths row
by longest-prefix match. Reuses the same backup_paths source the
walker reads, so toggling include_in_default off propagates
correctly. Falls back to LEGACY_BACKUP_PATHS if the table is
missing.
- runBackupInternal calls it after the destination implementation
reports back, includes the result in statistics under both
snake_case (`per_path`) and camelCase (`perPath`) keys for the
same alias treatment the existing fields get.
Frontend (BackupHistory.jsx):
- Backup History detail pane now renders one row per per_path entry
when present, with path label + count + formatted size.
- Falls back to the legacy Photos / Archives / "Other" rendering
when the field is absent (backups taken before this commit). No
breaking change for stored history.
Tests: new backupService.perPathStats.test.js — 2 scenarios pinning
attribution behaviour (single-path, nested-paths-don't-collide).
Plus a NOTE comment about overlapping-path walker behaviour (out of
scope; canonical seed doesn't hit it).
|
||
|
Luca
|
e0ace0864e |
fix(restore): preserve operator-meta settings across restore
Closes the chicken-and-egg where `restore_allow_force` (and its
auto-upgrade tracking flag) got overwritten on every restore by
whatever value happened to be in the backup. Net effect:
1. Admin enables Force Restore (via tonight's default-ON migration
edit, or hand-SQL on older installs).
2. Restore runs successfully.
3. Restored DB has `restore_allow_force = <backup's old value>`.
4. Next restore attempt: "Force restore is not allowed by system
settings" — admin needs the SQL workaround AGAIN.
Cure: snapshot a small list of operator-meta keys BEFORE the DROP
DATABASE (while we still have a working pool against the OLD DB),
then UPSERT them back AFTER the psql restore + migrate.latest.
The preserved set is intentionally narrow — currently just
`restore_allow_force` and `restore_allow_force_auto_upgraded`. These
are about how the operator wants the install to behave, not user-
facing state. Adding more keys is a one-line addition to the
PRESERVED_META_KEYS constant.
Survives both:
- backup is OLDER than the operator's most recent setting change
- backup is NEWER but had a different operator policy
Either way, the post-restore install reflects the LIVE operator
policy, not the backup's snapshot of it.
|
||
|
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
|
||
|
Luca
|
989b42c2f1 |
fix(restore-wizard): warn on backups without a database dump
Closes the loop on the original 2026-05-29 data-loss class: Ralf had
four "Run Backup Now" manifests sitting on disk with
database.backup_file = null because Stage A wasn't yet in place.
The restore wizard would have happily restored any of those four,
bringing back files (photos, PDFs) but leaving the database empty —
silently re-creating the exact data loss the rest of this branch
prevents going forward.
The /restore/list-backups endpoint now returns `database_included`
per row (parsed from the manifest at discovery time). The wizard
uses it to:
- Per-row badge: red "No DB" pill next to any backup where
database_included === false. Tooltip explains the consequence
in plain English: "restoring this will NOT recover the database".
- Selected-card callout: full red banner under the chosen row
when database_included is false, restating the warning + giving
the admin a clear path: "pick a different backup if you have
one with a database dump, or proceed only if files-only is
what you want."
The wizard does NOT block the restore — the admin may genuinely want
a files-only restore (e.g. recovering a deleted photo while keeping
current DB state). The warnings make sure that choice is informed.
|
||
|
Luca
|
155aa63103 |
fix(backup-dashboard): show last SUCCESSFUL backup + last attempt separately
The dashboard widget used `lastBackup.created_at` for the "Last
successful backup: X ago" text — but lastBackup is the most recent
row of any status. So a crashed restore (status=running, never
updated) or a recent failure showed up labeled as the last
successful backup. Same "silent failure not surfaced" class the
restore wizard had.
Backend now returns:
lastSuccessfulBackup — most recent backup_runs with status='completed'
zombieRuns — running rows older than 30 min (likely crashed mid-flight)
lastBackup — unchanged (most recent any status)
Frontend renders:
- "Last successful backup: X ago" — always from lastSuccessfulBackup
- "Last attempt: Y ago · failed/running" — when lastBackup differs
from lastSuccessful. failed shows the first line of error_message
in red; running stays neutral.
- Zombie callout — "N backup(s) running >30min — may have crashed"
in amber, so admin sees stuck rows at a glance.
- Health score downgrades from "excellent" to "warning" if the
latest attempt failed, even when older successes keep the age
fresh — surfaces regressions without erasing the green history.
|
||
|
Luca
|
48e9c9c79a |
fix(restore): re-init knex pool after DROP/CREATE DATABASE
`db.destroy()` during restore tore down the in-process connection
pool to release PG sessions so DROP DATABASE could succeed. After
CREATE DATABASE + psql restore, the old code did
`require('../database/db')` expecting a fresh instance — but Node
caches require results, so it got the SAME destroyed instance back.
Every subsequent query in the process failed with "Unable to acquire
a connection" until the container was manually restarted, even
though the restore technically succeeded.
Net effect for admins: login showed "An error occurred", customer /
invoice / quote pages were blank, no surface hinted at the dead pool.
Cure: db.js now wraps the live knex instance in a Proxy that forwards
to a mutable internal reference, with a `reinitPool()` function that
destroys the old instance + builds a fresh one + probes with `SELECT 1`
so any reconnect failure surfaces immediately. The thousands of
existing `const { db } = require(...)` imports work unchanged — they
capture the Proxy once, and every call goes through to the current pool.
restoreService calls reinitPool() after CREATE DATABASE and before
migrate.latest(), so the rest of the request + every subsequent admin
action runs against the fresh pool. Container restart no longer
needed after restore.
|
||
|
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
|
||
|
Luca
|
c435263744 |
fix(restore): default restore_allow_force=true + auto-upgrade existing installs
Root cause of the persistent "Force restore is not allowed by system
settings" error even on fresh installs after `docker compose down -v`:
migrations/core/032_add_restore_runs_table.js seeded the row with
`JSON.stringify(false)` = the literal string 'false'.
So every install (fresh OR upgraded) wrote restore_allow_force=false
at migration time. The boot self-heal added earlier today saw the row
and respected "admin policy" per its safety design — never noticing
that the row was the deprecated migration default, not an explicit
admin choice.
Cure follows [[feedback_migration_no_compensation]] +
[[feedback_self_heal_pattern]]:
1. Edit migration 032 IN PLACE — flip seed value from false to
true. Fresh installs forward get the correct default at install
time, no boot helper needed.
2. One-time auto-upgrade in _restoreSettingsBoot.js for installs
that already ran the OLD migration. Bumps restore_allow_force
to 'true' iff the current value is the deprecated literal
'false' AND the new tracking key
`restore_allow_force_auto_upgraded` doesn't yet exist. The
tracking flag is always written after the first boot pass, so
subsequent admin choices (e.g. deliberately disabling force)
are preserved on every boot after.
3. Defensive: adminRestore.js getRestoreSettings() now normalizes
'true'/'false'/'"true"'/'"false"' string shapes to JS booleans,
not just '1'/'0'. Belt-and-suspenders so any future seeder that
uses a different boolean serialization doesn't silently break
the !settings.restore_allow_force gate.
Net effect: any picpeak install pulling this image — fresh or
existing — gets restore_allow_force=true on first boot after the
upgrade. The catch-22 that forced every disaster-recovery admin to
hand-write SQL before their FIRST restore is closed.
|
||
|
Luca
|
dbcecfe2aa |
feat(restore): self-heal restore_allow_force default ON at boot
Fresh installs of picpeak had `restore_allow_force` defaulting to false (or missing entirely). Combined with the "1 active admin user" pre-restore warning that the fresh-install admin auto-creates, this meant the very first restore on every new install hit: Force restore is not allowed by system settings Admins then had to hand-craft SQL to flip the setting before they could recover their data — at the worst possible moment, when they were already mid-disaster. This isn't security: the admin who can SQL the setting on can also flip it via the UI. It's just a sharp edge that bites every new install once. Cure: boot-time self-heal that seeds restore_allow_force=true only when the row doesn't exist. Existing installs that explicitly set the row (true OR false) are NOT touched — admin policy wins. Pattern mirrors _backupPathsBoot.js and _emailTemplateBoot.js. Default-ON rationale matches Stage A's principle: the cost of forgetting (= can't recover from a disaster) outweighs the friction saved (= adversarial admins can't run forced restores). Audit logging keeps the accountability story intact. |
||
|
Luca
|
cfaa7eb095 |
fix(restore): re-sync PostgreSQL sequences after psql load
pg_dump emits setval() statements for SERIAL/IDENTITY columns, but
they don't always land cleanly: --clean ordering, knex pool sequence
caching, rows inserted mid-restore (the pre-restore safety backup
writes a database_backup_runs row before DROP), etc. Net result on
Ralf's install after a successful restore:
- "A record with this value already exists" on every CRUD action
- duplicate key value violates unique constraint
"database_backup_runs_pkey" on the next Run Backup Now
Same root cause: every SERIAL column's sequence was pointing at or
below MAX(id), so the next INSERT collided.
Fix: append a DO block after the psql restore that walks pg_class +
pg_attribute and setval()s every public-schema sequence to
GREATEST(MAX(<col>), 1). Cheap (a few ms even on large schemas),
safe (read-only on row data), idempotent — re-running it just
re-asserts the same values.
Seventh latent PG-restore bug discovered on Ralf's install tonight.
Manual hand-fix worked; this commit makes the fix automatic for
every future restore.
|
||
|
Luca
|
a39def672e |
fix(restore): evict active sessions before dropping target DB
PostgreSQL refuses DROP DATABASE while any session is connected:
ERROR: database "picpeak_prod" is being accessed by other users
DETAIL: There are 6 other sessions using the database.
The backend's own knex pool holds 5-25 active connections to the
target DB. So even after closing the request that initiated the
restore, the pool keeps the DB busy and the DROP statement fails.
Three-layered cure, all in the restore service's PG branch:
1. Call `db.destroy()` first to close the in-process knex pool so
we don't fight ourselves. Knex will lazily re-open on the next
query via db.js's retry logic, so this is safe to do mid-restore.
2. SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE
datname=<target> AND pid<>pg_backend_pid() — evicts any sessions
from other processes (other server replicas, leftover idle
transactions, things our own pool destroy missed).
3. DROP DATABASE IF EXISTS "<target>" WITH (FORCE) — PG13+ kills
remaining connections atomically with the DROP. Falls back to
plain DROP on older Postgres where WITH (FORCE) is a syntax error.
Surfaced as the FIFTH latent bug in the restore path tonight: the
DROP DATABASE statement always assumed a quiescent destination, but
the live backend keeps the destination busy at all times. Every
previous PG install of picpeak that ever tried Restore would have
hit this — meaning the disaster-recovery feature has shipped broken
for a long time without anyone exercising it end-to-end.
|
||
|
Luca
|
4c31a22626 |
fix(restore): DROP/CREATE DATABASE needs explicit -d maintenance DB
`psql` with no -d connects to a database whose name matches the connecting user. On installs where the user's home DB doesn't exist (common pattern: DB_USER=picpeak, DB_NAME=picpeak_prod, no `picpeak` DB), the restore's DROP DATABASE / CREATE DATABASE statements failed with: FATAL: database "picpeak" does not exist even though the target DB (picpeak_prod) was alive and connectable. And of course you can't connect to the target DB itself for DROP — PostgreSQL refuses while a connection is open to it. Fix: explicitly connect to `postgres` (the maintenance DB every PG cluster ships with) for the DROP/CREATE statements. Override via DB_CHECK_DB env var if the `postgres` DB is restricted to superusers on the cluster — matches the pattern wait-for-db.sh already exposes. Also quote the database name in the SQL so installs whose DB has unusual characters (numbers, hyphens) don't break the statement. Surfaced during Ralf's end-to-end restore validation — yet another "never been tested on a real PG install" latent bug exposed by the Stage A inline-dump path actually being able to produce a restorable manifest for the first time on his install. |
||
|
Luca
|
5c0be66a14 |
fix(restore): resolve local source + always rollback on failure
Two changes that close the disaster-recovery loop the Stage A-B-C backup-hardening plan opened: 1. Resolve 'local' source to backup_destination_path The wizard passes options.source = 'local' (the SOURCE TYPE string). The old code assigned that verbatim to localBackupPath and every downstream path.join() ended up with junk like 'local/database/<file>.sql.gz'. Fixed by looking up backup_destination_path from app_settings when source='local', plus a layered candidate fallback in performDatabaseRestore so absolute paths in manifests are honoured first. 2. Auto-rollback on ANY failure during restore Previously rollback only fired when post-restore VERIFICATION failed (inside the try block). Anything that threw earlier — path bugs, pg_restore failure, file copy errors — left the destination half-clobbered with no automatic recovery. Now the catch block always invokes attemptRollback if a pre-restore backup exists, and persists rollback status in was_rollback_attempted + an enriched error_message so the admin can tell at a glance whether the destination is safe to retry on top of or needs manual inspection first. Surfaced during Ralf's validation of the end-to-end backup + restore cycle (`docker compose down -v` then restore from disk). Every prior failed attempt left stray PDFs behind that the next attempt had to navigate around — exactly the "every failure makes the next worse" pattern this fix kills. |
||
|
Luca
|
44c7935b84 |
fix(restore): resolve 'local' source to backup_destination_path
Two stacked bugs in the disaster-recovery path:
1. The wizard passes `options.source = 'local'` (the source TYPE
string) and the service assigned it verbatim to `localBackupPath`.
Every downstream `path.join(localBackupPath, ...)` ended up with
junk like `local/database/<file>.sql.gz` and `local/events/...`.
2. performDatabaseRestore reconstructed the dump path from the
manifest by basename-only:
path.join(backupPath, 'database', path.basename(dbBackupFile))
discarding the absolute path the manifest actually recorded.
Cure:
- At the entry point, if `options.source === 'local'`, look up
`backup_destination_path` from app_settings and use that as the
local root. Honour s3:// downloads via the existing branch.
- In performDatabaseRestore, try the manifest's absolute path
first, then `localRoot + manifest_value`, then the legacy
`localRoot + 'database' + basename` reconstruct as a final
fallback. First hit wins; error message lists every candidate
so future failures are diagnosable.
Surfaced during Ralf's end-to-end validation of the Stage A-B-C
backup-hardening plan — restored fresh after `down -v`, the wizard
failed silently with `Database backup file not found: local/database/...`
even though the dump existed at the path the manifest recorded.
With this fix, the same destruction-and-recovery sequence completes.
|
||
|
Luca
|
f664fea60c |
fix(restore): discover backups from disk, not just the DB
The Restore wizard's "Choose Backup to Restore" list was driven only
by the backup_runs table. After `docker compose down -v` (the disaster
this whole hardening effort is designed to recover from), the DB is
empty and the wizard shows "No backups found in selected source" —
exactly when it's needed most. The manifest JSONs are still on disk;
the wizard just can't see them.
Adds disk-first discovery:
- Walks backup_destination_path AND backup_manifest_path (manifests
can live in a sibling directory under the canonical
<root>/manifests/backup-manifest-<id>.json layout). Depth-limited
recursion (3 levels) so the scan doesn't enumerate the photo tree.
- Matches backup-manifest-*.json|yaml AND legacy bare manifest.json.
- Parses each manifest for real metadata (timestamp, size, file
count, database.backup_file presence) instead of showing the
admin opaque filenames.
- Layers in surviving backup_runs rows, deduping by manifest_id.
Applied to both GET /available-backups (legacy) and POST /list-backups
(the one the frontend actually calls). Same helper, two call sites.
Side benefit: each returned row now carries `databaseIncluded` — so a
future Restore UI iteration can show a "this backup has no DB dump"
warning before the admin picks a files-only backup. Exactly the
surface that would have caught Ralf's original four files-only
manifests if it had existed.
|
||
|
Luca
|
0ad14899fa |
ix(database-backup): drop bogus --single-transaction flag from pg_dump
pg_dump rejects `--single-transaction` — it's a pg_restore / psql flag, never a pg_dump one. Triggered as soon as the inline-dump path landed on Ralf's install: pg_dump: unrecognized option: single-transaction pg_dump: hint: Try "pg_dump --help" for more information. pg_dump already wraps the entire export in a single REPEATABLE READ snapshot automatically (since Postgres 9.x), so the original intent — consistent snapshot of the live DB — is preserved by removing the flag. Same "latent until Stage A wired it in" pattern as the three prior bugs this rollout has surfaced (PG insert destructure → bind- mount EACCES → Node 22 stdio strict mode → this). |
||
|
Luca
|
d34036c4ef |
fix(safe-exec): Node 22-compatible stdio + error-bridge for spawnTo/FromFile
spawnToFile and spawnFromFile passed an unopened WriteStream/ReadStream
directly as a stdio entry to child_process.spawn. Older Node versions
auto-extracted .fd; Node 22 throws synchronously:
The argument 'stdio' is invalid.
Received WriteStream { fd: null, path: '/backup/database/...sql', ... }
Bug bit Ralf's install once today's `bugfix/crm-backup` image landed —
Node 22 came with that image, and Stage A's inline-dump path is the
first caller of spawnToFile on this install. Latent on the previous
image (Node 20); fatal on this one. restoreService's pre-restore
safety snapshot uses the same helper and would have hit it next time
a restore ran.
Cure: stdio: ['ignore', 'pipe', 'pipe'] (and ['pipe', 'pipe', 'pipe']
for spawnFromFile) + manual pipe of child.stdout/stdin through the
file stream. Works on every Node version. Also wires the WriteStream's
'error' event to the promise via settleReject so a future EACCES /
ENOSPC reaches the caller's try/catch instead of becoming a process-
fatal unhandled error event — closing the same "Stage A guard
bypassed" hole noted in the spawned follow-up task.
Side benefit: outStream.end() now awaits flush before resolving, so
fast pg_dump runs can no longer produce a truncated dump.
|
||
|
Luca
|
f741e88acb |
fix(database-backup): Postgres-safe insert destructure (runs the inline dump)
databaseBackupService.backup() did `const [runId] = await db(...).insert({...})`
without a .returning() — works on SQLite (knex returns [lastInsertId]) but
throws "(intermediate value) is not iterable" on Postgres (knex returns
a non-iterable shape).
Bug was latent until Stage A of the backup-hardening plan wired this
method into the "Run Backup Now" inline-dump path. Before Stage A only
the scheduled cron + the dedicated admin-DB-backup page called it, and
Ralf's install had never exercised either — so the inline-dump default
landing in production was the first time the destructure ran on his PG.
Cure: same explicit .returning('id') + dual-shape coalesce pattern that
backupService.js uses for its own backup_runs insert (line 949).
Two more sibling files have the same anti-pattern (userManagementService,
customerAccountsService — invitation flows) and will bite under the
same conditions; spawned a follow-up task to fix them in a separate PR.
|
||
|
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-... . |
||
|
Luca
|
03e6617f38 |
feat(backup): coverage diagnostic — what will the next backup miss?
Stage C of the three-stage backup-hardening plan (Stage A: inline
DB dump + fail-loud landed in 7fdf01a; Stage B: config-driven walker
in
|
||
|
Luca
|
302fc6b937 |
feat(backup): config-driven walker via backup_paths table
Stage B of the three-stage backup-hardening plan (Stage A:
inline-DB-dump + fail-loud guard already landed). The file-backup
walker used to hard-code its subdirectory list inside
`getFilesToBackupInternal`, which is the same footgun that hid the
`business-docs` gap for ~6 months — a new feature drops artefacts
under STORAGE_PATH and the maintainer has to remember to edit the
walker.
Now driven by a `backup_paths` table:
- Migration 108 creates the table and seeds the 7 canonical
defaults (events/active, events/archived, thumbnails, previews,
heroes, uploads, business-docs). Seed data lives on the
migration as `DEFAULT_PATHS` so the boot self-heal can re-use it.
- `_backupPathsBoot.js` mirrors `_emailTemplateBoot.js`: on every
boot it diffs the canonical list against the current rows and
`INSERT ... ON CONFLICT DO NOTHING`s the missing ones. Keeps
admin edits intact, picks up new defaults shipped after the
install (Knex won't re-run migration 108). Wired into server.js
just before `startBackupService()`.
- Walker now calls `resolveBackupPaths(config)` which:
* reads `backup_paths WHERE include_in_default=true ORDER BY
display_order`
* falls back to a hard-coded `LEGACY_BACKUP_PATHS` if the
table is missing OR empty (defense in depth — never silently
scans nothing)
* gates each row by its `feature_flag` column (matches how
`backup_include_archived` already worked; data-driven now)
- Backward compatible: `getFilesToBackup(true|false)` still works
for legacy callers and the existing businessDocs test. New
callers should pass the full config object so feature gates
other than `backup_include_archived` evaluate correctly.
Tests:
- new: `backupService.configurableWalker.test.js` — 7 cases
covering canonical seed, toggling include_in_default, runtime
INSERT picked up without restart, feature_flag gating both on
and off, empty-table → LEGACY fallback, boolean backward compat
- all 15 backup-walker integration tests pass
(configurableWalker 7 + inlineDbDump 5 + businessDocs 3)
- frontend build clean
- 4 pre-existing integration failures (webhookDelivery, storage
backend, adminPhotos.reference, imageProcessor.storage) confirmed
unrelated via `git stash` baseline run
Stage C (CRM feature coverage audit + diagnostic UI) follows
in a separate commit.
|
||
|
Paul Nothaft
|
433af15146 |
Merge pull request #582 from the-luap/feat/slovenian-locale-580
feat(i18n): add Slovenian (sl) language support |
||
|
Luca
|
7fdf01ad21 |
fix(backup): inline DB dump + fail-loud guard so "Run Backup Now" can't ship files-only
The previous file-backup workflow only LOOKED UP an existing
database dump via getDatabaseBackupInfo() and silently shipped a
files-only manifest when none was found. Admins clicking "Run
Backup Now" (or relying on the schedule) got an apparent success
that omitted every customer / quote / invoice / contract / payment-
log row. The data-loss footgun was discovered 2026-05-29 when an
admin who'd been "backing up" for weeks via the UI lost the entire
CRM after a routine docker compose down -v — every produced
manifest had database: { backup_file: null, size: 0, tables: {} }.
New helper `ensureDatabaseDumpForBackup(config)` encapsulates:
1. Inline pg_dump (or SQLite copy) before the file scan, via
databaseBackupService.backup(). Result lands in
database_backup_runs and is picked up by the existing
getDatabaseBackupInfo lookup that writes the manifest.
2. Fail-loud guard: if no usable dump file is reachable (path
missing, 0 bytes, or never existed), throw — the existing
catch in runBackupInternal marks the backup_runs row failed
with the error_message and emails the admin if configured.
No more silent files-only manifests.
3. Opt-out: `backup_database_inline_dump = false` skips the
inline dump for admins who already run their own scheduled
`backup_database_schedule`. The fail-loud guard still
applies, so an opted-out install with no recent dump still
aborts loudly instead of producing a partial backup. Default
ON is encoded as "skip only when explicitly false" — undefined
(existing installs upgrading) falls through to the safe-
default ON branch.
The helper returns the verified `databaseInfo` so the manifest-build
step at runBackupInternal:917 reuses it instead of calling
getDatabaseBackupInfo a second time. S3/future destinations that
override `result.databaseInfo` are still respected (the existing
`result.databaseInfo ||` fallback shape stays put).
Test suite covers: default-on happy path, dump-throws-aborts-run,
opt-out + recent dump + proceeds, opt-out + no-dump + fail-loud,
opt-out + 0-byte dump + fail-loud. Mocks
databaseBackupService.backup so the tests don't depend on pg_dump
or sqlite3 CLI binaries being installed.
Stage A of three-stage backup hardening plan. Stage B (config-
driven walker) and Stage C (audit + diagnostic UI) follow in
separate commits.
|
||
|
Luca
|
7c230bdc24 |
fix(backup): inline DB dump + fail-loud guard so "Run Backup Now" can't ship files-only
The previous file-backup workflow only LOOKED UP an existing database
dump via getDatabaseBackupInfo() and silently shipped a files-only
manifest when none was found. Admins clicking "Run Backup Now" (or
relying on the schedule) got an apparent success that omitted every
customer / quote / invoice / contract / payment-log row. The
data-loss footgun was discovered 2026-05-29 when an admin who'd been
"backing up" for weeks via the UI lost the entire CRM after a routine
docker compose down -v — every produced manifest had database:
{ backup_file: null, size: 0, tables: {} }.
Changes to runBackupInternal:
1. Inline pg_dump (or SQLite copy) before the file scan, via
databaseBackupService.backup(). Result lands in
database_backup_runs and is picked up by the existing
getDatabaseBackupInfo lookup that writes the manifest.
2. Fail-loud guard after the dump step: if no usable dump file is
reachable (path missing, 0 bytes, or never existed), throw —
the existing catch block marks the backup_runs row failed with
the error_message and emails the admin if configured. No more
silent files-only manifests.
3. Opt-out: `backup_database_inline_dump = false` skips the inline
dump for admins who already run their own scheduled
`backup_database_schedule`. The fail-loud guard still applies,
so an opted-out install with no recent dump still aborts loudly
instead of producing a partial backup. Default ON is encoded
as "skip only when explicitly false" — undefined (existing
installs upgrading) falls through to the safe-default ON path.
Test suite covers: default-on happy path, dump-throws-aborts-run,
opt-out + recent dump + proceeds, opt-out + no-dump + fail-loud,
opt-out + 0-byte dump + fail-loud. Mocks
databaseBackupService.backup so the tests don't depend on pg_dump
or sqlite3 CLI binaries being installed.
Stage A of three-stage backup hardening plan. Stage B (config-driven
walker) and Stage C (audit + diagnostic UI) follow in separate
commits.
|
||
|
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. |
||
|
Luca
|
3f5d006625 |
fix(test-infra): scope databaseBackup fs.unlink stub so it doesn't leak
Line 205 of databaseBackup.test.js reassigned `fs.unlink` directly (`fs.unlink = jest.fn(...)`), which permanently mutated the global fs.promises module. Every test running after this in the same jest worker process inherited the no-op stub, including integration/storageBackend.test.js — whose LocalFsStorage.delete() silently became a no-op, making the subsequent exists() assertion flip from false to true. Confirmed by adding a diagnostic patch to LocalFsStorage.delete: post-await fsp.unlink, fs.existsSync(abs) returned true. unlink had resolved without throwing but the file was still there → the unlink was a mock. Fix: jest.spyOn(fs, 'unlink').mockResolvedValue(undefined) + a matching mockRestore() at the end of the test. Behaviour is identical inside this test; the original fs.unlink is restored when the test finishes, so subsequent tests get real fs.unlink again. Pre-existing issue — has been latent on upstream/beta forever. Only surfaces consistently when CI load shifts jest's worker allocation such that databaseBackup and storageBackend land in the same worker process. This PR's extra integration test files made that allocation deterministic locally and frequent enough on CI to fail reliably. |
||
|
Luca
|
ecb2aeacf9 |
fix(test-infra): unref sessionTimeout cleanup interval so workers exit gracefully
The 5-minute session-sweep interval at sessionTimeout.js:17 fired at module-load time without .unref(), so every jest worker that transitively required this module (server.js → middleware → most of the route layer) kept the event loop alive forever. The worker then got force-killed on shutdown, surfacing as the longstanding "worker failed to exit gracefully" warning at the end of every CI run on upstream/beta. Under enough I/O / memory pressure on a CI runner, the force-kill could land MID-test rather than after the suite finished, taking out whatever else was running on that worker — most visibly integration/storageBackend.test.js on PR #555's runs. .unref() makes the timer not keep the loop alive on its own. Production behaviour is unchanged: the timer still fires every 5 min as long as anything else is holding the loop open (the HTTP server, always). |
||
|
Luca
|
4812fcdec3 |
feat(backup): admin endpoint to verify CRM document-artefact integrity
Diagnostic for the bug fixed in
|
||
|
Luca
|
a9280ea9ba |
fix(backup): include storage/business-docs/ in the in-app backup walker
backupService.getFilesToBackupInternal() enumerated a fixed list of
storage subdirectories (events/active, events/archived, thumbnails,
previews, heroes, uploads) and silently omitted the entire
business-docs/ tree. Every CRM PDF artefact and signature image fell
outside the in-app scheduled backup — restoring the DB without the
PDFs would have left every *_path column on quotes/contracts/invoices
as a broken FK and lost forensic evidence (the customer signature
PNG/JPG drawn on the public signing page is referenced by
contracts.signed_customer_signature_path; the rendered contract PDF
is referenced by signed_pdf_path with a stored signed_pdf_sha256
that would have nothing to verify against; wet-uploaded contracts
and admin-imported historical invoices are irrecoverable by design
since no renderer can reproduce them).
Single new scanDirectory call after the existing uploads scan,
covering:
- business-docs/quote/<year>/*.pdf
- business-docs/contract/<year>/*.pdf
- business-docs/contract/signatures/<contract_id>/*.{png,jpg}
- business-docs/invoice/<year>/*.pdf
- business-docs/invoice-imports/<year>/*.pdf
- and incidentally business-docs/dev-test/ (managed by adminDev.js,
bounded to 7 newest files, harmless to back up)
Verified that no migration is needed: hasFileChanged returns
!existing || checksum mismatch, so the first backup after this lands
flags every business-docs/** file as new and copies it. Restore path
in restoreService.performFilesRestore uses fs.mkdir({ recursive:
true }) on path.dirname(targetPath), so business-docs subdirectories
are recreated automatically from manifest entries — no restore-side
code change required.
Integration test pins the contract so a future refactor cannot
silently drop business-docs again.
The shell-script backup at scripts/backup.sh already covered all of
this via blanket `tar -czf storage`; only the in-app service was
affected.
|
||
|
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
|
48cf1121e5 |
Merge pull request #575 from the-luap/feat/clickable-version-links-566
feat(admin): clickable version links + update-available modal with changelog & upgrade command |
||
|
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. |
||
|
Luca
|
d1aecaa180 |
fix(crm): thread trx through sequence-claim sites to unblock SQLite
Reviewer feedback on #555: nextQuoteNumber inside createQuote's db.transaction was called without passing the outer trx, so claimNextSequence opened its own connection — Postgres tolerated this via the pool, SQLite (1-connection default) deadlocked on every quote creation. Audited the same pattern across invoiceService + contractService and found five more matching call sites: - createInvoice (single-row path after installment auto-route) - spawnInstallmentInvoices (per-sibling claim inside the loop) - createStorno - createContract - createFromQuote All now thread trx through to nextXxxNumber → claimNextSequence so the claim joins the caller's transaction on both engines. convertToInvoiceOnly's Path B (standalone-contract) is the lone remaining nextInvoiceNumber() call without trx — that path isn't wrapped in a transaction at all (separate concern: sequence-number leak on insert failure, tracked separately). |
||
|
Luca
|
83933baeec |
fix(crm): self-heal missing CRM email templates at boot + recover queue
The CRM template seeders (crmEmailTemplates / contractEmailTemplates / eventReminderTemplates) were idempotent and ready, but only contractEmailTemplates was actually called (lazily, by contractService sends). crmEmailTemplates had no caller anywhere — every install that didn't pre-exist its templates failed every quote_sent / invoice_sent / storno_issued / invoice_reminder_* send with "Email template '<key>' not found". The queue processor retries 3 times then leaves the row in status='pending', retry_count=3, silently dead with no admin surface (see project_crm_backlog for the eventual System Health page). Fix: wire all three seeders into server.js startServer() right before startEmailQueueProcessor. The new _emailTemplateBoot.js orchestrates all three and then, for any template_key it just inserted, resets retry_count on stuck email_queue rows of that email_type so the queue processor's next tick picks them back up. Recovery is targeted: unrelated retry-exhausted rows (e.g. SMTP-timeout failures) are not touched. Integration test boots a fresh CRM DB, pre-seeds a stuck quote_sent row plus an unrelated stuck row, runs the boot helper, and asserts: templates landed, stuck quote_sent row was reset, unrelated row was left alone. Already-deployed installs heal automatically on the next backend restart after this lands. |
||
|
Luca
|
09c5110d2b |
feat(crm): route billing docs to billing_email when set
Wires customer_accounts.billing_email into the invoice, Storno, and
payment-reminder send paths. Previously the column existed on the
schema and the customer-detail page rendered an input for it, but no
send path read it — every outbound email landed on customer_accounts.email
regardless. That mismatch is the failure mode flagged in
feedback_data_driven_completeness: a UI field that promises behavior
the backend silently doesn't deliver.
Routing matrix:
- invoice / Storno / payment reminder
To: billing_email (fallback email when unset)
CC: email (when billing_email took the To slot) + per-doc cc_pdf_email
- quote / contract / event reminder / gallery share
To: email (unchanged — decision-maker address)
- payment-check / paid-notification
To: admin contact (unchanged — internal flow)
A new resolveBillingRecipients helper centralises the rules:
prefer billing_email, dedupe addresses case-insensitively, keep
per-doc cc_pdf_email as a supplemental CC. Lives in its own file
(_billingRecipients.js) to match the _renderContext.js convention.
|
||
|
Luca
|
3d37324080 |
feat(crm): allow negative line items for manual discount/Rabatt rows
Drops the isInt({ min: 0 }) constraint on lineItems.*.unitPriceMinor
in both the adminInvoices and adminQuotes POST/PUT validators so
admins can add Treuerabatt / Frühbucherrabatt rows as standalone
negative-priced lines (matches standard DE/CH invoice practice).
A service-layer guard rejects saves whose computed total goes below
zero (INVOICE_TOTAL_NEGATIVE / QUOTE_TOTAL_NEGATIVE, both 400) so a
mis-typed discount can't accidentally mint a credit-balance invoice
that would masquerade as a regular row in dashboards. Credit notes
still belong in the Storno path (createStorno), which is unchanged.
Quote-side integration coverage is omitted for now — createQuote's
cold-require path takes ~30s under the test harness; the invoice
test exercises the same validator + guard shape.
|
||
|
Luca
|
6d302e7998 |
feat(crm): add event_reminder_* templates to dev email tester
The pre-event reminder feature shipped with 5 seeded templates
(event_reminder_default + wedding/birthday/corporate/other) but the
CRM → Development "Send any CRM email to me" picker only listed the
quote/invoice/contract templates. Maintainer can now eyeball each
reminder category's body without staging a real event.
Backend:
- Extend TEMPLATES_KEYS in adminDev.js with all 5 reminder keys.
- Add event_date (today+2d), days_before (2), business_name (from
business_profile.legal_name) to the common payload so the
{{tokens}} in the reminder bodies resolve.
Frontend:
- Extend CrmEmailTemplateKey union.
- Add TEMPLATE_LABEL_KEYS entries.
- EN+DE i18n labels under crmDev.templates.label.event_reminder_*.
No PDF attachment — reminders are body-only emails (matches the
real flow).
|
||
|
Luca
|
b9cadf002c |
test(crm): update mocks for new createInvitation + OG date-format behavior
Two upstream tests regressed because the CRM PR added expected behavior
they didn't anticipate:
- galleryOgService.shareImage.test.js: formatEventDate is now async and
routes through utils/dateFormatter so the OG card respects the admin's
general_date_format setting (per feedback_respect_general_format_settings).
That adds a third db('app_settings') call on every buildOgMetadata path.
Mock the formatter module directly — the format itself is irrelevant
to the cover-vs-logo contract this file pins.
- customerAccountsService.test.js: createInvitation now allows a duplicate
email when the existing row is PASSIVE (password_hash IS NULL) — that's
the "promote passive customer to portal" path. The active-customer
rejection mock now has to set password_hash so the guard fires.
Both are test-only changes; no service code touched.
|
||
|
Luca
|
d543949188 |
feat(crm): backend code — services + routes + utilities + tests
Brings in the full backend CRM stack on top of the consolidated
migration (
|
||
|
Luca
|
60abe8c76d |
chore(migrations): consolidate CRM migrations 102-143 + extract email-template seeds to self-heal services
Replaces what would have been 42 individual in-flight migrations
(102→143 on feat/crm) with one consolidated migration that creates
every CRM table in its final shape — no ALTER chains. Coexists with
upstream's pre-existing 102-106 by filename suffix; the runner sorts
within same-number groups.
Tables consolidated:
- business_profile + business_bank_accounts (issuer block, fonts,
PDF layout knobs, tax_id, timezone)
- payment_term_templates (legacy) + payment_net_days_templates +
payment_timing_templates (124's split)
- quotes / quote_line_items / quote_line_item_presets / quote_action_tokens
- invoices / invoice_line_items / invoice_payment_log /
invoice_payment_check_tokens
- contracts / contract_blocks (13 system blocks seeded) /
contract_block_inclusions / contract_action_tokens
- event_payment_plans, customer_hour_entries, document_sequences
ALTER on upstream tables (hasColumn-guarded):
- events: quote_id, calendar columns (event_time_*, is_full_day),
event_reminder_*
- customer_accounts: billing_cadence/cycle_day, country_name,
feature_hours_logging, hourly_rate_minor
Seeds:
- RBAC perms (quotes/bills/contracts .view/.manage) + customers.create
split into edit + events (mig 134)
- Feature flags (quotes, bills, contracts, hoursLogging, taxReport,
calendar, calendarBooking, reminderEmails, crmDevelopment, messaging
— all default OFF)
- 30+ CRM app_settings rows (skonto/QR/reminder windows, payment
defaults, installment defaults, ToS, event reminder defaults)
- 4 + 5 + 4 payment-term system rows across the legacy + split tables
Email-template content moves out of the schema diff into three
runtime self-heal service files that idempotently create missing
rows + backfill empty translations on first access (per the maintainer's
"never ship compensation migrations" rule):
- backend/src/services/crmEmailTemplates.js (NEW) — quote_sent,
quote_accepted_*, quote_declined_admin, invoice_sent,
invoice_reminder_first/second, invoice_paid_receipt,
invoice_cancelled, invoice_payment_check,
invoice_paid_admin_notification, storno_issued
- backend/src/services/contractEmailTemplates.js — contract_sent,
contract_fully_signed, contract_signed_admin_notification
- backend/src/services/eventReminderTemplates.js — event_reminder_default
+ per-event-type variants
Smoke-tested on fresh sqlite DB: 84 migrations apply cleanly,
all CRM tables present, seeds populated.
|
||
|
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
|
8b72721812 | fix(public-site): honor dark theme surface colors | ||
|
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
|
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 |