From 3b32dbc8148a87e33fcdc7773418b5a175cd2388 Mon Sep 17 00:00:00 2001 From: Francisco Solla Date: Mon, 24 Aug 2026 14:57:28 -0300 Subject: [PATCH] =?UTF-8?q?fix(OPS83-recurr):=20cast=20rotativa=20do=20ver?= =?UTF-8?q?ify=20#16=20=E2=80=94=20serializar=20admin=20atr=C3=A1s=20do=20?= =?UTF-8?q?frontend=20(stub=20IG=20compartilhado)=20+=20notifica=C3=A7?= =?UTF-8?q?=C3=A3o=20pula=20recipient=20sumido=20em=20vez=20de=20abortar?= =?UTF-8?q?=20transa=C3=A7=C3=A3o?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/changelog/2026-08-24-ops83-recurr.md | 1 + playwright.config.ts | 8 +- .../createCampaignNotification.ts | 27 ++++- .../e2e/campaignDemandVisibility.e2e.spec.ts | 22 +++-- tests/e2e/campaignLeaderships.e2e.spec.ts | 4 + tests/e2e/campaignMunicipalities.e2e.spec.ts | 10 +- tests/e2e/campaignNewsletter.e2e.spec.ts | 6 +- ...ignNotificationRecipientGuard.unit.spec.ts | 99 +++++++++++++++++++ 8 files changed, 163 insertions(+), 14 deletions(-) create mode 100644 docs/changelog/2026-08-24-ops83-recurr.md create mode 100644 tests/unit/campaignNotificationRecipientGuard.unit.spec.ts diff --git a/docs/changelog/2026-08-24-ops83-recurr.md b/docs/changelog/2026-08-24-ops83-recurr.md new file mode 100644 index 00000000..2412b52b --- /dev/null +++ b/docs/changelog/2026-08-24-ops83-recurr.md @@ -0,0 +1 @@ +**OPS83-recurr — deploy verify run #16 falhou de novo: cast rotativa + bug real de notificação (2026-08-24):** o run #16 (HEAD pós-OPS83) mostrou a cast rotativa que a intenção antecipou: 1 failed (admin:134, 3 retries) + 7 flaky, incluindo 3 classes já tocadas na OPS83 e classes novas. Fixes: (1) **admin:134 (hard fail)** — corrida cross-project real: os specs IG do `frontend` e o painel de sync IG do `admin` compartilham o stub do Instagram e o global `social-feed-settings`; em prod mode os projects rodavam concorrentes (sem dependências), então um teste IG do frontend virava o stub/global enquanto o admin:134 seedava e sincronizava — fix config-only: o project `admin` volta a depender de `frontend` **também em prod mode** (serializa os únicos dois projects que compartilham estado IG); (2) **newsletter:136** — o mesmo bug de regex `/#novidades$/` do :222 que ficou no sibling de captura (miss de família) — `/#novidades/` sem `$`; (3) **B176** — settle do stream também DEPOIS de cada clique de option (não só após remove-chip), o clique de Liderança corria o stream da navegação de facet-add; (4) **advisor-scopes Média** — poll de presença do campo `Média` antes do fill (o painel de pledges streama); (5) **B34** — gate `S:` após o `networkidle` da lista de lideranças (o chip da célula cai depois); (6) **C143 revoke** — o save de responsáveis é server-action otimista: agora espera o POST aterrissar via `expectPostResponse` antes de trocar para o peer, e a asserção de 404 navega via `about:blank` (quebra a cache RSC do router, que servia a demanda acessível após o cookie swap); (7) **bug real de app descoberto (fix com unit pin):** `createCampaignNotifications` insería notificações para recipient IDs já deletados por escritor paralelo (FK `notification_recipient_id_campaign_user_id_fk`) — e um segundo caso com `recipient_id` null — abortando a transação hospedeira inteira (`notifyMunicipalityUpdateCreated` crashou sob 4 workers; mesmo hazard em produção ao remover assessor durante fan-out). Fix: revalidação de existência dos recipients (`payload.find` em `campaignUser`, `overrideAccess: true`, mesmo `req`) + filtro de ids não-finitos antes do fan-out — recipient sumido é pulado, nunca um kill de transação. Sem mudança de schema/contrato público/acesso/Consent; 4 unit pins novos (`campaignNotificationRecipientGuard.unit.spec.ts`). Verificação: unit 2447, int 830, famílias afetadas verdes em dev (2× grupo C106/C143/AgendaFeed, 2× B176/advisors/newsletter), admin isolado verde em `E2E_PROD=1`; lint/tsc/format/knip/cycles verdes. diff --git a/playwright.config.ts b/playwright.config.ts index 2d2696eb..b5af32d2 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -182,7 +182,13 @@ export default defineConfig({ { name: 'admin', testMatch: /admin\.e2e\.spec\.ts/, - dependencies: isProdMode ? [] : ['frontend'], + // Serialize behind `frontend` in BOTH modes: admin's Instagram sync + // panel and the frontend IG feed specs share the instagram stub and the + // `social-feed-settings` global (single server process). Drop the + // dependency in prod and the two projects run concurrently, letting a + // frontend IG test flip the stub/global while admin:134 seeds and syncs + // (OPS83 run #16: hard-failed all 3 retries on the shared stub state). + dependencies: ['frontend'], fullyParallel: false, use: { ...devices['Desktop Chrome'], channel: 'chromium' }, }, diff --git a/src/utilities/notification/createCampaignNotification.ts b/src/utilities/notification/createCampaignNotification.ts index f50d98fd..94206e2d 100644 --- a/src/utilities/notification/createCampaignNotification.ts +++ b/src/utilities/notification/createCampaignNotification.ts @@ -67,11 +67,32 @@ export const createCampaignNotifications = async ( input: Omit, req?: NotificationWriteRequest, ): Promise => { - const uniqueRecipients = [...new Set(recipientIDs)] - if (uniqueRecipients.length === 0) return + const candidates = [...new Set(recipientIDs)].filter((id) => Number.isFinite(id)) + if (candidates.length === 0) return + + // A resolved recipient can be concurrently deleted (a staff member removed + // from a município's `advisors`, or a parallel writer's cleanup) between the + // recipient resolution and this insert. Inserting for a stale id aborts the + // WHOLE host transaction with an FK violation — observed crashing + // `notifyMunicipalityUpdateCreated` under the parallel e2e suite (OPS83 run + // #16) and the same hazard exists in production when an advisor is removed + // while a notification fan-out is in flight. Re-validate existence before + // the fan-out so a vanished recipient is skipped, never a transaction kill. + const existing = await payload.find({ + collection: 'campaignUser', + where: { id: { in: candidates } }, + depth: 0, + limit: 500, + pagination: false, + overrideAccess: true, + req, + }) + const existingIDs = new Set(existing.docs.map((doc) => doc.id)) + const recipients = candidates.filter((id) => existingIDs.has(id)) + if (recipients.length === 0) return await Promise.all( - uniqueRecipients.map((recipientID) => + recipients.map((recipientID) => createCampaignNotification( payload, { diff --git a/tests/e2e/campaignDemandVisibility.e2e.spec.ts b/tests/e2e/campaignDemandVisibility.e2e.spec.ts index f393d37c..6d836461 100644 --- a/tests/e2e/campaignDemandVisibility.e2e.spec.ts +++ b/tests/e2e/campaignDemandVisibility.e2e.spec.ts @@ -1,6 +1,7 @@ import { campaignPageChrome, expect, + expectPostResponse, test, type CampaignE2EFixture, } from './fixtures/campaignE2EFixtures.js' @@ -92,16 +93,23 @@ test.describe('C143 — demand visibility (explicit responsibles)', () => { await campaign.sessionFor(context, responsible) await page.goto(demandURL) await page.getByRole('button', { name: `Remover ${peer.name}` }).click() - await page.getByRole('button', { name: 'Salvar responsáveis' }).click() + // The responsibles save is an optimistic server-action write — wait for the + // POST to land BEFORE swapping to the peer, or the peer's navigation can + // race the revoke (OPS83 run #16: the accessible demand still served). + await Promise.all([ + expectPostResponse(page, demandURL), + page.getByRole('button', { name: 'Salvar responsáveis' }).click(), + ]) await expect(page.getByRole('button', { name: `Remover ${peer.name}` })).toHaveCount(0) - // The peer already rendered THIS URL (granted, line 88); the Next router - // cache serves the previous RSC payload on a plain goto even after the - // cookie swap (OPS83 run #15: the accessible demand showed instead of the - // 404). The route ignores unknown query params — bust the cache so the - // revoked access is observed from the server. + // The peer already rendered THIS URL (granted, line 88), so the Next RSC + // cache can serve the previous payload on a plain goto even after the + // cookie swap (OPS83 run #15/#16). Break the router context through a + // blank page so the 404 boundary is fetched fresh from the server. await campaign.sessionFor(context, peer) - await page.goto(`${demandURL}?e2e=${Date.now()}`) + await page.goto('about:blank') + await expect(page).toHaveURL(/about:blank/) + await page.goto(demandURL) await expect(page.getByText('This page could not be found.')).toBeVisible() }) }) diff --git a/tests/e2e/campaignLeaderships.e2e.spec.ts b/tests/e2e/campaignLeaderships.e2e.spec.ts index 149c068f..d52b5d1c 100644 --- a/tests/e2e/campaignLeaderships.e2e.spec.ts +++ b/tests/e2e/campaignLeaderships.e2e.spec.ts @@ -81,6 +81,10 @@ test.describe('campaign leaderships list', () => { await page.goto(`${campaign.baseURL}/campanha/liderancas?q=${encodeURIComponent(contactName)}`) await expect(campaignPageChrome(page, 'Lideranças')).toBeVisible() await page.waitForLoadState('networkidle') + // The cell chips stream with the list's RSC chunk; under 4-worker load the + // chip link can land after `networkidle` settles (OPS83 run #16: B34 + // flaked on the initial chip). Let the stream commit before asserting. + await page.waitForFunction(() => document.querySelectorAll('div[id^="S:"]').length === 0) // The chip is still a link to the município — editing did not cost the // navigation the column had before. diff --git a/tests/e2e/campaignMunicipalities.e2e.spec.ts b/tests/e2e/campaignMunicipalities.e2e.spec.ts index d120bdc5..a0b47d79 100644 --- a/tests/e2e/campaignMunicipalities.e2e.spec.ts +++ b/tests/e2e/campaignMunicipalities.e2e.spec.ts @@ -443,6 +443,7 @@ test.describe('Municípios — jornadas por papel', () => { // Dobradinha by contact name → chip + only municipality A remains. await omnibox.fill(deputyName) await listbox.getByRole('option', { name: new RegExp(`^${deputyName} \\(BR176\\)$`) }).click() + await settleMunicipalityStream(page) await expect(removeChip(`Dobradinha: ${deputyName} (BR176)`)).toBeVisible() await expect(tableRows).toHaveCount(1) await expect(page.getByRole('link', { name: municipalityA.name, exact: true })).toBeVisible() @@ -457,6 +458,7 @@ test.describe('Municípios — jornadas por papel', () => { // Partido by acronym → the same single municipality (its only dobradinha). await omnibox.fill('BR176') await listbox.getByRole('option', { name: 'BR176', exact: true }).click() + await settleMunicipalityStream(page) await expect(removeChip('Partido: BR176')).toBeVisible() await expect(tableRows).toHaveCount(1) await expect(page.getByRole('link', { name: municipalityA.name, exact: true })).toBeVisible() @@ -468,6 +470,7 @@ test.describe('Municípios — jornadas por papel', () => { // Liderança by contact name → municipality B only. await omnibox.fill(leadership.contactName) await listbox.getByRole('option', { name: new RegExp(`^${leadership.contactName}$`) }).click() + await settleMunicipalityStream(page) await expect(removeChip(`Liderança: ${leadership.contactName}`)).toBeVisible() await expect(tableRows).toHaveCount(1) await expect(page.getByRole('link', { name: municipalityB.name, exact: true })).toBeVisible() @@ -732,7 +735,12 @@ test.describe('Municípios — jornadas por papel', () => { // Advisor records an internal estimate on the overview pledges panel. await page.goto(`${campaign.baseURL}/campanha/municipios/${administered.slug}`) - await page.getByLabel('Média', { exact: true }).fill('90') + // The overview pledges panel streams under load; the `Média` estimate + // field can land after `goto` settles (OPS83 run #16: fill timed out). + await settleMunicipalityStream(page) + const estimateInput = page.getByLabel('Média', { exact: true }) + await expect.poll(() => estimateInput.count(), { timeout: 20_000 }).toBeGreaterThan(0) + await estimateInput.fill('90') await page.getByLabel('Justificativa').fill('Histórico da região indica menos.') await page.getByRole('button', { name: 'Salvar estimativa' }).click() await expect(page.getByText('Média: 90')).toBeVisible() diff --git a/tests/e2e/campaignNewsletter.e2e.spec.ts b/tests/e2e/campaignNewsletter.e2e.spec.ts index 83abac14..f1232cf9 100644 --- a/tests/e2e/campaignNewsletter.e2e.spec.ts +++ b/tests/e2e/campaignNewsletter.e2e.spec.ts @@ -144,9 +144,11 @@ test.describe('Campaign home novidades capture', () => { section.getByRole('heading', { name: 'Receba as novidades da campanha' }), ).toBeVisible() - // The hero CTA is the shortcut into the section (smooth anchor). + // The hero CTA is the shortcut into the section (smooth anchor). The URL + // legitimately carries the `?e2e=…` cache-buster query (OPS83), so assert + // only the hash — no trailing `$`. await page.locator('[data-cta="secondary"]').click() - await expect(page).toHaveURL(/#novidades$/) + await expect(page).toHaveURL(/#novidades/) await expect(section).toBeInViewport() // The toggle is pre-selected: the default level is "time". diff --git a/tests/unit/campaignNotificationRecipientGuard.unit.spec.ts b/tests/unit/campaignNotificationRecipientGuard.unit.spec.ts new file mode 100644 index 00000000..1fc7552d --- /dev/null +++ b/tests/unit/campaignNotificationRecipientGuard.unit.spec.ts @@ -0,0 +1,99 @@ +import type { Payload } from 'payload' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { stub } from '../helpers/stub' + +/** + * OPS83 — the notification fan-out must skip recipients whose `campaignUser` + * no longer resolves, instead of inserting and crashing the host transaction + * with an FK violation. Observed aborting `notifyMunicipalityUpdateCreated` + * under the parallel e2e suite (run #16) — a deleted advisor left the insert + * referencing a vanished row; the same hazard exists in production when a + * staff member is removed mid-fan-out. + */ + +const { sendMock, createMock, findMock } = vi.hoisted(() => ({ + sendMock: vi.fn(), + createMock: vi.fn(async (args: { data?: Record }) => ({ + id: 0, + doc: args.data, + })), + findMock: vi.fn(), +})) + +vi.mock('@/utilities/notification/sendCampaignPush', () => ({ + sendCampaignPushForNotification: sendMock, +})) + +vi.mock('@/utilities/payloadTransaction', () => ({ + onPayloadTransactionCommit: vi.fn(), +})) + +import { createCampaignNotifications } from '@/utilities/notification/createCampaignNotification' + +describe('createCampaignNotifications (stale-recipient guard, OPS83)', () => { + const payload = stub({ + find: findMock as unknown as Payload['find'], + create: createMock as unknown as Payload['create'], + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('skips recipients that no longer resolve to a campaignUser', async () => { + findMock.mockResolvedValue({ docs: [{ id: 7 }, { id: 9 }] }) + + await createCampaignNotifications(payload, [7, 9, 999], { + type: 'municipality_update', + payload: { title: 't', detail: 'd', href: '/x' }, + }) + + expect(findMock).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'campaignUser', + where: { id: { in: [7, 9, 999] } }, + }), + ) + // 999 did not resolve — never inserted. + expect(createMock).not.toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ recipient: 999 }), + }), + ) + expect(createMock).toHaveBeenCalledTimes(2) + }) + + it('drops non-finite recipient ids from the candidate set', async () => { + findMock.mockResolvedValue({ docs: [{ id: 5 }] }) + + await createCampaignNotifications(payload, [5, Number.NaN], { + type: 'new_supporter', + payload: { title: 't', detail: 'd', href: '/x' }, + }) + + expect(findMock).toHaveBeenCalledWith(expect.objectContaining({ where: { id: { in: [5] } } })) + expect(createMock).toHaveBeenCalledTimes(1) + }) + + it('is a no-op when no recipient resolves', async () => { + findMock.mockResolvedValue({ docs: [] }) + + await createCampaignNotifications(payload, [404], { + type: 'activity_attention', + payload: { title: 't', detail: 'd', href: '/x' }, + }) + + expect(createMock).not.toHaveBeenCalled() + }) + + it('is a no-op for an empty candidate set without querying', async () => { + await createCampaignNotifications(payload, [], { + type: 'municipality_update', + payload: { title: 't', detail: 'd', href: '/x' }, + }) + + expect(findMock).not.toHaveBeenCalled() + expect(createMock).not.toHaveBeenCalled() + }) +})