# Review — integracao-wms-desacoplada (Fase 1: saída plugável + config por empresa)

> **RE-REVIEW (2026-06-30, pós ciclo de correções).** A primeira passada (preservada no
> histórico git deste arquivo) achou 2 blockers (B1, B2), 2 majors (M1, M2) e alguns
> minors/nits. Esta versão verifica cada achado contra o código atual e checa regressões
> introduzidas pelos fixes. **Veredito: ✅ pronto para merge.**

**Base**: `dev_team` (sem merge p/ main — `project_git_workflow`)
**Escopo re-revisado**: Sinal `componentes/Missoes/Logistica/sinais/IntegracaoWms/` + 4 alterados
(`HookSincronizadorBase.php`, `ErpAlocacaoWmsHookStrategy.php`, `WmsAlocacaoWmsHookStrategy.php`,
`Api/Bootstrap.php`). Os ~125 arquivos do `operacao-wms` na árvore foram **ignorados** (não relacionados).
**`php -l`**: limpo nos 20 PHP da Fase 1 (sweep re-executado). JSON i18n válido nas 3 línguas.
**Testes unitários re-executados aqui**: AlocacaoHookStrategy 41/41, GalpaoProviderParidade 35/35,
WmsProviderRegistry 27/27, IntegracaoWmsConfigSonda 21/21 — todos PASS. (Integration
`ConfigIntegracaoTest` 47/47 reportado pelo dev; **não re-executado** neste ambiente por exigir `dev_gx_goldie`.)

## Sumário

Todos os achados acionáveis da primeira passada foram **resolvidos** e as correções **não
introduziram regressão**. B1 (typo camelCase) está corrigido nos 4 call sites + docblock; B2
(coluna `usuario_cadastro`) tem DDL + `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` idempotente; M1
ganhou um teste de strategy que exercita o caminho real (registra spy sob `'galpao'` **sem**
`setOverride`) e **genuinamente** pegaria o B1; M2 ganhou guard de backend (`PROVEDORES_STUB`) +
opção desabilitada na UI (defesa em profundidade). Os minors m1/m2/m4 e o nit n1 também foram
endereçados. A mudança no VO `WmsEnvioContexto` é backwards-compatible (params opcionais ao final).
**Recomendação: merge.** Restam apenas resíduos cosméticos sem impacto e 1 minor de Fase 2 (sem
Validator dedicado), todos aceitáveis.

## Status dos achados anteriores

| # | Achado | Status | Evidência |
|---|---|---|---|
| **B1** | `provedorSaida` camelCase → null → TypeError → push 100% quebrado | ✅ **Resolvido** | `provedor_saida` (snake) nos 4 call sites + docblock |
| **B2** | Migration omite `usuario_cadastro` → insert quebra | ✅ **Resolvido** | DDL linha 37 + `ALTER ... IF NOT EXISTS` linha 47 |
| **M1** | Suíte tautológica; nenhum teste exercita as strategies | ✅ **Resolvido** | `AlocacaoHookStrategyTddTest` (real path) + paridade de-tautologizada |
| **M2** | UI expõe stub `http_wms` selecionável sem aviso | ✅ **Resolvido** | guard `PROVEDORES_STUB` no Lab + `disabled`/"(Fase 2)" na UI |
| **m1** | Log do GalpaoProvider perdia Missão/Operação/Ref | ✅ **Resolvido** (resíduo cosmético) | VO ganhou `missaoId`/`referencia`; log restaurado |
| **m2** | `timeoutSegundos()` dead code | ✅ **Resolvido** | método removido; const `TIMEOUT_SEGUNDOS` mantida |
| **m3** | Sem Validator dedicado p/ config | 🟡 **Aberto** (aceito Fase 2) | `type="url"` na UI; sem validação server-side de formato |
| **m4** | JS hardcoda string PT fora do i18n | ✅ **Resolvido** (resíduo) | `data-str-erro-rede` injetado; fallback PT só como last-resort |
| **n1** | Vírgulas soltas na migration | ✅ **Resolvido** | commas na mesma linha da definição |

## Verificação detalhada dos blockers

### B1 — RESOLVIDO
Os 4 call sites agora leem `$config->provedor_saida` (snake): `ErpAlocacaoWmsHookStrategy.php:81`
(sincronizar) e `:143` (estornar); `WmsAlocacaoWmsHookStrategy.php:82` (sincronizar) e `:142`
(estornar). Docblock de exemplo em `WmsProviderFactory.php:12` corrigido com nota explícita
`// snake_case — coluna do banco`. Sweep `grep -rn provedorSaida componentes/` → **zero** ocorrências
camelCase remanescentes. `WmsProviderFactory::build()` continua não-nullable (correto): o contrato
está certo; o que estava errado era o argumento.

