Added mailers #34

jeremyFreeAgent · 2013-04-17T12:55:57Z

Now you can configure several mailers.

stof · 2013-04-17T13:00:40Z

Command/SendEmailCommand.php

@@ -50,25 +50,31 @@ protected function configure()
     */
    protected function execute(InputInterface $input, OutputInterface $output)
    {
-        $mailer     = $this->getContainer()->get('mailer');
-        $transport  = $mailer->getTransport();
+        if ($this->getContainer()->getParameter('swiftmailer.spool.enabled')) {


I think the command should send flush the spool for a single mailer instead of looping over all of them (with an argument or an option to select the mailer)

With an optional argument ?

stof · 2013-04-17T13:06:54Z

You should have 2 tests with the extended config formats: one with several mailers (the one you added here) and 1 with a single mailer IMO. It will show you that the XML config is wrong currently (parsing XML produces a different result for 1 or several childs, and using a bad XML structure does not allow the Config component to normalize it)

stof · 2013-04-17T13:12:37Z

you also need to update the profiler integration to avoid mixing the different mailers together (it might require branching for 2.3 as the DataCollector is in the bridge)

sstok · 2013-04-19T09:04:00Z

DependencyInjection/SwiftmailerExtension.php

+            $mailer['port'] = 'ssl' === $mailer['encryption'] ? 465 : 25;
+        }
+
+        foreach (array('encryption', 'port', 'host', 'username', 'password', 'auth_mode', 'timeout', 'source_ip') as $key) {


Should this be inside the if statement as it only applies to SMTP?

Thanks, I'll fix it.

jeremyFreeAgent · 2013-04-29T16:46:43Z

I've added the test with a single mailer and the DataCollector.

shouze · 2013-04-30T00:17:55Z

Nice job! 👍

bmeynell · 2013-05-02T20:20:20Z

👍 Much needed

jeremyFreeAgent · 2013-05-03T14:31:36Z

What to you think about it @stof ?

shouze · 2013-05-03T14:32:42Z

Tested, looks ready 🔨

bmeynell · 2013-05-03T17:38:19Z

@jeremyFreeAgent / @shouze - it says Travis is failing?

jeremyFreeAgent · 2013-05-03T17:42:10Z

@bmeynell it's because of the Resources/config/schema/swiftmailer-1.0.xsd update and the

<container xmlns="http://symfony.com/schema/dic/services"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:swiftmailer="http://symfony.com/schema/dic/swiftmailer"
    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
                        http://symfony.com/schema/dic/swiftmailer http://symfony.com/schema/dic/swiftmailer/swiftmailer-1.0.xsd">

in the tests XML files.

shouze · 2013-05-03T17:48:10Z

@bmeynell : @jeremyFreeAgent can't upload the new xsd on symfony.com... so unless he puts a file:// scheme with the complete local filesystem path to the xsd it won't pass tests... and of course no way to do it.

This feature is non BC break and tests will pass when the new xsd will be online.
If you try locally on your computer with the file:// scheme trick you'll see it's okay 😉

shouze · 2013-05-03T17:57:58Z

@bmeynell there's one solution : having a proxy that match & serve the local xsd during the tests instead of fetching the online (and maybe not up2date) one.

@stof what about implementing this solution ?

bmeynell · 2013-05-03T20:07:52Z

Is this a configuration BC break? I seemed to had to define the mailers: node even for just one mailer. I also had to comment out my antiflood configuration or else an exception was thrown. Other than that, I'm successfully using two mailers now in my fork! :)

bmeynell · 2013-05-03T20:27:39Z

Hmm.. CLI works but symfony doesn't load via the web:

ErrorException: Warning: Missing argument 2 for Symfony\Bridge\Swiftmailer\DataCollector\MessageDataCollector::__construct(), called in app/cache/dev/appDevDebugProjectContainer.php on line 3348 and defined in src/vendor/symfony/src/Symfony/Bridge/Swiftmailer/DataCollector/MessageDataCollector.php line 39

Thoughts?

shouze · 2013-05-03T20:53:16Z

Yes there's also a PR on SwiftMailerBridge ;)

Le vendredi 3 mai 2013, Ben Meynell a écrit :

Hmm.. CLI works but symfony doesn't load via the web:

ErrorException: Warning: Missing argument 2 for Symfony\Bridge\Swiftmailer\DataCollector\MessageDataCollector::__construct(), called in app/cache/dev/appDevDebugProjectContainer.php on line 3348 and defined in src/vendor/symfony/src/Symfony/Bridge/Swiftmailer/DataCollector/MessageDataCollector.php line 39

Thoughts?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/34#issuecomment-17416287
.

Cordialement,
Sébastien HOUZÉ

jeremyFreeAgent · 2013-05-03T22:26:18Z

Yes, you should update the bridge too. @bmeynell can you write your config please? I will take a look. Thanky.

bmeynell · 2013-05-06T16:08:42Z

Bridge updated! TY!

@jeremyFreeAgent - My config:

    mailers:                                                                       
        default:                                                                   
            encryption: ssl                                                        
            auth_mode:  login                                                      
            transport:  %mailer.transport%                                         
            host:       %mailer.host%                                              
            port:       %mailer.port%                                              
            username:   %mailer.user%                                              
            password:   %mailer.password%                                          
            antiflood:                                                             
                threshold:            100                                          
                sleep:                1                                            
        second:                                                                     
            encryption: ssl                                                        
            auth_mode:  login                                                      
            transport:  %mailer.transport%                                         
            host:       %mailer.host%                                              
            port:       %mailer.port%                                              
            username:   %second.mailer.user%                                        
            password:   %second.mailer.password%                                    
            antiflood:                                                             
                threshold:            100                                          
                sleep:                1

Error:

  [Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException]  
  The service "swiftmailer.mailer.default.pl                                    
  ugin.antiflood" has a dependency on a non-                                    
  existent parameter "swiftmailer.mailer.def                                    
  ault.antiflood.threshold".

jeremyFreeAgent · 2013-05-06T16:19:56Z

@bmeynell Fixed, typo in the parameter name. Sorry.

stof · 2013-05-09T10:03:24Z

Command/SendEmailCommand.php

@@ -35,6 +36,7 @@ protected function configure()
            ->addOption('message-limit', 0, InputOption::VALUE_OPTIONAL, 'The maximum number of messages to send.')
            ->addOption('time-limit', 0, InputOption::VALUE_OPTIONAL, 'The time limit for sending messages (in seconds).')
            ->addOption('recover-timeout', 0, InputOption::VALUE_OPTIONAL, 'The timeout for recovering messages that have taken too long to send (in seconds).')
+            ->addArgument('mailer', InputArgument::OPTIONAL, 'The mailer name.')


could you use an option rather than an optional argument ?

and the command help should be updated

jeremyFreeAgent · 2013-06-12T15:28:32Z

+2

fabpot · 2013-06-13T09:03:21Z

Command/DebugCommand.php

+            ->setHelp(<<<EOF
+The <info>%command.name%</info> displays the configured mailers:
+
+  <info>php %command.full_name%</info>


The name argument should be documented.

fabpot · 2013-06-13T09:12:17Z

Just to make sure: is this BC?

stof · 2013-06-13T09:21:15Z

@fabpot the user-facing services are BC. The only BC break is in the data collector for the profiler, which is why it should be kept in sync with the symfony PR, and why I suggested to move the collector to the bundle as it is not generic anyway (it relies on the container)

fabpot · 2013-06-13T09:29:19Z

ok, moving the data collector to the bundle is fine.

jeremyFreeAgent · 2013-07-08T13:35:46Z

Done ;)

This PR was merged into the master branch. Discussion ---------- Added mailers Now you can configure several mailers. Commits ------- cf5de43 Added mailers

fabpot · 2013-07-19T08:33:50Z

Merged now. Thank you very much to everyone who was involved in this long-running PR (especially @jeremyFreeAgent for his patience).

Anyone willing to update the documentation for this feature?

jeremyFreeAgent · 2013-07-19T08:38:20Z

Thanks @fabpot ! Can you give me a SensioLabsConnect "long PR badge" for that ? lol

This PR was squashed before being merged into the master branch (closes #42). Discussion ---------- fix OutOfBoundsException when using mail or sendmail transport Hi, Since the #34 had been merged, an exception occured when using mail or sendmail transport : ``` [Symfony\Component\DependencyInjection\Exception\OutOfBoundsException] The index "1" is not in the range [0, 0]. ``` This patch fix it. Commits ------- 01fb626 fix OutOfBoundsException when using mail or sendmail transport

…or (PR symfony#8520) This PR was merged into the master branch. Discussion ---------- [SwiftmailerBridge] Marked MessageDataCollector as deprecated | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | yes | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Now you can configure several mailers. Linked to the following PR: - [ ] symfony/swiftmailer-bundle#34 Commits ------- 15bf1d7 [SwiftmailerBridge] Marked MessageDataCollector as deprecated

lyrixx · 2015-01-19T18:41:54Z

DependencyInjection/Compiler/RegisterPluginsPass.php

            return;
        }

-        $definition = $container->findDefinition('swiftmailer.transport');
-        foreach ($container->findTaggedServiceIds('swiftmailer.plugin') as $id => $args) {


That is a BC break. ping @jeremyFreeAgent ;)

What about registering swiftmailer.plugin services to each transport?

I don't know swift mailer enough ...

It wasn't looking like a BC break at the time we did that. @lyrixx have you experienced any breakage?

Wep, my plugin is not registered anymore. Anyway, I just updated the tag name ;)

stof reviewed Apr 17, 2013
View reviewed changes

sstok reviewed Apr 19, 2013
View reviewed changes

stof reviewed May 9, 2013
View reviewed changes

fabpot reviewed Jun 13, 2013
View reviewed changes

Added mailers

cf5de43

fabpot added a commit that referenced this pull request Jul 19, 2013

merged branch jeremyFreeAgent/feature/mailers (PR #34)

69d6c36

This PR was merged into the master branch. Discussion ---------- Added mailers Now you can configure several mailers. Commits ------- cf5de43 Added mailers

fabpot merged commit cf5de43 into symfony:master Jul 19, 2013

jeremyFreeAgent mentioned this pull request Jul 19, 2013

[SwiftmailerBridge] Marked MessageDataCollector as deprecated symfony/symfony#8520

Merged

1 task

csanquer mentioned this pull request Jul 20, 2013

fix OutOfBoundsException when using mail or sendmail transport #42

Closed

smatyas mentioned this pull request Jul 26, 2013

Memory spooling is not working #43

Closed

acasademont mentioned this pull request Aug 9, 2013

Custom spooling broken in 2.3.3 #49

Closed

craue mentioned this pull request Aug 30, 2013

event listeners aren't called after updating from 2.3.2 to 2.3.3 #56

Closed

Koc mentioned this pull request Sep 2, 2013

Documentation outdated #58

Closed

Koc mentioned this pull request Jan 7, 2014

[Swiftmailer] Document mailers symfony/symfony-docs#3440

Closed

xabbuh mentioned this pull request Jan 11, 2014

document the ability to configure more than one mailer at the same time symfony/symfony-docs#3464

Merged

lyrixx reviewed Jan 19, 2015
View reviewed changes

Uh oh!

Added mailers #34

Added mailers #34

Uh oh!

Conversation

jeremyFreeAgent commented Apr 17, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stof commented Apr 17, 2013

Uh oh!

stof commented Apr 17, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremyFreeAgent commented Apr 29, 2013

Uh oh!

shouze commented Apr 30, 2013

Uh oh!

bmeynell commented May 2, 2013

Uh oh!

jeremyFreeAgent commented May 3, 2013

Uh oh!

shouze commented May 3, 2013

Uh oh!

bmeynell commented May 3, 2013

Uh oh!

jeremyFreeAgent commented May 3, 2013

Uh oh!

shouze commented May 3, 2013

Uh oh!

shouze commented May 3, 2013

Uh oh!

bmeynell commented May 3, 2013

Uh oh!

bmeynell commented May 3, 2013

Uh oh!

shouze commented May 3, 2013

Uh oh!

jeremyFreeAgent commented May 3, 2013

Uh oh!

bmeynell commented May 6, 2013

Uh oh!

jeremyFreeAgent commented May 6, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremyFreeAgent commented Jun 12, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabpot commented Jun 13, 2013

Uh oh!

stof commented Jun 13, 2013

Uh oh!

fabpot commented Jun 13, 2013

Uh oh!

jeremyFreeAgent commented Jul 8, 2013

Uh oh!

fabpot commented Jul 19, 2013

Uh oh!

jeremyFreeAgent commented Jul 19, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment