magento/graphql-ce#762: Subscribe for newsletter even when not allowed

Author: lenaorobei

app/code/Magento/Customer/Block/Form/Register.php

@@ -6,6 +6,8 @@
 namespace Magento\Customer\Block\Form;
 
 use Magento\Customer\Model\AccountManagement;
+use Magento\Framework\App\ObjectManager;
+use Magento\Newsletter\Model\Config;
 
 /**
  * Customer register form block
@@ -31,6 +33,11 @@ class Register extends \Magento\Directory\Block\Data
      */
     protected $_customerUrl;
 
+    /**
+     * @var Config
+     */
+    private $newsLetterConfig;
+
     /**
      * Constructor
      *
@@ -44,6 +51,7 @@ class Register extends \Magento\Directory\Block\Data
      * @param \Magento\Customer\Model\Session $customerSession
      * @param \Magento\Customer\Model\Url $customerUrl
      * @param array $data
+     * @param Config $newsLetterConfig
      *
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
@@ -57,11 +65,13 @@ public function __construct(
         \Magento\Framework\Module\ModuleManagerInterface $moduleManager,
         \Magento\Customer\Model\Session $customerSession,
         \Magento\Customer\Model\Url $customerUrl,
-        array $data = []
+        array $data = [],
+        Config $newsLetterConfig = null
     ) {
         $this->_customerUrl = $customerUrl;
         $this->_moduleManager = $moduleManager;
         $this->_customerSession = $customerSession;
+        $this->newsLetterConfig = $newsLetterConfig ?: ObjectManager::getInstance()->get(Config::class);
         parent::__construct(
             $context,
             $directoryHelper,
@@ -169,7 +179,7 @@ public function getRegion()
     public function isNewsletterEnabled()
     {
         return $this->_moduleManager->isOutputEnabled('Magento_Newsletter')
-            && $this->getConfig(\Magento\Newsletter\Model\Config::XML_PATH_NEWSLETTER_ACTIVE);
+            && $this->newsLetterConfig->isActive();
     }
 
     /**

app/code/Magento/Customer/Test/Unit/Block/Form/RegisterTest.php

@@ -48,6 +48,9 @@ class RegisterTest extends \PHPUnit\Framework\TestCase
     /** @var Register */
     private $_block;
 
+    /** @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Newsletter\Model\Config */
+    private $newsletterConfig;
+
     protected function setUp()
     {
         $this->_scopeConfig = $this->createMock(\Magento\Framework\App\Config\ScopeConfigInterface::class);
@@ -58,6 +61,7 @@ protected function setUp()
             \Magento\Customer\Model\Session::class,
             ['getCustomerFormData']
         );
+        $this->newsletterConfig = $this->createMock(\Magento\Newsletter\Model\Config::class);
         $context = $this->createMock(\Magento\Framework\View\Element\Template\Context::class);
         $context->expects($this->any())->method('getScopeConfig')->will($this->returnValue($this->_scopeConfig));
 
@@ -70,7 +74,9 @@ protected function setUp()
             $this->createMock(\Magento\Directory\Model\ResourceModel\Country\CollectionFactory::class),
             $this->_moduleManager,
             $this->_customerSession,
-            $this->_customerUrl
+            $this->_customerUrl,
+            [],
+            $this->newsletterConfig
         );
     }
 
@@ -292,12 +298,10 @@ public function testIsNewsletterEnabled($isNewsletterEnabled, $isNewsletterActiv
             $this->returnValue($isNewsletterEnabled)
         );
 