### B2 — RESOLVIDO
`Migrations/001_*.sql:37` adiciona `usuario_cadastro VARCHAR(255)` ao `CREATE TABLE`, e a linha 47
acrescenta `ALTER TABLE integracao_wms_config ADD COLUMN IF NOT EXISTS usuario_cadastro VARCHAR(255);`
para sanar ambientes onde a tabela já existia (já que `CREATE TABLE IF NOT EXISTS` não altera tabela
preexistente — `modelinteligencia_create_audit_columns`). Conjunto de auditoria/block agora completo
(`data_cadastro`, `data_atualizacao`, `usuario_cadastro`, `usuario_atualizacao`, `block_*`); nenhuma
outra coluna injetada por `ModelInteligencia::create()` ausente. Integration test passou de 38/47 →
47/47 (reportado pelo dev). O `ALTER ... IF NOT EXISTS` é **idempotente** (PostgreSQL 9.6+) e **seguro
em prod**: coluna nullable sem default = catalog update rápido, sem rewrite da tabela.

## Verificação detalhada dos majors

### M1 — RESOLVIDO (o ponto mais importante deste ciclo)
`Testes/Strategies/AlocacaoHookStrategyTddTest.php` (NOVO, 41 asserts, PASS verificado aqui):
- Registra `SpyWmsProvider` sob o nome `'galpao'` via `WmsProviderRegistry::registrar('galpao', …)`
  **sem** `setOverride()` — exatamente como o review pediu. Isso obriga a resolução a passar por
  `WmsProviderFactory::build($config->provedor_saida)` → `resolver('galpao')`.
- Chama `sincronizar()`/`estornar()` das **duas strategies reais** (8 invocações) e captura o
  `WmsEnvioContexto` montado pelo código de produção (não re-implementado à mão).
- **Pegaria o B1**: o `build($config->...)` está fora do try/catch por-item; com o bug,
  `build(null)` lançaria `TypeError` antes de o spy ser chamado → `SpyWmsProvider::$invocado===false`
  e o script abortaria sem imprimir "TODOS OS TESTES PASSARAM" → FAIL. Confirmado por leitura do fluxo.
- `GalpaoProviderParidadeTddTest` deixou de ser tautológico: as antigas asserções
  `$x === $x` (mesma expressão computada 2×) foram trocadas por comparações contra **literais legadas**
  (URL-final, header `Authorization: Bearer gnesistk`, body UTF-8 com/sem flag). 35/35 PASS.

