ENGCOM-3968: [Backport][2.2] Alternative fix for Multi Store Emails issue, Fix Async Emails issues, Fix Multiple Email issues #18472

sivaschenko · web-flow · commit dbfeb36a1f95 · 2019-03-04T12:36:02.000Z
diff --git a/app/code/Magento/Sales/Model/Order/Email/SenderBuilder.php b/app/code/Magento/Sales/Model/Order/Email/SenderBuilder.php
@@ -5,7 +5,6 @@
  */
 namespace Magento\Sales\Model\Order\Email;
 
-use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Mail\Template\TransportBuilder;
 use Magento\Framework\Mail\Template\TransportBuilderByStore;
 use Magento\Sales\Model\Order\Email\Container\IdentityInterface;
@@ -29,11 +28,8 @@ class SenderBuilder
     protected $transportBuilder;
 
     /**
-     * @var TransportBuilderByStore
-     */
-    private $transportBuilderByStore;
-
-    /**
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     *
      * @param Template $templateContainer
      * @param IdentityInterface $identityContainer
      * @param TransportBuilder $transportBuilder
@@ -48,9 +44,6 @@ public function __construct(
         $this->templateContainer = $templateContainer;
         $this->identityContainer = $identityContainer;
         $this->transportBuilder = $transportBuilder;
-        $this->transportBuilderByStore = $transportBuilderByStore ?: ObjectManager::getInstance()->get(
-            TransportBuilderByStore::class
-        );
     }
 
     /**
@@ -110,7 +103,7 @@ protected function configureEmailTemplate()
         $this->transportBuilder->setTemplateIdentifier($this->templateContainer->getTemplateId());
         $this->transportBuilder->setTemplateOptions($this->templateContainer->getTemplateOptions());
         $this->transportBuilder->setTemplateVars($this->templateContainer->getTemplateVars());
-        $this->transportBuilderByStore->setFromByStore(
+        $this->transportBuilder->setFromByScope(
             $this->identityContainer->getEmailIdentity(),
             $this->identityContainer->getStore()->getId()
         );
diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/Email/SenderBuilderTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/Email/SenderBuilderTest.php
@@ -6,7 +6,6 @@
 
 namespace Magento\Sales\Test\Unit\Model\Order\Email;
 
-use Magento\Framework\Mail\Template\TransportBuilderByStore;
 use Magento\Sales\Model\Order\Email\SenderBuilder;
 
 class SenderBuilderTest extends \PHPUnit\Framework\TestCase
@@ -36,11 +35,6 @@ class SenderBuilderTest extends \PHPUnit\Framework\TestCase
      */
     private $storeMock;
 
-    /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
-     */
-    private $transportBuilderByStore;
-
     protected function setUp()
     {
         $templateId = 'test_template_id';
@@ -82,11 +76,10 @@ protected function setUp()
                 'setTemplateIdentifier',
                 'setTemplateOptions',
                 'setTemplateVars',
+                'setFromByScope',
             ]
         );
 
-        $this->transportBuilderByStore = $this->createMock(TransportBuilderByStore::class);
-
         $this->templateContainerMock->expects($this->once())
             ->method('getTemplateId')
             ->will($this->returnValue($templateId));
@@ -109,9 +102,9 @@ protected function setUp()
         $this->identityContainerMock->expects($this->once())
             ->method('getEmailIdentity')
             ->will($this->returnValue($emailIdentity));
-        $this->transportBuilderByStore->expects($this->once())
-            ->method('setFromByStore')
-            ->with($this->equalTo($emailIdentity));
+        $this->transportBuilder->expects($this->once())
+            ->method('setFromByScope')
+            ->with($this->equalTo($emailIdentity), 1);
 
         $this->identityContainerMock->expects($this->once())
             ->method('getEmailCopyTo')
