Commit Graph

6 Commits

Author SHA1 Message Date
Darko Gjorgjijoski
bd9602b130 fix(pdf): stop an unresolvable template taking the PDF route down (#735)
* fix(pdf): stop an unresolvable template taking the PDF route down

RealisticDemoSeeder::seedEstimate() never set template_name, while seedInvoice()
has always set invoice1. Every demo estimate therefore had '', so
findFormattedTemplate() returned null and EstimateService did $template['custom']
on it -- a 500 on the estimate PDF route, on either driver, since the exception
is thrown before a driver is reached. That is the "Unable to load document
preview" people were seeing.

The seeder now sets estimate1, but seeding was only how this surfaced. The stored
name is validated when a document is saved through the UI and nowhere else:
seeders, imports, recurring-invoice copies and rows predating that validation all
bypass it, and a template can also be deleted from disk after the fact. A name
that cannot be resolved should fall back to the default design, not take the
route down.

PdfTemplateUtils::resolveView() -- already the resolver for payment receipts and
reports -- gains an optional fallback and tries each candidate as custom then
built-in. Both document services collapse to a single call and can no longer
index null. The fallback logs a warning, so a bad name stays visible rather than
being silently swapped.

Also casts two nulls in GeneratesPdfTrait: an address line or custom field that
was never filled in reaches htmlspecialchars() and strtr() as null, which every
PDF render was emitting a deprecation for on PHP 8.4 and would be an error on 9.

Claude-Session: https://claude.ai/code/session_01QmECndmNZwzN65Zz9P87dF

* feat(pdf): a command that measures the two drivers against each other

"The PDF looks different" has been diagnosed by eye every time, because nothing
compares the renderers. Asserting on PDF bytes is useless and rendering through
Gotenberg needs a live service, so the suite has never covered it.

pdf:compare renders each stock template through both drivers and reports the
page box, page count and the bounding box of the text on page one, then flags
any template whose ink lands more than --tolerance points apart. It goes through
the real document services, so it exercises the same shared view data and
template resolution a request would.

Two things it has to get right to be honest:

Comparing designs means persisting the template choice -- InvoiceService reads it
back with Invoice::find($id)->template_name, so assigning in memory silently
compares the same design every row. The run happens inside a transaction that is
always rolled back.

Page numbers are turned off for the duration. They are a Chromium capability with
no dompdf equivalent, so leaving them on puts ink at the foot of every Gotenberg
page and drowns out every difference worth seeing -- which is exactly what the
first run of this command did.

Word positions come from poppler's pdftotext, which is on most dev machines but
not in the app container; without it the command still compares page geometry and
says what it could not check.

Claude-Session: https://claude.ai/code/session_01QmECndmNZwzN65Zz9P87dF

* fix(pdf): let the two renderers agree on the items table, and drop the shim

Two parts: the stock templates stop doing their own page margins, and the items
table stops relying on a property that does not apply to it.

Page margins. The templates carried their own via `html { margin-top: 50px }`,
which predates page setup owning them. dompdf largely collapses that margin;
Chromium honours it and adds it to the page box, so the same template came out
38px from the top on one renderer and 77px on the other. The html rule is gone
and body is reset instead, which is what makes the page box agree. Headers that
were positioned absolutely at a negative offset -- only possible because of that
margin -- are back in flow.

The items table. Every stock template sets `table { border-collapse: collapse }`,
and CSS says padding does not apply to a table in that mode. dompdf applies it
anyway; Chromium follows the spec and drops it, so the table's `padding: 0 30px`
inset the content on one renderer and not the other. Measured in isolation: with
border-collapse, content starts at x=24.0 on dompdf and x=1.5 on Chromium -- the
full 30px. All of the table's spacing moves to .items-table-wrapper, a plain
block both engines treat the same, using padding so nothing collapses through it
either.

Measured across the seven document templates, that closes the horizontal gap
outright: xMin was 57 on dompdf against 37 on Chromium for five of them, and is
now within 3pt on all seven.

GotenbergStockTemplateCompatibility is removed. Its premise was that dompdf
inflates declared line heights by 1.5x, and that does not hold: rendering the
same text at 12px, 18px, 36px and unitless 1.0/1.5 through both engines gives
line spacing within 0.5pt every time. It also applied its multiplier to the
reports, where line-height 21px pairs with font sizes of 14, 16 and 20px -- so
.report-footer-value at a 1.05 ratio was being blown out to 31.5px, half again
taller than dompdf renders it.

A residual vertical difference remains and is localised, not guessed at: it
accumulates only in the address blocks, which are <br>-joined text emitted by
getFormattedString() with an <h3> in front. Reduced to that construct alone,
Chromium steps 11.25pt per line -- exactly the declared line-height: 15px --
while dompdf steps 14.4pt. That needs deciding on its own terms rather than a
global multiplier, so it is left visible and measurable via pdf:compare.

Claude-Session: https://claude.ai/code/session_01QmECndmNZwzN65Zz9P87dF

* fix(pdf): restore the stock template design

Two regressions from the page-setup work, both visible on the page.

The coloured header band stopped bleeding to the paper edges. invoice2 and
estimate2 are built around a full-width band, and it now sits in normal flow at
the top of body, so it only reaches the edge when the page margin is nothing.
#728 defaulted margins to 1.2cm on the reasoning that it matched dompdf's
built-in default and so kept existing output unchanged. That was the wrong
reference: the templates are drawn for a zero margin and carry their own 30px
insets, and Gotenberg rendered them at zero before #728, which is the intended
look. Margins now default to nothing. Setting one still works and is honoured by
both drivers, at the cost of the band no longer reaching the edge.

A bare `0` is valid CSS and the only length needing no unit, so CssLength and
PdfPageSetup accept it -- without that the new default would have thrown on
every render.

The totals block was pushed in from the items table's right edge. Fixing the
border-collapse padding problem moved the table's 30px inset onto a wrapper that
contains the whole partial, so it stacked on the insets the hr (25px) and the
totals container (25px) already had. Those two were always honoured by both
renderers; only the table's own padding was not. The inset now lives on a div
wrapping just the table, and the wrapper keeps vertical spacing only, which
restores the original 30px/25px relationship rather than inventing a new one.

Also drops the negative margin-bottom that pulled the addresses up into the band
and hid "Bill to,", and removes a stray `bottom: 0px` on invoice1's
.header-bottom-divider that combined with `top: 90px` to stretch the rule down
the page.

Checked by rendering, not only by measurement: invoice2 and invoice1 on both
drivers now match the intended design. pdf:compare puts the two renderers within
a few points horizontally on all seven documents, xMin 21-22 and xMax 564-575.
The remaining vertical difference is the address-block line spacing documented
earlier and is unchanged by this.

Claude-Session: https://claude.ai/code/session_01QmECndmNZwzN65Zz9P87dF

* feat(demo): make the demo data actually demo the product

The demo company had no address row, and Invoice::getCompanyAddress() returns
false outright in that case, so every seeded document rendered with an empty
company block -- the name only appeared because the header falls back to it when
there is no logo. Several headline features had no demo data at all: zero tax
types, zero notes, zero recurring invoices, zero custom fields.

DemoSeeder, which the test suite and reset:app both run, now creates Acme Inc
with a postal address, tax ids and a country -- the fields the default address
format actually renders. The address is created through the relation, as
CompaniesController does, so company_id is set and type/user_id/customer_id stay
null: Company::address() is an unscoped hasOne, so anything else carrying that
company_id would be picked up as the company's own.

It also stops trusting currency id 1. Migration 2025_08_18 inserts Algerian
Dinar via firstOrCreate() before any seeder runs, so on a fresh migrate+seed the
demo priced everything in "DA". RealisticDemoSeeder already worked around this
for itself; resolving USD by code fixes it at source for reset:app and the tests
too.

RealisticDemoSeeder gains a logo, two tax types, a notes library, custom fields
and an active recurring invoice. Notes are seeded twice over on purpose: the
library and a document's notes column are unrelated in this application -- there
is no foreign key, and is_default only drives a badge in the settings list, so
nothing pre-fills a document with one.

Tax is applied at document level to most but not all documents, so the demo has
a zero-rated example in it. The arithmetic is the caller's: the service layer
trusts whatever amount it is handed rather than recomputing it, so tax is
rounded once off the subtotal and carried through total, due_amount and every
base_* twin -- miss due_amount and a paid invoice renders as part-paid.

Custom fields are on Customer, the only model_type with a create/edit UI end to
end. The PDF renders only model_type 'Item', which would add a column to the
items table and disturb a layout that was just squared up across both drivers.

The logo is a generated Acme mark rather than one of InvoiceShelf's own, which
would read as InvoiceShelf billing the customer.

Also documents both seeders in AGENTS.md. RealisticDemoSeeder was referenced
nowhere outside database/seeders/, which is a poor place to keep the thing that
makes the app look real.

Claude-Session: https://claude.ai/code/session_01QmECndmNZwzN65Zz9P87dF

* fix(pdf): tighten the spacing the old absolute header left behind

invoice2 and estimate2 carried three stacked top offsets -- content-wrapper's
60px margin plus address-container's 18px margin and 20px padding -- 98px of
dead white between the coloured band and the first line of content. They existed
because the band used to be position: absolute and out of flow, so everything
below had to be pushed clear of where it visually sat. The band takes its own
height now, so the compensation is just a gap. Collapsed to a single 32px.

Only those two templates had it, which is the tell: they are exactly the two
whose headers were absolutely positioned.

Also pins the margins on the <h3> the address formats emit. Left to the
user-agent default it pushed the company column out of line with the Bill to /
Ship to columns beside it, so the three column headings started at three
different heights. They line up now.

That h3 is also where the two renderers were measured drifting apart, and
pinning it narrows invoice2 from 85.8pt to 72.0pt and estimate2 from 84.4 to
76.8. The templates without a coloured band barely move, which places the rest
of the difference in the per-line spacing of the <br>-joined address lines
rather than in the heading -- consistent with the isolated measurement earlier
(dompdf 14.4pt per line against Chromium's 11.25pt) and still open.

Claude-Session: https://claude.ai/code/session_01QmECndmNZwzN65Zz9P87dF
2026-08-01 14:45:58 +02:00
Darko Gjorgjijoski
3a989923d1 ci: reduce the release workflow to what it still does, rename to publish.yaml (#724)
Most of docker.yaml was doing work that was duplicated or hazardous now
that releases are cut from a tag.

The tests were duplicated exactly: release.yaml and docker.yaml called
the same reusable tests.yaml, on the same commit — once before drafting,
again after publishing. No new information, and the image build queued
behind it. Pint likewise: check.yaml already style-checked the commit
when it landed. Both jobs go; coverage is unchanged.

manual_docker_build goes too, and it was worse than redundant. Its
checkout took no ref, so it built the dispatched branch while tagging
with whatever string was typed — an image could be labelled 2.4.2 while
containing 2.x HEAD — and `tag` defaulted to "latest", so a careless
dispatch republished the moving stable tag from a branch. #710 had to add
a guard purely to stop the registration dispatch doing that by accident.
It was last used in September 2025 to push the legacy and alpha tags
during the Docker distribution work; that is finished, and releases
produce images now. A patched base image is better served by a patch
release than by silently changing what a pinned tag contains.

That cascade removes the tag input and the #710 guard as well, leaving
register_tag as the only dispatch input and two jobs in the file.

Losing the test gate would leave the image build ungated, so the release
build now refuses a release with no InvoiceShelf.zip. A release either
came from the tested pipeline or it gets no images — the updater is
already protected this way, since registration downloads that same asset.
GitHub offers no way to forbid hand-made releases; this is the closest
thing, which is to make them inert.

"Docker" no longer describes a workflow that registers on the updater and
publishes images, so it becomes publish.yaml — matching its trigger and
pairing with release.yaml, which prepares what this distributes. The
recovery instructions in AGENTS.md name this workflow and would have
broken silently, so they move with it.
2026-07-29 18:27:18 +02:00
Darko Gjorgjijoski
e958cada30 ci: stop at the draft; publishing is a human action (#720)
2.4.3-beta.2 was cut by tag and came out correctly — published,
pre-release, zip attached, notes from CHANGELOG.md — and then nothing
else happened. No updater registration, no Docker images.

GitHub does not start workflow runs from events created with
GITHUB_TOKEN. The publish step authenticated as github-actions[bot], so
`release: published` fired and triggered nothing. Every earlier
docker.yaml run was event=release from a release a human published,
which is why beta.1 worked and beta.2 did not. 3.x carries the same
design and would have failed identically on the first RC.

The workflow now stops at the draft. Everything else is unchanged: the
tag still runs the tests, reads the notes, builds the package and
attaches it. Pressing Publish fires the event under a real identity and
the proven downstream runs as it always has — and it puts a deliberate
gate in front of a release going public.

prerelease and make_latest move onto the draft rather than being applied
at publish time, so the flags are already right when the button is
pressed; GitHub's publish dialog otherwise defaults "Set as the latest
release" to checked, which would let a 3.0.0 alpha displace 2.4.x.

The run now ends by writing the draft URL and the resolved flags to the
job summary, since a draft nobody knows about is no use.

Documents the whole flow in AGENTS.md, including why publishing is
manual — the reasoning is not guessable from the workflow alone.
2026-07-29 17:04:15 +02:00
Darko Gjorgjijoski
421c385fa7 chore: drop Laravel Boost; make AGENTS.md the canonical agent doc (#684)
- Remove laravel/boost (+ its deps laravel/mcp, laravel/roster) and Boost's
  .cursor/ skills + MCP registration. No version sweep (composer remove).
- AGENTS.md is now the hand-maintained source of truth (was Boost-generated):
  folds in the CLAUDE.md content + a Conventions section codifying the migration
  FK rule (unsignedInteger for INT-PK refs, not foreignId — MySQL error 3780)
  and the MySQL/PostgreSQL/SQLite cross-DB requirement.
- CLAUDE.md / GEMINI.md / .github/copilot-instructions.md are now gitignored
  symlinks to AGENTS.md, created by bin/ai-docs.php (composer run ai-docs, also
  wired into post-autoload-dump).
2026-06-12 21:54:09 +02:00
Darko Gjorgjijoski
c3ad718799 Document Service pattern and TDD requirements in CLAUDE.md and AGENTS.md
Add mandatory Service pattern guidelines: all business logic in Services,
thin controllers, clean models. Document TDD approach with feature tests
for routes and unit tests for services. Add role definitions (super admin
vs owner) to AGENTS.md.
2026-04-03 22:38:53 +02:00
mchev
186ab35fd4 Laravel 13 upgrade, updates and fixes 2026-03-21 18:53:33 +01:00