fix: run Lettr setup even when build() is overridden - #15
Open
voj-tech-j wants to merge 1 commit into
Open
Conversation
Overriding build() in a LettrMailable subclass without calling parent::build() — the pattern shown in the README — meant none of the Lettr setup ran: no transport selection, no X-Lettr-* headers, no placeholder body. Delivery then aborted with "InvalidArgumentException: Invalid view." because the mailable had no renderable content. Move the setup into prepareLettrDelivery() and drive it from Laravel's own delivery hook (prepareMailableForDelivery), with content() as a second entry point. Both run regardless of what a subclass does with build(), which stays for backwards compatibility. The setup is idempotent: the withSymfonyMessage callback is registered once, and everything it reads is resolved when the callback fires, so the call order relative to template()/substitutionData() no longer matters. The placeholder body is now only applied when the mailable has no HTML body of its own, so a body set via Content(htmlString:) during content hydration is no longer clobbered by the later setup pass. Add delivery-level tests that send through the array transport and assert the Symfony headers — 5 of the 8 fail against the previous implementation. They live in a new file because ReadmeDocTest.php is generated and would drop them on regeneration; that file only called build() and inspected properties, which is why it passed on code that could not be delivered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Problem
Overriding
build()in aLettrMailablesubclass without callingparent::build()— the pattern shown in README.md:195, :246 and :368 — makes delivery fail outright:Laravel's
Mailable::prepareMailableForDelivery()callsbuild()viaContainer::call(), so a subclass override wins and none of the Lettr setup runs: nomailer('lettr'), noX-Lettr-*headers, no placeholder body.content()then returns an emptyContentin API-template mode,$this->viewstays null, andMailer::parseView(null)throws.The generated stubs are unaffected (they declare
$templateSlugas a property and never overridebuild()), so this only hit hand-written mailables that followed the README.Fix
The setup moved out of
build()intoprepareLettrDelivery(), driven by two hooks Laravel always reaches:prepareMailableForDelivery()(overridden) — runs on every send and render, afterbuild()/content(), so no subclass override can skip it.content()— a second entry point in case that protected framework method ever changes.prepareLettrDelivery()is idempotent: thewithSymfonyMessagecallback is registered once, and everything it reads (slug, version, substitution data, tags) resolves when the callback fires at send time — so the call order relative totemplate()/substitutionData()no longer matters.build()remains as a thin wrapper for backwards compatibility.Compatibility
Patch-level. Every pattern that worked before still works; previously-failing code now works.
build()override withoutparent::build()(README)Invalid viewbuild()override withparent::build()$templateSlugproperty, nobuild()(stubs)Three deliberate deltas:
This email uses Lettr template: …placeholder. Wire output is identical —LettrTransportFactoryrenders from the slug header and ignores the body in template mode. This is load-bearing: the setup now runs after content hydration, so without it a subclass returningContent(htmlString: …)would have its body clobbered.content()now has a side effect — calling it directly in a test mutates the mailable.LettrMailableto not send via Lettr would now getX-Lettr-*headers, and on the queued path would route to the lettr transport. Such code raisedInvalid viewbefore unless it also set a view, so the population is near-zero.prepareMailableForDelivery()is a protected framework internal, stable since Laravel 9 and present across the^10|^11|^12|^13range incomposer.json. If it ever disappears, thecontent()hook still covers every subclass that doesn't overridecontent().Tests
New
tests/Unit/LettrMailableTest.php— 8 tests that actually send through thearraytransport and assert the resulting Symfony headers, covering all four subclass patterns plus headers-registered-exactly-once, placeholder body, explicit-body-wins, and Blade untouched. 5 of the 8 fail against the pre-fix implementation.They're in a new file rather than
ReadmeDocTest.phpbecause that file is generated by the readme-doc-test skill and would drop them on regeneration. Worth noting why the gap existed:ReadmeDocTestcalls$mailable->build()and reflects on properties without ever sending, so it passed on code that could not be delivered.pest225 passed ·phpstanno errors ·pintpass.The README examples now work as written, so they're unchanged.
CHANGELOG.mdgets an[Unreleased]entry —LettrServiceProvider::VERSIONis untouched since bumping and tagging is a separate release step; this is a2.3.1.🤖 Generated with Claude Code