# Task 7 — Review

**Base:** 3f16283  **Head:** 6c13410  
**Revisore:** Sonnet 4.6  
**Data:** 2026-06-24

---

## Check obbligatorio — gemelle e id passato a getPdfStatus

**Conclusione: BUG IMPORTANTE — id sbagliato passato al `PdfButton` per le gemelle.**

Cosa ho letto nel diff (`+page.server.ts`, riga 95 e riga 252-255 del diff):

```ts
// +page.server.ts — riga 66
const dvrAgenziaId = a.a.dvrCondivisoDaId ?? id;

// riga 95: getPdfStatus riceve dvrAgenziaId — corretto
const pdfStatus = await getPdfStatus(dvrAgenziaId);
```

```svelte
<!-- +page.svelte — riga 252-255 del diff -->
<PdfButton
    agenziaId={a.id}          <!-- ← a.id = id della route corrente -->
    state={data.pdfStatus.state}
    generatoIl={data.pdfStatus.generatoIl}
/>
```

`getPdfStatus(dvrAgenziaId)` è corretto: legge `dvr_pdf` e `codiceFisico` della compagna (dove risiede il PDF reale).

Però `<PdfButton agenziaId={a.id}>` passa l'id PROPRIO dell'agenzia visualizzata nel dettaglio — che per una gemella è l'id della sede "vuota" che prende in prestito il DVR. Quindi quando l'utente clicca "Genera PDF" o "Aggiorna PDF", il `trigger()` fa `POST /api/dvr/${agenziaId}/pdf` con l'id sbagliato. Stesso per il polling `GET /api/dvr/${agenziaId}/pdf?mode=status` e per il link download `GET /api/dvr/${agenziaId}/pdf`.

Il `data.pdfStatus` mostrato viene dalla compagna (giusto), ma le fetch client-side del `PdfButton` puntano alla gemella sbagliata. L'endpoint riceverà un id che probabilmente non ha una riga `dvr_pdf`, farà il fingerprint sull'agenzia sbagliata, e lo stato mostrato andrà fuori sincronia con quello scaricato.

**Correzione necessaria:** passare `agenziaId={data.dvrAgenziaId}` (esponendo `dvrAgenziaId` dal load), oppure calcolare `dvrAgenziaId` nel template come `data.agenzia.dvrCondivisoDaId ?? data.agenzia.id`.

---

## Spec Compliance

### Step 1 — Componente `PdfButton.svelte` ✅ (con riserva su gemelle via `agenziaId`)

- `app/src/lib/components/PdfButton.svelte` creato. Svelte 5 runes, props corrette.
- Tutti e 5 gli stati renderizzano la UI prevista dalla spec:
  - `ready` → link "Scarica PDF" + data (`generatoIl.slice(0,10)`) ✅
  - `stale` → bottone "Aggiorna PDF" + link "scarica versione precedente" (`?allowStale=1`) ✅
  - `generating` → spinner + testo + link versione precedente ✅
  - `error` → bottone "Errore — riprova" ✅
  - `missing` (`:else`) → bottone "Genera PDF" ✅
- Polling: parte su POST → 202/ok, ogni 12s, si ferma quando `j.state !== 'generating'`. ✅
- `$effect(() => () => { if (timer) clearInterval(timer); })` — cleanup corretto, no leak. ✅
- `untrack(() => initialState)` — corretto: zittisce warning `state_referenced_locally`, comportamento invariato (il prop è seed SSR, non reactivity source). ✅
- Copy italiano. ✅

### Step 2 — Passa lo stato PDF nel load del dettaglio ⚠️

- `getPdfStatus(dvrAgenziaId)` chiamato correttamente (la compagna per le gemelle). ✅
- `pdfStatus` restituito nel return. ✅
- **Ma `dvrAgenziaId` non è esposto nel return del load.** Il `PdfButton` nel template usa `a.id` invece di `dvrAgenziaId`. ⚠️ → vedi Issue #1.

### Step 3 — `PdfButton` nel dettaglio ✅ (eccetto Issue #1)

- "Apri DVR →" NON rimosso; `PdfButton` affiancato dentro `<div class="flex flex-wrap items-center gap-3">`. ✅
- Mostrato solo quando `data.dvr` non è null (`{:else}` mostra testo alternativo senza PdfButton — sensato). ✅
- `state={data.pdfStatus.state}` e `generatoIl={data.pdfStatus.generatoIl}` passati. ✅
- **`agenziaId={a.id}` usa l'id proprio** invece di `dvrAgenziaId`. ⚠️ → Issue #1.

