Skip to content

Commit f976d46

Browse files
tatevikg1tatevikg1
committed
Refactor
1 parent 742ae52 commit f976d46

File tree

9 files changed

+139
-52
lines changed

9 files changed

+139
-52
lines changed

‎config/services/providers.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,6 @@ services:
2727
autowire: true
2828
arguments:
2929
$cache: '@Psr\SimpleCache\CacheInterface'
30+
31+
PhpList\Core\Domain\Subscription\Service\Provider\SubscriberAttributeProvider:
32+
autowire: true

‎resources/translations/messages.en.xlf‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,10 @@ Thank you.</target>
708708
<source>Marked unconfirmed while sending campaign %message_id%</source>
709709
<target>__Marked unconfirmed while sending campaign %message_id%</target>
710710
</trans-unit>
711+
<trans-unit id="duYukTn" resname="Update by %admin%">
712+
<source>Update by %admin%</source>
713+
<target>__Update by %admin%</target>
714+
</trans-unit>
711715
</body>
712716
</file>
713717
</xliff>

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,25 @@
99
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1010
use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeDefinition;
1111
use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeValue;
12+
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository;
1213
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository;
1314
use Symfony\Contracts\Translation\TranslatorInterface;
1415

1516
class SubscriberAttributeManager
1617
{
1718
private SubscriberAttributeValueRepository $attributeRepository;
19+
private SubscriberAttributeDefinitionRepository $attrDefinitionRepository;
1820
private EntityManagerInterface $entityManager;
1921
private TranslatorInterface $translator;
2022

2123
public function __construct(
2224
SubscriberAttributeValueRepository $attributeRepository,
25+
SubscriberAttributeDefinitionRepository $attrDefinitionRepository,
2326
EntityManagerInterface $entityManager,
2427
TranslatorInterface $translator,
2528
) {
2629
$this->attributeRepository = $attributeRepository;
30+
$this->attrDefinitionRepository = $attrDefinitionRepository;
2731
$this->entityManager = $entityManager;
2832
$this->translator = $translator;
2933
}
@@ -60,4 +64,27 @@ public function delete(SubscriberAttributeValue $attribute): void
6064
{
6165
$this->attributeRepository->remove($attribute);
6266
}
67+
68+
public function processAttributes(Subscriber $subscriber, array $extraData): void
69+
{
70+
// $oldAttributesMap = $this->subscriberAttributeProvider->getMappedValues($subscriber);
71+
72+
foreach ($extraData as $key => $value) {
73+
$lowerKey = strtolower((string)$key);
74+
if (in_array($lowerKey, ['password', 'modified'], true)) {
75+
continue;
76+
}
77+
78+
$attributeDefinition = $this->attrDefinitionRepository->findOneByName($key);
79+
if ($attributeDefinition !== null) {
80+
$this->createOrUpdate(
81+
subscriber: $subscriber,
82+
definition: $attributeDefinition,
83+
value: $value
84+
);
85+
}
86+
}
87+
88+
// $newAttributesMap = $this->subscriberAttributeProvider->getMappedValues($subscriber);
89+
}
6390
}

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,32 @@ public function addHistoryFromImport(
5858
): void {
5959
$headerLine = sprintf("API-v2-import - %s: %s%s", $admin ? 'Admin' : 'CLI', $admin?->getId(), "\n\n");
6060

61+
$lines = $this->getHistoryLines($updatedData, $listLines);
62+
63+
$this->addHistory(
64+
subscriber: $subscriber,
65+
message: 'Import by ' . $admin?->getLoginName(),
66+
details: $headerLine . implode(PHP_EOL, $lines) . PHP_EOL
67+
);
68+
}
69+
70+
public function addHistoryFromApi(
71+
Subscriber $subscriber,
72+
array $listLines,
73+
array $updatedData,
74+
?Administrator $admin = null,
75+
): void {
76+
$lines = $this->getHistoryLines($updatedData, $listLines);
77+
78+
$this->addHistory(
79+
subscriber: $subscriber,
80+
message: $this->translator->trans('Update by %admin%', ['%admin%' => $admin->getLoginName()]),
81+
details: implode(PHP_EOL, $lines) . PHP_EOL
82+
);
83+
}
84+
85+
private function getHistoryLines(array $updatedData, array $listLines): array
86+
{
6187
$lines = [];
6288
if (empty($updatedData) && empty($listLines)) {
6389
$lines[] = $this->translator->trans('No user details changed');
@@ -81,10 +107,6 @@ public function addHistoryFromImport(
81107
}
82108
}
83109

84-
$this->addHistory(
85-
subscriber: $subscriber,
86-
message: 'Import by ' . $admin?->getLoginName(),
87-
details: $headerLine . implode(PHP_EOL, $lines) . PHP_EOL
88-
);
110+
return $lines;
89111
}
90112
}

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

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

