Skip to content

Commit 9adb0ec

Browse files
TatevikGrtatevikg1
andauthored
Refactor import add subscriber history (#363)
* Add blacklisted stat to import result * Add history * Add translations * addHistory for unconfirmed * Refactor * Add changeSetDto * Do not send email on creating without any list * Flush in controller * Add test --------- Co-authored-by: Tatevik <[email protected]>
1 parent 175dacb commit 9adb0ec

File tree

17 files changed

+709
-156
lines changed

17 files changed

+709
-156
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\SubscriberAttributeChangeSetProvider:
32+
autowire: true

resources/translations/messages.en.xlf

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,48 @@ Thank you.</target>
684684
<source>Footer of public pages</source>
685685
<target>__Footer of public pages</target>
686686
</trans-unit>
687+
<trans-unit id="vmzIZ25" resname="Please confirm your subscription">
688+
<source>Please confirm your subscription</source>
689+
<target>__Please confirm your subscription</target>
690+
</trans-unit>
691+
<trans-unit id="rT_b2Tm" resname="No user details changed">
692+
<source>No user details changed</source>
693+
<target>__No user details changed</target>
694+
</trans-unit>
695+
<trans-unit id="qdTR5bi" resname="%field% = %new% *changed* from %old%">
696+
<source>%field% = %new% *changed* from %old%</source>
697+
<target>__%field% = %new% *changed* from %old%</target>
698+
</trans-unit>
699+
<trans-unit id="rns1KOG" resname="Subscribed to %list%">
700+
<source>Subscribed to %list%</source>
701+
<target>__Subscribed to %list%</target>
702+
</trans-unit>
703+
<trans-unit id="eXllcQa" resname="Subscriber marked unconfirmed for invalid email address">
704+
<source>Subscriber marked unconfirmed for invalid email address</source>
705+
<target>__Subscriber marked unconfirmed for invalid email address</target>
706+
</trans-unit>
707+
<trans-unit id="oY0Gck3" resname="Marked unconfirmed while sending campaign %message_id%">
708+
<source>Marked unconfirmed while sending campaign %message_id%</source>
709+
<target>__Marked unconfirmed while sending campaign %message_id%</target>
710+
</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>
715+
<trans-unit id="7oMcCd6" resname="(no data)">
716+
<source>(no data)</source>
717+
<target>__(no data)</target>
718+
</trans-unit>
719+
<trans-unit id="rSZSC.W" resname="%attribute% = %new_value% &#10; changed from %old_value%">
720+
<source>%attribute% = %new_value%
721+
changed from %old_value%</source>
722+
<target>__%attribute% = %new_value%
723+
changed from %old_value%</target>
724+
</trans-unit>
725+
<trans-unit id="jkLAivD" resname="No data changed">
726+
<source>No data changed</source>
727+
<target>__No data changed</target>
728+
</trans-unit>
687729
</body>
688730
</file>
689731
</xliff>

src/Domain/Messaging/Service/Processor/CampaignProcessor.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PhpList\Core\Domain\Messaging\Service\RateLimitedCampaignMailer;
1515
use PhpList\Core\Domain\Messaging\Service\MaxProcessTimeLimiter;
1616
use PhpList\Core\Domain\Messaging\Service\MessageProcessingPreparator;
17+
use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager;
1718
use PhpList\Core\Domain\Subscription\Service\Provider\SubscriberProvider;
1819
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1920
use Psr\Log\LoggerInterface;
@@ -23,6 +24,8 @@
2324

2425
/**
2526
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
27+
* @SuppressWarnings(PHPMD.StaticAccess)
28+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
2629
*/
2730
class CampaignProcessor
2831
{
@@ -35,6 +38,7 @@ class CampaignProcessor
3538
private MaxProcessTimeLimiter $timeLimiter;
3639
private RequeueHandler $requeueHandler;
3740
private TranslatorInterface $translator;
41+
private SubscriberHistoryManager $subscriberHistoryManager;
3842

3943
public function __construct(
4044
RateLimitedCampaignMailer $mailer,
@@ -46,6 +50,7 @@ public function __construct(
4650
MaxProcessTimeLimiter $timeLimiter,
4751
RequeueHandler $requeueHandler,
4852
TranslatorInterface $translator,
53+
SubscriberHistoryManager $subscriberHistoryManager,
4954
) {
5055
$this->mailer = $mailer;
5156
$this->entityManager = $entityManager;
@@ -56,6 +61,7 @@ public function __construct(
5661
$this->timeLimiter = $timeLimiter;
5762
$this->requeueHandler = $requeueHandler;
5863
$this->translator = $translator;
64+
$this->subscriberHistoryManager = $subscriberHistoryManager;
5965
}
6066

6167
public function process(Message $campaign, ?OutputInterface $output = null): void
@@ -89,6 +95,14 @@ public function process(Message $campaign, ?OutputInterface $output = null): voi
8995
$output?->writeln($this->translator->trans('Invalid email, marking unconfirmed: %email%', [
9096
'%email%' => $subscriber->getEmail(),
9197
]));
98+
$this->subscriberHistoryManager->addHistory(
99+
subscriber: $subscriber,
100+
message: $this->translator->trans('Subscriber marked unconfirmed for invalid email address'),
101+
details: $this->translator->trans(
102+
'Marked unconfirmed while sending campaign %message_id%',
103+
['%message_id%' => $campaign->getId()]
104+
)
105+
);
92106
continue;
93107
}
94108

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpList\Core\Domain\Subscription\Model\Dto;
6+
7+
class ChangeSetDto
8+
{
9+
public const IGNORED_ATTRIBUTES = ['password', 'modified'];
10+
11+
/**
12+
* @var array<string, array{0: mixed, 1: mixed}>
13+
*
14+
* Example:
15+
* [
16+
* 'email' => [null, '[email protected]'],
17+
* 'isActive' => [true, false]
18+
* ]
19+
*/
20+
private array $changes = [];
21+
22+
/**
23+
* @param array<string, array{0: mixed, 1: mixed}> $changes
24+
*/
25+
public function __construct(array $changes = [])
26+
{
27+
$this->changes = $changes;
28+
}
29+
30+
/**
31+
* @return array<string, array{0: mixed, 1: mixed}>
32+
*/
33+
public function getChanges(): array
34+
{
35+
return $this->changes;
36+
}
37+
38+
public function hasChanges(): bool
39+
{
40+
return !empty($this->changes);
41+
}
42+
43+
public function hasField(string $field): bool
44+
{
45+
return array_key_exists($field, $this->changes);
46+
}
47+
48+
/**
49+
* @return array{0: mixed, 1: mixed}|null
50+
*/
51+
public function getFieldChange(string $field): ?array
52+
{
53+
return $this->changes[$field] ?? null;
54+
}
55+
56+
/**
57+
* @return mixed|null
58+
*/
59+
public function getOldValue(string $field): mixed
60+
{
61+
return $this->changes[$field][0] ?? null;
62+
}
63+
64+
/**
65+
* @return mixed|null
66+
*/
67+
public function getNewValue(string $field): mixed
68+
{
69+
return $this->changes[$field][1] ?? null;
70+
}
71+
72+
public function toArray(): array
73+
{
74+
return $this->changes;
75+
}
76+
77+
public static function fromDoctrineChangeSet(array $changeSet): self
78+
{
79+
return new self($changeSet);
80+
}
81+
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,29 @@
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;
13+
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository;
1214
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository;
1315
use Symfony\Contracts\Translation\TranslatorInterface;
1416

1517
class SubscriberAttributeManager
1618
{
1719
private SubscriberAttributeValueRepository $attributeRepository;
20+
private SubscriberAttributeDefinitionRepository $attrDefinitionRepository;
1821
private EntityManagerInterface $entityManager;
1922
private TranslatorInterface $translator;
2023

2124
public function __construct(
2225
SubscriberAttributeValueRepository $attributeRepository,
26+
SubscriberAttributeDefinitionRepository $attrDefinitionRepository,
2327
EntityManagerInterface $entityManager,
2428
TranslatorInterface $translator,
2529
) {
2630
$this->attributeRepository = $attributeRepository;
31+
$this->attrDefinitionRepository = $attrDefinitionRepository;
2732
$this->entityManager = $entityManager;
2833
$this->translator = $translator;
2934
}
@@ -33,6 +38,8 @@ public function createOrUpdate(
3338
SubscriberAttributeDefinition $definition,
3439
?string $value = null
3540
): SubscriberAttributeValue {
41+
// phpcs:ignore Generic.Commenting.Todo
42+
// todo: clarify which attributes can be created/updated
3643
$subscriberAttribute = $this->attributeRepository
3744
->findOneBySubscriberAndAttribute($subscriber, $definition);
3845

@@ -60,4 +67,23 @@ public function delete(SubscriberAttributeValue $attribute): void
6067
{
6168
$this->attributeRepository->remove($attribute);
6269
}
70+
71+
public function processAttributes(Subscriber $subscriber, array $attributeData): void
72+
{
73+
foreach ($attributeData as $key => $value) {
74+
$lowerKey = strtolower((string)$key);
75+
if (in_array($lowerKey, ChangeSetDto::IGNORED_ATTRIBUTES, true)) {
76+
continue;
77+
}
78+
79+
$attributeDefinition = $this->attrDefinitionRepository->findOneByName($key);
80+
if ($attributeDefinition !== null) {
81+
$this->createOrUpdate(
82+
subscriber: $subscriber,
83+
definition: $attributeDefinition,
84+
value: $value
85+
);
86+
}
87+
}
88+
}
6389
}

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

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,37 @@
44

55
namespace PhpList\Core\Domain\Subscription\Service\Manager;
66

7+
use Doctrine\ORM\EntityManagerInterface;
78
use PhpList\Core\Domain\Common\ClientIpResolver;
89
use PhpList\Core\Domain\Common\SystemInfoCollector;
10+
use PhpList\Core\Domain\Identity\Model\Administrator;
11+
use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto;
912
use PhpList\Core\Domain\Subscription\Model\Filter\SubscriberHistoryFilter;
1013
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1114
use PhpList\Core\Domain\Subscription\Model\SubscriberHistory;
1215
use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository;
16+
use Symfony\Contracts\Translation\TranslatorInterface;
1317

1418
class SubscriberHistoryManager
1519
{
1620
private SubscriberHistoryRepository $repository;
1721
private ClientIpResolver $clientIpResolver;
1822
private SystemInfoCollector $systemInfoCollector;
23+
private TranslatorInterface $translator;
24+
private EntityManagerInterface $entityManager;
1925

2026
public function __construct(
2127
SubscriberHistoryRepository $repository,
2228
ClientIpResolver $clientIpResolver,
2329
SystemInfoCollector $systemInfoCollector,
30+
TranslatorInterface $translator,
31+
EntityManagerInterface $entityManager,
2432
) {
2533
$this->repository = $repository;
2634
$this->clientIpResolver = $clientIpResolver;
2735
$this->systemInfoCollector = $systemInfoCollector;
36+
$this->translator = $translator;
37+
$this->entityManager = $entityManager;
2838
}
2939

3040
public function getHistory(int $lastId, int $limit, SubscriberHistoryFilter $filter): array
@@ -40,8 +50,94 @@ public function addHistory(Subscriber $subscriber, string $message, ?string $det
4050
$subscriberHistory->setSystemInfo($this->systemInfoCollector->collectAsString());
4151
$subscriberHistory->setIp($this->clientIpResolver->resolve());
4252

43-
$this->repository->save($subscriberHistory);
53+
$this->entityManager->persist($subscriberHistory);
4454

4555
return $subscriberHistory;
4656
}
57+
58+
public function addHistoryFromChangeSet(
59+
Subscriber $subscriber,
60+
string $message,
61+
ChangeSetDto $changeSet,
62+
): SubscriberHistory {
63+
$details = '';
64+
foreach ($changeSet->getChanges() as $attribute => [$old, $new]) {
65+
if (in_array($attribute, ChangeSetDto::IGNORED_ATTRIBUTES, true) || $new === null) {
66+
continue;
67+
}
68+
$details .= $this->translator->trans(
69+
"%attribute% = %new_value% \n changed from %old_value%",
70+
[
71+
'%attribute%' => $attribute,
72+
'%new_value%' => $new,
73+
'%old_value%' => $old ?? $this->translator->trans('(no data)'),
74+
]
75+
) . PHP_EOL;
76+
}
77+
78+
if ($details === '') {
79+
$details .= $this->translator->trans('No data changed') . PHP_EOL;
80+
}
81+
82+
return $this->addHistory($subscriber, $message, $details);
83+
}
84+
85+
public function addHistoryFromImport(
86+
Subscriber $subscriber,
87+
array $listLines,
88+
ChangeSetDto $changeSetDto,
89+
?Administrator $admin = null,
90+
): void {
91+
$headerLine = sprintf('API-v2-import - %s: %s%s', $admin ? 'Admin' : 'CLI', $admin?->getId(), "\n\n");
92+
93+
$lines = $this->getHistoryLines($changeSetDto, $listLines);
94+
95+
$this->addHistory(
96+
subscriber: $subscriber,
97+
message: 'Import by ' . $admin?->getLoginName(),
98+
details: $headerLine . implode(PHP_EOL, $lines) . PHP_EOL
99+
);
100+
}
101+
102+
public function addHistoryFromApi(
103+
Subscriber $subscriber,
104+
array $listLines,
105+
ChangeSetDto $updatedData,
106+
?Administrator $admin = null,
107+
): void {
108+
$lines = $this->getHistoryLines($updatedData, $listLines);
109+
110+
$this->addHistory(
111+
subscriber: $subscriber,
112+
message: $this->translator->trans('Update by %admin%', ['%admin%' => $admin->getLoginName()]),
113+
details: implode(PHP_EOL, $lines) . PHP_EOL
114+
);
115+
}
116+
117+
private function getHistoryLines(ChangeSetDto $updatedData, array $listLines): array
118+
{
119+
$lines = [];
120+
if (!$updatedData->hasChanges() && empty($listLines)) {
121+
$lines[] = $this->translator->trans('No user details changed');
122+
} else {
123+
foreach ($updatedData->getChanges() as $field => [$old, $new]) {
124+
if (in_array($field, ChangeSetDto::IGNORED_ATTRIBUTES, true)) {
125+
continue;
126+
}
127+
$lines[] = $this->translator->trans(
128+
'%field% = %new% *changed* from %old%',
129+
[
130+
'%field' => $field,
131+
'%new%' => json_encode($new),
132+
'%old%' => json_encode($old)
133+
],
134+
);
135+
}
136+
foreach ($listLines as $line) {
137+
$lines[] = $line;
138+
}
139+
}
140+
141+
return $lines;
142+
}
47143
}

0 commit comments

Comments
 (0)