Skip to content

Commit 1d03b99

Browse files
Indrani SonawaneIndrani Sonawane
authored andcommitted
Merge remote-tracking branch '33375/33040_newsletter_subscription_bugfix' into 247beta3_jan
2 parents ebbb127 + 3d70958 commit 1d03b99

File tree

2 files changed

+64
-8
lines changed
  • app/code/Magento/Newsletter/Controller/Subscriber
  • dev/tests/integration/testsuite/Magento/Newsletter/Controller/Subscriber

2 files changed

+64
-8
lines changed

app/code/Magento/Newsletter/Controller/Subscriber/NewAction.php

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* Copyright © Magento, Inc. All rights reserved.
55
* See COPYING.txt for license details.
66
*/
7+
declare(strict_types=1);
8+
79
namespace Magento\Newsletter\Controller\Subscriber;
810

911
use Magento\Customer\Api\AccountManagementInterface as CustomerAccountManagement;
@@ -24,6 +26,7 @@
2426
use Magento\Newsletter\Model\SubscriptionManagerInterface;
2527
use Magento\Store\Model\ScopeInterface;
2628
use Magento\Store\Model\StoreManagerInterface;
29+
use Magento\Newsletter\Model\Config as NewsletterConfig;
2730
use Magento\Newsletter\Model\SubscriberFactory;
2831

2932
/**
@@ -43,6 +46,11 @@ class NewAction extends SubscriberController implements HttpPostActionInterface
4346
*/
4447
private $emailValidator;
4548

