Skip to content
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

MailerAssertionsTrait::getMailerMessage returns messages that aren't rendered (BC break) #47383

Closed
nesl247 opened this issue Aug 24, 2022 · 2 comments · Fixed by #47992
Closed

Comments

@nesl247
Copy link

nesl247 commented Aug 24, 2022

Symfony version(s) affected

6.2.0

Description

Prior to 6.2.0, a call to self::getMailerMessage(0) would return the only message that was sent. After #47191 was merged, this returns a message from the QueueingMessageEvent, which means it hasn't been rendered via MessageListener yet.

Given this is just in tests, it's not a critical BC break, but it is a BC break. I'm not sure if getMailerMessage and getMailerMessages is really intended to be returning RawMessages from QueueingMessageEvent or not.

How to reproduce

Create templates/test_email.html.twig

<?php

use Symfony\Bridge\Twig\Mime\TemplatedEmail;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\Component\Mailer\MailerInterface;

class TestCase extends KernelTestCase
{
    public function testItSendsAnEmail(): void
    {
        $mailer = self::getContainer()->get(MailerInterface::class);

        $email = (new TemplatedEmail())
            ->from('from@example.com')
            ->to('user@example.com')
            ->htmlTemplate('test_email.html.twig');

        $mailer->send($email);

        $email = self::getMailerMessage(0);
        self::assertNotNull($email->getHtmlBody());
    }
}

Possible Solution

Change getMailerMessage and getMailerMessages to filter the events to remove QueueingMessageEvent.

Additional Context

No response

@PhilETaylor
Copy link
Contributor

PhilETaylor commented Oct 25, 2022

Confirmed today in 6.2.0-Beta1

This was the ONLY breaking change in my unit tests going from 6.1 to 6.2

My test case was

public function testEmailHeaders()
    {
        $kernel = static::createKernel();
        $kernel->boot();

        // get our own Mailer Service, which decorates the mailer
        $mailer = $this->getContainer()->get(MailerInterface::class);

        $message = (new Email())
            ->from('phil@phil-taylor.com')
            ->to('phil@phil-taylor.com')
            ->text('Unit Test Message')
            ->subject('Unit Test Message');

        $mailer->send($message);

        $email = $this->getMailerMessage();
        $this->assertEmailCount(1);
        $this->assertEmailHeaderSame($email, 'To', 'phil@phil-taylor.com');
        $this->assertEmailHeaderSame($email, 'Subject', 'Unit Test Message');

        // Ensure custom headers are present (This fails in 6.2)
        $this->assertEmailHasHeader($email, 'List-Unsubscribe');
        $this->assertEmailHasHeader($email, 'X-Report-Abuse');
        $this->assertEmailHasHeader($email, 'X-About');
        $this->assertEmailHasHeader($email, 'X-Auto-Response-Suppress');

        $this->assertEmailTextBodyContains($email, 'Unit Test Message');
        $this->assertEmailAttachmentCount($email, 0);
    }

The headers are added in framework.mailer.headers configuration and not per email, this unit test passes in 6.1 and fails in 6.2 as $email = $this->getMailerMessage() returns the QueueingMessageEvent and not the real message sent.

@chalasr
Copy link
Member

chalasr commented Oct 25, 2022

See #47992

@fabpot fabpot closed this as completed Oct 26, 2022
fabpot added a commit that referenced this issue Oct 26, 2022
This PR was merged into the 6.2 branch.

Discussion
----------

[Mailer] Fix BC breaking event name change

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #47989, Fix #47383
| License       | MIT
| Doc PR        | -

While I get the motivation for introducing this `QueuingMessageEvent` child event in #47191, it's a BC break that is difficult to circumvent due to the fact it's an event (the parent may still be used as it's not deprecated).
So I propose to revert its addition and just add the needed stamp-related methods to the original event instead.

Commits
-------

cb64938 [Mailer] Fix BC breaking event name change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants