ENGCOM-2272: Fix newsletter subscription behaviour for registered customer. #15479

Stanislav Idolov · web-flow · commit ef16761ffb47 · 2018-07-13T14:18:01.000+03:00
diff --git a/app/code/Magento/Newsletter/Model/Subscriber.php b/app/code/Magento/Newsletter/Model/Subscriber.php
@@ -447,7 +447,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;
@@ -456,13 +455,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);
             }
@@ -488,9 +481,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)
  */
@@ -148,7 +150,7 @@ public function testSubscribe()
         );
         $this->resource->expects($this->any())->method('loadByCustomerData')->with($customer)->willReturn(
             [
-                'subscriber_status' => 3,
+                'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,
                 'subscriber_email' => $email,
                 'name' => 'subscriber_name'
             ]
@@ -165,7 +167,7 @@ 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()
@@ -187,7 +189,7 @@ public function testSubscribeNotLoggedIn()
         );
         $this->resource->expects($this->any())->method('loadByCustomerData')->with($customer)->willReturn(
             [
-                'subscriber_status' => 3,
+                'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,
                 'subscriber_email' => $email,
                 'name' => 'subscriber_name'
             ]
@@ -204,7 +206,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()
@@ -221,7 +223,7 @@ public function testUpdateSubscription()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 1
+                    'subscriber_status' => Subscriber::STATUS_SUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -256,7 +258,7 @@ public function testUnsubscribeCustomerById()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 1
+                    'subscriber_status' => Subscriber::STATUS_SUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -282,7 +284,7 @@ public function testSubscribeCustomerById()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 3
+                    'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -308,7 +310,7 @@ public function testSubscribeCustomerById1()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 3
+                    'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -322,7 +324,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()
@@ -339,7 +341,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()
             ->willReturn(
                 [
                     'subscriber_id' => 1,
-                    'subscriber_status' => 4
+                    'subscriber_status' => Subscriber::STATUS_UNCONFIRMED
                 ]
             );
         $customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
@@ -351,7 +353,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`	`*/`
`@@ -148,7 +150,7 @@ public function testSubscribe()`
`148`	`150`	`);`
`149`	`151`	`$this->resource->expects($this->any())->method('loadByCustomerData')->with($customer)->willReturn(`
`150`	`152`	`[`
`151`		`- 'subscriber_status' => 3,`
	`153`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,`
`152`	`154`	`'subscriber_email' => $email,`
`153`	`155`	`'name' => 'subscriber_name'`
`154`	`156`	`]`
`@@ -165,7 +167,7 @@ public function testSubscribe()`
`165`	`167`	`$this->sendEmailCheck();`
`166`	`168`	`$this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();`
`167`	`169`
`168`		`- $this->assertEquals(1, $this->subscriber->subscribe($email));`
	`170`	`+ $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));`
`169`	`171`	`}`
`170`	`172`
`171`	`173`	`public function testSubscribeNotLoggedIn()`
`@@ -187,7 +189,7 @@ public function testSubscribeNotLoggedIn()`
`187`	`189`	`);`
`188`	`190`	`$this->resource->expects($this->any())->method('loadByCustomerData')->with($customer)->willReturn(`
`189`	`191`	`[`
`190`		`- 'subscriber_status' => 3,`
	`192`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED,`
`191`	`193`	`'subscriber_email' => $email,`
`192`	`194`	`'name' => 'subscriber_name'`
`193`	`195`	`]`
`@@ -204,7 +206,7 @@ public function testSubscribeNotLoggedIn()`
`204`	`206`	`$this->sendEmailCheck();`
`205`	`207`	`$this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();`
`206`	`208`
`207`		`- $this->assertEquals(2, $this->subscriber->subscribe($email));`
	`209`	`+ $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->subscribe($email));`
`208`	`210`	`}`
`209`	`211`
`210`	`212`	`public function testUpdateSubscription()`
`@@ -221,7 +223,7 @@ public function testUpdateSubscription()`
`221`	`223`	`->willReturn(`
`222`	`224`	`[`
`223`	`225`	`'subscriber_id' => 1,`
`224`		`- 'subscriber_status' => 1`
	`226`	`+ 'subscriber_status' => Subscriber::STATUS_SUBSCRIBED`
`225`	`227`	`]`
`226`	`228`	`);`
`227`	`229`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -256,7 +258,7 @@ public function testUnsubscribeCustomerById()`
`256`	`258`	`->willReturn(`
`257`	`259`	`[`
`258`	`260`	`'subscriber_id' => 1,`
`259`		`- 'subscriber_status' => 1`
	`261`	`+ 'subscriber_status' => Subscriber::STATUS_SUBSCRIBED`
`260`	`262`	`]`
`261`	`263`	`);`
`262`	`264`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -282,7 +284,7 @@ public function testSubscribeCustomerById()`
`282`	`284`	`->willReturn(`
`283`	`285`	`[`
`284`	`286`	`'subscriber_id' => 1,`
`285`		`- 'subscriber_status' => 3`
	`287`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED`
`286`	`288`	`]`
`287`	`289`	`);`
`288`	`290`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -308,7 +310,7 @@ public function testSubscribeCustomerById1()`
`308`	`310`	`->willReturn(`
`309`	`311`	`[`
`310`	`312`	`'subscriber_id' => 1,`
`311`		`- 'subscriber_status' => 3`
	`313`	`+ 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED`
`312`	`314`	`]`
`313`	`315`	`);`
`314`	`316`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -322,7 +324,7 @@ public function testSubscribeCustomerById1()`
`322`	`324`	`$this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);`
`323`	`325`
`324`	`326`	`$this->subscriber->subscribeCustomerById($customerId);`
`325`		`- $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());`
	`327`	`+ $this->assertEquals(Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());`
`326`	`328`	`}`
`327`	`329`
`328`	`330`	`public function testSubscribeCustomerByIdAfterConfirmation()`
`@@ -339,7 +341,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()`
`339`	`341`	`->willReturn(`
`340`	`342`	`[`
`341`	`343`	`'subscriber_id' => 1,`
`342`		`- 'subscriber_status' => 4`
	`344`	`+ 'subscriber_status' => Subscriber::STATUS_UNCONFIRMED`
`343`	`345`	`]`
`344`	`346`	`);`
`345`	`347`	`$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');`
`@@ -351,7 +353,7 @@ public function testSubscribeCustomerByIdAfterConfirmation()`
`351`	`353`	`$this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);`
`352`	`354`
`353`	`355`	`$this->subscriber->updateSubscription($customerId);`
`354`		`- $this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());`
	`356`	`+ $this->assertEquals(Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());`
`355`	`357`	`}`
`356`	`358`
`357`	`359`	`public function testUnsubscribe()`