From f5ddd79cceaa2e828479173d72e47d27852a2286 Mon Sep 17 00:00:00 2001 From: Francisco Solla Date: Mon, 24 Aug 2026 22:11:22 -0300 Subject: [PATCH] =?UTF-8?q?S2-FOLLOWUP=20(#775)=20=E2=80=94=20validate=20d?= =?UTF-8?q?o=20itemId=20YouTube=20deixa=20de=20ser=20c=C3=B3digo=20morto?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit O validate de itemId em social-feed-settings lia data.platform (doc inteiro, sempre undefined) e nunca validava ID de vídeo em produção. Agora lê siblingData.platform via helper validateExcludedItemId. Stub YouTube ganha ids de 11 chars e o spec acompanha, destravando o e2e do describe 'Campaign home content section'. Sem migration. --- ...-24-s2-followup-validate-youtube-itemid.md | 1 + ...idate-itemid-excludeditems-youtube-impl.md | 78 +++++++++++++++++++ src/globals/SocialFeedSettings.ts | 12 +-- src/utilities/socialFeed/excludedItems.ts | 19 +++++ tests/e2e/frontend.e2e.spec.ts | 11 +-- tests/e2e/youtube-stub.mjs | 8 +- tests/unit/socialFeedExclusions.unit.spec.ts | 41 ++++++++++ 7 files changed, 150 insertions(+), 20 deletions(-) create mode 100644 docs/changelog/2026-08-24-s2-followup-validate-youtube-itemid.md create mode 100644 docs/plans/s2-followup-validate-itemid-excludeditems-youtube-impl.md create mode 100644 src/utilities/socialFeed/excludedItems.ts create mode 100644 tests/unit/socialFeedExclusions.unit.spec.ts diff --git a/docs/changelog/2026-08-24-s2-followup-validate-youtube-itemid.md b/docs/changelog/2026-08-24-s2-followup-validate-youtube-itemid.md new file mode 100644 index 00000000..8082f3a5 --- /dev/null +++ b/docs/changelog/2026-08-24-s2-followup-validate-youtube-itemid.md @@ -0,0 +1 @@ +**S2-FOLLOWUP (#775) — validate do itemId (excludedItems do YouTube) deixa de ser código morto:** o `validate` do campo `itemId` em `social-feed-settings` (`src/globals/SocialFeedSettings.ts`) lia `data.platform` — o documento inteiro, sempre `undefined` — então nunca validava ID de vídeo do YouTube em produção (código morto desde a S2). Agora lê `siblingData.platform` (a row do array) via helper puro `validateExcludedItemId` (`src/utilities/socialFeed/excludedItems.ts`), que rejeita IDs fora do padrão `[A-Za-z0-9_-]{11}` com a mensagem pt-BR já existente. Em dupla, os ids do stub YouTube (`tests/e2e/youtube-stub.mjs`) foram trocados de 20 chars para 11 chars exatos (`e2evideo001..004`) e o spec (`tests/e2e/frontend.e2e.spec.ts`) acompanhou (`excludedItems`, hrefs e mensagem de compartilhamento do WhatsApp), destravando o e2e do describe "Campaign home content section" que dependia da mortandade. Sem migration (validate é validação de runtime, não schema). Instagram não é afetado (`platform !== 'youtube'` → `true`). Débito: verificação read-only pré-deploy de `excludedItems` YouTube inválidos já salvos em prod (gatilho = resultado do pré-check no deploy manual; se houver ruído, fallback B licenciado pela intenção é remover o validate) — deferred_trigger, não criado agora. diff --git a/docs/plans/s2-followup-validate-itemid-excludeditems-youtube-impl.md b/docs/plans/s2-followup-validate-itemid-excludeditems-youtube-impl.md new file mode 100644 index 00000000..31669984 --- /dev/null +++ b/docs/plans/s2-followup-validate-itemid-excludeditems-youtube-impl.md @@ -0,0 +1,78 @@ +# Impl: S2-FOLLOWUP: validate do itemId (excludedItems) do YouTube e co + +Status: rascunho +Atualizado em: 2026-08-24 +Issue: #775 +Intenção: docs/plans/s2-followup-validate-itemid-excludeditems-youtube.md +Appetite restante: herdado — item pequeno de follow-up, P2 + +## Leitura da intenção + +- **Outcome:** o `validate` do campo `itemId` do array `excludedItems` em `SocialFeedSettings` deixa de ser código morto e passa a validar de verdade IDs de vídeo do YouTube (padrão `[A-Za-z0-9_-]{11}`), sem quebrar o e2e do S2 — ou, na alternativa licenciada pela própria intenção, é removido e o campo fica livre. +- **O que NÃO negociar:** correção em dupla — (1) validar via `siblingData.platform` OU remover o validate; (2) ids do stub de YouTube para 11 chars exatos + `excludedItems` do spec atualizados; (3) e2e do describe "Campaign home content section" verde. Fora de escopo: mexer no S2 entregue em produção. +- **O que reavaliar:** hipótese de que "produção tem IDs inválidos salvos" — não verificável do worktree (guards proíbem tocar prod); tratada em Riscos com verificação pré-deploy. + +## Abordagem recomendada + +```mermaid +flowchart LR + A[validate morto: data.platform undefined] --> B[Extrair helper puro em src/utilities/socialFeed/excludedItems.ts] + B --> C[validate usa siblingData.platform + unit test] + C --> D[Stub youtube: ids 11 chars e2evideo001-004] + D --> E[Spec: excludedItems/hrefs/share atualizados] + E --> F[e2e describe Campaign home + gate:fast] +``` + +**Opções consideradas:** A (consertar validate via `siblingData.platform`) | B (remover o validate) | C (validate "tolerante a legado" lendo o doc salvo via `req.payload`) +**Recomendação:** A — porque entrega o outcome primário da intenção (validação real de ID de vídeo, que hoje não existe), é mudança de ~1 linha com risco mitigado (ver Riscos) e o próprio guard nunca bloqueia o read nem corrompe dados; a alternativa B fica como fallback documentado com gatilho explícito (pré-check de prod), não como escolha primária — remover o validate joga fora a razão de ser do follow-up e deixa o erro de colar URL inteira continuar silencioso. +**Rejeitadas:** B como escolha primária — zera o risco, mas abdica do outcome primário da intenção e mantém o status quo de exclusões que falham silenciosamente com id mal colado (URL inteira, código curto); C — validate lendo o doc persistido (`req.payload.findGlobal`) para tolerar linhas legadas é over-engineering para um item P2, adiciona DB read dentro de validação e ciclo/recursão potencial; a permissão da intenção ("ou deixar o campo livre") já cobre o cenário de segurança máximo sem essa cerimônia. + +### Componentes / mudanças + +- **`validate` do `itemId`** (`src/globals/SocialFeedSettings.ts:204-212`): troca `{ data }`/`data.platform` por `{ siblingData }`/`siblingData.platform`; extrai a regra para helper puro `validateExcludedItemId(value, siblingData)` + `YOUTUBE_VIDEO_ID_PATTERN` em `src/utilities/socialFeed/excludedItems.ts` (mesmo diretório do `instagramSync` — o módulo que já é dono do concern "social feed"; a global já importa de `@/utilities/socialFeed/`). Mensagem de erro pt-BR existente fica: 'Informe o ID do vídeo (11 caracteres)'. Precedente do padrão siblingData: `Post.ts:61`, `Tag.ts:65` (beforeValidate), `AllocationDecision.ts:136` (condition). +- **`youtube-stub.mjs`** (`tests/e2e/`): os 4 ids de 20 chars viram 11 chars exatos — `e2e-video-destaque-1` → `e2evideo001`, `e2e-video-caravana-2` → `e2evideo002`, `e2e-video-entrevista-3` → `e2evideo003`, `e2e-video-excluido-4` → `e2evideo004` (todos `[A-Za-z0-9_-]{11}`, mantêm semântica de destaque/excluído). Thumbnails (`/thumbs/.jpg`), `statisticsResponse` e `searchResponse` derivam das entradas de `VIDEOS` — consistência interna basta, nada mais muda. +- **`frontend.e2e.spec.ts`**: `excludedItems` youtube em :1030 e :1152 → `itemId: 'e2evideo004'`; href `watch?v=e2e-video-destaque-1` em :1049 e na mensagem WhatsApp em :1584 → `watch?v=e2evideo001`. Canal `UCe2eTestChannel` e IG (`e2e-ig-grade-excluido-5`, `e2e-ig-muro-1`) não mudam — IG é `platform: 'instagram'`, o guard não dispara (`SocialFeedSettings.ts:208`). +- **Unit test** (`tests/unit/socialFeedExclusions.unit.spec.ts`, padrão de nome `*.unit.spec.ts`): helper puro coberto — aceita 11 chars válidos (`e2evideo001`, `dQw4w9WgXcQ`), rejeita 20 chars/URLs/curtos; só falha quando `siblingData.platform === 'youtube'`; retorna `true` para `instagram` e para valor vazio. +- **Migration:** sem migration — `validate` é validação de runtime no config Payload, não altera schema do DB (o array/select/text já existem desde o S2). +- **Access / Consent:** não se aplica — global admin-only já existente (`payloadAdminOnly`, `SocialFeedSettings.ts:64-67`). +- **UI:** não há UI nova — o array já tem o admin panel; o erro inline do campo já existe. + +### Dados → forma (se aplicável) + +- Não se aplica (nenhuma forma nova; copy de erro já existente em pt-BR). + +## Fases verificáveis + +1. **Fix do validate + unit test** — extrai helper (`src/utilities/socialFeed/excludedItems.ts`) e usa `siblingData.platform` no validate; `tests/unit/socialFeedExclusions.unit.spec.ts`; valida com `pnpm test -- unit` do arquivo e `tsc --noEmit`. (Se o implementador preferir manter o validate inline, a única perda é o unit test — decisão barata, registrada.) +2. **Ids do stub + spec** — `youtube-stub.mjs` (4 ids) e `frontend.e2e.spec.ts` (:1030, :1049, :1152, :1584). IG e `admin.e2e.spec.ts` intocados. +3. **E2E + gates** — `pnpm test:e2e --no-deps -- tests/e2e/frontend.e2e.spec.ts -g "Campaign home content section"` (cobre os dois testes afetados: exclusões e "keeps the last snapshot"); rodar também o teste de save do admin do IG (`admin.e2e.spec.ts` — deve continuar verde, IG não é validado); `pnpm gate:fast`. + +## Rabbit holes / Não escopo (engenharia) + +- Instagram não é afetado: `platform !== 'youtube'` → validate retorna `true` (linhas IG em `instagram-stub.mjs` e specs não mudam). +- Sem migração de dados: `excludedItems` em prod é dado de admin; ids inválidos já salvos não quebram o read (o board casa por id exato e apenas "não exclui"). +- Não mexer no S2 entregue em produção (nada do board/read path muda; o stub só ganha ids realistas). +- Não criar validate "tolerante a legado" com leitura do banco (opção C rejeitada). +- Não adicionar warning-mode no Payload (não existe — validate retorna `true` ou string de erro; sem meio-termo). + +## Riscos e mitigação + +- **Save-trap no admin/REST se prod tiver id youtube inválido pré-existente.** O validate roda por row quando o doc inteiro é submetido: o admin form PUTa o global completo e o e2e `updateSocialFeedSettings` idem — linhas existentes re-validam. Como o validate está morto desde o S2, nada garantiu o que foi salvo (id digitado à mão, URL inteira colada). (a) O risco é estruturalmente real, mas de severidade baixa: o erro é inline por row no form, e a row pode ser corrigida ou DELETADA no próprio form antes do save — a recuperação nunca exige acesso ao banco; o único dano realista é um save não relacionado falhar uma vez e a assessoria ter que ajustar/apagar a linha. Probabilidade de existir row inválida em prod é desconhecida-moderada (campo manual, sem validação), mas mitigável: **verificação pré-deploy read-only** de `excludedItems` com `platform: 'youtube'` fora do padrão (query única no homeserver, feita por quem dispara o `workflow_dispatch` — deploy é manual). Se achar rows inválidas: corrigir/apagar pela UI antes de publicar; se não for possível limpar → **fallback B (remover o validate)**, licenciado pela intenção e pré-comprometido neste plano. (b) Remover o validate é de fato mais seguro — por isso é o fallback; mas com o pré-check + recuperação 100% na UI, A não é fail-closed violado (sem corrupção, sem lockout, sem PII) e entrega o outcome primário. +- **Fix quebra o e2e existente (400 nos writes de exclusão).** Não é risco, é a causa raiz — a troca dos ids do stub para 11 chars (fase 2, mandatória) resolve na mesma entrega; ordem importa: aplicar fase 1 e 2 juntas antes de rodar o e2e. +- **Typing do validate:** `{ data }` hoje tipa `data.platform`; trocar o destructure para `siblingData` acompanha o tipo do row — sem `as` casts. +- **Save do admin (IG) no `admin.e2e.spec.ts:92-97`:** submete o doc inteiro com row IG → validate retorna `true` para `instagram`; confirmado na fase 3. + +## Aceite de engenharia + +- [ ] Aceite de produto da intenção ainda coberto (validação real via siblingData + ids do stub 11 chars + e2e do describe verde; fallback B explícito se o pré-check de prod falhar) +- [ ] Invariantes AGENTS/engineering-standards (sem schema → sem migration; sem escrita multi-collection; copy pt-BR / identificadores em inglês; nada de acesso/Consent novo) +- [ ] Testes de domínio previstos (unit do helper de validação na fase 1; e2e do describe + save admin IG na fase 3) + +## Decisões de engenharia + +``` +Opções: A (validate via siblingData.platform) | B (remover validate) | C (validate tolerante a legado lendo o doc salvo) +Recomendação: A — porque entrega o outcome primário da intenção (validação real de ID de vídeo), com risco de save-trap baixo (erro inline por row, row corrigível/deletável na própria UI, sem acesso a banco) e gatilho de fallback documentado (pré-check read-only de prod no deploy manual → B). +Alternativas rejeitadas: B como primária — zeraria o risco mas abdica do propósito do follow-up e mantém exclusões que falham silenciosamente com id mal colado; fica como fallback pré-comprometido. C — over-engineering para P2 (DB read dentro de validate, ciclo potencial), quando a intenção já licencia o caminho seguro máximo sem essa cerimônia. +Self-score decision-quality: 5/5 — (1) decisão cara (validate vivo vs morto em prod) tem rejeitadas A/B/C documentadas; (2) cabe no appetite P2 (2 fases de edição + gates, sem migration nem UI); (3) rabbit holes nomeados (IG, migração de dados, S2 em prod, warning-mode inexistente); (4) depth check reusa `src/utilities/socialFeed/` (mesmo dono do concern) e helpers de e2e existentes, sem módulo novo; (5) intenção preservada — correção em dupla e e2e do describe intactos. +``` diff --git a/src/globals/SocialFeedSettings.ts b/src/globals/SocialFeedSettings.ts index 0ec0a4fc..4cc57875 100644 --- a/src/globals/SocialFeedSettings.ts +++ b/src/globals/SocialFeedSettings.ts @@ -1,5 +1,6 @@ import { payloadAdminOnly } from '@/utilities/campaignAccess' import { REVALIDATE_SOCIAL_FEED_TAG } from '@/utilities/revalidateRequest' +import { validateExcludedItemId } from '@/utilities/socialFeed/excludedItems' import { INSTAGRAM_SYNC_HOOK_TIMEOUT_MS, instagramCredentialsChanged, @@ -10,8 +11,6 @@ import type { GlobalAfterChangeHook, GlobalConfig } from 'payload' const slug = 'social-feed-settings' -const YOUTUBE_VIDEO_ID_PATTERN = /^[A-Za-z0-9_-]{11}$/ - const revalidateFeed = async () => revalidateTag(REVALIDATE_SOCIAL_FEED_TAG) /** @@ -203,13 +202,8 @@ export const SocialFeedSettings: GlobalConfig = { required: true, validate: ( value: string | null | undefined, - { data }: { data: { platform?: string } }, - ) => { - if (data.platform === 'youtube' && value && !YOUTUBE_VIDEO_ID_PATTERN.test(value)) { - return 'Informe o ID do vídeo (11 caracteres)' - } - return true - }, + { siblingData }: { siblingData: { platform?: string } | undefined }, + ) => validateExcludedItemId(value, siblingData), }, { name: 'reason', diff --git a/src/utilities/socialFeed/excludedItems.ts b/src/utilities/socialFeed/excludedItems.ts new file mode 100644 index 00000000..2732e800 --- /dev/null +++ b/src/utilities/socialFeed/excludedItems.ts @@ -0,0 +1,19 @@ +/** + * Validation for `socialFeedSettings.excludedItems[].itemId` (the campaign home + * content board's manual exclusion list). The field is an array row: Payload + * passes the row's own siblings through `siblingData`, NOT the document, so the + * validate must read the platform from `siblingData` (the previous + * `validate` read `data.platform`, which is the whole document and therefore + * always `undefined` — dead code that never validated anything in production). + */ +export const YOUTUBE_VIDEO_ID_PATTERN = /^[A-Za-z0-9_-]{11}$/ + +export const validateExcludedItemId = ( + value: string | null | undefined, + siblingData?: { platform?: string }, +): true | string => { + if (siblingData?.platform === 'youtube' && value && !YOUTUBE_VIDEO_ID_PATTERN.test(value)) { + return 'Informe o ID do vídeo (11 caracteres)' + } + return true +} diff --git a/tests/e2e/frontend.e2e.spec.ts b/tests/e2e/frontend.e2e.spec.ts index f46eecbf..8bd49b52 100644 --- a/tests/e2e/frontend.e2e.spec.ts +++ b/tests/e2e/frontend.e2e.spec.ts @@ -1027,7 +1027,7 @@ test.describe('Campaign home content section', () => { youtubeApiKey: 'e2e-youtube-api-key', youtubeChannelId: 'UCe2eTestChannel', youtubeMaxItems: 3, - excludedItems: [{ platform: 'youtube', itemId: 'e2e-video-excluido-4' }], + excludedItems: [{ platform: 'youtube', itemId: 'e2evideo004' }], youtubeFeedSnapshot: null, }) @@ -1044,10 +1044,7 @@ test.describe('Campaign home content section', () => { const featured = section.getByRole('link', { name: /E2e Vídeo em destaque/ }) await expect(featured).toBeVisible() - await expect(featured).toHaveAttribute( - 'href', - 'https://www.youtube.com/watch?v=e2e-video-destaque-1', - ) + await expect(featured).toHaveAttribute('href', 'https://www.youtube.com/watch?v=e2evideo001') await expect(featured).toHaveAttribute('target', '_blank') await expect(featured).toHaveAttribute('rel', 'noopener noreferrer') await expect( @@ -1149,7 +1146,7 @@ test.describe('Campaign home content section', () => { youtubeApiKey: 'e2e-youtube-api-key', youtubeChannelId: 'UCe2eTestChannel', youtubeMaxItems: 3, - excludedItems: [{ platform: 'youtube', itemId: 'e2e-video-excluido-4' }], + excludedItems: [{ platform: 'youtube', itemId: 'e2evideo004' }], youtubeFeedSnapshot: null, } await updateSocialFeedSettings(request, headers, baseSettings) @@ -1581,7 +1578,7 @@ test.describe('Campaign home content section', () => { const videoShare = videoCard.locator('xpath=..').getByRole('button', { name: 'Compartilhar' }) await videoShare.click() expect(await waMessage(page.getByRole('link', { name: 'Compartilhar no WhatsApp' }))).toBe( - 'Olha esse vídeo do Solla: E2e Vídeo em destaque — https://www.youtube.com/watch?v=e2e-video-destaque-1', + 'Olha esse vídeo do Solla: E2e Vídeo em destaque — https://www.youtube.com/watch?v=e2evideo001', ) await page.keyboard.press('Escape') await expect(page.locator('[data-slot="popover-content"]')).toHaveCount(0) diff --git a/tests/e2e/youtube-stub.mjs b/tests/e2e/youtube-stub.mjs index 1e256e03..43a00df9 100644 --- a/tests/e2e/youtube-stub.mjs +++ b/tests/e2e/youtube-stub.mjs @@ -25,28 +25,28 @@ const minutesAgo = (minutes) => new Date(Date.now() - minutes * 60_000).toISOStr const VIDEOS = [ { - id: 'e2e-video-destaque-1', + id: 'e2evideo001', title: 'E2e Vídeo em destaque', minutesAgo: 30, views: 12400, maxres: true, }, { - id: 'e2e-video-caravana-2', + id: 'e2evideo002', title: 'E2e Vídeo de caravana', minutesAgo: 120, views: 8100, maxres: true, }, { - id: 'e2e-video-entrevista-3', + id: 'e2evideo003', title: 'E2e Vídeo de entrevista', minutesAgo: 26 * 60, views: 987, maxres: false, }, { - id: 'e2e-video-excluido-4', + id: 'e2evideo004', title: 'E2e Vídeo excluído', minutesAgo: 10, views: 5_000_000, diff --git a/tests/unit/socialFeedExclusions.unit.spec.ts b/tests/unit/socialFeedExclusions.unit.spec.ts new file mode 100644 index 00000000..4b1cd4fd --- /dev/null +++ b/tests/unit/socialFeedExclusions.unit.spec.ts @@ -0,0 +1,41 @@ +import { + validateExcludedItemId, + YOUTUBE_VIDEO_ID_PATTERN, +} from '@/utilities/socialFeed/excludedItems' +import { describe, expect, it } from 'vitest' + +describe('validateExcludedItemId', () => { + it('accepts a valid 11-char youtube id', () => { + expect(validateExcludedItemId('dQw4w9WgXcQ', { platform: 'youtube' })).toBe(true) + expect(validateExcludedItemId('e2evideo001', { platform: 'youtube' })).toBe(true) + }) + + it('rejects youtube ids that are not 11 chars', () => { + expect(validateExcludedItemId('e2e-video-excluido-4', { platform: 'youtube' })).toMatch( + /11 caracteres/, + ) + expect(validateExcludedItemId('short', { platform: 'youtube' })).toMatch(/11 caracteres/) + }) + + it('rejects a pasted youtube url as the id', () => { + expect( + validateExcludedItemId('https://youtube.com/watch?v=dQw4w9WgXcQ', { platform: 'youtube' }), + ).toMatch(/11 caracteres/) + }) + + it('only validates for the youtube platform', () => { + expect(validateExcludedItemId('anything-goes', { platform: 'instagram' })).toBe(true) + expect(validateExcludedItemId('', { platform: 'youtube' })).toBe(true) + expect(validateExcludedItemId(null, { platform: 'youtube' })).toBe(true) + expect(validateExcludedItemId(undefined, { platform: 'youtube' })).toBe(true) + }) +}) + +describe('YOUTUBE_VIDEO_ID_PATTERN', () => { + it('matches exactly 11 url-safe chars', () => { + expect(YOUTUBE_VIDEO_ID_PATTERN.test('dQw4w9WgXcQ')).toBe(true) + expect(YOUTUBE_VIDEO_ID_PATTERN.test('e2evideo001')).toBe(true) + expect(YOUTUBE_VIDEO_ID_PATTERN.test('too-short')).toBe(false) + expect(YOUTUBE_VIDEO_ID_PATTERN.test('e2e-video-excluido-4')).toBe(false) + }) +})