169 Commits

Include renames

Author	SHA1	Message	Date
Paul NothaftandPaul Nothaft	9003b34c8a	fix(docker): default NODE_ENV=production so non-compose deploys don't fall back to SQLite (#1038) (#1040) ... * fix(docker): default NODE_ENV=production so non-compose deploys don't fall back to SQLite (#1038) knexfile.js selects its config block by NODE_ENV and the `development` block defaults to sqlite3. The image never set NODE_ENV, so every deployment that doesn't go through our compose files — Kubernetes, Helm, plain `docker run` — silently ran on SQLite and ignored DB_HOST/DB_USER/DB_PASSWORD. It stayed invisible because wait-for-db.sh is shell: it reads DB_HOST directly, connects to Postgres, creates the database and logs "PostgreSQL is up" in the same container where the Node process then writes to a SQLite file. Migrations go through src/database/db.js → the same knexfile, so they also ran against SQLite, leaving the provisioned Postgres database empty. Setting the default alone would be unsafe: an affected install would flip to Postgres on its next image pull and come up against an EMPTY database, which reads as total data loss. So this adds a guard that runs before migrations touch anything: - logs the resolved engine + target at boot - refuses to start when pointed at a virgin Postgres while a populated SQLite file exists, naming the file and the .picpeak export path for moving the data, with PICPEAK_ALLOW_EMPTY_PG=true as the escape hatch - warns but boots when Postgres settings are present yet SQLite is in use Compose files already set NODE_ENV explicitly, so compose users are unaffected. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): stay on SQLite instead of blocking, and add a migration path (#1038) Reworks the guard after walking through what an existing install actually experiences on its next image pull. Blocking was the wrong trade. An operator who had unknowingly been running on SQLite (because the image left NODE_ENV unset) would have pulled the fix and got a CrashLoopBackOff: data safe, galleries offline, for something they did not do. Now the boot RESOLVES the engine before migrations run and stays on whichever one holds the data: - Postgres configured but holding no galleries, while a populated SQLite file exists → keep serving from SQLite, print what happened and how to migrate. - once Postgres holds the data, the next restart switches over on its own. - an explicit DATABASE_CLIENT is always honoured. Keyed on Postgres holding DATA, not on it having tables: a stray `run-migrations` against the empty database creates every table, which would otherwise blind the check. Adds scripts/migrate-sqlite-to-postgres.js, which reuses the .picpeak export/import services rather than hand-rolling a cross-engine copy. Two additions were needed for the SQLite → Postgres direction, both opt-in and CLI-only so the upload/restore UI is untouched: - `allowEngineSwitch` relaxes the importer's same-engine guard - cross-engine row coercion: SQLite has no real date or boolean types, so its rows carry epoch numbers where Postgres wants a timestamp and 0/1 where it wants a boolean, both of which Postgres rejects outright. Driven by the TARGET schema, never guessed from the value. DELTA FROM THE BETA PR: this branch's import service has no resyncSequences() — that landed on main only. Without it a cross-engine load leaves Postgres identity sequences at 1 and the next insert collides on the primary key, so the function is backported here and called ONLY on the cross-engine path. Same-engine restores through the UI keep their current behaviour exactly. Verified end to end on this branch against a real PostgreSQL 15: a seeded SQLite install migrated across with booleans, timestamps and foreign keys intact, and the next INSERT got id 2 rather than colliding. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close four review findings on the SQLite fallback + migration (#1038) External review (codex) found four issues, all confirmed against the code and fixed here. Two of them could have cost data. 1. The engine resolver was reachable only through wait-for-db.sh. A Kubernetes manifest that sets `command`/`args`, or a plain `docker run … node server.js`, bypasses the entrypoint — exactly the deployment styles this fix targets. With NODE_ENV now baked into the image, such an install would have resolved to Postgres and come up against an empty database while its SQLite data sat there unseen. server.js now resolves the engine itself, before anything requires knexfile, via the same script the entrypoint uses. Verified by running `node server.js` directly against an install with stranded SQLite data: it logs the banner and serves SQLite. 2. Cross-engine loads double-encoded JSON. SQLite has no json type, so its json columns are TEXT holding JSON; the export dumps that as a string and serialiseJsonColumns stringified it again, storing `true` as the scalar string "true". app_settings.setting_value is json on every install, so this reshaped every migrated setting. The text is decoded before serialisation now — verified against a real Postgres: json_typeof(setting_value) is `boolean`, matching a native install exactly. 3. The migration could silently miss concurrent writes. If the backend keeps serving, rows written after the export never reach Postgres and vanish from view once the engine switches. The script now fingerprints the SQLite tables whose loss would be noticed, checks for drift BEFORE loading Postgres (so a detected race leaves the target untouched) and again after, and refuses with the exact rows that moved. It also says plainly to stop the backend first. 4. The child phases shared stdout with winston. Outside production, and whenever LOG_TO_CONSOLE=true, createPicpeak's own log line was concatenated with the archive path and the migration failed on a bogus filename. Payloads travel through a result file now; verified with LOG_TO_CONSOLE=true. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 2 — six more data-safety findings (#1038) 1. The engine choice is now PINNED once the data is in Postgres. Previously the boot decided from "does Postgres hold galleries", so an operator who later deleted every gallery would be sent back to the stale pre-migration SQLite file while their settings, admins and CRM data stayed in Postgres. The migration writes a marker next to the database file (and retires the file itself by renaming it); the marker wins over any probe. 2. The migration refused to overwrite Postgres only when it held GALLERIES. A target with admins, customers, invoices or projects but no galleries was wiped without --force. Both the source and target checks now look for user data across the tables that are empty on a fresh install. 3. Same bug in the other direction: an install with no galleries but real admins/settings/customers was refused a migration it was entitled to. 4. Drift detection covered four tables and only count/max(id), so an in-place UPDATE (event edit, password change) or a write to any other table passed unnoticed. It now fingerprints every table the export carries, including max(updated_at). It still is not a substitute for stopping the backend, and the script says so rather than implying a guarantee. 5. probeSqliteData() treated an unreadable or corrupt file as "no data", which would have switched the install to an empty Postgres — the very failure this module exists to prevent. It fails closed now and stays on SQLite so the real error surfaces. 6. The "you are leaving SQLite data behind" warning was unreachable: setting DATABASE_CLIENT skipped the probes, so the branch that produces it never had the inputs. Postgres and SQLite are both probed whenever Postgres is the engine in play. Also: the final verification compares row counts for EVERY table rather than just galleries, and flags only a shortfall — the import legitimately adds an app_settings row (setSessionsValidAfter) that made the strict equality fail on a first real run. Verified against a real PostgreSQL 15 end to end, including: the marker keeps an install on Postgres after every gallery is deleted; removing the marker and restoring the file rolls back to SQLite as documented. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 3 — occupancy, bootstrap admin, secrets in /tmp (#1038) 1. Both engine probes judged occupancy by GALLERIES alone. An install whose galleries were all deleted, but which still has admins, customers or accounting records, was treated as empty: on the SQLite side that meant booting the empty Postgres and appearing to lose everything; on the Postgres side it meant diverting a live install to a stale SQLite file. Both now look across the tables that are empty on a fresh install, matching the migration script. 2. The migration ran migrate-schema BEFORE checking the target, and migration 001 seeds a bootstrap admin when ADMIN_PASSWORD is set (common on legacy installs). The occupancy check then saw that admin and refused, pushing the operator towards --force against a genuinely empty database. The target is read first now. 3. probeSqliteData()'s warning went through the app logger, which writes to STDOUT when LOG_TO_CONSOLE=true — and the resolver's stdout is the protocol channel wait-for-db.sh captures, so DATABASE_CLIENT could have been set to a JSON log line. Diagnostics take an injected sink (stderr in the resolver), and the shell now validates the value it captured instead of trusting it. 4. The .picpeak archive holds password hashes, SMTP credentials and API keys in plaintext, and was only removed on the fully-successful path — any drift or import failure left it in /tmp. Every exit path removes it now. 5. A database-only migration still hauled every business-doc and upload through /tmp and back into the same volume. createPicpeak takes includeFiles:false for this path; rows move, files stay where they already are. Verified against a real PostgreSQL 15: a gallery-less install with only an admin account now stays on SQLite and migrates successfully with ADMIN_PASSWORD set; the resolver emits exactly one token on stdout with LOG_TO_CONSOLE=true and a corrupt database; a drift failure leaves Postgres untouched and no archive behind. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): pin the boot to SQLite while a migration is unfinished (#1038) Review round 4. A migration that dies after touching Postgres leaves rows behind — schema creation alone seeds a bootstrap admin when ADMIN_PASSWORD is set, and a drift or row-count failure can leave a partial load. Since the occupancy probes were widened in round 3, those rows read as "Postgres is occupied", so the next restart would switch engines and hide the SQLite data that is still the database of record. The script now writes a pin file next to the database BEFORE its first Postgres write and clears it only on success (after the success marker exists, so no restart in between can pick the wrong engine). While the pin is present the resolver stays on SQLite and explains why. Verified against a real PostgreSQL 15 by reproducing the exact scenario: a migration failed mid-run with ADMIN_PASSWORD set, leaving one bootstrap admin in Postgres. With the pin the next boot resolves to sqlite3; with the pin removed it resolves to pg — the failure this closes. The subsequent successful re-run clears the pin and the boot moves to Postgres. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 5 — occupancy, path drift, retry, host default (#1038) 1. A seeded bootstrap admin counted as "Postgres is occupied". core/001_init.js inserts one whenever ADMIN_PASSWORD is set, so a Postgres that was initialised once and never used would have beaten a SQLite file full of real galleries — the exact failure the guard exists to prevent, reintroduced by widening the probe in round 3. The two sides are deliberately asymmetric now: the SQLite probe counts any user data (err towards keeping data visible), the Postgres probe ignores rows that schema creation seeds (err towards requiring proof of real use). 2. The guard resolved DATABASE_PATH with its own logic while knexfile trimmed whitespace and collapsed the legacy duplicated-backend form. A path either engine normalised differently meant probing a file nobody uses, concluding there was no SQLite data, and booting an empty Postgres. The resolution now lives in one module both require. 3. Re-running after a partial migration — the documented recovery — was refused unless the operator passed the destructive-sounding --force, because the half-written rows read as target data. An unfinished run of this same script is now recognised as a safe retry. 4. wait-for-db.sh verified readiness against its own default host (`postgres`) while knexfile's production block defaults to `db`. With NODE_ENV now baked in, a bare `docker run` without DB_HOST would have passed the readiness check against one host and then dialled another. The entrypoint exports the exact connection it verified. Compose sets DB_HOST explicitly and is unaffected. Verified: a Postgres holding only a seeded admin now loses to real SQLite data; a DATABASE_PATH with surrounding whitespace resolves to the identical file in both knexfile and the guard. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 6 — explicit-client bypasses, retry scope, cleanup (#1038) 1. An explicit DATABASE_CLIENT bypassed the unfinished-migration pin, because decideBootEngine honoured it first. docker-compose sets DATABASE_CLIENT=pg, so a failed migration would have restarted on a half-written Postgres on exactly the deployments that pin it. Worse in the other direction: with DATABASE_CLIENT=sqlite3, a SUCCESSFUL migration renames the source file, so the next start created a NEW, empty SQLite database and served that. The pin now outranks explicit pg (clearing the marker is the override), explicit sqlite3 is left alone since it already points at the data, and the migration refuses up front when the deployment pins anything other than pg. 2. The retry allowance was bound to the SQLite file, not to the target. An operator who repointed DB_HOST/DB_NAME between attempts could have replaced an unrelated populated database without --force. The pin records the target and the allowance only applies when it matches. 3. The printed rollback did not roll back: with data on both sides and no marker, the resolver still selects Postgres. It now spells out all three steps, including DATABASE_CLIENT=sqlite3. 4. A failure inside createPicpeak left a partial archive — plaintext hashes and credentials — in the caller-supplied temp dir, which that service deliberately does not clean. The export phase removes it on error. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 7 — pin bypass on direct start, real admins (#1038) 1. server.js only ran the engine resolver when DATABASE_CLIENT was unset, so a deployment that both bypasses the entrypoint (Kubernetes `command:`) AND pins DATABASE_CLIENT=pg never consulted the migration pin — the round-6 fix was unreachable on exactly that path, and a failed migration would have served a half-populated Postgres. The resolver now also runs whenever a pin file exists. 2. Round 5 excluded admin_users from Postgres occupancy to stop a seeded bootstrap admin counting as real data. That over-corrected: an install that has completed first-run setup but has no galleries yet has exactly one user-created row — an admin — so Postgres looked empty and, with a stale SQLite file present, the boot would switch away and the admin's credentials and configuration would disappear. core/001_init.js seeds must_change_password=true; setupService writes false once a human completes setup. The FLAG, not the table, distinguishes them, and a legacy NULL counts as a real admin. Verified against a real PostgreSQL 15: a Postgres holding only the seeded row loses to real SQLite data, the same Postgres wins once setup is completed, and a server started directly with DATABASE_CLIENT=pg and a pin present comes up on SQLite with the warning. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 8 — reset admins, CLI config, JSON nulls (#1038) 1. must_change_password is mutable: resetAdminPassword() re-raises it on REAL accounts (userManagementService.js:474). Round 7's discriminator therefore read a gallery-less Postgres whose only admin had been reset as an untouched bootstrap seed — and with a stale SQLite file present, the boot would have switched away and hidden those live credentials. The rule is layered now: more than one admin, any admin that has logged in, or must_change_password false all count as use. Only core/001_init.js's exact leftovers — one admin, never logged in, still flagged — read as a seed. 2. The CLI read process.env directly but never loaded the configuration the child phases get through knexfile, so running it directly (or via `docker exec`, which does not inherit wait-for-db.sh's exports) failed the pre-flight checks even with valid settings in backend/.env or /run/secrets/db_password. Both sources are loaded up front now. 3. The migration's target check counted a seeded bootstrap admin as user data while probePgData classified the identical row as empty, so migrating into a previously-initialised-but-unused Postgres demanded --force. Same rule on both sides. 4. Cross-engine JSON handling is simpler and no longer lossy. SQLite keeps json columns as TEXT holding valid JSON and Postgres accepts JSON text directly, so the correct action is to pass them through untouched. Round 1 parsed then re-serialised them to undo a double-stringify; that round-tripped the JSON literal `null` into SQL NULL, changing data and breaking NOT NULL json columns. Not serialising at all fixes both. Verified against a real PostgreSQL 15: a migrated install now carries json_typeof = null for a JSON null, object for a nested object, and boolean for a boolean — matching a native install exactly. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): close review round 9 — probe error classes, marker ordering (#1038) 1. probePgData() answered every failure with "Postgres has data". That is right for an unreachable server — the app cannot run on it either way, and diverting a healthy pg install to a stale SQLite file over a transient blip would be worse — but wrong for a server that answers and then fails the query, which is what a half-built or damaged schema looks like. That is not evidence of data, and reporting it as such booted the empty Postgres and hid a populated SQLite file: the exact failure this guard exists to prevent. Reachability is now established with SELECT 1 first, so the two cases get opposite answers: unreachable → leave the configured engine alone; reachable-but-uninspectable → unproven, and the SQLite side wins if it actually holds data. 2. The success marker was written after the SQLite file was renamed away. A failure in between — a full disk — left the source retired with no marker: the next attempt reported "No SQLite database", the in-progress pin stayed, and the operator never saw the rollback path. The marker is written first and updated with the retired filename once the rename succeeds, so a failure at any point leaves everything recoverable. Verified against a real PostgreSQL 15: a reachable database whose admin_users table lacks the probed column now resolves to sqlite3 rather than hiding the data, while an unreachable host still resolves to pg. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): don't fail the migration on empty SQLite-only tables (#1038) Review round 10. The final verification flagged every source table missing from Postgres, regardless of whether it held rows — and SQLite-only tables do exist: initializeDatabase() creates an `events_new` scratch table and, when its legacy column copy throws, the catch swallows the error and leaves the empty table behind (db.js:236). The importer correctly skips tables Postgres does not have, so verification then reported a mismatch AFTER the data had already landed, exited 1, and left the install pinned to SQLite with no way to finish. An absent target table only matters if the source actually had rows. Empty ones are now listed and skipped. Reproduced both ways against a real PostgreSQL 15 with an events_new table present: without the fix the run ends in "ROW COUNTS DO NOT MATCH" and leaves the in-progress pin; with it, the table is reported as skipped, the migration completes and the pin is released. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): a completed migration overrides an implicit SQLite config (#1038) Review round 11. The migration allowed the one configuration it should have worried about most: DATABASE_CLIENT unset AND NODE_ENV not "production", which resolves to the development block — i.e. sqlite3. That is precisely the state the affected installs are in, since it is why they ended up on SQLite at all, so an operator can easily run the migration before fixing it. The script then renames the source database away, and the next start resolved to the implicit sqlite3, created a NEW empty database and served it — after reporting success. The success marker now overrides an IMPLICITLY resolved sqlite3 when Postgres settings are present, because the marker is durable proof of where the data actually went. An explicit DATABASE_CLIENT=sqlite3 still wins: that is the documented rollback. The script says something rather than refusing — refusing would block exactly the population this exists for. Reproduced with NODE_ENV and DATABASE_CLIENT both empty, against a real PostgreSQL 15: the migration completes, the source is renamed away, and the next boot resolves to pg with the data intact. Before this it resolved to sqlite3 and would have served an empty database. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * refactor(db): drop the dead reachability flag in probePgData (#1038) github-code-quality flagged `if (reachable)` as always true, and it is right: the unreachable branch returns, so everything below it runs only when the probe connected. The variable and the conditional were leftovers from a first draft that used a single catch for both failure classes. No behaviour change — the two error paths still return opposite answers. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): refuse to choose when both databases hold data (#1038) Review round 12. 1. An install that ran on PostgreSQL, lost NODE_ENV/DATABASE_CLIENT, and kept working on SQLite has REAL data on both sides: old rows in Postgres, newer ones in SQLite. The stranded-data rule only protected SQLite when Postgres was empty, so pulling this fix would have booted Postgres and hidden every gallery created since the switch — the exact failure this PR exists to prevent, in a variant I had not considered. A completed migration leaves a marker saying which side is current. Without one, two populated databases are a conflict: the boot stops and prints both targets, the two DATABASE_CLIENT values that resolve it, and the migration command that merges them. This is the only deliberate refusal in the change — guessing here would hide data AND split subsequent writes across two databases. 2. probePgData was handed knexConfig.connection even when knexfile had resolved to SQLite (a completed migration whose environment still says sqlite3), so node-postgres dialled its own localhost defaults instead of DB_HOST/DB_NAME — false "unreachable" diagnostics and a needless delay on every boot. The probe target is now built from the environment when the config is not pg. The conflict is honoured by all three entry points: the resolver exits 3 with an empty stdout, wait-for-db.sh stops the container, and server.js refuses to start. Two existing tests asserted that Postgres wins when both sides hold data. They encoded the pre-conflict assumption and described a state that cannot occur after a real migration (which always leaves a marker); both now pass the marker. Found while testing: the resolver's logger shim had no .error, so the conflict path threw, was swallowed by the fallback, and silently chose Postgres — the precise outcome this refuses to make. The shim is complete now. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): symmetric bootstrap rule, one resolved Postgres target (#1038) Review round 13. Both findings are consequences of earlier rounds. 1. The conflict rule added in round 12 counted an untouched SQLite bootstrap admin as data. core/001_init.js seeds one whenever ADMIN_PASSWORD is set — including into the accidental SQLite database — so a healthy Postgres install that had ever started once without NODE_ENV would have had a seeded-only SQLite file beside it, been declared a both-populated conflict, and REFUSED TO BOOT. The bootstrap discrimination is applied on both sides now; a setup-completed or logged-in admin still counts as real use on either. 2. The CLI's child phases inherited whichever knexfile block NODE_ENV selected. The development block defaults Postgres to localhost/postgres/photo_sharing, production to db/picpeak/picpeak — and this script is explicitly meant to run with NODE_ENV unset. With DB_USER/DB_NAME left to defaults it would therefore have migrated into `photo_sharing`, after which following the script's own advice to set NODE_ENV=production pointed the app at an empty `picpeak`. The target is resolved once, with production defaults, and passed explicitly to every phase — so the block knexfile happens to pick can no longer decide which database the data lands in. The pin and success marker record that same resolved identity. Verified against a real PostgreSQL 15: a live Postgres beside a seeded-only SQLite file now boots pg rather than refusing, flipping that admin to setup-completed restores the conflict, and a migration records localhost:7102/picpeak_r13b as its target rather than a defaulted guess. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): one Postgres identity everywhere; protect the credentials file (#1038) Review round 14. Three of the six findings were the same defect as round 13's, surfacing through paths that fix did not cover: the connection used to PROBE or MIGRATE could differ from the one the application then OPENS, because knexfile's development block points Postgres at localhost/postgres/photo_sharing while production uses db/picpeak/picpeak. 1. server.js exported only DATABASE_CLIENT=pg after the resolver decided, so knexfile filled in host/user/database from whichever block NODE_ENV selected. With SQLite already retired by a migration, that meant opening an empty database. The whole connection is pinned now. 2. Two defaults existed for DB_HOST: wait-for-db.sh resolves and exports `postgres`, knexfile's production block says `db`. Since the entrypoint exports its value, `postgres` is what a running container actually uses — so a `docker exec` migration, which inherits neither, has to agree with that, not with the default that is only reached when the entrypoint did not run. 3. The migration's Postgres phases inherited an unset NODE_ENV and therefore the development block, which ignores DB_SSL entirely — a managed Postgres requiring TLS could never be migrated into. The phases run with production semantics now. 4. core/001_init.js writes data/ADMIN_CREDENTIALS.txt, and that data directory belongs to the SOURCE install. Bootstrapping the Postgres schema replaced the operator's real credentials file with ones for a temporary admin the import immediately discards. The file is preserved across the phase, including when it fails. 5. The boot line described knexConfig, so an install redirected to Postgres by a migration marker still logged "Database engine: sqlite (...)", contradicting the warning printed one line earlier. 6. On a both-populated conflict resolveBootEngine returns client:null, and both migration runners told the operator their data was in "null" and to set DATABASE_CLIENT=null. They now present the two real choices. Verified against a real PostgreSQL 15: a migrated install started directly with NODE_ENV unset now logs `postgres (localhost:7102/picpeak_r14)` and opens it, where before it would have gone to the development block's photo_sharing. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * refactor(db): resolve the PostgreSQL target in exactly one place (#1038) Rounds 13 and 14 both traced back to the same thing, each time through a caller the previous fix had not covered: three different defaults existed for the same connection. knexfile development : localhost / postgres / photo_sharing knexfile production : db / picpeak / picpeak wait-for-db.sh : postgres / picpeak / picpeak (and it EXPORTS them) So a process that probed or migrated against one could hand over to a process that opened another. Patching each caller was not converging — the guard, then the CLI's child phases, then server.js — so this deletes the divergence instead. `src/utils/pgConnection.js` now owns the resolution and knexfile's development and production blocks both derive from it, as does the engine guard. Same shape as the earlier sqlitePath.js extraction, for the same reason. The database NAME is what made this dangerous: a wrong host or user fails loudly at connect time, while a wrong name connects fine and presents an empty installation. BEHAVIOUR CHANGE: with DATABASE_CLIENT=pg and no DB_* variables, a non-production environment now resolves to postgres/picpeak/picpeak instead of localhost/postgres/photo_sharing. Deployments are unaffected — compose sets these explicitly and wait-for-db.sh exports them — but a local machine running Postgres bare now needs DB_HOST=localhost DB_USER=postgres DB_NAME=photo_sharing (or DATABASE_CLIENT=sqlite3, which is what backend/.env already uses). The failure mode of getting this wrong is a refused connection, not a silently empty database. Side effect worth having: DB_SSL is now honoured whatever NODE_ENV says, so the managed-Postgres case is fixed at the root rather than by forcing production semantics onto the migration's child phases. The test block keeps its own photo_sharing_test default — isolation is the point there. Verified: every block plus the guard resolve identically from the same environment; explicit DB_* still wins; production's pool tuning is preserved; and a full SQLite → PostgreSQL migration with NODE_ENV unset lands in the right database with JSON shapes intact. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): two more components that guessed the database instead of asking (#1038) Both found while sweeping for copies of the connection defaults. Checked in detail first — one of my suspicions about them was wrong. scripts/set-admin-password.js hand-rolled its own knex config while all four sibling scripts (reset-admin-password, create-admin, show-admin-credentials, reset-admin-mfa) use the application's connection. Two consequences: - it read DB_CLIENT, a variable nothing else in this codebase sets, so it defaulted to Postgres and could not work on a SQLite install at all; - it defaulted to database `picpeak_dev`, a name no other component uses. It now uses `require('../src/database/db')` like its siblings, so it follows whatever engine the install actually runs on. Timestamps are written as ISO strings because it reaches SQLite now, where raw Date objects are the documented landmine. NOT changed: the script's "all existing sessions have been invalidated" notice is accurate — auth.js compares token iat against password_changed_at — and it deliberately leaves must_change_password alone, which is right for an operator choosing a password rather than being issued one. routes/adminSystem.js re-derived three things the live connection already knows, and each could disagree with it: - the engine, from DATABASE_CLIENT || 'sqlite3' — so a Postgres install without an explicit DATABASE_CLIENT took the SQLite branch; - the Postgres database, from DB_NAME || 'picpeak'; - the SQLite file, from a hardcoded ../../data/photo_sharing.db that ignored DATABASE_PATH entirely. All three now come from db.client.config, with pg_database_size(current_database()). Verified: set-admin-password works on SQLite (new hash verifies, old rejected) and still on PostgreSQL; and on a SQLite install with a custom DATABASE_PATH the size logic reports the real database (1,748,992 bytes) where the old code reported a different file entirely (1,851,392) — or 0 where that path does not exist. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM * fix(db): bind the migration marker to its target; fix a phantom table (#1038) Review round 15. 1. The marker records `host:port/database`, but only its EXISTENCE was checked. Repoint DB_NAME or DB_HOST at a different, empty PostgreSQL after migrating and the marker would vouch for that one too — booting it, presenting an empty installation, and suppressing the SQLite fallback while the real data sits in the recorded target and the renamed rollback copy. The marker is compared against the current connection now, and a mismatch stops the boot with both targets named and the two ways out. 2. `incoming_invoices` is not a table — supplier documents live in `inbound_documents` (core migration 124). Both occupancy lists skip tables that do not exist, so those records were silently not protecting anything: an install whose only remaining data was inbound documents could be switched away from, or overwritten without --force. Verified every other name in the lists against the live schema at the same time. Verified: a marker naming picpeak_original with picpeak_mk configured refuses with exit 3 and prints both; making them agree boots pg. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-08-13 18:51:20 +02:00
Paul NothaftandPaul Nothaft	fb3d0b08b2	fix(events): make event_date/expires_at nullable on SQLite (#1029) (#1036) ... Clearing a gallery's expiration failed on every SQLite install with SQLITE_CONSTRAINT: NOT NULL constraint failed: events.expires_at surfacing in the admin UI as "Failed to update event". Migration 061 added the event_require_event_date / event_require_expiration settings and dropped the NOT NULL on both columns — but only for Postgres. It skipped SQLite on the premise that "SQLite doesn't enforce NOT NULL as strictly", which is untrue, so "never expires" was never reachable there. The #426 work that allows clearing the expiration on edit therefore never worked on SQLite either. Migration 174 finishes 061 for SQLite. Knex implements .alter() on SQLite by recreating the table; migration 073 already does that on `events`, so the path is well-trodden. Postgres is skipped — it was handled in 061 and .alter() there would needlessly rewrite a column that is already correct. The test asserts against the real engine (the Jest harness runs SQLite) and reproduces the reporter's exact error without the migration. Claude-Session: https://claude.ai/code/session_0168gubtwYYacJv8weAjy8DM Co-authored-by: Paul Nothaft <[email protected]>	2026-08-13 18:51:09 +02:00
Paul NothaftandPaul Nothaft	fecc18cbc8	fix(security): enforce project ownership on project + project-email routes (stable) (#966) ... * fix(security): enforce project ownership (GHSA-wrg5, GHSA-93x4) Project routes authorized on generic events.view / events.edit with NO ownership check, so an editor-like admin could enumerate, read, update and aggregate projects belonging to other admins' events. The project email endpoints keyed on an email_queue id alone — any admin with events.view / email.send could preview, resend, cancel or retry ANY queued mail by walking ids. The earlier 'needs a migration, deferred' assessment was wrong in one direction and right in another: ownership IS derivable transitively via events.project_id -> events.created_by, but only for projects that already have a linked event. A brand-new EMPTY project has no derivable owner, which is exactly where the create -> attach flow starts. So migration 167 adds projects.created_by (backfilled from the single linked event owner, skipping ambiguous multi-owner projects) and createProject finally persists the adminId it was already being passed. - ownedProjectIds(): union of the stored owner and the transitive path, so pre-167 rows and new empty projects both resolve. Reads created_by defensively so an instance that hasn't run 167 falls back to the transitive rule instead of throwing. - requireProjectOwnership on detail/update/attach-event/attach-quote/ attach-contract/overview; list filtered by an id allowlist (empty array means 'owns nothing' and must return no rows, hence null-vs-[] care). - POST /:id/events also validates the INCOMING eventId — owning the project is not enough, or an editor could pull a foreign event in and read its rolled-up documents via /:id/overview. - Queued-email routes scoped via email_queue.event_id. CRM document mail has event_id NULL and no ownable parent here, so a scoped caller is denied rather than guessed into access. 404 (not 403) so it isn't an id oracle. Note: adminEmail.js:315/332 let any email.view/edit admin archive or delete any email_queue row — the same class, pre-existing and outside these two advisories. Left untouched and reported rather than silently widened. * fix(security): codex round 2 — make the stored project owner authoritative (GHSA-wrg5) The first predicate union'd 'any linked event I can see' with the stored owner, which opened two holes: - A project owned by admin B containing ONE legacy ownerless event became readable by every admin — and /:id/overview aggregates B's other events, invoices and emails, so a single legacy event exposed the whole project. - Migration 167 deliberately leaves multi-owner (ambiguous) projects NULL rather than guessing an owner. A NULL owner was then treated as 'everyone's', so exactly those mixed projects became globally accessible. Now: the stored created_by wins outright, and a project without a usable stored owner only derives access when EVERY linked event is accessible (and at least one exists). A created_by pointing at a hard-deleted admin degrades to 'no usable owner' so the project falls back to its events instead of being locked away — no ON DELETE SET NULL migration needed. A project with neither a usable owner nor linked events stays super_admin-only: failing closed beats failing open, and a super_admin can reassign it. Also returns a knex SUBQUERY rather than a materialised id list, so a large project count can't hit the driver's bind-parameter limit. * fix(security): codex round 3 — enforce deal-lineage ownership on project attach (GHSA-wrg5) requireProjectOwnership vets only the DESTINATION project, while attaching a quote or contract cascades through linkDealToProject — which re-points every event the deal produced into that project. An editor could therefore create an empty project of their own, attach another admin's quote, and pull that admin's events (plus the invoices, emails and gallery that roll up with them) into a project they own and can read via /:id/overview. The single-customer guard did not stand in the way: an unassigned project ADOPTS the deal's customer rather than rejecting it. linkDealToProject now refuses to move lineage events the actor cannot own, and assignDocument cascades BEFORE stamping the document so a refused attach leaves nothing half-applied (the old order committed the foreign document into the caller's project and only then declined the cascade). The quote/contract create+update paths, which reach the same cascade with an arbitrary project_id, thread their adminId through as well; isSuperAdmin() resolves the role for them and fails closed when it cannot. Events are the only ownership signal a deal carries — quotes and contracts have no created_by in this schema — so a lineage that produced no event still cannot be attributed. That is a property of the CRM model, noted in the code. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me (cherry picked from commit 688e318850db1b5f4ea2a4ae3c0fcf0fc137620d) * docs(security): drop the stale ownership JSDoc left by the rebase (GHSA-wrg5) Rebasing onto stable (which had gained scopeEventsQuery from #963) replayed the round-1 doc block above round-2's replacement, leaving a comment that describes the ORIGINAL union rule — "a project is the caller's when … it has at least one linked event they own" — directly above the code that deliberately no longer does that. That union is the hole round 2 closed; a comment asserting it is worse than none. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-08-02 21:24:41 +02:00
Paul NothaftandPaul Nothaft	11f9f584de	fix(security): scope dashboard stats/analytics/activity to the caller's events (stable) (#964) ... * fix(security): scope dashboard endpoints to the caller's events (GHSA-c2jj, gqx7, jhcf) /dashboard/stats, /analytics and /activity are gated only by analytics.view, which the editor role holds — but the events LIST restricts editors to their own rows (adminEvents/crud.js: roleName === 'editor' -> created_by = admin.id). So an editor saw instance-wide totals, and via /analytics topGalleries other admins' gallery NAMES and SLUGS (the public gallery URL component), for events invisible to them everywhere else. - stats: all 10 aggregates scoped (events by id, photos/access_logs by event_id). - analytics: all 8 series/aggregates scoped, including topGalleries. The external tracker device breakdown reports instance-wide data with no event filter, so a scoped caller falls through to the access_logs heuristic instead, which IS scoped. - activity: feed scoped. activity_logs.event_id is nullable and the join is a leftJoin, so system-level rows (logins, settings changes) are deliberately excluded for a scoped caller — those are precisely the cross-admin actions the advisory is about. Scoping keys on 'editor' to mirror the events list exactly, so the admin role's dashboard is unchanged. filterOwnedEventIds uses the broader '!== super_admin' rule; the two conventions disagree in this codebase and matching the list is the no-regression choice. * fix(security): codex round 2 — fix activity misattribution, scope via subquery (GHSA-jhcf, c2jj, gqx7) - expenseService passed adminId as logActivity's THIRD positional parameter, which is eventId — so admin ids were being written into activity_logs.event_id. The /activity scoping filter trusts that column, and admin/event id sequences overlap, so a foreign admin's expense metadata could surface under an editor's event. All 11 calls now pass null for eventId and the admin as the actor, which is what they meant. - Dashboard scoping now uses a SUBQUERY instead of pluck()+whereIn. An editor owning more events than the driver's bind-parameter limit (~999 SQLite, 65535 Postgres) would have turned all three endpoints into 500s once each id became a placeholder; below the limit it still re-sent the full list for each of the ~10 aggregates per request. Note: two billInboundNow() calls also end in ', adminId)' but have an unrelated signature — verified untouched. * fix(security): codex round 3 — correct legacy accounting activity rows (GHSA-jhcf) expenseService called logActivity(type, metadata, adminId), but logActivity's third positional parameter is eventId. Every expense / incoming-invoice entry therefore stored the ACTING ADMIN'S ID in activity_logs.event_id. Round 2 scoped the activity feed with `WHERE activity_logs.event_id IN (SELECT id FROM events WHERE created_by = me)`, which does nothing about the rows already on disk. Admin ids and event ids are small integers from the same range, so on any upgraded instance an editor who owns the event whose id happens to equal another admin's id is served that admin's accounting activity, verbatim metadata included — GHSA-jhcf, still live. Migration 168 re-attributes those rows (event_id holds exactly the actor id that was lost) and then clears event_id so the scope predicate can no longer match them. All ten activity types are emitted by expenseService and nothing else, so no row with a genuine event_id is touched. Also: the round-2 rewrite passed `{ type: 'admin', id: adminId }` unconditionally, which stored actor_type='admin' with a null id for the automated mailbox intake (emailIntakeService calls recordInboundDocument with no adminId). adminActor() restores 'system' attribution for those. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me (cherry picked from commit 459e9e42434defd0dc7b87246e4d894dd47dcc56) --------- Co-authored-by: Paul Nothaft <[email protected]>	2026-08-02 21:18:46 +02:00
Luca	99d5996561	feat(messages): search bar + Archive/Delete with Archived & Deleted folders ... - Search box in the header filters the current folder's list (sender/subject), client-side; works across the merged Archived/Deleted views too. - Archive and Delete are now implemented as soft moves: migration 157 adds mailbox_state ('active'|'archived'|'deleted') to email_queue + received_emails. Archive → 'archived', Delete → 'deleted' (trash). Restore → 'active'. Deleting FROM the Deleted folder is permanent (hard row delete). - New cross-account system folders Archived + Deleted (merge sent + received of that state, sorted by date). Normal folders now exclude archived/deleted. - Backend: /queue + /received gain a `state` filter (default active + legacy NULL); new POST /item/:kind/:id/state (archive/delete/restore) and DELETE /item/:kind/:id (purge, email.edit). - Toolbar Archive/Delete wired; Restore + "Delete permanently" shown in the system folders. Frontend build + migration boot (157) verified.	2026-07-07 16:16:51 +02:00
Luca	f9c2b4ed75	fix(messages): dynamic addresses, branding accent, compose/sync, per-identity SMTP ... Addresses dev-test feedback: - Sidebar + reading-pane addresses (rechnungen@ / hello@ / no-reply@) are now read from the mail config via GET /admin/email/identities, not hardcoded. - Highlight/selection now uses the branding accent (bg-accent-soft / text-on-accent-soft / accent-dark) instead of hardcoded blue, so it follows the admin's CI colour like the sidebar. - Header gains "New message" (compose) and "Sync" (poll mailboxes now) buttons. - Composer modal enlarged (920px, taller editable body). - Customer mailbox (hello@) now has BOTH incoming (IMAP) and outgoing (SMTP) settings — migration 156 adds smtp_* + from_* to mail_accounts; emailProcessor.sendRawEmail takes an accountKey and sends via that mailbox's SMTP identity (falls back to the global from). Manual/reply sends from the Messages UI use the 'customers' identity, so replies come from hello@. Frontend build + migration boot (156) verified.	2026-07-07 11:58:41 +02:00
Luca	768e84711f	feat(messages): Phase 3 — editable-template composer, reply + create actions ... The CRM action buttons and Reply now open a send-composer, not a silent templated send. - New send-composer (MessageComposer): loads the rendered template (via previewTemplate) or a reply stub into a fully-editable body — the admin can rewrite it or drop a note anywhere before sending. On send it goes out as-is (server-sanitized), no template re-render. - Backend: emailProcessor.sendRawEmail() sends admin-edited HTML via the configured SMTP identity; POST /admin/email/send sanitizes + sends + records the message in email_queue as a 'manual' send. - Migration 155: email_queue.origin ('system' default | 'manual'). The Sent stream now splits by origin — Automated ▸ Sent = system, Customers ▸ Sent = the human/edited messages (which finally populates that folder). /queue gains an origin filter + returns origin. - Toolbar wired: Reply enabled on inbound customer mail (prefilled + quoted); Create Quote/Contract/Invoice open the composer with that template loaded; Gallery opens a blank compose. Accounting/Forward/Archive/Delete stay disabled (later phases). After send, jumps to Customers ▸ Sent. Deferred to a later phase: two-way IMAP write-back; per-identity SMTP (manual sends currently use the global from address). Frontend build + migration boot verified.	2026-07-07 10:42:54 +02:00
Luca	ee46cf2125	feat(messages): Phase 2 — customer (hello@) mailbox + inbound body capture ... Second inbound mailbox and real message bodies for the Messages viewer. Backend: - Migration 154: mail_accounts table (additional inbound mailboxes beyond the primary accounting IMAP) + received_emails.{account_key,to_address,body_html, body_text}. Additive/guarded. - emailIntakeService now polls the accounting mailbox AND every enabled mail_accounts row. Extracted pollAccountOnce(cfg, {accountKey, routeToExpenses}); accounting keeps its exact attachment->expenses behavior, customer mail is logged with its body and NOT routed to accounting. Inbound HTML is sanitized server-side (sanitize-html) on ingest. - adminEmail: /received gains an account filter + returns account_key/to_address (bodies excluded from the list); new GET /received/:id returns the body; GET/POST /accounts + /accounts/test manage the extra mailboxes. Frontend: - Customers inbox now pulls the hello@ mailbox; reading pane renders the sanitized body in a strict (script-less, no same-origin) sandboxed iframe. Accounting inbox shows bodies too. Toolbar context keys off the mailbox. - CustomerMailboxCard in Settings -> Email (behind the messaging flag) to configure + test the hello@ IMAP box. No behavior change to the existing accounting inbound flow. Frontend build + migration boot verified.	2026-07-07 10:11:58 +02:00
Paul Nothaft	60b03b1728	fix(branding): unify hero logo SIZE the same way as visibility (#756) ... Follow-on to the visibility fix in this PR — hero_logo_size had the same split-brain: GalleryLayout read the global branding_logo_size live while the hero-header path used the per-event snapshot, so a hero logo could render at different sizes on different layouts and the global size didn't reach hero-header galleries. Now mirrored on the visibility model: NULL per-event hero_logo_size = inherit branding_logo_size; explicit = override. - Migration 153: hero_logo_size nullable + backfill NULL so existing galleries inherit the global size (restores GalleryLayout's prior live-global behaviour and fixes the hero-header staleness). - Creation stores NULL unless explicit; gallery.js resolves per-event ?? global and sends the effective size. - GalleryLayout now consumes that resolved size for the hero logo (new heroLogoSize prop) instead of the global — both render paths match. - Admin size control gains a 'Use branding default' (inherit) option. Verified: migration on SQLite + PG; live resolution (inherit follows global both ways, override wins); creation stores NULL on PG; tsc clean, 106 adminEvents+gallery tests pass, build green.	2026-07-06 11:44:33 +02:00
Paul Nothaft	96fe478bf8	fix(branding): make 'Show logo in hero' a true global toggle with per-event override (#756) ... Before: the global branding_logo_display_hero toggle was only a creation-time default — snapshotted into each event's hero_logo_visible column at creation and never consulted again. Disabling it did nothing to existing galleries (the reporter's bug), and the two gallery render paths disagreed (GalleryLayout read the global, HeroHeader read the per-event snapshot). Now: NULL per-event hero_logo_visible = 'inherit the global toggle'; an explicit true/false is a per-gallery override. - Migration 152: make events.hero_logo_visible nullable and NULL out the defaulted rows so existing galleries follow the global going forward. Deliberate per-gallery hides () are preserved. - Creation stores NULL unless the admin explicitly sets it; the update path preserves NULL. - gallery.js resolves per-event ?? global (branding_logo_display_hero, default true) and sends the EFFECTIVE value on both gallery responses. - Both frontend render paths now consume that resolved value (GalleryLayout gets it via a new heroLogoVisible prop). - Admin per-event control is now tri-state: Use branding default / Always show / Always hide (en + de). Verified: SQLite migration + live resolution (inherit follows global both ways; override wins both ways); PG migration SQL dry-run; the admin tri-state renders 'Use branding default' for an inherited event; tsc clean, 106 adminEvents+gallery tests pass, build green.	2026-07-06 11:20:36 +02:00
Paul Nothaft	72e2ef6721	feat(auth): admin TOTP MFA — enrollment, login challenge, recovery, CLI reset ... Backend for #738. Real TOTP 2FA for admin accounts, all roles incl. super_admin (closes #735). - mfaService: otplib TOTP; AES-256-GCM encryption of the secret at rest (key derived from MFA_ENCRYPTION_KEY or JWT_SECRET); bcrypt-hashed, single-use recovery codes; otpauth URI + QR. - Migration 151: adds two_factor_recovery_codes + two_factor_enrolled_at (secret/enabled columns already existed from legacy 016). - Enrollment endpoints (behind adminAuth, per-user): GET /mfa/status, POST /mfa/{setup,enable,disable,recovery-codes}. Disable/regenerate require a current code so a hijacked session can't strip 2FA. - Login challenge: /admin/login returns {mfaRequired, mfaToken} (no session) when 2FA is on; /admin/login/mfa exchanges a TOTP or recovery code for the session. Lockout counter is NOT reset until the second factor passes, so MFA brute-force is rate-limited too. - CLI break-glass: scripts/reset-admin-mfa.js --email <e> | --all --yes, audit-logged, matches reset-admin-password.js convention. - Docs + optional MFA_ENCRYPTION_KEY env. Verified end-to-end on a live backend: enroll (super_admin), challenge, TOTP + single-use recovery login, disable, and CLI reset.	2026-07-03 11:33:38 +02:00
Luca	415bffa04c	feat: zero-config first run — in-browser admin bootstrap + auto-generated secrets ... Fresh installs need nothing in .env. See PR description for the full feature.	2026-07-01 14:49:18 +02:00
Paul Nothaft	56c2386c90	feat(gallery): branded URL shortener — /s/<slug> with OG injection (#699) ... Issue 3 from #699 (@alexvaltchev's report): expose a custom-named short URL per event that bots scrape for OG previews and browsers redirect to the underlying gallery. WhatsApp / iMessage / Facebook cache the OG metadata by the URL they crawl, so the SHORT URL becomes the cache key — admins can rotate or split-test underlying gallery URLs without re-pushing a fresh link to clients. Additive feature; no existing route, table, or column is modified. ## Backend - `gallery_short_urls` table (migration 150): id, short_slug UNIQUE, event_id FK CASCADE, target_path TEXT, created_by/at, hit_count, last_hit_at, deleted_at/by. hasTable-guarded so the migration is idempotent on re-run. - `src/services/galleryShortUrlService.js` — validator + CRUD + resolver. Slug rules: `/^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$/`, reserved blocklist (admin, api, auth, gallery, og, s, login, ...). target_path snapshots at create-time from the event + global short-URL toggle, so a later flip of the toggle does NOT silently change where existing short URLs resolve. - `src/routes/adminShortUrls.js` — `GET/POST /api/admin/events/:eventId/short-urls`, `DELETE /api/admin/short-urls/:id`. Structured errors: 400 INVALID_SLUG, 409 SLUG_TAKEN (with `suggested`), 404 EVENT_NOT_FOUND. Gated by events.view / events.edit + requireEventOwnership. - `server.js` /s/:shortSlug public route. Bot UA → server-render the same OG metadata the existing /og/gallery/<slug> handler produces, then override og:url to point at /s/<shortSlug> itself (cache-key invariant — social platforms key by the URL they scrape). Browser UA → 302 to target_path. Soft-deleted slug → 410 Gone (intentional-delete signal, distinct from 404 unknown slug). Hit accounting is fire-and-forget. ## Frontend - `services/shortUrls.service.ts` — list/create/remove. - `components/admin/ShortUrlsCard.tsx` — per-event card on the EventDetailsPage. Form for custom or auto-generated slug, list with copy-to-clipboard + soft-delete. SLUG_TAKEN error surfaces the service's `suggested` slug with a "use suggested" button. - i18n: events.shortUrls.* added to EN + DE. ## Tests 78 new tests, all passing: - `__tests__/utils/galleryShortUrlValidation.test.js` (48) — pure- function tests for validateSlug: accepts/rejects, reserved-slug blocklist, path-traversal + URL-injection vectors. - `__tests__/integration/galleryShortUrls.test.js` (19) — service layer against a real SQLite DB. Covers custom + auto-generated slugs, collision + SLUG_TAKEN + suggested, target_path snapshotting (backward-compat invariant), soft-delete + slug rotation, hit counting. - `__tests__/integration/galleryShortUrlRoute.test.js` (11) — HTTP-level: 302 redirect for browser UA, 200 + OG HTML for bot UA, og:url canonical points at /s/<slug>, 410 for soft-deleted + orphaned events, 404 unknown + malformed. Regression sweep: 47 existing migration-chain integration tests still pass; migration 150 is additive only. ## Backward compatibility - Existing `/gallery/<slug>`, `/gallery/<32-hex-share-token>`, `/gallery/<slug>/show/<token>`, `/og/gallery/<slug>`, `/og/gallery/<slug>/cover` routes are untouched. - The `/s/` namespace is new; no existing route lives there. - Migration 150 only ADDs the new table — no ALTERs on existing schema, no destructive changes. - target_path is snapshotted at create-time so flipping the global "Use short gallery URLs" setting after a short URL exists does NOT change where that short URL resolves.	2026-06-30 16:30:13 +02:00
Paul Nothaft	0205c7dcce	chore: migrate Docker registry + GitHub URLs to PicPeak org ... Repo transferred from the-luap/picpeak → PicPeak/picpeak. Docker images publish to ghcr.io/picpeak/picpeak/{backend,frontend} (lowercase, per the GHCR canonical form computed by docker-build.yml's `${GITHUB_REPOSITORY,,}`). Sweep covers: - docker-compose.production.yml + Dockerfiles → new image registry path - README, CONTRIBUTING, SECURITY, SIMPLE_SETUP, scripts/picpeak-setup.sh → new GitHub URLs - Update-check / release-notes services (updateCheckService, environmentService, updateNotificationService, adminSystem, UpdateNotification, githubReleaseUrl) → GitHub API + tag URLs use the canonical PicPeak/picpeak path - Issue templates + README-DOCKER + workflow README → updated package URLs - One commit-context comment in migrations/090 + customerAccountsService CHANGELOG.md is intentionally untouched (historical release entries are immutable; GitHub auto-redirects the old URLs indefinitely). CLAUDE.md keeps the bare `(the-luap)` reference — that's the maintainer's personal handle, not a repo URL. 22 files, 48/48 line swaps (every change is a 1:1 URL replacement).	2026-06-29 20:20:13 +02:00
Luca	539a83711d	fix(workflows): defer quote.accepted/declined emit until the 15-min response window locks ... The customer's accept/decline can be toggled for crm_quotes_accept_window_minutes (default 15) before it locks, and the public page promises exactly that. But the booking workflow fired on the FIRST accept click and immediately converted the quote (status -> 'converted'), so a decline within the window was rejected ('Quote cannot be responded to in status converted') — the grace period was dead on arrival. recordResponse / adminAcceptQuote now DEFER the workflow emit while the toggle window is open; the new scheduler sweep finalizeQuoteResponses fires the FINAL status once response_locked_at passes (idempotent via the new quotes.workflow_response_emitted_at column, migration 149). A response recorded with the window already closed (0-min window, or admin decline which locks immediately) still emits inline. So toggling accept->decline->accept inside the window converts at most once, for the final state, after the customer's grace period — and a plain decline never converts. Trade-off: with the hourly CRM scheduler, the booking flow now starts up to ~1h after the window locks instead of instantly. Acceptable — the flow gates on admin review anyway, and the alternative (graph-level wait) wouldn't reach already- enabled built-ins (admin_toggled_at blocks re-seed). Adds a finalize sweep test (deferred while open, fires + stamps once locked, idempotent).	2026-06-28 14:30:26 +02:00
Luca	5893ecb27a	fix(workflows): ship built-ins disabled for first beta + enabled-based mutex + admin sentinel ... Per review: the four cutover built-ins (dunning, gallery_expiring, gallery_expired, pre_event_email) now ship enabled:false. The mutual-exclusion guards revert to ENABLED-based (isBuiltinFlowActive, not existence) so the legacy paths keep running until the admin enables a built-in — enabling cuts over, disabling reverts to legacy (fixes concern #4's "disable = silent dark" foot-gun; no automation goes dark on upgrade). admin_toggled_at sentinel (migration 148) marks admin ownership; the boot re-seeder applies a shipped default-flip (enabled→disabled) only to never-touched built-ins and never overwrites an admin's enable/disable/edit (nit #1). SEED_VERSIONs bumped so the disabled default propagates. Nit: applyReminder unlinks the just-rendered Mahnung PDF if queueEmail throws (no orphan file).	2026-06-23 23:35:56 +02:00
Luca	d14f1d850c	feat(workflows): per-quote booking-workflow picker + quote→invoice (no gallery) built-in ... A quote can now choose which flow runs on acceptance instead of every enabled quote.accepted flow firing. Migration 147 adds quotes.booking_workflow_id; the editor shows a "Booking workflow (on acceptance)" dropdown listing the quote.accepted flows (workflow-engine flag only); emitQuoteEvent passes it as the new emitWorkflowEvent targetWorkflowId so ONLY the picked flow runs (still gated on enabled + trigger match → a disabled/None selection runs nothing). Adds the booking_invoice_only built-in (quote.accepted → prepare invoice → review gate → send; no event/gallery, no wait), the variant requested for shoots billed without an online gallery. Disabled stub like the other booking flows until the prepare_*/send_document cutover. Tests: targetWorkflowId runs only the selected flow; invoice-only built-in has no wait/prepare_event.	2026-06-23 23:15:31 +02:00
Luca	f78671fc6c	feat(crm): event-type dropdown on quotes; quote→event uses it (no more hardcoded 'wedding') ... Quotes now carry an event type (migration 146: quotes.event_type, the event_types.slug_prefix), chosen from the active event-types catalog in the quote editor's Event section. convertToEvent reads it instead of the unconditional hardcoded 'wedding': quote.event_type → crm_default_event_type setting → 'wedding' as last-resort seeded fallback. When the booking flow's prepare_event is wired, it reads the same field. Backend: createQuote/updateQuote persist event_type (hasColumn-guarded); adminQuotes route accepts + returns eventType. Frontend: FormState + payload + load + a catalog-sourced dropdown ("— Use default —"); EN/DE strings.	2026-06-23 17:52:50 +02:00
Luca	192d2cbc06	feat(workflows): crash recovery — resume runs orphaned mid-flow ... Closes the crash-safety gap: a run left in running/pending by a crash had nothing to resume it (the scheduler only wakes 'waiting'). Adds a heartbeat (workflow_runs.updated_at, stamped on every node advance + start/resume) and a recoverStaleRuns() sweep that re-enters runs whose heartbeat has gone stale (>10 min) from their persisted node. Runs on the scheduler tick AND the boot tick, so a restart catches anything stranded during downtime. Re-entry is at-least-once (the current node may re-execute) — loop counters + the late-fee math are idempotent, so the only residual risk is a duplicate reminder email. An attempts counter (migration 145, cap 5) marks a run failed instead of recovering a node that reliably crashes the process (crash-loop backstop). Flag-gated. Tests: orphan-resume + crash-loop cap.	2026-06-23 13:39:17 +02:00
Luca	5ed2fec2fe	feat(crm): Mahngebühr on a separate Mahnung document; invoice stays immutable ... Corrected dunning model (Mara): a Mahnung is a reminder LETTER showing the new total (original + Mahngebühr), NOT a separate invoice and NOT a mutation of the issued invoice. - The invoice PDF no longer shows the fee (buildInvoiceRenderContext reports lateFeeAmountMinor 0) and is NEVER re-rendered by a reminder — it stays immutable (§14/§11). - applyReminder now: tracks the fee as dunning state on the row (gross late_fee_amount_minor + new late_fee_vat_minor for the VAT portion, migration 144), renders a separate MAHNUNG PDF (pdfService 'mahnung' kind — reuses the invoice layout: same lines + Mahngebühr row + new total, 'Mahnung' title, no QR), stored under storage/business-docs/mahnung/, and attaches BOTH the unchanged original invoice + the Mahnung to the reminder email. - Fee resolvers split into net + VAT-rate (toggle + org-rate gated); a gross wrapper feeds the payment-check preview. en + de PDF title. Outstanding/collections still read late_fee_amount_minor (now dunning state). P3 (tax-report/Banana booking of the Mahngebühr VAT) stays Treuhänder-gated. Syntax + 17/17 workflow/invoice tests green. NOTE: the Mahnung PDF render path isn't unit-tested (PDF rendering is flaky in the test env) — eyeball on the dev box: fire a level-2 reminder, confirm the Mahnung PDF shows the new total and the original invoice PDF is unchanged.	2026-06-23 13:08:19 +02:00
Luca	eaceb7e71c	feat(crm): toggle for VAT on late fees (jurisdiction-dependent) ... Mahngebühr VAT differs by country (CH: liable; DE/AT: not), so it's now a toggle (crm_invoices_late_fee_vat_enabled, seeded into migration 143 in place since it isn't deployed yet — no compensation migration). When on, VAT is added on top of the net fee at the org's default rate (business_profile.vat_rate_default). Gated so it's a NO-OP when the org doesn't charge VAT (default rate 0/unset) — i.e. enabling the toggle on a non-VAT org adds nothing, as required. Settings UI: a self-documenting checkbox. The fee is treated as net + VAT-on-top; the tax-report VAT breakdown for the fee is part of the deferred dunning-document rework. tsc 0, build green, 9/9 workflow tests.	2026-06-23 12:41:44 +02:00
Luca	dcdbeb9cc5	feat(crm): 3-reminder dunning + flat/percent Mahngebühr on 2nd & 3rd + AGB notice ... - Late fee can now be a FLAT amount OR a PERCENTAGE of the invoice gross (crm_invoices_late_fee_type/_percent, migration 143; defaults preserve the current flat behaviour). - Fee is charged from the 2nd reminder onward and accumulates per fee-bearing reminder (2nd = 1×, 3rd = 2×), computed from the level so re-applying a level never stacks. New resolvePerReminderFeeMinor() shared by applyReminder + the payment-check fee preview. - Reminder ladder extended to 3 levels (caps raised in sendReminder + recordPaymentCheckAction); the built-in dunning flow now loops 3× (seed v3, re-seeds the disabled built-in on boot). - Settings UI: flat/percent toggle + percent field, and a prominent AGB callout — a late fee is only enforceable if the concrete amount is stated in the terms (Mara's wording), 'verify with your Treuhänder'. en + native de. The fee math is examples-only / Treuhänder-verify; issued invoices stay immutable (the fee is tracked in late_fee_amount_minor, not folded into the original total). Tests 17/17, tsc 0, build green.	2026-06-23 12:26:32 +02:00
Luca	c818c25cf2	feat(workflows): schema + permissions (migration 142) ... Adds the workflow engine's graph data model — workflows, workflow_nodes, workflow_edges (versioned so in-flight runs keep their version), workflow_runs (status/current_node/context, wake_at for the scheduler, unique dedup_key for idempotency), workflow_run_steps (per-node audit), and workflow_approvals (hashed email confirm/deny token + webview inbox). Seeds workflows.view / workflows.manage and grants them to super_admin + admin. Loose-FK integers per the whatsapp_queue/expenses convention; idempotent hasTable guards + reversible down().	2026-06-23 01:57:06 +02:00
Paul Nothaft	f2814e4a4c	feat(feedback): per-guest favorite + like caps with mobile-friendly limit modal (#655) ... Reporter @Duecki1 wants to stop telling guests "pick only 5 photos" by hand. Per-event cap, enforced server-side, with a clear popup when the 11th click would exceed the limit. Per-guest scope matches the "every couple picks their top 10" mental model; per-gallery aggregate is explicitly NOT in scope (creates weird "first 10 visitors use up all slots" race conditions). ## Schema (migration 141) Two nullable columns on `event_feedback_settings`: - `max_favorites_per_guest` - `max_likes_per_guest` null / 0 = unlimited (preserves current behaviour for every existing install — operator must opt in). Both shipped together because the code path is identical; photographers can cap either, both, or neither. ## Backend - `feedbackService.submitFeedback` cap check on the INSERT branch only. Toggle-off (un-favoriting) is always allowed, so a guest at 10/10 can free a slot by un-clicking an existing favorite. - New `countGuestFeedback(eventId, type, guestId, guestIdentifier)` — matches the exact same guest-key shape the existing duplicate-check uses (guest_id when present, fallback to guest_identifier in simple identity mode). - Limit reduction grandfathers: admin lowering 20→10 keeps existing rows in place; new adds blocked until the guest removes some. - Route layer (`galleryFeedback.js` POST) translates a `limit_reached` service-return into a structured 403 with `code: 'FAVORITE_LIMIT_REACHED'` / `'LIKE_LIMIT_REACHED'`, `limit`, and `current_count`. Stable UI contract. - `feedback-settings` GET exposes the caps so the gallery UI can optionally render a counter near the heart icon (UI extension TBD; the modal alone is the contract this PR commits to). - `feedbackValidation`: range guard `0..10000`, null allowed, per-field error messages. ## Frontend — the popup New `FeedbackLimitReachedModal` component renders via a `createPortal` to `document.body` so it escapes any lightbox / sticky parent stacking context and reliably sits above everything else. Mobile-first responsive: - `items-end sm:items-center` — slides up from the bottom on phones (native action-sheet feel), centers on desktop (familiar modal). - `w-full sm:max-w-md` — full-width on phones, clamps to 420px on desktop. - `rounded-2xl sm:rounded-xl` — more rounded on phones for the sheet feel. - `pb-[env(safe-area-inset-bottom)]` — respects the iOS home indicator and Android gesture bar. - `z-[60]` — above the lightbox's z-50. Title + body + "8 of 10 used" pill + "Got it" button. Backdrop click + Escape both dismiss. Focus management lands on the OK button so keyboard / screen-reader users can dismiss immediately. New `useFeedbackLimitModal()` hook is the shared API: components on every submit-feedback site wire `onError: (err) => handleError(err)` and render `{limitModal}` in their JSX. Returns `true` from `handleError` when the error is a structured cap-reached 403 (so the caller can skip its generic error toast). PhotoFavorites + PhotoLikes + PhotoLightbox all wire through the hook — every favorite/like submit path is covered, including the lightbox's three different submit sites (guest mode, simple mode, post-identity-modal-confirm). ## Admin UI `FeedbackSettings` card gets a new "Per-guest limits" section that only renders when at least one of `allow_favorites` / `allow_likes` is on. Two numeric inputs (0 / empty = unlimited) side-by-side on desktop, stacked on mobile. Hint text covers the limit-reduction grandfathering semantics so admins aren't surprised. ## i18n EN + DE for: - Modal title + body (parameterized with `{{limit}}`) - Counter pill (parameterized with `{{current}}` / `{{limit}}`) - OK button label - Admin field labels + hints + section header + grandfathering note ## Tests **Backend** (`__tests__/utils/feedbackPerGuestLimit.test.js`, 8 cases): - null cap → unlimited (back-compat) - 0 cap → unlimited (UI convenience) - cap=10: rows 1-10 succeed, 11 returns limit_reached - toggle-off frees a slot at the cap - limit reduction grandfathers existing rows - per-guest scope: guest A's cap doesn't affect guest B - favorite cap doesn't block likes (per-type) - like cap returns LIKE_LIMIT_REACHED-shaped payload **Frontend** (`__tests__/useFeedbackLimitModal.test.ts`, 7 cases): - Non-axios errors → null - Non-403 axios errors → null - 403 with wrong code → null - FAVORITE_LIMIT_REACHED parsed - LIKE_LIMIT_REACHED parsed - Falls back to code-implied type when feedback_type missing - Missing numeric fields → 0 (not NaN) All 15 pass. tsc --noEmit clean. eslint clean on changed files. Closes #655.	2026-06-22 22:02:13 +02:00
Paul Nothaft	80e8ec5bc7	Merge pull request #650 from the-luap/fix/whatsapp-template-params-647-followup ... feat(whatsapp): admin-selectable template parameters + reorder (#647 follow-up)	2026-06-21 21:35:09 +02:00
Paul Nothaft	cde028e919	Merge pull request #649 from the-luap/fix/branding-customcss-preset-drop-645 ... fix(branding+whatsapp): preserve customCss through preset switches (#645) + admin-pinned WhatsApp template language (#647)	2026-06-21 21:33:29 +02:00
Paul Nothaft	1f46a241d2	chore(whatsapp): renumber migration 138 → 140 (after PR #646's 138+139) ... PR #646's review-round renumbered its slideshow migrations to 138 + 139 to slot in after PR #649's 137 (whatsapp_template_language). That now collides with this PR's 138. Slide ours to 140 so all three land in strict order: #649 (137) → #646 (138, 139) → this PR (140). Content unchanged; pure rename + a one-line docstring tweak noting the slot.	2026-06-21 21:12:22 +02:00
Paul Nothaft	16055cdc41	feat(whatsapp): admin-selectable template parameters + reorder (#647 follow-up) ... Reporter @Rekoo-PS confirmed the language fix unblocked sending, then hit a second gap: their template uses only `{{1}} = event_name` + `{{2}} = gallery_link`, but the legacy `buildComponents` hardcoded all 5 positional values from the `gallery_ready` shape (customer_name, event_name, gallery_link, password_line, expiry_date). Meta rejected with a parameter-count mismatch even after the language matched. This adds a per-config slot list — which built-in values to send, and in what positional order — so admins can match templates of any shape without code changes. ## Schema (migration 138) Additive `template_params` TEXT column on `whatsapp_configs` (default empty string = legacy 5-slot behaviour for existing installs). Stored as a JSON-serialized array of slot keys: `customer_name`, `event_name`, `gallery_link`, `password_line`, `expiry_date`. Unknown / duplicate / non-string entries are sanitized out at read time. ## Processor - `parseTemplateParams(raw)` — defensive parser; falls back to the 5-slot default on empty / malformed / all-invalid input. - `buildComponents(data, metaLang, params)` — emits ONLY the listed slots in the listed order, computed via a small switch on slot key. The password line still receives the locale-specific 🔒 label and the empty-when-no-real-password sentinel handling. - Processor reads `config.template_params` once per cycle and passes the parsed array to `buildComponents` per message. ## Admin route - GET surfaces `template_params` as the parsed array (default 5-slot when null/empty). - PUT round-trips the incoming array through `parseTemplateParams` before persisting, so the stored value is always the canonical sanitized JSON. - Test send rebuilt to use the same `buildComponents` path so the admin's test message matches their configured slot shape — a reporter who configures 2 slots gets a 2-parameter test send, not the legacy 5-parameter payload. ## UI - `WhatsAppTab` gets a checkbox + up/down list under the Template language field. Each slot shows its current `{{N}}` position when checked, an em-dash when unchecked. Live preview below the list: "Your template will receive: {{1}} = event_name, {{2}} = gallery_link". - EN + DE i18n for the field labels, hint, preview, and per-slot human-readable names. ## Tests - 17 unit tests in `__tests__/utils/whatsappBuildComponents.test.js` covering: parseTemplateParams sanitization (unknown keys, duplicates, non-strings, malformed JSON, all-invalid fallback, pre-parsed array acceptance) and buildComponents shape (reporter's 2-slot case, reorder, empty list, locale-specific password label, password sentinel handling, expiry omission). - All 17 + the 34 existing networkValidation tests pass. ## Migration numbering Sits at 138 on top of PR #649's migration 137. If #646 (Live Slideshow) merges before this, #646's own 137 + 138 take precedence and this needs renumbering to 139. Coordinated via PR #646's review thread. ## Honest caveat Still no Meta Business API account on my side. Spec-built, sanitizer + shape unit-tested, lint + tsc clean. End-to-end against Meta needs the reporter (or a maintainer with an account) to verify. If a real round-trip surfaces a mismatch, drop it in #647 and I'll iterate.	2026-06-21 20:51:20 +02:00
Luca	e6655f613b	chore(slideshow): renumber migrations to 138/139 (after whatsapp #649's 137) ... PR #649 takes migration 137 (add_whatsapp_template_language). Renumber the slideshow migrations to slot in after it: - 137_add_slideshow_share.js -> 138_add_slideshow_share.js - 138_add_slideshow_styling.js -> 139_add_slideshow_styling.js and update the slideshow migration-number references in comments/types. No content change — both are additive + addColumnIfNotExists-guarded, so re-running under the new filename on an already-migrated DB is a safe no-op.	2026-06-21 02:59:19 +02:00
Paul Nothaft	4fd7709596	fix(whatsapp): admin-pinned template language + Arabic locale support (#647) ... Reporter @Rekoo-PS hit three independent gaps trying to deliver an Arabic Meta template. Bundled here because they fan out from the same root cause (no first-class language config on the WhatsApp tab) and the review surfaces are tightly coupled. **1. Test send hardcoded `en_US` (`adminWhatsapp.js:141`).** Smoking gun for "I can't make it work" — Meta returned template_not_found_in_language (132001) on every test send for non-English templates, no matter what else the admin configured. Replaced with `config.template_language || 'en_US'`. **2. No `template_language` field on `whatsapp_configs`.** The only priors were per-message `data.language` (always null from our callers in `adminEvents.js:854,1188`) and `app_settings.general_default_language` (the *system UI* language, not the *template's* language registered with Meta). Migration 137 adds the column; GET + PUT surface it; the processor uses it as the highest-priority default when message_data doesn't override. Resolution order in `whatsappProcessor.processWhatsAppQueue` is now: 1. message_data.language (per-event override — caller path TBD) 2. config.template_language (admin-pinned template language) 3. app_settings.general_default_language (system fallback) 4. en_US (hardcoded last resort) **3. `LANGUAGE_MAP` + `PASSWORD_LABELS` didn't cover Arabic.** Added `ar` (Meta's single-code form per RFC; no region variant). For any language we don't enumerate (e.g. Turkish `tr_TR`, Chinese `zh_CN`, Hebrew `he_IL`), `resolveLanguageCode` now pass-throughs valid-shape codes (lowercase-language + optional underscore + uppercase-region) and forwards them to Meta as-is. If they don't match a registered template Meta returns 132001, which the test route already surfaces back to the admin via `error.message` — fail-loud, no silent fallback. Validation: - Unit smoke on `resolveLanguageCode` across 18 representative inputs (in-map, pass-through, canonicalization, rejection) — all behaviours correct. - Lint clean on all 7 changed files. - Frontend `tsc --noEmit` clean. - Migration `node -c` syntax-checked; additive + `hasColumn`-guarded so re-running is safe. Frontend: free-text input on the WhatsApp tab with EN + DE i18n. Pointing at Meta's supported-languages docs via the hint text — Meta's list grows; a hardcoded dropdown would rot. Closes #647.	2026-06-20 23:29:08 +02:00
Luca	1029dd05bd	feat(slideshow): db columns for live slideshow ... - 137: events.show_share_token + show_interval_ms/transition/transition_ms - 138: per-event watermark (tri-state, nullable=inherit global), source/ position/opacity/style + color filter columns, and event_types.slideshow_preset JSON so new events inherit a per-type default. Opt-in, no backfill.	2026-06-19 22:10:48 +02:00
Paul Nothaft	78c8e9d9f9	feat(whatsapp): WhatsApp Business API notification channel (#640 part D) ... Ports filpgame/picpeak's WhatsApp integration with substantial adaptation to fit our codebase patterns. Deliver the gallery-ready notification via Meta Graph API in addition to (or instead of) email — useful where the customer base expects WhatsApp by default. Strictly opt-in behind the new `whatsapp` feature flag. ### Backend - **Migration 136** (`whatsapp_configs` + `whatsapp_queue`). Loose-FK on `event_id` matching our `inbound_documents` / `expenses` pattern (NOT filpgame's hard FK — deleting an event shouldn't RESTRICT on stale queue rows). Composite index on `(status, retry_count, created_at)` covers the poll path. - **`whatsappService.js`**: thin Meta Graph client. Meta API version bumped v19 → v20 (filpgame's v19 deprecates Q3 2026); configurable via `WHATSAPP_META_API_VERSION` env var. Timeout dropped 10s → 8s for processor budget. Errors surface the Meta `error.code` so the processor can tell retryable from permanent. - **`whatsappProcessor.js`**: queue processor polling every 30s (configurable via `WHATSAPP_QUEUE_POLL_MS`), 10 messages per cycle, 3 retries before marking `failed`. Default language sourced from `app_settings.general_default_language` (matches our email-language resolution pattern); replaces filpgame's hardcoded `pt_BR` fallback. Falls back to `en_US` if nothing is configured. No-ops gracefully when the `whatsapp` flag is off, the config row is missing, or the access token isn't set. - **`adminWhatsapp.js`**: three routes (GET/PUT config, POST test). Gated by `requireFeatureFlag('whatsapp')` so operators who haven't enabled it can't see the surface. Access token masked as `'********'` on GET; masked values silently preserve the stored token on PUT. Enabling with no Phone Number ID, template name, or token (and none stored) fails at the validator. - **Two hook points** in `adminEvents.js`: - **Create-and-publish-in-one-step**: queues immediately after the `gallery_created` email when `!isDraft && customerPhone && waConfig.enabled`. Password from `req.body` is still in scope. - **Publish-from-draft** (`POST /:id/publish`): queues with the password the admin re-typed via PR #627's `PublishGalleryDialog`. When no password was typed (legacy API consumers without dialog), passes empty string so the password line renders blank rather than leaking the `(set at creation)` sentinel. - **`server.js`**: starts `whatsappQueueProcessor` at boot. Non-fatal if it fails to start (logged as warning). - **`feature_flags`**: new `whatsapp` flag in `KNOWN_FLAGS` and `DEFAULT_FLAGS` (default false). ### Frontend - **`featureFlags.service.ts`**: `'whatsapp'` added to `FeatureKey` union. - **`FeaturesTab.tsx`**: WhatsApp card in the Communication section (between Incoming mail and Messaging). Smartphone icon, "new" status, sidebar-hidden (no sidebar entry — config lives under Settings). - **`whatsapp.service.ts`** (new): typed client for the three admin routes. - **`WhatsAppTab.tsx`** (new): Settings tab. Form for Phone Number ID, WABA ID, access token (masked toggle), template name, and enabled flag. Separate card below for a static test send. Token masking matches the server's `'********'` sentinel — admin can edit other fields without re-entering the token. - **`SettingsPage.tsx`**: WhatsApp tab nav item gated on `flags.whatsapp` (so it shows only when the feature is enabled); render block wires `<WhatsAppTab />`. ### i18n 22 new EN + 22 new DE entries covering the Settings tab form, the Features-tab card, plus `admin.activities.whatsapp_config_updated` + `admin.notificationMessages.whatsappConfigUpdated` for the bell / dashboard surfaces from PR #637. ### Deliberately NOT included - filpgame's **password-encryption-at-rest** layer (`password_encrypted`/`password_iv`/`password_key_version` columns). Our publish-from-draft password recovery uses the admin re-type flow from #627 (PublishGalleryDialog) — no plaintext at rest. ### Setup notes for operators 1. Create a Meta Business Account + WhatsApp Business App. 2. Register a phone number and obtain `phone_number_id` + `waba_id`. 3. Create a system-user access token (long-lived recommended). 4. Submit a message template for approval. The default `gallery_ready` expects 5 body parameters: customer name, event name, gallery link, password line, expiry date. 5. Enable the `whatsapp` feature flag. 6. Enter credentials under Settings → WhatsApp, send a test, then enable delivery. ### Test plan - [x] Backend `node -c` on all new/changed files clean - [x] `tsc --noEmit` on frontend clean - [x] Backend dev container restart picks up new files, /health OK - [ ] Manual: enable `whatsapp` flag → Settings → WhatsApp tab appears - [ ] Manual: save config with masked-only token (existing token preserved) - [ ] Manual: enable=true without phone_number_id rejected at PUT - [ ] Manual: enable=true without stored or new token rejected at PUT - [ ] Manual: create-and-publish event with customer_phone → queue row inserts with message_type='gallery_created' - [ ] Manual: publish-from-draft via PublishGalleryDialog with password → queue row uses the admin-typed password in the {{4}} line - [ ] Manual: test send to a real phone with valid Meta config + approved template → Meta returns messages[0].id, toast shows the id - [ ] Manual: bell renders "WhatsApp configuration updated" in DE when the config_updated activity fires (via PR #637 smart default)	2026-06-18 22:57:22 +02:00
Paul Nothaft	820f4835f1	feat(categories): per-category download permissions (#640 part B) ... Adds an `allow_downloads` boolean to `photo_categories` so admins can have different download policies per category — e.g. preview categories public, originals client-only. AND's with the event-level `allow_downloads`, so disabling at either level blocks downloads for that category's photos. Defaults to true so categories created before migration 135 keep working without admin intervention. Credit: 8digit/picpeak@928164b + @751ec75. ### Backend - **Migration 135**: additive `allow_downloads BOOLEAN NOT NULL DEFAULT true` on `photo_categories`, hasColumn-guarded + sane down. - **`adminCategories.js`**: PUT /:id accepts optional `allow_downloads` patch. - **`gallery.js`**: - `GET /:slug/photos` returns `allow_downloads` per category AND `category_allow_downloads` per photo. - `GET /:slug/download/:photoId` returns 403 when the photo's category disables downloads. - `GET /:slug/download-all` LEFT JOINs `photo_categories` and filters `whereNull(category_id) OR allow_downloads=true OR allow_downloads IS NULL`. The null check covers pre-migration-135 rows during the upgrade window. - `POST /:slug/download-selected` same filter pattern. ### Frontend - **`categories.service.ts`**: `updateCategory()` gains an optional `patch` argument carrying `{ allow_downloads }`. PhotoCategory interface gains the optional field. - **`EventCategoryManager.tsx`**: new toggle button next to the delete X. Green DownloadCloud icon when downloads are on, plain Download icon when off. Click toggles via the new mutation; toast confirms. - **`PhotoLightbox.tsx`**: `photoAllowsDownload = allowDownloads && currentPhoto?.category_allow_downloads !== false`. Hides the download button + blocks the 'D' keyboard shortcut + early-returns from handleDownload. - **Types**: Photo interface gains `category_allow_downloads`. - **i18n**: 5 new EN + DE entries for the toggle button toast + tooltip. No global-category surface change yet — global categories don't currently have a UI for the toggle. Admins can still flip the column directly via SQL or via a future global-categories editor. ### Test plan - [x] Backend syntax + TS check clean - [x] ESLint: no new warnings - [ ] Manual: admin → event detail → categories panel → click DownloadCloud icon → category flips, toast confirms - [ ] Manual: gallery (guest) → photo in disabled category → lightbox shows no download button, 'D' shortcut is a no-op - [ ] Manual: download-all on a gallery with one disabled category → ZIP excludes that category's photos - [ ] Manual: download-selected including a disabled-category photo → 404 (filtered out) and the response carries only the allowed selection - [ ] Manual: pre-migration-135 category (legacy row with NULL allow_downloads) → downloads still work (defaults true via fallback)	2026-06-18 22:29:35 +02:00
Luca	267b121d66	feat(accounting): supplier-country tax default + configurable default output VAT code ... VAT supplier-country reclaim default: - Migration 134 adds inbound_documents.supplier_country. - categorizeInbound auto-derives tax_treatment via resolveTaxTreatment: explicit treatment wins; else country in the reclaim list → domestic, outside it → foreign_vat_non_reclaimable, unknown → domestic. Consumes the previously-stored-but-unused accounting_vat_reclaim_countries. - Triage modal gains a Supplier country dropdown (saved via updateInbound). +5 unit tests for resolveTaxTreatment. Configurable default output VAT code for new invoices: - New accounting_default_output_vat_code setting (PUT wired; getSettings/type). - Settings → Accounting dropdown to pick it. - Invoice + quote editors seed their VAT picker (rate + code) from it on a blank new document — skipping edits/conversions, never clobbering a touched value. New docs no longer silently start at 0%. i18n en + de.	2026-06-18 15:35:58 +02:00
Luca	51837c3a88	feat(accounting): invoices force-enable the Accounting master ... Invoice VAT config (codes + label) and the hourly rate now live under Settings → Accounting, so an install with Invoices must have Accounting available. - applyDependencyRules (backend adminFeatureFlags.js + frontend FeatureFlagsContext.tsx): bills on → accounting on, before the accounting→children rule so the sub-features keep their own state. - Migration 133 corrects existing installs: set the STORED accounting=true where bills is on. requireFeatureFlag('accounting') reads the raw row, so without this an upgraded install (invoices on, accounting off) would show the tab but 403 its endpoints. Idempotent; only flips on; no down. - Features tab: the Accounting card shows locked-on (disabled + hint) while Invoices is enabled. Also includes the i18n keys (en/de) for the VAT/financial settings move.	2026-06-18 15:11:11 +02:00
Luca	36a8e42f90	feat(accounting): re-categorize incoming invoices, note field, pending re-bill pool ... Address three incoming-invoice issues: 1. Re-categorization: a categorized invoice can now be changed again (e.g. passthrough → company expense). New "Re-categorize" button pre-fills the triage modal from the existing disposition/customer/markup/note. categorizeInbound is re-runnable — it unwinds any prior re-bill line (removes the invoice line + recomputes totals) before applying the new disposition, and refuses (INVOICE_LOCKED) when the re-bill is on an already-issued invoice. 2. Note field: new `note` column (migration 132 — 126 is already on beta) captured in triage and shown in the read-only view. 3. Re-bill like hours: rebill/passthrough now persist customer_account_id. Per-event customers accumulate as PENDING items, surfaced in a new "Pending re-bills" card and bundled into one invoice via "Bill these" (mirrors unbilled-hours billing). Monthly/manual customers keep auto-consolidating onto their running draft. Passthrough (durchlaufend) can now also attach to a customer with optional markup. Adds backend unit tests for buildInboundLineItem + isInvoiceMutable and en/de translations (other locales fall back to English defaults).	2026-06-18 12:23:13 +02:00
Luca	cd6d57839b	fix(accounting): PR #622 blockers — CSV formula injection + IMAP double-ingest race ... Blocker 1 — CSV/Banana formula injection. Neither csvEscape (ledgerService) nor the tax-report CSV escape nor the unquoted tab-separated Banana cell formatter prefixed risky leading chars, so an admin-/sender-controlled cell beginning with = + - @ TAB CR executes as a formula when the Treuhänder opens the export. New shared util neutralizeSpreadsheetFormula() prepends a single quote; wired into all three sinks (quoted CSV + unquoted Banana). Unit test pins one of each char. Blocker 2 — IMAP intake double-ingest race. received_emails.message_id was INDEX, not UNIQUE, and the poller ingested attachments BEFORE writing the audit row, so a second replica / rolling-deploy overlap double-ingested the same mail. Migration 128 makes message_id UNIQUE (nulls stay distinct); the intake now CLAIMS the message row (status='processing') BEFORE ingesting — a concurrent claim hits the unique constraint and skips cleanly (shared isUniqueViolation helper). Stale 'processing' rows (worker crashed mid-ingest) are reclaimed after 10 min so no attachment is orphaned. NOT done (deliberate): the suggested UNIQUE on inbound_documents.file_sha256 — that column is a SOFT dedup key by design (manual re-uploads are kept as flagged 'duplicate' rows + duplicate_of_id for the Duplikat disposition); a unique index would break that feature. The file race only yields an extra 'unsorted' row (a data-quality nit, caught by the existing manual Duplikat backstop), not a double-count. Rationale to be added to the PR reply.	2026-06-16 18:21:02 +02:00
Luca	03fa3d8296	fix(flags): close CRM/accounting feature-gating gaps from the audit ... A sweep of every CRM/accounting toggle found surfaces still reachable with their flag OFF. Adds a shared requireFeatureFlag middleware (the two existing per-file copies predate it) and closes the gaps: - Hours logging: only createEntry checked the flag — edit/delete/bill and the list/summary routes were permission-only. Gate all six /hour-entries routes on the hoursLogging master so a disabled feature can't be read, mutated, or invoiced via a direct API hit. - Installment plans: PUT /deals/:uuid/installment-plan mutates invoices but wasn't bills-gated; add requireFeatureFlag('bills'). - Customer invoice PDF: /invoices/:id/pdf lacked the feature_bills check the list + quotes routes have. Also fixes the quotes-PDF gate, which read req.customer.feature_quotes (never populated → silent no-op). - Customer contracts: /contracts + /contracts/:id/pdf were gated by neither the master nor a per-customer column. Per-customer contracts override (the missing counterpart): - Migration 131 adds customer_accounts.feature_contracts, default TRUE so existing customers keep their Contracts tab (preserve-visuals). - Effective resolver now contractsMaster AND feature_contracts; admin detail page gains the toggle; service/validator/serializer wired. Cleanups: - Drop stale `taxReport` from the sidebar's Clients-reveal list (Tax moved to Accounting); add the missing `projects` so it mirrors the context derivation. - SettingsPage tab-snap effect now depends on flags.accounting. - Fix stale taxReport "forced off when bills off" comment (it's accounting).	2026-06-16 13:12:26 +02:00
Luca	0a7dc1cf5d	feat(accounting): snapshot vat_code on quotes/invoices + export prefers it (foundation) ... First slice of the VAT-consolidation: migration 130 adds a nullable vat_code snapshot column to quotes + invoices, and the Treuhänder export now prefers the invoice's snapshotted code over the (mutable) rate→code map, so a historical invoice's VatCode never changes when codes are re-mapped. Schema-drift guarded; behaviour-neutral until the editors start writing the snapshot (next slices). Part of: VAT registry → Settings→Accounting, invoice VAT dropdown, registration/ reclaim toggle.	2026-06-15 23:53:32 +02:00
Luca	402dbde0a1	Merge origin/beta into feat/accounting-inbound-invoices ... Resolves the 7 feature-flag / i18n conflicts (accounting flags vs upstream's Project Overview 'projects' flag, both registered in the same files) as additive unions — accounting + incomingInvoices + expenses AND projects all coexist. Migrations slot cleanly: projects 117-121, accounting 122-129, no collisions. Frontend build + backend node --check pass.	2026-06-15 16:37:23 +02:00
Luca	9d13880f2b	fix(projects): address review — cross-customer guards + email/queue hardening ... Resolves the two blockers and the actionable concerns/nits from review. Blockers (cross-customer leak): - linkDealToProject: collect the deal's customer + events BEFORE any write, then reject a cross-customer link with PROJECT_CUSTOMER_MISMATCH (422) before re-pointing events/quotes/contracts or adopting a customer. The editors set project_id via quoteService/contractService → linkDealToProject (not assignDocument), so the guard lives at that chokepoint. Null-project adoption ("first deal wins") preserved as intended. - assignDocument: boundary guard mirroring customerHoursService, defense-in-depth ahead of the cascade. - Frontend: translated PROJECT_CUSTOMER_MISMATCH (projects.error.customerMismatch, de+en) wired into HoursSection + quote/contract editor onError (concern 5). Concerns: - 1: processEmailQueue gains an onlyId option; cockpit "send now" scopes the flush to the single row so it can't force-retry other dead-lettered emails. - 2: resendEmail re-stringifies email_data when PG returns a parsed object, matching the canonical enqueue — no jsonb double-encode. - 3: cockpit email feed scoped to the project's own document numbers (event_id for gallery mails; email_data doc-number match for CRM mails) instead of the recipient string — a shared inbox no longer leaks another customer's mail. - 4: migration 117 backfill wrapped in a transaction (adds atomicity on SQLite, where the runner does not wrap; PG already wraps the whole migration). - 6: resend/cancel/retry/sendNow now logActivity uniformly (project_email_*), adminId threaded from the route. - 8: validator optional({ values: 'null' }) → optional({ nullable: true }). - 9: pre-121 list valuation falls back to customer-scoped quotes so the list isn't all-zero during the upgrade window. Nits: - milestone selection uses Array.at(-1); removed redundant in-loop require in emailProcessor; clarifying comments for the list/detail perms split and the count-vs-value (0 vs em-dash) convention.	2026-06-13 11:57:32 +02:00
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).	2026-06-11 21:37:48 +02:00
Luca	31867efcb9	fix(accounting): migration 127 must not insert created_at/updated_at into app_settings ... The app_settings table (per its migration schema) has no created_at/updated_at columns — the canonical seed pattern (migration 103) inserts only setting_key/setting_value/setting_type. Migration 127 wrongly added timestamps, so the insert threw `SQLITE_ERROR: table app_settings has no column named created_at` on every run of the migration suite. That broke the backend test job (cascading through every suite that builds the schema) and the Postgres-based fresh-install + schema-drift jobs. Fix: drop the timestamp columns from the insert, matching migration 103. Verified: full backend jest suite green (67 suites, 736 passed); migration harness still green.	2026-06-11 16:57:48 +02:00
Luca	5645c304ab	feat(email): incoming mail (IMAP) intake - backend + standalone flag ... Adds a second mail config (incoming/IMAP) alongside the outgoing SMTP one, a 1-minute poller, and a received-emails log. Standalone `incomingMail` feature flag (default off). - deps: imapflow + mailparser (receive-side; picpeak only had nodemailer). - migration 128: email_configs gains imap_* columns (same shape as smtp_*); seed incomingMail flag; new received_emails audit table. - emailIntakeService: polls the mailbox every 60s when the flag is on AND a mailbox is configured (no-op otherwise); parses each unseen message (mailparser flattens forwarded/nested attachments), drops PDF/JPEG/PNG into the incoming-invoices inbox (inbound_documents, source='email'), logs each message in received_emails (dedupe by message-id; duplicate attachments caught by the existing SHA-256 guard), marks it \Seen. - adminEmail: GET/POST /incoming-config (mirrors SMTP config, masks imap_pass, SSRF host guard) + GET /received (paginated log). - server.js starts the poller at boot. Verified: node -c, require-graph, migration-128 harness (imap columns, flag, received_emails). Frontend (IMAP block under SMTP + Received tab + flag card) follows.	2026-06-11 15:54:43 +02:00
Luca	c59df52d40	feat(accounting): split Incoming invoices vs Expenses - flags, schema, settings (stage 1) ... Foundation for separating external supplier invoices from internal expenses, per design review. This stage is additive + buildable; the service/route/UI data rework follows in stage 2. - Migration 126: incoming invoices own their payable on inbound_documents (supplier_paid/at/method/ref + disposition + tax_treatment + booking event_id + category_id + re-bill markup/linkage); expenses gain kind (amount/mileage/ per_diem) + quantity + snapshotted rate_minor. Additive, hasColumn-guarded. - Migration 127: seed `expenses` feature flag (default off) + accounting app_settings (accounting_km_rate_minor=70, accounting_per_diem_rate_minor=0, accounting_require_proof=false). - Backend: `expenses` added to feature-flag known/defaults/dependency (forced off when the accounting master is off); new PUT /admin/settings/accounting (read via the generic GET /:type). - Frontend: `expenses` flag (type + context + dependency); Features tab gets an Expenses sub-card; the Expenses sub-nav + route now gate on `expenses` (not incomingInvoices); AccountingIndex prefers inbox -> expenses -> tax. - i18n: settings.features.expenses.* (EN + DE). Verified: node -c; migration 124->126->127 harness (new columns, flag, settings + idempotency); en/de JSON valid; npm run build green.	2026-06-11 12:24:53 +02:00
Luca	502fbad5a8	feat(accounting): PDF/image preview in triage, opened at the QR-bill (no OCR) ... Instead of OCR, let the admin read the payment slip directly: the triage modal now embeds the captured document and, for PDFs, opens at the LAST page scrolled to the Swiss QR-bill area so IBAN/amount/reference are visible while typing. - backend: capture PDF page count at upload via pdf-lib (new inbound_documents.page_count, added to in-flight migration 124); new GET /api/admin/expenses/inbound/:id/file streams the stored file inline (safePath-guarded, nosniff). Raw-serve is acceptable here (admin views own uploads); the hardened rasterise-in-isolated-worker path stays a follow-up. - frontend: getInboundFileBlob fetches the file with Bearer auth as a blob; the triage modal renders it (iframe for PDF with #page=<last>&view=FitH,300, <img> for camera photos) in a two-column layout next to the form. - i18n: accounting.inbox.previewLoading / qrHint (EN + DE). Verified: node -c, require-graph, migration-124 harness (page_count), npm run build green.	2026-06-11 00:38:37 +02:00
Luca	2c351bf0c9	refactor(accounting): make Accounting a master flag with sub-toggles ... Replaces the earlier peer-`accounting` flag (which only *conditionally* relocated Tax) with a cleaner top-level master + sub-toggle model, per design discussion: - `accounting` = explicit top-level MASTER (Settings -> Features). Off hides the whole Accounting section. - Sub-toggles, gated under the master: - `taxReport` ("Tax export") moves PERMANENTLY out of CRM. Removed from the Clients sub-nav and from the derived `clients` flag. Now INDEPENDENT of Bills (per decision). Old /admin/clients/tax-report -> redirect to /admin/accounting/tax-report. - `incomingInvoices` (new) gates the supplier-invoice capture / expenses / re-bill feature; the /api/admin/expenses router now checks it. - Dependency rules (backend + frontend): accounting off forces taxReport + incomingInvoices off; taxReport dropped from the clients derivation; the bills->taxReport rule removed. - Preserve visuals: migration 122 rewritten to auto-enable `accounting` on installs that already had Tax on (so the tab doesn't vanish), and to seed `incomingInvoices` off. Verified with a SQLite harness (taxReport on -> accounting on; off -> off). - Settings -> Features: new "Accounting" section with the master card + Tax export + Incoming invoices sub-cards (disabled until the master is on). - i18n: navigation.accounting, accounting.*, settings.features.{accounting, incomingInvoices,taxReport.requiresAccounting}, sections.accounting (EN + DE, DE authored natively); Tax report relabelled "Tax export"/"Steuerexport". Verified: node -c, migration-122 harness, en/de JSON valid, npm run build green.	2026-06-11 00:17:44 +02:00
Luca	c305492845	feat(accounting): inbound supplier-invoice capture + expense re-bill (backend) ... New top-level Accounting area (gated by an `accounting` feature flag, default OFF, + accounting.view/manage permissions), separate from CRM. Lets an admin capture a received supplier invoice (upload OR phone/tablet camera), give it a disposition, and re-bill the cost to a client onto the relevant event's invoice with a contract-driven markup. Mirrors the billable-hours model. Backend foundation only — frontend pages (inbox / expenses UI + camera widget) and the heavy extractors (Tesseract OCR / Swiss-QR decode / isolated rasterise worker) are follow-ups; extractionService is scaffolded so the upload path is already wired. Migrations 122-125 (numbered above the in-flight feat/crm 117-121): - 122 seed `accounting` flag (default OFF, idempotent) - 123 seed accounting.view/manage permissions + grant super_admin/admin - 124 inbound_documents + expenses + expense_categories (+ seed categories) - 125 contracts Spesen-Zuschlag clause (expense_markup_type/_percent/_flat_minor) API: /api/admin/expenses — inbound capture/list/confirm/categorize, expense CRUD, /:id/rebill (event-scoped; markup = expense override -> contract clause -> 0%; mints an editable scheduled invoice), /:id/supplier-payment, categories. adminFeatureFlags KNOWN_FLAGS/DEFAULT_FLAGS gain `accounting`. Conventions: idempotent hasTable/hasColumn-guarded migrations; money in integer *_minor; QR amount stored separately + untrusted; requirePermission guards; camelCase API <-> snake_case columns; multer + 15MB cap for PDF/JPEG/PNG. VAT/tax handling is v1 capture-only — verify with a Treuhaender before relying. Verified: node -c all files, require-graph smoke test, and a SQLite migration harness (schema + seeds + idempotency + defaults assert green).	2026-06-11 00:04:16 +02:00
Luca	6420047e7c	feat(projects): link quotes & contracts to a project (precise cockpit rollup) ... - Migration 121 adds quotes.project_id + contracts.project_id (nullable FK, index) and backfills the unambiguous single-project-per-customer case. - projectService rolls quotes/contracts up by project_id, with a customer-based fallback on pre-121 DBs (hasColumnCached guarded). - quote/contract create+update accept an optional projectId; detail transforms surface it for editor prefill. - POST /projects/:id/quotes and /:id/contracts assign endpoints.	2026-06-06 13:05:36 +02:00
Luca	1bf0b34ea5	feat(projects): gate Project Overview behind a projects feature flag + cockpit email actions ... - Migration 120 seeds the projects flag (default OFF), idempotent. - Backend feature-flags whitelist + DEFAULT_FLAGS + clients derivation. - adminProjects routes 403 PROJECTS_DISABLED when the flag is off. - projectService email actions (resend/cancel/retry/send-now) + routes. - Frontend flag type, DEFAULT_FLAGS, Features tab card (en+de).	2026-06-06 12:59:57 +02:00