Updated method used to determine whether emails are sent on creation …

…of invoices,

creditmemos and shipments from the admin page. Now takes into account global "enabled"
setting and the checkbox on creation page.

Updated Unit and Integration tests to suit.

app/code/Magento/Sales/Controller/Adminhtml/Order/Creditmemo/Save.php

@@ -7,6 +7,7 @@
 
 use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface;
 use Magento\Backend\App\Action;
+use Magento\Sales\Helper\Data as SalesData;
 use Magento\Sales\Model\Order;
 use Magento\Sales\Model\Order\Email\Sender\CreditmemoSender;
 
@@ -34,21 +35,30 @@ class Save extends \Magento\Backend\App\Action implements HttpPostActionInterfac
      */
     protected $resultForwardFactory;
 
+    /**
+     * @var SalesData
+     */
+    private $salesData;
+
     /**
      * @param Action\Context $context
      * @param \Magento\Sales\Controller\Adminhtml\Order\CreditmemoLoader $creditmemoLoader
      * @param CreditmemoSender $creditmemoSender
      * @param \Magento\Backend\Model\View\Result\ForwardFactory $resultForwardFactory
+     * @param SalesData $salesData
      */
     public function __construct(
         Action\Context $context,
         \Magento\Sales\Controller\Adminhtml\Order\CreditmemoLoader $creditmemoLoader,
         CreditmemoSender $creditmemoSender,
-        \Magento\Backend\Model\View\Result\ForwardFactory $resultForwardFactory
+        \Magento\Backend\Model\View\Result\ForwardFactory $resultForwardFactory,
+        SalesData $salesData = null
     ) {
         $this->creditmemoLoader = $creditmemoLoader;
         $this->creditmemoSender = $creditmemoSender;
         $this->resultForwardFactory = $resultForwardFactory;
+        $this->salesData = $salesData ?: \Magento\Framework\App\ObjectManager::getInstance()
+            ->get(SalesData::class);
         parent::__construct($context);
     }
 
@@ -108,7 +118,7 @@ public function execute()
                 $doOffline = isset($data['do_offline']) ? (bool)$data['do_offline'] : false;
                 $creditmemoManagement->refund($creditmemo, $doOffline);
 