-        $this->_scopeConfig->expects(
+        $this->newsletterConfig->expects(
             $this->any()
         )->method(
-            'getValue'
-        )->with(
-            \Magento\Newsletter\Model\Config::XML_PATH_NEWSLETTER_ACTIVE
+            'isActive'
         )->will(
             $this->returnValue($isNewsletterActive)
         );

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

@@ -17,12 +17,12 @@ class Config
     /**
      * Configuration path to newsletter active setting
      */
-    const XML_PATH_NEWSLETTER_ACTIVE = 'newsletter/general/active';
+    private const XML_PATH_NEWSLETTER_ACTIVE = 'newsletter/general/active';
 
     /**
      * @var ScopeConfigInterface
      */
-    protected $scopeConfig;
+    private $scopeConfig;
 
     /**
      * Config constructor.
@@ -41,7 +41,7 @@ public function __construct(
      * @param string $scopeType
      * @return bool
      */
-    public function isActive($scopeType = ScopeConfigInterface::SCOPE_TYPE_DEFAULT): bool
+    public function isActive(string $scopeType = ScopeConfigInterface::SCOPE_TYPE_DEFAULT): bool
     {
         return $this->scopeConfig->isSetFlag(self::XML_PATH_NEWSLETTER_ACTIVE, $scopeType);
     }

app/code/Magento/Newsletter/Observer/PredispatchNewsletterObserver.php

@@ -22,14 +22,14 @@ class PredispatchNewsletterObserver implements ObserverInterface
 {
     /**
      * @deprecated
-     * use Magento\Newsletter\Model\Config::XML_PATH_NEWSLETTER_ACTIVE instead
+     * @see \Magento\Newsletter\Model\Config::isActive()
      */
     const XML_PATH_NEWSLETTER_ACTIVE = 'newsletter/general/active';
 
     /**
      * @var Newsletter Config
      */
-    private $config;
+    private $newsletterConfig;
 
     /**
      * @var ScopeConfigInterface
@@ -51,11 +51,11 @@ class PredispatchNewsletterObserver implements ObserverInterface
     public function __construct(
         ScopeConfigInterface $scopeConfig,
         UrlInterface $url,
-        Config $config = null
+        Config $newsletterConfig = null
     ) {
         $this->scopeConfig = $scopeConfig;
         $this->url = $url;
-        $this->config = $config ?: ObjectManager::getInstance()->get(Config::class);
+        $this->newsletterConfig = $newsletterConfig ?: ObjectManager::getInstance()->get(Config::class);
     }
 
     /**
@@ -65,7 +65,7 @@ public function __construct(
      */
     public function execute(Observer $observer) : void
     {
-        if (!$this->config->isActive(ScopeInterface::SCOPE_STORE)) {
+        if (!$this->newsletterConfig->isActive(ScopeInterface::SCOPE_STORE)) {
             $defaultNoRouteUrl = $this->scopeConfig->getValue(
                 'web/default/no_route',
                 ScopeInterface::SCOPE_STORE

app/code/Magento/Newsletter/Test/Unit/Observer/PredispatchNewsletterObserverTest.php

@@ -13,6 +13,7 @@
 use Magento\Framework\Event\Observer;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\Framework\UrlInterface;
+use Magento\Newsletter\Model\Config;
 use Magento\Newsletter\Observer\PredispatchNewsletterObserver;
 use Magento\Store\Model\ScopeInterface;
 use PHPUnit\Framework\TestCase;
@@ -52,30 +53,29 @@ class PredispatchNewsletterObserverTest extends TestCase
      */
     private $objectManager;
 
+    /**
+     * @var Config
+     */
+    private $newsletterConfig;
+
     /**
      * @inheritdoc
      */
     protected function setUp() : void
     {
-        $this->configMock = $this->getMockBuilder(ScopeConfigInterface::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $this->urlMock = $this->getMockBuilder(UrlInterface::class)
-            ->disableOriginalConstructor()
-            ->getMock();
+        $this->configMock = $this->createMock(ScopeConfigInterface::class);
+        $this->urlMock = $this->createMock(UrlInterface::class);
         $this->responseMock = $this->getMockBuilder(ResponseInterface::class)
             ->disableOriginalConstructor()
             ->setMethods(['setRedirect'])
             ->getMockForAbstractClass();
-        $this->redirectMock = $this->getMockBuilder(RedirectInterface::class)
-            ->getMock();
+        $this->redirectMock = $this->createMock(RedirectInterface::class);
+        $this->newsletterConfig = $this->createMock(Config::class);
         $this->objectManager = new ObjectManager($this);
-        $this->mockObject = $this->objectManager->getObject(
-            PredispatchNewsletterObserver::class,
-            [
-                'scopeConfig' => $this->configMock,
-                'url' => $this->urlMock
-            ]
+        $this->mockObject = new PredispatchNewsletterObserver(
+            $this->configMock,
+            $this->urlMock,
+            $this->newsletterConfig
         );
     }
 
@@ -89,8 +89,9 @@ public function testNewsletterEnabled() : void
             ->setMethods(['getResponse', 'getData', 'setRedirect'])
             ->getMockForAbstractClass();
 
-        $this->configMock->method('getValue')
-            ->with(\Magento\Newsletter\Model\Config::XML_PATH_NEWSLETTER_ACTIVE, ScopeInterface::SCOPE_STORE)
+        $this->newsletterConfig->expects($this->once())
+            ->method('isActive')
+            ->with(ScopeInterface::SCOPE_STORE)
             ->willReturn(true);
         $observerMock->expects($this->never())
             ->method('getData')
@@ -114,14 +115,14 @@ public function testNewsletterDisabled() : void
             ->setMethods(['getControllerAction', 'getResponse'])
             ->getMockForAbstractClass();
 
-        $this->configMock->expects($this->at(0))
-            ->method('getValue')
-            ->with(\Magento\Newsletter\Model\Config::XML_PATH_NEWSLETTER_ACTIVE, ScopeInterface::SCOPE_STORE)
+        $this->newsletterConfig->expects($this->once())
+            ->method('isActive')
+            ->with(ScopeInterface::SCOPE_STORE)
             ->willReturn(false);
 
         $expectedRedirectUrl = 'https://test.com/index';
 
-        $this->configMock->expects($this->at(1))
+        $this->configMock->expects($this->once())
             ->method('getValue')
             ->with('web/default/no_route', ScopeInterface::SCOPE_STORE)
             ->willReturn($expectedRedirectUrl);