Skip to content

Commit a9515fe

Browse files
tatevikg1tatevikg1
committed
ISSUE-124: style + test fix
1 parent c904eb7 commit a9515fe

File tree

4 files changed

+84
-130
lines changed

4 files changed

+84
-130
lines changed

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

Lines changed: 43 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,80 +17,61 @@
1717

1818
class SubscriberDeletionService
1919
{
20-
private LinkTrackUmlClickRepository $linkTrackUmlClickRepository;
20+
private LinkTrackUmlClickRepository $linkTrackUmlClickRepo;
2121
private EntityManagerInterface $entityManager;
22-
private UserMessageRepository $userMessageRepository;
23-
private SubscriberAttributeValueRepository $subscriberAttributeValueRepository;
24-
private SubscriberHistoryRepository $subscriberHistoryRepository;
25-
private UserMessageBounceRepository $userMessageBounceRepository;
26-
private UserMessageForwardRepository $userMessageForwardRepository;
27-
private UserMessageViewRepository $userMessageViewRepository;
28-
private SubscriptionRepository $subscriptionRepository;
22+
private UserMessageRepository $userMessageRepo;
23+
private SubscriberAttributeValueRepository $subscriberAttrValueRepo;
24+
private SubscriberHistoryRepository $subscriberHistoryRepo;
25+
private UserMessageBounceRepository $userMessageBounceRepo;
26+
private UserMessageForwardRepository $userMessageForwardRepo;
27+
private UserMessageViewRepository $userMessageViewRepo;
28+
private SubscriptionRepository $subscriptionRepo;
2929

3030
public function __construct(
31-
LinkTrackUmlClickRepository $linkTrackUmlClickRepository,
31+
LinkTrackUmlClickRepository $linkTrackUmlClickRepo,
3232
EntityManagerInterface $entityManager,
33-
UserMessageRepository $userMessageRepository,
34-
SubscriberAttributeValueRepository $subscriberAttributeValueRepository,
35-
SubscriberHistoryRepository $subscriberHistoryRepository,
36-
UserMessageBounceRepository $userMessageBounceRepository,
37-
UserMessageForwardRepository $userMessageForwardRepository,
38-
UserMessageViewRepository $userMessageViewRepository,
39-
SubscriptionRepository $subscriptionRepository
33+
UserMessageRepository $userMessageRepo,
34+
SubscriberAttributeValueRepository $subscriberAttrValueRepo,
35+
SubscriberHistoryRepository $subscriberHistoryRepo,
36+
UserMessageBounceRepository $userMessageBounceRepo,
37+
UserMessageForwardRepository $userMessageForwardRepo,
38+
UserMessageViewRepository $userMessageViewRepo,
39+
SubscriptionRepository $subscriptionRepo,
4040
) {
41-
$this->linkTrackUmlClickRepository = $linkTrackUmlClickRepository;
41+
$this->linkTrackUmlClickRepo = $linkTrackUmlClickRepo;
4242
$this->entityManager = $entityManager;
43-
$this->userMessageRepository = $userMessageRepository;
44-
$this->subscriberAttributeValueRepository = $subscriberAttributeValueRepository;
45-
$this->subscriberHistoryRepository = $subscriberHistoryRepository;
46-
$this->userMessageBounceRepository = $userMessageBounceRepository;
47-
$this->userMessageForwardRepository = $userMessageForwardRepository;
48-
$this->userMessageViewRepository = $userMessageViewRepository;
49-
$this->subscriptionRepository = $subscriptionRepository;
43+
$this->userMessageRepo = $userMessageRepo;
44+
$this->subscriberAttrValueRepo = $subscriberAttrValueRepo;
45+
$this->subscriberHistoryRepo = $subscriberHistoryRepo;
46+
$this->userMessageBounceRepo = $userMessageBounceRepo;
47+
$this->userMessageForwardRepo = $userMessageForwardRepo;
48+
$this->userMessageViewRepo = $userMessageViewRepo;
49+
$this->subscriptionRepo = $subscriptionRepo;
5050
}
5151

5252
public function deleteLeavingBlacklist(Subscriber $subscriber): void
5353
{
54-
$linkTrackUmlClick = $this->linkTrackUmlClickRepository->findBy(['userId' => $subscriber->getId()]);
55-
foreach ($linkTrackUmlClick as $click) {
56-
$this->entityManager->remove($click);
57-
}
58-
59-
$subscriptions = $this->subscriptionRepository->findBy(['subscriber' => $subscriber]);
60-
foreach ($subscriptions as $subscription) {
61-
$this->entityManager->remove($subscription);
62-
}
63-
64-
$userMessages = $this->userMessageRepository->findBy(['user' => $subscriber]);
65-
foreach ($userMessages as $message) {
66-
$this->entityManager->remove($message);
67-
}
68-
69-
$subscriberAttributes = $this->subscriberAttributeValueRepository->findBy(['subscriber' => $subscriber]);
70-
foreach ($subscriberAttributes as $attribute) {
71-
$this->entityManager->remove($attribute);
72-
}
73-
74-
$subscriberHistory = $this->subscriberHistoryRepository->findBy(['subscriber' => $subscriber]);
75-
foreach ($subscriberHistory as $history) {
76-
$this->entityManager->remove($history);
77-
}
54+
$this->removeEntities($this->linkTrackUmlClickRepo->findBy(['userId' => $subscriber->getId()]));
55+
$this->removeEntities($this->subscriptionRepo->findBy(['subscriber' => $subscriber]));
56+
$this->removeEntities($this->userMessageRepo->findBy(['user' => $subscriber]));
57+
$this->removeEntities($this->subscriberAttrValueRepo->findBy(['subscriber' => $subscriber]));
58+
$this->removeEntities($this->subscriberHistoryRepo->findBy(['subscriber' => $subscriber]));
59+
$this->removeEntities($this->userMessageBounceRepo->findBy(['userId' => $subscriber->getId()]));
60+
$this->removeEntities($this->userMessageForwardRepo->findBy(['userId' => $subscriber->getId()]));
61+
$this->removeEntities($this->userMessageViewRepo->findBy(['userId' => $subscriber->getId()]));
7862

79-
$userMessageBounces = $this->userMessageBounceRepository->findBy(['userId' => $subscriber->getId()]);
80-
foreach ($userMessageBounces as $bounce) {
81-
$this->entityManager->remove($bounce);
82-
}
83-
84-
$userMessageForwards = $this->userMessageForwardRepository->findBy(['userId' => $subscriber->getId()]);
85-
foreach ($userMessageForwards as $forward) {
86-
$this->entityManager->remove($forward);
87-
}
63+
$this->entityManager->remove($subscriber);
64+
}
8865

89-
$userMessageViews = $this->userMessageViewRepository->findBy(['userId' => $subscriber->getId()]);
90-
foreach ($userMessageViews as $view) {
91-
$this->entityManager->remove($view);
66+
/**
67+
* Remove a collection of entities
68+
*
69+
* @param array $entities
70+
*/
71+
private function removeEntities(array $entities): void
72+
{
73+
foreach ($entities as $entity) {
74+
$this->entityManager->remove($entity);
9275
}
93-
94-
$this->entityManager->remove($subscriber);
9576
}
9677
}

‎tests/Integration/Domain/Subscription/Service/SubscriberDeletionServiceTest.php

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@ public function testDeleteSubscriberWithRelatedDataDoesNotThrowDoctrineError():
5555
$this->entityManager->persist($admin);
5656

5757
$msg = new Message(
58-
new MessageFormat(true, MessageFormat::FORMAT_TEXT),
59-
new MessageSchedule(1, null, 3, null, null),
60-
new MessageMetadata('done'),
61-
new MessageContent('Owned by Admin 1!'),
62-
new MessageOptions(),
63-
$admin
58+
format: new MessageFormat(true, MessageFormat::FORMAT_TEXT),
59+
schedule: new MessageSchedule(1, null, 3, null, null),
60+
metadata: new MessageMetadata('done'),
61+
content: new MessageContent('Owned by Admin 1!'),
62+
options: new MessageOptions(),
63+
owner: $admin
6464
);
6565
$this->entityManager->persist($msg);
6666

@@ -119,27 +119,31 @@ public function testDeleteSubscriberWithRelatedDataDoesNotThrowDoctrineError():
119119
$this->fail('Exception was thrown: ' . $e->getMessage());
120120
}
121121

