diff --git a/config/services/providers.yml b/config/services/providers.yml index f4f06010..b7b66be8 100644 --- a/config/services/providers.yml +++ b/config/services/providers.yml @@ -27,3 +27,6 @@ services: autowire: true arguments: $cache: '@Psr\SimpleCache\CacheInterface' + + PhpList\Core\Domain\Subscription\Service\Provider\SubscriberAttributeChangeSetProvider: + autowire: true diff --git a/resources/translations/messages.en.xlf b/resources/translations/messages.en.xlf index 5bfb12fd..9a9fae29 100644 --- a/resources/translations/messages.en.xlf +++ b/resources/translations/messages.en.xlf @@ -684,6 +684,48 @@ Thank you. Footer of public pages __Footer of public pages + + Please confirm your subscription + __Please confirm your subscription + + + No user details changed + __No user details changed + + + %field% = %new% *changed* from %old% + __%field% = %new% *changed* from %old% + + + Subscribed to %list% + __Subscribed to %list% + + + Subscriber marked unconfirmed for invalid email address + __Subscriber marked unconfirmed for invalid email address + + + Marked unconfirmed while sending campaign %message_id% + __Marked unconfirmed while sending campaign %message_id% + + + Update by %admin% + __Update by %admin% + + + (no data) + __(no data) + + + %attribute% = %new_value% + changed from %old_value% + __%attribute% = %new_value% + changed from %old_value% + + + No data changed + __No data changed + diff --git a/src/Domain/Messaging/Service/Processor/CampaignProcessor.php b/src/Domain/Messaging/Service/Processor/CampaignProcessor.php index a5deb074..e16d4246 100644 --- a/src/Domain/Messaging/Service/Processor/CampaignProcessor.php +++ b/src/Domain/Messaging/Service/Processor/CampaignProcessor.php @@ -14,6 +14,7 @@ use PhpList\Core\Domain\Messaging\Service\RateLimitedCampaignMailer; use PhpList\Core\Domain\Messaging\Service\MaxProcessTimeLimiter; use PhpList\Core\Domain\Messaging\Service\MessageProcessingPreparator; +use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Provider\SubscriberProvider; use PhpList\Core\Domain\Subscription\Model\Subscriber; use Psr\Log\LoggerInterface; @@ -23,6 +24,8 @@ /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.StaticAccess) + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ class CampaignProcessor { @@ -35,6 +38,7 @@ class CampaignProcessor private MaxProcessTimeLimiter $timeLimiter; private RequeueHandler $requeueHandler; private TranslatorInterface $translator; + private SubscriberHistoryManager $subscriberHistoryManager; public function __construct( RateLimitedCampaignMailer $mailer, @@ -46,6 +50,7 @@ public function __construct( MaxProcessTimeLimiter $timeLimiter, RequeueHandler $requeueHandler, TranslatorInterface $translator, + SubscriberHistoryManager $subscriberHistoryManager, ) { $this->mailer = $mailer; $this->entityManager = $entityManager; @@ -56,6 +61,7 @@ public function __construct( $this->timeLimiter = $timeLimiter; $this->requeueHandler = $requeueHandler; $this->translator = $translator; + $this->subscriberHistoryManager = $subscriberHistoryManager; } public function process(Message $campaign, ?OutputInterface $output = null): void @@ -89,6 +95,14 @@ public function process(Message $campaign, ?OutputInterface $output = null): voi $output?->writeln($this->translator->trans('Invalid email, marking unconfirmed: %email%', [ '%email%' => $subscriber->getEmail(), ])); + $this->subscriberHistoryManager->addHistory( + subscriber: $subscriber, + message: $this->translator->trans('Subscriber marked unconfirmed for invalid email address'), + details: $this->translator->trans( + 'Marked unconfirmed while sending campaign %message_id%', + ['%message_id%' => $campaign->getId()] + ) + ); continue; } diff --git a/src/Domain/Subscription/Model/Dto/ChangeSetDto.php b/src/Domain/Subscription/Model/Dto/ChangeSetDto.php new file mode 100644 index 00000000..ef3cc2da --- /dev/null +++ b/src/Domain/Subscription/Model/Dto/ChangeSetDto.php @@ -0,0 +1,81 @@ + + * + * Example: + * [ + * 'email' => [null, 'newemail@example.com'], + * 'isActive' => [true, false] + * ] + */ + private array $changes = []; + + /** + * @param array $changes + */ + public function __construct(array $changes = []) + { + $this->changes = $changes; + } + + /** + * @return array + */ + public function getChanges(): array + { + return $this->changes; + } + + public function hasChanges(): bool + { + return !empty($this->changes); + } + + public function hasField(string $field): bool + { + return array_key_exists($field, $this->changes); + } + + /** + * @return array{0: mixed, 1: mixed}|null + */ + public function getFieldChange(string $field): ?array + { + return $this->changes[$field] ?? null; + } + + /** + * @return mixed|null + */ + public function getOldValue(string $field): mixed + { + return $this->changes[$field][0] ?? null; + } + + /** + * @return mixed|null + */ + public function getNewValue(string $field): mixed + { + return $this->changes[$field][1] ?? null; + } + + public function toArray(): array + { + return $this->changes; + } + + public static function fromDoctrineChangeSet(array $changeSet): self + { + return new self($changeSet); + } +} diff --git a/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php b/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php index 4446e0bf..75b9e3e2 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php @@ -6,24 +6,29 @@ use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Domain\Subscription\Exception\SubscriberAttributeCreationException; +use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeDefinition; use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeValue; +use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository; use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository; use Symfony\Contracts\Translation\TranslatorInterface; class SubscriberAttributeManager { private SubscriberAttributeValueRepository $attributeRepository; + private SubscriberAttributeDefinitionRepository $attrDefinitionRepository; private EntityManagerInterface $entityManager; private TranslatorInterface $translator; public function __construct( SubscriberAttributeValueRepository $attributeRepository, + SubscriberAttributeDefinitionRepository $attrDefinitionRepository, EntityManagerInterface $entityManager, TranslatorInterface $translator, ) { $this->attributeRepository = $attributeRepository; + $this->attrDefinitionRepository = $attrDefinitionRepository; $this->entityManager = $entityManager; $this->translator = $translator; } @@ -33,6 +38,8 @@ public function createOrUpdate( SubscriberAttributeDefinition $definition, ?string $value = null ): SubscriberAttributeValue { + // phpcs:ignore Generic.Commenting.Todo + // todo: clarify which attributes can be created/updated $subscriberAttribute = $this->attributeRepository ->findOneBySubscriberAndAttribute($subscriber, $definition); @@ -60,4 +67,23 @@ public function delete(SubscriberAttributeValue $attribute): void { $this->attributeRepository->remove($attribute); } + + public function processAttributes(Subscriber $subscriber, array $attributeData): void + { + foreach ($attributeData as $key => $value) { + $lowerKey = strtolower((string)$key); + if (in_array($lowerKey, ChangeSetDto::IGNORED_ATTRIBUTES, true)) { + continue; + } + + $attributeDefinition = $this->attrDefinitionRepository->findOneByName($key); + if ($attributeDefinition !== null) { + $this->createOrUpdate( + subscriber: $subscriber, + definition: $attributeDefinition, + value: $value + ); + } + } + } } diff --git a/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php b/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php index bac2ef8d..f8d35c0a 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php @@ -4,27 +4,37 @@ namespace PhpList\Core\Domain\Subscription\Service\Manager; +use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Domain\Common\ClientIpResolver; use PhpList\Core\Domain\Common\SystemInfoCollector; +use PhpList\Core\Domain\Identity\Model\Administrator; +use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto; use PhpList\Core\Domain\Subscription\Model\Filter\SubscriberHistoryFilter; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberHistory; use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository; +use Symfony\Contracts\Translation\TranslatorInterface; class SubscriberHistoryManager { private SubscriberHistoryRepository $repository; private ClientIpResolver $clientIpResolver; private SystemInfoCollector $systemInfoCollector; + private TranslatorInterface $translator; + private EntityManagerInterface $entityManager; public function __construct( SubscriberHistoryRepository $repository, ClientIpResolver $clientIpResolver, SystemInfoCollector $systemInfoCollector, + TranslatorInterface $translator, + EntityManagerInterface $entityManager, ) { $this->repository = $repository; $this->clientIpResolver = $clientIpResolver; $this->systemInfoCollector = $systemInfoCollector; + $this->translator = $translator; + $this->entityManager = $entityManager; } public function getHistory(int $lastId, int $limit, SubscriberHistoryFilter $filter): array @@ -40,8 +50,94 @@ public function addHistory(Subscriber $subscriber, string $message, ?string $det $subscriberHistory->setSystemInfo($this->systemInfoCollector->collectAsString()); $subscriberHistory->setIp($this->clientIpResolver->resolve()); - $this->repository->save($subscriberHistory); + $this->entityManager->persist($subscriberHistory); return $subscriberHistory; } + + public function addHistoryFromChangeSet( + Subscriber $subscriber, + string $message, + ChangeSetDto $changeSet, + ): SubscriberHistory { + $details = ''; + foreach ($changeSet->getChanges() as $attribute => [$old, $new]) { + if (in_array($attribute, ChangeSetDto::IGNORED_ATTRIBUTES, true) || $new === null) { + continue; + } + $details .= $this->translator->trans( + "%attribute% = %new_value% \n changed from %old_value%", + [ + '%attribute%' => $attribute, + '%new_value%' => $new, + '%old_value%' => $old ?? $this->translator->trans('(no data)'), + ] + ) . PHP_EOL; + } + + if ($details === '') { + $details .= $this->translator->trans('No data changed') . PHP_EOL; + } + + return $this->addHistory($subscriber, $message, $details); + } + + public function addHistoryFromImport( + Subscriber $subscriber, + array $listLines, + ChangeSetDto $changeSetDto, + ?Administrator $admin = null, + ): void { + $headerLine = sprintf('API-v2-import - %s: %s%s', $admin ? 'Admin' : 'CLI', $admin?->getId(), "\n\n"); + + $lines = $this->getHistoryLines($changeSetDto, $listLines); + + $this->addHistory( + subscriber: $subscriber, + message: 'Import by ' . $admin?->getLoginName(), + details: $headerLine . implode(PHP_EOL, $lines) . PHP_EOL + ); + } + + public function addHistoryFromApi( + Subscriber $subscriber, + array $listLines, + ChangeSetDto $updatedData, + ?Administrator $admin = null, + ): void { + $lines = $this->getHistoryLines($updatedData, $listLines); + + $this->addHistory( + subscriber: $subscriber, + message: $this->translator->trans('Update by %admin%', ['%admin%' => $admin->getLoginName()]), + details: implode(PHP_EOL, $lines) . PHP_EOL + ); + } + + private function getHistoryLines(ChangeSetDto $updatedData, array $listLines): array + { + $lines = []; + if (!$updatedData->hasChanges() && empty($listLines)) { + $lines[] = $this->translator->trans('No user details changed'); + } else { + foreach ($updatedData->getChanges() as $field => [$old, $new]) { + if (in_array($field, ChangeSetDto::IGNORED_ATTRIBUTES, true)) { + continue; + } + $lines[] = $this->translator->trans( + '%field% = %new% *changed* from %old%', + [ + '%field' => $field, + '%new%' => json_encode($new), + '%old%' => json_encode($old) + ], + ); + } + foreach ($listLines as $line) { + $lines[] = $line; + } + } + + return $lines; + } } diff --git a/src/Domain/Subscription/Service/Manager/SubscriberManager.php b/src/Domain/Subscription/Service/Manager/SubscriberManager.php index 59cd2505..40bfcc20 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriberManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriberManager.php @@ -5,38 +5,40 @@ namespace PhpList\Core\Domain\Subscription\Service\Manager; use Doctrine\ORM\EntityManagerInterface; -use PhpList\Core\Domain\Messaging\Message\SubscriberConfirmationMessage; +use PhpList\Core\Domain\Identity\Model\Administrator; +use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto; use PhpList\Core\Domain\Subscription\Model\Dto\CreateSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Dto\UpdateSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; -use PhpList\Core\Domain\Subscription\Service\SubscriberBlacklistService; use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; -use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Contracts\Translation\TranslatorInterface; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class SubscriberManager { private SubscriberRepository $subscriberRepository; private EntityManagerInterface $entityManager; - private MessageBusInterface $messageBus; private SubscriberDeletionService $subscriberDeletionService; private TranslatorInterface $translator; + private SubscriberHistoryManager $subscriberHistoryManager; public function __construct( SubscriberRepository $subscriberRepository, EntityManagerInterface $entityManager, - MessageBusInterface $messageBus, SubscriberDeletionService $subscriberDeletionService, - TranslatorInterface $translator + TranslatorInterface $translator, + SubscriberHistoryManager $subscriberHistoryManager, ) { $this->subscriberRepository = $subscriberRepository; $this->entityManager = $entityManager; - $this->messageBus = $messageBus; $this->subscriberDeletionService = $subscriberDeletionService; $this->translator = $translator; + $this->subscriberHistoryManager = $subscriberHistoryManager; } public function createSubscriber(CreateSubscriberDto $subscriberDto): Subscriber @@ -51,30 +53,16 @@ public function createSubscriber(CreateSubscriberDto $subscriberDto): Subscriber $this->subscriberRepository->save($subscriber); - if ($subscriberDto->requestConfirmation) { - $this->sendConfirmationEmail($subscriber); - } - return $subscriber; } - private function sendConfirmationEmail(Subscriber $subscriber): void - { - $message = new SubscriberConfirmationMessage( - email: $subscriber->getEmail(), - uniqueId:$subscriber->getUniqueId(), - htmlEmail: $subscriber->hasHtmlEmail() - ); - - $this->messageBus->dispatch($message); - } - public function getSubscriberById(int $subscriberId): ?Subscriber { return $this->subscriberRepository->find($subscriberId); } - public function updateSubscriber(UpdateSubscriberDto $subscriberDto): Subscriber + /** @SuppressWarnings(PHPMD.StaticAccess) */ + public function updateSubscriber(UpdateSubscriberDto $subscriberDto, Administrator $admin): Subscriber { /** @var Subscriber $subscriber */ $subscriber = $this->subscriberRepository->find($subscriberDto->subscriberId); @@ -86,7 +74,12 @@ public function updateSubscriber(UpdateSubscriberDto $subscriberDto): Subscriber $subscriber->setDisabled($subscriberDto->disabled); $subscriber->setExtraData($subscriberDto->additionalData); - $this->entityManager->flush(); + $uow = $this->entityManager->getUnitOfWork(); + $meta = $this->entityManager->getClassMetadata(Subscriber::class); + $uow->computeChangeSet($meta, $subscriber); + $changeSet = ChangeSetDto::fromDoctrineChangeSet($uow->getEntityChangeSet($subscriber)); + + $this->subscriberHistoryManager->addHistoryFromApi($subscriber, [], $changeSet, $admin); return $subscriber; } @@ -129,14 +122,11 @@ public function createFromImport(ImportSubscriberDto $subscriberDto): Subscriber $this->entityManager->persist($subscriber); - if ($subscriberDto->sendConfirmation) { - $this->sendConfirmationEmail($subscriber); - } - return $subscriber; } - public function updateFromImport(Subscriber $existingSubscriber, ImportSubscriberDto $subscriberDto): Subscriber + /** @SuppressWarnings(PHPMD.StaticAccess) */ + public function updateFromImport(Subscriber $existingSubscriber, ImportSubscriberDto $subscriberDto): ChangeSetDto { $existingSubscriber->setEmail($subscriberDto->email); $existingSubscriber->setConfirmed($subscriberDto->confirmed); @@ -145,7 +135,11 @@ public function updateFromImport(Subscriber $existingSubscriber, ImportSubscribe $existingSubscriber->setDisabled($subscriberDto->disabled); $existingSubscriber->setExtraData($subscriberDto->extraData); - return $existingSubscriber; + $uow = $this->entityManager->getUnitOfWork(); + $meta = $this->entityManager->getClassMetadata(Subscriber::class); + $uow->computeChangeSet($meta, $existingSubscriber); + + return ChangeSetDto::fromDoctrineChangeSet($uow->getEntityChangeSet($existingSubscriber)); } public function decrementBounceCount(Subscriber $subscriber): void diff --git a/src/Domain/Subscription/Service/Manager/SubscriptionManager.php b/src/Domain/Subscription/Service/Manager/SubscriptionManager.php index c9921bab..b3631d91 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriptionManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriptionManager.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Domain\Subscription\Service\Manager; +use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Domain\Subscription\Exception\SubscriptionCreationException; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberList; @@ -19,17 +20,20 @@ class SubscriptionManager private SubscriberRepository $subscriberRepository; private SubscriberListRepository $subscriberListRepository; private TranslatorInterface $translator; + private EntityManagerInterface $entityManager; public function __construct( SubscriptionRepository $subscriptionRepository, SubscriberRepository $subscriberRepository, SubscriberListRepository $subscriberListRepository, - TranslatorInterface $translator + TranslatorInterface $translator, + EntityManagerInterface $entityManager ) { $this->subscriptionRepository = $subscriptionRepository; $this->subscriberRepository = $subscriberRepository; $this->subscriberListRepository = $subscriberListRepository; $this->translator = $translator; + $this->entityManager = $entityManager; } public function addSubscriberToAList(Subscriber $subscriber, int $listId): ?Subscription @@ -49,7 +53,7 @@ public function addSubscriberToAList(Subscriber $subscriber, int $listId): ?Subs $subscription->setSubscriber($subscriber); $subscription->setSubscriberList($subscriberList); - $this->subscriptionRepository->save($subscription); + $this->entityManager->persist($subscription); return $subscription; } @@ -83,7 +87,7 @@ private function createSubscription(SubscriberList $subscriberList, string $emai $subscription->setSubscriber($subscriber); $subscription->setSubscriberList($subscriberList); - $this->subscriptionRepository->save($subscription); + $this->entityManager->persist($subscription); return $subscription; } @@ -111,7 +115,7 @@ private function deleteSubscription(SubscriberList $subscriberList, string $emai throw new SubscriptionCreationException($message, 404); } - $this->subscriptionRepository->remove($subscription); + $this->entityManager->remove($subscription); } /** @return Subscriber[] */ diff --git a/src/Domain/Subscription/Service/Provider/SubscriberAttributeChangeSetProvider.php b/src/Domain/Subscription/Service/Provider/SubscriberAttributeChangeSetProvider.php new file mode 100644 index 00000000..290c6de8 --- /dev/null +++ b/src/Domain/Subscription/Service/Provider/SubscriberAttributeChangeSetProvider.php @@ -0,0 +1,81 @@ + $attributeData + * @return ChangeSetDto + */ + public function getAttributeChangeSet(Subscriber $subscriber, array $attributeData): ChangeSetDto + { + $oldMap = $this->getMappedValues($subscriber); + + $canon = static function (array $attributes): array { + $out = []; + foreach ($attributes as $key => $value) { + $out[mb_strtolower((string)$key)] = $value; + } + return $out; + }; + + $oldC = $canon($oldMap); + $newC = $canon($attributeData); + + foreach (ChangeSetDto::IGNORED_ATTRIBUTES as $ignoredAttribute) { + $lowerCaseIgnoredAttribute = mb_strtolower($ignoredAttribute); + unset($oldC[$lowerCaseIgnoredAttribute], $newC[$lowerCaseIgnoredAttribute]); + } + + $keys = array_values(array_unique(array_merge(array_keys($oldC), array_keys($newC)))); + + $changeSet = []; + foreach ($keys as $key) { + $hasOld = array_key_exists($key, $oldC); + $hasNew = array_key_exists($key, $newC); + + if ($hasOld && !$hasNew) { + $changeSet[$key] = [$oldC[$key], null]; + continue; + } + + if (!$hasOld && $hasNew) { + $changeSet[$key] = [null, $newC[$key]]; + continue; + } + + if ($oldC[$key] !== $newC[$key]) { + $changeSet[$key] = [$oldC[$key], $newC[$key]]; + } + } + + return new ChangeSetDto($changeSet); + } + + private function getMappedValues(Subscriber $subscriber): array + { + $userAttributes = $this->attributesRepository->getForSubscriber($subscriber); + foreach ($userAttributes as $userAttribute) { + $data[$userAttribute->getAttributeDefinition()->getName()] = $this->resolver->resolve($userAttribute); + } + + return $data ?? []; + } +} diff --git a/src/Domain/Subscription/Service/SubscriberCsvImporter.php b/src/Domain/Subscription/Service/SubscriberCsvImporter.php index f5d9e535..e80db165 100644 --- a/src/Domain/Subscription/Service/SubscriberCsvImporter.php +++ b/src/Domain/Subscription/Service/SubscriberCsvImporter.php @@ -5,14 +5,16 @@ namespace PhpList\Core\Domain\Subscription\Service; use Doctrine\ORM\EntityManagerInterface; +use PhpList\Core\Domain\Identity\Model\Administrator; use PhpList\Core\Domain\Messaging\Message\SubscriptionConfirmationMessage; use PhpList\Core\Domain\Subscription\Exception\CouldNotReadUploadedFileException; +use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto; use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Dto\SubscriberImportOptions; use PhpList\Core\Domain\Subscription\Model\Subscriber; -use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberAttributeManager; +use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberManager; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriptionManager; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -21,8 +23,11 @@ use Throwable; /** + * phpcs:ignore Generic.Commenting.Todo + * @todo: check if dryRun will work (some function flush) * Service for importing subscribers from a CSV file. * @SuppressWarnings("CouplingBetweenObjects") + * @SuppressWarnings("ExcessiveParameterList") */ class SubscriberCsvImporter { @@ -31,10 +36,10 @@ class SubscriberCsvImporter private SubscriptionManager $subscriptionManager; private SubscriberRepository $subscriberRepository; private CsvImporter $csvImporter; - private SubscriberAttributeDefinitionRepository $attrDefinitionRepository; private EntityManagerInterface $entityManager; private TranslatorInterface $translator; private MessageBusInterface $messageBus; + private SubscriberHistoryManager $subscriberHistoryManager; public function __construct( SubscriberManager $subscriberManager, @@ -42,36 +47,37 @@ public function __construct( SubscriptionManager $subscriptionManager, SubscriberRepository $subscriberRepository, CsvImporter $csvImporter, - SubscriberAttributeDefinitionRepository $attrDefinitionRepository, EntityManagerInterface $entityManager, TranslatorInterface $translator, MessageBusInterface $messageBus, + SubscriberHistoryManager $subscriberHistoryManager, ) { $this->subscriberManager = $subscriberManager; $this->attributeManager = $attributeManager; $this->subscriptionManager = $subscriptionManager; $this->subscriberRepository = $subscriberRepository; $this->csvImporter = $csvImporter; - $this->attrDefinitionRepository = $attrDefinitionRepository; $this->entityManager = $entityManager; $this->translator = $translator; $this->messageBus = $messageBus; + $this->subscriberHistoryManager = $subscriberHistoryManager; } /** * Import subscribers from a CSV file. - * - * @param UploadedFile $file The uploaded CSV file - * @param SubscriberImportOptions $options * @return array Import statistics - * @throws CouldNotReadUploadedFileException When the uploaded file cannot be read during import + * @throws CouldNotReadUploadedFileException */ - public function importFromCsv(UploadedFile $file, SubscriberImportOptions $options): array - { + public function importFromCsv( + UploadedFile $file, + SubscriberImportOptions $options, + ?Administrator $admin = null + ): array { $stats = [ 'created' => 0, 'updated' => 0, 'skipped' => 0, + 'blacklisted' => 0, 'errors' => [], ]; @@ -87,8 +93,22 @@ public function importFromCsv(UploadedFile $file, SubscriberImportOptions $optio foreach ($result['valid'] as $dto) { try { - $this->processRow($dto, $options, $stats); + $this->entityManager->beginTransaction(); + + $message = $this->processRow($dto, $options, $stats, $admin); + + if ($options->dryRun) { + $this->entityManager->rollback(); + } else { + $this->entityManager->flush(); + $this->entityManager->commit(); + if ($message !== null) { + $this->messageBus->dispatch($message); + } + } } catch (Throwable $e) { + $this->entityManager->rollback(); + $stats['errors'][] = $this->translator->trans( 'Error processing %email%: %error%', ['%email%' => $dto->email, '%error%' => $e->getMessage()] @@ -117,11 +137,16 @@ public function importFromCsv(UploadedFile $file, SubscriberImportOptions $optio * @param UploadedFile $file The uploaded CSV file * @return array Import statistics */ - public function importAndUpdateFromCsv(UploadedFile $file, ?array $listIds = [], bool $dryRun = false): array - { + public function importAndUpdateFromCsv( + UploadedFile $file, + Administrator $admin, + ?array $listIds = [], + bool $dryRun = false + ): array { return $this->importFromCsv( file: $file, - options: new SubscriberImportOptions(updateExisting: true, listIds: $listIds, dryRun: $dryRun) + options: new SubscriberImportOptions(updateExisting: true, listIds: $listIds, dryRun: $dryRun), + admin: $admin, ); } @@ -131,58 +156,74 @@ public function importAndUpdateFromCsv(UploadedFile $file, ?array $listIds = [], * @param UploadedFile $file The uploaded CSV file * @return array Import statistics */ - public function importNewFromCsv(UploadedFile $file, ?array $listIds = [], bool $dryRun = false): array - { + public function importNewFromCsv( + UploadedFile $file, + Administrator $admin, + ?array $listIds = [], + bool $dryRun = false + ): array { return $this->importFromCsv( file: $file, - options: new SubscriberImportOptions(listIds: $listIds, dryRun: $dryRun) + options: new SubscriberImportOptions(listIds: $listIds, dryRun: $dryRun), + admin: $admin, ); } /** * Process a single row from the CSV file. - * - * @param ImportSubscriberDto $dto - * @param SubscriberImportOptions $options - * @param array $stats Statistics to update */ private function processRow( ImportSubscriberDto $dto, SubscriberImportOptions $options, array &$stats, - ): void { + ?Administrator $admin = null + ): ?SubscriptionConfirmationMessage { if ($this->handleInvalidEmail($dto, $options, $stats)) { - return; + return null; } $subscriber = $this->subscriberRepository->findOneByEmail($dto->email); - if ($subscriber && !$options->updateExisting) { - $stats['skipped']++; - - return; + if ($this->handleSkipCase($subscriber, $options, $stats)) { + return null; } if ($subscriber) { - $this->subscriberManager->updateFromImport($subscriber, $dto); + $changeSet = $this->subscriberManager->updateFromImport($subscriber, $dto); $stats['updated']++; } else { $subscriber = $this->subscriberManager->createFromImport($dto); $stats['created']++; } - $this->processAttributes($subscriber, $dto); + $this->attributeManager->processAttributes($subscriber, $dto->extraAttributes); $addedNewSubscriberToList = false; + $listLines = []; if (!$subscriber->isBlacklisted() && count($options->listIds) > 0) { foreach ($options->listIds as $listId) { $created = $this->subscriptionManager->addSubscriberToAList($subscriber, $listId); if ($created) { $addedNewSubscriberToList = true; + $listLines[] = $this->translator->trans( + 'Subscribed to %list%', + ['%list%' => $created->getSubscriberList()->getName()] + ); } } } - $this->handleFlushAndEmail($subscriber, $options, $dto, $addedNewSubscriberToList); + if ($subscriber->isBlacklisted()) { + $stats['blacklisted']++; + } + + $this->subscriberHistoryManager->addHistoryFromImport( + subscriber: $subscriber, + listLines: $listLines, + changeSetDto: $changeSet ?? new ChangeSetDto(), + admin: $admin + ); + + return $this->prepareConfirmationMessage($subscriber, $options, $dto, $addedNewSubscriberToList); } private function handleInvalidEmail( @@ -205,55 +246,35 @@ private function handleInvalidEmail( return false; } - private function handleFlushAndEmail( - Subscriber $subscriber, + private function handleSkipCase( + ?Subscriber $existingSubscriber, SubscriberImportOptions $options, - ImportSubscriberDto $dto, - bool $addedNewSubscriberToList - ): void { - if (!$options->dryRun) { - $this->entityManager->flush(); - if ($dto->sendConfirmation && $addedNewSubscriberToList) { - $this->sendSubscribeEmail($subscriber, $options->listIds); - } - } - } + array &$stats + ): bool { + if ($existingSubscriber && !$options->updateExisting) { + $stats['skipped']++; - private function sendSubscribeEmail(Subscriber $subscriber, array $listIds): void - { - $message = new SubscriptionConfirmationMessage( - email: $subscriber->getEmail(), - uniqueId: $subscriber->getUniqueId(), - listIds: $listIds, - htmlEmail: $subscriber->hasHtmlEmail(), - ); + return true; + } - $this->messageBus->dispatch($message); + return false; } - /** - * Process subscriber attributes. - * - * @param Subscriber $subscriber The subscriber - * @param ImportSubscriberDto $dto - */ - private function processAttributes(Subscriber $subscriber, ImportSubscriberDto $dto): void - { - foreach ($dto->extraAttributes as $key => $value) { - $lowerKey = strtolower((string)$key); - // Do not import or update sensitive/system fields from CSV - if (in_array($lowerKey, ['password', 'modified'], true)) { - continue; - } - - $attributeDefinition = $this->attrDefinitionRepository->findOneByName($key); - if ($attributeDefinition !== null) { - $this->attributeManager->createOrUpdate( - subscriber: $subscriber, - definition: $attributeDefinition, - value: $value - ); - } + private function prepareConfirmationMessage( + Subscriber $subscriber, + SubscriberImportOptions $options, + ImportSubscriberDto $dto, + bool $addedNewSubscriberToList + ): ?SubscriptionConfirmationMessage { + if ($dto->sendConfirmation && $addedNewSubscriberToList) { + return new SubscriptionConfirmationMessage( + email: $subscriber->getEmail(), + uniqueId: $subscriber->getUniqueId(), + listIds: $options->listIds, + htmlEmail: $subscriber->hasHtmlEmail(), + ); } + + return null; } } diff --git a/tests/Unit/Domain/Messaging/Service/Processor/CampaignProcessorTest.php b/tests/Unit/Domain/Messaging/Service/Processor/CampaignProcessorTest.php index e1976202..f2f82752 100644 --- a/tests/Unit/Domain/Messaging/Service/Processor/CampaignProcessorTest.php +++ b/tests/Unit/Domain/Messaging/Service/Processor/CampaignProcessorTest.php @@ -16,6 +16,7 @@ use PhpList\Core\Domain\Messaging\Service\Processor\CampaignProcessor; use PhpList\Core\Domain\Messaging\Service\RateLimitedCampaignMailer; use PhpList\Core\Domain\Subscription\Model\Subscriber; +use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Provider\SubscriberProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -54,6 +55,7 @@ protected function setUp(): void timeLimiter: $this->createMock(MaxProcessTimeLimiter::class), requeueHandler: $this->createMock(RequeueHandler::class), translator: new Translator('en'), + subscriberHistoryManager: $this->createMock(SubscriberHistoryManager::class), ); } diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberAttributeManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberAttributeManagerTest.php index a827ab3f..332b3a7c 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberAttributeManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberAttributeManagerTest.php @@ -9,6 +9,7 @@ use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeDefinition; use PhpList\Core\Domain\Subscription\Model\SubscriberAttributeValue; +use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository; use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeValueRepository; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberAttributeManager; use PHPUnit\Framework\TestCase; @@ -35,7 +36,12 @@ public function testCreateNewSubscriberAttribute(): void return $attr->getValue() === 'US'; })); - $manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en')); + $manager = new SubscriberAttributeManager( + attributeRepository: $subscriberAttrRepo, + attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class), + entityManager: $entityManager, + translator: new Translator('en') + ); $attribute = $manager->createOrUpdate($subscriber, $definition, 'US'); self::assertInstanceOf(SubscriberAttributeValue::class, $attribute); @@ -61,7 +67,12 @@ public function testUpdateExistingSubscriberAttribute(): void ->method('persist') ->with($existing); - $manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en')); + $manager = new SubscriberAttributeManager( + attributeRepository: $subscriberAttrRepo, + attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class), + entityManager: $entityManager, + translator: new Translator('en') + ); $result = $manager->createOrUpdate($subscriber, $definition, 'Updated'); self::assertSame('Updated', $result->getValue()); @@ -77,7 +88,12 @@ public function testCreateFailsWhenValueAndDefaultAreNull(): void $subscriberAttrRepo->method('findOneBySubscriberAndAttribute')->willReturn(null); - $manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en')); + $manager = new SubscriberAttributeManager( + attributeRepository: $subscriberAttrRepo, + attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class), + entityManager: $entityManager, + translator: new Translator('en') + ); $this->expectException(SubscriberAttributeCreationException::class); $this->expectExceptionMessage('Value is required'); @@ -96,7 +112,12 @@ public function testGetSubscriberAttribute(): void ->with(5, 10) ->willReturn($expected); - $manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en')); + $manager = new SubscriberAttributeManager( + attributeRepository: $subscriberAttrRepo, + attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class), + entityManager: $entityManager, + translator: new Translator('en') + ); $result = $manager->getSubscriberAttribute(5, 10); self::assertSame($expected, $result); @@ -112,7 +133,12 @@ public function testDeleteSubscriberAttribute(): void ->method('remove') ->with($attribute); - $manager = new SubscriberAttributeManager($subscriberAttrRepo, $entityManager, new Translator('en')); + $manager = new SubscriberAttributeManager( + attributeRepository: $subscriberAttrRepo, + attrDefinitionRepository: $this->createMock(SubscriberAttributeDefinitionRepository::class), + entityManager: $entityManager, + translator: new Translator('en') + ); $manager->delete($attribute); self::assertTrue(true); diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php index 43ae2fcc..85e99730 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Tests\Unit\Domain\Subscription\Service\Manager; +use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Domain\Common\ClientIpResolver; use PhpList\Core\Domain\Common\SystemInfoCollector; use PhpList\Core\Domain\Subscription\Model\Filter\SubscriberHistoryFilter; @@ -12,6 +13,7 @@ use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Contracts\Translation\TranslatorInterface; class SubscriberHistoryManagerTest extends TestCase { @@ -25,6 +27,8 @@ protected function setUp(): void repository: $this->subscriberHistoryRepository, clientIpResolver: $this->createMock(ClientIpResolver::class), systemInfoCollector: $this->createMock(SystemInfoCollector::class), + translator: $this->createMock(TranslatorInterface::class), + entityManager: $this->createMock(EntityManagerInterface::class), ); } diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php index f96f32e2..4f11c393 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php @@ -5,38 +5,34 @@ namespace PhpList\Core\Tests\Unit\Domain\Subscription\Service\Manager; use Doctrine\ORM\EntityManagerInterface; -use PhpList\Core\Domain\Messaging\Message\SubscriberConfirmationMessage; use PhpList\Core\Domain\Subscription\Model\Dto\CreateSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; +use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberManager; use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Symfony\Component\Messenger\Envelope; -use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Translation\Translator; class SubscriberManagerTest extends TestCase { private SubscriberRepository|MockObject $subscriberRepository; private EntityManagerInterface|MockObject $entityManager; - private MessageBusInterface|MockObject $messageBus; private SubscriberManager $subscriberManager; protected function setUp(): void { $this->subscriberRepository = $this->createMock(SubscriberRepository::class); $this->entityManager = $this->createMock(EntityManagerInterface::class); - $this->messageBus = $this->createMock(MessageBusInterface::class); $subscriberDeletionService = $this->createMock(SubscriberDeletionService::class); $this->subscriberManager = new SubscriberManager( subscriberRepository: $this->subscriberRepository, entityManager: $this->entityManager, - messageBus: $this->messageBus, subscriberDeletionService: $subscriberDeletionService, translator: new Translator('en'), + subscriberHistoryManager: $this->createMock(SubscriberHistoryManager::class) ); } @@ -65,7 +61,7 @@ public function testCreateSubscriberPersistsAndReturnsProperlyInitializedEntity( $this->assertFalse($result->isDisabled()); } - public function testCreateSubscriberPersistsAndSendsEmail(): void + public function testCreateSubscriberPersists(): void { $this->subscriberRepository ->expects($this->once()) @@ -79,13 +75,6 @@ public function testCreateSubscriberPersistsAndSendsEmail(): void && $sub->isDisabled() === false; })); - $this->messageBus - ->expects($this->once()) - ->method('dispatch') - ->willReturnCallback(function ($message) { - return new Envelope($message); - }); - $dto = new CreateSubscriberDto(email: 'foo@bar.com', requestConfirmation: true, htmlEmail: true); $result = $this->subscriberManager->createSubscriber($dto); @@ -97,7 +86,7 @@ public function testCreateSubscriberPersistsAndSendsEmail(): void $this->assertFalse($result->isDisabled()); } - public function testCreateSubscriberWithConfirmationSendsConfirmationEmail(): void + public function testCreateSubscriberWithConfirmation(): void { $capturedSubscriber = null; $this->subscriberRepository @@ -109,19 +98,6 @@ public function testCreateSubscriberWithConfirmationSendsConfirmationEmail(): vo return true; })); - $this->messageBus - ->expects($this->once()) - ->method('dispatch') - ->with($this->callback(function (SubscriberConfirmationMessage $message) { - $this->assertEquals('test@example.com', $message->getEmail()); - $this->assertEquals('test-unique-id-123', $message->getUniqueId()); - $this->assertTrue($message->hasHtmlEmail()); - return true; - })) - ->willReturnCallback(function ($message) { - return new Envelope($message); - }); - $dto = new CreateSubscriberDto(email: 'test@example.com', requestConfirmation: true, htmlEmail: true); $this->subscriberManager->createSubscriber($dto); @@ -131,16 +107,12 @@ public function testCreateSubscriberWithConfirmationSendsConfirmationEmail(): vo $this->assertFalse($capturedSubscriber->isConfirmed()); } - public function testCreateSubscriberWithoutConfirmationDoesNotSendConfirmationEmail(): void + public function testCreateSubscriberWithoutConfirmation(): void { $this->subscriberRepository ->expects($this->once()) ->method('save'); - $this->messageBus - ->expects($this->never()) - ->method('dispatch'); - $dto = new CreateSubscriberDto(email: 'test@example.com', requestConfirmation: false, htmlEmail: true); $this->subscriberManager->createSubscriber($dto); } diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscriptionManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscriptionManagerTest.php index f0c1d3af..edbe1c07 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscriptionManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscriptionManagerTest.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Tests\Unit\Domain\Subscription\Service\Manager; +use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Domain\Subscription\Exception\SubscriptionCreationException; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberList; @@ -21,6 +22,7 @@ class SubscriptionManagerTest extends TestCase private SubscriptionRepository&MockObject $subscriptionRepository; private SubscriberRepository&MockObject $subscriberRepository; private TranslatorInterface&MockObject $translator; + private EntityManagerInterface&MockObject $entityManager; private SubscriptionManager $manager; protected function setUp(): void @@ -29,11 +31,13 @@ protected function setUp(): void $this->subscriberRepository = $this->createMock(SubscriberRepository::class); $subscriberListRepository = $this->createMock(SubscriberListRepository::class); $this->translator = $this->createMock(TranslatorInterface::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); $this->manager = new SubscriptionManager( subscriptionRepository: $this->subscriptionRepository, subscriberRepository: $this->subscriberRepository, subscriberListRepository: $subscriberListRepository, translator: $this->translator, + entityManager: $this->entityManager, ); } @@ -45,7 +49,7 @@ public function testCreateSubscriptionWhenSubscriberExists(): void $this->subscriberRepository->method('findOneBy')->with(['email' => $email])->willReturn($subscriber); $this->subscriptionRepository->method('findOneBySubscriberListAndSubscriber')->willReturn(null); - $this->subscriptionRepository->expects($this->once())->method('save'); + $this->entityManager->expects($this->once())->method('persist'); $subscriptions = $this->manager->createSubscriptions($list, [$email]); @@ -78,7 +82,7 @@ public function testDeleteSubscriptionSuccessfully(): void ->with($subscriberList->getId(), $email) ->willReturn($subscription); - $this->subscriptionRepository->expects($this->once())->method('remove')->with($subscription); + $this->entityManager->expects($this->once())->method('remove')->with($subscription); $this->manager->deleteSubscriptions($subscriberList, [$email]); } diff --git a/tests/Unit/Domain/Subscription/Service/Provider/SubscriberAttributeChangeSetProviderTest.php b/tests/Unit/Domain/Subscription/Service/Provider/SubscriberAttributeChangeSetProviderTest.php new file mode 100644 index 00000000..a38baabf --- /dev/null +++ b/tests/Unit/Domain/Subscription/Service/Provider/SubscriberAttributeChangeSetProviderTest.php @@ -0,0 +1,175 @@ +resolver = $this->createMock(AttributeValueResolver::class); + $this->resolver + ->method('resolve') + ->willReturnCallback(function (SubscriberAttributeValue $attr) { + return $attr->getValue(); + }); + + $this->repository = $this->createMock(SubscriberAttributeValueRepository::class); + + $this->provider = new SubscriberAttributeChangeSetProvider( + resolver: $this->resolver, + attributesRepository: $this->repository, + ); + } + + public function testNoChangesWhenNewAndExistingAreIdenticalCaseInsensitive(): void + { + $subscriber = new Subscriber(); + $existing = [ + $this->attr('Name', 'John', $subscriber), + $this->attr('Age', '30', $subscriber), + ]; + + $this->repository->expects(self::once()) + ->method('getForSubscriber') + ->with($subscriber) + ->willReturn($existing); + + $newData = [ + 'name' => 'John', + 'AGE' => '30', + ]; + + $changeSet = $this->provider->getAttributeChangeSet($subscriber, $newData); + + self::assertInstanceOf(ChangeSetDto::class, $changeSet); + self::assertFalse($changeSet->hasChanges()); + self::assertSame([], $changeSet->getChanges()); + } + + public function testAddedAttributeAppearsWithNullOldValue(): void + { + $subscriber = new Subscriber(); + $existing = [ + $this->attr('Name', 'John', $subscriber), + ]; + + $this->repository->method('getForSubscriber')->willReturn($existing); + + $newData = [ + 'name' => 'John', + 'city' => 'NY', + ]; + + $changeSet = $this->provider->getAttributeChangeSet($subscriber, $newData); + + self::assertTrue($changeSet->hasField('city')); + self::assertSame([null, 'NY'], $changeSet->getFieldChange('city')); + + self::assertSame(['city' => [null, 'NY']], $changeSet->getChanges()); + } + + public function testRemovedAttributeAppearsWithNullNewValue(): void + { + $subscriber = new Subscriber(); + $existing = [ + $this->attr('Country', 'US', $subscriber), + ]; + + $this->repository->method('getForSubscriber')->willReturn($existing); + + $changeSet = $this->provider->getAttributeChangeSet($subscriber, []); + + self::assertTrue($changeSet->hasField('country')); + self::assertSame(['US', null], $changeSet->getFieldChange('country')); + self::assertSame(['country' => ['US', null]], $changeSet->getChanges()); + } + + public function testChangedAttributeShowsOldAndNewValues(): void + { + $subscriber = new Subscriber(); + $existing = [ + $this->attr('Phone', '123', $subscriber), + ]; + + $this->repository->method('getForSubscriber')->willReturn($existing); + + $newData = [ + 'phone' => '456', + ]; + + $changeSet = $this->provider->getAttributeChangeSet($subscriber, $newData); + + self::assertSame(['123', '456'], $changeSet->getFieldChange('phone')); + self::assertSame(['phone' => ['123', '456']], $changeSet->getChanges()); + } + + public function testIgnoredAttributesAreExcluded(): void + { + $subscriber = new Subscriber(); + $existing = [ + $this->attr('Password', 'old', $subscriber), + $this->attr('Modified', 'yesterday', $subscriber), + $this->attr('Nickname', 'Bob', $subscriber), + ]; + + $this->repository->method('getForSubscriber')->willReturn($existing); + + $newData = [ + 'password' => 'new', + 'MODIFIED' => null, + 'nickname' => 'Bobby', + ]; + + $changeSet = $this->provider->getAttributeChangeSet($subscriber, $newData); + + self::assertFalse($changeSet->hasField('password')); + self::assertFalse($changeSet->hasField('modified')); + self::assertTrue($changeSet->hasField('nickname')); + self::assertSame(['Bob', 'Bobby'], $changeSet->getFieldChange('nickname')); + self::assertSame(['nickname' => ['Bob', 'Bobby']], $changeSet->getChanges()); + } + + public function testCaseInsensitiveKeyComparisonAndResultLowercasing(): void + { + $subscriber = new Subscriber(); + $existing = [ + $this->attr('FirstName', 'Ann', $subscriber), + ]; + + $this->repository->method('getForSubscriber')->willReturn($existing); + + $newData = [ + 'firstname' => 'Anna', + ]; + + $changeSet = $this->provider->getAttributeChangeSet($subscriber, $newData); + + self::assertTrue($changeSet->hasField('firstname')); + self::assertSame(['Ann', 'Anna'], $changeSet->getFieldChange('firstname')); + self::assertSame(['firstname' => ['Ann', 'Anna']], $changeSet->getChanges()); + } + + private function attr(string $name, ?string $value, Subscriber $subscriber): SubscriberAttributeValue + { + $def = (new SubscriberAttributeDefinition())->setName($name); + $attr = new SubscriberAttributeValue($def, $subscriber); + $attr->setValue($value); + return $attr; + } +} diff --git a/tests/Unit/Domain/Subscription/Service/SubscriberCsvImporterTest.php b/tests/Unit/Domain/Subscription/Service/SubscriberCsvImporterTest.php index 1453cfa2..424a7e0d 100644 --- a/tests/Unit/Domain/Subscription/Service/SubscriberCsvImporterTest.php +++ b/tests/Unit/Domain/Subscription/Service/SubscriberCsvImporterTest.php @@ -5,6 +5,7 @@ namespace PhpList\Core\Tests\Unit\Domain\Subscription\Service; use Doctrine\ORM\EntityManagerInterface; +use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto; use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Dto\SubscriberImportOptions; use PhpList\Core\Domain\Subscription\Model\Subscriber; @@ -13,6 +14,7 @@ use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; use PhpList\Core\Domain\Subscription\Service\CsvImporter; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberAttributeManager; +use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberManager; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriptionManager; use PhpList\Core\Domain\Subscription\Service\SubscriberCsvImporter; @@ -47,10 +49,10 @@ protected function setUp(): void subscriptionManager: $subscriptionManagerMock, subscriberRepository: $this->subscriberRepositoryMock, csvImporter: $this->csvImporterMock, - attrDefinitionRepository: $this->attributeDefinitionRepositoryMock, entityManager: $entityManager, translator: new Translator('en'), messageBus: $this->createMock(MessageBusInterface::class), + subscriberHistoryManager: $this->createMock(SubscriberHistoryManager::class), ); } @@ -120,10 +122,10 @@ public function testImportFromCsvCreatesNewSubscribers(): void $this->attributeManagerMock ->expects($this->exactly(2)) - ->method('createOrUpdate') + ->method('processAttributes') ->withConsecutive( - [$subscriber1, $attributeDefinition, 'John'], - [$subscriber2, $attributeDefinition, 'Jane'] + [$subscriber1], + [$subscriber2] ); $options = new SubscriberImportOptions(); @@ -156,8 +158,6 @@ public function testImportFromCsvUpdatesExistingSubscribers(): void ->with('existing@example.com') ->willReturn($existingSubscriber); - $updatedSubscriber = $this->createMock(Subscriber::class); - $importDto = new ImportSubscriberDto( email: 'existing@example.com', confirmed: true, @@ -180,7 +180,15 @@ public function testImportFromCsvUpdatesExistingSubscribers(): void ->expects($this->once()) ->method('updateFromImport') ->with($existingSubscriber, $importDto) - ->willReturn($updatedSubscriber); + ->willReturn(new ChangeSetDto( + [ + 'extra_data' => [null, 'Updated data'], + 'confirmed' => [false, true], + 'html_email' => [false, true], + 'blacklisted' => [true, false], + 'disabled' => [true, false], + ] + )); $options = new SubscriberImportOptions(updateExisting: true); $result = $this->subject->importFromCsv($uploadedFile, $options);