Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Mailer] Fix rendered templates for notifications #48505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Dec 6, 2022

Q A
Branch? 6.2
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #48472
License MIT
Doc PR n/a

Alternative to #48481

Copy link
Contributor

@ThomasTr ThomasTr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me

@xabbuh
Copy link
Member

xabbuh commented Dec 6, 2022

looks like we need to bump the minimum allowed version of symfony/mime in the Mailer component

@fabpot fabpot force-pushed the mailer-rendered-template-fix-62 branch from 87276fb to ba1bd59 Compare December 6, 2022 12:49
src/Symfony/Component/Mailer/composer.json Outdated Show resolved Hide resolved
src/Symfony/Component/Mailer/composer.json Outdated Show resolved Hide resolved
@fabpot fabpot force-pushed the mailer-rendered-template-fix-62 branch 2 times, most recently from 46d2f8a to bf9aea9 Compare December 6, 2022 16:47
if (3 === \count($data)) {
if (4 === \count($data)) {
[$this->context, $this->theme, $this->rendered, $parentData] = $data;
} elseif (3 === \count($data)) {
[$this->context, $this->theme, $parentData] = $data;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is worth adding a comment as done two lines below or should it be updated to cover this new case?

@@ -67,6 +67,18 @@ public function getContext(): array
return $this->context;
}

public function isRendered(): bool
{
return null === $this->htmlTemplate && null === $this->textTemplate;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using a boolean property as done in the other class?
Setting those to null in the next method could lead to inconsistency?

@fabpot fabpot force-pushed the mailer-rendered-template-fix-62 branch from bf9aea9 to 677e188 Compare December 6, 2022 16:51
@fabpot fabpot force-pushed the mailer-rendered-template-fix-62 branch from 677e188 to 085185d Compare December 6, 2022 16:54
@@ -76,7 +76,7 @@ public function send(RawMessage $message, Envelope $envelope = null): ?SentMessa
$envelope = $event->getEnvelope();
$message = $event->getMessage();

if ($message instanceof TemplatedEmail && ($message->getTextTemplate() || $message->getHtmlTemplate())) {
if ($message instanceof TemplatedEmail && !$message->isRendered()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior seems different here now, the error may be that the template has already been rendered.

@fabpot fabpot merged commit 4cb2a86 into symfony:6.2 Dec 6, 2022
@fabpot fabpot deleted the mailer-rendered-template-fix-62 branch December 6, 2022 17:02
@fabpot fabpot mentioned this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.