### M2 — RESOLVIDO (defesa em profundidade)
- Backend: `SalvarConfigLaboratorio.php:25` define `PROVEDORES_STUB = ['http_wms']` e o guard
  (`:44`) rejeita **antes** do guard de "registrado", com mensagem clara ("não está disponível nesta
  fase … Use 'galpao'."). Como `processarSalvar()` sempre envia `provedor_saida`, o guard é sempre
  avaliado — protege mesmo contra POST direto que ignore a UI.
- UI: `html/integracaowms.php:324-330` marca a `<option>` do `http_wms` como `disabled` e anexa
  `show('(Fase 2 — em breve)')`. O provedor continua **registrado** no bootstrap (necessário para o
  guard de coerência), só não é selecionável.

## Checagem de regressão introduzida pelos fixes

1. **VO `WmsEnvioContexto` (novos params `missaoId`/`referencia`)** — ✅ backwards-compatible.
   Ambos opcionais (`?int $missaoId = null`, `?string $referencia = null`) e **anexados ao final**,
   depois do já-opcional `externalRef`. Call sites encontrados (`grep new WmsEnvioContexto`):
   as 4 strategies passam 8 args posicionais; `GalpaoProviderParidadeTddTest.php:242` passa 5 args
   nomeados (`destino/opc/payload/url/bearerToken`). Nenhum quebra. `HttpWmsProvider` não constrói o VO.
2. **Log restaurado no `GalpaoProvider`** — ✅ bate com o legado e o fallback é seguro.
   Original (`HookSincronizadorBase:192`): `[data] {label} Missão #{id} | Operação: {op} | Ref: {ref} |
   HTTP: {code} | Resposta: {resp}`. Novo (`GalpaoProvider:65-68`): mesmo esqueleto, com
   `$missaoLabel`/`$refLabel` montados condicionalmente. Quando `missaoId`/`referencia` são `null`
   (ex.: VO construído sem eles), os labels são **omitidos** — não quebra, só encurta a linha. No
   caminho real das strategies ambos são sempre passados (`missaoId` é int obrigatório; `referencia` =
   `(string) importacaoId|alocacaoId`). Resíduo cosmético: o label é `'GalpaoProvider'` (diferença
   **deliberada e documentada** no docblock, não mais o label da strategy) e `Operação:` usa
   `$contexto->opc`. Sem impacto funcional; observabilidade (agrupar por missão) restaurada.
3. **`ALTER TABLE` idempotente/seguro em prod** — ✅ (ver B2 acima).

## Issues remanescentes

### 🟡 Major
Nenhum.

### 🟢 Minor

#### m3 — Sem Validator dedicado para a config (mantido como TODO de Fase 2)
`endpoint_erp`/`endpoint_wms` são gravados e usados direto no `curl_init()` sem validação de formato
server-side (`SalvarConfigLaboratorio` só valida provedor registrado/stub). A UI usa `type="url"`
(validação client-side), o que mitiga, mas não é autoritativo. Aceitável para 7 campos simples de
Fase 1; os `OrdemEnvelopeValidator`/`OrdemIdentidadeValidator` planejados devem cobrir isso na Fase 2.

### ⚪ Nit

- **n-log** — Resíduo do m1: label de log `'GalpaoProvider'` ≠ label original da strategy
  (`Webhook ERP Alocação WMS`). É deliberado e está no docblock do `GalpaoProvider`; só registrar que
  greps por `Webhook ERP Alocação WMS` no `/tmp/genesis_webhook.log` deixarão de casar para alocação/sync.
- **n-js** — Resíduo do m4: a string PT continua hardcoded em `integracaowms.js:161` como **fallback**
  caso `data-str-erro-rede` esteja ausente. É só guard de borda; aceitável.
- **n2/n3** (1ª passada): `block_usuario_id` redundante no insert e `obterConfig()` chamado 2× por
  grupo de missão — micro, não tocados, ok p/ Fase 1.

## Pontos fortes

- **Correção cirúrgica dos blockers**: B1 resolvido em 4 linhas + docblock, sem refactor colateral;
  B2 com DDL **e** `ALTER ... IF NOT EXISTS` (pensou no banco já criado). Disciplina boa.
- **M1 virou o melhor teste do entregável**: o `AlocacaoHookStrategyTddTest` faz exatamente o que
  blindaria contra a classe de bug do B1 (resolução por nome via Registry, sem `setOverride`, captura
  do VO real). Isso é cobertura de não-regressão de verdade, não teatro de testes.
- **Defesa em profundidade no M2**: guard de backend (autoritativo) + UI desabilitada (UX). Mesmo um
  POST forjado não trava o tenant.
- **VO evoluído sem quebrar contrato**: params opcionais ao final = mudança aditiva limpa; todos os
  call sites validados.
- Demais qualidades da 1ª passada **mantidas**: Registry+Interface+Factory espelhando `OtpCanal*`;
  Sonda espelhando `ScanCotaConfigSonda` (customerControl=false, isolamento por `block_empresa_id`,
  `configDefault()` desligado, guard `\Throwable` deploy-safe); `block_*` sem `DEFAULT 1`; segredo
  fora do banco (`token_saida_ref`); i18n completo e **traduzido de verdade** nas 3 línguas (as 2
  chaves novas — `c78c0961…` "(Fase 2 — em breve)" e `fba31adb…` "Erro de rede…" — confirmadas em
  pt-br/en/es); naming V2 correto; sem raw SQL; CSS via `var(--bs-*)`.

## Recomendação

✅ **Merge.** Os 2 blockers e os 2 majors estão resolvidos e verificados; nenhuma regressão nova
introduzida pelos fixes (VO aditivo, log com fallback seguro, `ALTER` idempotente). Resta apenas o
minor m3 (Validator de URL) explicitamente diferido para a Fase 2, mais resíduos cosméticos de log/JS
sem impacto. Arquitetura permanece fiel às referências V2 (Otp/ScanCota).

_Observação operacional_: o integration `ConfigIntegracaoTest` (47/47) foi reportado pelo dev mas
**não foi re-executado** nesta máquina (exige `dev_gx_goldie`). Recomendo confirmar a aplicação da
migration 001 (incl. o `ALTER`) no ambiente alvo antes de habilitar a tela de config para usuários.