Skip to content

fix(journeys): lazy-upsert contact row so session persistence never violates FK (EVO-1929)#103

Open
daniloleonecarneiro wants to merge 1 commit into
developfrom
danilocarneiro/evo-1929-jornadas-persistencia-de-sessao-ainda-viola-fk-para-contato
Open

fix(journeys): lazy-upsert contact row so session persistence never violates FK (EVO-1929)#103
daniloleonecarneiro wants to merge 1 commit into
developfrom
danilocarneiro/evo-1929-jornadas-persistencia-de-sessao-ainda-viola-fk-para-contato

Conversation

@daniloleonecarneiro

Copy link
Copy Markdown

Summary

journey_sessions.contact_id carries FK_journey_sessions_contact_id to evo_campaign.contacts (ON DELETE CASCADE), but the evo-flow community surface never populates that table — ContactsService/ContactsClientService are HTTP proxies to the CRM (Rails), there is no contact sync/backfill.

EVO-1892 desacoplou apenas o START da jornada (persist best-effort, swallow do FK violation, sem sessão órfã). Porém a persistência DURANTE a execução (writes per-node vindas da Temporal activity via set()/updateSessionStatus()) continua usando o caminho durável (que faz throw). Resultado: para um contato existente só no CRM, a 1ª persistência de sessão runtime dispara violates foreign key constraint "FK_journey_sessions_contact_id" e a jornada falha no 1º nó. Contatos semeados em evo_campaign completam; não-semeados falham.

Decisão: upsert lazy (opção b) vs relaxar FK (opção c) vs sync (opção a)

Escolhida a opção (b) upsert lazy por ser a correção da causa-raiz mais cirúrgica que mantém a integridade referencial:

  • persistToDatabase agora garante uma linha mínima em contacts (INSERT INTO contacts (id) VALUES ($1) ON CONFLICT (id) DO NOTHING) antes de salvar a sessão.
  • O schema de contacts (ver 1745200000001-init-base-tables) exige apenas id; todas as outras colunas têm default no banco (name='', blocked=false, custom_attributes='{}'::jsonb, …) ou são nullable. Logo um insert só com id produz uma linha válida.
  • ON CONFLICT (id) DO NOTHING torna idempotente e nunca sobrescreve um contato real já sincronizado pelo CRM — seguro chamar em toda escrita de sessão.
  • Como persistToDatabase é o funil único de TODAS as escritas (start E runtime per-node), o fix resolve a causa-raiz nos dois caminhos de uma vez.

Opção (a) sync foi descartada (não há colunas/origem para sincronizar contatos CRM→evo_campaign de forma confiável no runtime). Opção (c) relaxar/remover a FK foi descartada por sacrificar integridade referencial sem necessidade — o upsert mínimo já satisfaz a FK.

Branch criada de origin/develop (que ainda não contém EVO-1892); o fix é complementar e independente do best-effort de start.

Test plan

  • npx tsc -b limpo.
  • npx jest src/modules/journeys src/modules/cache → 63/63 verde (10 suites), incluindo 3 novos testes EVO-1929:
    • upsert mínimo executa ANTES do save da sessão para contato só-CRM;
    • upsert idempotente roda também em writes runtime per-node (updateSessionStatus), não só no start;
    • upsert é pulado quando a sessão não tem contactId.
  • Falhas pré-existentes por falta de EVOAI_CRM_API_TOKEN (crm-client) são independentes; nenhuma nova introduzida.

Notas

  • AC1 (jornada para contato existente só no CRM executa os nós sem violar FK): satisfeito — o contato passa a existir como linha mínima antes da 1ª persistência.
  • Sem migration: a correção é puramente em runtime, preservando a FK.

… never violates contact FK (EVO-1929)

journey_sessions.contact_id carries FK_journey_sessions_contact_id to
evo_campaign.contacts (ON DELETE CASCADE), but the evo-flow community
surface never populates that table (ContactsService/ContactsClientService
are HTTP proxies to the CRM). EVO-1892 made only the journey START path
swallow the FK violation (best-effort persist); the per-node runtime
writes from the Temporal activity still go through the durable (throwing)
persist path, so the first per-node session save aborts the journey at
node 1 for any contact not pre-seeded into evo_campaign.contacts.

Fix (option b — lazy upsert): persistToDatabase ensures a minimal
contacts row (id only) via INSERT ... ON CONFLICT (id) DO NOTHING before
saving the session. The contacts table requires only id (every other
column has a DB default or is nullable), so an id-only row is valid; the
ON CONFLICT keeps it idempotent and never clobbers a real CRM-synced
contact. This resolves the root cause on ALL write paths (start and
runtime) while preserving referential integrity, instead of relaxing the
FK.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @daniloleonecarneiro, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@dpaes dpaes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved (merge held)

Verified the fix end-to-end. ensureContactRow runs unconditionally inside persistToDatabase — the single funnel for every set()/updateSessionStatus() — so it covers both the start path and the per-node runtime writes that EVO-1892 left throwing on the FK. The INSERT INTO contacts (id) ... ON CONFLICT (id) DO NOTHING is idempotent, concurrency-safe, and never clobbers a real contact.

On the headline concern (would the id-only insert violate NOT NULL on created_at/updated_at?): for this service's database (evo_campaign), contacts is evo-flow's own table from init-base-tables, where created_at/updated_at are NOT NULL DEFAULT CURRENT_TIMESTAMP — so an id-only insert is valid. A Postgres FK can't cross databases, and the e2e seeds evo_campaign.contacts, which confirms the FK target is evo-flow's own table, not the CRM's. So the fix works in the actual topology.

Non-blocking follow-ups:

  1. The code comment "The contacts table is owned by the CRM (Rails)" is inaccurate — it's evo-flow's own evo_campaign.contacts (init-base-tables). Please correct it. It's also a real deployment caveat: if evo-flow is ever pointed at the CRM's Postgres (evo_community), that contacts (Rails) has created_at/updated_at NOT NULL without a DB default and the id-only insert would fail — worth a one-line note.
  2. The 3 new tests mock manager.query (they assert the SQL is issued, not that a real DB accepts an id-only row). Please confirm verify-fixes.sh D1 is green without seeding as the runtime proof.
  3. Minor: a failed save leaves a committed id-only stub row (cosmetic — ON CONFLICT covers re-runs); the JSDoc "a missing/non-uuid id is skipped" actually fails loud, not skips.

Merge intentionally held pending the batch's merge ordering (this resolves EVO-1892's FK problem; #96's best-effort approach was rejected, and both touch the same file). #103 is clean vs develop and can merge standalone when ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants