Skip to content

Commit

Permalink
ENGCOM-2428: [Forwardport] Fix newsletter subscription behaviour for …
Browse files Browse the repository at this point in the history
…registered customer. #16947
  • Loading branch information
Stanislav Idolov authored Jul 20, 2018
2 parents 8700de5 + 3dbac7a commit 20a332f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 22 deletions.
13 changes: 2 additions & 11 deletions app/code/Magento/Newsletter/Model/Subscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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();
Expand Down
24 changes: 13 additions & 11 deletions app/code/Magento/Newsletter/Test/Unit/Model/SubscriberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/
namespace Magento\Newsletter\Test\Unit\Model;

use Magento\Newsletter\Model\Subscriber;

/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
Expand Down Expand Up @@ -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'
]
Expand All @@ -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'
]
Expand All @@ -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()
Expand All @@ -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');
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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()
Expand All @@ -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');
Expand All @@ -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()
Expand Down

0 comments on commit 20a332f

Please sign in to comment.