feat(export): add a Helligdagstimer column for statutory holidays only - #1704
Merged
Merged
Conversation
The Total sheet of the all-workers Excel export gains one column,
"Helligdagstimer", immediately after "Søn- og helligdagstimer"
(column J -> new column K).
The existing column J counts Sundays plus EVERY entry in
danish_holidays_2025_2030.json. The new column counts only holidays as
defined by law, i.e. entries whose category is 'official_holiday' --
never the agreement-based days off ('overenskomstfastsat_fridag':
Grundlovsdag and Juleaften), and never a Sunday.
The distinction already existed in the JSON's `category` field, which
was deserialised but never read. IsStatutoryHoliday now reads it.
IsOfficialHoliday is deliberately left alone: it is load-bearing for
column J AND for the pay-code day classification
(GetDayCodeForDate -> DayType.Holiday), so the two predicates must not
collapse into one.
Also fixes a pre-existing bug in the column J loop, which summed the
bare NettoHours and ignored an active override while column G
(siteTotalNettoHours) honours it -- so column I + column J disagreed
with column G on any overridden Sunday or holiday. Both sums now select
the day's hours exactly as column G does.
Known gap, left as-is and documented at the call site: an overridden
Grundlovsdag still ignores the override, because CalculateHoursAfterNoon
derives from the shift stamps and a netto override carries no
information about which hours fell after 12:00. Splitting an override
across noon is a payroll policy decision, not a code fix.
Column M (Saturday hours) has the same bare-NettoHours issue and is
deliberately out of scope here.
The new test class is registered in the CI shard filters in both
workflow files; without that it would silently never run.
Tests run only in CI, so these have not been executed locally.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified that would block approval.
Pull request overview
Adds a statutory-holiday-only hours column to the all-workers Excel export while preserving existing Sunday/holiday calculations.
Changes:
- Adds and localizes the new Total-sheet column.
- Introduces category-based statutory holiday detection and override-aware totals.
- Adds workbook, helper, and CI-shard test coverage.
File summaries
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.resx | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.Designer.cs | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.da.resx | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelShiftColumnOrderTests.cs | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelHolidayColumnTests.cs | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TimePlanningWorkingHoursExportTests.cs | Updated as part of this pull request. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PlanRegistrationHelperHolidayTests.cs | Updated as part of this pull request. |
| .github/workflows/dotnet-core-pr.yml | Updated as part of this pull request. |
| .github/workflows/dotnet-core-master.yml | Updated as part of this pull request. |
Review details
Files not reviewed (1)
- eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Resources/Translations.Designer.cs: Generated file
- Files reviewed: 10/11 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What
Adds one column to the Total sheet of the all-workers Excel export on
plugins/time-planning-pn/planning, named Helligdagstimer, sitting immediately after Søn- og helligdagstimer (column J → new column K).The new column counts only holidays as defined by law — not every day flagged as a holiday in the backend.
Why the two columns differ
official_holidayoverenskomstfastsat_fridag(Grundlovsdag, Juleaften)The distinction already existed in
danish_holidays_2025_2030.jsonas thecategoryfield, which was deserialised but never read.IsStatutoryHolidaynow reads it.IsOfficialHolidayis deliberately unchanged — it is load-bearing for column J and for the pay-code day classification (GetDayCodeForDate→DayType.Holiday). The two predicates must not collapse into one.Bug fixed along the way
The column J loop summed the bare
NettoHours, ignoring an active override, while column G (siteTotalNettoHours) honours it. So column I + column J did not reconcile with column G on any overridden Sunday or holiday. Both sums now select the day's hours exactly as column G does.Known gaps, deliberate and documented
CalculateHoursAfterNoonderives from the shift stamps, and a netto override carries no information about which hours fell after 12:00. Splitting an override across noon is a payroll policy decision, not a code fix — documented at the call site.NettoHoursissue; out of scope here.2028-06-05: Grundlovsdag collides with 2. pinsedag, so that date carries both categories and is statutory. On that one date column K can exceed column J. Pinned by a test so it cannot surprise anyone.2030-12-26— both predicates fail closed after that.Hours Holidaykey is added toen+daonly, matching the adjacentNormal Hourscolumn, which is already English-only in the other locales. Other languages fall back to "Holiday Hours" rather than shipping invented payroll terminology.Tests
New
WorkingHoursExcelHolidayColumnTestsopens the real produced .xlsx and asserts cell values for columns G/I/J/K:col I + col J == col G0so later columns cannot shiftPlus
IsStatutoryHolidayunit coverage, including a whole-calendar subset sweep and the 2028 collision.The new test class is registered in the CI shard filters in both workflow files — without that it would silently never run.
🤖 Generated with Claude Code