mirror of
https://github.com/fsolla/teqo.git
synced 2026-11-13 06:43:06 +00:00
fix(campaign): D13 — align supporter_import_batch.actor_id schema drift (Closes #643)
Make actor_id nullable to match its ON DELETE set null FK; set SupporterImportBatch.actor required:false so config agrees with schema. Keep the deleteCampaignUserImportBatches hook + hand-deletes as explicit cleanup (no schema cascade, per webauthn precedent). Adds int coverage for account-delete with an orphan batch.
This commit is contained in:
parent
f8ead7c4b7
commit
e625c129fc
8 changed files with 195 additions and 8 deletions
1
docs/changelog/2026-08-24-d13.md
Normal file
1
docs/changelog/2026-08-24-d13.md
Normal file
|
|
@ -0,0 +1 @@
|
|||
**D13 — drift C6: `supporter_import_batch.actor_id` nullable (#643):** a migration `20260824_010000_make_supporter_import_batch_actor_nullable` alinha o schema — a coluna `actor_id` deixava de ser `NOT NULL` para concordar com a FK `ON DELETE set null` (a contradição original fazia o delete de um `campaignUser` dono de um lote não consumido falhar com 23502). O campo `actor` em `SupporterImportBatch` passa a `required: false` (config reflete o schema, sem drift futuro em `migrate:create`). O hook `deleteCampaignUserImportBatches` (CampaignUser.ts) e os hand-deletes em `personDelete`/`personCapacityExit` permanecem como cleanup explícito (evitam lotes órfãos até o sweep), seguindo o precedente de passkeys/notificações — sem `CASCADE` de schema. Cobertura: int em `personDelete.int.spec.ts` (lote com `actor` nulo é criável + delete de conta com lote órfão remove ambos sem violação).
|
||||
|
|
@ -0,0 +1,67 @@
|
|||
# Impl: Alinhar schema de `supporter_import_batch.actor_id` (NOT NULL ↔ ON DELETE set null)
|
||||
|
||||
Status: aprovado
|
||||
Atualizado em: 2026-08-24
|
||||
Issue: #643
|
||||
Intenção: docs/plans/drift-c6-supporter-import-batch-actor.md
|
||||
Appetite restante: herdado — correção de drift de schema pequena, sem novo comportamento de produto.
|
||||
|
||||
## Leitura da intenção
|
||||
|
||||
- **Outcome:** O schema de `supporter_import_batch` deixa de ser contraditório: `actor_id` passa a ser compatível com a FK `ON DELETE set null` existente, sem reintroduzir o erro de runtime que C111 já contornou no app.
|
||||
- **O que NÃO negociar:** Fail-closed em LGPD/consent (não afetado); contratos de URL pública (não afetado); o padrão repo de cleanup via hook (não via cascade de schema) para relações de `CampaignUser`.
|
||||
- **O que reavaliar:** Se o hook `deleteCampaignUserImportBatches` torna-se redundante após a mudança — reavaliado e mantido por higiene de dados (ver Abordagem).
|
||||
|
||||
## Abordagem recomendada
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
M[Atual: actor_id NOT NULL + FK set null] --> A[Migration: DROP NOT NULL]
|
||||
A --> F[Campo actor required:false em SupporterImportBatch.ts]
|
||||
F --> H[Hook deleteCampaignUserImportBatches MANTIDO]
|
||||
H --> T[int test: delete conta com batch orfão]
|
||||
T --> G[Gates: gate:fast + build + snapshot migration]
|
||||
```
|
||||
|
||||
**Opções consideradas:** A | B
|
||||
**Recomendação:** A — tornar `actor_id` nullable (migration `DROP NOT NULL` + campo `required:false`). Torna a FK `ON DELETE set null` coerente com a coluna, remove a contradição de schema, e não introduz cascade de schema (alinhado à convenção repo: cleanup por hook, não por cascade).
|
||||
**Rejeitadas:** B (CASCADE) — viola a convenção documentada do repo (CampaignUser.ts:325-343): cascade escrita à mão numa migration é drift revertido pelo próximo `migrate:create` (precedente webauthn).
|
||||
|
||||
### Componentes / mudanças
|
||||
|
||||
- **`src/collections/SupporterImportBatch.ts`** (`actor` field): alterar `required: false` para que a config Payload reflita o schema nullable e evite futuro drift em `migrate:create`. Reuso do campo/relação existente — nenhuma nova collection.
|
||||
- **Migration:** `2026XXXX_XXXXXX_make_supporter_import_batch_actor_nullable` — `ALTER TABLE supporter_import_batch ALTER COLUMN actor_id DROP NOT NULL;` no `up()`; `ALTER COLUMN actor_id SET NOT NULL;` no `down()`. Idempotente via `IF NOT EXISTS`/probe de constraint pg, seguindo o padrão hand-trimmed do repo (`push:false`).
|
||||
- **`src/collections/CampaignUser.ts:369`** (`deleteCampaignUserImportBatches`, wired em `beforeDelete` :443): **MANTIDO**. O schema fica coerente, mas o hook continua removendo as linhas de staging transitórias na deleção da conta (evita órfãos com `actor_id NULL` até o sweep de expiração). Precedente passkey/notificação respeitado.
|
||||
- **`src/utilities/people/personDelete.ts:374`** e **`src/utilities/people/personCapacityExit.ts:189`**: hand-deletes de batch permanecem inalterados (já não dependem do hook; não afetados).
|
||||
- **Access / Consent:** nenhuma mudança. Sem novo PII, sem nova chave Consent, fail-closed intacto.
|
||||
- **UI:** nenhuma mudança de superfície. Criação de batch (`supporterImportToken.ts:37`) continua passando `actor` — válido sob nullable.
|
||||
|
||||
### Dados → forma
|
||||
|
||||
Não aplicável: não há nova apresentação de dados; é correção de restrição de coluna.
|
||||
|
||||
## Fases verificáveis
|
||||
|
||||
1. **Tracer / schema+server** — criar migration `make_supporter_import_batch_actor_nullable` (up/down idempotente) + `required:false` no campo `actor`; rodar `pnpm generate:types` e `pnpm migrate` localmente. Quota: pequena, dentro do appetite.
|
||||
2. **Int test** — em `tests/int/personDelete.int.spec.ts` (caminho de deleção de `campaignUser` que dispara `beforeDelete`), adicionar caso: deletar conta com batch órfão e afirmar remoção limpa (sem órfão, sem falha de FK). `campaignFixtures.ts:819-826` já descobre/own batches, sem leak.
|
||||
3. **Gates** — `pnpm gate:fast`; `pnpm build` (aplica `payload migrate`); snapshot da migration validado (coluna nullable aplicada). Push via `pnpm push`.
|
||||
|
||||
## Rabbit holes / Não escopo
|
||||
|
||||
- `payload_locked_documents_rels` FK é cascade — não afetado, fora de escopo.
|
||||
- Tornar `actor` opcional NÃO quebra as actions de import (continuam passando `actor`).
|
||||
- Não escopo: remover o hook `deleteCampaignUserImportBatches` (seria "mais minimal" mas deixaria órfãos NULL — rejeitado).
|
||||
- Não escopo: mudar semântica de expiração/sweep dos batches.
|
||||
|
||||
## Riscos e mitigação
|
||||
|
||||
- **Regressão do padrão passkey/notificação:** intacto — hook mantido, sem alteração de access. Mitigação: int test na Fase 2 cobre o path.
|
||||
- **Linhas órfãs com NULL actor:** evitadas mantendo o hook + hand-deletes. Mitigação: teste afirma remoção na deleção da conta.
|
||||
- **Idempotência em DBs onde a migration já rodou / NOT NULL já removido:** usar probe de constraint + `IF NOT EXISTS` no up/down. Mitigação: re-execução segura em trabalhos de deploy.
|
||||
|
||||
## Aceite de engenharia
|
||||
|
||||
- [ ] Aceite de produto da intenção ainda coberto (schema não-contraditório, sem regressão de runtime)
|
||||
- [ ] Invariantes AGENTS/engineering-standards (migrations commitadas, `push:false`, idempotência, padrão hook-based)
|
||||
- [ ] Testes de domínio previstos (int test de delete de conta com batch órfão em `personDelete.int.spec.ts`)
|
||||
- [ ] Self-score decision-quality ≥4 (decisão cara com rejeitadas; appetite respeitado; rabbit holes nomeados; reuso de hook/fixtures existentes; outcome de produto inalterado)
|
||||
|
|
@ -359,11 +359,11 @@ const deleteCampaignUserNotifications: CollectionBeforeDeleteHook = async ({ id,
|
|||
|
||||
/**
|
||||
* Staged CSV-import batches authored by the deleted account must go BEFORE the
|
||||
* account: `supporter_import_batch.actor_id` is NOT NULL with an `ON DELETE
|
||||
* set null` FK (C6 schema drift — the column and the FK action disagree), so
|
||||
* without this the delete fails with a not-null violation whenever the account
|
||||
* left an unconsumed batch (10-minute window, no sweep). Same pattern as the
|
||||
* passkey/notification cascade above; `deletePersonRecord` spells the same
|
||||
* account. D13 aligned the schema (`actor_id` is now nullable, matching the
|
||||
* `ON DELETE set null` FK — issue #643), so the FK no longer violates NOT NULL;
|
||||
* the hook still deletes the transient staging rows explicitly rather than
|
||||
* leaving orphaned NULL-actor batches until the expiry sweep. Same pattern as
|
||||
* the passkey/notification cascade above; `deletePersonRecord` spells the same
|
||||
* cleanup by hand.
|
||||
*/
|
||||
const deleteCampaignUserImportBatches: CollectionBeforeDeleteHook = async ({ id, req }) => {
|
||||
|
|
|
|||
|
|
@ -47,7 +47,7 @@ export const SupporterImportBatch: CollectionConfig = {
|
|||
type: 'relationship',
|
||||
relationTo: 'campaignUser',
|
||||
label: 'Operador',
|
||||
required: true,
|
||||
required: false,
|
||||
index: true,
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -0,0 +1,54 @@
|
|||
import { MigrateDownArgs, MigrateUpArgs, sql } from '@payloadcms/db-postgres'
|
||||
|
||||
/**
|
||||
* Reconciles production schema drift (drift C6, D13 / issue #643):
|
||||
* `supporter_import_batch.actor_id` was created as `integer NOT NULL` in
|
||||
* `20260719_011015_add_supporter_import_batch`, but its FK is `ON DELETE set
|
||||
* null` — a contradiction. Deleting a campaignUser that owns an unconsumed
|
||||
* batch failed with a not-null violation (23502) on the FK set-null UPDATE.
|
||||
*
|
||||
* C111 already made the delete safe at runtime via the
|
||||
* `deleteCampaignUserImportBatches` beforeDelete hook (and the hand-deletes in
|
||||
* `personDelete`/`personCapacityExit`), but the schema stayed contradictory.
|
||||
* This migration drops the NOT NULL so the column agrees with the `ON DELETE
|
||||
* set null` FK, matching the Payload config (`SupporterImportBatch.actor` is
|
||||
* now `required: false`). No schema cascade is introduced — the repo resolves
|
||||
* this class of cleanup with a hook, not a migration cascade (webauthn
|
||||
* precedent).
|
||||
*
|
||||
* Idempotent: only drops NOT NULL when the column is still NOT NULL, so it is a
|
||||
* no-op on any database already reconciled.
|
||||
*/
|
||||
export async function up({ db }: MigrateUpArgs): Promise<void> {
|
||||
await db.execute(sql`
|
||||
DO $$
|
||||
BEGIN
|
||||
IF EXISTS (
|
||||
SELECT 1 FROM information_schema.columns
|
||||
WHERE table_schema = 'public'
|
||||
AND table_name = 'supporter_import_batch'
|
||||
AND column_name = 'actor_id'
|
||||
AND is_nullable = 'NO'
|
||||
) THEN
|
||||
ALTER TABLE "supporter_import_batch" ALTER COLUMN "actor_id" DROP NOT NULL;
|
||||
END IF;
|
||||
END $$;
|
||||
`)
|
||||
}
|
||||
|
||||
export async function down({ db }: MigrateDownArgs): Promise<void> {
|
||||
await db.execute(sql`
|
||||
DO $$
|
||||
BEGIN
|
||||
IF EXISTS (
|
||||
SELECT 1 FROM information_schema.columns
|
||||
WHERE table_schema = 'public'
|
||||
AND table_name = 'supporter_import_batch'
|
||||
AND column_name = 'actor_id'
|
||||
AND is_nullable = 'YES'
|
||||
) THEN
|
||||
ALTER TABLE "supporter_import_batch" ALTER COLUMN "actor_id" SET NOT NULL;
|
||||
END IF;
|
||||
END $$;
|
||||
`)
|
||||
}
|
||||
|
|
@ -63,6 +63,7 @@ import * as migration_20260819_233503_add_campaign_demand_responsibles from './2
|
|||
import * as migration_20260820_003426_add_instagram_sync_status from './20260820_003426_add_instagram_sync_status';
|
||||
import * as migration_20260823_161905_add_municipality_update_deliberation from './20260823_161905_add_municipality_update_deliberation';
|
||||
import * as migration_20260824_000000_reconcile_allocation_decision_rationale_nullable from './20260824_000000_reconcile_allocation_decision_rationale_nullable';
|
||||
import * as migration_20260824_010000_make_supporter_import_batch_actor_nullable from './20260824_010000_make_supporter_import_batch_actor_nullable';
|
||||
|
||||
export const migrations = [
|
||||
{
|
||||
|
|
@ -390,4 +391,9 @@ export const migrations = [
|
|||
down: migration_20260824_000000_reconcile_allocation_decision_rationale_nullable.down,
|
||||
name: '20260824_000000_reconcile_allocation_decision_rationale_nullable'
|
||||
},
|
||||
{
|
||||
up: migration_20260824_010000_make_supporter_import_batch_actor_nullable.up,
|
||||
down: migration_20260824_010000_make_supporter_import_batch_actor_nullable.down,
|
||||
name: '20260824_010000_make_supporter_import_batch_actor_nullable'
|
||||
},
|
||||
];
|
||||
|
|
|
|||
|
|
@ -857,7 +857,7 @@ export interface Supporter {
|
|||
export interface SupporterImportBatch {
|
||||
id: number;
|
||||
batchId: string;
|
||||
actor: number | CampaignUser;
|
||||
actor?: (number | null) | CampaignUser;
|
||||
expiresAt: string;
|
||||
rows:
|
||||
| {
|
||||
|
|
|
|||
|
|
@ -28,7 +28,8 @@ const countRemaining = async (
|
|||
| 'stateDeputy'
|
||||
| 'campaignUser'
|
||||
| 'votePledge'
|
||||
| 'supporter',
|
||||
| 'supporter'
|
||||
| 'supporterImportBatch',
|
||||
where: Where,
|
||||
): Promise<number> => {
|
||||
const result = await payload.find({
|
||||
|
|
@ -185,3 +186,61 @@ describe('C100 — apagar pessoa (manifest + cascata)', () => {
|
|||
expect(manifest).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('D13 — delete de conta com batch de importação órfão (drift C6)', () => {
|
||||
beforeAll(async () => {
|
||||
payload = await getPayload({ config: await config })
|
||||
})
|
||||
|
||||
it('applies the migration: supporter_import_batch.actor_id accepts a null actor', async () => {
|
||||
// The migration dropped NOT NULL, so a NULL `actor` (the FK set-null result
|
||||
// on account delete) is a valid row — proving the column now agrees with
|
||||
// the `ON DELETE set null` FK.
|
||||
const orphan = await payload.create({
|
||||
collection: 'supporterImportBatch',
|
||||
data: {
|
||||
batchId: `null-actor-batch-${Date.now()}`,
|
||||
actor: null,
|
||||
expiresAt: new Date(Date.now() + 10 * 60 * 1000).toISOString(),
|
||||
rows: [{ ok: true }],
|
||||
},
|
||||
depth: 0,
|
||||
overrideAccess: true,
|
||||
})
|
||||
expect(orphan.actor).toBeNull()
|
||||
|
||||
await payload.delete({
|
||||
collection: 'supporterImportBatch',
|
||||
id: orphan.id,
|
||||
depth: 0,
|
||||
overrideAccess: true,
|
||||
})
|
||||
})
|
||||
|
||||
it('deletes a campaignUser that owns an unconsumed import batch without a not-null violation', async () => {
|
||||
const fixtures = campaignFixtures()
|
||||
const actor = await fixtures.createCampaignUser('coordinator')
|
||||
|
||||
await payload.create({
|
||||
collection: 'supporterImportBatch',
|
||||
data: {
|
||||
batchId: `orphan-batch-${actor.id}-${Date.now()}`,
|
||||
actor: actor.id,
|
||||
expiresAt: new Date(Date.now() + 10 * 60 * 1000).toISOString(),
|
||||
rows: [{ ok: true }],
|
||||
},
|
||||
depth: 0,
|
||||
overrideAccess: true,
|
||||
})
|
||||
|
||||
// The beforeDelete hook (`deleteCampaignUserImportBatches`) removes the
|
||||
// batch first; with the column now nullable the FK set-null path is also
|
||||
// safe, so the delete must not throw 23502 (not-null violation).
|
||||
await expect(
|
||||
payload.delete({ collection: 'campaignUser', id: actor.id, overrideAccess: true }),
|
||||
).resolves.toBeDefined()
|
||||
|
||||
expect(await countRemaining('campaignUser', { id: { equals: actor.id } })).toBe(0)
|
||||
expect(await countRemaining('supporterImportBatch', { actor: { equals: actor.id } })).toBe(0)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue