MAGETWO-67680: Replace Zend_Mail (ZF1) with Zend\Mail (ZF2) #8608

 - code review changes

Author: Oleksii Korshenko

app/code/Magento/Newsletter/Model/Template.php

@@ -38,6 +38,13 @@
  */
 class Template extends \Magento\Email\Model\AbstractTemplate
 {
+    /**
+     * Mail object
+     * 
+     * @deprecated Unused property
+     *
+     */
+    protected $_mail;
 
     /**
      * Store manager to emulate design

lib/internal/Magento/Framework/Mail/MailMessageInterface.php

@@ -13,19 +13,23 @@
 interface MailMessageInterface extends MessageInterface
 {
     /**
+     * Set mail message body in HTML format.
+     *
      * @param string $html
      * @return $this
      */
     public function setBodyHtml($html);
 
     /**
+     * Set mail message body in text format.
+     *
      * @param string $text
      * @return $this
      */
     public function setBodyText($text);
 
     /**
-     * Get message source code
+     * Get message source code.
      *
      * @return string
      */

lib/internal/Magento/Framework/Mail/Message.php

@@ -25,7 +25,7 @@ class Message implements MailMessageInterface
      *
      * @var string
      */
-    protected $messageType = self::TYPE_TEXT;
+    private $messageType = self::TYPE_TEXT;
 
     /**
      * Initialize dependencies.
@@ -68,8 +68,7 @@ public function setBody($body)
     }
 
     /**
-     * @param string $subject
-     * @return $this
+     * {@inheritdoc}
      */
     public function setSubject($subject)
     {
@@ -78,7 +77,7 @@ public function setSubject($subject)
     }
 
     /**
-     * @return null|string
+     * {@inheritdoc}
      */
     public function getSubject()
     {
@@ -94,8 +93,7 @@ public function getBody()
     }
 
     /**
-     * @param array|string $fromAddress
-     * @return $this
+     * {@inheritdoc}
      */
     public function setFrom($fromAddress)
     {
@@ -104,8 +102,7 @@ public function setFrom($fromAddress)
     }
 
     /**
-     * @param array|string $toAddress
-     * @return $this
+     * {@inheritdoc}
      */
     public function addTo($toAddress)
     {
@@ -114,8 +111,7 @@ public function addTo($toAddress)
     }
 
     /**
-     * @param array|string $ccAddress
-     * @return $this
+     * {@inheritdoc}
      */
     public function addCc($ccAddress)
     {
@@ -124,8 +120,7 @@ public function addCc($ccAddress)
     }
 
     /**
-     * @param array|string $bccAddress
-     * @return $this
+     * {@inheritdoc}
      */
     public function addBcc($bccAddress)
     {
@@ -134,8 +129,7 @@ public function addBcc($bccAddress)
     }
 
     /**
-     * @param array|string $replyToAddress
-     * @return $this
+     * {@inheritdoc}
      */
     public function setReplyTo($replyToAddress)
     {
@@ -144,14 +138,16 @@ public function setReplyTo($replyToAddress)
     }
 
     /**
-     * @return string
+     * {@inheritdoc}
      */
     public function getRawMessage()
     {
         return $this->zendMessage->toString();
     }
 
     /**
+     * Create HTML mime message from the string.
+     *
      * @param string $htmlBody
      * @return \Zend\Mime\Message
      */

lib/internal/Magento/Framework/Mail/MessageInterface.php

@@ -9,7 +9,7 @@
  * Mail Message interface
  *
  * @api
- * @deprecated
+ * @deprecated in favor of MailMessageInterface to avoid temporal coupling (setMessageType + setBody)
  * @see \Magento\Framework\Mail\MailMessageInterface
  */
 interface MessageInterface

lib/internal/Magento/Framework/Mail/Template/TransportBuilder.php

@@ -284,7 +284,6 @@ protected function prepareMessage()
                 throw new LocalizedException(
                     new Phrase('Unknown template type')
                 );
-            break;
         }
         $this->message->setSubject(html_entity_decode($template->getSubject(), ENT_QUOTES));
         return $this;

lib/internal/Magento/Framework/Mail/Test/Unit/MessageTest.php

@@ -8,87 +8,41 @@
 class MessageTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @var \PHPUnit_Framework_MockObject
+     * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Mail\Message
      */
     protected $_messageMock;
 
     protected function setUp()
     {
-        $this->markTestSkipped('obsolete ZF 1 test');
         $this->_messageMock = $this->getMock(
             \Magento\Framework\Mail\Message::class,
-            ['getBodyText', 'getBodyHtml', 'setBodyText', 'setBodyHtml']
+            ['setBody', 'setMessageType']
         );
     }
 
-    /**
-     * @param string $messageType
-     * @param string $method
-     *
-     * @covers \Magento\Framework\Mail\Message::setBody
-     * @covers \Magento\Framework\Mail\Message::setMessageType
-     * @dataProvider setBodyDataProvider
-     */
-    public function testSetBody($messageType, $method)
+    public function testSetBodyHtml()
     {
-        $this->_messageMock->setMessageType($messageType);
+        $this->_messageMock->expects($this->once())
+            ->method('setMessageType')
+            ->with('text/html');
 
         $this->_messageMock->expects($this->once())
-            ->method($method)
+            ->method('setBody')
             ->with('body');
 
-        $this->_messageMock->setBody('body');
+        $this->_messageMock->setBodyHtml('body');
     }
 
-    /**
-     * @return array
-     */
-    public function setBodyDataProvider()
+    public function testSetBodyText()
     {
-        return [
-            [
-                'messageType' => 'text/plain',
-                'method' => 'setBodyText',
-            ],
-            [
-                'messageType' => 'text/html',
-                'method' => 'setBodyHtml'
-            ]
-        ];
-    }
-
-    /**
-     * @param string $messageType
-     * @param string $method
-     *
-     * @covers \Magento\Framework\Mail\Message::getBody
-     * @covers \Magento\Framework\Mail\Message::setMessageType
-     * @dataProvider getBodyDataProvider
-     */
-    public function testGetBody($messageType, $method)
-    {
-        $this->_messageMock->setMessageType($messageType);
-
         $this->_messageMock->expects($this->once())
-            ->method($method);
+            ->method('setMessageType')
+            ->with('text/plain');
 
-        $this->_messageMock->getBody('body');
-    }
+        $this->_messageMock->expects($this->once())
+            ->method('setBody')
+            ->with('body');
 
-    /**
-     * @return array
-     */
-    public function getBodyDataProvider()
-    {
-        return [
-            [
-                'messageType' => 'text/plain',
-                'method' => 'getBodyText',
-            ],
-            [
-                'messageType' => 'text/html',
-                'method' => 'getBodyHtml'
-            ]
-        ];
+        $this->_messageMock->setBodyText('body');
     }
 }