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.
This commit is contained in:
Luca
2026-06-13 11:57:32 +02:00
parent a702f33004
commit 9d13880f2b
10 changed files with 240 additions and 93 deletions
+8 -2
View File
@@ -798,9 +798,13 @@ async function renderQueuedEmail(templateKey, variables = {}, to = '') {
// limit max emails per pass. The flush raises this to drain the
// whole queue in a single pass (no re-query, so a failing
// email isn't retried in a tight loop within one flush).
// onlyId when set, process EXACTLY this one queue row. Used by the
// cockpit "send now" so a forced send never sweeps up OTHER
// dead-lettered emails (those past the retry cap) just
// because ignoreSchedule also bypasses that cap.
//
// Returns { processed, sent, failed }.
async function processEmailQueue({ ignoreSchedule = false, limit = 10 } = {}) {
async function processEmailQueue({ ignoreSchedule = false, limit = 10, onlyId = null } = {}) {
logger.info('Email queue processor: Checking for pending emails...');
const result = { processed: 0, sent: 0, failed: 0 };
@@ -823,6 +827,9 @@ async function processEmailQueue({ ignoreSchedule = false, limit = 10 } = {}) {
const now = new Date();
const query = db('email_queue')
.where('status', 'pending');
// Targeted single-email flush (cockpit "send now"): scope to that row
// only, so we never force-retry other dead-lettered emails.
if (onlyId != null) query.where('id', onlyId);
if (!ignoreSchedule) {
// Automatic runs: respect the retry cap (don't hammer a failing
// address) AND the schedule (business-hours floor / future send).
@@ -868,7 +875,6 @@ async function processEmailQueue({ ignoreSchedule = false, limit = 10 } = {}) {
// 119 just skip it).
const sentUpdate = { status: 'sent', sent_at: new Date() };
try {
const { hasColumnCached } = require('../utils/schemaCache');
if (sendResult && sendResult.html && await hasColumnCached('email_queue', 'rendered_html')) {
sentUpdate.rendered_html = sendResult.html;
}