49+
/**
50+
* @var NewsletterConfig
51+
*/
52+
private $newsletterConfig;
53+
4654
/**
4755
* @var SubscriptionManagerInterface
4856
*/
@@ -65,6 +73,8 @@ class NewAction extends SubscriberController implements HttpPostActionInterface
6573
* @param SubscriptionManagerInterface $subscriptionManager
6674
* @param EmailValidator|null $emailValidator
6775
* @param CustomerRepositoryInterface|null $customerRepository
76+
* @param NewsletterConfig|null $newsletterConfig
77+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
6878
*/
6979
public function __construct(
7080
Context $context,
@@ -75,13 +85,16 @@ public function __construct(
7585
CustomerAccountManagement $customerAccountManagement,
7686
SubscriptionManagerInterface $subscriptionManager,
7787
EmailValidator $emailValidator = null,
78-
CustomerRepositoryInterface $customerRepository = null
88+
CustomerRepositoryInterface $customerRepository = null,
89+
NewsletterConfig $newsletterConfig = null
7990
) {
8091
$this->customerAccountManagement = $customerAccountManagement;
8192
$this->subscriptionManager = $subscriptionManager;
8293
$this->emailValidator = $emailValidator ?: ObjectManager::getInstance()->get(EmailValidator::class);
8394
$this->customerRepository = $customerRepository ?: ObjectManager::getInstance()
8495
->get(CustomerRepositoryInterface::class);
96+
$this->newsletterConfig = $newsletterConfig?: ObjectManager::getInstance()
97+
->get(NewsletterConfig::class);
8598
parent::__construct(
8699
$context,
87100
$subscriberFactory,
@@ -95,8 +108,9 @@ public function __construct(
95108
* Validates that the email address isn't being used by a different account.
96109
*
97110
* @param string $email
98-
* @throws LocalizedException
111+
*
99112
* @return void
113+
* @throws LocalizedException
100114
*/
101115
protected function validateEmailAvailable($email)
102116
{
@@ -139,8 +153,9 @@ protected function validateGuestSubscription()
139153
* Validates the format of the email address
140154
*
141155
* @param string $email
142-
* @throws LocalizedException
156+
*
143157
* @return void
158+
* @throws LocalizedException
144159
*/
145160
protected function validateEmailFormat($email)
146161
{
@@ -156,7 +171,10 @@ protected function validateEmailFormat($email)
156171
*/
157172
public function execute()
158173
{
159-
if ($this->getRequest()->isPost() && $this->getRequest()->getPost('email')) {
174+
if ($this->getRequest()->isPost()
175+
&& $this->getRequest()->getPost('email')
176+
&& $this->newsletterConfig->isActive()
177+
) {
160178
$email = (string)$this->getRequest()->getPost('email');
161179

162180
try {
@@ -195,6 +213,7 @@ public function execute()
195213
$redirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT);
196214
// phpcs:ignore Magento2.Legacy.ObsoleteResponse
197215
$redirectUrl = $this->_redirect->getRedirectUrl();
216+
198217
return $redirect->setUrl($redirectUrl);
199218
}
200219

@@ -203,6 +222,7 @@ public function execute()
203222
*
204223
* @param string $email
205224
* @param int $websiteId
225+
*
206226
* @return int|null
207227
*/
208228
private function getCustomerId(string $email, int $websiteId): ?int
@@ -219,6 +239,7 @@ private function getCustomerId(string $email, int $websiteId): ?int
219239
* Get success message
220240
*
221241
* @param int $status
242+
*
222243
* @return Phrase
223244
*/
224245
private function getSuccessMessage(int $status): Phrase

dev/tests/integration/testsuite/Magento/Newsletter/Controller/Subscriber/NewActionTest.php

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
namespace Magento\Newsletter\Controller\Subscriber;
99

10+
use Exception;
11+
use Laminas\Stdlib\Parameters;
1012
use Magento\Customer\Api\CustomerRepositoryInterface;
1113
use Magento\Customer\Model\AccountManagement;
1214
use Magento\Customer\Model\Session;
@@ -18,7 +20,6 @@
1820
use Magento\Newsletter\Model\ResourceModel\Subscriber\Grid\Collection as GridCollection;
1921
use Magento\Store\Model\ScopeInterface;
2022
use Magento\TestFramework\TestCase\AbstractController;
21-
use Laminas\Stdlib\Parameters;
2223

2324
/**
2425
* Class checks subscription behaviour from frontend
@@ -28,6 +29,9 @@
2829
*/
2930
class NewActionTest extends AbstractController
3031
{
32+
/** @var CustomerRepositoryInterface */
33+
private $customerRepository;
34+
3135
/** @var Session */
3236
private $session;
3337

@@ -40,9 +44,6 @@ class NewActionTest extends AbstractController
4044
/** @var string|null */
4145
private $subscriberToDelete;
4246

43-
/** @var CustomerRepositoryInterface */
44-
private $customerRepository;
45-
4647
/** @var Url */
4748
private $customerUrl;
4849

@@ -88,6 +89,38 @@ public function testNewAction(string $email, string $expectedMessage): void
8889
$this->performAsserts($expectedMessage);
8990
}
9091

92+
/**
93+
* @magentoConfigFixture newsletter/general/active 1
94+
*
95+
* @return void
96+
*/
97+
public function testNewActionWithSubscriptionConfigEnabled(): void
98+
{
99+
$email = 'good_subscription@example.com';
100+
$this->subscriberToDelete = $email;
101+
$this->prepareRequest($email);
102+
$this->dispatch('newsletter/subscriber/new');
103+
$subscriberCollection = $this->subscriberCollectionFactory->create();
104+
$subscriberCollection->addFieldToFilter('subscriber_email', $email)->setPageSize(1);
105+
$this->assertEquals(1, count($subscriberCollection));
106+
$this->assertEquals($email, $subscriberCollection->getFirstItem()->getEmail());
107+
}
108+
109+
/**
110+
* @magentoConfigFixture newsletter/general/active 0
111+
*
112+
* @return void
113+
*/
114+
public function testNewActionWithSubscriptionConfigDisabled(): void
115+
{
116+
$email = 'bad_subscription@example.com';
117+
$this->prepareRequest($email);
118+
$this->dispatch('newsletter/subscriber/new');
119+
$subscriberCollection = $this->subscriberCollectionFactory->create();
120+
$subscriberCollection->addFieldToFilter('subscriber_email', $email)->setPageSize(1);
121+
$this->assertEquals(0, count($subscriberCollection));
122+
}
123+
91124
/**
92125
* @return array
93126
*/
@@ -272,7 +305,9 @@ private function performAsserts(string $message): void
272305
* Delete subscribers by email
273306
*
274307
* @param string $email
308+
*
275309
* @return void
310+
* @throws Exception
276311
*/
277312
private function deleteSubscriber(string $email): void
278313
{

0 commit comments

Comments
 (0)