Files
SIRH/doc/hours-save-dirty-tracking.md
T
tristan a67e77dc50 fix(heures) : garde backend anti-suppression sur grille périmée (delete explicite)
## Problème
Le correctif #31 (dirty-tracking front) ne protège que les sessions qui
chargent le nouveau bundle. Un vieil onglet ouvert avant déploiement tourne
encore sur l'ancien JS et envoie toute la grille périmée → reproduit en prod :
un onglet ouvert le matin a supprimé ~10 lignes saisies dans la journée par
d'autres utilisateurs (entrée vide = suppression côté backend, sans garde).

## Correctif (suppression sur intention explicite)
`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, 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`, 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 (par 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). Front : revue de code (pas de harnais).

## Doc
- doc/hours-save-dirty-tracking.md, CLAUDE.md, doc in-app.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-25 09:19:10 +02: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).