ENGCOM-2428: [Forwardport] Fix newsletter subscription behaviour for registered customer. #16947

Stanislav Idolov · web-flow · commit 20a332ffb1bf · 2018-07-20T16:13:05.000+03:00
diff --git a/app/code/Magento/Newsletter/Model/Subscriber.php b/app/code/Magento/Newsletter/Model/Subscriber.php
@@ -419,7 +419,6 @@ public function subscribe($email)
             self::XML_PATH_CONFIRMATION_FLAG,
             \Magento\Store\Model\ScopeInterface::SCOPE_STORE
         ) == 1 ? true : false;
-        $isOwnSubscribes = false;
 
         $isSubscribeOwnEmail = $this->_customerSession->isLoggedIn()
             && $this->_customerSession->getCustomerDataObject()->getEmail() == $email;
@@ -428,13 +427,7 @@ public function subscribe($email)
             || $this->getStatus() == self::STATUS_NOT_ACTIVE
         ) {
             if ($isConfirmNeed === true) {
-                // if user subscribes own login email - confirmation is not needed
-                $isOwnSubscribes = $isSubscribeOwnEmail;
-                if ($isOwnSubscribes == true) {
-                    $this->setStatus(self::STATUS_SUBSCRIBED);
-                } else {
-                    $this->setStatus(self::STATUS_NOT_ACTIVE);
-                }
+                $this->setStatus(self::STATUS_NOT_ACTIVE);
             } else {
                 $this->setStatus(self::STATUS_SUBSCRIBED);
             }
@@ -460,9 +453,7 @@ public function subscribe($email)
         try {
             /* Save model before sending out email */
             $this->save();
-            if ($isConfirmNeed === true
-                && $isOwnSubscribes === false
-            ) {
+            if ($isConfirmNeed === true) {
                 $this->sendConfirmationRequestEmail();
             } else {
                 $this->sendConfirmationSuccessEmail();
diff --git a/app/code/Magento/Newsletter/Test/Unit/Model/SubscriberTest.php b/app/code/Magento/Newsletter/Test/Unit/Model/SubscriberTest.php
@@ -5,6 +5,8 @@
  */
 namespace Magento\Newsletter\Test\Unit\Model;
 
+use Magento\Newsletter\Model\Subscriber;
+
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
@@ -116,7 +118,7 @@ public function testSubscribe()
         $email = 'subscriber_email@magento.com';
         $this->resource->expects($this->any())->method('loadByEmail')->willReturn(
             [
-                'subscriber_status' => 3,
+                'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,
                 'subscriber_email' => $email,
                 'name' => 'subscriber_name'
             ]
@@ -133,15 +135,15 @@ public function testSubscribe()
         $this->sendEmailCheck();
         $this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();
 
-        $this->assertEquals(1, $this->subscriber->subscribe($email));
+        $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));
     }
 
     public function testSubscribeNotLoggedIn()
     {
         $email = 'subscriber_email@magento.com';
         $this->resource->expects($this->any())->method('loadByEmail')->willReturn(
             [
-                'subscriber_status' => 3,
+                'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,
                 'subscriber_email' => $email,
                 'name' => 'subscriber_name'
             ]
@@ -158,7 +160,7 @@ public function testSubscribeNotLoggedIn()
         $this->sendEmailCheck();
         $this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();
 
-        $this->assertEquals(2, $this->subscriber->subscribe($email));
+        $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));
     }
 
     public function testUpdateSubscription()
@@ -175,7 +177,7 @@ public function testUpdateSubscription()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 1
+                    'subscriber_status' => Subscriber::STATUS_SUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -210,7 +212,7 @@ public function testUnsubscribeCustomerById()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 1
+                    'subscriber_status' => Subscriber::STATUS_SUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -236,7 +238,7 @@ public function testSubscribeCustomerById()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 3
+                    'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -262,7 +264,7 @@ public function testSubscribeCustomerById1()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 3
+                    'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -276,7 +278,7 @@ public function testSubscribeCustomerById1()
         $this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);
 
         $this->subscriber->subscribeCustomerById($customerId);
-        $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());
+        $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());
     }
 
     public function testSubscribeCustomerByIdAfterConfirmation()
@@ -293,7 +295,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 4
+                    'subscriber_status' => Subscriber::STATUS_UNCONFIRMED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -305,7 +307,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()
         $this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);
 
         $this->subscriber->updateSubscription($customerId);