122-
// Verify the subscriber was deleted
123122
$deletedSubscriber = $this->entityManager->getRepository(Subscriber::class)->find($subscriberId);
124123
$this->assertNull($deletedSubscriber, 'Subscriber should be deleted');
125124

126-
// Verify related entities were deleted
127-
$subscriptions = $this->entityManager->getRepository(Subscription::class)->findBy(['subscriber' => $subscriber]);
125+
$subscriptionRepo = $this->entityManager->getRepository(Subscription::class);
126+
$subscriptions = $subscriptionRepo->findBy(['subscriber' => $subscriber]);
128127
$this->assertEmpty($subscriptions, 'Subscriptions should be deleted');
129128

130-
$linkTrackUmlClicks = $this->entityManager->getRepository(LinkTrackUmlClick::class)->findBy(['userId' => $subscriberId]);
129+
$linkTrackRepo = $this->entityManager->getRepository(LinkTrackUmlClick::class);
130+
$linkTrackUmlClicks = $linkTrackRepo->findBy(['userId' => $subscriberId]);
131131
$this->assertEmpty($linkTrackUmlClicks, 'LinkTrackUmlClicks should be deleted');
132132

133-
$userMessages = $this->entityManager->getRepository(UserMessage::class)->findBy(['user' => $subscriber]);
133+
$userMessageRepo = $this->entityManager->getRepository(UserMessage::class);
134+
$userMessages = $userMessageRepo->findBy(['user' => $subscriber]);
134135
$this->assertEmpty($userMessages, 'UserMessages should be deleted');
135136

