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.
Implements the split decided in review:
Incoming invoices (external) - the inbound_documents row IS the payable:
- categorizeInbound now UPDATES the document (disposition + tax_treatment +
booking event_id (null=company) + category), no derived expense row, so a
supplier invoice appears only in the incoming-invoices surface.
- rebillInbound mints the client invoice from the document (base = invoice
total + markup) and links it on the doc.
- markInboundSupplierPayment records supplier payment ON the incoming invoice
(mark-paid lives here now).
Expenses (internal) - own costs only:
- createExpense: kind = amount|mileage|per_diem; amount = quantity x rate
(rate from accounting settings, per-entry override; snapshotted); optional
proof file; booked to an event or the company; require-proof enforced from
settings. No supplier payment, always own-cost.
- listExpenses returns internal rows only (inbound_document_id IS NULL).
Routes: per-flag gating (incomingInvoices vs expenses; categories on the
accounting master); supplier-payment + re-bill moved under /inbound/:id/*;
POST/PATCH expenses accept a multipart proof upload; GET /:id/proof streams it
(PDF download-only, image inline). getAccountingSettings reads app_settings.
Verified: node -c, require-graph, 12 unit tests (markup + expense amount/build).
Frontend rework (service + the two UIs + settings tab + category i18n) follows.
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.
Covers the silently-regressable money + classification bits of the re-bill
flow (the maintainer's "thin CRM test coverage" concern). Pure functions via a
new expenseService._internal export — no DB, no date-harness pitfalls:
- computeMarkupMinor: percent rounding, flat, none/null.
- resolveMarkup precedence: override > expense clause > none.
- buildExpenseInsert: bad-disposition guard, tax_treatment/status defaults,
declined -> status+reason, markup field matches type, parked -> status.
11 tests, all green (npx jest expenseService.markup).
Security hardening for inbound supplier-invoice previews. The admin UI no
longer renders raw PDFs — a malicious inbound PDF could otherwise run embedded
JS or phone home in the admin's session. Instead PDFs are rasterised to flat
PNGs server-side and only those images are shown.
- backend: new rasterizeService shells out to poppler `pdftoppm` (added to the
Docker image via apk poppler-utils — an OS package, NOT a Node PDF lib, so it
respects the pdfkit+pdf-lib "no third PDF lib" rule). pdftoppm executes no JS
and fetches no remote resources, so it doubles as the SSRF/phone-home guard.
Rendered pages cached under storage/business-docs/inbound/rendered/<id>/.
- GET /inbound/:id/page/:n streams the rasterised PNG (CSP default-src 'none'
+ nosniff). GET /inbound/:id/file now serves PDFs as a DOWNLOAD only
(Content-Disposition: attachment) — never inline; images still inline.
- frontend: triage preview switched from a raw-PDF <iframe> to rasterised page
images (getInboundPageBlob), defaulting to the LAST page (QR-bill) with
prev/next nav for multi-page PDFs; images stream as before.
- i18n: previewError / prevPage / nextPage / pageOf (EN + DE).
REQUIRES A BACKEND IMAGE REBUILD (Dockerfile adds poppler-utils) — a plain
`docker compose pull` of a stale image won't have pdftoppm; the route then
returns 503 RASTERIZER_UNAVAILABLE and the UI shows "preview unavailable".
Verified: node -c, a pdfkit->pdftoppm rasterise smoke test (renders + caches),
en/de JSON valid, npm run build green.
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.
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.
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).
Zszywany reported on v3.44.0 (Ubuntu, Postgres 15): with Settings →
General → "Max Files per Upload" set to 10, an event configured with
allow_user_uploads + a guest selecting 16 files in the gallery's
"Upload Photos" modal succeeded silently — admin uploads to the same
event correctly refused with "Upload limit reached". On top of that,
the gallery modal's "fileRequirements" hint literally rendered
`{{limit}}` instead of the configured number.
Two separate misses for the guest path, both fixed here:
1. **Backend enforcement** — `backend/src/routes/gallery.js:1641` had
`limits: { fileSize: 50MB, files: 10 }` and `.array('photos', 10)`
hardcoded. The admin path at adminPhotos.js:131 has always resolved
files-per-batch via `getMaxFilesPerUpload()` (cached 60s read of
`general_max_files_per_upload`); guest path just never used it.
Mirror the admin: `const maxFilesPerUpload = await getMaxFilesPerUpload()`
and feed multer both `limits.files` AND the `.array(...)` cap. The
50MB per-file size is a separate concern from this issue and stays
as-is for now.
2. **i18n interpolation missing on the guest modal** —
`UserPhotoUpload.tsx:203` called `t('upload.fileRequirements')` with
no arguments. The translation string at `en.json:160` is
"JPEG, PNG or WebP (max 50MB per file, {{limit}} files per upload)"
— `{{limit}}` is unbound, so i18next emits it literally. The admin
variant `PhotoUpload.tsx:414` correctly passes
`{ limit: maxFilesPerUpload }`.
Also wired up the same client-side count guard the admin component
uses: addFiles refuses additions past the limit (`upload.limitReached`)
and warns on partial-truncate (`upload.someFilesSkipped`). Backend
enforces too, but the client guard saves a 4MB+ multipart POST when
the user is clearly over.
To surface the setting on the guest side, `general_max_files_per_upload`
joins the public-settings whitelist + projection (publicSettings.js)
and the `PublicSettings` TS interface gets the new field. Default
fallback (500, matching `uploadSettings.js` DEFAULT_MAX_FILES_PER_UPLOAD)
in both backend projection and frontend reader so an install that's
never set the value renders a sensible number rather than "undefined".
jodrmx reported on v3.44.0 (Pi Lite, Docker compose): admin-UI event
delete removes the DB row but leaves `storage/events/active/<event>/`
intact on disk.
Root cause: `deleteEventCascade` in adminEvents.js read
`event.folder_path` and gated the `fs.rm` on it. That column is NEVER
WRITTEN anywhere in the codebase — grep confirms two reads in this one
function, zero writes elsewhere. So `event.folder_path` was always
undefined, `if (event.folder_path)` always false, and the per-folder
cleanup silently no-op'd for every delete. The DB-cascade transaction
ran fine, so the symptom was always "row gone, files stay" — exactly
what jodrmx hit.
The actual on-disk location is `events/active/{slug}` everywhere else
in the codebase:
- adminPhotos.js:260 — `path.posix.join('events/active', event.slug)`
- adminEvents.js:610, events.js:155, adminThumbnails.js:153 — read
from `events/active/{slug}`
- adminArchives.js:171 — reads from same root
- photoResolver.js:14-15 — documents the layout
The delete cascade was the only path looking at the non-existent column.
Cure: drop the `if (event.folder_path)` guard, read `event.slug`
instead, and remove from both `events/active/{slug}` (active gallery
folder) and `events/archived/{slug}` (the post-archive copy that
survives the archive flow). `event.slug` is NOT NULL and slugify-
sanitized (lower-case ASCII + dashes only via utils/slug.js), so the
path is well-formed and path-traversal-safe. Best-effort `fs.rm`
semantics + try/catch unchanged — failures still log a warning rather
than unwinding the DB transaction, since orphan files are recoverable
noise compared to a half-deleted DB row.
Forward fix only — does not retroactively clean up the orphans that
have accumulated on existing installs. Admins can `rm -rf
storage/events/active/<old-slug>` manually for those; not worth a
migration script for a one-time deploy ritual.
patchingfailed reported on v3.44 stable: a gallery named `Ägypten` with
photo `Ägypten_individual_0050.jpg` downloads as `gypten_...` —
the leading umlaut is dropped entirely. Their hypothesis was a
Content-Disposition encoding issue, but the actual root cause sits
one layer earlier: at UPLOAD time when `generatePhotoFilename` calls
`sanitizeFilename`.
`sanitizeFilename` did:
String(str).trim()
.replace(/\s+/g, '_')
.replace(/[^a-zA-Z0-9_\-\.]/g, '') // ← drops `Ä` outright
.replace(/[_\-]{2,}/g, '_')
.replace(/^[_\-]+|[_\-]+$/g, ''); // ← would strip a leading _ too
For `Ägypten`: alphanumeric-strip → `gypten` (Ä gone, no underscore
left behind because the regex used '' as the replacement, not '_'). The
result is stored in `photos.filename` and that's what downloads serve.
By that point `buildContentDisposition` is doing the right thing
(emits both `filename="..."` ASCII fallback AND RFC 5987
`filename*=UTF-8''…` — Chrome correctly picks the UTF-8 form), but the
string it's encoding has already lost the umlaut at the DB layer.
Cure: NFD-normalize + strip combining marks BEFORE the alphanumeric
strip. Same pipeline `utils/slug.js` (#525) already uses for URL
slugs:
sanitized = sanitized
.normalize('NFD')
.replace(/[̀-ͯ]/g, '');
Now `Ägypten` → NFD-decomposed `A` + combining diaeresis → strip
combining mark → `Agypten` survives the alphanumeric pass. Filename
and URL slug stay in sync (the URL was already `Agypten`, per
patchingfailed's report — the filename now matches).
Test surface: new `filenameSanitizer.test.js` pins:
- the headline #607 contract for German / Portuguese / French / Spanish
accented inputs (with a counter-example using the pre-fix pipeline so
a future edit can't quietly regress it)
- ASCII-input parity — pre-#607 byte-identical output for every
pre-existing ASCII case
- `generatePhotoFilename` composed round-trip
- `sanitizeForContentDisposition` + `buildContentDisposition` RFC 6266
dual-form output (since the helper sits next to this function and is
the next thing to break if a refactor goes sideways)
- `sanitizeForZipEntry` path-traversal blocking
31 cases total, all pass.
Bundled into PR #609 since it's a small targeted fix and that PR is
already an admin-UI polish branch with low review weight.
Two gaps left emails stuck 'pending' after (re)configuring SMTP:
1. Saving the email config never re-initialised the transporter. The queue
processor only re-inits when its cached transporter is null, so a changed
SMTP account had no effect until a backend restart. Now call
initializeTransporter(true) after save (it self-catches; invalid config
just leaves it null, surfaced via the Test-email button).
2. The manual 'send now' flush (ignoreSchedule) still enforced retry_count<3,
so emails that failed 3× while SMTP was broken could never be retried from
the UI. Move the retry-cap (and schedule gate) to automatic runs only;
a manual flush forces a retry of every pending email.
Per decision: keep dashboard revenue windows on pure cash basis (recognise
by paid_at for ALL invoices) and give the admin control over paid_at.
- adminDashboard: revert the imported-vs-native split; winSum is paid_at >=
cutoff for every paid invoice again (clean cash basis).
- BillDetailPage mark-paid dialog: add an optional 'Payment date' field
(LocalizedDateInput, defaults to today) so a payment can be backdated to
when it actually arrived. Backend already accepted paidAt end-to-end
(route validator + markPaid service + payment-log) — only the UI was
missing. EN/DE 'bills.payment.date' added.
This fixes the collapsed 30=90=365 windows (they were collapsing because
many invoices were marked paid in one session, all stamped 'now').
The dashboard revenue windows (30/90/365 days) keyed purely on paid_at.
Imported historical invoices therefore landed in the recent window whenever
their paid_at sat there — notably legacy rows imported before commit c6b8cc9
began anchoring an import's paid_at to its issue_date, which still carry an
import-time paid_at. Recognise imported invoices (imported_pdf_path NOT NULL)
on their issue_date instead; native invoices keep cash-basis paid_at. No data
migration needed — fixes already-imported year-old invoices too.
The public quote, contract-signing, and payment-check pages baked a single
light logo (the contract page showed none), so the dark page rendered a
dark-text logo on a dark background.
- usePublicDarkMode now returns { isDark } (reactive) alongside applying
the .dark class, so pages can pick a theme-aware asset.
- The three public routes now surface both branding logo URLs (logoUrl +
logoUrlDark) in the issuer block; the contract issuer gains a logo too.
- QuoteResponsePage, ContractResponsePage, and the payment-check
BrandingHeader pick the dark variant when isDark, falling back to
whichever exists. Covers the accept/accepted states of each page.
The SVG->PNG cache was keyed only by source path + mtime + size, so an
override logo rasterised once WITHOUT fonts (text -> tofu) stayed cached
after the font fix - the source SVG was unchanged, so the stale tofu PNG
kept being served. Add a RASTER_VERSION component to the cache key; bumping
it (v2-fonts) invalidates every prior rasterisation without clearing the
cache dir by hand.
The favicon upload allowed only PNG/ICO, so an SVG favicon was rejected.
Accept image/svg+xml (.svg) too - DynamicFavicon already emits the right
MIME type and served SVGs are CSP-locked (render-only) by secureStatic.
Update the EN/DE help text accordingly.
The upload never enforced 32x32 - only the help text recommended it,
which misled admins. Update the EN/DE guidance to recommend a larger
square image (512x512) and raise the favicon upload cap 1MB -> 2MB so
high-resolution PNGs fit comfortably.
Serve uploaded SVGs (admin logos etc.) with a restrictive
Content-Security-Policy (default-src 'none'; style-src 'unsafe-inline';
img-src 'self' data:) + X-Content-Type-Options: nosniff in secureStatic.
The browser still renders the vector, but any embedded <script>/on*
handler can't execute if the SVG is opened directly — keeps real SVGs
(scalable) instead of rasterising them. Applies to all secureStatic
mounts (uploads/photos/thumbnails/fonts); only SVGs get the header.
New /admin/system-health page (sidebar entry, settings.view) that lists
emails the queue gave up on (status='failed' or pending+retry>=3) with
retry (re-queue) and dismiss (delete) actions. Backend adds /failures,
/failures/email/:id/retry and DELETE on adminSystemHealth. First source
is email failures (the original trigger — quote_sent template errors
left invoices unsent for 14h with no signal); more sources can be added.
Add an optional dark-mode logo (branding_logo_url_dark) alongside the
main logo. Upload/remove via the logo endpoint (?variant=dark) on the
Branding settings page. Admin header (admin dark mode) and the public
gallery (dark themes) pick the dark logo when active, falling back to
the light logo when unset. PDFs keep using the light logo.
Add a bills-gated button that opens the bill editor pre-filled with the
event (eventId FK + name/date snapshot) and the linked customer (when
exactly one). BillEditorPage gains eventId state + query-param prefill +
sends eventId on create; backend validates eventId (already forwarded +
persisted). Reuses the editor — no empty drafts. Does not auto-pull hours.
queueEmail gains options.respectBusinessHours: snaps the send time to the
next open business-hours block (from now), only deferring when it actually
falls outside hours. Applied to dunning reminders + gallery-expiry warnings;
transactional/admin-initiated mail stays immediate. No-op until business
hours are configured.
Add a paginated, filterable view of the email_queue (recipient, type,
status, queued/sent timestamps, error, event link) as a third tab in
Email config, beside SMTP + Templates. Filters: status, recipient/type
search, created-at range. email_data is never exposed. Pairs with the
"Send queued emails now" flush — flush, then watch what sent/failed.
Add a "Decline on behalf" action mirroring accept-on-behalf, for when a
customer says no by phone/email. Flips a draft/sent/expired quote to
declined, stamps declined_at, closes the public response window, and
invalidates outstanding accept/decline tokens so the emailed link can't
toggle it back. Optional free-text reason persisted to a new
quotes.decline_reason column (migration 115) and shown on the quote
detail page. Hard-delete intentionally not included.
Move the scheduled-email business-hours floor onto the business profile
as Google-style per-weekday opening blocks (multiple blocks/day for lunch
breaks). Migration 114 adds business_profile.business_hours (JSON) +
scheduled_email_floor_enabled; emailProcessor snaps a queued email to the
next open block, read in the profile timezone. Editor lives under
Settings → Business profile.
Add an admin "Send queued emails now" flush (POST /admin/email/flush-queue)
that drains the queue immediately, ignoring the business-hours floor — the
escape hatch before maintenance/updates. processEmailQueue now takes
{ignoreSchedule, limit} and returns send counts; the scheduled interval
run is unchanged.
When no customer is selected, list every customer with unbilled hour
entries — entry count, total hours, and open amount (resolved via the
override → customer-rate → install-default chain). Rows with no
resolvable rate are flagged "Rate not set" rather than undercounted.
Click a row to drill into the per-customer logging section.
Backend: getUnbilledSummaryByCustomer() + GET
/api/admin/customers/hour-entries/unbilled-summary (customers.view).
Hour-entry saves hard-failed with an English-only error when a customer
had no rate, and the standalone hours page showed a disabled rate field
that looked set. Add a global business_profile default_hourly_rate_minor
(migration 113) as the last link in the rate chain
(entry override → customer → install default), so saves succeed with the
global rate. When no rate resolves anywhere, replace the save-time error
with a read-only resolved-rate display + a CTA to set a customer or
install-wide rate, disable Add-entry until a rate/override exists, and
translate the backend HOURLY_RATE_REQUIRED toast (en+de).
Replace the sort <select> dropdowns on the invoice, quote and contract
list pages with clickable column headers that toggle asc/desc and show a
chevron indicator. Adds a shared SortableHeader component + useColumnSort
hook that maps clickable columns onto the server-side sort enum.
Make issue date (newest first) the standard sort on all three lists,
set at the frontend, route and service layers. Adds issue_asc/issue_desc
to invoices and an "Issued" column to the bills table so the default is
visible and toggleable. Extends sort coverage so every clickable column
has both directions (+customer_desc on all; +issue_asc/desc on
quotes/contracts). Storno rows remain listed.
The historical-invoice import form had no event field, so imported rows
landed with event_name = NULL even when the admin knew the occasion. Add
free-text Event name + Event date inputs to the import modal, thread them
through billsService.importHistorical and the POST /admin/invoices/import
validator, and store them in the event_name/event_date snapshot columns
(migration 107). event_id stays NULL — no FK, since the event may predate
picpeak. Autocomplete-to-event_id linking deferred as a future bonus.
Adds customer_accounts.skonto_disabled (migration 112) so a customer
that negotiated "no early-payment discount" can be flagged once instead
of ticking the per-invoice toggle on every invoice. resolveSkontoPercent
ForInvoice and the PDF render context both honour it, extending the
resolution chain to customer → invoice → snapshot → quote → global.
Checkbox added to the customer detail Billing card (en + de).
A scheduled invoice's issue_date was stamped at creation, so a long-
scheduled invoice printed a stale date by the time it shipped — the
relative Skonto window ("pay within N working days") and the net-days
due date were then counted from the authoring day, not the send day.
sendInvoice now stamps issue_date = send date on the first send and
re-derives the due date from it, preserving a manual due-date override.
Adds resolveNetDaysForRow to read net days from the persisted snapshot.
Deselecting Skonto before the scheduled send already propagates (the
scheduler re-reads the row fresh and the render context honours
skonto_disabled); no change needed there.
Due date now derives from (scheduled send date else issue date) plus the
selected Net-days template, both in the editor and on save. The bill
editor renders it read-only with an Override toggle for manual entry;
existing invoices preserve their stored due date. Backend adds a single
resolveNetDays resolver that honors the split payment-net-days template
(previously only the legacy FK was read) and the
crm_payment_default_net_days setting, used by createInvoice and the
installment-spawn path alike.
The invoice-import endpoint stamped sent_at and paid_at with the moment
of import (new Date()) instead of the document's historical dates. The
CRM dashboard "Revenue · last 30 days" card keys on paid_at, so a
year-old paid invoice imported today wrongly counted toward the rolling
window. The dashboard windowing is correct (cash-basis "received in the
window") — the bug was the wrong paid_at on imported rows.
POST /admin/invoices/import now anchors sent_at to issue_date and
paid_at to issue_date (or an optional new paidAt param when the admin
knows the real payment date), never to import time.
Migration 111 backfills rows imported under the old behaviour: for every
invoice with imported_pdf_path set, sent_at/paid_at are reset to
issue_date. The old code never captured a real payment date, so
issue_date is the only sensible anchor. Idempotent and scoped strictly
to imported rows, so picpeak-issued invoices are untouched.
paid_at/sent_at are operational timestamps, not the invoice's immutable
legal content, so correcting the import-time error is safe under the
§14/§11 UStG immutability rule.
Replace the free-text 2-char country code field on the inline customer
create form and the customer detail page with a dropdown that shows
localized country names (Intl.DisplayNames, no hardcoded map) while
still storing the ISO 3166-1 alpha-2 code. The create form now seeds the
default country from the business profile instead of leaving it blank or
guessing CH/FL. The free-text countryName override is kept for the rare
case where an operator wants a custom display string.
Standardize Liechtenstein on the ISO code LI instead of the colloquial
plate code FL so it matches the PDF renderer's locale-aware lookup and
the new dropdown. Migration 110 normalizes existing FL rows to LI on
customer_accounts and business_profile (idempotent, case-insensitive).
Require at least one human-readable identifier (company name or a
contact name) at create time so the form can't produce a nameless row
that's impossible to recognise in lists later. Enforced on both the
frontend (isValid + toast) and the backend POST /admin/customers
validator so the API can't be bypassed.
i18n: en + de updated; other locales fall back to inline English
defaults and should get a native review before release.
Caught during the round-4 e2e validation on real PG: every
successful restore landed with `status='completed', was_successful=false`
because the success-branch update only wrote `status` but not
`was_successful` (column default is false). Visible side effect: the
BackupDashboard's "last successful restore" filter would skip the
row + any future audit query gating on was_successful would miss it.
One-line cure: include `was_successful: true` in the success-branch
update payload. Inline comment explains why and references the
review note so future edits keep the two fields together.
Source-inspection test in restoreService.pgBranch.test.js pins the
contract: after `performPostRestoreVerification(...)`, the
`status: 'completed'` update payload must also contain
`was_successful: true`. Future refactors of the success payload that
drop the flag fail the test before merge.
36/36 backup-related integration tests pass.
End-to-end DR cycle surfaced one more PG-only landmine — and it
turned out to be a side-effect of the round-1 replay placement, not
a new bug. Round 2 fixed the comparison logic; round 3 fixes the
ordering.
Symptom on real PG install:
[install-from-backup] FAILED — Post-restore verification failed:
Table app_settings row count mismatch: expected 190, got 191.
Trigger file left in place for retry.
Root cause: the operator-meta replay (introduced in round 1) ran
INSIDE performDatabaseRestore, lined up BEFORE the post-restore
verification step in the parent restore() method. So:
1. psql restores app_settings → 190 rows (matches backup)
2. Replay upserts `restore_allow_force_auto_upgraded` (which the
fresh-install seeded but the backup didn't have) → 191 rows
3. performPostRestoreVerification counts 191, manifest says 190,
verification fails the row-count check.
Replay is doing the right thing (preserving operator policy). The
verification is doing the right thing (counts must match). They
disagree because the replay landed in the wrong sequence relative
to verification.
Cure: move the replay out of performDatabaseRestore and into
restore() AFTER `performPostRestoreVerification` passes.
Verification now sees the as-restored DB (matches the backup
exactly), replay layers on top once verification has signed off.
Mechanism: snapshot stashed on `this.preservedMetaSnapshot`
(initialised in constructor, reset per run at the top of restore()).
performDatabaseRestore writes it in the PG branch before DROP;
restore() drains it after verification. SQLite leaves it empty,
both steps no-op there.
Tests:
- Updated `restoreService.pgBranch.test.js` to pin the new shape:
* `this.preservedMetaSnapshot` is initialised in the constructor
* No stray `let preservedMeta = []` local declarations anywhere
* Replay drain (`this.preservedMetaSnapshot.length > 0`) sits in
restore() AFTER `performPostRestoreVerification(...)` and is
lexically OUTSIDE `performDatabaseRestore`.
- The bigint-as-string contract from round 2 still holds.
34/34 backup-related integration tests pass.
pg-driver serialises `bigint` (which is what `COUNT(*)` returns) as a
JavaScript STRING to preserve precision for huge counts. The manifest
stores `expected.rowCount` as a JS number (parseInt'd at
databaseBackup.js:118). Strict `!==` in performPostRestoreVerification
flagged every match as a mismatch on PG:
Table activity_logs row count mismatch: expected 16, got 16
Table admin_users row count mismatch: expected 1, got 1
Table app_settings row count mismatch: expected 165, got 165
... (every table, all matching)
Symptom matched the preservedMeta scope leak from round 1: install-
from-backup logged FAILED, trigger file wasn't cleaned, data was
actually intact. Caught on PR #596 e2e re-run.
Cure: coerce both sides with `Number(...)` at the comparison AND in
the interpolated value so the warning text renders `16` not `"16"`.
Pre-emptive: lines 448 + 458-459 had the same string-vs-number issue
masked by `>` (JS coerces operands for `>`), but the warning text
printed `"5"` on PG vs `5` on SQLite, and a future patch changing
`>` to `=== 0` or `!== expectedCount` would silently break on PG.
Coerced at the read site into `eventCountN` / `activeUsersN` locals
+ added a comment block explaining the contract so future edits
don't drop the Number() calls without re-auditing.
New source-inspection test: pins the contract that every `.count`
result in restoreService.js MUST be wrapped in `Number(...)` when
used in a comparison (===/!==/>/</>=/<=). Same source-inspection
pattern as the preservedMeta test added round 1 — pragmatic until
the real-PG integration test follow-up lands.
The maintainer's audit of the rest of the backup/restore surface
(_installFromBackupBoot, _restoreSettingsBoot, _backupPathsBoot,
backupCoverageService, backupIntegrityService, backupService,
databaseBackup) confirmed no other bigint-as-string sites — the
class is now closed in the audited scope.
Two nice-to-haves from the PR #596 review.
1. Install-from-backup logging mirrors to stdout
The winston logger writes to /app/logs/combined.log and may not
tee to stdout. Operators tailing `docker logs picpeak-beta-backend`
after a `compose up` saw the migration sweep + npm notice and
nothing about the restore. Three key events now also fire through
`console.log` with a `[install-from-backup] ` prefix:
- "trigger file detected → <manifest>"
- "starting restore from <manifest>"
- "restore completed successfully" / "FAILED — <reason>"
Plus the "skipping — existing data" branch.
docker-logs surface now tells the restore story without requiring
an `exec into the container` step.
2. ADMIN_CREDENTIALS.txt flags stale creds when restore is queued
Migration 001 detects a pending `RESTORE_ON_INSTALL` file BEFORE
writing the fresh-install credentials file. If a trigger will fire
on the next boot, the file now opens with a clear warning:
⚠️ RESTORE_ON_INSTALL TRIGGER DETECTED ⚠️
These credentials are temporary. An install-from-backup run is
queued to fire on the next server start, which will REPLACE
this admin row with the one from the backup. After the restore
completes, log in with your ORIGINAL pre-disaster credentials
— not the ones below. If the restore fails for some reason,
the credentials below remain valid as a fallback recovery path.
Doesn't skip the file (so a failed restore still has the fallback
credentials), just annotates it. Closes the maintainer's "stale
junk credentials" observation.
`preservedMeta` was declared with `let` INSIDE the PostgreSQL else
branch of performDatabaseRestore (~L850), then read AFTER the else
block closed at the shared replay site (~L1030). On every real PG
restore, this threw:
ReferenceError: preservedMeta is not defined
after psql had already loaded the data successfully. Knock-on
effects per the maintainer's review:
- Loud `Install-from-backup: FAILED` line in combined.log even
though the data restored cleanly
- Trigger file in `_installFromBackupBoot.js` was left in place
because the success branch never ran — admin had to manually
rm it before the next boot
- The operator-meta replay (restore_allow_force,
restore_allow_force_auto_upgraded) silently dropped, exactly
the chicken-and-egg the snapshot was added to close.
`restore_allow_force` reverted to the backup's value on every
PG restore.
CI missed it because integration tests around `performFullRestore`
only exercise the SQLite branch (`this.dbType === 'sqlite'`). The PG
branch requires a real psql binary + cluster, which lives in the
"real-PG integration test in CI" follow-up.
Cure: hoist the `const PRESERVED_META_KEYS = [...]` + `let
preservedMeta = []` declarations above the SQLite/PG split. SQLite
leaves them empty; PG branch fills them; replay block at the bottom
reads them on both paths (no-op on SQLite).
New test: `restoreService.pgBranch.test.js` pins the scope contract
via source inspection. Two assertions:
1. Exactly one `let preservedMeta = []` declaration in the file,
positioned before the SQLite/PG branch split
2. The replay block `if (preservedMeta.length > 0)` sits outside
the else block (closing ` }` exists between the branch
opener and the replay site)
Source-inspection beats a runtime test here because (a) it doesn't
need a real PG cluster + psql binary, (b) it pins the EXACT property
that broke, more directly than a runtime test would.
Closes PR #596 review blocker.
The AdminHeader "Clear All" notifications button has been 404'ing for
a while: frontend `notifications.service.ts` calls
`DELETE /admin/notifications/clear-all`, backend only defined
`DELETE /admin/notifications/clear-old`.
The /clear-old route was misleadingly named anyway — it tried to
delete read OR >30-days-old rows, then had a fallback that nuked
EVERY row when nothing matched. Both the frontend and the existing
test expect a simple Clear All shape, so just rename to /clear-all,
drop the tiered logic, and return the plain
`{ message, deletedCount }` payload the test asserts on.
The test (adminNotifications.test.js) was hiding the breakage —
it was on CI's --testPathIgnorePatterns ignore list and so never
ran. Two reasons it failed locally before this fix:
1. Route path mismatch (the actual #597 bug).
2. The mock only stubbed adminAuth — requirePermission lives in
its own middleware module and ran for real, 403'ing before
the handler. Add a passthrough mock for that too.
With both fixed, the test passes. Drop adminNotifications from the
CI ignore list so future regressions in this route fail loudly
instead of going to ground.
upstream/beta independently shipped 108_seed_sl_email_template_translations.js
(Slovenian email template translations) using the migration number
this branch had already claimed for 108_add_backup_paths.js. Knex's
filename-based ordering would have caused both to attempt the slot
at merge time.
Renamed via `git mv` so file history is preserved. All five
references updated in lockstep:
- backend/src/services/_backupPathsBoot.js (require + comments)
- backend/src/services/backupService.js (LEGACY_BACKUP_PATHS comment)
- 3 integration test files (require + "migration 108" prose)
- migration's own header comment, with a paragraph explaining the
rename so reviewers don't wonder why the number jumped
**No data-migration impact for installs that already ran the
108-named version** (Ralf's beta, primarily): the migration's body
is idempotent — createTable is guarded by `hasTable`, and the seed
uses `onConflict('path').ignore()`. So when 109 runs against an
install whose backup_paths table is already populated, both the
schema step and the seed step no-op cleanly. The orphaned
`108_add_backup_paths.js` row in the `migrations` tracking table
sits harmlessly alongside the new `109_add_backup_paths.js` row.
No data lost, no double-insert, no schema drift. Mechanical rename
ahead of the PR opening.
The four backup admin panes (BackupHistory, BackupDashboard,
BackupCoverageCard, BackupIntegrityCard) used raw date-fns
`format()` with hard-coded tokens like 'p' (12-hour AM/PM), 'PP',
'PPP', 'PPp', and 'yyyy-MM-dd HH:mm:ss' — ignoring the admin's
configured `general_date_format` and `general_time_format`
settings.
Net effect on a 24h-configured install: backup History row showed
"11:25 PM" instead of "23:25", and the Coverage tab's "Last dump"
+ "Coverage generated" timestamps were stuck on
yyyy-MM-dd HH:mm:ss regardless of the admin's date-format choice.
All four panes now route through `useLocalizedDate()` which honors
both settings + the active i18n locale (per the existing
[[feedback_respect_general_format_settings]] pattern).
Tokens replaced:
format(date, 'p') → formatTime(date)
format(date, 'PP') → format(date)
format(date, 'PPP') → format(date)
format(date, 'PPp') → formatDateTime(date)
format(date, 'yyyy-MM-dd HH:mm:ss') → formatDateTime(date)
format(date, 'yyyy-MM-dd HH:mm') → formatDateTime(date)
No backend changes — settings already shipped via /admin/settings;
this just makes the consumers actually read them.
Closes the six-step DR dance ("onboard throwaway admin → restore via
wizard → log out → log back in with originals") by letting admins
recover an install with zero clicks past `docker compose up`.
Convention: drop a file named `RESTORE_ON_INSTALL` (no extension OR
.txt) into the existing `/backup` bind mount. On next container
start, the new boot hook detects it, runs the restore, and starts
the server with the restored state. Admin opens the browser, login
works first try.
Payload variants:
- empty file → auto-picks newest backup-manifest-*.json from
/backup/manifests/. Useful for "restore the latest".
- path inside the file → uses that specific manifest. Useful for
"I want this older backup, not the most recent".
Safety gates (three layers):
1. Trigger file must exist — no auto-magic, admin signals intent
2. DB must be empty (no events, ≤1 admin) — refuses to clobber
production data
3. Restore failure leaves the trigger file in place for retry on
next container start. Success deletes it so subsequent boots
don't redo the work.
Override hook: INSTALL_FROM_BACKUP_FORCE=true skips guard #2 for the
"I know what I'm doing" edge case (dev env rebuilds, etc).
No docker-compose changes required — uses the bind mount picpeak
already has, env vars are optional. The minimal admin workflow now
matches the bare-minimum mental model: "copy my backup files,
restart the container, log in with original credentials."
Tests: 7 scenarios covering trigger detection, payload variants,
safety gates, success/failure trigger-file lifecycle.
The #592 fix added a devtools-detection probe, and the #592 follow-up
added a require_password probe + a branding-defaults whereIn().select().
Both shift the db() call indices the existing #550 test relied on, and
the branding probe needed `.select()` to resolve to an array (the mock
chain wasn't thenable, so `for..of` on the result threw → 500 on every
test that hit BASE_BODY).
Add `whereIn` + `selectResult` to buildChain so the branding probe
yields an iterable. Factor the three pre-slug app_settings chains into
a baseSettingsChains() helper and update each test's queued sequence
and toHaveBeenNthCalledWith / toHaveBeenCalledTimes expectations to
match the new shape. No behaviour change in v1/events.js — only the
test scaffolding moves.
Luca