Skip to content

Commit 98f1892

Browse files
renemadsenrenemadsen
authored
Merge pull request #1604 from microting/fix/per-worker-paycode-columns
fix(export): per-worker pay-code columns in the all-workers report
2 parents 4c2f6e1 + 2208fe6 commit 98f1892

5 files changed

Lines changed: 728 additions & 5 deletions

File tree

‎docs/superpowers/plans/2026-06-10-per-worker-paycode-columns.md‎

Lines changed: 318 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,318 @@
1+
# Per-Worker Pay-Code Columns Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** In the all-workers Excel export, make each per-site (per-worker) sheet show only the pay-code columns declared in that worker's own pay-rule-set (none if the worker has no rule-set), leaving the Total summary sheet and the single-worker export unchanged.
6+
7+
**Architecture:** Add a pure `GetDeclaredPayCodes(PayRuleSet)` helper that returns a rule-set's declared pay codes. In the all-workers per-site loop, compute `sitePayCodes` from the cached per-site `PayRuleSet` and use it (instead of the global `allPayCodes`) for the per-site sheet's pay-code header, per-day cells, and totals row. The global `allPayCodes` and `siteTotalsByPayCode` stay as-is so the Total sheet is byte-identical.
8+
9+
**Tech Stack:** C# / .NET 10, DocumentFormat.OpenXml, NUnit 4 + Testcontainers.MariaDb. Base dev mode: edit in host-app mirror `eform-angular-frontend/eFormAPI/Plugins/TimePlanning.Pn/`, then `devgetchanges.sh` to the source repo.
10+
11+
**Spec:** `docs/superpowers/specs/2026-06-10-per-worker-paycode-columns-design.md`
12+
13+
**Key file:** `…/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs`
14+
15+
**Verified facts:**
16+
- All-workers per-site loop emits pay codes at 3 points, all using global `allPayCodes`:
17+
header (~line 3194), per-day cells (~3267), per-site totals row (~3346).
18+
- `siteTotalsByPayCode = allPayCodes.ToDictionary(p => p, p => 0.0)` (~3252) is accumulated
19+
(~3287–3290, guarded by `ContainsKey`) and consumed by BOTH the per-site totals row AND the
20+
Total-sheet row (~3403). It must stay keyed by `allPayCodes` for the Total sheet.
21+
- The per-site pay-code header (~3194) is built BEFORE `perSiteCache.TryGetValue(... out var cache)`
22+
(~3258). So `sitePayCodes` must be computed before the header loop (use a separate lookup var to
23+
avoid touching the existing `cache` declaration).
24+
- `AutoFilter` uses `GetColumnLetter(headerStrings.Count)` (~3353) — auto-adjusts to the per-site
25+
column count once the header uses `sitePayCodes`. No change needed there.
26+
- `PayRuleSet` entity (eform-timeplanning-base): `DayRules: ICollection<PayDayRule>`,
27+
`DayTypeRules: ICollection<PayDayTypeRule>`, `HolidayPaidOffPayCode: string`.
28+
`PayDayRule.Tiers: ICollection<PayTierRule>`; `PayTierRule.PayCode: string`, `.Order: int`.
29+
`PayDayTypeRule.DefaultPayCode: string`, `.TimeBandRules: ICollection<PayTimeBandRule>`;
30+
`PayTimeBandRule.PayCode: string`.
31+
- `CalculatePayLinesForDay` is `internal static` (~line 4168); `MergeByPayCode` static (~3922).
32+
`InternalsVisibleTo("TimePlanning.Pn.Test")` is configured, so `internal static` helpers are
33+
unit-testable directly without a service instance.
34+
- `CalculatePayLinesForDayTests.cs` is NOT in any CI shard — do NOT add CI-critical tests there.
35+
`PlanRegistrationHelperTests.cs` is in shard **f** and already hosts static-helper tests
36+
(GetShiftTime). `DagsoversigtWorksheetExportTests.cs` is in shard **g** and exercises the
37+
all-workers export.
38+
39+
---
40+
41+
## Task 1: Add `GetDeclaredPayCodes` (TDD)
42+
43+
**Files:**
44+
- Modify: `…/TimePlanningWorkingHoursService.cs` (add static helper near `MergeByPayCode`, ~line 3922)
45+
- Test: `…/TimePlanning.Pn.Test/PlanRegistrationHelperTests.cs`
46+
47+
- [ ] **Step 1: Write the failing unit tests**
48+
49+
Add to `PlanRegistrationHelperTests.cs` (inside the top-level `[TestFixture]`). The helper is
50+
`internal static`, so call it directly on the type — no instance/DB needed. Match the file's
51+
existing NUnit 4 style (`using` for the entities may be needed — `Microting.TimePlanningBase.Infrastructure.Data.Entities`).
52+
53+
```csharp
54+
[Test]
55+
public void GetDeclaredPayCodes_CollectsFromAllSources_DedupsAndSkipsEmpty_OrdersTiersByOrder()
56+
{
57+
var ruleSet = new PayRuleSet
58+
{
59+
DayRules = new List<PayDayRule>
60+
{
61+
new PayDayRule { Tiers = new List<PayTierRule>
62+
{
63+
new PayTierRule { PayCode = "B", Order = 2 },
64+
new PayTierRule { PayCode = "A", Order = 1 },
65+
}},
66+
new PayDayRule { Tiers = new List<PayTierRule>
67+
{
68+
new PayTierRule { PayCode = "A", Order = 1 }, // duplicate
69+
}},
70+
},
71+
DayTypeRules = new List<PayDayTypeRule>
72+
{
73+
new PayDayTypeRule { DefaultPayCode = "C", TimeBandRules = new List<PayTimeBandRule>
74+
{
75+
new PayTimeBandRule { PayCode = "D" },
76+
new PayTimeBandRule { PayCode = "" }, // skipped
77+
}},
78+
new PayDayTypeRule { DefaultPayCode = null, TimeBandRules = new List<PayTimeBandRule>() }, // skipped
79+
},
80+
HolidayPaidOffPayCode = "E",
81+
};
82+
83+
var codes = TimePlanningWorkingHoursService.GetDeclaredPayCodes(ruleSet);
84+
85+
// tiers ordered by Order (A before B) within the first day rule, then C, D, then holiday E;
86+
// duplicate A and empty/null skipped.
87+
Assert.That(codes, Is.EqualTo(new List<string> { "A", "B", "C", "D", "E" }));
88+
}
89+
90+
[Test]
91+
public void GetDeclaredPayCodes_NullRuleSet_ReturnsEmpty()
92+
{
93+
Assert.That(TimePlanningWorkingHoursService.GetDeclaredPayCodes(null), Is.Empty);
94+
}
95+
96+
[Test]
97+
public void GetDeclaredPayCodes_EmptyRuleSet_ReturnsEmpty()
98+
{
99+
Assert.That(TimePlanningWorkingHoursService.GetDeclaredPayCodes(new PayRuleSet()), Is.Empty);
100+
}
101+
```
102+
103+
- [ ] **Step 2: Run the tests to verify they FAIL**
104+
105+
Run: `cd /home/rene/Documents/workspace/microting/eform-angular-frontend/eFormAPI && dotnet test Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TimePlanning.Pn.Test.csproj --filter GetDeclaredPayCodes`
106+
Expected: FAIL — `GetDeclaredPayCodes` does not exist (compile error). (These tests are pure/in-memory; if the fixture needs a DB container that can't start here, report DONE_WITH_CONCERNS — CI shard f runs them.)
107+
108+
- [ ] **Step 3: Implement `GetDeclaredPayCodes`**
109+
110+
Add near `MergeByPayCode` (~line 3922) in `TimePlanningWorkingHoursService.cs`:
111+
112+
```csharp
113+
/// <summary>
114+
/// Returns the pay codes DECLARED by a pay-rule-set, in structural order
115+
/// (day-rule tiers by Order, then day-type default codes and their time-band codes,
116+
/// then the holiday code), de-duplicated (first-seen wins), skipping null/empty codes.
117+
/// Returns an empty list when payRuleSet is null. Used to scope per-worker pay-code
118+
/// columns in the all-workers export to each worker's own rule-set.
119+
/// </summary>
120+
internal static List<string> GetDeclaredPayCodes(PayRuleSet payRuleSet)
121+
{
122+
var codes = new List<string>();
123+
if (payRuleSet == null)
124+
{
125+
return codes;
126+
}
127+
128+
void Add(string code)
129+
{
130+
if (!string.IsNullOrWhiteSpace(code) && !codes.Contains(code))
131+
{
132+
codes.Add(code);
133+
}
134+
}
135+
136+
if (payRuleSet.DayRules != null)
137+
{
138+
foreach (var dayRule in payRuleSet.DayRules)
139+
{
140+
if (dayRule.Tiers == null) continue;
141+
foreach (var tier in dayRule.Tiers.OrderBy(t => t.Order))
142+
{
143+
Add(tier.PayCode);
144+
}
145+
}
146+
}
147+
148+
if (payRuleSet.DayTypeRules != null)
149+
{
150+
foreach (var dayTypeRule in payRuleSet.DayTypeRules)
151+
{
152+
Add(dayTypeRule.DefaultPayCode);
153+
if (dayTypeRule.TimeBandRules == null) continue;
154+
foreach (var timeBand in dayTypeRule.TimeBandRules)
155+
{
156+
Add(timeBand.PayCode);
157+
}
158+
}
159+
}
160+
161+
Add(payRuleSet.HolidayPaidOffPayCode);
162+
163+
return codes;
164+
}
165+
```
166+
167+
(`System.Linq` and the `PayRuleSet` type are already in scope in this file.)
168+
169+
- [ ] **Step 4: Run the tests to verify they PASS**
170+
171+
Run: `cd /home/rene/Documents/workspace/microting/eform-angular-frontend/eFormAPI && dotnet test Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TimePlanning.Pn.Test.csproj --filter GetDeclaredPayCodes`
172+
Expected: all 3 pass.
173+
174+
- [ ] **Step 5: Do not commit** (base dev mode — commit once at the end, Task 4).
175+
176+
---
177+
178+
## Task 2: Use per-site codes in the all-workers per-site sheet
179+
180+
**Files:**
181+
- Modify: `…/TimePlanningWorkingHoursService.cs` — all-workers `GenerateExcelDashboard(TimePlanningWorkingHoursReportForAllWorkersRequestModel)` per-site loop.
182+
183+
Read the per-site loop first to anchor on exact current text (the three `foreach (var payCode in allPayCodes)` blocks and the header). Make these four edits. Do NOT touch the global `allPayCodes` pre-pass, `siteTotalsByPayCode`'s seed/accumulation, the Total-sheet header/row, or the single-worker export.
184+
185+
- [ ] **Step 1: Compute `sitePayCodes` before the per-site pay-code header**
186+
187+
Find the per-site header pay-code loop:
188+
```csharp
189+
// Append one column header per pay code discovered across all sites
190+
foreach (var payCode in allPayCodes)
191+
{
192+
headerStrings.Add(payCode);
193+
}
194+
```
195+
Replace it with (compute `sitePayCodes` from the cached per-site rule-set, then use it):
196+
```csharp
197+
// Per-worker pay-code columns: only the codes declared in THIS site's
198+
// pay-rule-set (empty when the site has no rule-set). The global allPayCodes
199+
// is still used for the Total sheet, which is intentionally unchanged.
200+
perSiteCache.TryGetValue(siteIds[i], out var siteCacheForCodes);
201+
var sitePayCodes = GetDeclaredPayCodes(siteCacheForCodes?.PayRuleSet);
202+
foreach (var payCode in sitePayCodes)
203+
{
204+
headerStrings.Add(payCode);
205+
}
206+
```
207+
(`siteCacheForCodes` is a new local distinct from the existing `cache` declared later — no conflict.)
208+
209+
- [ ] **Step 2: Per-day pay-code cells use `sitePayCodes`**
210+
211+
Find:
212+
```csharp
213+
foreach (var payCode in allPayCodes)
214+
{
215+
var payLine = dayPayLines.FirstOrDefault(pl => pl.PayCode == payCode);
216+
dataRow.Append(CreateNumericCell(payLine?.Hours ?? 0));
217+
}
218+
```
219+
Change the loop source to `sitePayCodes`:
220+
```csharp
221+
foreach (var payCode in sitePayCodes)
222+
{
223+
var payLine = dayPayLines.FirstOrDefault(pl => pl.PayCode == payCode);
224+
dataRow.Append(CreateNumericCell(payLine?.Hours ?? 0));
225+
}
226+
```
227+
Leave the `siteTotalsByPayCode` accumulation block (the `foreach (var pl in dayPayLines) { if (siteTotalsByPayCode.ContainsKey(pl.PayCode)) ... }`) UNCHANGED — it feeds the Total sheet.
228+
229+
- [ ] **Step 3: Per-site totals row uses `sitePayCodes` (safe lookup)**
230+
231+
Find:
232+
```csharp
233+
foreach (var payCode in allPayCodes)
234+
{
235+
siteTotalsRow.Append(CreateNumericCell(siteTotalsByPayCode[payCode]));
236+
}
237+
```
238+
Change to iterate `sitePayCodes` and read totals safely (a declared code with 0 hours everywhere is not a key in `siteTotalsByPayCode`):
239+
```csharp
240+
foreach (var payCode in sitePayCodes)
241+
{
242+
siteTotalsRow.Append(CreateNumericCell(siteTotalsByPayCode.GetValueOrDefault(payCode, 0)));
243+
}
244+
```
245+
246+
- [ ] **Step 4: Confirm the column counts stay aligned & build**
247+
248+
The per-site header, each data row, and the totals row now all iterate `sitePayCodes` → equal pay-code cell counts. `AutoFilter` uses `headerStrings.Count` so it auto-adjusts. Build:
249+
250+
Run: `cd /home/rene/Documents/workspace/microting/eform-angular-frontend/eFormAPI && dotnet build Plugins/TimePlanning.Pn/TimePlanning.Pn/TimePlanning.Pn.csproj`
251+
Expected: Build succeeded.
252+
253+
- [ ] **Step 5: Do not commit.**
254+
255+
---
256+
257+
## Task 3: Export regression/coverage test (different rule-sets per worker)
258+
259+
**Files:**
260+
- Modify: `…/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs` (shard g; exercises the all-workers export)
261+
262+
Read the file's `SeedSiteAndPlanRegistration`/seed helpers and `TestBaseSetup` first to learn how sites, assigned sites, and (if present) pay-rule-sets are created. Look at `PayRuleSetServiceTests.cs` for how a `PayRuleSet` with `DayRules`/`Tiers` is constructed/persisted, and reuse that pattern to seed rule-sets in the plugin DbContext.
263+
264+
- [ ] **Step 1: Write the test**
265+
266+
Add `[Test] AllWorkers_PerSiteSheets_UseEachWorkersOwnPayRuleSetCodes` that:
267+
- Seeds **site A** with an `AssignedSite.PayRuleSetId` → a `PayRuleSet` whose declared codes are e.g. `{ "AAA" }` (one `PayDayRule` with one `PayTierRule { PayCode = "AAA" }`), plus a PlanRegistration in the period.
268+
- Seeds **site B** with a different `PayRuleSet` declaring `{ "BBB" }`, plus a PlanRegistration.
269+
- Seeds **site C** with NO `PayRuleSetId`, plus a PlanRegistration.
270+
- Calls the all-workers export `GenerateExcelDashboard(new TimePlanningWorkingHoursReportForAllWorkersRequestModel { DateFrom, DateTo })`.
271+
- Opens the produced xlsx and, for each per-site sheet (named `site.Name`, i.e. "Site A"/"Site B"/"Site C"), reads the header row and asserts:
272+
- site A's sheet header contains `"AAA"` and NOT `"BBB"`.
273+
- site B's sheet header contains `"BBB"` and NOT `"AAA"`.
274+
- site C's sheet header contains neither `"AAA"` nor `"BBB"` (no pay-code columns).
275+
- the **Total** sheet header still contains BOTH `"AAA"` and `"BBB"` (union unchanged).
276+
277+
Read sheet headers by resolving each `Sheet` by name via the workbook `Sheets` and `GetPartById`, then reading row 1 cells (reuse/extend the file's existing header-reading helper; pay-code headers are plain string cells appended after the fixed columns). Dispose each returned stream as the existing tests do (the export returns an open FileStream; the file-collision pattern from the prior fix applies if multiple exports run in the same second — call the all-workers export once here).
278+
279+
- [ ] **Step 2: Build + run**
280+
281+
Run: `cd /home/rene/Documents/workspace/microting/eform-angular-frontend/eFormAPI && dotnet test Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TimePlanning.Pn.Test.csproj --filter AllWorkers_PerSiteSheets_UseEachWorkersOwnPayRuleSetCodes`
282+
Expected: PASS (with Task 2 applied). Needs Docker/Testcontainers; if unavailable here, report DONE_WITH_CONCERNS with the test written — CI shard g runs it.
283+
284+
- [ ] **Step 3: Do not commit.**
285+
286+
> If seeding a full `PayRuleSet` graph in the fixture proves disproportionately complex (e.g. the
287+
> seed helpers don't support assigned-site → rule-set linkage and constructing it inline is
288+
> heavy), STOP and report NEEDS_CONTEXT with what you found, rather than writing a test that
289+
> doesn't actually exercise per-site codes. The Task 1 unit test + the manual verification in
290+
> Task 4 still cover correctness; this integration test is the ideal but not worth a brittle hack.
291+
292+
---
293+
294+
## Task 4: Verify, code-review, sync, PR, CI
295+
296+
- [ ] **Step 1: Full build + targeted tests**
297+
298+
Run: `cd /home/rene/Documents/workspace/microting/eform-angular-frontend/eFormAPI && dotnet build Plugins/TimePlanning.Pn/TimePlanning.Pn/TimePlanning.Pn.csproj && dotnet test Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TimePlanning.Pn.Test.csproj --filter "GetDeclaredPayCodes|AllWorkers_PerSiteSheets_UseEachWorkersOwnPayRuleSetCodes"`
299+
Expected: build succeeds; the new tests pass.
300+
301+
- [ ] **Step 2: Code review** — use `superpowers:requesting-code-review` on the diff (service helper + per-site wiring + tests). Confirm the Total sheet and single-worker export are untouched.
302+
303+
- [ ] **Step 3: Sync to source repo**
304+
305+
From `/home/rene/Documents/workspace/microting/eform-angular-timeplanning-plugin`: create branch `fix/per-worker-paycode-columns` off `stable`, run `./devgetchanges.sh`, then `git checkout -- '*.csproj' '*.conf.ts' '*.xlsx' '*.docx'`, then `git status`. Confirm only intended files changed: `…/TimePlanningWorkingHoursService.cs`, `…/TimePlanning.Pn.Test/PlanRegistrationHelperTests.cs`, `…/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs`, plus the spec/plan docs. `git checkout` anything unintended.
306+
307+
- [ ] **Step 4: Commit + push + PR + watch CI**
308+
309+
Stage only the intended files by name, commit (end message with `Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>`), push, open a PR toward `stable`, and watch CI green. The unit test runs in shard **f** (`PlanRegistrationHelperTests`) and the export test in shard **g** (`DagsoversigtWorksheetExportTests`) — both already in the matrix, so no workflow change is needed. Treat any known-flaky playwright shard failures as flaky (re-run); fix genuine failures.
310+
311+
---
312+
313+
## Self-Review
314+
315+
- **Spec coverage:** per-site sheets use declared per-site codes (Task 2 Steps 1–3) ✓; declared-code helper from all four sources, dedup, order, null→empty (Task 1) ✓; no rule-set → no columns (`GetDeclaredPayCodes(null)`→empty drives empty header — Task 1 + Task 2 Step 1) ✓; Total sheet unchanged (`allPayCodes`/`siteTotalsByPayCode` untouched; per-site totals row reads via `GetValueOrDefault` — Task 2 Steps 2–3) ✓; single-worker untouched ✓; tests (unit + export E2E) ✓.
316+
- **Placeholder scan:** none — concrete code/commands throughout. The Task 3 fallback is an explicit escalation condition, not a vague placeholder.
317+
- **Type/name consistency:** `GetDeclaredPayCodes(PayRuleSet) : List<string>` (internal static) defined in Task 1, called identically in Task 2 Step 1 and the Task 1 tests; `sitePayCodes`/`siteCacheForCodes` locals introduced in Task 2 Step 1 and reused in Steps 2–3; `siteTotalsByPayCode.GetValueOrDefault(payCode, 0)` matches the existing `Dictionary<string,double>` type.
318+
- **Total-sheet safety:** the only read of `siteTotalsByPayCode` that changes is the per-site totals row (now `sitePayCodes` + `GetValueOrDefault`); the Total-sheet row (`foreach allPayCodes … siteTotalsByPayCode[payCode]`) and the seed/accumulation are untouched, so the Total sheet stays byte-identical.

0 commit comments

Comments
 (0)