fix(praktikant): normal-time/overtime split for the two § 50 presets - #1676
Merged
Conversation
Not ready to merge — the weekly-hours question is still under research and may change the model. Splits a praktikant day at the normal-time boundary: minutes up to it are attributed by clock-time bands, minutes beyond it to the overtime tiers, because § 50 stk. 4 d pays the stald supplements only 'for arbejde i normal arbejdstid'. The split is opt-in by preset name (normalised so 2024-2026 and 2026-2029 rows both match). An earlier revision gated on rule-set SHAPE instead, which silently caught 13 non-praktikant preset/day combinations whose tier 1 is a mirror of their clock split rather than a normal-time boundary — that would have awarded afternoon supplements for morning-only Saturdays across seven other collective agreements. Truncation is pause-aware: the budget is netto but the segments are gross, so without this a pause inside the normal-time window pushed the boundary earlier and cost the worker an hour of Saturday-afternoon supplement. Grundlovsdag gets its § 29 noon split in the plugin, since TryGetDayType rejects GRUNDLOVSDAG so bands can never reach it. The normal-time boundary applies first, then the clock split picks the code, with the afternoon treatment read from the preset's own SUNDAY rule rather than hardcoded. 45 routing tests, including a guard that a non-praktikant banded preset still takes the bands-only path — the test whose absence let the scope breach through. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the TimePlanning pay-line routing for the two § 50 “Udenlandske praktikanter Landbrug …” presets so normal-time minutes are attributed by clock-time bands while minutes beyond the normal-time boundary are attributed by overtime tiers, including special handling for Grundlovsdag (noon split) and pause-aware truncation to prevent pauses from consuming the normal-time budget.
Changes:
- Implement opt-in normal-time/overtime split for banded days (by preset name identity), including Grundlovsdag noon split logic.
- Introduce a shared pause-field enumerator and pause-aware worked-segment truncation to prevent boundary drift.
- Add end-to-end routing regression tests + wire the new test class into CI shard filters; update preset definitions accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs | Adds opt-in split routing (bands then overtime tiers), Grundlovsdag noon split, pause interval enumeration + pause-aware truncation, and DEFAULT→NORMAL mapping at the router exit. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PayRuleSetLock.cs | Adds preset-name identity gate (normalized) for enabling the normal-time split behavior. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PraktikantPayLineRoutingTests.cs | New end-to-end router tests covering praktikant presets, Grundlovsdag behavior, scope guard, pauses, and documented midnight clamping. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/Helpers/PraktikantFixtures.cs | New in-memory fixtures mirroring the two praktikant presets used by routing tests. |
| eform-client/src/app/plugins/modules/time-planning-pn/models/pay-rule-sets/pay-rule-set-presets.ts | Updates the two praktikant presets’ tiers to encode the normal-time boundary and overtime progression; adds documentation notes for Grundlovsdag behavior. |
| .github/workflows/dotnet-core-pr.yml | Adds the new routing test class to shard d filter so it runs in PR CI. |
| .github/workflows/dotnet-core-master.yml | Adds the new routing test class to shard d filter so it runs on master CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4757
to
+4759
| var bandSeconds = Math.Min(totalSeconds, normalSeconds); | ||
| var overtimeSeconds = Math.Max(0, totalSeconds - normalSeconds); | ||
|
|
| { | ||
| // WHY the split below: clock-time supplements (Saturday-afternoon | ||
| // animal care, Sunday/holiday) are payable per § 50 stk. 4 d only | ||
| // "For arbejde I NORMAL ARBEJDSTID". Minutes worked beyond the daily |
This was referenced Aug 7, 2026
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.
Companion to microting/eform-timeplanning-base#898 (merged — test-only, no NuGet release needed).
The bug
For
Udenlandske praktikanter Landbrug Staldarbejde, Saturday, Sunday and holidays declare time bands. The router picks bands over tiers, so those days' tiers never executed — and they encoded no overtime anyway. A praktikant working 12 hours on a Saturday was credited zero overtime minutes.The fix
A praktikant day now splits at the normal-time boundary: minutes up to it are attributed by clock-time bands, minutes beyond it to the overtime tiers, carrying no supplement code — because § 50 stk. 4 d pays the stald supplements only "for arbejde i normal arbejdstid".
The split is opt-in by preset name, normalised so both
2024-2026and2026-2029rows match. An earlier revision gated on rule-set shape instead (has bands + >1 tier); review caught that this silently captured 13 non-praktikant preset/day combinations. Five of those encode tier 1 as a mirror of their clock split, so the shape gate would have paid afternoon supplements for morning-only Saturdays across seven other collective agreements — with green CI. There is now a regression test driving a non-praktikant banded preset through the router.Also fixed
CalculateHoursAfterNoon(one source of truth, so the two can't drift).TryGetDayTyperejectsGRUNDLOVSDAG, so bands can never reach it; the split is done in the plugin. The normal-time boundary applies first, then the clock split picks the code, with the afternoon treatment read from each preset's own SUNDAY rule rather than hardcoded.DEFAULT→NORMALfor days with no matching rule.Tests
45 routing tests, all through
CalculatePayLinesForDay— the layer the previous 14 praktikant tests bypassed by calling the generators directly, which is why they passed while asserting a path that could never run. Every test uses a strict conservation helper (exact code set, exact seconds per code, sum equals worked seconds, and the test's own expectations cross-checked against that total). Registered in sharddof both workflows — a new class does not run otherwise.Verified against the agreement, not assumed
Researched against the primary texts (Jordbrug 2021-2024 / 2024-2026 / 2026-2029 + GLS-A lønoversigt). A hypothesis that overtime also triggers on a 37-hour weekly total was refuted: § 22 stk. 1 is explicitly daily ("efter normal daglig arbejdstids ophør"), and the only weekly threshold in the agreement is 45 h under an opt-in § 9 stk. 5 local agreement. The daily model is therefore correct in structure.
Known limitations (documented, not fixed here)
🤖 Generated with Claude Code