Skip to content

Commit 8f6458f

Browse files
adifucanadifucan
committed
MAGETWO-66793: [Backport] Merchant can't unsubscribe Customer from Newsletter in Admin
1 parent ca8ee57 commit 8f6458f

File tree

4 files changed

+118
-19
lines changed

4 files changed

+118
-19
lines changed

‎app/code/Magento/Customer/Controller/Adminhtml/Index/Save.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@
1313
use Magento\Customer\Model\Metadata\Form;
1414
use Magento\Framework\Exception\LocalizedException;
1515

16-
/**
17-
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
18-
*/
1916
class Save extends \Magento\Customer\Controller\Adminhtml\Index
2017
{
2118
/**
@@ -210,7 +207,7 @@ public function execute()
210207
$this->dataObjectHelper->populateWithArray(
211208
$customer,
212209
$customerData,
213-
'\Magento\Customer\Api\Data\CustomerInterface'
210+
\Magento\Customer\Api\Data\CustomerInterface::class
214211
);
215212
$addresses = [];
216213
foreach ($addressesData as $addressData) {
@@ -224,7 +221,7 @@ public function execute()
224221
$this->dataObjectHelper->populateWithArray(
225222
$addressDataObject,
226223
$addressData,
227-
'\Magento\Customer\Api\Data\AddressInterface'
224+
\Magento\Customer\Api\Data\AddressInterface::class
228225
);
229226
$addresses[] = $addressDataObject;
230227
}
@@ -253,7 +250,7 @@ public function execute()
253250
$isSubscribed = $this->getRequest()->getPost('subscription');
254251
}
255252
if ($isSubscribed !== null) {
256-
if ($isSubscribed !== 'false') {
253+
if ($isSubscribed !== '0') {
257254
$this->_subscriberFactory->create()->subscribeCustomerById($customerId);
258255
} else {
259256
$this->_subscriberFactory->create()->unsubscribeCustomerById($customerId);

‎app/code/Magento/Customer/Test/Unit/Controller/Adminhtml/Index/SaveTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ public function testExecuteWithExistentCustomer()
280280
{
281281
$customerId = 22;
282282
$addressId = 11;
283-
$subscription = 'true';
283+
$subscription = '1';
284284
$postValue = [
285285
'customer' => [
286286
'entity_id' => $customerId,
@@ -610,7 +610,7 @@ public function testExecuteWithNewCustomer()
610610
{
611611
$customerId = 22;
612612
$addressId = 11;
613-
$subscription = 'false';
613+
$subscription = '0';
614614
$postValue = [
615615
'customer' => [
616616
'coolness' => false,
@@ -879,7 +879,7 @@ public function testExecuteWithNewCustomer()
879879
*/
880880
public function testExecuteWithNewCustomerAndValidationException()
881881
{
882-
$subscription = 'false';
882+
$subscription = '0';
883883
$postValue = [
884884
'customer' => [
885885
'coolness' => false,
@@ -1022,7 +1022,7 @@ public function testExecuteWithNewCustomerAndValidationException()
10221022
*/
10231023
public function testExecuteWithNewCustomerAndLocalizedException()
10241024
{
1025-
$subscription = 'false';
1025+
$subscription = '0';
10261026
$postValue = [
10271027
'customer' => [
10281028
'coolness' => false,
@@ -1165,7 +1165,7 @@ public function testExecuteWithNewCustomerAndLocalizedException()
11651165
*/
11661166
public function testExecuteWithNewCustomerAndException()
11671167
{
1168-
$subscription = 'false';
1168+
$subscription = '0';
11691169
$postValue = [
11701170
'customer' => [
11711171
'coolness' => false,

‎app/code/Magento/Newsletter/Model/Subscriber.php

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77

88
use Magento\Customer\Api\AccountManagementInterface;
99
use Magento\Customer\Api\CustomerRepositoryInterface;
10-
use Magento\Framework\Exception\NoSuchEntityException;
10+
use Magento\Framework\App\ObjectManager;
1111
use Magento\Framework\Exception\MailException;
12+
use Magento\Framework\Exception\NoSuchEntityException;
13+
use Magento\Framework\Stdlib\DateTime\DateTime;
1214

1315
/**
1416
* Subscriber model
1517
*
16-
* @method \Magento\Newsletter\Model\ResourceModel\Subscriber _getResource()
17-
* @method \Magento\Newsletter\Model\ResourceModel\Subscriber getResource()
1818
* @method int getStoreId()
1919
* @method $this setStoreId(int $value)
2020
* @method string getChangeStatusAt()
@@ -93,6 +93,13 @@ class Subscriber extends \Magento\Framework\Model\AbstractModel
9393
*/
9494
protected $_customerSession;
9595

96+
/**
97+
* Date
98+
*
99+
* @var DateTime
100+
*/
101+
private $dateTime;
102+
96103
/**
97104
* Store manager
98105
*
@@ -133,9 +140,10 @@ class Subscriber extends \Magento\Framework\Model\AbstractModel
133140
* @param CustomerRepositoryInterface $customerRepository
134141
* @param AccountManagementInterface $customerAccountManagement
135142
* @param \Magento\Framework\Translate\Inline\StateInterface $inlineTranslation
136-
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
137-
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
143+
* @param \Magento\Framework\Model\ResourceModel\AbstractResource|null $resource
144+
* @param \Magento\Framework\Data\Collection\AbstractDb|null $resourceCollection
138145
* @param array $data
146+
* @param DateTime|null $dateTime
139147
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
140148
*/
141149
public function __construct(
@@ -151,7 +159,8 @@ public function __construct(
151159
\Magento\Framework\Translate\Inline\StateInterface $inlineTranslation,
152160
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
153161
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
154-
array $data = []
162+
array $data = [],
163+
DateTime $dateTime = null
155164
) {
156165
$this->_newsletterData = $newsletterData;
157166
$this->_scopeConfig = $scopeConfig;
@@ -161,6 +170,7 @@ public function __construct(
161170
$this->customerRepository = $customerRepository;
162171
$this->customerAccountManagement = $customerAccountManagement;
163172
$this->inlineTranslation = $inlineTranslation;
173+
$this->dateTime = $dateTime ?: ObjectManager::getInstance()->get(DateTime::class);
164174
parent::__construct($context, $registry, $resource, $resourceCollection, $data);
165175
}
166176

@@ -263,7 +273,6 @@ public function setStatus($value)
263273
* @param boolean $scope
264274
* @return $this
265275
*/
266-
267276
public function setMessagesScope($scope)
268277
{
269278
$this->getResource()->setMessagesScope($scope);
@@ -349,6 +358,7 @@ public function loadByCustomerId($customerId)
349358
{
350359
try {
351360
$customerData = $this->customerRepository->getById($customerId);
361+
$customerData->setStoreId($this->_storeManager->getStore()->getId());
352362
$data = $this->getResource()->loadByCustomerData($customerData);
353363
$this->addData($data);
354364
if (!empty($data) && $customerData->getId() && !$this->getCustomerId()) {
@@ -446,6 +456,7 @@ public function subscribe($email)
446456
$this->setStatusChanged(true);
447457

448458
try {
459+
/* Save model before sending out email */
449460
$this->save();
450461
if ($isConfirmNeed === true
451462
&& $isOwnSubscribes === false
@@ -545,12 +556,21 @@ protected function _updateCustomerSubscription($customerId, $subscribe)
545556

546557
$sendInformationEmail = false;
547558
$status = self::STATUS_SUBSCRIBED;
559+
$isConfirmNeed = $this->_scopeConfig->getValue(
560+
self::XML_PATH_CONFIRMATION_FLAG,
561+
\Magento\Store\Model\ScopeInterface::SCOPE_STORE
562+
) == 1 ? true : false;
548563
if ($subscribe) {
549564
if (AccountManagementInterface::ACCOUNT_CONFIRMATION_REQUIRED
550565
== $this->customerAccountManagement->getConfirmationStatus($customerId)
551566
) {
552567
$status = self::STATUS_UNCONFIRMED;
568+
} elseif ($isConfirmNeed) {
569+
$status = self::STATUS_NOT_ACTIVE;
553570
}
571+
} elseif (($this->getStatus() == self::STATUS_UNCONFIRMED) && ($customerData->getConfirmation() === null)) {
572+
$status = self::STATUS_SUBSCRIBED;
573+
$sendInformationEmail = true;
554574
} else {
555575
$status = self::STATUS_UNSUBSCRIBED;
556576
}
@@ -584,7 +604,9 @@ protected function _updateCustomerSubscription($customerId, $subscribe)
584604
$sendSubscription = $sendInformationEmail;
585605
if ($sendSubscription === null xor $sendSubscription) {
586606
try {
587-
if ($this->isStatusChanged() && $status == self::STATUS_UNSUBSCRIBED) {
607+
if ($isConfirmNeed) {
608+
$this->sendConfirmationRequestEmail();
609+
} elseif ($this->isStatusChanged() && $status == self::STATUS_UNSUBSCRIBED) {
588610
$this->sendUnsubscriptionEmail();
589611
} elseif ($this->isStatusChanged() && $status == self::STATUS_SUBSCRIBED) {
590612
$this->sendConfirmationSuccessEmail();
@@ -798,4 +820,18 @@ public function getSubscriberFullName()
798820
}
799821
return $name;
800822
}
823+
824+
/**
825+
* Set date of last changed status
826+
*
827+
* @return $this
828+
*/
829+
public function beforeSave()
830+
{
831+
parent::beforeSave();
832+
if ($this->dataHasChangedFor('subscriber_status')) {
833+
$this->setChangeStatusAt($this->dateTime->gmtDate());
834+
}
835+
return $this;
836+
}
801837
}

‎app/code/Magento/Newsletter/Test/Unit/Model/SubscriberTest.php

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,12 @@ public function testUpdateSubscription()
202202
$customerDataMock->expects($this->once())->method('getStoreId')->willReturn('store_id');
203203
$customerDataMock->expects($this->once())->method('getEmail')->willReturn('email');
204204

205+
$storeModel = $this->getMockBuilder(\Magento\Store\Model\Store::class)
206+
->disableOriginalConstructor()
207+
->setMethods(['getId'])
208+
->getMock();
209+
$this->storeManager->expects($this->any())->method('getStore')->willReturn($storeModel);
210+
205211
$this->assertEquals($this->subscriber, $this->subscriber->updateSubscription($customerId));
206212
}
207213

@@ -257,6 +263,66 @@ public function testSubscribeCustomerById()
257263
$this->subscriber->subscribeCustomerById($customerId);
258264
}
259265

266+
public function testSubscribeCustomerById1()
267+
{
268+
$customerId = 1;
269+
$customerDataMock = $this->getMockBuilder(\Magento\Customer\Api\Data\CustomerInterface::class)
270+
->getMock();
271+
$this->customerRepository->expects($this->atLeastOnce())
272+
->method('getById')
273+
->with($customerId)->willReturn($customerDataMock);
274+
$this->resource->expects($this->atLeastOnce())
275+
->method('loadByCustomerData')
276+
->with($customerDataMock)
277+
->willReturn(
278+
[
279+
'subscriber_id' => 1,
280+
'subscriber_status' => 3
281+
]
282+
);
283+
$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
284+
$this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();
285+
$customerDataMock->expects($this->once())->method('getStoreId')->willReturn('store_id');
286+
$customerDataMock->expects($this->once())->method('getEmail')->willReturn('email');
287+
$this->sendEmailCheck();
288+
$this->customerAccountManagement->expects($this->once())
289+
->method('getConfirmationStatus')
290+
->willReturn(\Magento\Customer\Api\AccountManagementInterface::ACCOUNT_CONFIRMATION_NOT_REQUIRED);
291+
$this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);
292+
293+
$this->subscriber->subscribeCustomerById($customerId);
294+
$this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_NOT_ACTIVE, $this->subscriber->getStatus());
295+
}
296+
297+
public function testSubscribeCustomerByIdAfterConfirmation()
298+
{
299+
$customerId = 1;
300+
$customerDataMock = $this->getMockBuilder(\Magento\Customer\Api\Data\CustomerInterface::class)
301+
->getMock();
302+
$this->customerRepository->expects($this->atLeastOnce())
303+
->method('getById')
304+
->with($customerId)->willReturn($customerDataMock);
305+
$this->resource->expects($this->atLeastOnce())
306+
->method('loadByCustomerData')
307+
->with($customerDataMock)
308+
->willReturn(
309+
[
310+
'subscriber_id' => 1,
311+
'subscriber_status' => 4
312+
]
313+
);
314+
$customerDataMock->expects($this->atLeastOnce())->method('getId')->willReturn('id');
315+
$this->resource->expects($this->atLeastOnce())->method('save')->willReturnSelf();
316+
$customerDataMock->expects($this->once())->method('getStoreId')->willReturn('store_id');
317+
$customerDataMock->expects($this->once())->method('getEmail')->willReturn('email');
318+
$this->sendEmailCheck();
319+
$this->customerAccountManagement->expects($this->never())->method('getConfirmationStatus');
320+
$this->scopeConfig->expects($this->atLeastOnce())->method('getValue')->with()->willReturn(true);
321+
322+
$this->subscriber->updateSubscription($customerId);
323+
$this->assertEquals(\Magento\Newsletter\Model\Subscriber::STATUS_SUBSCRIBED, $this->subscriber->getStatus());
324+
}
325+
260326
public function testUnsubscribe()
261327
{
262328
$this->resource->expects($this->once())->method('save')->willReturnSelf();

0 commit comments

Comments
 (0)