Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge branch 'next-22891/fix-newsletter-route-does-not-handle-double-…
…optin-setting-correctly' into '6.4.18.1'

NEXT-22891 - Fix newsletter route does not handle double optin setting correctly

See merge request shopware/6/product/platform!8819
  • Loading branch information
pweyck committed Jan 3, 2023
2 parents b45e77c + 5da3b31 commit f5a95ee
Show file tree
Hide file tree
Showing 15 changed files with 231 additions and 71 deletions.
@@ -0,0 +1,12 @@
---
title: Fix newsletter route does not handle double optin setting correctly
issue: NEXT-22891
author: Michel Bade
author_email: m.bade@shopware.com
---
# API
* Changed option selection behavior in `src/Core/Content/Newsletter/SalesChannel/NewsletterSubscribeRoute.php` to remove client side decision on newsletter double opt-in
___
# Storefront
* Added missing snippets for newsletter double opt-in in `src/Storefront/Controller/NewsletterController.php`
* Changed client side decision on newsletter double opt-in in `src/Storefront/Controller/NewsletterController.php`
30 changes: 0 additions & 30 deletions phpstan-baseline.neon
Expand Up @@ -25361,16 +25361,6 @@ parameters:
count: 1
path: src/Storefront/Controller/FormController.php

-
message: "#^Cannot call method getCity\\(\\) on Shopware\\\\Core\\\\Checkout\\\\Customer\\\\Aggregate\\\\CustomerAddress\\\\CustomerAddressEntity\\|null\\.$#"
count: 1
path: src/Storefront/Controller/NewsletterController.php

-
message: "#^Cannot call method getZipCode\\(\\) on Shopware\\\\Core\\\\Checkout\\\\Customer\\\\Aggregate\\\\CustomerAddress\\\\CustomerAddressEntity\\|null\\.$#"
count: 1
path: src/Storefront/Controller/NewsletterController.php

-
message: "#^Method Shopware\\\\Storefront\\\\Controller\\\\ScriptController\\:\\:renderStorefront\\(\\) has parameter \\$parameters with no value type specified in iterable type array\\.$#"
count: 1
Expand Down Expand Up @@ -25971,26 +25961,6 @@ parameters:
count: 1
path: src/Storefront/Test/Controller/CookieControllerTest.php

-
message: "#^Cannot call method getCustomer\\(\\) on Shopware\\\\Core\\\\System\\\\SalesChannel\\\\SalesChannelContext\\|null\\.$#"
count: 1
path: src/Storefront/Test/Controller/NewsletterControllerTest.php

-
message: "#^Parameter \\#3 \\$message of static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertEquals\\(\\) expects string, string\\|null given\\.$#"
count: 2
path: src/Storefront/Test/Controller/NewsletterControllerTest.php

-
message: "#^Parameter \\#3 \\$message of static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertSame\\(\\) expects string, string\\|false given\\.$#"
count: 4
path: src/Storefront/Test/Controller/NewsletterControllerTest.php

-
message: "#^Property Shopware\\\\Storefront\\\\Test\\\\Controller\\\\NewsletterControllerTest\\:\\:\\$customerData type has no value type specified in iterable type array\\.$#"
count: 1
path: src/Storefront/Test/Controller/NewsletterControllerTest.php