77
use Doctrine\ORM\EntityManagerInterface;
8+
use PhpList\Core\Domain\Identity\Model\Administrator;
89
use PhpList\Core\Domain\Messaging\Message\SubscriberConfirmationMessage;
910
use PhpList\Core\Domain\Subscription\Model\Dto\CreateSubscriberDto;
1011
use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto;
1112
use PhpList\Core\Domain\Subscription\Model\Dto\UpdateSubscriberDto;
1213
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1314
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
14-
use PhpList\Core\Domain\Subscription\Service\SubscriberBlacklistService;
1515
use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService;
1616
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
1717
use Symfony\Component\Messenger\MessageBusInterface;
@@ -24,19 +24,22 @@ class SubscriberManager
2424
private MessageBusInterface $messageBus;
2525
private SubscriberDeletionService $subscriberDeletionService;
2626
private TranslatorInterface $translator;
27+
private SubscriberHistoryManager $subscriberHistoryManager;
2728

2829
public function __construct(
2930
SubscriberRepository $subscriberRepository,
3031
EntityManagerInterface $entityManager,
3132
MessageBusInterface $messageBus,
3233
SubscriberDeletionService $subscriberDeletionService,
33-
TranslatorInterface $translator
34+
TranslatorInterface $translator,
35+
SubscriberHistoryManager $subscriberHistoryManager,
3436
) {
3537
$this->subscriberRepository = $subscriberRepository;
3638
$this->entityManager = $entityManager;
3739
$this->messageBus = $messageBus;
3840
$this->subscriberDeletionService = $subscriberDeletionService;
3941
$this->translator = $translator;
42+
$this->subscriberHistoryManager = $subscriberHistoryManager;
4043
}
4144

4245
public function createSubscriber(CreateSubscriberDto $subscriberDto): Subscriber
@@ -74,7 +77,7 @@ public function getSubscriberById(int $subscriberId): ?Subscriber
7477
return $this->subscriberRepository->find($subscriberId);
7578
}
7679

77-
public function updateSubscriber(UpdateSubscriberDto $subscriberDto): Subscriber
80+
public function updateSubscriber(UpdateSubscriberDto $subscriberDto, Administrator $admin): Subscriber
7881
{
7982
/** @var Subscriber $subscriber */
8083
$subscriber = $this->subscriberRepository->find($subscriberDto->subscriberId);
@@ -86,8 +89,15 @@ public function updateSubscriber(UpdateSubscriberDto $subscriberDto): Subscriber
8689
$subscriber->setDisabled($subscriberDto->disabled);
8790
$subscriber->setExtraData($subscriberDto->additionalData);
8891

92+
$uow = $this->entityManager->getUnitOfWork();
93+
$meta = $this->entityManager->getClassMetadata(Subscriber::class);
94+
$uow->computeChangeSet($meta, $subscriber);
95+
$changeSet = $uow->getEntityChangeSet($subscriber);
96+
8997
$this->entityManager->flush();
9098

99+
$this->subscriberHistoryManager->addHistoryFromApi($subscriber, [], $changeSet, $admin);
100+
91101
return $subscriber;
92102
}
93103

‎src/Domain/Subscription/Service/Provider/SubscriberAttributeProvider.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpList\Core\Domain\Subscription\Service\Provider;
6+
7+
use PhpList\Core\Domain\Subscription\Model\Subscriber;
8+
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository;
9+
use PhpList\Core\Domain\Subscription\Service\Resolver\AttributeValueResolver;
10+
11+
class SubscriberAttributeProvider
12+
{
13+
public function __construct(
14+
private readonly AttributeValueResolver $resolver,
15+
private readonly SubscriberAttributeValueRepository $attributesRepository,
16+
) {
17+
}
18+
19+
public function getMappedValues(Subscriber $subscriber): array
20+
{
21+
$userAttributes = $this->attributesRepository->getForSubscriber($subscriber);
22+
foreach ($userAttributes as $userAttribute) {
23+
$data[$userAttribute->getAttributeDefinition()->getName()] = $this->resolver->resolve($userAttribute);
24+
}
25+
26+
return $data ?? [];
27+
}
28+
}

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

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto;
1212
use PhpList\Core\Domain\Subscription\Model\Dto\SubscriberImportOptions;
1313
use PhpList\Core\Domain\Subscription\Model\Subscriber;
14-
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository;
1514
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
1615
use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberAttributeManager;
1716
use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager;
@@ -21,7 +20,7 @@
2120
use Symfony\Component\Messenger\MessageBusInterface;
2221
use Symfony\Contracts\Translation\TranslatorInterface;
2322
use Throwable;
24-
23+
// todo: check if dryRun will work (some function flush)
2524
/**
2625
* Service for importing subscribers from a CSV file.
2726
* @SuppressWarnings("CouplingBetweenObjects")
@@ -34,7 +33,6 @@ class SubscriberCsvImporter
3433
private SubscriptionManager $subscriptionManager;
3534
private SubscriberRepository $subscriberRepository;
3635
private CsvImporter $csvImporter;
37-
private SubscriberAttributeDefinitionRepository $attrDefinitionRepository;
3836
private EntityManagerInterface $entityManager;
3937
private TranslatorInterface $translator;
4038
private MessageBusInterface $messageBus;
@@ -46,7 +44,6 @@ public function __construct(
4644
SubscriptionManager $subscriptionManager,
4745
SubscriberRepository $subscriberRepository,
4846
CsvImporter $csvImporter,
49-
SubscriberAttributeDefinitionRepository $attrDefinitionRepository,
5047
EntityManagerInterface $entityManager,
5148
TranslatorInterface $translator,
5249
MessageBusInterface $messageBus,
@@ -57,7 +54,6 @@ public function __construct(
5754
$this->subscriptionManager = $subscriptionManager;
5855
$this->subscriberRepository = $subscriberRepository;
5956
$this->csvImporter = $csvImporter;
60-
$this->attrDefinitionRepository = $attrDefinitionRepository;
6157
$this->entityManager = $entityManager;
6258
$this->translator = $translator;
6359
$this->messageBus = $messageBus;
@@ -182,7 +178,7 @@ private function processRow(
182178
$stats['created']++;
183179
}
184180

185-
$this->processAttributes($subscriber, $dto);
181+
$this->attributeManager->processAttributes($subscriber, $dto->extraAttributes);
186182

187183
$addedNewSubscriberToList = false;
188184
$listLines = [];
@@ -261,30 +257,4 @@ private function handleFlushAndEmail(
261257
}
262258
}
263259
}
264-
265-
/**
266-
* Process subscriber attributes.
267-
*
268-
* @param Subscriber $subscriber The subscriber
269-
* @param ImportSubscriberDto $dto
270-
*/
271-
private function processAttributes(Subscriber $subscriber, ImportSubscriberDto $dto): void
272-
{
273-
foreach ($dto->extraAttributes as $key => $value) {
274-
$lowerKey = strtolower((string)$key);
275-
// Do not import or update sensitive/system fields from CSV
276-
if (in_array($lowerKey, ['password', 'modified'], true)) {
277-
continue;
278-
}
279-
280-
$attributeDefinition = $this->attrDefinitionRepository->findOneByName($key);
281-
if ($attributeDefinition !== null) {
282-
$this->attributeManager->createOrUpdate(
283-
subscriber: $subscriber,
284-
definition: $attributeDefinition,
285-
value: $value
286-
);
287-
}
288-
}
289-
}
290260
}

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

