From 1a7c8cd619b4936fd544ac669bf4f5e66bcc7d34 Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Thu, 13 Aug 2020 10:51:33 +0530 Subject: [PATCH 1/4] Fixed the resending Newsletter confirmation issue --- app/code/Magento/Newsletter/Model/SubscriptionManager.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Newsletter/Model/SubscriptionManager.php b/app/code/Magento/Newsletter/Model/SubscriptionManager.php index 846d095625e0c..eac43e1bcea5f 100644 --- a/app/code/Magento/Newsletter/Model/SubscriptionManager.php +++ b/app/code/Magento/Newsletter/Model/SubscriptionManager.php @@ -200,6 +200,7 @@ private function saveSubscriber( && (int)$subscriber->getCustomerId() === (int)$customer->getId() && (int)$subscriber->getStoreId() === $storeId && !$emailChanged + && $status !== Subscriber::STATUS_NOT_ACTIVE ) { return false; } @@ -220,10 +221,10 @@ private function saveSubscriber( /** * If the subscriber is waiting to confirm from the customer - * and customer changed the email + * or customer changed the email * than need to send confirmation letter to the new email */ - return $status === Subscriber::STATUS_NOT_ACTIVE && $emailChanged; + return $status === Subscriber::STATUS_NOT_ACTIVE || $emailChanged; } /** From 6e53e0d90b3cbc86ee368c35605615d713fe9682 Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Fri, 14 Aug 2020 11:04:05 +0530 Subject: [PATCH 2/4] Fixed Unit Tests --- .../Newsletter/Test/Unit/Model/SubscriptionManagerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Newsletter/Test/Unit/Model/SubscriptionManagerTest.php b/app/code/Magento/Newsletter/Test/Unit/Model/SubscriptionManagerTest.php index 4e1f18a26a95a..6139d86191f44 100644 --- a/app/code/Magento/Newsletter/Test/Unit/Model/SubscriptionManagerTest.php +++ b/app/code/Magento/Newsletter/Test/Unit/Model/SubscriptionManagerTest.php @@ -454,7 +454,7 @@ public function subscribeCustomerDataProvider(): array 'subscriber_status' => Subscriber::STATUS_SUBSCRIBED, 'subscriber_confirm_code' => '', ], - 'needToSendEmail' => false, + 'needToSendEmail' => true, ], 'Update subscription data: subscription confirm required ' => [ 'subscriber_data' => [ @@ -618,7 +618,7 @@ public function unsubscribeCustomerDataProvider(): array 'subscriber_status' => Subscriber::STATUS_NOT_ACTIVE, 'subscriber_confirm_code' => '', ], - 'needToSendEmail' => false, + 'needToSendEmail' => true, ], 'Update subscription data' => [ 'subscriber_data' => [ @@ -642,7 +642,7 @@ public function unsubscribeCustomerDataProvider(): array 'subscriber_status' => Subscriber::STATUS_UNSUBSCRIBED, 'subscriber_confirm_code' => '', ], - 'needToSendEmail' => false, + 'needToSendEmail' => true, ], ]; } From ea584f9815ec3b119e9ac73ccc773435c7727960 Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Wed, 19 Aug 2020 11:01:04 +0530 Subject: [PATCH 3/4] Fixed cyclomatic complexity --- .../Newsletter/Model/SubscriptionManager.php | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/Newsletter/Model/SubscriptionManager.php b/app/code/Magento/Newsletter/Model/SubscriptionManager.php index eac43e1bcea5f..f01b4784581c3 100644 --- a/app/code/Magento/Newsletter/Model/SubscriptionManager.php +++ b/app/code/Magento/Newsletter/Model/SubscriptionManager.php @@ -195,13 +195,14 @@ private function saveSubscriber( ): bool { $statusChanged = (int)$subscriber->getStatus() !== $status; $emailChanged = $subscriber->getEmail() !== $customer->getEmail(); - if ($subscriber->getId() - && !$statusChanged - && (int)$subscriber->getCustomerId() === (int)$customer->getId() - && (int)$subscriber->getStoreId() === $storeId - && !$emailChanged - && $status !== Subscriber::STATUS_NOT_ACTIVE - ) { + if ($this->dontNeedToSaveSubscriber( + $subscriber, + $customer, + $statusChanged, + $storeId, + $status, + $emailChanged + )) { return false; } @@ -227,6 +228,32 @@ private function saveSubscriber( return $status === Subscriber::STATUS_NOT_ACTIVE || $emailChanged; } + /** + * + * @param Subscriber $subscriber + * @param CustomerInterface $customer + * @param bool $statusChanged + * @param int $storeId + * @param int $status + * @param bool $emailChanged + * @return bool + */ + private function dontNeedToSaveSubscriber( + Subscriber $subscriber, + CustomerInterface $customer, + bool $statusChanged, + int $storeId, + int $status, + bool $emailChanged + ): bool { + return $subscriber->getId() + && !$statusChanged + && (int)$subscriber->getCustomerId() === (int)$customer->getId() + && (int)$subscriber->getStoreId() === $storeId + && !$emailChanged + && $status !== Subscriber::STATUS_NOT_ACTIVE; + } + /** * Get new subscription status * From ab3db167e1594e935350d4be6494ce1f56c48b4f Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Wed, 19 Aug 2020 12:30:25 +0530 Subject: [PATCH 4/4] Fixed static test --- app/code/Magento/Newsletter/Model/SubscriptionManager.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Newsletter/Model/SubscriptionManager.php b/app/code/Magento/Newsletter/Model/SubscriptionManager.php index f01b4784581c3..57c6cd8b843a7 100644 --- a/app/code/Magento/Newsletter/Model/SubscriptionManager.php +++ b/app/code/Magento/Newsletter/Model/SubscriptionManager.php @@ -229,6 +229,7 @@ private function saveSubscriber( } /** + * Don't need to save subscriber model * * @param Subscriber $subscriber * @param CustomerInterface $customer