-
message: "#^Cannot call method getCustomer\\(\\) on Shopware\\\\Core\\\\System\\\\SalesChannel\\\\SalesChannelContext\\|null\\.$#"
count: 1
Expand Down
Expand Up @@ -175,7 +175,7 @@ public function subscribe(RequestDataBag $dataBag, SalesChannelContext $context,

$recipient = $this->getNewsletterRecipient('email', $data['email'], $context->getContext());

if ($data['status'] === self::STATUS_DIRECT) {
if (!$this->systemConfigService->getBool('core.newsletter.doubleOptIn')) {
$event = new NewsletterConfirmEvent($context->getContext(), $recipient, $context->getSalesChannel()->getId());
$this->eventDispatcher->dispatch($event);

Expand Down Expand Up @@ -262,8 +262,8 @@ private function getNewsletterRecipientId(string $email, SalesChannelContext $co
private function getOptionSelection(): array
{
return [
self::OPTION_DIRECT => self::STATUS_DIRECT,
self::OPTION_SUBSCRIBE => self::STATUS_NOT_SET,
self::OPTION_DIRECT => $this->systemConfigService->getBool('core.newsletter.doubleOptIn') ? self::STATUS_NOT_SET : self::STATUS_DIRECT,
self::OPTION_SUBSCRIBE => $this->systemConfigService->getBool('core.newsletter.doubleOptIn') ? self::STATUS_NOT_SET : self::STATUS_DIRECT,
self::OPTION_CONFIRM_SUBSCRIBE => self::STATUS_OPT_IN,
self::OPTION_UNSUBSCRIBE => self::STATUS_OPT_OUT,
];
Expand Down
25 changes: 20 additions & 5 deletions src/Storefront/Controller/NewsletterController.php
Expand Up @@ -14,6 +14,7 @@
use Shopware\Core\Framework\Validation\DataBag\QueryDataBag;
use Shopware\Core\Framework\Validation\DataBag\RequestDataBag;
use Shopware\Core\System\SalesChannel\SalesChannelContext;
use Shopware\Core\System\SystemConfig\SystemConfigService;
use Shopware\Storefront\Framework\Routing\RequestTransformer;
use Shopware\Storefront\Page\Newsletter\Subscribe\NewsletterSubscribePageLoader;
use Shopware\Storefront\Pagelet\Newsletter\Account\NewsletterAccountPageletLoader;
Expand Down Expand Up @@ -61,6 +62,8 @@ class NewsletterController extends StorefrontController

private NewsletterAccountPageletLoader $newsletterAccountPageletLoader;

private SystemConfigService $systemConfigService;

/**
* @internal
*/
Expand All @@ -70,7 +73,8 @@ public function __construct(
AbstractNewsletterSubscribeRoute $newsletterSubscribeRoute,
AbstractNewsletterConfirmRoute $newsletterConfirmRoute,
AbstractNewsletterUnsubscribeRoute $newsletterUnsubscribeRoute,
NewsletterAccountPageletLoader $newsletterAccountPageletLoader
NewsletterAccountPageletLoader $newsletterAccountPageletLoader,
SystemConfigService $systemConfigService
) {
$this->newsletterConfirmRegisterPageLoader = $newsletterConfirmRegisterPageLoader;
/* @deprecated tag:v6.5.0 - remove next line */
Expand All @@ -81,6 +85,7 @@ public function __construct(
/* @deprecated tag:v6.5.0 - remove next line */
$this->newsletterUnsubscribeRoute = $newsletterUnsubscribeRoute;
$this->newsletterAccountPageletLoader = $newsletterAccountPageletLoader;
$this->systemConfigService = $systemConfigService;
}

/**
Expand Down Expand Up @@ -118,7 +123,7 @@ public function subscribeCustomer(Request $request, RequestDataBag $dataBag, Sal
'newsletterAccountPagelet' => $pagelet,
]);
}
$subscribed = $request->get('option', false) === 'direct';
$subscribed = $request->get('option', false) === 'subscribe' || $request->get('option', false) === 'direct';

if (!$subscribed) {
$dataBag->set('option', 'unsubscribe');
Expand All @@ -139,7 +144,11 @@ public function subscribeCustomer(Request $request, RequestDataBag $dataBag, Sal
$this->setNewsletterFlag($customer, true, $context);

$success = true;
$messages[] = ['type' => 'success', 'text' => $this->trans('newsletter.subscriptionConfirmationSuccess')];
if ($this->systemConfigService->getBool('core.newsletter.doubleOptIn')) {
$messages[] = ['type' => 'info', 'text' => $this->trans('newsletter.subscriptionConfirmationNeeded')];
} else {
$messages[] = ['type' => 'success', 'text' => $this->trans('newsletter.subscriptionConfirmationSuccess')];
}
} catch (\Exception $exception) {
$success = false;
$messages[] = ['type' => 'danger', 'text' => $this->trans('newsletter.subscriptionConfirmationFailed')];
Expand Down Expand Up @@ -183,8 +192,14 @@ private function hydrateFromCustomer(RequestDataBag $dataBag, CustomerEntity $cu
$dataBag->set('title', $customer->getTitle());
$dataBag->set('firstName', $customer->getFirstName());
$dataBag->set('lastName', $customer->getLastName());
$dataBag->set('zipCode', $customer->getDefaultShippingAddress()->getZipCode());
$dataBag->set('city', $customer->getDefaultShippingAddress()->getCity());
$dataBag->set(
'zipCode',
($customer->getDefaultShippingAddress() ? $customer->getDefaultShippingAddress()->getZipCode() : null)
);
$dataBag->set(
'city',
($customer->getDefaultShippingAddress() ? $customer->getDefaultShippingAddress()->getCity() : null)
);
$dataBag->set(
'street',
($customer->getDefaultShippingAddress() ? $customer->getDefaultShippingAddress()->getStreet() : null)
Expand Down
1 change: 1 addition & 0 deletions src/Storefront/DependencyInjection/controller.xml
Expand Up @@ -231,6 +231,7 @@
<argument type="service" id="Shopware\Core\Content\Newsletter\SalesChannel\NewsletterConfirmRoute"/>
<argument type="service" id="Shopware\Core\Content\Newsletter\SalesChannel\NewsletterUnsubscribeRoute"/>
<argument type="service" id="Shopware\Storefront\Pagelet\Newsletter\Account\NewsletterAccountPageletLoader"/>
<argument type="service" id="Shopware\Core\System\SystemConfig\SystemConfigService"/>

<call method="setContainer">
<argument type="service" id="service_container"/>
Expand Down
Expand Up @@ -470,6 +470,7 @@
"subscriptionPersistedInfo": "Sollten Sie keine Mail erhalten haben, wiederholen Sie den Vorgang oder wenden Sie sich an den Support.",
"subscriptionRevokeSuccess": "Sie haben sich erfolgreich vom Newsletter abgemeldet.",
"subscriptionConfirmationSuccess": "Ihr Newsletter-Abonnement wurde erfolgreich registriert.",
"subscriptionConfirmationNeeded": "Wir haben Ihnen eine Bestätigungsmail geschickt. Bitte prüfen Sie Ihr E-Mail-Postfach und klicken Sie auf den enthaltenen Link, um ihr Newsletter-Abonnement abzuschließen.",
"subscriptionConfirmationFailed": "Bei Ihrer Newsletter-Anmeldung ist etwas schief gelaufen. Bitte wenden Sie sich an den Support.",
"subscriptionCompleted": "Vielen Dank. Wir haben Ihre E-Mail-Adresse eingetragen.",
"labelActionSelect": "Aktion",
Expand Down
Expand Up @@ -470,6 +470,7 @@
"subscriptionPersistedInfo": "If you did not received an email, please repeat the process or contact our support team.",
"subscriptionRevokeSuccess": "You have unsubscribed from the newsletter.",
"subscriptionConfirmationSuccess": "You have subscribed to the newsletter.",
"subscriptionConfirmationNeeded": "We have sent a confirmation email containing an activation link. Please check your inbox and click the link to complete your newsletter subscription.",
"subscriptionConfirmationFailed": "Newsletter subscription did not work properly, please contact our support team.",
"subscriptionCompleted": "Thank you! We have registered your address.",
"labelActionSelect": "Action",
Expand Down
Expand Up @@ -65,7 +65,7 @@
id="newsletterRegister"
name="option"
autocomplete="off"
value="{% if newsletterAccountPagelet.newsletterDoi %}{{ subscribe }}{% else %}{{ direct }}{% endif %}"
value="subscribe"
{% if status %}checked="checked"{% endif %}>
{% endblock %}

Expand Down
Expand Up @@ -42,7 +42,7 @@ describe('Storefront profile settings', () => {
});
cy.get('label[for="newsletterRegister"]').click();
cy.wait('@checkNewsletter').its('response.statusCode').should('equal', 200);
cy.contains('You have subscribed to the newsletter.').should('exist');
cy.contains('We have sent a confirmation email containing an activation link. Please check your inbox and click the link to complete your newsletter subscription.').should('exist');

// Verify the subscription from the newsletter recipients
cy.visit(`${Cypress.env('admin')}#/sw/newsletter/recipient/index`);
Expand Down
Expand Up @@ -9,15 +9,17 @@
use Shopware\Core\Defaults;
use Shopware\Core\Framework\Context;
use Shopware\Core\Framework\DataAbstractionLayer\EntityRepositoryInterface;
use Shopware\Core\Framework\Test\IdsCollection;
use Shopware\Core\Framework\Test\TestCaseBase\IntegrationTestBehaviour;
use Shopware\Core\Framework\Test\TestDataCollection;
use Shopware\Core\Framework\Uuid\Uuid;
use Shopware\Core\System\SystemConfig\SystemConfigService;
use Shopware\Core\Test\TestDefaults;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;

/**
* @internal
*
* @covers \Shopware\Core\Checkout\Customer\SalesChannel\ChangeEmailRoute
*/
class ChangeEmailRouteTest extends TestCase
{
Expand All @@ -26,13 +28,13 @@ class ChangeEmailRouteTest extends TestCase

private KernelBrowser $browser;

private TestDataCollection $ids;
private IdsCollection $ids;

private EntityRepositoryInterface $customerRepository;

protected function setUp(): void
{
$this->ids = new TestDataCollection();
$this->ids = new IdsCollection();

$this->browser = $this->createCustomSalesChannelBrowser([
'id' => $this->ids->create('sales-channel'),
Expand All @@ -56,6 +58,10 @@ protected function setUp(): void
$response = json_decode((string) $this->browser->getResponse()->getContent(), true);

$this->browser->setServerParameter('HTTP_SW_CONTEXT_TOKEN', $response['contextToken']);

$systemConfig = $this->getContainer()->get(SystemConfigService::class);
static::assertNotNull($systemConfig);
$systemConfig->set('core.newsletter.doubleOptIn', false);
}

public function testEmptyRequest(): void
Expand Down
Expand Up @@ -15,6 +15,7 @@
use Shopware\Core\Framework\Test\TestCaseBase\IntegrationTestBehaviour;
use Shopware\Core\Framework\Test\TestDataCollection;
use Shopware\Core\Framework\Uuid\Uuid;
use Shopware\Core\System\SystemConfig\SystemConfigService;
use Shopware\Core\Test\TestDefaults;

/**
Expand Down Expand Up @@ -73,6 +74,10 @@ protected function setUp(): void
$response = json_decode((string) $this->browser->getResponse()->getContent(), true);

$this->browser->setServerParameter('HTTP_SW_CONTEXT_TOKEN', $response['contextToken']);

$systemConfig = $this->getContainer()->get(SystemConfigService::class);
static::assertNotNull($systemConfig);
$systemConfig->set('core.newsletter.doubleOptIn', false);
}

public function testEmptyRequest(): void
Expand Down
Expand Up @@ -21,6 +21,7 @@
/**
* @internal
* @group store-api
* @covers \Shopware\Core\Content\Newsletter\SalesChannel\NewsletterSubscribeRoute
*/
class NewsletterSubscribeRouteTest extends TestCase
{
Expand All @@ -33,6 +34,8 @@ class NewsletterSubscribeRouteTest extends TestCase

private string $salesChannelId;

private SystemConfigService $systemConfig;

protected function setUp(): void
{
$this->ids = new TestDataCollection();
Expand All @@ -42,6 +45,10 @@ protected function setUp(): void
$this->browser = $this->createCustomSalesChannelBrowser([
'id' => $this->ids->create('sales-channel'),
]);

$this->systemConfig = $this->getContainer()->get(SystemConfigService::class);
static::assertNotNull($this->systemConfig);
$this->systemConfig->set('core.newsletter.doubleOptIn', false);
}

public function testSubscribeWithoutFields(): void
Expand Down Expand Up @@ -90,6 +97,8 @@ public function testSubscribeWithInvalidStorefrontUrl(): void

public function testResubscribeAfterUnsubscribe(): void
{
$this->systemConfig->set('core.newsletter.doubleOptIn', true);

$connection = $this->getContainer()->get(Connection::class);
$newsletterRecipientRepository = $this->getContainer()->get('newsletter_recipient.repository');

Expand Down Expand Up @@ -205,11 +214,10 @@ public function testSubscribeIfAlreadyRegistered(): void

public function testSubscribeChangedConfirmUrl(): void
{
$systemConfig = $this->getContainer()->get(SystemConfigService::class);
$this->systemConfig->set('core.newsletter.doubleOptIn', true);

try {
$systemConfig->set('core.newsletter.doubleOptIn', true);
$systemConfig->set('core.newsletter.subscribeUrl', '/custom-newsletter/confirm/%%HASHEDEMAIL%%/%%SUBSCRIBEHASH%%');
$this->systemConfig->set('core.newsletter.subscribeUrl', '/custom-newsletter/confirm/%%HASHEDEMAIL%%/%%SUBSCRIBEHASH%%');

/** @var EventDispatcherInterface $dispatcher */
$dispatcher = $this->getContainer()->get('event_dispatcher');
Expand Down Expand Up @@ -248,19 +256,17 @@ static function (NewsletterRegisterEvent $event) use (&$caughtEvent): void {
static::assertStringStartsWith('http://localhost/custom-newsletter/confirm/', $caughtEvent->getUrl());
static::assertStringEndsWith('?specialParam=false', $caughtEvent->getUrl());
} finally {
$systemConfig->set('core.newsletter.doubleOptIn', false);
$systemConfig->set('core.newsletter.subscribeUrl', null);
$this->systemConfig->set('core.newsletter.subscribeUrl', null);
}
}

public function testSubscribeChangedConfirmDomain(): void
{
Feature::skipTestIfInActive('FEATURE_NEXT_16200', $this);

$systemConfig = $this->getContainer()->get(SystemConfigService::class);
$this->systemConfig->set('core.newsletter.doubleOptIn', true);

try {
$systemConfig->set('core.newsletter.doubleOptInDomain', 'http://test.test');
$this->systemConfig->set('core.newsletter.doubleOptInDomain', 'http://test.test');

/** @var EventDispatcherInterface $dispatcher */
$dispatcher = $this->getContainer()->get('event_dispatcher');
Expand Down Expand Up @@ -289,7 +295,7 @@ static function (NewsletterRegisterEvent $event) use (&$caughtEvent): void {
static::assertInstanceOf(NewsletterRegisterEvent::class, $caughtEvent);
static::assertStringStartsWith('http://test.test/newsletter-subscribe?em=', $caughtEvent->getUrl());
} finally {
$systemConfig->set('core.newsletter.doubleOptInDomain', null, $this->salesChannelId);
$this->systemConfig->set('core.newsletter.doubleOptInDomain', null, $this->salesChannelId);
}
}

Expand Down
@@ -1,19 +1,21 @@
<?php declare(strict_types=1);

namespace Shopware\Core\Content\Test\Newsletter\SalesChannel;
namespace Shopware\Tests\Integration\Core\Content\Newsletter\SalesChannel;

use Doctrine\DBAL\Connection;
use PHPUnit\Framework\TestCase;
use Shopware\Core\Content\Newsletter\Event\NewsletterUnsubscribeEvent;
use Shopware\Core\Framework\Test\IdsCollection;
use Shopware\Core\Framework\Test\TestCaseBase\IntegrationTestBehaviour;
use Shopware\Core\Framework\Test\TestCaseBase\SalesChannelApiTestBehaviour;
use Shopware\Core\Framework\Test\TestCaseHelper\CallableClass;
use Shopware\Core\Framework\Test\TestDataCollection;
use Shopware\Core\System\SystemConfig\SystemConfigService;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;

/**
* @internal
* @group store-api
* @covers \Shopware\Core\Content\Newsletter\SalesChannel\NewsletterUnsubscribeRoute
*/
class NewsletterUnsubscribeRouteTest extends TestCase
{
Expand All @@ -22,15 +24,19 @@ class NewsletterUnsubscribeRouteTest extends TestCase

private KernelBrowser $browser;

private TestDataCollection $ids;
private IdsCollection $ids;

protected function setUp(): void
{
$this->ids = new TestDataCollection();
$this->ids = new IdsCollection();

$this->browser = $this->createCustomSalesChannelBrowser([
'id' => $this->ids->create('sales-channel'),
]);

$systemConfig = $this->getContainer()->get(SystemConfigService::class);
static::assertNotNull($systemConfig);
$systemConfig->set('core.newsletter.doubleOptIn', false);
}

public function testUnsubscribe(): void
Expand Down

0 comments on commit f5a95ee

Please sign in to comment.