-                if (!empty($data['send_email'])) {
+                if (!empty($data['send_email']) && $this->salesData->canSendNewCreditMemoEmail()) {
                     $this->creditmemoSender->send($creditmemo);
                 }
 

app/code/Magento/Sales/Controller/Adminhtml/Order/Invoice/Save.php

@@ -86,7 +86,8 @@ public function __construct(
         $this->shipmentFactory = $shipmentFactory;
         $this->invoiceService = $invoiceService;
         parent::__construct($context);
-        $this->salesData = $salesData ?? $this->_objectManager->get(SalesData::class);
+        $this->salesData = $salesData ?: \Magento\Framework\App\ObjectManager::getInstance()
+            ->get(SalesData::class);
     }
 
     /**
@@ -213,7 +214,7 @@ public function execute()
 
             // send invoice/shipment emails
             try {
-                if (!empty($data['send_email']) || $this->salesData->canSendNewInvoiceEmail()) {
+                if (!empty($data['send_email']) && $this->salesData->canSendNewInvoiceEmail()) {
                     $this->invoiceSender->send($invoice);
                 }
             } catch (\Exception $e) {
@@ -222,7 +223,7 @@ public function execute()
             }
             if ($shipment) {
                 try {
-                    if (!empty($data['send_email']) || $this->salesData->canSendNewShipmentEmail()) {
+                    if (!empty($data['send_email']) && $this->salesData->canSendNewShipmentEmail()) {
                         $this->shipmentSender->send($shipment);
                     }
                 } catch (\Exception $e) {

app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Creditmemo/SaveTest.php

@@ -21,9 +21,13 @@
 use Magento\Framework\Registry;
 use Magento\Framework\Session\Storage;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Sales\Api\CreditmemoManagementInterface;
 use Magento\Sales\Controller\Adminhtml\Order\Creditmemo\Save;
 use Magento\Sales\Controller\Adminhtml\Order\CreditmemoLoader;
+use Magento\Sales\Helper\Data as SalesData;
+use Magento\Sales\Model\Order;
 use Magento\Sales\Model\Order\Creditmemo;
+use Magento\Sales\Model\Order\Email\Sender\CreditmemoSender;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -87,6 +91,16 @@ class SaveTest extends TestCase
      */
     protected $resultRedirectMock;
 
+    /**
+     * @var CreditmemoSender|MockObject
+     */
+    private $creditmemoSender;
+
+    /**
+     * @var SalesData|MockObject
+     */
+    private $salesData;
+
     /**
      * Init model for future tests
      */
@@ -147,12 +161,32 @@ protected function setUp(): void
 
         $context = $helper->getObject(Context::class, $arguments);
 
+        $creditmemoManagement =  $this->getMockBuilder(CreditmemoManagementInterface::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->_objectManager->expects($this->any())
+            ->method('create')
+            ->with(CreditmemoManagementInterface::class)
+            ->willReturn($creditmemoManagement);
+        $this->creditmemoSender = $this->getMockBuilder(CreditMemoSender::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['send'])
+            ->getMock();
+        $this->creditmemoSender->expects($this->any())
+            ->method('send')
+            ->willReturn(true);
+        $this->salesData = $this->getMockBuilder(SalesData::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['canSendNewCreditmemoEmail'])
+            ->getMock();
         $this->memoLoaderMock = $this->createMock(CreditmemoLoader::class);
         $this->_controller = $helper->getObject(
             Save::class,
             [
                 'context' => $context,
                 'creditmemoLoader' => $this->memoLoaderMock,
+                'creditmemoSender' => $this->creditmemoSender,
+                'salesData' => $this->salesData
             ]
         );
     }
@@ -258,4 +292,94 @@ protected function _setSaveActionExpectationForMageCoreException($data, $errorMe
         $this->_messageManager->expects($this->once())->method('addErrorMessage')->with($errorMessage);
         $this->_sessionMock->expects($this->once())->method('setFormData')->with($data);
     }
+
+    /**
+     * @return array
+     */
+    public function testExecuteEmailsDataProvider()
+    {
+        /**
+        * string $sendEmail
+        * bool $emailEnabled
+        * bool $shouldEmailBeSent
+        */
+        return [
+            ['', false, false],
+            ['', true, false],
+            ['on', false, false],
+            ['on', true, true]
+        ];
+    }
+
+    /**
+     * @param string $sendEmail
+     * @param bool $emailEnabled
+     * @param bool $shouldEmailBeSent
+     * @dataProvider testExecuteEmailsDataProvider
+     */
+    public function testExecuteEmails(
+        $sendEmail,
+        $emailEnabled,
+        $shouldEmailBeSent
+    ) {
+        $orderId = 1;
+        $creditmemoId = 2;
+        $invoiceId = 3;
+        $creditmemoData = ['items' => [], 'send_email' => $sendEmail];
+
+        $this->resultRedirectFactoryMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->resultRedirectMock);
+        $this->resultRedirectMock->expects($this->once())
+            ->method('setPath')
+            ->with('sales/order/view', ['order_id' => $orderId])
+            ->willReturnSelf();
+
+        $order = $this->createPartialMock(
+            Order::class,
+            []
+        );
+
+        $creditmemo = $this->createPartialMock(
+            Creditmemo::class,
+            ['isValidGrandTotal', 'getOrder', 'getOrderId']
+        );
+        $creditmemo->expects($this->once())
+            ->method('isValidGrandTotal')
+            ->willReturn(true);
+        $creditmemo->expects($this->once())
+            ->method('getOrder')
+            ->willReturn($order);
+        $creditmemo->expects($this->once())
+            ->method('getOrderId')
+            ->willReturn($orderId);
+
+        $this->_requestMock->expects($this->any())
+            ->method('getParam')
+            ->willReturnMap(
+                [
+                    ['order_id', null, $orderId],
+                    ['creditmemo_id', null, $creditmemoId],
+                    ['creditmemo', null, $creditmemoData],
+                    ['invoice_id', null, $invoiceId]
+                ]
+            );
+
+        $this->_requestMock->expects($this->any())
+            ->method('getPost')
+            ->willReturn($creditmemoData);
+
+        $this->memoLoaderMock->expects($this->once())
+            ->method('load')
+            ->willReturn($creditmemo);
+
+        $this->salesData->expects($this->any())
+            ->method('canSendNewCreditmemoEmail')
+            ->willReturn($emailEnabled);
+        if ($shouldEmailBeSent) {
+            $this->creditmemoSender->expects($this->once())
+                ->method('send');
+        }
+        $this->assertEquals($this->resultRedirectMock, $this->_controller->execute());
+    }
 }