feat(compliance): Detaljer view with week grouping, server-side pagination and delete (#1165) - #1176
Merged
Merged
Conversation
…#1165) Add the Detaljer view mode to the standalone Compliance page: the week-grouped chronological task log, server-side paged through the shell, with row click into the completion modal and an anchored delete confirmation. Week blocks open on a change of week while walking the page in server order -- the view never re-sorts. Month, year and week number all derive from the occurrence's Monday, so a week straddling a month or year boundary carries one header. The week key is that Monday's ISO date rather than the prototype's year-plus-week-number, which collides: Monday 1 Jan 2024 and Monday 30 Dec 2024 are both ISO week 1 with getFullYear() 2024. Dates are parsed into local midnight from their parts. new Date('2026-08-11') is UTC midnight and renders as the previous day west of Greenwich, which would shift the date cell, the week header and the overdue test together. Parts that would roll over -- month 13, 31 February -- are rejected rather than silently landing in another month. The view owns only its query. It subscribes to the shell's fetchRequested$, reads requestModel at fetch time, and reports back through setTotalCount; the shell owns the pagination chrome and the loading flag. Every path clears loading: success, transport error, switchMap cancellation and teardown. Delete calls the existing compliances/delete/{id} endpoint and changes nothing about it -- that endpoint is shared with the standalone /compliances table, which task-tracker depends on. The confirmation is an anchored CDK overlay with no backdrop, so clicking another row's delete button moves it. That gesture needs care: CDK's outside-click dispatcher listens on body in the CAPTURE phase, so a naive same-button toggle would close and immediately reopen. Importing CalendarModule for the completion modal used to flatten its MAT_DATE_FORMATS override into this module's injector, which stripped the YEAR from the filter bar's period inputs -- on a report that is explicitly retrospective. The calendar's format is now a shared const, the modal carries it as a component provider so it keeps it wherever it is opened from, and this module re-provides the eForm default for its own components. The same leak still affects task-list; filed separately as #1175. One new i18n key. All 25 non-English locales gained exactly one line, none lost any, and the escaped-quote lines that the propagation script truncates are intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LcMeVFxqWqQzvqHjAa3Xkc
There was a problem hiding this comment.
🟡 Changes recommended
There is at least one confirmed correctness issue in time formatting (possible HH:60) and a stability issue in date-dependent tests that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements the Compliance “Detaljer” view mode within the standalone compliance-report page, including week-grouped rendering, server-driven paging integration, row click into the existing completion modal, and an anchored delete confirmation—while also fixing a MAT_DATE_FORMATS provider leak introduced by importing the calendar module.
Changes:
- Added pure, unit-tested helper functions for task-date parsing, ISO week grouping, and row display formatting used by Detaljer.
- Added
ComplianceDetailsViewComponentand wired it into the compliance-report page shell’s view-mode switch. - Scoped calendar
MAT_DATE_FORMATScorrectly by extracting the calendar formats and re-providing defaults in the compliance-report module; added i18n keys for the new UI strings.
File summaries
| File | Description |
|---|---|
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/helpers/index.ts | Exports the new week-grouping/date helper API. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/helpers/compliance-week-grouping.ts | Pure functions for parsing task dates, ISO week grouping, and row label formatting. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/helpers/compliance-week-grouping.spec.ts | Jest coverage for week grouping, date parsing, overdue rule, and formatting helpers. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/index.ts | Exposes the new Detaljer view component from the components barrel. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-report-page/compliance-report-page.component.html | Renders <app-compliance-details-view> for the details view mode; updates pagination wording comment. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-details-view/compliance-details-view.component.ts | Implements Detaljer data fetch, week-group rendering, row click completion modal, and delete overlay behavior. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-details-view/compliance-details-view.component.html | Template for week headers, rows, completion indicator, and anchored delete confirm popover. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/components/compliance-details-view/compliance-details-view.component.scss | Theme-token-based styling for Detaljer rows, chips, and overlay popover. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/compliance-report/compliance-report.module.ts | Declares Detaljer component; imports Overlay/Portal; re-provides MAT_DATE_FORMATS to prevent leaks. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/modals/calendar-complete-event-modal/calendar-complete-event-modal.component.ts | Adds component-level MAT_DATE_FORMATS provider so the modal keeps its calendar format when opened cross-module. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/calendar.module.ts | Replaces inline MAT_DATE_FORMATS object with shared CALENDAR_MAT_DATE_FORMATS. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/modules/calendar/calendar-date-formats.ts | New shared constant defining the calendar’s MAT_DATE_FORMATS. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/enUS.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/da.ts | Adds Detaljer-related translation keys (Danish). |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/bgBG.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/csCZ.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/deDE.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/elGR.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/esES.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/etET.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/fiFI.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/frFR.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/hrHR.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/huHU.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/isIS.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/itIT.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ltLT.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/lvLV.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/nlNL.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/noNO.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/plPL.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ptBR.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ptPT.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/roRO.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/skSK.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/slSL.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/svSE.ts | Adds Detaljer-related translation keys. |
| eform-client/src/app/plugins/modules/backend-configuration-pn/i18n/ukUA.ts | Adds Detaljer-related translation keys. |
Review details
- Files reviewed: 38/38 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+60
to
+73
| // Detaljer (#1165) opens the calendar's completion modal | ||
| // (CalendarCompleteEventModalComponent) rather than re-declaring the whole | ||
| // prepare-complete pipeline. The import is for the COMPONENT only — its | ||
| // providers are an unwanted side effect: an imported NgModule's providers | ||
| // merge into the importer's own injector, so CalendarModule's | ||
| // MAT_DATE_FORMATS override would otherwise reach every component declared | ||
| // here, including the filter bar's date-range input (see the re-provide | ||
| // below). | ||
| // | ||
| // Listed LAST on purpose. CalendarModule pulls in CalendarRouting, whose | ||
| // `path: ''` route would otherwise compete with this module's own; the | ||
| // sibling modules that already import CalendarModule (task-list, | ||
| // calendar-task-list) order it the same way. | ||
| CalendarModule, |
Comment on lines
+229
to
+253
| // Pins the CURRENT behaviour, which is a deliberate trade: an Invalid Date | ||
| // would render the literal string "Invalid Date" in every cell it reaches, | ||
| // so the helper falls back to local midnight TODAY. The cost is that a | ||
| // corrupt row is silently filed under the CURRENT week rather than standing | ||
| // out — worth flagging if the wire format ever becomes unreliable, but it is | ||
| // what the view does today and is asserted, not changed, here. | ||
| const cases = ['', ' ', 'not-a-date', '2026-13', 'T08:00:00']; | ||
|
|
||
| it.each(cases)('falls back to local midnight today for %p', (input) => { | ||
| const today = new Date(); | ||
| today.setHours(0, 0, 0, 0); | ||
| expect(parseTaskDate(input).getTime()).toBe(today.getTime()); | ||
| }); | ||
|
|
||
| it('files a corrupt row under the current week rather than throwing', () => { | ||
| const groups = groupRowsByWeek([row('not-a-date')], LOCALE, WEEK); | ||
| expect(groups).toHaveLength(1); | ||
| expect(groups[0].key).toBe(complianceWeekKey(mondayOf(new Date()))); | ||
| }); | ||
|
|
||
| it('still parses a full ISO timestamp by its date half', () => { | ||
| expect(parseTaskDate('2026-08-11T22:30:00Z').getTime()) | ||
| .toBe(new Date(2026, 7, 11).getTime()); | ||
| }); | ||
| }); |
Comment on lines
+161
to
+166
| const toHm = (hours: number) => { | ||
| const hh = Math.floor(hours); | ||
| const mm = Math.round((hours - hh) * 60); | ||
| return `${hh.toString().padStart(2, '0')}:${mm.toString().padStart(2, '0')}`; | ||
| }; | ||
| return `${toHm(row.startHour)} - ${toHm(row.startHour + row.duration)}`; |
15 tasks
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.
Closes #1165. Part of #1160. Depends on #1161 and #1163 (both merged).
The Detaljer view mode: the week-grouped chronological task log, server-side paged, with row click into the completion modal and an anchored delete confirmation.
Week grouping and dates
Blocks open on a change of week while walking the page in server order — the view never re-sorts. Month, year and week number all derive from the occurrence's Monday, so a week straddling a month or year boundary carries one header (31 Aug–6 Sep 2026 is all
August 2026 Uge 36).Two deliberate deviations from the prototype, both commented:
getFullYear() + '-W' + weekNum. That form collides: Monday 1 Jan 2024 and Monday 30 Dec 2024 are both ISO week 1 withgetFullYear() === 2024. The rule the issue protects — never the row's year — is preserved.new Date('2026-08-11')is UTC midnight and renders as the previous day west of Greenwich, which would shift the date cell, the week header and the overdue test together. Parts that would roll over (2026-13-05,2026-02-31) are rejected via a round-trip check rather than silently landing in another month.The shell contract
The view owns only its query. It subscribes to
fetchRequested$, readsrequestModelat fetch time, and reports back viasetTotalCount; the shell owns the pagination chrome and theloadingflag. Every path clearsloading— success, transport error,switchMapcancellation, and teardown viafinalizeupstream oftakeUntil.Delete
Calls the existing
DELETE compliances/delete/{id}and changes nothing about it — that endpoint is shared with the standalone/compliancestable, which task-tracker depends on.git diffonbackend-configuration-pn-compliances.service.ts, the compliance module and thecompliances/case/…route is empty; this PR adds a caller, not a change.The confirmation is an anchored CDK overlay with no backdrop, so clicking another row's delete button moves it rather than being swallowed. That gesture needs care: CDK's
OverlayOutsideClickDispatcherlistens onbodyin the capture phase, so it fires before the button's own(click)and a naive same-button toggle would close-then-immediately-reopen.The
MAT_DATE_FORMATSleak — a fix to already-merged codeImporting
CalendarModulefor the completion modal flattened itsMAT_DATE_FORMATSoverride into this module's injector, which stripped the year from #1163's filter-bar period inputs (tirsdag 11. august) — on a report that is explicitly retrospective and whose custom range is the one control that can cross a year boundary.Fixed three-sided: the calendar's format is extracted to a shared const, the completion modal carries it as a
@Componentprovider so it keeps its format wherever it is opened from, andComplianceReportModulere-provides the eForm default for its own components. Calendar pages are unaffected — verified there is no datepicker on a calendar page outside the two modals, and both still resolve the calendar format.The same leak still affects
task-listandcalendar-task-list("Kopiér til dato" renders with no year today). Pre-existing and outside this issue's scope, so it is filed as #1175 rather than folded in silently — it changes visible behaviour in modules this PR has no business touching.Reviewed twice — 11 defects found
Gate 1 (8). The
MAT_DATE_FORMATSleak above. The completion highlight was gated on nothing, so under the default Ikke udførte opgaver filter it fired only on cancel and never on save — a saved row leaves theopenset, so the scroll target no longer exists. KeyboardEnteron the delete button opened the eForm and the popover.suppressReopenForleaked on right/middle click, eating one subsequent left click.aria-modal="true"on a non-modal popover with no focus move, trap or Escape. A child-sidesetLoading(false)on destroy that would clobber an incoming sibling once #1164/#1167 land. A "never re-sorts" test that fed ascending input and so could not fail. Overstated contrast figures in the SCSS header.Gate 2 (3). A comment stating a factually wrong premise about
NgSwitchCaseordering (the removal was right, the reason was not). A failed first fetch left a completely blank card — spinner gone, placeholder gone, "no tasks match" suppressed — now a distinct "could not load" line, while a failed re-fetch still leaves the existing rows standing rather than blaming the filters for a transport error. And the date-part rollover above.i18n
One new key. All 25 non-English locales +1 line, zero removed lines, and the escaped-quote lines the propagation script is known to truncate are intact (frFR 7, itIT 6, nlNL 1). The script was deliberately not run wholesale — the locales are ~100 keys behind
enUS, so a run would have machine-backfilled thousands of unrelated keys and stripped every comment. Danish hand-corrected.Previous/Next/ofare left to the core bundle rather than shadowed.Known and deliberate
complianceId. In practice completion changes neithertaskDatenorstartHourand the view never re-sorts, so a row that survives the filter keeps its page.Vis alle5000-row cap is not surfaced. The response carries onlytotal, with no truncation flag, so a user with 6000 rows sees "Viser alle 6000" above 5000 rows. Needs a flag on the response DTO — raising on feat(compliance): server-side CSV, Excel and PDF export for all three views #1169, which consumes the same unpaged path.isTaskOverdueis exported but renders nothing. Neither the issue's row anatomy nor the prototype's Detaljer rows define an overdue affordance; the rule lives with this page's date maths and feat(compliance): Oversigt view with per-property compliance rates and drill-down #1164's Overskredet column is its consumer..is-donerow's stacked surfaces. Worth ten minutes undertheme-eformdark before release.element-picturesniffscaseIdfrom the router URL, so this page is a new consumer of a pre-existing bug, exactly as the calendar is.<button>insiderole="button") is guarded, not restructured — the keyboard behaviour is fixed, but untangling the DOM means re-cutting the row's grid and hover rules.Verification
ng buildsucceeds and emits thecompliance-reportlazy chunk; Playwright type-check clean; the CI button-convention gate reports no violations. Tests were not run locally; CI runs them.🤖 Generated with Claude Code
https://claude.ai/code/session_01LcMeVFxqWqQzvqHjAa3Xkc