feat(compliance-report): server-side CSV, Excel and PDF export for all three views (#1169) - #1178
Merged
Merged
Conversation
Add POST api/backend-configuration-pn/compliance-report/export, rendering any of the three views as CSV, XLSX or PDF from the same filter set the screen is showing. Numbers are never recomputed. Oversigt's percentages and its weighted totals row come verbatim from the aggregation endpoint, which is exactly why #1162 put that maths server-side: a report of completed work runs to roughly 135 A4 sheets, and an export cannot render a number that only exists in a browser. The three views share one intermediate document. It keeps the existing report model's tag-then-template grouping and drops its positional cell list, which is the root cause of the column-desync bug in the shipped report code: Rapport cells are looked up by the projector's stable column key, so an unanswered column emits the en dash in place rather than shifting every later column. ExcelService.GenerateExcelDashboard and WordService.GenerateWordDashboard could not be reused. Both hard-code a six-column preamble that Oversigt (3 columns) and Detaljer (8) do not have, ReportEformItemModel has a fixed field per column, and the Excel path dereferences a nullable DoneAt unconditionally, which throws on any open task -- and Detaljer is mostly open tasks. Making those optional would have meant editing methods that are live on the existing report endpoint. The MECHANISMS are reused: the same OpenXml helper, styles and theme parts, the same WordProcessor over the same embedded docx shell, the same ImageMagick + S3-or-disk image path, and the same docx-to-soffice route. Images are read as a stream from S3 or local disk, never over HTTP -- which is the whole reason PDF is generated server-side rather than in the browser, since the image endpoint is bearer-only and does no per-case authorisation. CSV neutralises a leading formula character. Every Rapport answer cell is worker-typed free text, and this is the plugin's first user-facing CSV download, so an unguarded cell is a spreadsheet that executes on open. Excel consumes the guard invisibly; LibreOffice shows it, which is the accepted trade. soffice runs with a private user profile per invocation. Two concurrent headless runs sharing the default profile is the classic LibreOffice failure, and this endpoint has no claim gating, so two people pressing Download PDF at once is ordinary traffic. A missing or failing soffice degrades to a localised 400; a client that navigates away mid-download is not reported as a timeout and is not sent to Sentry. The image appendix is opt-in, off by default, PDF-only, capped at four images per case and 200 per document, emitted once per case even when the case carries several tags, and the document states both limits when they bite. 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 are a few confirmed correctness issues in the export implementation (date/time handling, sheet-name sanitization, and status defaulting) that can lead to incorrect or failing exports.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a backend-only export pipeline for the Compliance feature, introducing a single endpoint that can export any of the three compliance views (overview/details/report) as CSV, XLSX, or PDF, using the same filter set and read models as the on-screen data.
Changes:
- Introduces
POST api/backend-configuration-pn/compliance-report/exportand a new export service that orchestrates calling the existing compliance-report read endpoints and rendering the result. - Implements format-agnostic export intermediate (
ComplianceExportDocument) plus CSV/XLSX/Word(PDF via soffice) renderers, including download naming andContent-Disposition. - Adds i18n keys and integration-test coverage for document building, writers, and export-service orchestration.
File summaries
| File | Description |
|---|---|
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/IBackendConfigurationComplianceExportService.cs | Introduces export service interface contract. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/BackendConfigurationComplianceExportService.cs | Orchestrates export: validates request, calls compliance-report service, builds document, renders file, sets MIME/name. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/ComplianceExportDocumentBuilder.cs | Maps the three view models into a common typed-table document model (including optional image appendix for report+pdf). |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/ComplianceExportCsvWriter.cs | Implements semicolon-separated UTF-8 BOM CSV with formula-injection guard. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/ComplianceExportExcelWriter.cs | Writes OpenXML XLSX workbook (one sheet per table) using shared workbook/theme/style helper. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/ComplianceExportWordWriter.cs | Renders DOCX via existing Word template/resources; used as the PDF source document. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/ComplianceExportPdfConverter.cs | Converts DOCX→PDF via soffice with timeout, per-invocation profile, cleanup, and better diagnostics. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Services/BackendConfigurationComplianceExportService/ComplianceExportFileNaming.cs | Builds safe filenames and RFC6266/5987 Content-Disposition values. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Models/ComplianceReport/ComplianceReportExportRequestModel.cs | Adds request contract for combined view+format export with filter set in body. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Models/ComplianceReport/ComplianceExportFileModel.cs | Defines export output model (stream + filename + MIME type). |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Infrastructure/Models/ComplianceReport/ComplianceExportDocument.cs | Defines the shared typed-cell intermediate format used by all renderers. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Controllers/ComplianceReportController.cs | Adds /export endpoint; sets Content-Disposition and returns file response. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/EformBackendConfigurationPlugin.cs | Registers the new export service in DI. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn/Resources/localization.json | Adds export-related localization keys (and expands All). |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn.Integration.Test/ComplianceExportWriterTests.cs | Tests CSV/XLSX/Word rendering boundaries, naming, and PDF converter “missing soffice” behavior. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn.Integration.Test/ComplianceExportServiceTests.cs | Tests export-service validation, request pass-through, MIME/stream behavior, and appendix gate. |
| eFormAPI/Plugins/BackendConfiguration.Pn/BackendConfiguration.Pn.Integration.Test/ComplianceExportDocumentBuilderTests.cs | Tests document-builder mapping rules across overview/details/report, including totals and keyed cells. |
Review details
- Files reviewed: 17/17 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
+235
to
+243
| var cleaned = (sheetName ?? string.Empty) | ||
| .Replace(":", "") | ||
| .Replace("\\", "") | ||
| .Replace("/", "") | ||
| .Replace("?", "") | ||
| .Replace("*", "") | ||
| .Replace("[", "") | ||
| .Replace("]", "") | ||
| .Trim(); |
Comment on lines
+190
to
+197
| public static ComplianceExportCell FromDate(DateTime? value) => | ||
| value.HasValue | ||
| ? new ComplianceExportCell | ||
| { | ||
| Date = value, | ||
| Text = value.Value.ToString("dd.MM.yyyy", System.Globalization.CultureInfo.InvariantCulture) | ||
| } | ||
| : new ComplianceExportCell(); |
Comment on lines
+234
to
+246
| private static ComplianceReportRequestModel BuildReportRequest( | ||
| ComplianceReportExportRequestModel requestModel) => new() | ||
| { | ||
| PropertyId = requestModel.PropertyId, | ||
| BoardIds = requestModel.BoardIds ?? [], | ||
| TagIds = requestModel.TagIds ?? [], | ||
| SiteIds = requestModel.SiteIds ?? [], | ||
| Status = requestModel.Status, | ||
| DateFrom = requestModel.DateFrom, | ||
| DateTo = requestModel.DateTo, | ||
| PageIndex = 0, | ||
| PageSize = 0 | ||
| }; |
This was referenced Sep 4, 2026
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 #1169. Part of #1160. Depends on #1161, #1162 and #1166 (all merged).
POST api/backend-configuration-pn/compliance-report/exportrenders any of the three views as CSV, XLSX or PDF from the same filter set the screen is showing. Backend only — the format select and Download button are #1169 task 10 / the view issues.Numbers are never recomputed
Oversigt's percentages and its weighted totals row come verbatim from
Overview(). That is precisely why #1162 put the maths server-side: a completed-work report runs to ~135 A4 sheets, and an export cannot render a number that only exists in a browser.nullpercentage renders the en dash–, never0.One intermediate, keyed cells
All three views map onto a single export document that keeps the existing report model's tag → template grouping and drops its positional cell list. That positional list is the root cause of the column-desync bug in the shipped report code (headers from a filtered field list, cells from the unfiltered one, so
Audio/ShowPdf/Movieshift every later column). Here Rapport cells are looked up by the projector's stable column key, so an unanswered column emits the dash in place.Why
ExcelService/WordServicewere not reusedThe epic points at
GenerateExcelDashboard/GenerateWordDashboard, and I did not use them. Verified reasons:ExcelService.cs:333-341,WordService.cs:850-855) that Oversigt (3 columns) and Detaljer (8) do not have.ReportEformItemModelhas a fixed field per column.ExcelService.cs:417doesCreateDateCell(dataModel.MicrotingSdkCaseDoneAt!.Value)unconditionally — it throws on any not-yet-completed row, and Detaljer is dominated by open tasks.Making those optional means editing methods live on
GET report/reports/file, which this issue forbids regressing and which has no test coverage to catch a regression.The mechanisms are reused: the same
OpenXMLHelperworkbook/styles/theme parts with the identical rId layout and style indices, the sameWordProcessorover the same two embedded resources (no csproj change), the same ImageMagick + S3-or-disk image embedding, the same docx→sofficeroute.Task 5's second half is deliberately not done. Adding a
"csv"arm toBackendConfigurationReportService's switches andReportController's ContentType switch was premised on the export routing through them. It does not — it is a separate action on a separate controller that sets its own MIME type — so those arms would be dead code plus a second, differently-shaped CSV that nothing calls. No acceptance criterion depends on them.Security and robustness
= + - @ TAB CRis prefixed with'. Excel consumes that invisibly; LibreOffice shows it, so-5 graderdisplays as'-5 graderthere — the accepted trade, stated in the code.sofficeruns with a private user profile per invocation. Two concurrent headless runs sharing the default profile is the classic LibreOffice failure (the second exits 0 having produced nothing, or blocks on the lock), and this endpoint is bare[Authorize]per decision 6, so two people pressing Download PDF at once is ordinary traffic. Also:ArgumentListrather than an interpolated command line, a bounded timeout withKill(entireProcessTree), pipes drained before returning, and the temp directory removed infinally.Stream— which is the whole reason PDF is server-side, sinceget-imageis bearer-only and does no per-case authorisation.The image appendix
Opt-in, off by default, PDF-only, capped at 4 images per case and 200 per document, emitted once per case even when a case carries several tags, and the document states both limits when they bite. The per-tag duplication mattered: nothing streams, so a case with three tags contributed three identical base64 blocks into a
StringBuilderthat is then copied twice more.Reviewed twice — 18 defects found
Gate 1 (11): CSV formula injection; no LibreOffice profile isolation; the export service and controller had zero test coverage (the appendix gate could be flipped
&&→||with every test still green, and nothing pinnedPageSize = 0— the load-bearing line for "exports the full filtered set"); a near-vacuous PDF test whose file summary overclaimed; a test that passed for the wrong reason; a CSV preamble that pushed the header off row 1; abandoned pipe readers on the timeout path; client aborts logged as timeouts and captured to Sentry;GetString("All")rendering the literal key in 13 of 26 locales; and three small items.Gate 2 (7): a named tag whose name cannot be resolved was exported as "Uden tag" — the builder discriminated on name emptiness rather than
TagId.HasValue, which directly undid #1166's deliberate choice to keep such rows in their named group, silently merging two sections in theDelrapportpivot and making the export disagree with the screen; the appendix duplication above;TotalImagescounted post-drop so the document hid truncation it had caused; a stale contract comment; an over-stated "invisible on import" claim; one remaining unobserved-task hole; and two missing Excel sheet-name rules (leading/trailing apostrophe, the reserved nameHistory) that make Excel refuse to open a workbook.Services/BackendConfigurationComplianceReportService/is byte-identical tostable— all 86 existing tests are unaffected and none was edited.Two localization keys change output elsewhere — please look
19 keys added to
Resources/localization.json(append-only, all 26 locales, no duplicates). Two of them are already looked up by existing code that currently renders the raw key:ErrorWhileGeneratingReportFile—BackendConfigurationReportServicereturns the literal key as a user-facing error on a failed docx/pdf report today. Pure fix.Status— six pre-existing call sites (WordService.cs:176,ExcelService.cs:627,BackendConfigurationTaskManagementService.cs:647,BackendConfigurationTaskManagementHelper.cs:146/321/573), two of them column headers in the existing Word and Excel reports. In da/de/nl/sv/no/pl/pt-BR/hr the rendered string is unchanged; in the other 18 locales a report header flips fromStatusto the translation.Both are the correct string in every one of those places, so this reads as a latent-bug fix — but a reviewer looking only at the diff would not see that adding a key to a JSON file changes a Word report, hence flagging it here.
Header wording — a decision, recorded
The Oversigt property column is "Ejendom" and the tag column "Etiketter", not the "Virksomhed"/"Tags" the acceptance criteria quote from the prototype. The plugin's own Danish dictionary already says
Property: 'Ejendom'andTags: 'Etiketter'; following the AC literally would put two Danish words for one entity into one document. Recorded as a comment on #1164 so the on-screen table matches.Known and deliberate
sofficeconversion is never exercised — LibreOffice is not on the CI image. Everything up to handing the docx over is covered, including the missing-LibreOffice degradation. The conversion itself needs a manual pass on a box withsoffice..xlsxpart↔sheet mapping is structurally sound (both loops index the same list) but no test resolvesGetPartById(sheet.Id); five tests do open the produced package, so a corrupt one fails loudly.URL.createObjectURL+a.downloadignoresContent-Dispositionentirely, so the client must read the header off the response or none of the filename work reaches the user.Index's default sort — the request carries no sort field. It matches the screen for anyone who has not clicked a sort header. Two lines to close if wanted.(ColumnsUnavailable)annotation on the first Rapport table is absent in CSV only, a consequence of making line 1 a header row.Verification
dotnet buildon both projects → 0 errors. 60 tests across three new fixtures. Tests were not run locally; CI runs them.🤖 Generated with Claude Code
https://claude.ai/code/session_01LcMeVFxqWqQzvqHjAa3Xkc