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.

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

[Mailer] Add support for allowing some users even if recipients is defined in EnvelopeListener #54044

Merged
merged 1 commit into from Apr 9, 2024
Merged

[Mailer] Add support for allowing some users even if recipients is defined in EnvelopeListener #54044

merged 1 commit into from Apr 9, 2024

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Feb 23, 2024 •
edited

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

I'm migrate an application from SwiftMailer to symfony/mailer, and this options was used.

depends on #54292

@carsonbot carsonbot added this to the 7.1 milestone Feb 23, 2024
@nicolas-grekas
Copy link
Member

I don't understand what's this for, can you please give me a few examples ? The code is fairly intricated for only two options and I'm wondering how this logic is sensible for real world use cases. 🙏

@lyrixx
Copy link
Member Author

lyrixx commented Mar 14, 2024

Sure, here is the documentation

And our real world example is for the preproduction:

mailer_recipients: [ 'REDACTED@jolicode.com' ]
mailer_recipients_allowed: [ '/@jolicode\.com$/', '/^foobar@example\.com$/', '/^baz@example\.com$/' ]

TL;DR:

in preprod, we want that all mails to be sent to 'REDACTED@jolicode.com' except the ones targeting our agency, or some specials client mail addresses

@lyrixx lyrixx mentioned this pull request Mar 14, 2024
Merged
@lyrixx
Copy link
Member Author

lyrixx commented Mar 15, 2024

I added some tests for the DIC part. But the configuration was broken in XML. I need #54292 first to be merged

@lyrixx lyrixx requested a review from stof March 15, 2024 07:33
lyrixx added a commit that referenced this pull request Mar 15, 2024
@lyrixx
bug #54292 [FrameworkBundle] Fix mailer config with XML (lyrixx)
cb3ec06 
This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] Fix mailer config with XML

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

I noticed that, while adding test for #54044

---

Before my patch, if I keep only one recipients:

```
>…ome/gregoire/dev/github.com/lyrixx/symfony(5.4 *) git di
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/mailer_with_dsn.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/mailer_with_dsn.xml
index be53f59bc3..5ccdefaf32 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/mailer_with_dsn.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/mailer_with_dsn.xml
@@ -11,7 +11,7 @@
             <framework:envelope>
                 <framework:sender>sender@example.org</framework:sender>
                 <framework:recipients>redirected@example.org</framework:recipients>
-                <framework:recipients>redirected1@example.org</framework:recipients>
+                <!-- <framework:recipients>redirected1@example.org</framework:recipients> -->
             </framework:envelope>
             <framework:header name="from">from@example.org</framework:header>
             <framework:header name="bcc">bcc1@example.org</framework:header>
>…ome/gregoire/dev/github.com/lyrixx/symfony(5.4 *) ./phpunit src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/XmlFrameworkExtensionTest.php --filter 'testMailer#0'
PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Testing Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\XmlFrameworkExtensionTest
E                                                                   1 / 1 (100%)R

Time: 00:00.103, Memory: 21.88 MB

There was 1 error:

1) Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\XmlFrameworkExtensionTest::testMailer with data set #0 ('mailer_with_dsn', array('smtp://example.com'))
Symfony\Component\Config\Definition\Exception\InvalidTypeException: Invalid type for path "framework.mailer.envelope.recipients". Expected "array", but got "string"

```

And I cannot add more XML configuration, without this patch

Commits
-------

0cfdf2f [FrameworkBundle] Fix mailer config with XML
@lyrixx
Copy link
Member Author

lyrixx commented Mar 15, 2024

I have rebased, now the PR is to merge

@lyrixx lyrixx force-pushed the mailer-EnvelopeListener branch 2 times, most recently from 22d2b54 to 331438d Compare March 21, 2024 08:04
@lyrixx
Copy link
Member Author

lyrixx commented Mar 21, 2024

@nicolas-grekas I have addressed your comments

fabpot
fabpot approved these changes Apr 8, 2024
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

@lyrixx We should probably add an entry in the framework bundle changelog as well like:

 * Add the `allowed_recipients` option for mailer to allow some users to receive emails even if `recipients` is defined.

@lyrixx
Copy link
Member Author

lyrixx commented Apr 8, 2024

@fabpot thanks. Added (+ rebased)

@fabpot
Copy link
Member

fabpot commented Apr 9, 2024

Thank you @lyrixx.

@fabpot fabpot merged commit ce408ab into symfony:7.1 Apr 9, 2024
7 of 10 checks passed
@fabpot fabpot mentioned this pull request Apr 9, 2024
Closed
@lyrixx lyrixx deleted the mailer-EnvelopeListener branch April 9, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stloyd stloyd stloyd left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@OskarStark OskarStark OskarStark left review comments

@fabpot fabpot fabpot approved these changes

@stof stof stof approved these changes

@welcoMattic welcoMattic welcoMattic approved these changes

Assignees
No one assigned
Labels
Feature Mailer Status: Reviewed
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@lyrixx @nicolas-grekas @fabpot @stloyd @stof @welcoMattic @OskarStark @carsonbot