136-
$userMessageBounces = $this->entityManager->getRepository(UserMessageBounce::class)->findBy(['userId' => $subscriberId]);
137+
$bounceRepo = $this->entityManager->getRepository(UserMessageBounce::class);
138+
$userMessageBounces = $bounceRepo->findBy(['userId' => $subscriberId]);
137139
$this->assertEmpty($userMessageBounces, 'UserMessageBounces should be deleted');
138140

139-
$userMessageForwards = $this->entityManager->getRepository(UserMessageForward::class)->findBy(['userId' => $subscriberId]);
141+
$forwardRepo = $this->entityManager->getRepository(UserMessageForward::class);
142+
$userMessageForwards = $forwardRepo->findBy(['userId' => $subscriberId]);
140143
$this->assertEmpty($userMessageForwards, 'UserMessageForwards should be deleted');
141144

142-
$userMessageViews = $this->entityManager->getRepository(UserMessageView::class)->findBy(['userId' => $subscriberId]);
145+
$viewRepo = $this->entityManager->getRepository(UserMessageView::class);
146+
$userMessageViews = $viewRepo->findBy(['userId' => $subscriberId]);
143147
$this->assertEmpty($userMessageViews, 'UserMessageViews should be deleted');
144148
}
145149
}

‎tests/Unit/Domain/Subscription/Service/SubscriberDeletionServiceTest.php

Lines changed: 20 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace PhpList\Core\Tests\Unit\Domain\Subscription\Service;
66

7-
use Doctrine\Common\Collections\ArrayCollection;
87
use Doctrine\ORM\EntityManagerInterface;
98
use PhpList\Core\Domain\Analytics\Repository\LinkTrackUmlClickRepository;
109
use PhpList\Core\Domain\Analytics\Repository\UserMessageViewRepository;
@@ -23,6 +22,7 @@
2322
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository;
2423
use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository;
2524
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
25+
use PhpList\Core\Domain\Subscription\Repository\SubscriptionRepository;
2626
use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService;
2727
use PHPUnit\Framework\MockObject\MockObject;
2828
use PHPUnit\Framework\TestCase;
@@ -32,36 +32,36 @@ class SubscriberDeletionServiceTest extends TestCase
3232
private LinkTrackUmlClickRepository&MockObject $linkTrackUmlClickRepository;
3333
private EntityManagerInterface&MockObject $entityManager;
3434
private UserMessageRepository&MockObject $userMessageRepository;
35-
private SubscriberRepository&MockObject $subscriberRepository;
3635
private SubscriberAttributeValueRepository&MockObject $subscriberAttributeValueRepository;
3736
private SubscriberHistoryRepository&MockObject $subscriberHistoryRepository;
3837
private UserMessageBounceRepository&MockObject $userMessageBounceRepository;
3938
private UserMessageForwardRepository&MockObject $userMessageForwardRepository;
4039
private UserMessageViewRepository&MockObject $userMessageViewRepository;
40+
private SubscriptionRepository&MockObject $subscriptionRepository;
4141
private SubscriberDeletionService $service;
4242