@@ -120,15 +113,16 @@ protected function setUp()
         $this->senderBuilder = new SenderBuilder(
             $this->templateContainerMock,
             $this->identityContainerMock,
-            $this->transportBuilder,
-            $this->transportBuilderByStore
+            $this->transportBuilder
         );
     }
 
     public function testSend()
     {
         $customerName = 'test_name';
         $customerEmail = 'test_email';
+        $identity = 'email_identity_test';
+
         $transportMock = $this->createMock(
             \Magento\Sales\Test\Unit\Model\Order\Email\Stub\TransportInterfaceMock::class
         );
@@ -151,6 +145,9 @@ public function testSend()
         $this->storeMock->expects($this->once())
             ->method('getId')
             ->willReturn(1);
+        $this->transportBuilder->expects($this->once())
+            ->method('setFromByScope')
+            ->with($identity, 1);
         $this->transportBuilder->expects($this->once())
             ->method('addTo')
             ->with($this->equalTo($customerEmail), $this->equalTo($customerName));
@@ -164,6 +161,7 @@ public function testSend()
 
     public function testSendCopyTo()
     {
+        $identity = 'email_identity_test';
         $transportMock = $this->createMock(
             \Magento\Sales\Test\Unit\Model\Order\Email\Stub\TransportInterfaceMock::class
         );
@@ -177,6 +175,9 @@ public function testSendCopyTo()
         $this->transportBuilder->expects($this->once())
             ->method('addTo')
             ->with($this->equalTo('example@mail.com'));
+        $this->transportBuilder->expects($this->once())
+            ->method('setFromByScope')
+            ->with($identity, 1);
         $this->identityContainerMock->expects($this->once())
             ->method('getStore')
             ->willReturn($this->storeMock);
diff --git a/lib/internal/Magento/Framework/Mail/Message.php b/lib/internal/Magento/Framework/Mail/Message.php
@@ -89,10 +89,23 @@ public function getBody()
 
     /**
      * @inheritdoc
+     *
+     * @deprecated This function is missing the from name. The
+     * setFromAddress() function sets both from address and from name.
+     * @see setFromAddress()
      */
     public function setFrom($fromAddress)
     {
-        $this->zendMessage->setFrom($fromAddress);
+        $this->setFromAddress($fromAddress, null);
+        return $this;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function setFromAddress($fromAddress, $fromName = null)
+    {
+        $this->zendMessage->setFrom($fromAddress, $fromName);
         return $this;
     }
 
diff --git a/lib/internal/Magento/Framework/Mail/Template/TransportBuilder.php b/lib/internal/Magento/Framework/Mail/Template/TransportBuilder.php
@@ -48,13 +48,6 @@ class TransportBuilder
      */
     protected $templateOptions;
 
-    /**
-     * Mail from address
-     *
-     * @var string|array
-     */
-    private $from;
-
     /**
      * Mail Transport
      *
@@ -180,12 +173,29 @@ public function setReplyTo($email, $name = null)
     /**
      * Set mail from address
      *
+     * @deprecated This function sets the from address but does not provide
+     * a way of setting the correct from addresses based on the scope.
+     * @see setFromByScope()
+     *
      * @param string|array $from
      * @return $this
      */
     public function setFrom($from)
     {
-        $this->from = $from;
+        return $this->setFromByScope($from, null);
+    }
+
+    /**
+     * Set mail from address by Scope
+     *
+     * @param string|array $from
+     * @param string|int $scopeId
+     * @return $this
+     */
+    public function setFromByScope($from, $scopeId = null)
+    {
+        $result = $this->_senderResolver->resolve($from, $scopeId);
+        $this->message->setFromAddress($result['email'], $result['name']);
         return $this;
     }
 
@@ -262,7 +272,6 @@ protected function reset()
         $this->templateIdentifier = null;
         $this->templateVars = null;
         $this->templateOptions = null;
-        $this->from = null;
         return $this;
     }
 
@@ -296,14 +305,6 @@ protected function prepareMessage()
             ->setBody($body)
             ->setSubject(html_entity_decode($template->getSubject(), ENT_QUOTES));
 
-        if ($this->from) {
-            $from = $this->_senderResolver->resolve(
-                $this->from,
-                $template->getDesignConfig()->getStore()
-            );
-            $this->message->setFrom($from['email'], $from['name']);
-        }
-
         return $this;
     }
 }