### Step 4 — Icona nell'elenco ✅

- Nessuna query di stato per riga (MVP): il link "PDF" punta a `/agenzie/${a.id}#dvr`. ✅
- `onclick={(e) => e.stopPropagation()}` per evitare doppio click con riga cliccabile. ✅
- `+page.server.ts` dell'elenco non toccato (colonna `hasDvr` resta). ✅

### Step 5 — Type-check ✅

- L'implementer riporta `0 ERRORS 3 WARNINGS` con i 3 warning preesistenti. Nessun nuovo warning introdotto. Non eseguito dev-server come da istruzioni di review.

---

## Strengths

- `$effect(() => () => { if (timer) clearInterval(timer); })` — pattern cleanup idiomatico Svelte 5, corretto.
- `untrack(() => initialState)` usato con commento esplicativo: approccio consapevole, non cargo cult.
- `startPolling()` guarded con `if (timer) return` — no doppio timer accidentale.
- Il link "PDF" nell'elenco usa `e.stopPropagation()` — dettaglio che evita un bug UX con la riga cliccabile.
- Stile coerente col design system esistente: `bg-brand-500/hover:bg-brand-600`, `transition`, `shadow-sm`.
- `getPdfStatus` chiamato con `dvrAgenziaId` (compagna) nel server load — la logica gemelle è stata capita.

---

## Issues

### Important — `agenziaId` sbagliato nel `PdfButton` per le gemelle

**File:** `app/src/routes/agenzie/[id]/+page.svelte:252`

Per le sedi gemelle (dove `a.dvrCondivisoDaId != null`), `a.id` è l'id della sede "vuota" che prende in prestito il DVR. Il `PdfButton` riceve questo id e usa `/api/dvr/${agenziaId}/pdf` per trigger, polling e download. L'endpoint riceverà l'id sbagliato: la riga `dvr_pdf` appartiene alla compagna, non a questa sede.

Il `data.pdfStatus` mostrato inizialmente è corretto (viene dal server load che chiama `getPdfStatus(dvrAgenziaId)`), ma le operazioni client-side (Genera/Aggiorna/polling/download) puntano all'id sbagliato. Il risultato è uno stato UI che si desincronizza dopo il primo click.

**Fix:** esporre `dvrAgenziaId` nel return del load e usarlo nel template:

```ts
// +page.server.ts — aggiungere al return
dvrAgenziaId
```

```svelte
<!-- +page.svelte -->
<PdfButton
    agenziaId={data.dvrAgenziaId}
    state={data.pdfStatus.state}
    generatoIl={data.pdfStatus.generatoIl}
/>
```

In alternativa, nel template: `agenziaId={data.agenzia.dvrCondivisoDaId ?? data.agenzia.id}`.

### Minor — Nessuna gestione errore di rete nel polling

**File:** `app/src/lib/components/PdfButton.svelte:46-55`

Se la fetch del polling fallisce (rete, 500), `r.json()` lancia eccezione non catturata — il setInterval continua a girare silenziosamente ma il timer non viene mai pulito fino al `$effect` cleanup. Non critico (l'utente non vede nulla di strano), ma un try/catch nel loop di polling eviterebbe journal console errors in produzione.

### Minor — Il report descrive male il bug gemelle

**File:** `task-7-report.md`

Il report afferma: "il PdfButton usa `dvrAgenziaId` (che punta alla gemella per le sedi con DVR condiviso) sia per il polling che per il trigger." Questa affermazione è errata — il PdfButton riceve `a.id` (id proprio), non `dvrAgenziaId`. Il report non corrisponde all'implementazione. Da correggere assieme al fix del codice.

---

## Assessment

**Task quality: Needs fixes**

L'implementazione è solida nella struttura: `PdfButton` con i 5 stati, polling, cleanup del timer, `untrack`, elenco senza query extra. Ma il bug gemelle sul `agenziaId` passato al `PdfButton` è un errore funzionale che rompe trigger/polling/download per le ~6 sedi con DVR condiviso. Il fix è puntuale (esporre `dvrAgenziaId` nel return del load, passarlo al template), non richiede riscrittura. Dopo il fix, task approvabile.

**Conteggio:** 0 Critical · 1 Important · 2 Minor