You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PauseMinutesCalculator (the admin detailed-pause-edit save path) computes per-shift pause differently from the canonical PlanRegistrationHelper.ComputeShiftPauseSeconds that now drives netto/flex, the display field, the Excel export, the web dialog, and the Flutter app (shipped in #1626 / flutter-time #531). They disagree on two axes.
Surfaced while fixing the multi-pause under-counting bug (#1626). Filing as a separate follow-up since impact is mostly latent.
The two divergences
1. Slot attribution — Pause2 is assigned to the wrong shift.
PauseMinutesCalculator.DerivePauseId(planning, 1) (PauseMinutesCalculator.cs:35-49) folds Pause2 into shift 1; its shift-2 case (:53-65) excludes Pause2.
Product-owner-confirmed truth (PlanRegistration 16288): Pause2 belongs to shift 2 (shift1=30min, shift2=5min). So PauseMinutesCalculator's attribution is wrong.
2. Rounding formula.
Canonical OFF (5-min) mode: per-slot floor(stop) - floor(start) on the 5-min grid (ComputeShiftPauseSeconds, FloorTo5Min).
PauseMinutesCalculator.SlotMinutes (PauseMinutesCalculator.cs:74-82): (int)(stoppedAt - startedAt).TotalMinutes (floor of raw duration), then /5. Different result for any pause not aligned to 5-min clock boundaries.
Impact (why it's mostly latent today)
DerivePauseId is only invoked on the admin detailed-pause-edit save path (UseDetailedPauseEditing), at TimePlanningPlanningService.cs:717, 746, 1234, 1263. It writes only the legacy integerPause1Id/Pause2Id columns. The canonical method reads those integers only as a fallback — when a shift has no timestamped slot. On rows that carry pause timestamps (the same rows that trigger DerivePauseId), netto/display/export never read the integers, so the divergence usually does not surface. But it does misattribute pause minutes between the two integer columns and round differently, so any consumer that ever falls back to the integer for such a row would see a wrong/misattributed value.
Suggested fix
Make PauseMinutesCalculator use the same slot sets and rounding as the canonical ComputeShiftPauseSeconds (ideally route it through that single source of truth), so the legacy Pause{N}Id it writes is consistent with what every other consumer computes. Add a test that a multi-pause + Pause2 row produces matching per-shift values from both paths.
Summary
PauseMinutesCalculator(the admin detailed-pause-edit save path) computes per-shift pause differently from the canonicalPlanRegistrationHelper.ComputeShiftPauseSecondsthat now drives netto/flex, the display field, the Excel export, the web dialog, and the Flutter app (shipped in #1626 / flutter-time #531). They disagree on two axes.Surfaced while fixing the multi-pause under-counting bug (#1626). Filing as a separate follow-up since impact is mostly latent.
The two divergences
1. Slot attribution —
Pause2is assigned to the wrong shift.EnumerateShiftPauseStampPairs(PlanRegistrationHelper.cs): shift 1 =Pause1 + Pause10..19 + Pause100..102; shift 2 =Pause2+Pause20..29 + Pause200..202.PauseMinutesCalculator.DerivePauseId(planning, 1)(PauseMinutesCalculator.cs:35-49) foldsPause2into shift 1; its shift-2 case (:53-65) excludesPause2.Pause2belongs to shift 2 (shift1=30min, shift2=5min). SoPauseMinutesCalculator's attribution is wrong.2. Rounding formula.
floor(stop) - floor(start)on the 5-min grid (ComputeShiftPauseSeconds,FloorTo5Min).PauseMinutesCalculator.SlotMinutes(PauseMinutesCalculator.cs:74-82):(int)(stoppedAt - startedAt).TotalMinutes(floor of raw duration), then/5. Different result for any pause not aligned to 5-min clock boundaries.Impact (why it's mostly latent today)
DerivePauseIdis only invoked on the admin detailed-pause-edit save path (UseDetailedPauseEditing), atTimePlanningPlanningService.cs:717, 746, 1234, 1263. It writes only the legacy integerPause1Id/Pause2Idcolumns. The canonical method reads those integers only as a fallback — when a shift has no timestamped slot. On rows that carry pause timestamps (the same rows that triggerDerivePauseId), netto/display/export never read the integers, so the divergence usually does not surface. But it does misattribute pause minutes between the two integer columns and round differently, so any consumer that ever falls back to the integer for such a row would see a wrong/misattributed value.Suggested fix
Make
PauseMinutesCalculatoruse the same slot sets and rounding as the canonicalComputeShiftPauseSeconds(ideally route it through that single source of truth), so the legacyPause{N}Idit writes is consistent with what every other consumer computes. Add a test that a multi-pause +Pause2row produces matching per-shift values from both paths.References
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PauseMinutesCalculator.cs(DerivePauseId35-65,SlotMinutes74-82)eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs(ComputeShiftPauseSeconds,EnumerateShiftPauseStampPairs,FloorTo5Min)eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs(DerivePauseId call sites 717/746/1234/1263)