diff --git a/lib/internal/Magento/Framework/Mail/Template/TransportBuilderByStore.php b/lib/internal/Magento/Framework/Mail/Template/TransportBuilderByStore.php
@@ -8,6 +8,13 @@
 
 use Magento\Framework\Mail\MessageInterface;
 
+/**
+ * Class TransportBuilderByStore
+ *
+ * @deprecated The ability to set From address based on store is now available
+ * in the \Magento\Framework\Mail\Template\TransportBuilder class
+ * @see \Magento\Framework\Mail\Template\TransportBuilder::setFromByScope
+ */
 class TransportBuilderByStore
 {
     /**
diff --git a/lib/internal/Magento/Framework/Mail/Test/Unit/Template/TransportBuilderTest.php b/lib/internal/Magento/Framework/Mail/Test/Unit/Template/TransportBuilderTest.php
@@ -7,7 +7,6 @@
 namespace Magento\Framework\Mail\Test\Unit\Template;
 
 use Magento\Framework\App\TemplateTypesInterface;
-use Magento\Framework\DataObject;
 use Magento\Framework\Mail\MessageInterface;
 
 /**
@@ -100,37 +99,17 @@ protected function setUp()
      */
     public function testGetTransport($templateType, $messageType, $bodyText, $templateNamespace)
     {
+        $this->builder->setTemplateModel($templateNamespace);
+        
         $vars = ['reason' => 'Reason', 'customer' => 'Customer'];
         $options = ['area' => 'frontend', 'store' => 1];
-        $from = 'email_from';
-        $sender = ['email' => 'from@example.com', 'name' => 'name'];
-
-        $this->builder->setTemplateModel($templateNamespace);
-        $this->builder->setFrom($from);
 
-        $template = $this->createPartialMock(
-            \Magento\Framework\Mail\TemplateInterface::class,
-            [
-                'setVars',
-                'isPlain',
-                'setOptions',
-                'getSubject',
-                'getType',
-                'processTemplate',
-                'getDesignConfig',
-            ]
-        );
+        $template = $this->createMock(\Magento\Framework\Mail\TemplateInterface::class);
         $template->expects($this->once())->method('setVars')->with($this->equalTo($vars))->willReturnSelf();
         $template->expects($this->once())->method('setOptions')->with($this->equalTo($options))->willReturnSelf();
         $template->expects($this->once())->method('getSubject')->willReturn('Email Subject');
         $template->expects($this->once())->method('getType')->willReturn($templateType);
         $template->expects($this->once())->method('processTemplate')->willReturn($bodyText);
-        $template->method('getDesignConfig')->willReturn(new DataObject($options));
-
-        $this->senderResolverMock->expects($this->once())
-            ->method('resolve')
-            ->with($from, 1)
-            ->willReturn($sender);
 
         $this->templateFactoryMock->expects($this->once())
             ->method('get')
@@ -149,9 +128,6 @@ public function testGetTransport($templateType, $messageType, $bodyText, $templa
             ->method('setBody')
             ->with($this->equalTo($bodyText))
             ->willReturnSelf();
-        $this->messageMock->method('setFrom')
-            ->with($sender['email'], $sender['name'])
-            ->willReturnSelf();
 
         $transport = $this->createMock(\Magento\Framework\Mail\TransportInterface::class);
 
@@ -184,6 +160,25 @@ public function getTransportDataProvider()
             ]
         ];
     }
+    
+    /**
+     * @return void
+     */
+    public function testSetFromByScope()
+    {
+        $sender = ['email' => 'from@example.com', 'name' => 'name'];
+        $scopeId = 1;
+        $this->senderResolverMock->expects($this->once())
+            ->method('resolve')
+            ->with($sender, $scopeId)
+            ->willReturn($sender);
+        $this->messageMock->expects($this->once())
+            ->method('setFromAddress')
+            ->with('from@example.com', 'name')
+            ->willReturnSelf();
+
+        $this->builder->setFromByScope($sender, $scopeId);
+    }
 
     /**
      * @return void