4343
protected function setUp(): void
4444
{
4545
$this->linkTrackUmlClickRepository = $this->createMock(LinkTrackUmlClickRepository::class);
4646
$this->entityManager = $this->createMock(EntityManagerInterface::class);
4747
$this->userMessageRepository = $this->createMock(UserMessageRepository::class);
48-
$this->subscriberRepository = $this->createMock(SubscriberRepository::class);
4948
$this->subscriberAttributeValueRepository = $this->createMock(SubscriberAttributeValueRepository::class);
5049
$this->subscriberHistoryRepository = $this->createMock(SubscriberHistoryRepository::class);
5150
$this->userMessageBounceRepository = $this->createMock(UserMessageBounceRepository::class);
5251
$this->userMessageForwardRepository = $this->createMock(UserMessageForwardRepository::class);
5352
$this->userMessageViewRepository = $this->createMock(UserMessageViewRepository::class);
53+
$this->subscriptionRepository = $this->createMock(SubscriptionRepository::class);
5454

5555
$this->service = new SubscriberDeletionService(
5656
$this->linkTrackUmlClickRepository,
5757
$this->entityManager,
5858
$this->userMessageRepository,
59-
$this->subscriberRepository,
6059
$this->subscriberAttributeValueRepository,
6160
$this->subscriberHistoryRepository,
6261
$this->userMessageBounceRepository,
6362
$this->userMessageForwardRepository,
64-
$this->userMessageViewRepository
63+
$this->userMessageViewRepository,
64+
$this->subscriptionRepository
6565
);
6666
}
6767

@@ -72,88 +72,56 @@ public function testDeleteLeavingBlacklistRemovesAllRelatedData(): void
7272
$subscriber->method('getId')->willReturn($subscriberId);
7373

7474
$subscription = $this->createMock(Subscription::class);
75-
$subscriptions = new ArrayCollection([$subscription]);
76-
$subscriber->method('getSubscriptions')->willReturn($subscriptions);
75+
$this->subscriptionRepository
76+
->method('findBy')
77+
->with(['subscriber' => $subscriber])
78+
->willReturn([$subscription]);
7779

7880
$linkTrackUmlClick = $this->createMock(LinkTrackUmlClick::class);
7981
$this->linkTrackUmlClickRepository
8082
->method('findBy')
81-
->with(['userid' => $subscriberId])
83+
->with(['userId' => $subscriberId])
8284
->willReturn([$linkTrackUmlClick]);
83-
$this->linkTrackUmlClickRepository
84-
->expects($this->once())
85-
->method('remove')
86-
->with($linkTrackUmlClick);
8785

8886
$this->entityManager
89-
->expects($this->once())
90-
->method('remove')
91-
->with($subscription);
87+
->expects($this->atLeast(1))
88+
->method('remove');
9289

9390
$userMessage = $this->createMock(UserMessage::class);
9491
$this->userMessageRepository
9592
->method('findBy')
9693
->with(['user' => $subscriber])
9794
->willReturn([$userMessage]);
98-
$this->userMessageRepository
99-
->expects($this->once())
100-
->method('remove')
101-
->with($userMessage);
10295

10396
$subscriberAttribute = $this->createMock(SubscriberAttributeValue::class);
10497
$this->subscriberAttributeValueRepository
10598
->method('findBy')
10699
->with(['subscriber' => $subscriber])
107100
->willReturn([$subscriberAttribute]);
108-
$this->subscriberAttributeValueRepository
109-
->expects($this->once())
110-
->method('remove')
111-
->with($subscriberAttribute);
112101

113102
$subscriberHistory = $this->createMock(SubscriberHistory::class);
114103
$this->subscriberHistoryRepository
115104
->method('findBy')
116105
->with(['subscriber' => $subscriber])
117106
->willReturn([$subscriberHistory]);
118-
$this->subscriberHistoryRepository
119-
->expects($this->once())
120-
->method('remove')
121-
->with($subscriberHistory);
122107

