Skip to content

Commit 2c587aa

Browse files
tatevikg1tatevikg1
committed
Do not send email on creating without any list
1 parent b01f665 commit 2c587aa

File tree

4 files changed

+36
-77
lines changed

4 files changed

+36
-77
lines changed

‎src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Doctrine\ORM\EntityManagerInterface;
88
use PhpList\Core\Domain\Subscription\Exception\SubscriberAttributeCreationException;
9+
use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto;
910
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1011
use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeDefinition;
1112
use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeValue;
@@ -71,7 +72,7 @@ public function processAttributes(Subscriber $subscriber, array $attributeData):
7172
{
7273
foreach ($attributeData as $key => $value) {
7374
$lowerKey = strtolower((string)$key);
74-
if (in_array($lowerKey, ['password', 'modified'], true)) {
75+
if (in_array($lowerKey, ChangeSetDto::IGNORED_ATTRIBUTES, true)) {
7576
continue;
7677
}
7778

‎src/Domain/Subscription/Service/Manager/SubscriberManager.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use Doctrine\ORM\EntityManagerInterface;
88
use PhpList\Core\Domain\Identity\Model\Administrator;
9-
use PhpList\Core\Domain\Messaging\Message\SubscriberConfirmationMessage;
109
use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto;
1110
use PhpList\Core\Domain\Subscription\Model\Dto\CreateSubscriberDto;
1211
use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto;
@@ -15,7 +14,6 @@
1514
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
1615
use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService;
1716
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
18-
use Symfony\Component\Messenger\MessageBusInterface;
1917
use Symfony\Contracts\Translation\TranslatorInterface;
2018

2119
/**
@@ -25,22 +23,19 @@ class SubscriberManager
2523
{
2624
private SubscriberRepository $subscriberRepository;
2725
private EntityManagerInterface $entityManager;
28-
private MessageBusInterface $messageBus;
2926
private SubscriberDeletionService $subscriberDeletionService;
3027
private TranslatorInterface $translator;
3128
private SubscriberHistoryManager $subscriberHistoryManager;
3229

3330
public function __construct(
3431
SubscriberRepository $subscriberRepository,
3532
EntityManagerInterface $entityManager,
36-
MessageBusInterface $messageBus,
3733
SubscriberDeletionService $subscriberDeletionService,
3834
TranslatorInterface $translator,
3935
SubscriberHistoryManager $subscriberHistoryManager,
4036
) {
4137
$this->subscriberRepository = $subscriberRepository;
4238
$this->entityManager = $entityManager;
43-
$this->messageBus = $messageBus;
4439
$this->subscriberDeletionService = $subscriberDeletionService;
4540
$this->translator = $translator;
4641
$this->subscriberHistoryManager = $subscriberHistoryManager;
@@ -58,24 +53,9 @@ public function createSubscriber(CreateSubscriberDto $subscriberDto): Subscriber
5853

5954
$this->subscriberRepository->save($subscriber);
6055

61-
if ($subscriberDto->requestConfirmation) {
62-
$this->sendConfirmationEmail($subscriber);
63-
}
64-
6556
return $subscriber;
6657
}
6758

68-
private function sendConfirmationEmail(Subscriber $subscriber): void
69-
{
70-
$message = new SubscriberConfirmationMessage(
71-
email: $subscriber->getEmail(),
72-
uniqueId:$subscriber->getUniqueId(),
73-
htmlEmail: $subscriber->hasHtmlEmail()
74-
);
75-
76-
$this->messageBus->dispatch($message);
77-
}
78-
7959
public function getSubscriberById(int $subscriberId): ?Subscriber
8060
{
8161
return $this->subscriberRepository->find($subscriberId);
@@ -142,10 +122,6 @@ public function createFromImport(ImportSubscriberDto $subscriberDto): Subscriber
142122

143123
$this->entityManager->persist($subscriber);
144124

145-
if ($subscriberDto->sendConfirmation) {
146-
$this->sendConfirmationEmail($subscriber);
147-
}
148-
149125
return $subscriber;
150126
}
151127

‎src/Domain/Subscription/Service/SubscriberCsvImporter.php‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,22 @@ public function importFromCsv(
9393

9494
foreach ($result['valid'] as $dto) {
9595
try {
96-
$this->processRow($dto, $options, $stats, $admin);
96+
$this->entityManager->beginTransaction();
97+
98+
$message = $this->processRow($dto, $options, $stats, $admin);
99+
100+
if ($options->dryRun) {
101+
$this->entityManager->rollback();
102+
} else {
103+
$this->entityManager->flush();
104+
$this->entityManager->commit();
105+
if ($message !== null) {
106+
$this->messageBus->dispatch($message);
107+
}
108+
}
97109
} catch (Throwable $e) {
110+
$this->entityManager->rollback();
111+
98112
$stats['errors'][] = $this->translator->trans(
99113
'Error processing %email%: %error%',
100114
['%email%' => $dto->email, '%error%' => $e->getMessage()]
@@ -163,14 +177,14 @@ private function processRow(
163177
SubscriberImportOptions $options,
164178
array &$stats,
165179
?Administrator $admin = null
166-
): void {
180+
): ?SubscriptionConfirmationMessage {
167181
if ($this->handleInvalidEmail($dto, $options, $stats)) {
168-
return;
182+
return null;
169183
}
170184

171185
$subscriber = $this->subscriberRepository->findOneByEmail($dto->email);
172186
if ($this->handleSkipCase($subscriber, $options, $stats)) {
173-
return;
187+
return null;
174188
}
175189

176190
if ($subscriber) {
@@ -208,7 +222,8 @@ private function processRow(
208222
changeSetDto: $changeSet ?? new ChangeSetDto(),
209223
admin: $admin
210224
);
211-
$this->handleFlushAndEmail($subscriber, $options, $dto, $addedNewSubscriberToList);
225+
226+
return $this->prepareConfirmationMessage($subscriber, $options, $dto, $addedNewSubscriberToList);
212227
}
213228

214229
private function handleInvalidEmail(
@@ -245,24 +260,21 @@ private function handleSkipCase(
245260
return false;
246261
}
247262

248-
private function handleFlushAndEmail(
263+
private function prepareConfirmationMessage(
249264
Subscriber $subscriber,
250265
SubscriberImportOptions $options,
251266
ImportSubscriberDto $dto,
252267
bool $addedNewSubscriberToList
253-
): void {
254-
if (!$options->dryRun) {
255-
$this->entityManager->flush();
256-
if ($dto->sendConfirmation && $addedNewSubscriberToList) {
257-
$message = new SubscriptionConfirmationMessage(
258-
email: $subscriber->getEmail(),
259-
uniqueId: $subscriber->getUniqueId(),
260-
listIds: $options->listIds,
261-
htmlEmail: $subscriber->hasHtmlEmail(),
262-
);
263-
264-
$this->messageBus->dispatch($message);
265-
}
268+
): ?SubscriptionConfirmationMessage {
269+
if ($dto->sendConfirmation && $addedNewSubscriberToList) {
270+
return new SubscriptionConfirmationMessage(
271+
email: $subscriber->getEmail(),
272+
uniqueId: $subscriber->getUniqueId(),
273+
listIds: $options->listIds,
274+
htmlEmail: $subscriber->hasHtmlEmail(),
275+
);
266276
}
277+
278+
return null;
267279
}
268280
}

‎tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
namespace PhpList\Core\Tests\Unit\Domain\Subscription\Service\Manager;
66

77
use Doctrine\ORM\EntityManagerInterface;
8-
use PhpList\Core\Domain\Messaging\Message\SubscriberConfirmationMessage;
98
use PhpList\Core\Domain\Subscription\Model\Dto\CreateSubscriberDto;
109
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1110
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
@@ -14,28 +13,23 @@
1413
use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService;
1514
use PHPUnit\Framework\MockObject\MockObject;
1615
use PHPUnit\Framework\TestCase;
17-
use Symfony\Component\Messenger\Envelope;
18-
use Symfony\Component\Messenger\MessageBusInterface;
1916
use Symfony\Component\Translation\Translator;
2017

2118
class SubscriberManagerTest extends TestCase
2219
{
2320
private SubscriberRepository|MockObject $subscriberRepository;
2421
private EntityManagerInterface|MockObject $entityManager;
25-
private MessageBusInterface|MockObject $messageBus;
2622
private SubscriberManager $subscriberManager;
2723

2824
protected function setUp(): void
2925
{
3026
$this->subscriberRepository = $this->createMock(SubscriberRepository::class);
3127
$this->entityManager = $this->createMock(EntityManagerInterface::class);
32-
$this->messageBus = $this->createMock(MessageBusInterface::class);
3328
$subscriberDeletionService = $this->createMock(SubscriberDeletionService::class);
3429

3530
$this->subscriberManager = new SubscriberManager(
3631
subscriberRepository: $this->subscriberRepository,
3732
entityManager: $this->entityManager,
38-
messageBus: $this->messageBus,
3933
subscriberDeletionService: $subscriberDeletionService,
4034
translator: new Translator('en'),
4135
subscriberHistoryManager: $this->createMock(SubscriberHistoryManager::class)
@@ -67,7 +61,7 @@ public function testCreateSubscriberPersistsAndReturnsProperlyInitializedEntity(
6761
$this->assertFalse($result->isDisabled());
6862
}
6963

70-
public function testCreateSubscriberPersistsAndSendsEmail(): void
64+
public function testCreateSubscriberPersists(): void
7165
{
7266
$this->subscriberRepository
7367
->expects($this->once())
@@ -81,13 +75,6 @@ public function testCreateSubscriberPersistsAndSendsEmail(): void
8175
&& $sub->isDisabled() === false;
8276
}));
8377

84-
$this->messageBus
85-
->expects($this->once())
86-
->method('dispatch')
87-
->willReturnCallback(function ($message) {
88-
return new Envelope($message);
89-
});
90-
9178
$dto = new CreateSubscriberDto(email: '[email protected]', requestConfirmation: true, htmlEmail: true);
9279

9380
$result = $this->subscriberManager->createSubscriber($dto);
@@ -99,7 +86,7 @@ public function testCreateSubscriberPersistsAndSendsEmail(): void
9986
$this->assertFalse($result->isDisabled());
10087
}
10188

102-
public function testCreateSubscriberWithConfirmationSendsConfirmationEmail(): void
89+
public function testCreateSubscriberWithConfirmation(): void
10390
{
10491
$capturedSubscriber = null;
10592
$this->subscriberRepository
@@ -111,19 +98,6 @@ public function testCreateSubscriberWithConfirmationSendsConfirmationEmail(): vo
11198
return true;
11299
}));
113100

114-
$this->messageBus
115-
->expects($this->once())
116-
->method('dispatch')
117-
->with($this->callback(function (SubscriberConfirmationMessage $message) {
118-
$this->assertEquals('[email protected]', $message->getEmail());
119-
$this->assertEquals('test-unique-id-123', $message->getUniqueId());
120-
$this->assertTrue($message->hasHtmlEmail());
121-
return true;
122-
}))
123-
->willReturnCallback(function ($message) {
124-
return new Envelope($message);
125-
});
126-
127101
$dto = new CreateSubscriberDto(email: '[email protected]', requestConfirmation: true, htmlEmail: true);
128102
$this->subscriberManager->createSubscriber($dto);
129103

@@ -133,16 +107,12 @@ public function testCreateSubscriberWithConfirmationSendsConfirmationEmail(): vo
133107
$this->assertFalse($capturedSubscriber->isConfirmed());
134108
}
135109

136-
public function testCreateSubscriberWithoutConfirmationDoesNotSendConfirmationEmail(): void
110+
public function testCreateSubscriberWithoutConfirmation(): void
137111
{
138112
$this->subscriberRepository
139113
->expects($this->once())
140114
->method('save');
141115

142-
$this->messageBus
143-
->expects($this->never())
144-
->method('dispatch');
145-
146116
$dto = new CreateSubscriberDto(email: '[email protected]', requestConfirmation: false, htmlEmail: true);
147117
$this->subscriberManager->createSubscriber($dto);
148118
}

0 commit comments

Comments
 (0)