fix(heures) : n'enregistrer que les lignes modifiées (anti-écrasement concurrent) (#31)
Auto Tag Develop / tag (push) Successful in 10s
Auto Tag Develop / tag (push) Successful in 10s
## Problème Sur l'écran **Heures** / **Heures Conducteurs**, l'enregistrement envoyait au bulk-upsert une entrée pour **tous** les employés visibles non verrouillés, à partir de l'état en mémoire de la grille. Le backend (`WorkHourBulkUpsertProcessor`) traitant une **entrée vide comme une suppression**, un admin avec une grille **périmée** pouvait supprimer une ligne saisie entre-temps par un autre utilisateur. ### Scénario reproduit 1. Un admin ouvre l'écran ; la ligne d'un salarié `ROLE_SELF` est vide. 2. Ce salarié saisit ses heures dans sa propre session → ligne créée, **non validée** (donc non verrouillée). 3. L'admin, sur sa grille périmée, enregistre d'autres employés. 4. Le payload contient une entrée **vide** pour le salarié → le backend supprime sa ligne. **Perte de données.** ## Correctif (suivi des lignes modifiées) `hydrateRows` capture un instantané `loadedRows` de l'état chargé depuis le serveur. `handleSave` ne transmet plus que les lignes **dont l'état courant diffère de l'instantané**. - Ligne **intouchée** → jamais envoyée → jamais supprimée ✅ - Ligne **vidée volontairement** → envoyée vide → supprimée (métier conservé) - Ligne **remplie/modifiée** → envoyée → créée/mise à jour Symétrique dans `useHoursPage.ts` et `useDriverHoursPage.ts`. ## Limite connue Pas de verrou optimiste backend : l'édition **explicite** d'une ligne sur données périmées peut toujours écraser une saisie concurrente sur cette même ligne (hors périmètre). ## Doc - `doc/hours-save-dirty-tracking.md` (nouveau) - Note `CLAUDE.md` (section *Validation Rules*) ## Vérification - Pre-commit hook : **236 tests PHPUnit OK**. - Pas de harnais de tests frontend (revue de code uniquement). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #31 Co-authored-by: tristan <tristan@yuno.malio.fr> Co-committed-by: tristan <tristan@yuno.malio.fr>
This commit was merged in pull request #31.
This commit is contained in:
@@ -64,6 +64,7 @@
|
|||||||
- `isSiteValid` (site manager): locks for non-admin, admin can still edit
|
- `isSiteValid` (site manager): locks for non-admin, admin can still edit
|
||||||
- Any real modification resets both `isSiteValid=false` and `isValid=false`
|
- Any real modification resets both `isSiteValid=false` and `isValid=false`
|
||||||
- No-op saves preserve existing validations
|
- No-op saves preserve existing validations
|
||||||
|
- **Enregistrement = seules les lignes modifiées sont envoyées (anti-écrasement concurrent)** : l'écran Heures / Heures Conducteurs affiche toute la journée, et le bulk-upsert (`WorkHourBulkUpsertProcessor`) traite une **entrée vide comme une suppression**. Pour éviter qu'un admin avec une grille **périmée** ne supprime une ligne saisie entre-temps par un autre utilisateur (ex. `ROLE_SELF` non encore validé → non verrouillé), `handleSave` ne transmet **que les lignes dont l'état courant diffère de l'instantané chargé** (`loadedRows`, capturé dans `hydrateRows` ; comparaison `JSON.stringify(buildEntry(current)) !== buildEntry(original)`). Une ligne intouchée n'est jamais envoyée → jamais supprimée. Vidée volontairement → envoyée vide → supprimée (métier conservé). Symétrique dans `useHoursPage.ts` et `useDriverHoursPage.ts`. **Limite** : pas de verrou optimiste backend — l'édition explicite d'une ligne sur données périmées peut toujours écraser une saisie concurrente sur cette même ligne. Doc : `doc/hours-save-dirty-tracking.md`.
|
||||||
|
|
||||||
## Overtime Rules
|
## Overtime Rules
|
||||||
- Contracts <= 35h: +25% from 35h to 43h, +50% beyond
|
- Contracts <= 35h: +25% from 35h to 43h, +50% beyond
|
||||||
|
|||||||
@@ -0,0 +1,54 @@
|
|||||||
|
# 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é** :
|
||||||
|
|
||||||
|
```ts
|
||||||
|
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).
|
||||||
|
|
||||||
|
## Limite connue (hors périmètre de ce correctif)
|
||||||
|
|
||||||
|
Le suivi des lignes modifiées **ne couvre pas** 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 résiduel relèverait d'un **verrou optimiste**
|
||||||
|
(comparaison d'`updatedAt`/version côté backend), non implémenté ici. Le backend n'a aucune
|
||||||
|
détection de conflit concurrent (pas de version, pas d'horodatage comparé).
|
||||||
@@ -48,6 +48,11 @@ export const useDriverHoursPage = () => {
|
|||||||
const selectedSiteIds = ref<number[]>([])
|
const selectedSiteIds = ref<number[]>([])
|
||||||
const sitesInitialized = ref(false)
|
const sitesInitialized = ref(false)
|
||||||
const rows = ref<Record<number, DriverHourRow>>({})
|
const rows = ref<Record<number, DriverHourRow>>({})
|
||||||
|
// Instantané des lignes telles que chargées depuis le serveur (clé = employeeId).
|
||||||
|
// Sert à n'envoyer au bulk-upsert que les lignes réellement modifiées, afin de ne jamais
|
||||||
|
// écraser/supprimer une ligne saisie entre-temps par un autre utilisateur (enregistrement
|
||||||
|
// « à l'aveugle » d'une grille périmée).
|
||||||
|
const loadedRows = ref<Record<number, DriverHourRow>>({})
|
||||||
const dayContext = ref<WorkHourDayContext | null>(null)
|
const dayContext = ref<WorkHourDayContext | null>(null)
|
||||||
const weeklySummary = ref<WeeklyWorkHourSummary | null>(null)
|
const weeklySummary = ref<WeeklyWorkHourSummary | null>(null)
|
||||||
const absenceTypes = ref<AbsenceType[]>([])
|
const absenceTypes = ref<AbsenceType[]>([])
|
||||||
@@ -458,6 +463,10 @@ export const useDriverHoursPage = () => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
rows.value = nextRows
|
rows.value = nextRows
|
||||||
|
// Clone indépendant : les éditions mutent les objets de rows.value, pas ceux-ci.
|
||||||
|
loadedRows.value = Object.fromEntries(
|
||||||
|
Object.entries(nextRows).map(([employeeId, row]) => [employeeId, { ...row }])
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
const loadAbsenceTypes = async () => {
|
const loadAbsenceTypes = async () => {
|
||||||
@@ -924,6 +933,32 @@ export const useDriverHoursPage = () => {
|
|||||||
await refreshByDate()
|
await refreshByDate()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Construit l'entrée bulk-upsert à partir d'une ligne (état courant OU instantané chargé).
|
||||||
|
const buildEntry = (employeeId: number, row: DriverHourRow) => {
|
||||||
|
const dayMin = toMinutes(row.dayHours)
|
||||||
|
const nightMin = toMinutes(row.nightHours)
|
||||||
|
const workshopMin = toMinutes(row.workshopHours)
|
||||||
|
|
||||||
|
return {
|
||||||
|
employeeId,
|
||||||
|
morningFrom: null,
|
||||||
|
morningTo: null,
|
||||||
|
afternoonFrom: null,
|
||||||
|
afternoonTo: null,
|
||||||
|
eveningFrom: null,
|
||||||
|
eveningTo: null,
|
||||||
|
isPresentMorning: false,
|
||||||
|
isPresentAfternoon: false,
|
||||||
|
dayHoursMinutes: dayMin || null,
|
||||||
|
nightHoursMinutes: nightMin || null,
|
||||||
|
workshopHoursMinutes: workshopMin || null,
|
||||||
|
hasBreakfast: row.hasBreakfast,
|
||||||
|
hasLunch: row.hasLunch,
|
||||||
|
hasDinner: row.hasDinner,
|
||||||
|
hasOvernight: row.hasOvernight
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const handleSave = async () => {
|
const handleSave = async () => {
|
||||||
if (isSubmitting.value || employees.value.length === 0) return
|
if (isSubmitting.value || employees.value.length === 0) return
|
||||||
|
|
||||||
@@ -933,32 +968,16 @@ export const useDriverHoursPage = () => {
|
|||||||
(e) => e.isDriver === true && hasContractAtSelectedDate(e.id)
|
(e) => e.isDriver === true && hasContractAtSelectedDate(e.id)
|
||||||
)
|
)
|
||||||
|
|
||||||
const entries = driverEmployees.map((employee) => {
|
const entries = driverEmployees
|
||||||
const employeeId = employee.id
|
.map((employee) => {
|
||||||
const row = rows.value[employeeId] ?? emptyRow()
|
const current = buildEntry(employee.id, rows.value[employee.id] ?? emptyRow())
|
||||||
const dayMin = toMinutes(row.dayHours)
|
const original = buildEntry(employee.id, loadedRows.value[employee.id] ?? emptyRow())
|
||||||
const nightMin = toMinutes(row.nightHours)
|
return { current, original }
|
||||||
const workshopMin = toMinutes(row.workshopHours)
|
})
|
||||||
|
// N'envoie que les lignes réellement modifiées : une ligne intouchée n'est jamais
|
||||||
return {
|
// transmise, donc jamais supprimée même si un autre utilisateur l'a saisie entre-temps.
|
||||||
employeeId,
|
.filter(({ current, original }) => JSON.stringify(current) !== JSON.stringify(original))
|
||||||
morningFrom: null,
|
.map(({ current }) => current)
|
||||||
morningTo: null,
|
|
||||||
afternoonFrom: null,
|
|
||||||
afternoonTo: null,
|
|
||||||
eveningFrom: null,
|
|
||||||
eveningTo: null,
|
|
||||||
isPresentMorning: false,
|
|
||||||
isPresentAfternoon: false,
|
|
||||||
dayHoursMinutes: dayMin || null,
|
|
||||||
nightHoursMinutes: nightMin || null,
|
|
||||||
workshopHoursMinutes: workshopMin || null,
|
|
||||||
hasBreakfast: row.hasBreakfast,
|
|
||||||
hasLunch: row.hasLunch,
|
|
||||||
hasDinner: row.hasDinner,
|
|
||||||
hasOvernight: row.hasOvernight
|
|
||||||
}
|
|
||||||
})
|
|
||||||
|
|
||||||
if (entries.length === 0) return
|
if (entries.length === 0) return
|
||||||
|
|
||||||
|
|||||||
@@ -50,6 +50,11 @@ export const useHoursPage = () => {
|
|||||||
const selectedSiteIds = ref<number[]>([])
|
const selectedSiteIds = ref<number[]>([])
|
||||||
const sitesInitialized = ref(false)
|
const sitesInitialized = ref(false)
|
||||||
const rows = ref<Record<number, HourRow>>({})
|
const rows = ref<Record<number, HourRow>>({})
|
||||||
|
// Instantané des lignes telles que chargées depuis le serveur (clé = employeeId).
|
||||||
|
// Sert à n'envoyer au bulk-upsert que les lignes réellement modifiées par l'utilisateur,
|
||||||
|
// afin de ne jamais écraser/supprimer une ligne saisie entre-temps par un autre utilisateur
|
||||||
|
// (perte de données par enregistrement « à l'aveugle » d'une grille périmée).
|
||||||
|
const loadedRows = ref<Record<number, HourRow>>({})
|
||||||
const dayContext = ref<WorkHourDayContext | null>(null)
|
const dayContext = ref<WorkHourDayContext | null>(null)
|
||||||
const weeklySummary = ref<WeeklyWorkHourSummary | null>(null)
|
const weeklySummary = ref<WeeklyWorkHourSummary | null>(null)
|
||||||
const absenceTypes = ref<AbsenceType[]>([])
|
const absenceTypes = ref<AbsenceType[]>([])
|
||||||
@@ -600,6 +605,10 @@ export const useHoursPage = () => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
rows.value = nextRows
|
rows.value = nextRows
|
||||||
|
// Clone indépendant : les éditions mutent les objets de rows.value, pas ceux-ci.
|
||||||
|
loadedRows.value = Object.fromEntries(
|
||||||
|
Object.entries(nextRows).map(([employeeId, row]) => [employeeId, { ...row }])
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
const loadAbsenceTypes = async () => {
|
const loadAbsenceTypes = async () => {
|
||||||
@@ -1136,6 +1145,36 @@ export const useHoursPage = () => {
|
|||||||
await refreshByDate()
|
await refreshByDate()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Construit l'entrée bulk-upsert à partir d'une ligne (état courant OU instantané chargé).
|
||||||
|
const buildEntry = (employee: Employee, row: HourRow) => {
|
||||||
|
const employeeId = employee.id
|
||||||
|
if (isPresenceTracking(employee)) {
|
||||||
|
return {
|
||||||
|
employeeId,
|
||||||
|
morningFrom: null,
|
||||||
|
morningTo: null,
|
||||||
|
afternoonFrom: null,
|
||||||
|
afternoonTo: null,
|
||||||
|
eveningFrom: null,
|
||||||
|
eveningTo: null,
|
||||||
|
isPresentMorning: row.isPresentMorning,
|
||||||
|
isPresentAfternoon: row.isPresentAfternoon
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
employeeId,
|
||||||
|
morningFrom: normalizeTime(row.morningFrom),
|
||||||
|
morningTo: normalizeTime(row.morningTo),
|
||||||
|
afternoonFrom: normalizeTime(row.afternoonFrom),
|
||||||
|
afternoonTo: normalizeTime(row.afternoonTo),
|
||||||
|
eveningFrom: normalizeTime(row.eveningFrom),
|
||||||
|
eveningTo: normalizeTime(row.eveningTo),
|
||||||
|
isPresentMorning: false,
|
||||||
|
isPresentAfternoon: false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const handleSave = async () => {
|
const handleSave = async () => {
|
||||||
if (isSubmitting.value || employees.value.length === 0) return
|
if (isSubmitting.value || employees.value.length === 0) return
|
||||||
|
|
||||||
@@ -1144,34 +1183,14 @@ export const useHoursPage = () => {
|
|||||||
const entries = employees.value
|
const entries = employees.value
|
||||||
.filter((employee) => hasContractAtSelectedDate(employee.id) && !isRowLocked(employee.id))
|
.filter((employee) => hasContractAtSelectedDate(employee.id) && !isRowLocked(employee.id))
|
||||||
.map((employee) => {
|
.map((employee) => {
|
||||||
const employeeId = employee.id
|
const current = buildEntry(employee, rows.value[employee.id] ?? emptyRow())
|
||||||
const row = rows.value[employeeId] ?? emptyRow()
|
const original = buildEntry(employee, loadedRows.value[employee.id] ?? emptyRow())
|
||||||
if (isPresenceTracking(employee)) {
|
return { current, original }
|
||||||
return {
|
|
||||||
employeeId,
|
|
||||||
morningFrom: null,
|
|
||||||
morningTo: null,
|
|
||||||
afternoonFrom: null,
|
|
||||||
afternoonTo: null,
|
|
||||||
eveningFrom: null,
|
|
||||||
eveningTo: null,
|
|
||||||
isPresentMorning: row.isPresentMorning,
|
|
||||||
isPresentAfternoon: row.isPresentAfternoon
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return {
|
|
||||||
employeeId,
|
|
||||||
morningFrom: normalizeTime(row.morningFrom),
|
|
||||||
morningTo: normalizeTime(row.morningTo),
|
|
||||||
afternoonFrom: normalizeTime(row.afternoonFrom),
|
|
||||||
afternoonTo: normalizeTime(row.afternoonTo),
|
|
||||||
eveningFrom: normalizeTime(row.eveningFrom),
|
|
||||||
eveningTo: normalizeTime(row.eveningTo),
|
|
||||||
isPresentMorning: false,
|
|
||||||
isPresentAfternoon: false
|
|
||||||
}
|
|
||||||
})
|
})
|
||||||
|
// N'envoie que les lignes réellement modifiées : une ligne intouchée n'est jamais
|
||||||
|
// transmise, donc jamais supprimée même si un autre utilisateur l'a saisie entre-temps.
|
||||||
|
.filter(({ current, original }) => JSON.stringify(current) !== JSON.stringify(original))
|
||||||
|
.map(({ current }) => current)
|
||||||
|
|
||||||
if (entries.length === 0) {
|
if (entries.length === 0) {
|
||||||
return
|
return
|
||||||
|
|||||||
Reference in New Issue
Block a user