Files
SIRH/doc/hours-save-dirty-tracking.md
T
tristan d66288d061
Auto Tag Develop / tag (push) Successful in 13s
fix(heures) : garde backend anti-suppression sur grille périmée (delete explicite) (#36)
## Contexte (incident prod)
Le correctif #31 (dirty-tracking front) ne protège que les sessions chargeant le nouveau bundle. Un **vieil onglet** ouvert avant déploiement tourne encore sur l'ancien JS et envoie toute la grille périmée. Hier soir, un onglet ouvert le matin a **supprimé ~10 lignes d'heures** saisies dans la journée par d'autres utilisateurs (journal BDD à l'appui : 1 save = 2 updates + 8 deletes de lignes intactes).

Cause : le backend traitait toute **entrée vide comme une suppression**, sans aucune garde indépendante du client.

## Correctif — suppression sur intention explicite (`delete: true`)
`WorkHourBulkUpsertProcessor` ne supprime une ligne existante sur entrée vide **que si l'entrée porte `delete: true`**. Sinon → **no-op** (ligne préservée). Aucune grille périmée, quel que soit le client (vieil onglet inclus), ne peut plus détruire une saisie concurrente. La création de ligne technique de validation reste limitée à `null === $existing`.

Le front (à jour) pose `delete: true` sur une ligne **vidée volontairement** (helper `isEntryEmpty`, appliqué après le filtre dirty-tracking) → suppression métier conservée. Flag optionnel ajouté au DTO front (`WorkHourEntryPayload`) et back (`WorkHourBulkUpsert`), défaut `false`.

## Testabilité
Le processor dépend désormais des interfaces repo (`EmployeeScopedRepositoryInterface` / `WorkHourReadRepositoryInterface`, repos concrets `final` non mockables) → nouveau test unitaire `WorkHourBulkUpsertProcessorTest` (no-op sans flag / suppression avec flag / update normal).

## Limite résiduelle (choix : suppression explicite, pas verrou optimiste)
L'**édition explicite** d'une ligne sur données périmées peut encore écraser une saisie concurrente sur cette même ligne. Seule la **suppression** est blindée.

## Vérification
- **267 tests PHPUnit OK** (dont 3 nouveaux), via le pre-commit hook.
- Front : revue de code (pas de harnais de tests front).

## Doc
- `doc/hours-save-dirty-tracking.md`, `CLAUDE.md`, doc in-app (`documentation-content.ts`).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed-on: #36
Co-authored-by: tristan <tristan@yuno.malio.fr>
Co-committed-by: tristan <tristan@yuno.malio.fr>
2026-06-25 07:29:02 +00:00

5.3 KiB

Enregistrement des heures — envoi des seules lignes modifiées

Problème corrigé (perte de données par écrasement « à l'aveugle »)

L'écran Heures (et Heures Conducteurs) présente une grille d'une journée avec tous les employés du périmètre. L'enregistrement (POST /work-hours/bulk-upsert, WorkHourBulkUpsertProcessor) a une sémantique upsert par (employé, date) où une entrée vide supprime la ligne existante (« une ligne vide supprime l'enregistrement »).

Avant correctif, handleSave (front) envoyait une entrée pour chaque employé visible non verrouillé, à partir de l'état en mémoire de la grille. Conséquence en cas de concurrence :

  1. Un admin ouvre l'écran ; la ligne d'un salarié (ex. utilisateur ROLE_SELF) est vide.
  2. Ce salarié saisit ses heures dans sa propre session → ligne créée en BDD, non validée (donc non verrouillée).
  3. L'admin, sur sa grille périmée, saisit les heures d'autres employés et enregistre.
  4. Le payload contient une entrée vide pour le salarié (état périmé). Le backend relit la BDD (ligne désormais remplie), constate « entrée vide ≠ existant » → supprime la ligne fraîchement saisie. Perte de données.

Correctif (suivi des lignes modifiées côté front)

hydrateRows capture désormais un instantané des lignes telles que chargées depuis le serveur (loadedRows, clone indépendant de rows). À l'enregistrement, handleSave ne transmet que les lignes dont l'état courant diffère de l'instantané chargé :

const entries = candidates
  .map((employee) => ({
    current:  buildEntry(employee, rows.value[employee.id] ?? emptyRow()),
    original: buildEntry(employee, loadedRows.value[employee.id] ?? emptyRow()),
  }))
  .filter(({ current, original }) => JSON.stringify(current) !== JSON.stringify(original))
  .map(({ current }) => current)

Conséquences :

  • Une ligne intouchée n'est jamais transmise → jamais supprimée, même si un autre utilisateur l'a saisie/modifiée entre-temps. C'est le correctif du bug.
  • Une ligne vidée volontairement par l'utilisateur diffère de l'instantané → transmise vide → supprimée (comportement métier conservé).
  • Une ligne remplie diffère → transmise → créée/mise à jour.

Implémenté symétriquement dans frontend/composables/useHoursPage.ts (non-conducteurs) et frontend/composables/useDriverHoursPage.ts (conducteurs).

Garde backend : suppression sur intention explicite (delete: true)

Le suivi front ne protège que les sessions qui chargent le nouveau bundle. Un vieil onglet ouvert avant le déploiement continue de tourner sur l'ancien JavaScript (sans dirty-tracking) et envoie toute la grille périmée → le bug s'est reproduit en prod (un onglet ouvert le matin a supprimé une dizaine de lignes saisies dans la journée par d'autres utilisateurs). La protection front est donc insuffisante : il faut une garde côté backend, indépendante de la version du client.

WorkHourBulkUpsertProcessor ne supprime désormais une ligne existante sur entrée vide que si la suppression est explicitement demandée par le flag delete: true sur l'entrée :

$deleteRequested = true === ($entry['delete'] ?? false);
if ($existing && $deleteRequested) {
    // suppression (audit 'delete' + remove)
} elseif (null === $existing && ($absence || $is4hContract)) {
    // création d'une ligne technique (validation d'une journée d'absence / contrat 4h)
}
// existing && !deleteRequested → NO-OP : la ligne existante est préservée

Conséquences :

  • Une entrée vide sans flag sur une ligne existante est un no-op → une grille périmée (n'importe quel client, vieil onglet inclus) ne peut plus détruire une saisie concurrente.
  • Le front (à jour) pose delete: true sur une ligne vidée volontairement (entrée vide qui diffère de l'instantané chargé, donc transmise) → la suppression métier reste possible. Helper isEntryEmpty(...) dans les deux composables, appliqué après le filtre dirty-tracking.

Le delete est optionnel (WorkHourEntryPayload front, DTO WorkHourBulkUpsert back) et vaut false par défaut. Les appels de création de ligne technique de validation (toggles de validation quand aucune ligne n'existe) envoient une entrée vide sans flag → inchangés (branche null === $existing).

Tests : tests/State/WorkHourBulkUpsertProcessorTest.php (no-op sans flag / suppression avec flag / mise à jour d'une entrée non vide). Le processor dépend des interfaces EmployeeScopedRepositoryInterface / WorkHourReadRepositoryInterface (repos concrets final, non mockables) pour permettre ces tests unitaires.

Limite connue résiduelle (hors périmètre)

Reste le cas où l'admin édite explicitement une ligne sur des données périmées (il voit la ligne vide, tape une valeur, écrasant une saisie concurrente sur cette même ligne). Ce cas relèverait d'un verrou optimiste (comparaison d'updatedAt/version côté backend), non implémenté ici — choix métier (option « suppression explicite » retenue plutôt que verrou optimiste). Le backend n'a aucune détection de conflit concurrent sur l'édition (seule la suppression est désormais protégée par l'intention explicite).