feat(#130): report builder — phases 1-4 - #720
Draft
nielsdrost7 wants to merge 92 commits into
Draft
nielsdrost7 wants to merge 92 commits into
nielsdrost7 wants to merge 92 commits into
Conversation
- docker-compose.yml: add MariaDB healthcheck (connection check) - docker-compose.yml: add depends_on with service_healthy for cli - docker-compose.yml: add init volume for test DB setup - AGENTS.md: use cp -n to prevent .env.testing overwrite
…river) (InvoicePlane#714) * chore: eliminate the SQLite testing fallback, run against real MariaDB Local tests silently diverging from CI's MariaDB (via a documented SQLite .env.testing fallback) has repeatedly masked real bugs this session — ->latest() defaulting to a nonexistent created_at column, and identifier quoting differences, both passed locally on SQLite and only failed on CI. - docker-compose.yml: cli service now injects DB_CONNECTION=mysql/DB_HOST=db etc. itself and depends_on db, so `docker compose run --rm cli php artisan test` works against real MariaDB with zero per-developer .env.testing edits - Add docker-resources/mariadb/init/01-create-test-db.sql to provision a dedicated invoiceplane_test database alongside the dev one on first boot - Fix db service: the named `database` volume was declared but never mounted, so all local dev/test data was lost on every container recreate - docker-resources/php-cli/Dockerfile: rebuild on Debian (php:8.4-cli) with the minimal proven extension set, matching the ip2-test-php:8.4 image this session used successfully throughout — see InvoicePlane#689 for a still-open false- failure issue found with a fresh cli image build, flagged in the docs - Update AGENTS.md/CLAUDE.md/README.md/.github/DOCKER.md/Makefile to point at the compose db/cli path instead of the SQLite instructions - Note throughout: use `php artisan test`, not raw vendor/bin/phpunit — the two were observed to behave differently for this app's Livewire form tests * chore: remove accidentally committed infrastructure files * feat(InvoicePlane#130): report template storage layer and brick registry (Phase 1) 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 InvoicePlane#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 InvoicePlane#130 InvoicePlane#521 InvoicePlane#528 * feat(InvoicePlane#130): five-band report builder UI in admin and company panels (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 InvoicePlane#130 InvoicePlane#519 InvoicePlane#523 InvoicePlane#525 * feat(InvoicePlane#130): report renderer and PDF pipeline (Phase 3) 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 (InvoicePlane#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 InvoicePlane#130 InvoicePlane#95 * feat(InvoicePlane#411): per-document PDF template selection (Phase 4) 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 InvoicePlane#411; Refs InvoicePlane#130 * fix(InvoicePlane#130): dompdf-safe row layout in ReportRenderer 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 InvoicePlane#130 InvoicePlane#95 * feat(InvoicePlane#130): opt-in Browsershot (headless Chromium) PDF driver 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 InvoicePlane#130 * fix: use explicit invoice/quote item values to bypass factory tax recalculation The InvoiceItemFactory and QuoteItemFactory have an afterMaking hook that recalculates tax_total based on tax_rate_id. The PdfGenerationServiceTest was passing hard-coded tax values to the factory, but the afterMaking hook was overwriting them (defaulting to 0% tax when no tax_rate_id was set). Fixed by: 1. Creating a 21% TaxRate for each test 2. Creating items directly with Model::create() instead of factory(), bypassing the afterMaking recalculation hook entirely This ensures the golden-snapshot fixtures use the correct hard-coded values (tax=21.00, total=121.00) regardless of factory behavior. Refs InvoicePlane#130 * fix(tests): compare rendered invoice HTML against the escaped customer name Blade's {{ }} auto-escapes output, so an unescaped comparison only fails when Faker happens to generate a company name with an apostrophe or other HTML-special character. Confirmed by forcing "O'Kon, Schneider and Wisozk" locally against real MariaDB: fails without e(), passes with it. See InvoicePlane#687. Refs InvoicePlane#130 * fix: add missing imports for EmailTemplateResource and CompanyUserResource Refs InvoicePlane#130 * fix: comment out invalid ReportTemplates navigation (Page, not Resource) Refs InvoicePlane#130 * fix: comment out ReportTemplates import (class does not exist in this context) Refs InvoicePlane#130 * fix: disable ReportTemplates page (not used, breaks discovery) Refs InvoicePlane#130
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true📝 WalkthroughWalkthroughAdded a file-backed report-template system with Mason editing, configurable report bricks, invoice and quote template selection, HTML rendering, and PDF generation. Added system-template synchronization, company template management, panel integration, PDF drivers, default templates, and automated tests. ChangesReport Builder and PDF Templates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Document
participant PdfGenerationService
participant ReportTemplateStorage
participant ReportDataMapper
participant ReportRenderer
participant PDFDriver
Document->>PdfGenerationService: Request invoice or quote PDF
PdfGenerationService->>ReportTemplateStorage: Resolve document template
PdfGenerationService->>ReportDataMapper: Map document data
PdfGenerationService->>ReportRenderer: Render template and data
ReportRenderer-->>PdfGenerationService: Return HTML
PdfGenerationService->>PDFDriver: Generate PDF
PDFDriver-->>Document: Stream PDF download
Merge Risk: 🔴 Critical · up to This PR adds report-template management and PDF generation, but the current implementation can let company users modify shared system templates and can break admin-page navigation at runtime; the declared dependency stack is also inconsistent. The PR is not merge-ready until these issues are fixed or explicitly accepted by the owner. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
nielsdrost7
force-pushed
the
feature/130-report-builder
branch
from
August 16, 2026 12:15
6bc7037 to
1ca45d6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
nielsdrost7
force-pushed
the
feature/130-report-builder
branch
from
August 16, 2026 12:22
7a7cdde to
5793edd
Compare
…ershot driver) (InvoicePlane#714)" This reverts commit b60a396.
nielsdrost7
force-pushed
the
feature/130-report-builder
branch
from
August 16, 2026 12:34
5878be9 to
d47fded
Compare
Local tests silently diverging from CI's MariaDB (via a documented SQLite .env.testing fallback) has repeatedly masked real bugs this session — ->latest() defaulting to a nonexistent created_at column, and identifier quoting differences, both passed locally on SQLite and only failed on CI. - docker-compose.yml: cli service now injects DB_CONNECTION=mysql/DB_HOST=db etc. itself and depends_on db, so `docker compose run --rm cli php artisan test` works against real MariaDB with zero per-developer .env.testing edits - Add docker-resources/mariadb/init/01-create-test-db.sql to provision a dedicated invoiceplane_test database alongside the dev one on first boot - Fix db service: the named `database` volume was declared but never mounted, so all local dev/test data was lost on every container recreate - docker-resources/php-cli/Dockerfile: rebuild on Debian (php:8.4-cli) with the minimal proven extension set, matching the ip2-test-php:8.4 image this session used successfully throughout — see InvoicePlane#689 for a still-open false- failure issue found with a fresh cli image build, flagged in the docs - Update AGENTS.md/CLAUDE.md/README.md/.github/DOCKER.md/Makefile to point at the compose db/cli path instead of the SQLite instructions - Note throughout: use `php artisan test`, not raw vendor/bin/phpunit — the two were observed to behave differently for this app's Livewire form tests
The CompanySettings page was imported and used in navigation but not registered in the pages array, causing "Route [filament.company.pages.settings] not defined" errors. Also includes: - fix(local-dev): make HTTPS forcing conditional on production only - fix(yarn): updated yarn.lock to match workspace dependencies
- docker-compose.yml: add MariaDB healthcheck (connection check) - docker-compose.yml: add depends_on with service_healthy for cli - docker-compose.yml: add init volume for test DB setup - AGENTS.md: use cp -n to prevent .env.testing overwrite
…r and brick registry (Phase 1) 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 InvoicePlane#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 InvoicePlane#130 InvoicePlane#521 InvoicePlane#528
…eport builder UI in admin and company panels (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 InvoicePlane#130 InvoicePlane#519 InvoicePlane#523 InvoicePlane#525
…ine (Phase 3) 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 (InvoicePlane#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 InvoicePlane#130 InvoicePlane#95
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 InvoicePlane#411; Refs InvoicePlane#130
…rtRenderer 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 InvoicePlane#130 InvoicePlane#95
…iver 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 InvoicePlane#130
…e/delete actions Validate that rename and delete operations are permitted before calling storage methods. Protects against UI bypass attempts.
Import Illuminate\Http\Response and add native return types to downloadInvoice() and downloadQuote() for type safety.
Replace direct company_id assignment with factory relationship pattern for Relation, Numbering, and Invoice.
Replace direct company_id assignment with factory relationship pattern for Relation, Invoice, and Quote in golden fixture helpers.
…ort pages Import ReportTemplates and ReportBuilder in CompanyPanelProvider, and ImportV1Page in AdminPanelProvider to resolve class not found errors.
…rmission checks The renameAction and deleteAction were creating a template array without the 'editable' key, causing an "Undefined array key" error when canModify() tried to check it. Pass the editable flag from arguments to ensure proper permission validation.
Report template bricks now respect their configured width (full, half, two_thirds, one_third) in the preview view, fixing layout issues where bricks would not display side-by-side as intended. Each brick preview now wraps itself with the appropriate width class and inline-block display, allowing proper grid-based layout. Fixes: InvoicePlane#130
The CompanySettings page was imported but not registered in the pages() array, causing the settings navigation link to throw a route-not-found error. Fixes: InvoicePlane#130
Changed from inline-block to float-left for better side-by-side brick layout in Mason canvas. Float-based layout is more reliable for positioning bricks with width constraints. Fixes: InvoicePlane#130
The committed yarn.lock predated several package.json entries (@playwright/test and its tree, the @tailwindcss/oxide / @rolldown / lightningcss platform binaries), so `yarn install --frozen-lockfile` failed with "Your lockfile needs to be updated" — breaking the Install JS dependencies step of both the Quickstart Smoke Test and Run PHPUnit Tests workflows. Pure lockfile regeneration for the unchanged package.json: no dependency version changes (zero removed `version` lines), just the missing transitive and optional-platform entries plus registry URL normalisation.
PdfGenerationServiceTest's three golden-HTML snapshot tests (default quote, default invoice, grouped invoice) failed on `assertSame` — the only difference is the leading indentation of the detail band's closing `</tbody>` (20 → 36 spaces), a Blade-partial whitespace change the fixtures predate. No data, tags, classes, or structure changed (git --word-diff shows whitespace-only). Regenerated the fixtures from the current renderer output; PdfGenerationServiceTest is 10/10 green.
# Conflicts: # .github/workflows/e2e-tests.yml # Modules/Core/Providers/CoreServiceProvider.php # Modules/Core/Tests/E2E/auth-helpers.js # Modules/Core/Tests/E2E/global-setup.js # Modules/Core/Tests/E2E/required-field-helpers.js # yarn.lock
… CompanyUserResource Inherited from upstream/develop via the merge (phpstan.yml is workflow_dispatch only there, so it never flagged these): - static::currentCompany() on a private static method -> self:: - currentCompany() narrowed to ?Company (Filament::getTenant() is Model|null)
The branch's composer.json requires php ^8.4 but composer.lock's platform block still pinned ^8.3, so `composer validate` reported the lock out of date. ToolchainMatchesCiTest (new on this branch via the InvoicePlane#724 merge) turns that into a hard failure. `composer update --lock` only — no package versions changed.
…tch only The PHPUnit suite was manual-dispatch only, so no push or PR ever ran it in CI — a whole class of regressions (e.g. composer.lock/composer.json drift caught only by ToolchainMatchesCiTest) could land on develop unnoticed. Match smoke.yml's standard trigger set.
Same rationale as 1794902 for phpunit.yml — these were workflow_dispatch only, so static analysis and the quickstart bootstrap smoke never ran on a push or PR. phpstan.yml's 'Comment on PR' step now actually fires.
… review
- M1: config/purify.php 'report' set (no img/a/style, no external resources,
https+mailto only); footer notes/terms/summary bricks use it. Closes the
blind-SSRF-via-<img> vector under the Browsershot driver.
- M2: config('ip.report.*') ceilings — row cap in ReportDataMapper with a
truncation notice, bricks-per-band cap and template-size limit in
ReportTemplateStorage, set_time_limit() guard on inline renders.
- M2: opt-in IP_REPORT_QUEUE — "Download PDF" dispatches GenerateDocumentPdfJob
to render/store on the new private report_pdfs disk instead of rendering in
the web request; default path unchanged.
- L1: ReportBrick::filterConfig() coerces presentational config values
(font_size clamp, enum allow-lists) server-side, not just in the Filament form.
- L3: drop the unreachable download() methods (unsanitised Content-Disposition)
from the PDF drivers and PDFInterface.
- I1: ReportsSyncSystemCommand fails loudly on a partial disk write.
Regression guards in ReportBuilderSecurityTest (11 cases). Golden HTML
snapshots regenerated (whitespace-only). Fixed two pre-existing issues in the
download-action tests (compressed-bytes assertion, Numbering phpstan type).
Details: _notes/report-builder-security-review-2026-09-10.md
…nderer (S3-1) BaseReportBuilderPage::renderPreviewHtml() had its own band -> entry -> brick walk with a parallel width->percent match, so the preview and the print path could drift on layout (where the grouped-details bug lived). - ReportRenderer::renderPreview(array $bands) reuses renderBand/chunkIntoRows/ renderRow/renderBrickSafely via a threaded `preview` flag: each brick shows toPreviewHtml() (no entity data), no <html> wrapper. - renderPreviewHtml() is now a 6-line delegation. - Preview shares the 12-column grid with print (was flex-wrap divs); the AdminReportBuilderTest width-wrapper case asserts the grid widths now. Details: _notes/report-builder-lens3-simplification-2026-09-10.md
storeInvoicePdf()/storeQuotePdf() discarded the Storage::put() return, so a read-only or full report_pdfs disk let the job report success with nothing written — the download handler then re-queues on every click with no error anywhere. Throw a RuntimeException on a false return so the job fails visibly. Closes InvoicePlane#755
GenerateDocumentPdfJob had no failed() handler, so a render failure landed silently in failed_jobs while the user was still told "being prepared", and repeat Download clicks queued a fresh render each time. Add a failed() that logs the document and error, and ShouldBeUnique keyed on the document so rapid clicks collapse to one job. tries=1 is kept intentionally. Closes InvoicePlane#756
The queue download handler only had coverage for queue-off and queue-on-with-nothing-stored. Add cases for a fresh stored copy being streamed back with nothing re-queued, a stale copy being bypassed for a fresh render, and the fresh-copy path for quotes. Closes InvoicePlane#757
CompanyObserver::deleted() was empty, so a deleted company left its
report_templates/{id} and report_pdfs/{id} directories on disk forever.
Delete both. Also note in the production checklist that
storage/app/report_templates must be in the backup set — those layouts are
not in the database.
Closes InvoicePlane#758
…lowing it readJson() caught JsonException and returned null with no trace, so a present-but-unparseable bands.json rendered as a silently-empty document, indistinguishable from an intentionally empty one. Log a warning with the path before returning null. Closes InvoicePlane#760
…test The "SSRF is Browsershot-only" conclusion from the security review rests on dompdf running with remote fetching off, but nothing failed if that flag flipped. Extract the options build into buildOptions() and assert getIsRemoteEnabled()/getIsJavascriptEnabled() are both false; also disable dompdf's default-on JavaScript, which invoice/quote rendering never needs. Closes InvoicePlane#759
calculateGroupTotals() has a five-way fallback (subtotal / unit_price*qty / price*qty / amount / total-tax) but the grouped render tests only ever fed the price+tax+total shape. Add a data-provider hitting each branch so a reorder or a broken branch changes an assertion instead of a silent number. Closes InvoicePlane#761
…ickTest coerceConfigValues() was only exercised from ReportBuilderSecurityTest against one brick, so deleting it broke nothing in ReportBrickTest — the natural home for ReportBrick behaviour. Add a case asserting a crafted font_size is clamped to an int and a non-enum text_align is dropped. Closes InvoicePlane#762
getEngine() maps ip.browsershot.* config + orientation onto the Browsershot instance and was entirely uncovered. Assert the format, landscape, node/npm binaries, chrome executablePath and no_sandbox mapping via reflection, without launching Chromium. Closes InvoicePlane#753
… tests The previous commit rewrote BrowsershotDriverTest and dropped three existing cases, one of them the --allow-file-access-from-files regression guard from the security review. Restore all three alongside the new getEngine() mapping cases. Refs InvoicePlane#753
…-prefixed
assertStringStartsWith('%PDF') + assertNotEmpty passes on a blank or
truncated render. Add an AssertsRenderedPdf trait that also requires the
%%EOF trailer and a byte floor, and use it in the representative
invoice/quote happy-path PDF tests. (pdftotext is not available in the test
container, so a text-fragment assertion isn't possible here; the
veteran-checklist page-count checks cover deeper content.)
Closes InvoicePlane#752
… unused S3-8: loadBySlug() now caches the resolved template per (company, slug, type) for the request, so a batch that renders many documents re-reads the JSON once instead of per document. S3-9: renderInvoiceHtml/renderQuoteHtml pass the resolved template's brick ids to the data mapper; forInvoice() skips the per-invoice open-invoice aging query unless the template actually has the detail_customer_aging brick. An empty brick-id list keeps the "build everything" behaviour for direct callers. The wider per-brick requiredRelations() eager-load map (S3-7) is left for a dedicated load-tested PR. Refs InvoicePlane#764
Existing report-builder E2E specs cover Mason drag/drop and brick config but not the page-level save + preview slide-over golden path — the wiring (asset build, canvas JS, action routes, the renderPreview path) the PHP suite can't see. Adds one spec for mount -> save -> preview, plus a check that the invoice list still exposes the Download PDF row action. Written against the established E2E helpers; not run locally (the E2E app is not served in this environment) — to be validated in CI. Closes InvoicePlane#763
…template S3-7: forInvoice()/forQuote() no longer eager-load the customer project/task tree or the invoice expenses unconditionally. The resolved template's brick ids drive it — the project relations load only when the template has a project/task brick (detail_invoice_project, detail_quote_project, detail_tasks, header_project), expenses only with detail_expense — and the matching data keys short-circuit to empty so a lazy read can't reintroduce the query. An empty brick-id list keeps "load everything" for direct callers. Completes the S3-7/S3-8/S3-9 set. Closes InvoicePlane#764
Open
5 tasks
…he open dropdown
The smoke spec's page-wide getByText('Download PDF') matched every rendered
row's pre-rendered ActionGroup panel (10 matches) and tripped Playwright
strict mode. Open the first row's trigger and assert a single visible
dropdown item instead.
Refs InvoicePlane#763
guardRenderTime() called @set_time_limit(120) on every invoice/quote PDF render. PHP CLI defaults max_execution_time to 0 (unlimited); the first render in the single-process test suite installed a 120s ceiling and reset the counter, so every test after the last render raced a 120s bomb. On CI (slower than local) the tail exceeded it and PHP fatard mid-test — surfacing as a bogus timeout in ExportFormDbSchemaCommandTest. The inline-render guard is a production safety net; skip it under app()->runningUnitTests().
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
2 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.
Implements the Report Builder (bands + bricks) per the v2 spec in #130 (epic #506, Track A) — Phases 1–4 in twelve commits across storage, UI, rendering, and PDF pipeline.
What's in here
ReportTemplateStorage(manifest.json + bands.json, zero DB tables), shipped default invoice/quote templates, idempotentreports:sync-system, 17 bricks harvested intoModules\Core\Mason+ new PageBreak/Spacer bricksReportRenderer+ReportDataMapper+ realPdfGenerationService; wired the stubbed "download pdf" actions; golden-file HTML snapshots + real dompdf byte teststemplatecolumns; resolution chain document → company default → system defaultPDFFactory(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.Closes
Select::make('_width')is now a real field in every content-bearing brick'sconfigureBrickAction()(added in commita147c34, after this PR's original implementation), so Filament's own form submission carries_widththrough the save — the "config rebuild drops_widthand resets to full width" regression Report Builder: per-brick width editing in the builder UI #598 described no longer reproduces. Round trip covered byAdminReportBuilderTest::it_lifts_the_block_width_out_of_the_config_when_saving/::it_hands_the_stored_width_back_to_the_canvas_on_load(both green as of 2026-09-08).Follow-up issues (not resolved here — reopen if auto-closed on merge)
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).
Summary by CodeRabbit