feat(pause): per-shift pause override (non-destructive) + server-side app compat#1629
Merged
Conversation
Approach C: per-shift Pause{N}OverrideMinutes override that ComputeShiftPauseSeconds
honors, preserving the worker's recorded pause start/stops for documentation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ride scope Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… time edit Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
….53 published) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… app compat
Lets web and the UNCHANGED mobile app set a shift's total pause without
destroying the worker's recorded pause start/stops (kept for documentation).
Consumes base 10.0.53 Pause{N}OverrideMinutes columns.
- ComputeShiftPauseSeconds + ComputePlanningNettoMinutes honor the override
(else sum all recorded slots) — one chokepoint → netto, display, export agree.
- WRITE (UpdateByCurrentUserNam + Update): infer the override from the app's
Break{N}Shift only when it differs from the currently-shown coarse tick
(override → (override/5)+1, else Pause{N}Id) — so editing only start/stop
never spuriously locks an override and re-saves are idempotent (incl.
non-5-min exact overrides). Recorded timestamps never touched. Explicit web
override fields (Pause{N}OverrideMinutesSpecified / ClearPauseOverrides) and
the one-minute exact-minute path take precedence.
- READ projection (GetPlanningsByUser history + ReadWorkingHours today):
when an override is set, the gRPC RESPONSE synthesizes a single
Pause{N}StartedAt/StoppedAt pair of the override duration, zeroes that shift's
sub-slots, and sets Break{N}Shift/Pause{N}Id/Shift{N}Pause + pauseMinutes to
the override — so the unchanged app displays it. DB rows untouched (no app or
proto change; wire-compatible).
- EnsureTimestampsFromIds no longer fabricates Pause{N}StartedAt/StoppedAt for
shifts with an active override.
- Removed the destructive ApplyExactMinutePause/ClearPauseTimestamps path.
- REST DTO override fields for the web dialog (Phase 3). SQL dump + unit and
integration tests (incl. the regression: edit pause on a sub-slot row →
effective netto reflects edit, slots preserved).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a non-destructive, per-shift pause total override that becomes authoritative for pause/netto calculations while preserving workers’ recorded pause start/stop timestamps (including sub-slots), and adds a server-side projection so the unchanged mobile app can display the override correctly.
Changes:
- Add
Pause{N}OverrideMinutesoverride support to pause computation and netto calculations, plus write-path inference / explicit clear handling. - Project overrides onto outgoing DTOs/working-hours models by synthesizing a single pause interval and clearing sub-slots in the response (DB remains untouched).
- Update test schema + add unit/integration tests covering override precedence, inference idempotency, and non-destructive behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/TimePlanning.Pn.csproj | Bumps Microting.TimePlanningBase to 10.0.53 to pick up override columns. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs | Adds capture/change-detect inference + exact-minute override writing; guards timestamp synthesis under overrides; updates netto computation to respect overrides. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/Planning/TimePlanningPlanningPrDayModel.cs | Adds override + “specified/clear” signals needed to distinguish “not sent” vs explicit null/clear. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs | Makes override authoritative in ComputeShiftPauseSeconds; adds read-projection helpers to synthesize pause stamps for app compatibility. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/SQL/420_eform-angular-time-planning-plugin.sql | Adds override columns to test DB schema dump. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PlanningServiceMultiShiftTests.cs | Adds integration coverage for override write + non-destructive preservation + projection behavior. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ComputeShiftPauseSecondsTests.cs | Adds unit coverage for override behavior and inference helper contracts. |
| docs/superpowers/specs/2026-06-25-pause-override-design.md | Design/spec doc capturing the approach and phasing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+646
to
+651
| // Snapshot each shift's PRE-EDIT legacy coarse pause tick (Pause{N}Id) | ||
| // BEFORE the model is applied, so the pause-override inference can | ||
| // change-detect an admin pause edit (Approach C) by comparing the | ||
| // submitted Break{N}Shift against this tick — without locking an | ||
| // override when only start/stop changed (16288-shape rows), and | ||
| // idempotently for non-5-multiple overrides on unrelated re-saves. |
Comment on lines
1847
to
+1851
| if (useOneMinuteIntervals && startedAt.HasValue && stoppedAt.HasValue && stoppedAt.Value > startedAt.Value) | ||
| { | ||
| var shiftMinutes = (stoppedAt.Value - startedAt.Value).TotalMinutes; | ||
| double pauseMinutes = 0; | ||
| if (pauseStarted.HasValue && pauseStopped.HasValue && pauseStopped.Value > pauseStarted.Value) | ||
| double pauseMinutes; | ||
| if (overrideMinutes.HasValue) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
After #1626/#531 made pause timestamp slots the source of truth, neither the web admin dialog nor the mobile app could edit a shift's total pause (the legacy
Pause{N}Idis ignored when slots exist; the app's manual edit wrote onlybreakNShift). This adds a per-shift override that is authoritative for the total, while the worker's recorded pause start/stops stay intact for documentation.Depends on base Microting.TimePlanningBase 10.0.53 (
Pause{N}OverrideMinutescolumns).What
ComputeShiftPauseSeconds+ComputePlanningNettoMinutesreturn the override when set, else sum all recorded slots — one chokepoint, so netto/display/export agree.Break{N}Shiftonly when it differs from the currently-shown coarse tick (override→(override/5)+1, elsePause{N}Id). Editing only start/stop never locks an override; re-saves don't round non-5-min overrides. Recorded timestamps untouched. Explicit web fields (Pause{N}OverrideMinutesSpecified/ClearPauseOverrides) and the one-minute exact-minute path take precedence.GetPlanningsByUser+ todayReadWorkingHours) synthesizes onePause{N}StartedAt/StoppedAtpair of the override duration, zeroes that shift's sub-slots, and setsBreak{N}Shift/Pause{N}Id/Shift{N}Pause+pauseMinutes— so the unchanged app displays the override. DB rows untouched.EnsureTimestampsFromIdsno longer fabricates pause timestamps under an active override.ApplyExactMinutePause/ClearPauseTimestamps.Tests
Unit (
ComputeShiftPauseSecondsTestsincl.PauseOverrideInferenceTests, 25 pass, no Docker): override-wins, override=0, null→slot-sum, per-shift no-leak, change-detection (edit-only-start/stop leaves null), idempotent non-5-min override, re-edit, explicit clear, exact-minute-not-overwritten,EnsureTimestampsFromIdsguard. Integration (Testcontainer, run in CI): edit pause on a sub-slot row → effective netto reflects edit + recorded slots preserved (the regression-catcher); read-projection DTO round-trip.Two reviews + a fix round (correctness + simplification) on the diff before commit.
🤖 Generated with Claude Code