Feature ETP-4908: Inject document QR as data field in report-server render - #129
Open
valenvivaldi wants to merge 4 commits into
Open
Feature ETP-4908: Inject document QR as data field in report-server render#129valenvivaldi wants to merge 4 commits into
valenvivaldi wants to merge 4 commits into
Conversation
Document print templates previously relied on an async qrCode Handlebars
helper, which fails on the local HTML render path (helpers whitelist excludes
it since ETP-4083, and Handlebars.compile is synchronous). The QR is now
precomputed once in the report server before the format branch and injected
as header.qrDataUrl into both the local HTML render and the jsreport
PDF/XLSX payload.
- report-html-helpers.js: add pure buildDocumentQrText(header) (exact port of
the per-report qrCode text logic) and async computeDocumentQrDataUrl(header,
{ qrcode }) with injectable qrcode module (the report-server Docker image
installs node_modules only under tools/report-server, unreachable from the
templates/ module path) and lazy dynamic import fallback.
- server.js: injectDocumentQr() computes header.qrDataUrl before rendering;
QR failure degrades to a report without QR instead of a 500.
- tools/report-server/package.json: add qrcode ^1.5.4.
- Tests: cli/test/report-qr.test.js (text building, data URL, injected module,
no-header/empty cases) and a regression guard in
generate-report-template.test.js asserting the generator never emits a
qrCode helper call.
Contributor
Copilot PR ReviewOutcome: Comment only Warnings
|
The 8 per-report qrCode helpers were not identical: invoices used dateinvoiced/grandtotal, orders and quotations dateordered, shipments and receipts movementdate (no amount/currency), payment-in paymentdate/amount. buildDocumentQrText only checked dateinvoiced/grandtotal, so 7 of 8 document types lost the date (and payment-in its amount) in the QR. Fallback chains, verified against the original artifacts/print-*/helpers.js at the parent commit of the functional repo: - date: dateinvoiced || dateordered || movementdate || paymentdate - amount: grandtotal || amount Segment order and prefixes are unchanged (identical across all 8 originals). Tests cover each variant plus precedence when multiple candidates exist.
…module
The report server's PDF/XLSX branch sent the raw artifact helpers.js as
jsreport's helpers string. Post-ETP-4083 that file no longer defines the
formatting helpers, so any template using {{#ifCond}}/{{formatDate}} failed
in jsreport with 400 missing helper "ifCond" (pre-existing, reproduced on
epic too).
- templates/reports/helpers/report-html-helpers.js: synced byte-identical
with the functional repo's copy — brings JSREPORT_HELPER_SOURCES (the
minification-safe verbatim source-text map, ETP-4728) and
buildJsreportHelpersString(), plus the ETP-4314 es-ES/useGrouping currency
behavior, killing the drift between the two copies. QR functions unchanged.
- server.js: the jsreport payload now sends
buildJsreportHelpersString(helpersCode, undefined, separators) — canonical
helper sources plus only the report-specific extras — with instance
separators fetched (and cached) from /sws/neo/currency-format, mirroring
the Vite dev plugin's composition.
- Tests: ported the functional repo's jsreport helpers builder parity suite
(46 tests, includes new Function compilation of the emitted string) into
cli/test; updated server-render.test.js currency assertions to the
canonical es-ES grouped format.
Verified live against the etendo-jsreport container: print-sales-invoice
format=pdf now returns HTTP 200 with %PDF magic and the QR image embedded;
format=html unchanged.
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
Contributor
|
📦 Preview packages published — dist-tag To exercise the published-package path (no make bump-core-version VERSION=0.3.34-preview.feature-ETP-4908.20260814144445.3d351b1Re-posted on each push to this branch; supersedes older previews. |
Agustin-Calderon
approved these changes
Aug 14, 2026
ivanrobledofutit
approved these changes
Aug 14, 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
3 participants
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.
Summary
qrCode, but report templates still call{{qrCode header}}directly. This produces "Missing helper" on the server-side HTML render (pre-ETP-4083 builds instead threw "require is not defined").header.qrDataUrlviacomputeDocumentQrDataUrl/buildDocumentQrTextinreport-html-helpers.js, injected inserver.jsbefore the format branch so both the HTML and jsreport PDF render paths receive it. Added theqrcodedependency. QR failure degrades gracefully (no QR rendered, no request failure).cli/test/report-qr.test.jsplus a generator regression guard. Full suite: 2957 green.Jira: ETP-4908
Coordination note: the
schema_forgecounterpart PR (template edits{{qrCode header}}→{{header.qrDataUrl}}) is coming separately. Both PRs must deploy together — this one alone leaves templates calling the now-removedqrCodehelper.🤖 Generated with Claude Code