feat(pause-web): admin dialog edits total pause via override (non-destructive)#1630
Merged
Conversation
…tructive)
Wires the workday-entity dialog's per-shift Pause field to the explicit
pause{N}OverrideMinutes + pause{N}OverrideMinutesSpecified DTO fields (Phase 2),
set only when the admin actually changed the pause (baseline captured on load) —
so editing only start/stop never locks an override. Adds a "reset pause to
recorded" affordance per shift (override→null, revert to recorded slots). The
field prefers the raw served override for bit-exact display. The worker's
recorded pause start/stops are never modified.
Tests: Angular unit (set/unchanged/clear/served-override-display) + the
previously-missing e2e round-trip (edit→save→reopen→assert pause+netto) for both
flag-off (5-min) and flag-on (1-min off-grid) sites.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Wires the Time Planning admin workday dialog to persist per-shift total pause edits via the new non-destructive Pause{N}OverrideMinutes / ...Specified fields, and adds unit + Playwright e2e coverage to prevent regressions where pause edits don’t round-trip after reopen.
Changes:
- Extend the Angular planning day model with per-shift pause override fields matching the backend DTO.
- Add dialog display + save wiring to prefer served overrides, emit overrides only on real changes, and provide a “reset pause to recorded” affordance.
- Add unit tests and two Playwright round-trip e2e specs (flag-off 5-min + flag-on 1-min) validating save → reopen correctness.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eform-client/src/app/plugins/modules/time-planning-pn/models/plannings/planning-pr-day.model.ts | Adds pause override + specified + clear fields to the planning day model. |
| eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/workday-entity/workday-entity-dialog.component.ts | Implements override display precedence, baseline tracking, save-time change detection, and reset-to-recorded behavior. |
| eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/workday-entity/workday-entity-dialog.component.spec.ts | Adds unit tests for override/specification save semantics and display precedence. |
| eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/workday-entity/workday-entity-dialog.component.html | Adds a per-shift “reset pause to recorded” icon button. |
| eform-client/playwright/e2e/plugins/time-planning-pn/l1m/dashboard-edit-pause-override.spec.ts | New e2e: 1-minute-interval site pause override round-trip (save + reopen exact minutes). |
| eform-client/playwright/e2e/plugins/time-planning-pn/b/dashboard-edit-pause-override.spec.ts | New e2e: 5-minute-grid site pause override + netto round-trip after reopen. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+225
to
+232
| // Capture the displayed pause baseline (in minutes) per shift for save-time | ||
| // change detection, and reset the per-shift clear flags. | ||
| [pause1Exact, pause2Exact, pause3Exact, pause4Exact, pause5Exact] | ||
| .forEach((hhmm, idx) => { | ||
| const shift = idx + 1; | ||
| this.loadedPauseMinutes[shift] = this.toRawMinutes(hhmm); | ||
| this.pauseOverrideCleared[shift] = false; | ||
| }); |
Comment on lines
+1588
to
+1594
| private applyPauseOverrideForShift(shift: number, pauseHhmm: string | null | undefined): void { | ||
| const m = this.data.planningPrDayModels; | ||
| const specifiedKey = `pause${shift}OverrideMinutesSpecified`; | ||
| const overrideKey = `pause${shift}OverrideMinutes`; | ||
|
|
||
| const currentMinutes = this.toRawMinutes(pauseHhmm); | ||
|
|
Comment on lines
+1620
to
+1632
| resetPauseToRecorded(shift: number): void { | ||
| const recordedMinutes = this.useOneMinuteIntervals | ||
| ? this.computeExactPauseMinutes(shift) | ||
| : (this.computeFiveMinutePauseMinutes(shift) ?? 0); | ||
| const hhmm = this.convertMinutesToTime(recordedMinutes); | ||
| this.pauseOverrideCleared[shift] = true; | ||
| // Store the picker's resulting raw minutes (convertMinutesToTime(0) → null → | ||
| // toRawMinutes → null) so the "still showing the recorded value" guard in | ||
| // applyPauseOverrideForShift compares like-for-like. | ||
| this.pauseOverrideClearedMinutes[shift] = this.toRawMinutes(hhmm); | ||
| this.workdayForm.get(`actual.shift${shift}.pause`)?.setValue(hhmm); | ||
| this.calculatePlanHours(); | ||
| } |
Comment on lines
+229
to
+237
| <button | ||
| mat-icon-button | ||
| type="button" | ||
| color="primary" | ||
| [matTooltip]="'Reset pause to recorded' | translate" | ||
| [attr.data-testid]="'resetPauseToRecorded' + shiftId" | ||
| (click)="resetPauseToRecorded(+shiftId)"> | ||
| <mat-icon>restore</mat-icon> | ||
| </button> |
Comment on lines
+17
to
+35
| const formatDate = (date: Date): string => { | ||
| const day = date.getDate(); | ||
| const month = date.getMonth() + 1; | ||
| const year = date.getFullYear(); | ||
| return `${day}.${month}.${year}`; | ||
| }; | ||
|
|
||
| const getMonday = (baseDate: Date): Date => { | ||
| const dayOfWeek = baseDate.getDay(); | ||
| const diffToMonday = dayOfWeek === 0 ? -6 : 1 - dayOfWeek; | ||
| const monday = new Date(baseDate); | ||
| monday.setDate(baseDate.getDate() + diffToMonday); | ||
| return monday; | ||
| }; | ||
|
|
||
| const today = new Date(); | ||
| const lastWeekBase = new Date(today); | ||
| lastWeekBase.setDate(today.getDate() - 7); | ||
| const lastWeekMonday = getMonday(lastWeekBase); |
… driver Test-only (production code unchanged): - workday-entity-dialog.component.spec.ts: the first pause test shared the module-level mockData (mutated by earlier describes), poisoning the loaded pause baseline so change-detection saw "unchanged". Reset a fresh pause baseline (pause1Id=0, no override, sub-slots null, past date) before ngOnInit. - b/dashboard-edit-pause-override.spec.ts: replace the brittle rotateZ minute selector (overlapped → timeout) with the coordinate-based .clock-face driver from the passing l1m spec, so the flag-off edit→save→reopen round-trip runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The isolation reset added const m at the top of the test; the original declaration before the assertions was left in, causing a duplicate block-scoped variable compile error that failed the whole angular-unit-test suite. Reuse the single declaration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…test The model-field reset didn't reliably produce a 0 loaded baseline in jest, so the change-detection saw 'unchanged' and Specified stayed false. Set the private loadedPauseMinutes baseline directly after ngOnInit so editing the field to 45 is an unambiguous change (tests applyPauseOverrideForShift directly). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The test routed through onUpdateWorkDayEntity, whose a1.pause (form-group value) came back undefined under jsdom, so currentMinutes was null and the override wasn't set. Call the private applyPauseOverrideForShift(1,'00:45') directly with a forced zero baseline — tests the change-detection unit deterministically. The form→value plumbing is covered by the passing e2e round-trip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pendent)
Root cause of the recurring angular-unit-test failures: shared module-level
mockData passed by reference to MAT_DIALOG_DATA, never deep-reset between tests,
so override fields leaked across the describe block (and onUpdateWorkDayEntity's
form value comes back undefined under jsdom). Add a describe-local beforeEach
that resets pause{N}OverrideMinutes/Specified/clearPauseOverrides, and drive
applyPauseOverrideForShift / resetPauseToRecorded directly with explicit
baselines. Same 4 behaviors, now deterministic and order-independent.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Phase 3 of the pause-override feature (base v10.0.53 + plugin #1629 already on stable). Wires the admin workday-entity dialog to actually edit a shift's total pause again — via the explicit override fields — without destroying the worker's recorded pause start/stops.
What
pause{N}OverrideMinutes/pause{N}OverrideMinutesSpecified/clearPauseOverrides(match the C# DTO)....Specified=true); unchanged →...Specified=false(no spurious lock). Exact HH:mm minutes for both 1-min and 5-min sites.Tests
Internal code-review + simplifier gates run on the diff (clean). eform-client has no local Angular toolchain (plugin split) — specs run in CI.
🤖 Generated with Claude Code