Reviewer feedback on #555: nextQuoteNumber inside createQuote's
db.transaction was called without passing the outer trx, so
claimNextSequence opened its own connection — Postgres tolerated this
via the pool, SQLite (1-connection default) deadlocked on every quote
creation.
Audited the same pattern across invoiceService + contractService and
found five more matching call sites:
- createInvoice (single-row path after installment auto-route)
- spawnInstallmentInvoices (per-sibling claim inside the loop)
- createStorno
- createContract
- createFromQuote
All now thread trx through to nextXxxNumber → claimNextSequence so
the claim joins the caller's transaction on both engines.
convertToInvoiceOnly's Path B (standalone-contract) is the lone
remaining nextInvoiceNumber() call without trx — that path isn't
wrapped in a transaction at all (separate concern: sequence-number
leak on insert failure, tracked separately).
Previous fix (45f0606) papered over the bug by changing the German
wording from "innerhalb von {{minutes}} Minuten" to "bis {{at}}" —
that worked but changed the UX intent. The original German wording
("you have N minutes left") was deliberate and clearer than an
absolute clock time; the actual bug was that no caller ever computed
`minutes` from `responseLockedAt`.
Revert the DE translation to its original wording, then build a
{ at, minutes } object at the call site so EN ("until {{at}}") and
DE ("innerhalb von {{minutes}} Minuten") each pick up the variable
they need. `minutes` rounds UP so a 14m 32s remainder displays as
"15 Minuten" rather than promising 14 the customer can't actually
hit.
The German string used `{{minutes}}` while the call site at
QuoteResponsePage.tsx:300 passes `{ at: <localized time> }`, matching
the English string's `{{at}}`. Result on the public quote page when
the customer had already responded: the literal text "{{minutes}}"
rendered instead of the unlock time.
Switched the German wording to match the English semantics
("until X:XX") since the underlying value is an absolute time, not a
minutes-remaining count — the previous DE wording was also wrong about
WHAT the variable meant.
The CRM template seeders (crmEmailTemplates / contractEmailTemplates /
eventReminderTemplates) were idempotent and ready, but only
contractEmailTemplates was actually called (lazily, by contractService
sends). crmEmailTemplates had no caller anywhere — every install that
didn't pre-exist its templates failed every quote_sent / invoice_sent /
storno_issued / invoice_reminder_* send with "Email template '<key>'
not found". The queue processor retries 3 times then leaves the row
in status='pending', retry_count=3, silently dead with no admin
surface (see project_crm_backlog for the eventual System Health page).
Fix: wire all three seeders into server.js startServer() right before
startEmailQueueProcessor. The new _emailTemplateBoot.js orchestrates
all three and then, for any template_key it just inserted, resets
retry_count on stuck email_queue rows of that email_type so the
queue processor's next tick picks them back up. Recovery is targeted:
unrelated retry-exhausted rows (e.g. SMTP-timeout failures) are not
touched.
Integration test boots a fresh CRM DB, pre-seeds a stuck quote_sent
row plus an unrelated stuck row, runs the boot helper, and asserts:
templates landed, stuck quote_sent row was reset, unrelated row was
left alone.
Already-deployed installs heal automatically on the next backend
restart after this lands.
Wires customer_accounts.billing_email into the invoice, Storno, and
payment-reminder send paths. Previously the column existed on the
schema and the customer-detail page rendered an input for it, but no
send path read it — every outbound email landed on customer_accounts.email
regardless. That mismatch is the failure mode flagged in
feedback_data_driven_completeness: a UI field that promises behavior
the backend silently doesn't deliver.
Routing matrix:
- invoice / Storno / payment reminder
To: billing_email (fallback email when unset)
CC: email (when billing_email took the To slot) + per-doc cc_pdf_email
- quote / contract / event reminder / gallery share
To: email (unchanged — decision-maker address)
- payment-check / paid-notification
To: admin contact (unchanged — internal flow)
A new resolveBillingRecipients helper centralises the rules:
prefer billing_email, dedupe addresses case-insensitively, keep
per-doc cc_pdf_email as a supplemental CC. Lives in its own file
(_billingRecipients.js) to match the _renderContext.js convention.
Drops the isInt({ min: 0 }) constraint on lineItems.*.unitPriceMinor
in both the adminInvoices and adminQuotes POST/PUT validators so
admins can add Treuerabatt / Frühbucherrabatt rows as standalone
negative-priced lines (matches standard DE/CH invoice practice).
A service-layer guard rejects saves whose computed total goes below
zero (INVOICE_TOTAL_NEGATIVE / QUOTE_TOTAL_NEGATIVE, both 400) so a
mis-typed discount can't accidentally mint a credit-balance invoice
that would masquerade as a regular row in dashboards. Credit notes
still belong in the Storno path (createStorno), which is unchanged.
Quote-side integration coverage is omitted for now — createQuote's
cold-require path takes ~30s under the test harness; the invoice
test exercises the same validator + guard shape.
The pre-event reminder feature shipped with 5 seeded templates
(event_reminder_default + wedding/birthday/corporate/other) but the
CRM → Development "Send any CRM email to me" picker only listed the
quote/invoice/contract templates. Maintainer can now eyeball each
reminder category's body without staging a real event.
Backend:
- Extend TEMPLATES_KEYS in adminDev.js with all 5 reminder keys.
- Add event_date (today+2d), days_before (2), business_name (from
business_profile.legal_name) to the common payload so the
{{tokens}} in the reminder bodies resolve.
Frontend:
- Extend CrmEmailTemplateKey union.
- Add TEMPLATE_LABEL_KEYS entries.
- EN+DE i18n labels under crmDev.templates.label.event_reminder_*.
No PDF attachment — reminders are body-only emails (matches the
real flow).
The full reminder-email implementation (eventReminderService,
eventReminderTemplates self-heal, ReminderTemplatesPage,
EventReminderOverrideCard) shipped in the CRM bundle but the
FeaturesTab card kept lockedReason=NOT_YET_AVAILABLE — so the
working feature was invisible.
Flip the card to the same shape as customerPortal: status="beta",
real setFlag handler, no disabled/lockedReason. The sub-tab in
Settings → Reminder templates already self-mounts when the flag
is on, and the per-event override card already self-renders on
the event detail page.
Description copy + EN/DE i18n updated to describe what the feature
actually does (per-category pre-event nudge) instead of the old
"coming soon" placeholder.
Adds two pieces:
- __tests__/integration/helpers/crmDb.js — boots a temp-SQLite test
DB by invoking every migrations/core/*.up() directly. Bypasses
knex's Migrator because its exclusive write lock deadlocks
001_init's nested initializeDatabase() call. ~1 second cold start.
- __tests__/integration/crmSchema.test.js — 36 assertions on the
table + column layout after the consolidated CRM migration runs.
Pins:
- every CRM table present (quotes, contracts, invoices + the
eight supporting tables)
- deal_uuid columns on all three lineage tables (the column
DocumentLineageCard joins on — drop it anywhere and the card
silently returns partial data)
- back-pointer FKs (converted_contract_id, source_contract_id,
source_quote_id) — the exact columns that triggered the
Postgres FK-ordering bug fixed earlier in this PR
- Storno discriminator (kind, cancels_invoice_id, replaces_
invoice_id) per feedback_storno_filter_everywhere
- event time columns from migration 137
A full quote→contract→invoice lineage walk is deferred — quote
service's nextQuoteNumber() opens an inner transaction from inside
the createQuote outer transaction, which deadlocks SQLite's default
1-connection pool. Postgres dev DBs never see it. Either fix the
service to thread trx through, or run lineage tests against a real
Postgres in CI (mirror schema-drift.yml). Filed as separate work.
The suites already existed (538 backend tests, 40 frontend tests, with
solid CRM coverage on quoteService/contractService/invoiceService/
customerHoursService/eventService.calendar) but no CI workflow invoked
them. Wire both into a single Tests workflow that triggers on any push
or PR to main/beta.
Six backend suites are excluded — they fail on upstream/beta too
(supertest fixture + knex mock chain issues unrelated to CRM). The
explicit ignore pattern keeps the workflow green on day 1; each
excluded suite is listed inline as test-infra debt to fix individually.
Backend job pins SKIP_S3_TESTS=true (the same default the test setup
file applies) so the backup-service integration doesn't try a real S3
round-trip when no MinIO is provisioned.
Two upstream tests regressed because the CRM PR added expected behavior
they didn't anticipate:
- galleryOgService.shareImage.test.js: formatEventDate is now async and
routes through utils/dateFormatter so the OG card respects the admin's
general_date_format setting (per feedback_respect_general_format_settings).
That adds a third db('app_settings') call on every buildOgMetadata path.
Mock the formatter module directly — the format itself is irrelevant
to the cover-vs-logo contract this file pins.
- customerAccountsService.test.js: createInvitation now allows a duplicate
email when the existing row is PASSIVE (password_hash IS NULL) — that's
the "promote passive customer to portal" path. The active-customer
rejection mock now has to set password_hash so the guard fires.
Both are test-only changes; no service code touched.
quotes.converted_contract_id and invoices.source_contract_id were
declared with inline FKs to contracts(id), but contracts is created
later in the same migration. SQLite accepted the forward reference;
Postgres rejected it ("relation \"contracts\" does not exist"), which
broke the Schema drift (#530) workflow and any fresh Postgres install.
Same pattern as events.hero_photo_id → photos.id in db.js: declare the
column without a constraint, then add the FK in a separate alterTable
after both sides exist. Wrapped in try/catch so re-runs against a DB
that already has the constraint are a no-op.
Verified locally against the #530 recovery scenario (initializeDatabase
then migrate:safe) and the fresh-install path: both converge cleanly,
both FKs land on the expected tables.
Adds a top-level disclaimer section to README + a dedicated
docs/crm-disclaimers.md spelling out two areas where picpeak ships
defaults the operator MUST review before going live:
1. Contract blocks (image rights, NDA, model release, cancellation,
jurisdiction, …) — written by the maintainer, NOT by a lawyer.
Every operator must have their lawyer review and adapt them
before sending any contract to a customer.
2. QR-bills and SEPA EPC payloads — rendered from the data the
operator typed. Picpeak is open source; we recommend scanning a
test invoice with the operator's bank app to verify the QR
actually works.
Matches the on-screen amber disclaimers already shown on the
Contract Block Library page and the Business Profile payment-block
editor.
~940 new keys per primary locale covering every CRM surface:
quote / invoice / contract editor + list + detail + public response
pages, calendar, hours, tax report, deals lineage, reminder emails,
feature toggles, settings tabs, error toasts.
en.json + de.json are hand-translated by the maintainer and are
authoritative. fr / nl / pt / ru received the same key set but
machine-derived strings — flagged for native review in the PR
description per project policy (see memory feedback_translation_flagging).
3-way merge note: 1 conflict (fr.json) hand-resolved to keep
upstream's improved phrasing for previewLayout / livePreview /
heroPlaceholderText alongside feat/crm's pdfTypography keys.
Previously these locales fell through to en for every customer.* /
customers.* / settings.customerSurface / settings.features.customerPortal
key. Machine-translated and flagged in the PR description as needing
native review.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Keeps the customer-surface branding toggles adjacent to the other
brand-visibility controls instead of floating at the bottom of the
page, where they were easy to miss.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Adds back the "Show logo" / "Show company name" toggles for the
customer dashboard, scoped to /customer/* surfaces only. Lives as a
dedicated card at the bottom of Settings → Branding, gated by the
customerPortal feature flag so admins who haven't enabled the portal
don't see it.
* Backend: restored GET/PUT /admin/settings/customer-surface
endpoints, whitelisted only to the two branding keys
(customer_show_logo, customer_show_company_name). The
calendar/quotes/bills feature globals that used to live on this
endpoint are now driven by the Features tab (feature_flags table).
* customerAccountsService.getCustomerSurfaceGlobals() reads from
app_settings again so /api/customer/auth/session honours the
toggles in its branding payload.
* New CustomerDashboardBrandingCard component with its own save
flow — separate from the main BrandingPage payload so flipping a
toggle doesn't replay the full branding mutation.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
RequireFeature calls useFeatureFlags(), which throws unless mounted
inside FeatureFlagsProvider — and that provider only wraps
AdminLayout. So unauthenticated visitors hitting /customer/login
crashed into the React error boundary with 'Oops! Something went
wrong'.
The customerPortal flag continues to hide every admin-side surface
(sidebar entry, /admin/customers routes, CustomerAccountPicker on
event forms), which is what the flag is actually for. The
customer-side tree stays reachable so existing customers can still
log in even if the admin flips the flag off temporarily.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The route was registered in upstream/beta's server.js but dropped
during the rebase squash — the Features tab GET/PUT both 404'd, so
the customerPortal flag (and every other flag) couldn't be toggled.
Restored the mount in its upstream/beta position.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
server.js was still requiring ./src/middleware/requireCustomerPortal
— a file deleted during the AdvancedFeaturesTab cleanup — which
crashed the backend on boot in production (MODULE_NOT_FOUND).
The customerPortal feature flag is now enforced on the frontend via
<RequireFeature flag="customerPortal" /> route guards (App.tsx) and
AdminSidebar visibility. Defence in depth is provided by
customerAccountsService.isCustomerPortalEnabled() in adminEvents.
Routes themselves are still protected by adminAuth / customerAuth.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The customer-portal squash inadvertently reverted the upstream/beta
fix from PR #427: production NODE_ENV was flipping the cookie Secure
flag back to hard `true`, which broke admin login on
HTTPS-frontend → HTTP-backend reverse-proxy stacks (browser drops
the Secure cookie over HTTP, login loops indefinitely).
Restored upstream/beta's tokenUtils.js verbatim and re-layered only
the customer cookie helpers (CUSTOMER_COOKIE_NAME,
setCustomerAuthCookie, clearCustomerAuthCookie,
getCustomerTokenFromRequest) on top.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Implements the recurring-customer login surface from
the-luap/picpeak#354 plugged into the maintainer's
new feature-flag infrastructure (PR #443) instead of
a parallel toggle.
* New `customerPortal` feature flag (foundation flag for the
not-yet-built calendar/quotes/bills/messaging customer
surfaces). Defaults FALSE on fresh installs, TRUE on existing
installs (events > 0) via migration 095 so live customer
accounts don't disappear mid-deployment.
* Foundation schema: customer_accounts, customer_invitations,
event_customer_assignments, customer_password_resets, plus
RBAC permissions customers.view / .create / .delete granted
to super_admin + admin system roles.
* Backend: /api/admin/customers (invite, list, search, assign,
deactivate, reset password) + /api/customer/auth/* +
/api/customer/* (login, dashboard, accept-invite, reset).
Customer JWT bypass minted via
/api/customer/events/:slug/access-token so existing gallery
middleware stays untouched.
* Frontend: /customer/* route tree gated by RequireFeature flag
customerPortal, with login / dashboard / accept-invite /
reset pages and a customer-side sidebar layout.
/admin/customers and /admin/customers/:id gated identically.
* Settings → Features grows a "Customers" section with a
Customer portal card. The maintainer's Features tab stays the
single source of truth — no parallel Advanced features tab.
* CustomerAccountPicker on event create/edit forms hides itself
when the flag is off; backend ignores customer_account_ids in
that case instead of erroring the whole event save.
Translations: en + de hand-translated. nl/pt/ru fall through to
en — flagged here as needing native review.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Addresses the-luap/picpeak#386 — gallery header layout cleanup.
- Drop the redundant "Menu" text label; menu button is icon-only with
tight padding (p-2).
- Absolute-position the menu icon at the very left of the header so it
no longer pushes the logo right with every other action. Logo wrapper
picks up pl-12 sm:pl-14 only when a menu button is rendered, so the
icon and logo don't overlap. When no menu button (controlsStyle:
classic), logo is flush with .container.
- New accent-coloured "Download" CTA placed immediately left of Logout.
Always visible when downloads are allowed; replaces the previous
primary-coloured "Download All" header button. Same CTA appears in
standard, hero, and minimal headers. Intentionally NOT shown in the
no-header variant (chromeless by design).
- Coloured via var(--color-accent) inline so the button automatically
tracks whatever palette the admin has chosen — works on plain beta
today (#22c55e) and auto-upgrades to the CI accent when #400 lands.
The sidebar's own Download All is untouched. Old showDownloadAll prop
stays on GalleryLayout for back-compat; GalleryView now passes
showDownloadAll={false} so only the new accent button renders in the
header.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Luca