Lines changed: 31 additions & 5 deletions
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\Model\SubscriberAttributeDefinition;
1111
use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeValue;
12+
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository;
1213
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository;
1314
use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberAttributeManager;
1415
use PHPUnit\Framework\TestCase;
@@ -35,7 +36,12 @@ public function testCreateNewSubscriberAttribute(): void
3536
return $attr->getValue() === 'US';
3637
}));
3738

38-
$manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en'));
39+
$manager = new SubscriberAttributeManager(
40+
attributeRepository: $subscriberAttrRepo,
41+
attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class),
42+
entityManager: $entityManager,
43+
translator: new Translator('en')
44+
);
3945
$attribute = $manager->createOrUpdate($subscriber, $definition, 'US');
4046

4147
self::assertInstanceOf(SubscriberAttributeValue::class, $attribute);
@@ -61,7 +67,12 @@ public function testUpdateExistingSubscriberAttribute(): void
6167
->method('persist')
6268
->with($existing);
6369

64-
$manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en'));
70+
$manager = new SubscriberAttributeManager(
71+
attributeRepository: $subscriberAttrRepo,
72+
attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class),
73+
entityManager: $entityManager,
74+
translator: new Translator('en')
75+
);
6576
$result = $manager->createOrUpdate($subscriber, $definition, 'Updated');
6677

6778
self::assertSame('Updated', $result->getValue());
@@ -77,7 +88,12 @@ public function testCreateFailsWhenValueAndDefaultAreNull(): void
7788

7889
$subscriberAttrRepo->method('findOneBySubscriberAndAttribute')->willReturn(null);
7990

80-
$manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en'));
91+
$manager = new SubscriberAttributeManager(
92+
attributeRepository: $subscriberAttrRepo,
93+
attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class),
94+
entityManager: $entityManager,
95+
translator: new Translator('en')
96+
);
8197

8298
$this->expectException(SubscriberAttributeCreationException::class);
8399
$this->expectExceptionMessage('Value is required');
@@ -96,7 +112,12 @@ public function testGetSubscriberAttribute(): void
96112
->with(5, 10)
97113
->willReturn($expected);
98114

99-
$manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en'));
115+
$manager = new SubscriberAttributeManager(
116+
attributeRepository: $subscriberAttrRepo,
117+
attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class),
118+
entityManager: $entityManager,
119+
translator: new Translator('en')
120+
);
100121
$result = $manager->getSubscriberAttribute(5, 10);
101122

102123
self::assertSame($expected, $result);
@@ -112,7 +133,12 @@ public function testDeleteSubscriberAttribute(): void
112133
->method('remove')
113134
->with($attribute);
114135

115-
$manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en'));
136+
$manager = new SubscriberAttributeManager(
137+
attributeRepository: $subscriberAttrRepo,
138+
attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class),
139+
entityManager: $entityManager,
140+
translator: new Translator('en')
141+
);
116142
$manager->delete($attribute);
117143

118144
self::assertTrue(true);

0 commit comments

Comments
 (0)