mirror of
https://github.com/fsolla/teqo.git
synced 2026-11-13 15:13:07 +00:00
fix(E16): reconcile allocation_decision.rationale NOT NULL drift
Drop the NOT NULL constraint on allocation_decision.rationale to align the DB schema with the Payload config (required: false since B134). Hand-written idempotent migration following the consent_text_to_jsonb precedent. Adds idempotency test in campaignMigrationReconciliation. Closes #371
This commit is contained in:
parent
a79d2d3790
commit
2c7d55dcbe
4 changed files with 153 additions and 0 deletions
69
docs/plans/reconciliar-rationale-alocacao-impl.md
Normal file
69
docs/plans/reconciliar-rationale-alocacao-impl.md
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
# Impl: Reconciliar drift — `allocation_decision.rationale` NOT NULL vs config
|
||||
|
||||
Status: rascunho
|
||||
Atualizado em: 2026-08-24
|
||||
Issue: #371
|
||||
Intenção: docs/plans/reconciliar-rationale-alocacao.md
|
||||
Appetite restante: ~0,25 dia eng
|
||||
|
||||
## Leitura da intenção
|
||||
|
||||
- **Outcome:** O campo `rationale` na collection `allocationDecision` deve ser nullable no banco de dados, alinhando o schema real com a config (`required: false`). O drift que faz `pnpm migrate:create` re-emitir `ALTER TABLE allocation_decision ALTER COLUMN rationale DROP NOT NULL` deve desaparecer.
|
||||
- **O que NÃO negociar:** A config `required: false` já está correta (B134). O hook `validateAllocationDecision` já permite `rationale` vazio/null para `outcome: 'movimento'`. A mudança é apenas de schema.
|
||||
- **O que reavaliar:** Nenhuma hipótese incerta — o problema é um drift factual documentado.
|
||||
|
||||
## Abordagem recomendada
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
A[Migration idempotente] --> B[Test idempotência]
|
||||
B --> C[Verificar drift zero]
|
||||
```
|
||||
|
||||
**Opções consideradas:**
|
||||
|
||||
- **A: Migration hand-written idempotente** — seguir o padrão `consent_text_to_jsonb.ts`: check `information_schema.columns` antes de alterar. Seguro, idempotente, testado.
|
||||
- **B: `pnpm migrate:create` gerado** — deixar o Payload gerar a migration. **Rejeitado:** o Payload pode gerar um diff mais amplo (drop/recreate de colunas relacionadas) e não garante idempotência.
|
||||
- **C: Editar a migration inicial** — **Rejeitado:** viola o contrato de que migrations são história congelada.
|
||||
|
||||
**Recomendação:** Opção A — porque é o padrão estabelecido no repo, idempotente, e mínima.
|
||||
|
||||
### Componentes / mudanças
|
||||
|
||||
- **Migration:** `src/migrations/<timestamp>_reconcile_allocation_decision_rationale_nullable.ts` — hand-written, idempotent, segue padrão `consent_text_to_jsonb.ts`
|
||||
- **Registro:** `src/migrations/index.ts` — adicionar entrada após a última migration existente
|
||||
- **Teste:** `tests/int/campaignMigrationReconciliation.int.spec.ts` — adicionar caso que verifica idempotência (executar duas vezes, segunda é no-op)
|
||||
- **Access / Consent:** Nenhuma mudança — o hook `validateAllocationDecision` já valida corretamente
|
||||
- **UI:** Nenhuma mudança
|
||||
|
||||
### Dados → forma
|
||||
|
||||
N/A — mudança de schema apenas.
|
||||
|
||||
## Fases verificáveis
|
||||
|
||||
1. **Migration** — criar arquivo hand-written com guard `is_nullable = 'NO'` + `DROP NOT NULL`; registrar em `index.ts`; aplicar em dev e test
|
||||
2. **Teste** — adicionar caso em `campaignMigrationReconciliation.int.spec.ts` que:
|
||||
- Executa a migration duas vezes
|
||||
- Verifica que a segunda execução é no-op (schema inalterado)
|
||||
- Verifica que `rationale` é nullable após a migration
|
||||
3. **Verificação de drift** — rodar `pnpm migrate:create <probe>` e confirmar que não re-emite mais o ALTER; deletar o probe
|
||||
4. **Gates** — `pnpm gate:fast`; push via `pnpm push`
|
||||
|
||||
## Rabbit holes / Não escopo (engenharia)
|
||||
|
||||
- Não alterar o fluxo E14 (motivo opcional do wizard/lista) — escopo do #288
|
||||
- Não tocar `snapshot`/`reversalSignals` (vivem só no JSON do snapshot; sem migration)
|
||||
- Não alterar o hook `validateAllocationDecision` — já está correto
|
||||
- Não criar migration gerada pelo Payload — hand-written é mais seguro para drift reconciliation
|
||||
|
||||
## Riscos e mitigação
|
||||
|
||||
- **Risco:** Migration falha em DB onde `rationale` já é nullable (fresh local DB). **Mitigação:** guard idempotente verifica `is_nullable` antes de alterar.
|
||||
- **Risco:** Prod tem `NOT NULL` com dados existentes. **Mitigação:** `DROP NOT NULL` é seguro — não altera dados, apenas relaxa a restrição.
|
||||
|
||||
## Aceite de engenharia
|
||||
|
||||
- [ ] Aceite de produto da intenção ainda coberto (rationale nullable, drift zero)
|
||||
- [ ] Invariantes AGENTS/engineering-standards (migration idempotente, padrão estabelecido)
|
||||
- [ ] Testes de domínio previstos (int spec de idempotência da migration)
|
||||
|
|
@ -0,0 +1,45 @@
|
|||
import { MigrateDownArgs, MigrateUpArgs, sql } from '@payloadcms/db-postgres'
|
||||
|
||||
/**
|
||||
* Reconciles production schema drift: `allocation_decision.rationale` was created
|
||||
* as `varchar NOT NULL` in the foundation migration, but the Payload config
|
||||
* (`AllocationDecision.rationale.required`) was later set to `false` (B134).
|
||||
* Without this reconciliation, every `pnpm migrate:create` re-emits
|
||||
* `ALTER TABLE allocation_decision ALTER COLUMN rationale DROP NOT NULL`.
|
||||
*
|
||||
* This migration is idempotent — it only drops the constraint 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 = 'allocation_decision'
|
||||
AND column_name = 'rationale'
|
||||
AND is_nullable = 'NO'
|
||||
) THEN
|
||||
ALTER TABLE "allocation_decision" ALTER COLUMN "rationale" 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 = 'allocation_decision'
|
||||
AND column_name = 'rationale'
|
||||
AND is_nullable = 'YES'
|
||||
) THEN
|
||||
ALTER TABLE "allocation_decision" ALTER COLUMN "rationale" SET NOT NULL;
|
||||
END IF;
|
||||
END $$;
|
||||
`)
|
||||
}
|
||||
|
|
@ -62,6 +62,7 @@ import * as migration_20260819_214755_add_campaign_user_advisor_permission_profi
|
|||
import * as migration_20260819_233503_add_campaign_demand_responsibles from './20260819_233503_add_campaign_demand_responsibles';
|
||||
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';
|
||||
|
||||
export const migrations = [
|
||||
{
|
||||
|
|
@ -384,4 +385,9 @@ export const migrations = [
|
|||
down: migration_20260823_161905_add_municipality_update_deliberation.down,
|
||||
name: '20260823_161905_add_municipality_update_deliberation'
|
||||
},
|
||||
{
|
||||
up: migration_20260824_000000_reconcile_allocation_decision_rationale_nullable.up,
|
||||
down: migration_20260824_000000_reconcile_allocation_decision_rationale_nullable.down,
|
||||
name: '20260824_000000_reconcile_allocation_decision_rationale_nullable'
|
||||
},
|
||||
];
|
||||
|
|
|
|||
|
|
@ -20,6 +20,8 @@ const testDatabaseUrl =
|
|||
const adminUrl = testDatabaseUrl.replace(/\/[^/]+$/, '/postgres')
|
||||
const consolidatedName = '20260718_010733_consolidate_campaign_schema'
|
||||
const stateDeputyContactMigrationName = '20260806_082110_add_state_deputy_contact'
|
||||
const rationaleNullableMigrationName =
|
||||
'20260824_000000_reconcile_allocation_decision_rationale_nullable'
|
||||
type MigrationDatabase = MigrateUpArgs['db']
|
||||
let databaseSequence = 0
|
||||
let databaseName = ''
|
||||
|
|
@ -301,4 +303,35 @@ describe('campaign migration existing-schema reconciliation', () => {
|
|||
`)
|
||||
expect(columns.rows).toEqual([{ column_name: 'contact_id' }])
|
||||
})
|
||||
|
||||
it('reconciles allocation_decision.rationale nullable and remains idempotent', async () => {
|
||||
if (!database) throw new Error('Disposable database is not initialized.')
|
||||
const migration = migrations.find(({ name }) => name === rationaleNullableMigrationName)
|
||||
if (!migration) throw new Error('Rationale nullable migration is not registered.')
|
||||
|
||||
const getNullability = async () => {
|
||||
const result = await database!.execute(sql`
|
||||
SELECT is_nullable
|
||||
FROM information_schema.columns
|
||||
WHERE table_schema = 'public'
|
||||
AND table_name = 'allocation_decision'
|
||||
AND column_name = 'rationale'
|
||||
`)
|
||||
return String(result.rows[0]?.is_nullable)
|
||||
}
|
||||
|
||||
expect(await getNullability()).toBe('YES')
|
||||
|
||||
await migration.down(stub<MigrateDownArgs>({ db: database }))
|
||||
expect(await getNullability()).toBe('NO')
|
||||
|
||||
await migration.up(stub<MigrateUpArgs>({ db: database }))
|
||||
expect(await getNullability()).toBe('YES')
|
||||
|
||||
await migration.up(stub<MigrateUpArgs>({ db: database }))
|
||||
expect(await getNullability()).toBe('YES')
|
||||
|
||||
await migration.down(stub<MigrateDownArgs>({ db: database }))
|
||||
expect(await getNullability()).toBe('NO')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue