Skip to content

Navigation Menu

Sign in

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

[Mime] Keep context after rendering emails #50233

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Mime] Keep context after rendering emails #50233

wants to merge 1 commit into from

Conversation

Rainrider
Copy link
Contributor

Q A
Branch? 6.2
Bug fix? no
New feature? no
Deprecations? no
License MIT

This PR keeps the context after rendering emails. This behavior was present up to 6.1 and was changed with cc3b871.

The context is useful when testing generated values passed to the template. I.e. we generate a salutation for emails depending on user gender and pass it to the template through the context. A test would then look like that:

final class EmailTest extends KernelTestCase
{
    public function testContext(): void
    {
        $this->createUser(); // trigger welcome email

        $this->assertQueuedEmailCount(1);

        $message = $this->getMailerMessage();
        $salutation = $message->getContext()['salutation'];

        $this->assertEmailHtmlBodyContains($message, $salutation);
    }
}

This way we can change the salutation generator implementation without having to adapt the functional tests for email delivery and content.

@carsonbot carsonbot added this to the 6.3 milestone May 3, 2023
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "6.3" but it seems your PR description refers to branch "6.2".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@Rainrider Rainrider changed the base branch from 6.3 to 6.2 May 3, 2023 19:10
@fabpot
Copy link
Member

fabpot commented May 3, 2023

See #48300 for a discussion about why this cannot be done this way.

@fabpot fabpot closed this May 3, 2023
@Rainrider
Copy link
Contributor Author

@fabpot #47075 (comment) states this is for serialization. Could we just remove the context from __serialize if this is the only problem with this PR?

@Rainrider
Copy link
Contributor Author

Rainrider commented May 3, 2023 •
edited
Loading

@fabpot I updated my branch based on this comment. In case you might consider this a solution, see 6.2...Rainrider:symfony:mime/keep-email-context (the PR here does not reflect this because you closed it)

@Rainrider
Copy link
Contributor Author

I just switched to using the DomCrawler component to extract the values I need from the rendered email, so I don't have a valid use case for this anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Mime Status: Needs Review
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
3 participants
@Rainrider @carsonbot @fabpot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.