[IP-130]: Report Builder — bands + bricks (IP-130, Phases 1-4 + Browsershot driver)#603
Closed
nielsdrost7 wants to merge 6 commits into
Closed
[IP-130]: Report Builder — bands + bricks (IP-130, Phases 1-4 + Browsershot driver)#603nielsdrost7 wants to merge 6 commits into
nielsdrost7 wants to merge 6 commits into
Conversation
Pure-file report template storage — a template is a folder with manifest.json + bands.json on the report_templates disk, no database tables. Ships default invoice/quote templates in resources/ and a reports:sync-system command that copies them into system storage. Harvests the 17 report bricks from feature/145-report-builder into Modules\Core\Mason (per module convention), rebased onto a ReportBrick base class that adds band placement rules and config-schema introspection. Brick signatures updated for the current awcodes/mason API (nullable toHtml data, BrickAction-managed insert flow). Adds PageBreak and Spacer utility bricks for #95. Security per PRD: slugs restricted to [a-z0-9-], company paths derived from tenant context only, bands validated on load (unknown bricks skipped, widths enum-checked, configs filtered per brick schema). Refs #130 #521 #528 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Phase 2) Custom Filament pages (no Eloquent resource): a template list page and a builder page with five stacked Mason canvases, one per ReportBand, each offering only the bricks allowed in that band. Shared base classes in Modules\Core\Filament\Pages\Reports; the admin panel edits system templates, the company panel shows system templates read-only and manages tenant-scoped clones. Actions: Clone (into the panel's own scope), Rename, Delete (shipped defaults protected), Preview, and an explicit "move to band" action that validates allowedBands() — cross-canvas dragging is deliberately not supported. MasonDocumentConverter round-trips bands.json entries to mason editor state, carrying block width in a reserved config key. Also fixes AbstractCompanyPanelTestCase::testLivewire, which imported the mason schema component instead of the Livewire test facade and chained withSession onto the wrong object; imports mason plugin CSS into all five panel themes. Refs #130 #519 #523 #525 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ReportRenderer turns manifest + bands + entity data into the HTML document for the PDF driver: bands render in document order, block widths become percentage columns, a band with keep_together in the manifest's band_options gets page-break-inside: avoid, and the PageBreak/Spacer bricks provide manual pagination control (#95). ReportDataMapper builds the data arrays the brick views consume from Invoice/Quote models (company, client, meta, items, totals, terms, summary, footer). PdfGenerationService resolves the template chain — document slug, then company default (existing invoices.template and companies.invoice_template/quote_template columns), then the system default — and renders via the domPDF driver. The stubbed "download pdf" table actions on invoices and quotes now stream a real PDF. Fixes two latent bugs the pipeline exposed: the domPDF driver had no Dompdf imports (fatal on first use) and remote fetching enabled (violates the security requirement — now off, images must be local); Company::communications() used the wrong morph name (communicable vs the table's communicationable). Golden-file HTML snapshots for the default invoice and quote templates pin rendering output; PDF byte tests run the real dompdf engine. Refs #130 #95 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a nullable "PDF Template" select to the invoice and quote forms, listing the disk templates for that document type (system defaults plus the current company's clones, clones shadowing same-slug system templates). The selection persists in the existing invoices.template / quotes.template columns; clearing it falls back to the company default (companies.invoice_template / quote_template) and then the shipped system default — no schema changes were needed, the columns already existed. Resolution-chain behavior is covered in PdfGenerationServiceTest; this adds the picker-level scenarios (options per type, clone shadowing, persistence, clearing). Closes #411; Refs #130 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Visual smoke-testing the generated PDF showed dompdf ignores float clearing, so a full-width brick after two half-width bricks rendered on top of them. Bands now chunk consecutive bricks into 12-grid rows rendered as tables, which dompdf lays out correctly. Page-break bricks are emitted at block level between row tables because dompdf also ignores page-break CSS inside table cells. Golden fixtures regenerated for the new markup. Refs #130 #95 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a Browsershot driver behind the existing PDFFactory abstraction for installs that have Node + Chromium: select it with IP_PDF_DRIVER=Browsershot, point IP_BROWSERSHOT_CHROME_PATH at a Chromium binary (node/npm/no-sandbox overrides available). dompdf stays the zero-dependency default; wkhtmltopdf/Snappy was rejected because its upstream is archived with unpatched CVEs, and mPDF (the v1 engine) is a CSS sidegrade. Verified end-to-end against a real Chromium: the same invoice HTML renders the identical band layout on both engines — the table-row markup from the dompdf fix is engine-independent by design. Also pins dompdf/dompdf as a direct composer requirement: the merged driver had been relying on a transitive install, which this change's composer update silently removed — a latent packaging bug. And fixes the company-header logo guard (isset → !empty) so documents without a logo don't render a broken image icon on Chromium. Refs #130 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 13, 2026
nielsdrost7
changed the title
Collaborator
Author
|
Recreated as #608 with the head branch on the fork (underdogg-forks) per the repo workflow — development on the fork, PRs land here. Same commits. |
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
1 participant
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.
Rebased onto #602 — merge that first; this PR contains only the Report Builder commits.
Implements the Report Builder (bands + bricks) per the v2 spec in #130 (epic #506, Track A) — Phases 1–4 in five commits, one per phase plus a rendering fix. Supersedes the stalled #532 (assets harvested, not merged).
What's in here
bcbbafbReportTemplateStorage(manifest.json + bands.json, zero DB tables), shipped default invoice/quote templates, idempotentreports:sync-system, 17 bricks harvested intoModules\Core\Mason+ new PageBreak/Spacer bricks0f6aa7b83abbe1ReportRenderer+ReportDataMapper+ realPdfGenerationService; wired the stubbed "download pdf" actions; golden-file HTML snapshots + real dompdf byte tests84d061ctemplatecolumns; resolution chain document → company default → system defaultacdb249297c649PDFFactory(IP_PDF_DRIVER=Browsershot+IP_BROWSERSHOT_CHROME_PATH); pinsdompdf/dompdfas a direct dependency (was only transitive — latent packaging bug); logo guard fix. Verified against a real Chromium: identical band layout on both enginesSecurity (hard requirements from the PRD)
No user-authored PHP/Blade anywhere; slugs restricted to
[a-z0-9-](path traversal impossible); company storage paths derived from tenant context only; bands.json validated on load (unknown bricks skipped, widths enum-checked, configs filtered against each brick's own schema); dompdf remote fetching disabled (was enabled — fixed here); all brick views escape output.Latent bugs fixed en route
domPDFdriver had noDompdfimports (fatal on first use) andsetIsRemoteEnabled(true)Company::communications()used the wrong morph name (communicablevs table'scommunicationable)AbstractCompanyPanelTestCase::testLivewire()imported a mason schema component instead of the Livewire test facadeTesting
Full suite green: 400 tests, 1,339 assertions, 0 failures (develop baseline: 307 tests, also green). New coverage: storage round-trip/isolation/traversal, brick registry + band rules, builder pages in both panels (load/save/move/clone/delete/read-only), renderer (toggles, keep-together, page breaks, widths), golden snapshots for
invoice/default+quote/default, PDF bytes via real dompdf, and the #411 resolution/picker scenarios. Plus an end-to-end smoke: app boot → migrate → sync → factory invoice → visually verified PDF.Follow-up issues (tracked, not resolved here)
All hanging items have tracked issues on the main repo (sub-issues of #130); plain references only — this fork PR must not auto-close them:
group_by, valid item keys, per-group totals for Group Footer bricks); renderer currently renders every band exactly onceband_options.<band>.keep_togetherbut only hand-edited manifests can set it (remaining builder-side slice of [Core]: PDF templates — page-break support #95)Independent tracks, unchanged: tabular reports (#145, Track B) and email variables (#363, Track C).