03cc250b47518573d2cd4fa45da331a3e47ea3fd
51 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Luca
|
03cc250b47 |
feat(accounting): Layer A backend — chart of accounts, VAT codes, Treuhänder export
Prepares picpeak to feed a Treuhänder's double-entry software once a user crosses the CHF ~500k threshold (LI PGR Art. 1045), without becoming an ERP. - migration 129: ledger_accounts (seeded Swiss/LI KMU-Kontenrahmen) + vat_codes (CH/LI MWST 8.1/2.6/3.8/0 + reverse charge), expense_categories gains ledger_account_id, app_settings default-account + VAT-map seeds - ledgerService: full CRUD for accounts + VAT codes + mappings; buildPostings() turns revenue invoices + incoming invoices + expenses into accrual Buchungssätze (Dr/Cr + VAT code); generic/banana/bexio CSV export - routes /api/admin/ledger/* (accounting master gated; export also requires taxReport); 12 unit tests (posting engine + formatters) Accrual basis only — payment/bank postings are Layer B. Output is a guideline (Treuhänder caveat on the UI). |
||
|
Luca
|
545ef334f4 |
feat(accounting): tax window shows all costs (incoming invoices + expenses) alongside revenue
Einnahmen-Ausgaben view for the Milchbüchlein/simple-accounting case: - taxReportService.getTaxReport now returns a cost side (loadCosts: incoming invoices + internal expenses, company- or event-booked, schema-guarded) plus a summary (income / costs / result, VAT payable) - declined/duplicate costs excluded; re-billed costs kept (matching re-bill revenue is counted, so the net is correct) - CSV + PDF exports gain a Costs section and an income/costs/result summary; pdf-i18n keys added for all 6 locales (fr/nl/pt/ru machine — flag for native review) - frontend tax page renders the summary card, a costs table (company vs event), and a 'verify with Treuhänder' disclaimer - tax-report tests cover the cost aggregation + zeroed summary when the accounting tables are absent; adminCrmAuth test enables the accounting master flag the route now requires fr/nl/pt/ru strings are machine-generated and need native review. |
||
|
Luca
|
5e78fb6475 |
feat(accounting): backend rework - incoming invoices vs internal expenses (stage 2)
Implements the split decided in review: Incoming invoices (external) - the inbound_documents row IS the payable: - categorizeInbound now UPDATES the document (disposition + tax_treatment + booking event_id (null=company) + category), no derived expense row, so a supplier invoice appears only in the incoming-invoices surface. - rebillInbound mints the client invoice from the document (base = invoice total + markup) and links it on the doc. - markInboundSupplierPayment records supplier payment ON the incoming invoice (mark-paid lives here now). Expenses (internal) - own costs only: - createExpense: kind = amount|mileage|per_diem; amount = quantity x rate (rate from accounting settings, per-entry override; snapshotted); optional proof file; booked to an event or the company; require-proof enforced from settings. No supplier payment, always own-cost. - listExpenses returns internal rows only (inbound_document_id IS NULL). Routes: per-flag gating (incomingInvoices vs expenses; categories on the accounting master); supplier-payment + re-bill moved under /inbound/:id/*; POST/PATCH expenses accept a multipart proof upload; GET /:id/proof streams it (PDF download-only, image inline). getAccountingSettings reads app_settings. Verified: node -c, require-graph, 12 unit tests (markup + expense amount/build). Frontend rework (service + the two UIs + settings tab + category i18n) follows. |
||
|
Luca
|
413a200592 |
test(accounting): unit tests for expense markup / disposition logic
Covers the silently-regressable money + classification bits of the re-bill flow (the maintainer's "thin CRM test coverage" concern). Pure functions via a new expenseService._internal export — no DB, no date-harness pitfalls: - computeMarkupMinor: percent rounding, flat, none/null. - resolveMarkup precedence: override > expense clause > none. - buildExpenseInsert: bad-disposition guard, tax_treatment/status defaults, declined -> status+reason, markup field matches type, parked -> status. 11 tests, all green (npx jest expenseService.markup). |
||
|
Luca
|
333379b321 | Merge branch 'beta' of https://github.com/the-luap/picpeak into feat/crm-improvements | ||
|
Luca
|
621ce942b5 |
feat(email): per-weekday business hours + manual queue flush
Move the scheduled-email business-hours floor onto the business profile
as Google-style per-weekday opening blocks (multiple blocks/day for lunch
breaks). Migration 114 adds business_profile.business_hours (JSON) +
scheduled_email_floor_enabled; emailProcessor snaps a queued email to the
next open block, read in the profile timezone. Editor lives under
Settings → Business profile.
Add an admin "Send queued emails now" flush (POST /admin/email/flush-queue)
that drains the queue immediately, ignoring the business-hours floor — the
escape hatch before maintenance/updates. processEmailQueue now takes
{ignoreSchedule, limit} and returns send counts; the scheduled interval
run is unchanged.
|
||
|
Luca
|
ab6bad17c9 |
feat(hours): install-wide default rate + inline missing-rate CTA
Hour-entry saves hard-failed with an English-only error when a customer had no rate, and the standalone hours page showed a disabled rate field that looked set. Add a global business_profile default_hourly_rate_minor (migration 113) as the last link in the rate chain (entry override → customer → install default), so saves succeed with the global rate. When no rate resolves anywhere, replace the save-time error with a read-only resolved-rate display + a CTA to set a customer or install-wide rate, disable Add-entry until a rate/override exists, and translate the backend HOURLY_RATE_REQUIRED toast (en+de). |
||
|
Paul Nothaft
|
9d424d0dbb |
Merge pull request #596 from Luca-Timo/bugfix/crm-backup
Backup & Restore hardening — close the silent files-only data-loss class |
||
|
Luca
|
7988c18972 |
fix(restore): set was_successful=true on the completed update
Caught during the round-4 e2e validation on real PG: every successful restore landed with `status='completed', was_successful=false` because the success-branch update only wrote `status` but not `was_successful` (column default is false). Visible side effect: the BackupDashboard's "last successful restore" filter would skip the row + any future audit query gating on was_successful would miss it. One-line cure: include `was_successful: true` in the success-branch update payload. Inline comment explains why and references the review note so future edits keep the two fields together. Source-inspection test in restoreService.pgBranch.test.js pins the contract: after `performPostRestoreVerification(...)`, the `status: 'completed'` update payload must also contain `was_successful: true`. Future refactors of the success payload that drop the flag fail the test before merge. 36/36 backup-related integration tests pass. |
||
|
Luca
|
20e3092c14 |
fix(restore): move operator-meta replay after post-restore verification (PR #596 round 3)
End-to-end DR cycle surfaced one more PG-only landmine — and it
turned out to be a side-effect of the round-1 replay placement, not
a new bug. Round 2 fixed the comparison logic; round 3 fixes the
ordering.
Symptom on real PG install:
[install-from-backup] FAILED — Post-restore verification failed:
Table app_settings row count mismatch: expected 190, got 191.
Trigger file left in place for retry.
Root cause: the operator-meta replay (introduced in round 1) ran
INSIDE performDatabaseRestore, lined up BEFORE the post-restore
verification step in the parent restore() method. So:
1. psql restores app_settings → 190 rows (matches backup)
2. Replay upserts `restore_allow_force_auto_upgraded` (which the
fresh-install seeded but the backup didn't have) → 191 rows
3. performPostRestoreVerification counts 191, manifest says 190,
verification fails the row-count check.
Replay is doing the right thing (preserving operator policy). The
verification is doing the right thing (counts must match). They
disagree because the replay landed in the wrong sequence relative
to verification.
Cure: move the replay out of performDatabaseRestore and into
restore() AFTER `performPostRestoreVerification` passes.
Verification now sees the as-restored DB (matches the backup
exactly), replay layers on top once verification has signed off.
Mechanism: snapshot stashed on `this.preservedMetaSnapshot`
(initialised in constructor, reset per run at the top of restore()).
performDatabaseRestore writes it in the PG branch before DROP;
restore() drains it after verification. SQLite leaves it empty,
both steps no-op there.
Tests:
- Updated `restoreService.pgBranch.test.js` to pin the new shape:
* `this.preservedMetaSnapshot` is initialised in the constructor
* No stray `let preservedMeta = []` local declarations anywhere
* Replay drain (`this.preservedMetaSnapshot.length > 0`) sits in
restore() AFTER `performPostRestoreVerification(...)` and is
lexically OUTSIDE `performDatabaseRestore`.
- The bigint-as-string contract from round 2 still holds.
34/34 backup-related integration tests pass.
|
||
|
Luca
|
354fbed182 |
fix(restore): coerce pg bigint counts to Number before comparing (PR #596 round 2)
pg-driver serialises `bigint` (which is what `COUNT(*)` returns) as a JavaScript STRING to preserve precision for huge counts. The manifest stores `expected.rowCount` as a JS number (parseInt'd at databaseBackup.js:118). Strict `!==` in performPostRestoreVerification flagged every match as a mismatch on PG: Table activity_logs row count mismatch: expected 16, got 16 Table admin_users row count mismatch: expected 1, got 1 Table app_settings row count mismatch: expected 165, got 165 ... (every table, all matching) Symptom matched the preservedMeta scope leak from round 1: install- from-backup logged FAILED, trigger file wasn't cleaned, data was actually intact. Caught on PR #596 e2e re-run. Cure: coerce both sides with `Number(...)` at the comparison AND in the interpolated value so the warning text renders `16` not `"16"`. Pre-emptive: lines 448 + 458-459 had the same string-vs-number issue masked by `>` (JS coerces operands for `>`), but the warning text printed `"5"` on PG vs `5` on SQLite, and a future patch changing `>` to `=== 0` or `!== expectedCount` would silently break on PG. Coerced at the read site into `eventCountN` / `activeUsersN` locals + added a comment block explaining the contract so future edits don't drop the Number() calls without re-auditing. New source-inspection test: pins the contract that every `.count` result in restoreService.js MUST be wrapped in `Number(...)` when used in a comparison (===/!==/>/</>=/<=). Same source-inspection pattern as the preservedMeta test added round 1 — pragmatic until the real-PG integration test follow-up lands. The maintainer's audit of the rest of the backup/restore surface (_installFromBackupBoot, _restoreSettingsBoot, _backupPathsBoot, backupCoverageService, backupIntegrityService, backupService, databaseBackup) confirmed no other bigint-as-string sites — the class is now closed in the audited scope. |
||
|
Luca
|
a23fa3bb12 |
fix(restore): hoist preservedMeta above SQLite/PG split (PR #596 blocker)
`preservedMeta` was declared with `let` INSIDE the PostgreSQL else
branch of performDatabaseRestore (~L850), then read AFTER the else
block closed at the shared replay site (~L1030). On every real PG
restore, this threw:
ReferenceError: preservedMeta is not defined
after psql had already loaded the data successfully. Knock-on
effects per the maintainer's review:
- Loud `Install-from-backup: FAILED` line in combined.log even
though the data restored cleanly
- Trigger file in `_installFromBackupBoot.js` was left in place
because the success branch never ran — admin had to manually
rm it before the next boot
- The operator-meta replay (restore_allow_force,
restore_allow_force_auto_upgraded) silently dropped, exactly
the chicken-and-egg the snapshot was added to close.
`restore_allow_force` reverted to the backup's value on every
PG restore.
CI missed it because integration tests around `performFullRestore`
only exercise the SQLite branch (`this.dbType === 'sqlite'`). The PG
branch requires a real psql binary + cluster, which lives in the
"real-PG integration test in CI" follow-up.
Cure: hoist the `const PRESERVED_META_KEYS = [...]` + `let
preservedMeta = []` declarations above the SQLite/PG split. SQLite
leaves them empty; PG branch fills them; replay block at the bottom
reads them on both paths (no-op on SQLite).
New test: `restoreService.pgBranch.test.js` pins the scope contract
via source inspection. Two assertions:
1. Exactly one `let preservedMeta = []` declaration in the file,
positioned before the SQLite/PG branch split
2. The replay block `if (preservedMeta.length > 0)` sits outside
the else block (closing ` }` exists between the branch
opener and the replay site)
Source-inspection beats a runtime test here because (a) it doesn't
need a real PG cluster + psql binary, (b) it pins the EXACT property
that broke, more directly than a runtime test would.
Closes PR #596 review blocker.
|
||
|
Luca
|
07f9110674 |
chore(migrations): renumber 108_add_backup_paths to 109 to avoid upstream collision
upstream/beta independently shipped 108_seed_sl_email_template_translations.js
(Slovenian email template translations) using the migration number
this branch had already claimed for 108_add_backup_paths.js. Knex's
filename-based ordering would have caused both to attempt the slot
at merge time.
Renamed via `git mv` so file history is preserved. All five
references updated in lockstep:
- backend/src/services/_backupPathsBoot.js (require + comments)
- backend/src/services/backupService.js (LEGACY_BACKUP_PATHS comment)
- 3 integration test files (require + "migration 108" prose)
- migration's own header comment, with a paragraph explaining the
rename so reviewers don't wonder why the number jumped
**No data-migration impact for installs that already ran the
108-named version** (Ralf's beta, primarily): the migration's body
is idempotent — createTable is guarded by `hasTable`, and the seed
uses `onConflict('path').ignore()`. So when 109 runs against an
install whose backup_paths table is already populated, both the
schema step and the seed step no-op cleanly. The orphaned
`108_add_backup_paths.js` row in the `migrations` tracking table
sits harmlessly alongside the new `109_add_backup_paths.js` row.
No data lost, no double-insert, no schema drift. Mechanical rename
ahead of the PR opening.
|
||
|
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.
|
||
|
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).
|
||
|
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
|
d32bdda1b3 |
Merge pull request #586 from the-luap/feat/crm-route-tests-570
test(crm): HTTP route tests for CRM public + admin surface |
||
|
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) |
||
|
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.
|
||
|
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
|
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
|
614c8b9b8f |
test(backup-integrity): tolerate both knex .returning('id') return shapes
CI's SQLite returned `[N]` (plain int) from `.insert().returning('id')`
while local SQLite returned `[{ id: N }]` (object form). The brittle
`const [{ id }] = ...` destructure crashed on the int shape. Switched
to the unwrap pattern used by the existing crmDb test harness so the
suite runs on both PG and every SQLite/knex combo the project supports.
|
||
|
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 `john.doe@gmail.com` getting silently stored as `johndoe@gmail.com` 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
|
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
|
3240137f1e |
test(crm): integration harness + schema-shape regression net
Adds two pieces:
- __tests__/integration/helpers/crmDb.js — boots a temp-SQLite test
DB by invoking every migrations/core/*.up() directly. Bypasses
knex's Migrator because its exclusive write lock deadlocks
001_init's nested initializeDatabase() call. ~1 second cold start.
- __tests__/integration/crmSchema.test.js — 36 assertions on the
table + column layout after the consolidated CRM migration runs.
Pins:
- every CRM table present (quotes, contracts, invoices + the
eight supporting tables)
- deal_uuid columns on all three lineage tables (the column
DocumentLineageCard joins on — drop it anywhere and the card
silently returns partial data)
- back-pointer FKs (converted_contract_id, source_contract_id,
source_quote_id) — the exact columns that triggered the
Postgres FK-ordering bug fixed earlier in this PR
- Storno discriminator (kind, cancels_invoice_id, replaces_
invoice_id) per feedback_storno_filter_everywhere
- event time columns from migration 137
A full quote→contract→invoice lineage walk is deferred — quote
service's nextQuoteNumber() opens an inner transaction from inside
the createQuote outer transaction, which deadlocks SQLite's default
1-connection pool. Postgres dev DBs never see it. Either fix the
service to thread trx through, or run lineage tests against a real
Postgres in CI (mirror schema-drift.yml). Filed as separate work.
|
||
|
Luca
|
d543949188 |
feat(crm): backend code — services + routes + utilities + tests
Brings in the full backend CRM stack on top of the consolidated
migration (
|
||
|
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
|
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
|
b106da1ede |
fix(auth): /auth/session must enforce session timeout symmetrically (#350 recurrence)
Third loop fix in the same /admin/login → /admin/dashboard → /admin/login pattern as #355 and #363. Reported on v3.39.1-beta.0 — the loop returns after a server restart or after an idle gap longer than the configured session timeout. ## Root cause (server) `sessionTimeoutMiddleware` is mounted on `/api/admin` (server.js:411). It rejects with `401 SESSION_TIMEOUT` when either: - the in-memory `lastActivity` for the token is older than the timeout, or - this is the first request with this token AND the token's `iat` is older than the timeout (post-restart guard). `/auth/session` lives under `/api/auth/session`, NOT under `/api/admin`, so the middleware never runs for it. Result: an idle/old-iat admin token returns `valid: true` from `/auth/session` while every protected endpoint immediately rejects it with `401 SESSION_TIMEOUT`. Frontend's 401 interceptor hard-redirects to `/admin/login`, `/auth/session` says valid again, loop closes — exact same shape as the previous two asymmetries the symmetry pass missed. Fix: add a non-mutating `isSessionExpired(token, decoded)` helper to `middleware/sessionTimeout.js` that reads the same in-memory map and applies the same lastActivity / iat-vs-timeout logic as the middleware, without updating the map (the middleware is the only place that records activity; `/auth/session` is read-only by design). `/auth/session` calls the helper for `decoded.type === 'admin'` after the existing admin-existence and password-change checks. Same try/catch fall-through pattern as the prior fixes so a missing/broken helper doesn't fail-closed during early bootstrap or in test stubs. ## Root cause (client race amplifying the loop) Even with the server fix, the previous `useSessionTimeout` hook called `AdminAuthContext.logout()` which dispatches `POST /auth/logout` fire-and-forget AND has its own `finally { window.location.href }`, then immediately set `window.location.href = '/admin/login?session=expired'` on top. Two consequences: - The cookie wasn't reliably cleared before the new page loaded — if any /auth/session asymmetry slipped through, the loop replayed inside the same tab. New-tab and "refresh several times" "fixes" were just the logout request eventually completing. - Two redirects raced; sometimes the `?session=expired` query was dropped, breaking the login-page toast. Fix: rewrite the hook to (a) await `POST /auth/logout` so the cookie is guaranteed cleared, (b) clear `sessionStorage.admin_user` directly instead of going through AdminAuthContext.logout (which has the side-effect redirect we don't want), and (c) navigate exactly once with the `?session=expired` query. ## Tests - `__tests__/routes/authSession.symmetry.test.js` — 4 new cases under a `session-timeout symmetry` describe block: helper says expired → valid:false; helper says active → valid:true; helper not called for gallery tokens; helper throws → fall through to valid:true (defensive). Existing 9 tests still pass (mock now includes `isSessionExpired: jest.fn(() => Promise.resolve(false))` as the default). - `__tests__/middleware/sessionTimeout.isSessionExpired.test.js` — 7 new unit tests for the helper itself: fresh token / old-iat / recently-active / null-input / no-mutation / 60-min default boundary cases. 20 cases total, all green. Lint clean on every touched file. |
||
|
Paul Nothaft
|
51f28d7330 |
test(fonts): fix mock bypass and case-insensitive FS skip (#390 follow-up)
Two issues in the fonts service test suite added by #390 — the behaviour assertions all passed, but 5 of 24 tests had assertions that silently no-op'd, so any regression in those code paths would not have been caught. ## Issue 1: jest.resetModules() bypassed the logger mock `beforeEach` called `jest.resetModules()` then re-required `fontsService`. After resetModules, the `jest.mock('../../src/utils/logger', ...)` factory at the top of the file no longer applied to subsequent requires — so the freshly-required `fontsService` captured the REAL logger while the test file's `logger` variable still pointed at the mocked one. The 4 "warning logged" / "info logged" assertions resolved as 0 calls and silently passed-as-noop. The resetModules call wasn't necessary in the first place — module-level state in fontsService is just the cache, which clearFontsCache() already resets. And both getBundledFontsRoot() and getUserFontsRoot() read process.env at call-time, not at module load, so the env vars set in beforeEach are picked up without needing a fresh require. Fix: require fontsService once at module top (inside the jest.mock hoisting scope) and drop resetModules + the per-test re-require. ## Issue 2: case-insensitive filesystem (macOS / Windows) The "case-insensitive duplicate within the same root" test created `Inter/` and `INTER/` to trigger the dedup warning. On a case-sensitive FS (Linux ext4) both directory entries exist and the dedup branch fires; on macOS APFS or Windows NTFS the second mkdir resolves to the same folder as the first, so only one ever exists and the dedup is unreachable from this test setup. Test failed on macOS dev, passed on Linux CI. Fix: probe at load time by creating a lowercase file and checking if its uppercase variant resolves to the same inode, then conditionally test.skip the affected test on case-insensitive hosts. Comment in the test body explains why. ## Result 23 of 24 tests now pass on macOS; the case-sensitive-only test runs on Linux CI. All previously-no-op'd assertions now exercise their code paths. |
||
|
Luca
|
b609a7a88c | test(fonts): unit-test scanner edge cases and meta.json handling | ||
|
Paul Nothaft
|
e8052adf1d |
fix(email): render conditionals, localise password placeholders, fix caller/template variable drift
Bundle of email-renderer and email-caller fixes triggered by a
reproducer on picpeak.nothaft.cloud (gallery_created mail showing
literal `{{#if welcome_message}}` markers and `Passwort: (set at
creation)`). The audit that followed surfaced six more user-visible
defects in the same surface; all are fixed here so customer-facing
mail renders cleanly.
Renderer (`backend/src/services/emailProcessor.js`)
- `safeTemplateReplace` now resolves `{{#if VAR}}…{{/if}}` blocks
before flat `{{var}}` substitution. The shipped templates have used
Handlebars-style conditionals since migration 026; the renderer
ignored them, so the markers leaked verbatim into every mail with
an empty welcome_message. Lifted to module scope and exported so
the conditional contract is unit-testable. Single-pass, non-nested
(commented).
- Added `passwordSetAtCreationI18n` next to the existing two i18n
password sentinels so `(set at creation)` (sent by the publish-
from-draft flow when only the bcrypt hash remains) is localised
to "Das bei der Erstellung der Galerie gesetzte Passwort" /
equivalent in EN/DE/NL/PT/RU instead of the raw English string.
- Added an opt-in `{ escapeHtml: true }` mode to `safeTemplateReplace`
so admin-supplied free text (`event_name`, `host_name`, …) is
HTML-escaped on substitution into the HTML body. Allowlist of
passthrough keys (`welcome_message` already-HTML, server-generated
URLs `gallery_link` / `client_link`). Subject and text body keep
the legacy unescaped behaviour. `formatWelcomeMessage` now escapes
before nl2br so the welcome_message allowlist is safe.
- New `htmlToText()` strips `<style>` and `<script>` blocks (and
their content) before tag-stripping, decodes common entities, and
collapses whitespace. Used by the textBody fallback in
`sendTemplateEmail` — without this, every template missing a
`body_text` produced a "plain-text" mail starting with the 100+
lines of CSS embedded by `wrapEmailHtml()`.
- The client-access section (#172) now mirrors its HTML block into
`textBody` using the same per-language strings, so plain-text
recipients see the link / PIN / warning. `pinLabel = 'PIN'` moved
into `clientAccessI18n` (RU uses ПИН-код).
- Added `getSupportEmail()` exported helper that reads
`branding_support_email` from `app_settings` (JSON-decoded), with
the SMTP from-address as fallback. Used by the gallery_expired and
archive_complete callers below.
- Removed dead `require('handlebars')` (unused since the regex
renderer landed; pre-existing lint error in this file).
Callers (data the templates already reference)
- `expirationChecker.js queueExpirationWarning`: send `expiry_date`
(templates use this, the old code sent `expiration_date` —
typo'd key, never read), drop the hard-coded `.de`/`en` sniff
(the processor formats with the recipient's resolved language),
add the `{{password_security_message}}` sentinel for
`gallery_password` (plaintext is gone by warning time, so
customers used to see literal `{{gallery_password}}` in the mail).
- `expirationChecker.js handleExpiredEvent`: both queueEmail calls
now supply `host_name`, `event_date`, `expiry_date`,
`support_email` so the EN/DE/NL/PT/RU `gallery_expired` template
doesn't render literal `{{host_name}}, your gallery expired on
{{expiry_date}}`. Skip the duplicate admin send when
admin_email == customer_email.
- `archiveService.js`: `archive_complete` queue now supplies
`host_name`, `photo_count` (from `photoEntries.length`),
`archive_date`, `support_email` — the previous payload had only
`event_name` and `archive_size`, so most of the mail was
unfilled placeholders.
Tests
- `__tests__/services/emailProcessor.safeTemplateReplace.test.js`:
16 cases — flat substitution, conditional truthy/falsy/missing/
multi-line/sibling/numeric-0, plus 5 cases for the new
`escapeHtml` option (default off, escape on, allowlist
passthrough for welcome_message and gallery_link).
- `__tests__/services/emailProcessor.htmlToText.test.js`: 7 cases —
the regression scenario (full wrapped body with embedded `<style>`
block), tag-stripping, entity decoding, paragraph spacing.
- `__tests__/utils/formatters.test.js`: 12 cases for `escapeHtml`,
`nl2br`, and the now-escaping `formatWelcomeMessage`.
35 cases total, all green. Lint clean on every touched file
(also fixes a pre-existing `no-prototype-builtins` warning in the
process). Pre-existing failures in
`__tests__/services/backupService.enhanced.test.js` are unrelated
and pre-date this branch.
|
||
|
Paul Nothaft
|
b30010eb6f |
test(upload): unit tests for backgroundProcessor + processPhoto
Cover the two new pieces of the async pipeline:
backgroundProcessor.claimNextPhoto
- returns null when no pending rows
- returns the row + flips status under postgres FOR UPDATE SKIP LOCKED
- returns null when SQLite UPDATE-with-guard loses the race
- returns the row when the SQLite guard wins
photoProcessor.processPhoto
- happy path: writes thumbnail / dimensions / EXIF capture date and
marks 'complete'; fires watermark queue + photo.uploaded webhook
with the right payload
- video path: writes ffmpeg duration / codec / dimensions; does NOT
queue watermark (image-only)
- throws cleanly when the photo row no longer exists
Mocks db / imageProcessor / videoProcessor / storage / sharp /
watermarkGeneratorService / webhookService / logger so the tests run
without a real DB or any image library calls — fast and deterministic.
|
||
|
Paul Nothaft
|
f905f7e733 |
fix(auth): /auth/session must reject tokens that adminAuth/galleryAuth would reject
Second loop fix in the same /admin/login → /admin/dashboard → /admin/login pattern as #355. The frontend trusts /auth/session as the source of truth for "is the user authenticated?". When that endpoint is more lenient than the protected middleware, every admin endpoint 401s right after /auth/session said valid:true, the response interceptor hard-redirects to /admin/login, /auth/session says valid again, and the cycle closes — exactly the loop reported on v3.32.4-beta.0. #355 fixed the issuer-claim asymmetry. This commit fixes the remaining asymmetries: /auth/session was missing the admin-existence, admin-active, password-change-after-iat, and gallery-existence / gallery-archived / gallery-expired checks that adminAuth and galleryAuth perform on every protected request. The fix is to mirror those checks in /auth/session, scoped by token type, and degrade gracefully when the underlying tables aren't present (test fixtures, early bootstrap) so the endpoint never fails-closed because of a missing table. Reproducer that the new test covers: 1. Admin logs in (token issued at T). 2. Admin (or another admin) changes their own password at T+1. 3. Browser still has the cookie from T. 4. /auth/session says valid:true (no password-change check). 5. /admin/dashboard fires queries; adminAuth rejects with PASSWORD_CHANGED 401. 6. Frontend redirects to /admin/login. 7. /auth/session says valid:true again. → loop. Other surfaces this also covers: - admin user deactivated (admin_users.is_active = false) - admin user deleted - gallery token whose event is archived - gallery token whose event has expired Tests live in __tests__/routes/authSession.symmetry.test.js — 9 cases, mocking db / tokenRevocation / tokenUtils / recaptcha / sessionTimeout so the suite runs without a real database. |
||
|
Paul Nothaft
|
e232f9f2cf |
fix(backup): incremental backups against S3 + jsonb stats parsing
Three fixes uncovered while bringing the backup-s3 integration suite to 12/12 against MinIO + Postgres: - backupService.getDatabaseBackupInfo: pg's jsonb driver auto-parses `statistics` / `table_checksums` to objects; the old JSON.parse() then threw "[object Object]" is not valid JSON and the manifest dropped database info silently. Accept both string and object inputs. - backupService.runBackup: incremental path called backupManifest.loadManifest() with an s3:// URI directly, which falls through to fs.readFile() and ENOENTs — every "incremental" backup silently downgraded to a full one. Added loadManifestFromAnywhere() helper that downloads s3:// to a tmp file before delegating. - backupManifest.generateIncrementalManifest: attached the `incremental` section AFTER generateManifest() had already stamped verification.total_checksum, so every incremental manifest failed validateManifest() on read-back. Recompute the checksum after. Test side: updated assertions to the current manifest shape (`incremental.changes.modified_files_count`), Number()-coerce bigint columns from pg, and gate the logger mock on UNMOCK_LOGGER for diagnosing similar silent-failure modes in the future. |
||
|
Paul Nothaft
|
ab4095f592 |
fix(backup): cron schedule mapping + manifest format detection + bigint coerce
Three pre-existing bugs surfaced by re-running the backup-s3 integration
suite. backup-s3 went 0/12 → 7/12 (storage-refactor session bootstrap
fixes) → 10/12 with this commit.
1. Backup service crashes on backend startup with
`TypeError: Cannot read properties of undefined (reading 'replace')`
from node-cron's expression parser.
Root cause: `backup_schedule` stores a UI label like "weekly", while
`backup_schedule_cron` stores the actual cron expression. Startup
code read the label and passed it straight to cron.schedule() —
"weekly" is not a cron expression.
Fix in startBackupService(): read backup_schedule_cron first; fall
back to mapping known labels (hourly/daily/weekly/monthly) to cron
expressions; back-compat for deployments that wrote a cron expression
into the legacy backup_schedule field.
2. Backup manifest retrieval fails with
`SyntaxError: Unexpected token 'a', "applicatio"...` when the
manifest format is YAML.
Root cause: getBackupManifest() downloads the s3:// manifest to a
tmp file hardcoded as `manifest-N.json`. loadManifest() then
detects format from extension only — sees .json, runs JSON.parse on
YAML content (which starts with "application: …"), fails.
Fix in backupManifest.loadManifest(): detect format from BOTH the
extension AND the content's first non-whitespace character. JSON
starts with { or [; anything else falls through to yaml.load.
Backwards compatible — extension is still authoritative when present
AND content matches.
3. Test assertion `expect(backupRun.total_size_bytes).toBeGreaterThan(0)`
fails with "received value must be a number or bigint" because pg
driver returns bigint columns as strings. Coerce via Number() in
the test.
Remaining 2 failures (out of scope here, both are spec-level drift):
- "should include database backup" expects the runBackup() flow to
upload the database backup file at S3 key `database/db-backup.sql`.
Current implementation reads db backup metadata for the manifest but
does not upload the file itself. Missing feature, not a test bug.
- "should only upload changed files" expects manifest.incremental.
modified_files_count. Implementation writes backupType: 'incremental'
on the run row but no per-run incremental subobject in the manifest.
Field shape mismatch.
|
||
|
Paul Nothaft
|
c488f481ca |
feat: outbound webhooks for event/photo lifecycle (#327)
PicPeak POSTs lifecycle notifications to admin-configured URLs. Each delivery is signed HMAC-SHA256 in the X-PicPeak-Signature header. Verified end-to-end: 1/1 Playwright spec, 8/8 backend integration tests, full UI click-through via Chrome DevTools. Schema (migration 082) - webhooks: id, name, url, secret (plaintext — required to compute HMAC for every outbound POST), secret_preview, events[], active, filter, template, created_by, timestamps, last_success_at/last_failure_at. - webhook_deliveries: webhook_id (FK CASCADE), event_type, payload, attempt_count, status (pending|success|failed), response_status, response_body (truncated to 1KB), latency_ms, next_retry_at, last_error, created_at, completed_at. Composite index (status, next_retry_at) serves the worker's hot-path query. Service + worker - webhookService.fire(eventType, data) — non-throwing entry point used by lifecycle hooks. Looks up active webhooks subscribed to the event and applies their per-webhook filter (dot-path equality predicate) before enqueueing one webhook_deliveries row per match. Filter and template logic ship in this commit; admin surfaces in the follow-up. - webhookDeliveryWorker — setInterval(5s) poller; fetches up to 5 pending rows; per delivery: re-validates URL via networkValidation (DNS-rebinding mitigation, opt-out via WEBHOOK_ALLOW_PRIVATE_URLS), signs body with HMAC-SHA256, POSTs with 10s timeout, records outcome. Backoff schedule: 1m → 5m → 30m → 2h → 12h, max 5 attempts. Response body truncated to 1KB before storage. If a webhook has a template, the rendered string replaces the JSON envelope as the request body (signature is computed over the bytes actually sent). Lifecycle wiring - adminEvents.js POST /events → event.created (+ event.published when not draft); POST /:id/publish → event.published. - routes/events.js (legacy public POST) → event.created + event.published. - routes/v1/events.js (#322 API) → event.created + event.published on create, photo.uploaded on photo POST. - archiveService.archiveEvent() → event.archived. Per-photo photo.deleted intentionally NOT fired during cascade — receivers infer from event.archived to avoid flooding (issue spec). - expirationChecker.handleExpiredEvent() → event.expired BEFORE the cascading archive (so receivers see expired→archived in order). - adminPhotos.js — photo.uploaded on each batch row, photo.deleted on single + bulk delete. - photoProcessor.js — photo.uploaded for guest uploads + auto-import (covers all entry paths). - fileWatcher.js — photo.uploaded on add, photo.deleted on unlink (local mode only). Admin endpoints (mirrors adminApiTokens.js pattern) - /api/admin/webhooks: GET list, POST create (returns plaintext secret exactly once), GET :id, PUT :id, DELETE :id, POST :id/test (synthetic fire), GET :id/deliveries (paginated, filter by status), GET :id/deliveries/:deliveryId, POST :id/deliveries/:deliveryId/replay. Frontend - Settings → Webhooks tab (mirrors API Tokens layout): name + URL + event checkboxes + "Advanced" expander for filter (JSON) and template. Plaintext secret shown once on creation with a Copy button. Active/ Disabled toggle button per row. - /admin/webhooks/:id/deliveries — operational debug surface. Table with timestamp/event/status/attempts/HTTP/latency. Status filter chips (all/pending/success/failed). Row click → slide-over with payload + signature + response body. Replay button on failed rows. Send-test-event dialog. Auto-refresh every 10s. Dev infrastructure - dev/webhook-receiver/ — tiny node:alpine HTTP server (~100 LOC) that records every POST to an in-memory ring buffer. Exposes GET /requests for the E2E spec to assert deliveries landed with the right HMAC. Sibling pattern to MinIO. Reachable from the backend at http://webhook-receiver:8888 inside the picpeak network. Tests - backend/__tests__/integration/webhookDelivery.test.js (8/8) — signature verification, headers, retry/backoff, max-attempts → failed, response truncation, disabled-mid-flight, SSRF block, start/stop idempotency. - tests/e2e/webhooks-roundtrip.spec.ts (1/1) — create webhook → trigger event.published → assert receiver got POST with valid HMAC → visit deliveries page → row visible with status=success → API test event → API replay → disable webhook → assert no new delivery. Docs - README §"Webhooks" — event catalog, payload shape, HMAC verification in Node + Python + bash, retry semantics, SSRF protection. - .env.example — WEBHOOK_ALLOW_PRIVATE_URLS, WEBHOOK_DELIVERY_INTERVAL_MS, WEBHOOK_DELIVERY_CONCURRENCY, WEBHOOK_HTTP_TIMEOUT_MS, WEBHOOK_MAX_ATTEMPTS. Out of scope for v1 (per issue): webhook templates' code-eval (the ${dot.path} substitution that ships is pure string replacement, no expression engine — see follow-up commit), per-webhook rate limiting beyond the global concurrency cap, synchronous "ask before delete" webhooks. Spanning files - App.tsx pulls in this commit with both the AnalyticsBootstrap (#325 dedup) and the WebhookDeliveriesPage route registration. Splitting via git add -p was forfeit for sanity; the single 92-line diff is honest about both contributions. - adminEvents.js diff bundles the webhook fires AND the allow_presigned_download field plumbing (#328 follow-up). Same reasoning. - The new webhookService/Worker/adminWebhooks files include the filter and template logic from the follow-up — they were authored in one pass; splitting them post-hoc would have produced fragile partial files. The follow-up commit covers the migration and the UI for these. |
||
|
Paul Nothaft
|
1b717ce5ed |
feat: native S3 storage backend (#328) + presigned download follow-up
Lets PicPeak write photos, thumbnails, hero images, watermarks, and archive zips to any S3-compatible bucket (AWS S3, MinIO, Cloudflare R2, Backblaze B2, Wasabi, DigitalOcean Spaces) instead of the local filesystem. Selected via STORAGE_BACKEND=local|s3. Architecture - backend/src/services/storage/StorageBackend.js — abstract interface (put/get/exists/stat/delete/list/copy/rename/signedUrl/putFromFile/ getToFile) — typedef-only, documents the contract. - LocalFsStorage.js — wraps fs with atomic-write-via-tmp-rename, path traversal protection, list-as-walker. - S3StorageBackend.js — thin wrapper around the existing S3StorageAdapter (used by backupService) mapping it onto the canonical interface; supports optional STORAGE_S3_PREFIX namespace. - index.js — factory selected by STORAGE_BACKEND with startup ping (HEADs sentinel key on S3, fs.stat on local) so misconfig fails fast before the first request. Consumer refactors (~12 services + routes), each parametrized over the abstraction: - imageProcessor / videoProcessor — pipe Sharp/ffmpeg output through storage.put; expose withLocalCopy() helper for S3-mode regeneration paths that need a local file for sharp/ffmpeg. - archiveService / downloadZipService — finalize zip in tmp dir, then storage.putFromFile. Atomic-rename pattern preserved on local; S3 emulates via copy + delete (worker prunes orphaned .tmp.* on startup). - photoProcessor / photoReplacementService / adminPhotos upload+delete / routes/v1/events.js POST /events/:id/photos / routes/events.js — every upload path now goes storage.putFromFile(temp) → unlink temp. - gallery.js bulk-download (cached + on-the-fly + selected) — managed photos via storage.get, external-mode unchanged. - protectedImages / secureImages / photoResolver — read via storage.get; resolvePhotoStorageKey returns the canonical key. - watermarkService / watermarkGeneratorService — persistent watermarks via storage.put. - fileWatcher — bails out with a clear log warning when STORAGE_BACKEND=s3 (chokidar can't watch S3); auto-import lands via the S3 prefix walker introduced in the follow-up commit. - expirationChecker — small touch (event.expired webhook fire from #327 shipping in the next commit). Migration tooling - backend/scripts/migrate-storage.js — one-shot --dry-run capable script that walks photos.path, thumbnail_path, hero_path, watermark_path and events.archive_path/download_zip_path; streams local → S3; sha256 size-match skip for idempotent re-run; failures CSV. Presigned-URL "Download All" (#328 follow-up shipped in this commit) - routes/gallery.js — when STORAGE_BACKEND=s3 + event.allow_presigned_download + downloads enabled + watermark NOT enabled, /download-all returns a 302 redirect to a 5-minute presigned S3 URL. Per-event opt-in surface ships in the next commit's UI. Tests - backend/__tests__/integration/storageBackend.test.js — parametrized contract suite running against BOTH LocalFs AND MinIO (18 tests, both backends — 36 cases total). - backend/__tests__/integration/imageProcessor.storage.test.js — same parametrized pattern for the image processor (10 tests × 2 backends). - backend/__tests__/integration/backup-s3.test.js — bootstrap fix: drop the redundant initDb() (001_init handles it) and remove schema-drift in configureS3Backup (app_settings has no created_at anymore and the unique constraint is on setting_key alone, not composite). 0/12 → 7/12 (5 remaining are unrelated assertion drift). - backend/src/services/photoResolver.js — mixed-source events (reference mode with managed-uploaded photos) now fall back to managed when external_relpath is missing instead of throwing. - tests/e2e/s3-storage-roundtrip.spec.ts — Playwright spec that auto-skips against local backend; full upload → serve → delete round-trip when run against an S3-mode backend. Server wiring (server.js) - initStorage() called after database init, before rate limiters. - This commit's diff also includes the webhook delivery worker startup and the S3 auto-importer startup. Those features ship in the next two commits — co-located here for one bisectable diff per file. Docs + ops - README §"Storage Backends" — capability matrix, switching playbook, IAM policy snippet, MinIO/R2/B2 examples. - README §"Webhooks" — also added here (full diff bundled). - .env.example — STORAGE_BACKEND + STORAGE_S3_* + STORAGE_AUTO_IMPORT documented; WEBHOOK_* added in the same diff. - .gitignore — re-anchor the existing `storage/` rule to `/storage/` so backend/src/services/storage/ (the new abstraction code) is trackable. The runtime ./storage/ data dir stays ignored. Out of scope for v1 (per the issue): presigned URLs for individual photo display (always streamed for protection middleware), CDN integration, hybrid hot/cold tiers, S3 → local migration, multi-bucket per-event. |
||
 paul
|
775e417e55 | Fix admin reference mode regressions | ||
|
paul
|
fc1bf53412 | fix: harden gallery downloads and per-gallery auth | ||
|
paul
|
5f8c8c5508 | Fix branding asset storage path | ||
|
paul
|
2a4d38813f | feat: overhaul public landing page and backup tooling |