From d5b0ffdd32ccc0d6ed23b41b33617958a659fc75 Mon Sep 17 00:00:00 2001 From: NazarKlovanych Date: Thu, 6 Dec 2018 16:42:01 +0200 Subject: [PATCH 1/8] fix issue-19596 Images in XML sitemap --- app/code/Magento/Sitemap/Model/Observer.php | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index a536ec998b827..54b36ddaa278b 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -5,6 +5,8 @@ */ namespace Magento\Sitemap\Model; +use Magento\Store\Model\App\Emulation; +use Magento\Framework\App\ObjectManager; /** * Sitemap module observer * @@ -67,13 +69,21 @@ class Observer protected $inlineTranslation; /** + * @var \Magento\Store\Model\App\Emulation $appEmulation + */ + protected $appEmulation; + + /** + * Observer constructor. + * @param Emulation|null $appEmulation * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig - * @param \Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory $collectionFactory + * @param ResourceModel\Sitemap\CollectionFactory $collectionFactory * @param \Magento\Store\Model\StoreManagerInterface $storeManager * @param \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder * @param \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation */ public function __construct( + Emulation $appEmulation = null, \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, \Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory $collectionFactory, \Magento\Store\Model\StoreManagerInterface $storeManager, @@ -85,6 +95,8 @@ public function __construct( $this->_storeManager = $storeManager; $this->_transportBuilder = $transportBuilder; $this->inlineTranslation = $inlineTranslation; + $this->appEmulation = $appEmulation ?: ObjectManager::getInstance() + ->get(\Magento\Store\Model\App\Emulation::class); } /** @@ -112,10 +124,20 @@ public function scheduledGenerateSitemaps() foreach ($collection as $sitemap) { /* @var $sitemap \Magento\Sitemap\Model\Sitemap */ try { + + $this->appEmulation->startEnvironmentEmulation( + $sitemap->getStoreId(), + \Magento\Framework\App\Area::AREA_FRONTEND, + true + ); + $sitemap->generateXml(); } catch (\Exception $e) { $errors[] = $e->getMessage(); + } finally { + $this->appEmulation->stopEnvironmentEmulation(); } + } if ($errors && $this->_scopeConfig->getValue( From 16f11f760b89bb240ce2f6948214da5775695d92 Mon Sep 17 00:00:00 2001 From: NazarKlovanych Date: Fri, 7 Dec 2018 14:11:16 +0200 Subject: [PATCH 2/8] Fix failed static tests --- app/code/Magento/Sitemap/Model/Observer.php | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index 54b36ddaa278b..62d7a5ab124e5 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -6,7 +6,7 @@ namespace Magento\Sitemap\Model; use Magento\Store\Model\App\Emulation; -use Magento\Framework\App\ObjectManager; + /** * Sitemap module observer * @@ -71,32 +71,31 @@ class Observer /** * @var \Magento\Store\Model\App\Emulation $appEmulation */ - protected $appEmulation; + private $appEmulation; /** * Observer constructor. - * @param Emulation|null $appEmulation * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig * @param ResourceModel\Sitemap\CollectionFactory $collectionFactory * @param \Magento\Store\Model\StoreManagerInterface $storeManager * @param \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder * @param \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation + * @param Emulation|null $appEmulation */ public function __construct( - Emulation $appEmulation = null, \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, - \Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory $collectionFactory, + ResourceModel\Sitemap\CollectionFactory $collectionFactory, \Magento\Store\Model\StoreManagerInterface $storeManager, \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder, - \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation + \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation, + Emulation $appEmulation ) { $this->_scopeConfig = $scopeConfig; $this->_collectionFactory = $collectionFactory; $this->_storeManager = $storeManager; $this->_transportBuilder = $transportBuilder; $this->inlineTranslation = $inlineTranslation; - $this->appEmulation = $appEmulation ?: ObjectManager::getInstance() - ->get(\Magento\Store\Model\App\Emulation::class); + $this->appEmulation = $appEmulation; } /** @@ -124,10 +123,9 @@ public function scheduledGenerateSitemaps() foreach ($collection as $sitemap) { /* @var $sitemap \Magento\Sitemap\Model\Sitemap */ try { - $this->appEmulation->startEnvironmentEmulation( $sitemap->getStoreId(), - \Magento\Framework\App\Area::AREA_FRONTEND, + 'frontend', true ); @@ -137,7 +135,6 @@ public function scheduledGenerateSitemaps() } finally { $this->appEmulation->stopEnvironmentEmulation(); } - } if ($errors && $this->_scopeConfig->getValue( From c8afe9f71d99367bcdd24e4900e60bbba2fbc0dd Mon Sep 17 00:00:00 2001 From: Sergii Ivashchenko Date: Fri, 7 Dec 2018 15:45:16 +0200 Subject: [PATCH 3/8] Update app/code/Magento/Sitemap/Model/Observer.php Co-Authored-By: Nazar65 --- app/code/Magento/Sitemap/Model/Observer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index 62d7a5ab124e5..7211db4f171f6 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -69,7 +69,7 @@ class Observer protected $inlineTranslation; /** - * @var \Magento\Store\Model\App\Emulation $appEmulation + * @var Emulation */ private $appEmulation; From fda07a290af211356db74eb0cb890dc4bf42a388 Mon Sep 17 00:00:00 2001 From: NazarKlovanych Date: Tue, 11 Dec 2018 13:54:44 +0200 Subject: [PATCH 4/8] Refactoring Sitemap Observer --- app/code/Magento/Sitemap/Model/Observer.php | 98 ++++------------ .../Sitemap/Model/SitemapSendEmail.php | 109 ++++++++++++++++++ 2 files changed, 132 insertions(+), 75 deletions(-) create mode 100644 app/code/Magento/Sitemap/Model/SitemapSendEmail.php diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index 62d7a5ab124e5..c12db1498d699 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -6,6 +6,11 @@ namespace Magento\Sitemap\Model; use Magento\Store\Model\App\Emulation; +use Magento\Sitemap\Model\SitemapSendEmail as SitemapEmail; +use Magento\Store\Model\StoreManagerInterface; +use Magento\Framework\App\Config\ScopeConfigInterface; +use ResourceModel\Sitemap\CollectionFactory; +use Magento\Store\Model\ScopeInterface; /** * Sitemap module observer @@ -26,21 +31,6 @@ class Observer */ const XML_PATH_CRON_EXPR = 'crontab/default/jobs/generate_sitemaps/schedule/cron_expr'; - /** - * Error email template configuration - */ - const XML_PATH_ERROR_TEMPLATE = 'sitemap/generate/error_email_template'; - - /** - * Error email identity configuration - */ - const XML_PATH_ERROR_IDENTITY = 'sitemap/generate/error_email_identity'; - - /** - * 'Send error emails to' configuration - */ - const XML_PATH_ERROR_RECIPIENT = 'sitemap/generate/error_email'; - /** * Core store config * @@ -53,49 +43,41 @@ class Observer */ protected $_collectionFactory; - /** - * @var \Magento\Framework\Mail\Template\TransportBuilder - */ - protected $_transportBuilder; - /** * @var \Magento\Store\Model\StoreManagerInterface */ protected $_storeManager; /** - * @var \Magento\Framework\Translate\Inline\StateInterface + * @var Emulation */ - protected $inlineTranslation; + private $appEmulation; /** - * @var \Magento\Store\Model\App\Emulation $appEmulation + * @var $sitemapEmail */ - private $appEmulation; + private $sitemapEmail; /** * Observer constructor. - * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig - * @param ResourceModel\Sitemap\CollectionFactory $collectionFactory - * @param \Magento\Store\Model\StoreManagerInterface $storeManager - * @param \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder - * @param \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation - * @param Emulation|null $appEmulation + * @param ScopeConfigInterface $scopeConfig + * @param CollectionFactory $collectionFactory + * @param StoreManagerInterface $storeManager + * @param SitemapSendEmail $sitemapEmail + * @param Emulation $appEmulation */ public function __construct( - \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, - ResourceModel\Sitemap\CollectionFactory $collectionFactory, - \Magento\Store\Model\StoreManagerInterface $storeManager, - \Magento\Framework\Mail\Template\TransportBuilder $transportBuilder, - \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation, + ScopeConfigInterface $scopeConfig, + CollectionFactory $collectionFactory, + StoreManagerInterface $storeManager, + SitemapEmail $sitemapEmail, Emulation $appEmulation ) { $this->_scopeConfig = $scopeConfig; $this->_collectionFactory = $collectionFactory; $this->_storeManager = $storeManager; - $this->_transportBuilder = $transportBuilder; - $this->inlineTranslation = $inlineTranslation; $this->appEmulation = $appEmulation; + $this->sitemapEmail = $sitemapEmail; } /** @@ -112,7 +94,7 @@ public function scheduledGenerateSitemaps() // check if scheduled generation enabled if (!$this->_scopeConfig->isSetFlag( self::XML_PATH_GENERATION_ENABLED, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE + ScopeInterface::SCOPE_STORE ) ) { return; @@ -125,52 +107,18 @@ public function scheduledGenerateSitemaps() try { $this->appEmulation->startEnvironmentEmulation( $sitemap->getStoreId(), - 'frontend', + \Magento\Framework\App\Area::AREA_FRONTEND, true ); $sitemap->generateXml(); } catch (\Exception $e) { $errors[] = $e->getMessage(); + + $this->sitemapEmail->sendErrorEmail($errors); } finally { $this->appEmulation->stopEnvironmentEmulation(); } } - - if ($errors && $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_RECIPIENT, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) - ) { - $this->inlineTranslation->suspend(); - - $this->_transportBuilder->setTemplateIdentifier( - $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_TEMPLATE, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) - )->setTemplateOptions( - [ - 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, - ] - )->setTemplateVars( - ['warnings' => join("\n", $errors)] - )->setFrom( - $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_IDENTITY, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) - )->addTo( - $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_RECIPIENT, - \Magento\Store\Model\ScopeInterface::SCOPE_STORE - ) - ); - $transport = $this->_transportBuilder->getTransport(); - $transport->sendMessage(); - - $this->inlineTranslation->resume(); - } } } diff --git a/app/code/Magento/Sitemap/Model/SitemapSendEmail.php b/app/code/Magento/Sitemap/Model/SitemapSendEmail.php new file mode 100644 index 0000000000000..cf450eba6e1e4 --- /dev/null +++ b/app/code/Magento/Sitemap/Model/SitemapSendEmail.php @@ -0,0 +1,109 @@ +inlineTranslation = $inlineTranslation; + $this->_scopeConfig = $scopeConfig; + $this->_transportBuilder = $transportBuilder; + } + + /** + * Send's error email if sitemap generated with errors. + * + * @param array $errors + * @throws \Magento\Framework\Exception\MailException + */ + public function sendErrorEmail($errors) + { + $recipient = $this->_scopeConfig->getValue( + self::XML_PATH_ERROR_RECIPIENT, + ScopeInterface::SCOPE_STORE + ); + if ($errors && $recipient) { + $this->inlineTranslation->suspend(); + + $this->_transportBuilder->setTemplateIdentifier( + $this->_scopeConfig->getValue( + self::XML_PATH_ERROR_TEMPLATE, + ScopeInterface::SCOPE_STORE + ) + )->setTemplateOptions( + [ + 'area' => FrontNameResolver::AREA_CODE, + 'store' => Store::DEFAULT_STORE_ID, + ] + )->setTemplateVars( + ['warnings' => join("\n", $errors)] + )->setFrom( + $this->_scopeConfig->getValue( + self::XML_PATH_ERROR_IDENTITY, + ScopeInterface::SCOPE_STORE + ) + )->addTo($recipient); + + $transport = $this->_transportBuilder->getTransport(); + $transport->sendMessage(); + + $this->inlineTranslation->resume(); + } + } +} From d2d04b6ee9df35b9906f16459bdee1bba0db658d Mon Sep 17 00:00:00 2001 From: NazarKlovanych Date: Tue, 11 Dec 2018 13:58:05 +0200 Subject: [PATCH 5/8] Refactoring Sitemap Observer --- app/code/Magento/Sitemap/Model/Observer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index c12db1498d699..10bb95b8029cf 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -9,7 +9,7 @@ use Magento\Sitemap\Model\SitemapSendEmail as SitemapEmail; use Magento\Store\Model\StoreManagerInterface; use Magento\Framework\App\Config\ScopeConfigInterface; -use ResourceModel\Sitemap\CollectionFactory; +use Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory; use Magento\Store\Model\ScopeInterface; /** From b88fa48da750a1f7be759b37d9007bf2ecdc5270 Mon Sep 17 00:00:00 2001 From: NazarKlovanych Date: Wed, 12 Dec 2018 14:41:44 +0200 Subject: [PATCH 6/8] Refactoring --- ...mapSendEmail.php => EmailNotification.php} | 80 +++++++++---------- app/code/Magento/Sitemap/Model/Observer.php | 53 ++++++++---- 2 files changed, 74 insertions(+), 59 deletions(-) rename app/code/Magento/Sitemap/Model/{SitemapSendEmail.php => EmailNotification.php} (53%) diff --git a/app/code/Magento/Sitemap/Model/SitemapSendEmail.php b/app/code/Magento/Sitemap/Model/EmailNotification.php similarity index 53% rename from app/code/Magento/Sitemap/Model/SitemapSendEmail.php rename to app/code/Magento/Sitemap/Model/EmailNotification.php index cf450eba6e1e4..515121de6e6e1 100644 --- a/app/code/Magento/Sitemap/Model/SitemapSendEmail.php +++ b/app/code/Magento/Sitemap/Model/EmailNotification.php @@ -3,6 +3,7 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types = 1); namespace Magento\Sitemap\Model; @@ -11,28 +12,14 @@ use Magento\Framework\Mail\Template\TransportBuilder; use Magento\Store\Model\ScopeInterface; use Magento\Backend\App\Area\FrontNameResolver; -use Magento\Store\Model\Store; +use Magento\Sitemap\Model\Observer as Observer; +use Psr\Log\LoggerInterface; /** * Sends emails for the scheduled generation of the sitemap file */ -class SitemapSendEmail +class EmailNotification { - /** - * Error email template configuration - */ - const XML_PATH_ERROR_TEMPLATE = 'sitemap/generate/error_email_template'; - - /** - * Error email identity configuration - */ - const XML_PATH_ERROR_IDENTITY = 'sitemap/generate/error_email_identity'; - - /** - * 'Send error emails to' configuration - */ - const XML_PATH_ERROR_RECIPIENT = 'sitemap/generate/error_email'; - /** * @var \Magento\Framework\Translate\Inline\StateInterface */ @@ -43,66 +30,75 @@ class SitemapSendEmail * * @var \Magento\Framework\App\Config\ScopeConfigInterface */ - private $_scopeConfig; + private $scopeConfig; /** * @var \Magento\Framework\Mail\Template\TransportBuilder */ - private $_transportBuilder; + private $transportBuilder; + + /** + * @var $logger + */ + private $logger; /** - * SitemapSendEmail constructor. + * EmailNotification constructor. * @param StateInterface $inlineTranslation * @param TransportBuilder $transportBuilder * @param ScopeConfigInterface $scopeConfig + * @param LoggerInterface $logger */ public function __construct( StateInterface $inlineTranslation, TransportBuilder $transportBuilder, - ScopeConfigInterface $scopeConfig + ScopeConfigInterface $scopeConfig, + LoggerInterface $logger ) { $this->inlineTranslation = $inlineTranslation; - $this->_scopeConfig = $scopeConfig; - $this->_transportBuilder = $transportBuilder; + $this->scopeConfig = $scopeConfig; + $this->transportBuilder = $transportBuilder; + $this->logger = $logger; } /** * Send's error email if sitemap generated with errors. * - * @param array $errors - * @throws \Magento\Framework\Exception\MailException + * @param array| $errors */ - public function sendErrorEmail($errors) + public function sendErrors($errors) { - $recipient = $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_RECIPIENT, - ScopeInterface::SCOPE_STORE - ); - if ($errors && $recipient) { - $this->inlineTranslation->suspend(); - - $this->_transportBuilder->setTemplateIdentifier( - $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_TEMPLATE, + $this->inlineTranslation->suspend(); + try { + $this->transportBuilder->setTemplateIdentifier( + $this->scopeConfig->getValue( + Observer::XML_PATH_ERROR_TEMPLATE, ScopeInterface::SCOPE_STORE ) )->setTemplateOptions( [ 'area' => FrontNameResolver::AREA_CODE, - 'store' => Store::DEFAULT_STORE_ID, + 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, ] )->setTemplateVars( ['warnings' => join("\n", $errors)] )->setFrom( - $this->_scopeConfig->getValue( - self::XML_PATH_ERROR_IDENTITY, + $this->scopeConfig->getValue( + Observer::XML_PATH_ERROR_IDENTITY, + ScopeInterface::SCOPE_STORE + ) + )->addTo( + $this->scopeConfig->getValue( + Observer::XML_PATH_ERROR_RECIPIENT, ScopeInterface::SCOPE_STORE ) - )->addTo($recipient); + ); - $transport = $this->_transportBuilder->getTransport(); + $transport = $this->transportBuilder->getTransport(); $transport->sendMessage(); - + } catch (\Exception $e) { + $this->logger->error('Sitemap sendErrors: '.$e->getMessage()); + } finally { $this->inlineTranslation->resume(); } } diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index 10bb95b8029cf..887b5a6541a56 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -6,7 +6,7 @@ namespace Magento\Sitemap\Model; use Magento\Store\Model\App\Emulation; -use Magento\Sitemap\Model\SitemapSendEmail as SitemapEmail; +use Magento\Sitemap\Model\EmailNotification as SitemapEmail; use Magento\Store\Model\StoreManagerInterface; use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory; @@ -31,22 +31,37 @@ class Observer */ const XML_PATH_CRON_EXPR = 'crontab/default/jobs/generate_sitemaps/schedule/cron_expr'; + /** + * Error email template configuration + */ + const XML_PATH_ERROR_TEMPLATE = 'sitemap/generate/error_email_template'; + + /** + * Error email identity configuration + */ + const XML_PATH_ERROR_IDENTITY = 'sitemap/generate/error_email_identity'; + + /** + * 'Send error emails to' configuration + */ + const XML_PATH_ERROR_RECIPIENT = 'sitemap/generate/error_email'; + /** * Core store config * * @var \Magento\Framework\App\Config\ScopeConfigInterface */ - protected $_scopeConfig; + private $scopeConfig; /** * @var \Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory */ - protected $_collectionFactory; + private $collectionFactory; /** * @var \Magento\Store\Model\StoreManagerInterface */ - protected $_storeManager; + private $storeManager; /** * @var Emulation @@ -54,30 +69,30 @@ class Observer private $appEmulation; /** - * @var $sitemapEmail + * @var $emailNotification */ - private $sitemapEmail; + private $emailNotification; /** * Observer constructor. * @param ScopeConfigInterface $scopeConfig * @param CollectionFactory $collectionFactory * @param StoreManagerInterface $storeManager - * @param SitemapSendEmail $sitemapEmail + * @param EmailNotification $emailNotification * @param Emulation $appEmulation */ public function __construct( ScopeConfigInterface $scopeConfig, CollectionFactory $collectionFactory, StoreManagerInterface $storeManager, - SitemapEmail $sitemapEmail, + SitemapEmail $emailNotification, Emulation $appEmulation ) { - $this->_scopeConfig = $scopeConfig; - $this->_collectionFactory = $collectionFactory; - $this->_storeManager = $storeManager; + $this->scopeConfig = $scopeConfig; + $this->collectionFactory = $collectionFactory; + $this->storeManager = $storeManager; $this->appEmulation = $appEmulation; - $this->sitemapEmail = $sitemapEmail; + $this->emailNotification = $emailNotification; } /** @@ -90,9 +105,12 @@ public function __construct( public function scheduledGenerateSitemaps() { $errors = []; - + $recipient = $this->scopeConfig->getValue( + Observer::XML_PATH_ERROR_RECIPIENT, + ScopeInterface::SCOPE_STORE + ); // check if scheduled generation enabled - if (!$this->_scopeConfig->isSetFlag( + if (!$this->scopeConfig->isSetFlag( self::XML_PATH_GENERATION_ENABLED, ScopeInterface::SCOPE_STORE ) @@ -100,7 +118,7 @@ public function scheduledGenerateSitemaps() return; } - $collection = $this->_collectionFactory->create(); + $collection = $this->collectionFactory->create(); /* @var $collection \Magento\Sitemap\Model\ResourceModel\Sitemap\Collection */ foreach ($collection as $sitemap) { /* @var $sitemap \Magento\Sitemap\Model\Sitemap */ @@ -114,11 +132,12 @@ public function scheduledGenerateSitemaps() $sitemap->generateXml(); } catch (\Exception $e) { $errors[] = $e->getMessage(); - - $this->sitemapEmail->sendErrorEmail($errors); } finally { $this->appEmulation->stopEnvironmentEmulation(); } } + if ($errors && $recipient) { + $this->emailNotification->sendErrors($errors); + } } } From 85a2a561bf5f5fe585c27ef40564692860f31be3 Mon Sep 17 00:00:00 2001 From: NazarKlovanych Date: Wed, 12 Dec 2018 14:47:02 +0200 Subject: [PATCH 7/8] Refactoring --- app/code/Magento/Sitemap/Model/EmailNotification.php | 2 +- app/code/Magento/Sitemap/Model/Observer.php | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/app/code/Magento/Sitemap/Model/EmailNotification.php b/app/code/Magento/Sitemap/Model/EmailNotification.php index 515121de6e6e1..27c042870a1d6 100644 --- a/app/code/Magento/Sitemap/Model/EmailNotification.php +++ b/app/code/Magento/Sitemap/Model/EmailNotification.php @@ -38,7 +38,7 @@ class EmailNotification private $transportBuilder; /** - * @var $logger + * @var \Psr\Log\LoggerInterface $logger */ private $logger; diff --git a/app/code/Magento/Sitemap/Model/Observer.php b/app/code/Magento/Sitemap/Model/Observer.php index 887b5a6541a56..bd7a84f601b77 100644 --- a/app/code/Magento/Sitemap/Model/Observer.php +++ b/app/code/Magento/Sitemap/Model/Observer.php @@ -7,7 +7,6 @@ use Magento\Store\Model\App\Emulation; use Magento\Sitemap\Model\EmailNotification as SitemapEmail; -use Magento\Store\Model\StoreManagerInterface; use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Sitemap\Model\ResourceModel\Sitemap\CollectionFactory; use Magento\Store\Model\ScopeInterface; @@ -58,11 +57,6 @@ class Observer */ private $collectionFactory; - /** - * @var \Magento\Store\Model\StoreManagerInterface - */ - private $storeManager; - /** * @var Emulation */ @@ -77,20 +71,17 @@ class Observer * Observer constructor. * @param ScopeConfigInterface $scopeConfig * @param CollectionFactory $collectionFactory - * @param StoreManagerInterface $storeManager * @param EmailNotification $emailNotification * @param Emulation $appEmulation */ public function __construct( ScopeConfigInterface $scopeConfig, CollectionFactory $collectionFactory, - StoreManagerInterface $storeManager, SitemapEmail $emailNotification, Emulation $appEmulation ) { $this->scopeConfig = $scopeConfig; $this->collectionFactory = $collectionFactory; - $this->storeManager = $storeManager; $this->appEmulation = $appEmulation; $this->emailNotification = $emailNotification; } From a04cbf2acd3a149d9349ec2ea738010190dc7e10 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Fri, 21 Dec 2018 13:10:17 +0200 Subject: [PATCH 8/8] ENGCOM-3683: Unit test fix. --- .../Test/Unit/Model/EmailNotificationTest.php | 134 ++++++++++++++++++ .../Sitemap/Test/Unit/Model/ObserverTest.php | 108 ++++++-------- 2 files changed, 176 insertions(+), 66 deletions(-) create mode 100644 app/code/Magento/Sitemap/Test/Unit/Model/EmailNotificationTest.php diff --git a/app/code/Magento/Sitemap/Test/Unit/Model/EmailNotificationTest.php b/app/code/Magento/Sitemap/Test/Unit/Model/EmailNotificationTest.php new file mode 100644 index 0000000000000..eafb47c086bac --- /dev/null +++ b/app/code/Magento/Sitemap/Test/Unit/Model/EmailNotificationTest.php @@ -0,0 +1,134 @@ +objectManagerMock = $this->getMockBuilder(ObjectManagerInterface::class) + ->getMock(); + $this->scopeConfigMock = $this->getMockBuilder(ScopeConfigInterface::class) + ->getMock(); + $this->transportBuilderMock = $this->getMockBuilder(TransportBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + $this->inlineTranslationMock = $this->getMockBuilder(StateInterface::class) + ->getMock(); + + $this->objectManager = new ObjectManager($this); + $this->model = $this->objectManager->getObject( + EmailNotification::class, + [ + 'inlineTranslation' => $this->inlineTranslationMock, + 'scopeConfig' => $this->scopeConfigMock, + 'transportBuilder' => $this->transportBuilderMock, + ] + ); + } + + public function testSendErrors() + { + $exception = 'Sitemap Exception'; + $transport = $this->createMock(TransportInterface::class); + + $this->scopeConfigMock->expects($this->at(0)) + ->method('getValue') + ->with( + Observer::XML_PATH_ERROR_TEMPLATE, + ScopeInterface::SCOPE_STORE + ) + ->willReturn('error-recipient@example.com'); + + $this->inlineTranslationMock->expects($this->once()) + ->method('suspend'); + + $this->transportBuilderMock->expects($this->once()) + ->method('setTemplateIdentifier') + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setTemplateOptions') + ->with([ + 'area' => FrontNameResolver::AREA_CODE, + 'store' => Store::DEFAULT_STORE_ID, + ]) + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setTemplateVars') + ->with(['warnings' => $exception]) + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('setFrom') + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('addTo') + ->will($this->returnSelf()); + + $this->transportBuilderMock->expects($this->once()) + ->method('getTransport') + ->willReturn($transport); + + $transport->expects($this->once()) + ->method('sendMessage'); + + $this->inlineTranslationMock->expects($this->once()) + ->method('resume'); + + $this->model->sendErrors(['Sitemap Exception']); + } +} diff --git a/app/code/Magento/Sitemap/Test/Unit/Model/ObserverTest.php b/app/code/Magento/Sitemap/Test/Unit/Model/ObserverTest.php index ac88f23ff9d69..5fae54ff3c5d0 100644 --- a/app/code/Magento/Sitemap/Test/Unit/Model/ObserverTest.php +++ b/app/code/Magento/Sitemap/Test/Unit/Model/ObserverTest.php @@ -5,10 +5,14 @@ */ namespace Magento\Sitemap\Test\Unit\Model; +use Magento\Framework\App\Area; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Sitemap\Model\EmailNotification; +use Magento\Store\Model\App\Emulation; /** * Class ObserverTest + * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ObserverTest extends \PHPUnit\Framework\TestCase @@ -33,21 +37,6 @@ class ObserverTest extends \PHPUnit\Framework\TestCase */ private $collectionFactoryMock; - /** - * @var \Magento\Framework\Mail\Template\TransportBuilder|\PHPUnit_Framework_MockObject_MockObject - */ - private $transportBuilderMock; - - /** - * @var \Magento\Store\Model\StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject - */ - private $storeManagerMock; - - /** - * @var \Magento\Framework\Translate\Inline\StateInterface|\PHPUnit_Framework_MockObject_MockObject - */ - private $inlineTranslationMock; - /** * @var \Magento\Sitemap\Model\ResourceModel\Sitemap\Collection|\PHPUnit_Framework_MockObject_MockObject */ @@ -63,6 +52,16 @@ class ObserverTest extends \PHPUnit\Framework\TestCase */ private $objectManagerMock; + /** + * @var Emulation|\PHPUnit_Framework_MockObject_MockObject + */ + private $appEmulationMock; + + /** + * @var EmailNotification|\PHPUnit_Framework_MockObject_MockObject + */ + private $emailNotificationMock; + protected function setUp() { $this->objectManagerMock = $this->getMockBuilder(\Magento\Framework\ObjectManagerInterface::class) @@ -74,28 +73,28 @@ protected function setUp() )->disableOriginalConstructor() ->setMethods(['create']) ->getMock(); - $this->transportBuilderMock = $this->getMockBuilder(\Magento\Framework\Mail\Template\TransportBuilder::class) - ->disableOriginalConstructor() - ->getMock(); - $this->storeManagerMock = $this->getMockBuilder(\Magento\Store\Model\StoreManagerInterface::class) - ->getMock(); - $this->inlineTranslationMock = $this->getMockBuilder(\Magento\Framework\Translate\Inline\StateInterface::class) - ->getMock(); $this->sitemapCollectionMock = $this->createPartialMock( \Magento\Sitemap\Model\ResourceModel\Sitemap\Collection::class, ['getIterator'] ); - $this->sitemapMock = $this->createPartialMock(\Magento\Sitemap\Model\Sitemap::class, ['generateXml']); - + $this->sitemapMock = $this->createPartialMock( + \Magento\Sitemap\Model\Sitemap::class, + [ + 'generateXml', + 'getStoreId', + ] + ); + $this->appEmulationMock = $this->createMock(Emulation::class); + $this->emailNotificationMock = $this->createMock(EmailNotification::class); $this->objectManager = new ObjectManager($this); + $this->observer = $this->objectManager->getObject( \Magento\Sitemap\Model\Observer::class, [ 'scopeConfig' => $this->scopeConfigMock, 'collectionFactory' => $this->collectionFactoryMock, - 'storeManager' => $this->storeManagerMock, - 'transportBuilder' => $this->transportBuilderMock, - 'inlineTranslation' => $this->inlineTranslationMock + 'appEmulation' => $this->appEmulationMock, + 'emailNotification' => $this->emailNotificationMock ] ); } @@ -103,7 +102,7 @@ protected function setUp() public function testScheduledGenerateSitemapsSendsExceptionEmail() { $exception = 'Sitemap Exception'; - $transport = $this->createMock(\Magento\Framework\Mail\TransportInterface::class); + $storeId = 1; $this->scopeConfigMock->expects($this->once())->method('isSetFlag')->willReturn(true); @@ -115,11 +114,15 @@ public function testScheduledGenerateSitemapsSendsExceptionEmail() ->method('getIterator') ->willReturn(new \ArrayIterator([$this->sitemapMock])); - $this->sitemapMock->expects($this->once()) + $this->sitemapMock->expects($this->at(0)) + ->method('getStoreId') + ->willReturn($storeId); + + $this->sitemapMock->expects($this->at(1)) ->method('generateXml') ->willThrowException(new \Exception($exception)); - $this->scopeConfigMock->expects($this->at(1)) + $this->scopeConfigMock->expects($this->at(0)) ->method('getValue') ->with( \Magento\Sitemap\Model\Observer::XML_PATH_ERROR_RECIPIENT, @@ -127,43 +130,16 @@ public function testScheduledGenerateSitemapsSendsExceptionEmail() ) ->willReturn('error-recipient@example.com'); - $this->inlineTranslationMock->expects($this->once()) - ->method('suspend'); - - $this->transportBuilderMock->expects($this->once()) - ->method('setTemplateIdentifier') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setTemplateOptions') - ->with([ - 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, - ]) - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setTemplateVars') - ->with(['warnings' => $exception]) - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('setFrom') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('addTo') - ->will($this->returnSelf()); - - $this->transportBuilderMock->expects($this->once()) - ->method('getTransport') - ->willReturn($transport); - - $transport->expects($this->once()) - ->method('sendMessage'); + $this->appEmulationMock->expects($this->at(0)) + ->method('startEnvironmentEmulation') + ->with( + $storeId, + Area::AREA_FRONTEND, + true + ); - $this->inlineTranslationMock->expects($this->once()) - ->method('resume'); + $this->appEmulationMock->expects($this->at(1)) + ->method('stopEnvironmentEmulation'); $this->observer->scheduledGenerateSitemaps(); }