fix(pay-rule-sets): keep GLS-A presets locked, show durations as hh:mm - #1673
Merged
Conversation
Locked-preset status is a name-string match, and the preset catalogue was renamed '... 2024-2026' -> '... 2026-2029' without migrating existing rows, so every pre-existing GLS-A rule set silently became editable and deletable. Names are now compared with the trailing agreement period normalized away. The lock also had three open side doors: the nested pay-tier-rules, pay-day-type-rules and pay-time-band-rules endpoints had no guard at all, so a locked set's overtime thresholds could be rewritten one tier at a time (verified: PUT returned 200). The lock logic moves to a shared PayRuleSetLock helper and is enforced on Create/Update/Delete in all three nested services as well as the aggregate one, plus the edit modal, which was a fourth call site still matching on the exact name. Tier durations were entered and shown as raw seconds, and an unlimited top tier rendered as an empty field behind a placeholder='28800' - which reads as a real, nonsensically low threshold (lower than the tier below it). Durations are now edited and displayed as hours:minutes, and an unbounded tier says 'Unlimited' in words. Malformed input no longer writes null (which would read as unlimited) and blocks saving. Tests: 16 nested-service guard tests across CI shards c/d/e covering legacy and current agreement periods plus a custom-name negative, a delete guard test for a current-period name, and a Playwright spec asserting the row menu, the API guards and the 'unlimited' wording. Note: the GLS-A weekday tiers themselves are correctly encoded (7h24m -> 9h24m -> unlimited). Separately, CalculatePayLinesForDay applies time bands INSTEAD of tiers, so those overtime tiers never execute for a day that has bands - reported for a product decision, not changed here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens “locked” GLS-A pay rule set presets so they remain read-only even when stored under legacy agreement-period names, and improves tier threshold UX by editing durations as hh:mm and rendering unlimited tiers with an explicit label.
Changes:
- Centralizes locked-preset name matching (validity-period–agnostic) and enforces it server-side across pay rule set and nested rule mutation endpoints.
- Updates pay-tier UI formatting/editing to
hh:mminput with validation and clearer “Unlimited” rendering; removes legacy “seconds” wording from translations. - Adds/extends automated coverage (backend guard tests + Playwright e2e for lock + unlimited wording).
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/PayTimeBandRuleService/PayTimeBandRuleService.cs | Adds server-side locked-preset guard for time band rule create/update/delete. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/PayTierRuleService/PayTierRuleService.cs | Adds server-side locked-preset guard for tier rule create/update/delete. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/PayRuleSetService/PayRuleSetService.cs | Switches locked-preset detection to shared helper and normalizes agreement period suffix. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/PayDayTypeRuleService/PayDayTypeRuleService.cs | Adds server-side locked-preset guard for day type rule create/update/delete. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PayRuleSetLock.cs | Introduces shared normalization + locked-preset detection helper. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PayTimeBandRuleServiceTests.cs | Adds guard tests and updates service construction for localization dependency. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PayTierRuleServiceTests.cs | Adds guard tests and updates service construction for localization dependency. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PayRuleSetServiceTests.cs | Adds tests for legacy/current period locking and rename-into-locked prevention. |
| eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PayDayTypeRuleServiceTests.cs | Adds guard tests and updates service construction for localization dependency. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/pay-rule-sets.module.ts | Declares new hh:mm seconds value-accessor directive in module. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/pay-rule-lock.util.ts | Adds client-side normalization + locked-preset matching utility. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/pay-rule-format.util.ts | Adds hh:mm parsing/formatting helpers and “Unlimited” label support in tier chain formatting. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/directives/hh-mm-seconds.directive.ts | Adds ControlValueAccessor/Validator to edit seconds-backed values as hh:mm. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-rule-sets-view-modal/pay-rule-sets-view-modal.component.ts | Renders unlimited tier with translated label in tier chain display. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-rule-sets-table/pay-rule-sets-table.component.ts | Uses normalized locked-preset matching to disable edit/delete actions for legacy names. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-rule-sets-edit-modal/pay-rule-sets-edit-modal.component.ts | Normalizes locked preset detection and renders unlimited tier label in summaries. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-rule-sets-create-modal/pay-rule-sets-create-modal.component.ts | Renders unlimited tier label in tier summaries. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-rule-sets-container/pay-rule-sets-container.component.ts | Uses normalized locked-preset matching for belt-and-braces edit/delete guards. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-day-rule-list/pay-day-rule-list.component.ts | Shows tier breakdown using translated “Unlimited”/“No tiers” and human-readable durations. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-day-rule-form/pay-day-rule-form.component.ts | Adds hint rendering aligned to hh:mm editor + validation state. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/pay-rule-sets/components/pay-day-rule-form/pay-day-rule-form.component.html | Switches “Up To” editor to hh:mm text input with validation + updated help text. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/ukUA.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/svSE.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/slSL.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/skSK.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/roRO.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/ptPT.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/ptBR.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/plPL.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/noNO.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/nlNL.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/lvLV.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/ltLT.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/itIT.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/isIS.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/huHU.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/hrHR.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/frFR.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/fiFI.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/etET.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/esES.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/enUS.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/elGR.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/deDE.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/da.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/csCZ.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/src/app/plugins/modules/time-planning-pn/i18n/bgBG.ts | Updates tier-related strings/keys for hh:mm and “Unlimited”. |
| eform-client/playwright/e2e/plugins/time-planning-pn/c/pay-rule-sets-lock-and-hhmm.spec.ts | Adds e2e coverage for legacy-name locking and unlimited-tier wording. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+209
to
+212
| if (await OwningPayRuleSetIsLocked(rule.PayDayRuleId)) | ||
| { | ||
| return new OperationResult(false, _localizationService.GetString("CannotEditLockedPreset")); | ||
| } |
Comment on lines
+212
to
+215
| if (await OwningPayRuleSetIsLocked(rule.PayDayTypeRuleId)) | ||
| { | ||
| return new OperationResult(false, _localizationService.GetString("CannotEditLockedPreset")); | ||
| } |
Comment on lines
+308
to
+311
| if (await OwningPayRuleSetIsLocked(rule.PayRuleSetId)) | ||
| { | ||
| return new OperationResult(false, _localizationService.GetString("CannotEditLockedPreset")); | ||
| } |
Comment on lines
+74
to
+76
| const list = await page.request.get(`${BASE_URL}/api/time-planning-pn/pay-rule-sets`, { headers }); | ||
| const rows = (await list.json()).model?.payRuleSets || (await list.json()).model || []; | ||
| const found = (Array.isArray(rows) ? rows : []).filter((r: any) => r.name === name); |
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
Reported: GLS-A rule sets were editable when they shouldn't be, thresholds were shown in raw seconds, and the third weekday tier appeared to have a lower number than the two above it.
Findings first
The GLS-A rule is correctly encoded. The stored WEEKDAY tiers are 26640s (7h24m) →
NORMAL, 33840s (9h24m cumulative) →OVERTIME_30, null = unlimited →OVERTIME_80, matching the agreement. The "lower third number" was a UI artifact: that field is empty, andplaceholder="28800"rendered as grey text that reads like a value.Why the sets became editable. Lock status is a hardcoded name match;
bf340d6frenamed the catalogue… 2024-2026→… 2026-2029without migrating rows, so existing sets matched nothing.The lock had three side doors.
pay-tier-rules,pay-day-type-rulesandpay-time-band-ruleshad no guard — verified live,PUT /api/time-planning-pn/pay-tier-rules/{id}on a locked set returned HTTP 200. A locked agreement's overtime thresholds could be rewritten one tier at a time.Changes
PayRuleSetLockhelper and enforced on Create/Update/Delete in all three nested services plus the aggregate one. The edit modal (a fourth call site still using exact-name matching) now renders read-only for legacy-named sets.ControlValueAccessor; an unbounded tier reads "Ubegrænset". Malformed input no longer writesnull(which read as "unlimited") and keeps the form invalid. Two stale i18n keys mentioning "28800 seconds" removed from all 26 locales.Tests
16 nested-service guard tests (CI shards c/d/e) covering legacy + current agreement periods and a custom-name negative; a delete-guard test for a current-period name; a Playwright spec (shard c) asserting the row menu, the API guards, and the unlimited wording.
Verified locally
Full-solution build clean. Against the running backend: the tier PUT that previously returned 200 now returns "locked preset and is read-only", delete likewise, tier data unchanged; the row menu disables Rediger/Slet; force-opening the edit modal yields the read-only summary; the view shows
NORMAL (7h24m) → OVERTIME_30 (9h24m) → OVERTIME_80 (Ubegrænset).CalculatePayLinesForDayroutes a day exclusively: if the DayType has any time bands, only band attribution runs and the tier rules never execute. The GLS-A presets define bands for every weekday, so the overtime tiers are unreachable — existing testTimeBand_Standard_Weekday_Normal_0600_1800locks in a 12-hour shift producing oneNORMALline and no overtime. The agreement intends clock-time supplements and duration-based overtime to coexist, so long weekday shifts are currently under-paid. Changing this is payroll semantics and needs a product decision.🤖 Generated with Claude Code