Skip to content

PauseMinutesCalculator diverges from canonical ComputeShiftPauseSeconds (slot attribution + rounding) #1628

Description

@renemadsen

Summary

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.

  • Canonical 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) 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 integer Pause1Id/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.

References

  • eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PauseMinutesCalculator.cs (DerivePauseId 35-65, SlotMinutes 74-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)
  • Context: PR fix(timeplanning): sum all pause slots for netto, display, and Excel export #1626 (multi-pause netto/display/export fix)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions