(feat) O3-5675: add facility header, patient age, page numbers and empty-state styling and fix repeat table column headers across page breaks - #33
Conversation
…pty-state styling (fix): repeat table column headers across page breaks
Before:
After:
|
| Assertions.assertTrue(pageCount > 1, | ||
| "test data must force a page break, but the table fit on one page"); | ||
|
|
||
| for (int page = 1; page <= pageCount; page++) { |
There was a problem hiding this comment.
Since these cases render real multi-page PDFs, this is also the natural place to lock in the footer's "Page N of M" (and the facility name). Those print on every page too, and they're currently the one behavior this PR adds that nothing tests.
The failure mode is quiet: rename the visit-summary-end anchor, or drop the fo:page-number-citation, and every page's total silently breaks with no test to catch it.
The fixture here doesn't emit a <footer> element, so covering it means adding a <footer lbl-page="Page" lbl-of="of" .../> to visitSummaryXml(...) and asserting the "Page N of M" line shows up on each page, alongside the column-header checks you already do.
Non-blocking, and the mechanism itself looks right (the regression run here is green). I'm just flagging the gap while the multi-page harness is sitting right here.
| PatientDocumentsConstants.NO_DATA_RECORDED_PLACEHOLDER, Context.getLocale()); | ||
| } | ||
| catch (Exception e) { | ||
| label = PatientDocumentsConstants.NO_DATA_RECORDED_PLACEHOLDER; |
There was a problem hiding this comment.
Don't we need to log a warning here?
| return null; | ||
| } | ||
| return "data:" + mediaType + ";base64," + Base64.getEncoder().encodeToString(imageBytes); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Don't we need to log here?
| Locale locale = Context.getLocale(); | ||
| try { | ||
| return new SimpleDateFormat(pattern, locale).format(date); |
There was a problem hiding this comment.
This could cause a problem on modern Java versions. After Java 9, the Arabic locale's default numbering system is Arabic-Indic. So with the default yyyy-MM-dd pattern, an ar-locale user gets:
en :2026-06-24
arabic :٢٠٢٦-٠٦-٢٤
I'm not sure if this is desirable on a clinical document or not. Feels like we might need to keep the localized text but force Latin (ASCII) digits.
@ibacher, @VeronicaMuthee any suggestions?
There was a problem hiding this comment.
Just looked at core, and core does the same thing. So this feels like a decision we don't need to make for this PR; it's more of a whole-product consideration.
If someone needs a locale with a different language and a different numbering system, we'd need to update core. That would give users the choice to customize it themselves.
There was a problem hiding this comment.
Thanks for catching this and for checking core too, I learned something :D
Agreed it's better handled at the core level than here, and keeping this PR consistent with core makes sense. Good to have it documented in the thread if an ar-locale deployment ever needs a different numbering system.
|
|
cc: @wikumChamith |
|
@claude review |
3 participants
Summary
Polish pass on the visit summary PDF layout, plus a fix for column headers not repeating across page breaks.
What this does
The header is now a horizontal band: configured facility logo, name and contact on the left, document title and visit date on the right. The logo resolves through the same app-data-dir mechanism the patient ID sticker uses, and renders nothing rather than failing the PDF when unconfigured or unreadable.
The patient block is two compact rows of four fields. Age is derived at the visit start date rather than today, so it stays consistent with the vitals, diagnoses and medications on the same page, and reprinting an old visit does not change what it says. The date of birth prints beside it either way.
The footer carries the facility name and
Page N of Mon every page. Section spacing was tightened without reducing body font sizes.Empty sections render a grey dashed block, visually distinct from the red section-error banner, so a clinician can tell "nothing was recorded" from "this could not be retrieved" on paper alone.
The bug fix: column header rows lived in
fo:table-body, which XSL-FO treats as ordinary data, so a table spanning a page break opened the next page with unlabeled columns. Page 2 of a lab table showed values and flags with no indication of which column was the result and which was the reference range. Moved tofo:table-header, which repeats by default. Affects diagnoses, conditions, lab results, allergies and medications.Supporting changes: date and datetime patterns are configurable and formatted in the user's locale;
Helper.getImageAsDataUridetects PNG, JPEG and GIF from signature bytes instead of labelling every fileimage/png.Testing
api 197, omod 13, 0 failures.
The table-header fix has a regression test that renders each affected section through FOP with enough rows to force a page break, then asserts every column header appears on every page. It also asserts the table actually spanned more than one page, so a future change making the data fit on one page cannot turn it into a silent pass. Verified by reverting one header back into
fo:table-bodyand confirming only that test failed.Adding a test class that sets date-format properties exposed a test-isolation issue:
AdministrationServicecaches global-property values in memory, so a value saved by one test class survives its rolled-back transaction and leaks into later classes. Test classes that set properties now clean up after themselves.Related
https://openmrs.atlassian.net/browse/O3-5675
Notes
report.visitSummary.logourl,report.visitSummary.facility.phoneAttributeType,report.visitSummary.dateFormat,report.visitSummary.datetimeFormat. Naming followsreport.patientIdSticker.logourlandreport.visitSummary.visitNotes.clinicianEncounterRole.