123108
$userMessageBounce = $this->createMock(UserMessageBounce::class);
124109
$this->userMessageBounceRepository
125110
->method('findBy')
126111
->with(['userId' => $subscriberId])
127112
->willReturn([$userMessageBounce]);
128-
$this->userMessageBounceRepository
129-
->expects($this->once())
130-
->method('remove')
131-
->with($userMessageBounce);
132113

133114
$userMessageForward = $this->createMock(UserMessageForward::class);
134115
$this->userMessageForwardRepository
135116
->method('findBy')
136117
->with(['userId' => $subscriberId])
137118
->willReturn([$userMessageForward]);
138-
$this->userMessageForwardRepository
139-
->expects($this->once())
140-
->method('remove')
141-
->with($userMessageForward);
142119

143120
$userMessageView = $this->createMock(UserMessageView::class);
144121
$this->userMessageViewRepository
145122
->method('findBy')
146-
->with(['userid' => $subscriberId])
123+
->with(['userId' => $subscriberId])
147124
->willReturn([$userMessageView]);
148-
$this->userMessageViewRepository
149-
->expects($this->once())
150-
->method('remove')
151-
->with($userMessageView);
152-
153-
$this->subscriberRepository
154-
->expects($this->once())
155-
->method('remove')
156-
->with($subscriber);
157125

158126
$this->service->deleteLeavingBlacklist($subscriber);
159127
}
@@ -164,16 +132,15 @@ public function testDeleteLeavingBlacklistHandlesEmptyRelatedData(): void
164132
$subscriberId = 123;
165133
$subscriber->method('getId')->willReturn($subscriberId);
166134

167-
$emptySubscriptions = new ArrayCollection();
168-
$subscriber->method('getSubscriptions')->willReturn($emptySubscriptions);
135+
$this->subscriptionRepository
136+
->method('findBy')
137+
->with(['subscriber' => $subscriber])
138+
->willReturn([]);
169139

170140
$this->linkTrackUmlClickRepository
171141
->method('findBy')
172-
->with(['userid' => $subscriberId])
142+
->with(['userId' => $subscriberId])
173143
->willReturn([]);
174-
$this->linkTrackUmlClickRepository
175-
->expects($this->never())
176-
->method('remove');
177144

178145
$this->userMessageRepository
179146
->method('findBy')
@@ -217,13 +184,13 @@ public function testDeleteLeavingBlacklistHandlesEmptyRelatedData(): void
217184

218185
$this->userMessageViewRepository
219186
->method('findBy')
220-
->with(['userid' => $subscriberId])
187+
->with(['userId' => $subscriberId])
221188
->willReturn([]);
222189
$this->userMessageViewRepository
223190
->expects($this->never())
224191
->method('remove');
225192

226-
$this->subscriberRepository
193+
$this->entityManager
227194
->expects($this->once())
228195
->method('remove')
229196
->with($subscriber);

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1010
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
1111
use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberManager;
12+
use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService;
1213
use PHPUnit\Framework\TestCase;
1314

1415
class SubscriberManagerTest extends TestCase
@@ -17,6 +18,7 @@ public function testCreateSubscriberPersistsAndReturnsProperlyInitializedEntity(
1718
{
1819
$repoMock = $this->createMock(SubscriberRepository::class);
1920
$emMock = $this->createMock(EntityManagerInterface::class);
21+
$deletionServiceMock = $this->createMock(SubscriberDeletionService::class);
2022
$repoMock
2123
->expects($this->once())
2224
->method('save')
@@ -28,7 +30,7 @@ public function testCreateSubscriberPersistsAndReturnsProperlyInitializedEntity(
2830
&& $sub->isDisabled() === false;
2931
}));
3032

31-
$manager = new SubscriberManager($repoMock, $emMock);
33+
$manager = new SubscriberManager($repoMock, $emMock, $deletionServiceMock);
3234

3335
$dto = new CreateSubscriberDto(email: '[email protected]', requestConfirmation: true, htmlEmail: true);
3436

0 commit comments

Comments
 (0)