-        $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());
+        $this->assertEquals(Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());
     }
 
     public function testUnsubscribe()

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,8 @@`
`5`	`5`	`*/`
`6`	`6`	`namespace Magento\Newsletter\Test\Unit\Model;`
`7`	`7`
	`8`	`+use Magento\Newsletter\Model\Subscriber;`
	`9`	`+`
`8`	`10`	`/**`
`9`	`11`	`* @SuppressWarnings(PHPMD.CouplingBetweenObjects)`
`10`	`12`	`*/`
`@@ -116,7 +118,7 @@ public function testSubscribe()`
`116`	`118`	`$email = '[email protected]';`
`117`	`119`	`$this->resource->expects($this->any())->method('loadByEmail')->willReturn(`
`118`	`120`	`[`
`119`		`- 'subscriber_status' => 3,`
	`121`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,`
`120`	`122`	`'subscriber_email' => $email,`
`121`	`123`	`'name' => 'subscriber_name'`
`122`	`124`	`]`
`@@ -133,15 +135,15 @@ public function testSubscribe()`
`133`	`135`	`$this->sendEmailCheck();`
`134`	`136`	`$this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();`
`135`	`137`
`136`		`- $this->assertEquals(1, $this->subscriber->subscribe($email));`
	`138`	`+ $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));`
`137`	`139`	`}`
`138`	`140`
`139`	`141`	`public function testSubscribeNotLoggedIn()`
`140`	`142`	`{`
`141`	`143`	`$email = '[email protected]';`
`142`	`144`	`$this->resource->expects($this->any())->method('loadByEmail')->willReturn(`
`143`	`145`	`[`
`144`		`- 'subscriber_status' => 3,`
	`146`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,`
`145`	`147`	`'subscriber_email' => $email,`
`146`	`148`	`'name' => 'subscriber_name'`
`147`	`149`	`]`
`@@ -158,7 +160,7 @@ public function testSubscribeNotLoggedIn()`
`158`	`160`	`$this->sendEmailCheck();`
`159`	`161`	`$this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();`
`160`	`162`
`161`		`- $this->assertEquals(2, $this->subscriber->subscribe($email));`
	`163`	`+ $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));`
`162`	`164`	`}`
`163`	`165`
`164`	`166`	`public function testUpdateSubscription()`
`@@ -175,7 +177,7 @@ public function testUpdateSubscription()`
`175`	`177`	`->willReturn(`
`176`	`178`	`[`
`177`	`179`	`'subscriber_id' => 1,`
`178`		`- 'subscriber_status' => 1`
	`180`	`+ 'subscriber_status' => Subscriber::STATUS_SUBSCRIBED`
`179`	`181`	`]`
`180`	`182`	`);`
`181`	`183`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -210,7 +212,7 @@ public function testUnsubscribeCustomerById()`
`210`	`212`	`->willReturn(`
`211`	`213`	`[`
`212`	`214`	`'subscriber_id' => 1,`
`213`		`- 'subscriber_status' => 1`
	`215`	`+ 'subscriber_status' => Subscriber::STATUS_SUBSCRIBED`
`214`	`216`	`]`
`215`	`217`	`);`
`216`	`218`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -236,7 +238,7 @@ public function testSubscribeCustomerById()`
`236`	`238`	`->willReturn(`
`237`	`239`	`[`
`238`	`240`	`'subscriber_id' => 1,`
`239`		`- 'subscriber_status' => 3`
	`241`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED`
`240`	`242`	`]`
`241`	`243`	`);`
`242`	`244`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -262,7 +264,7 @@ public function testSubscribeCustomerById1()`
`262`	`264`	`->willReturn(`
`263`	`265`	`[`
`264`	`266`	`'subscriber_id' => 1,`
`265`		`- 'subscriber_status' => 3`
	`267`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED`
`266`	`268`	`]`
`267`	`269`	`);`
`268`	`270`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -276,7 +278,7 @@ public function testSubscribeCustomerById1()`
`276`	`278`	`$this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);`
`277`	`279`
`278`	`280`	`$this->subscriber->subscribeCustomerById($customerId);`
`279`		`- $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());`
	`281`	`+ $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());`
`280`	`282`	`}`
`281`	`283`
`282`	`284`	`public function testSubscribeCustomerByIdAfterConfirmation()`
`@@ -293,7 +295,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()`
`293`	`295`	`->willReturn(`
`294`	`296`	`[`
`295`	`297`	`'subscriber_id' => 1,`
`296`		`- 'subscriber_status' => 4`
	`298`	`+ 'subscriber_status' => Subscriber::STATUS_UNCONFIRMED`
`297`	`299`	`]`
`298`	`300`	`);`
`299`	`301`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -305,7 +307,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()`
`305`	`307`	`$this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);`
`306`	`308`
`307`	`309`	`$this->subscriber->updateSubscription($customerId);`
`308`		`- $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());`
	`310`	`+ $this->assertEquals(Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());`
`309`	`311`	`}`
`310`	`312`
`311`	`313`	`public function testUnsubscribe()`