From a6502a4c61d4b16097f2e1a14144a813f2c1d0e2 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Wed, 1 Apr 2026 21:53:06 +0400 Subject: [PATCH 1/2] Fix: mailing --- .../Analytics/Service/LinkTrackService.php | 19 ++++++++++++------- .../Messaging/Service/MessageDataLoader.php | 2 +- .../Service/MessagePrecacheService.php | 2 +- .../Service/LinkTrackServiceTest.php | 13 +++++++++++-- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/Domain/Analytics/Service/LinkTrackService.php b/src/Domain/Analytics/Service/LinkTrackService.php index 60230dd3..62e364cc 100644 --- a/src/Domain/Analytics/Service/LinkTrackService.php +++ b/src/Domain/Analytics/Service/LinkTrackService.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Domain\Analytics\Service; +use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Core\ParameterProvider; use PhpList\Core\Domain\Analytics\Exception\MissingMessageIdException; use PhpList\Core\Domain\Analytics\Model\LinkTrack; @@ -12,13 +13,11 @@ class LinkTrackService { - private LinkTrackRepository $linkTrackRepository; - private ParameterProvider $paramProvider; - - public function __construct(LinkTrackRepository $linkTrackRepository, ParameterProvider $paramProvider) - { - $this->linkTrackRepository = $linkTrackRepository; - $this->paramProvider = $paramProvider; + public function __construct( + private readonly LinkTrackRepository $linkTrackRepository, + private readonly ParameterProvider $paramProvider, + private readonly EntityManagerInterface $entityManager + ) { } public function getUrlById(int $id): ?string @@ -58,6 +57,7 @@ public function extractAndSaveLinks(MessagePrecacheDto $content, int $userId, ?i $links = array_unique($links); $savedLinks = []; + $newLinksPersisted = false; foreach ($links as $url) { $existingLinkTrack = $this->linkTrackRepository->findByUrlUserIdAndMessageId($url, $userId, $messageId); @@ -71,9 +71,14 @@ public function extractAndSaveLinks(MessagePrecacheDto $content, int $userId, ?i $linkTrack->setUrl($url); $this->linkTrackRepository->persist($linkTrack); + $newLinksPersisted = true; $savedLinks[] = $linkTrack; } + if ($newLinksPersisted) { + $this->entityManager->flush(); + } + return $savedLinks; } diff --git a/src/Domain/Messaging/Service/MessageDataLoader.php b/src/Domain/Messaging/Service/MessageDataLoader.php index c3577bec..7643a831 100644 --- a/src/Domain/Messaging/Service/MessageDataLoader.php +++ b/src/Domain/Messaging/Service/MessageDataLoader.php @@ -148,7 +148,7 @@ private function normaliseScheduleFields(array &$messageData): void private function populateTargetLists(array &$messageData, Message $message): void { foreach ($message->getListMessages() as $listMessage) { - $messageData['targetlist'][$listMessage->getListId()] = 1; + $messageData['targetlist'][$listMessage->getList()->getId()] = 1; } } diff --git a/src/Domain/Messaging/Service/MessagePrecacheService.php b/src/Domain/Messaging/Service/MessagePrecacheService.php index dfab25ec..de808ec0 100644 --- a/src/Domain/Messaging/Service/MessagePrecacheService.php +++ b/src/Domain/Messaging/Service/MessagePrecacheService.php @@ -71,7 +71,7 @@ public function precacheMessage(Message $campaign, array $loadedMessageData, ?bo return false; } - $messagePrecacheDto->googleTrack = $loadedMessageData['google_track']; + $messagePrecacheDto->googleTrack = (bool) $loadedMessageData['google_track']; $this->applyBasicReplacements($messagePrecacheDto, $loadedMessageData); $this->populateAdminAttributes($messagePrecacheDto, $campaign); diff --git a/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php b/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php index a2a53892..668ac2b8 100644 --- a/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php +++ b/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Tests\Unit\Domain\Analytics\Service; +use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Core\ParameterProvider; use PhpList\Core\Domain\Analytics\Exception\MissingMessageIdException; use PhpList\Core\Domain\Analytics\Model\LinkTrack; @@ -29,7 +30,11 @@ protected function setUp(): void ->with('click_track', false) ->willReturn(true); - $this->subject = new LinkTrackService($this->linkTrackRepository, $paramProvider); + $this->subject = new LinkTrackService( + $this->linkTrackRepository, + $paramProvider, + $this->createMock(EntityManagerInterface::class) + ); } public function testExtractAndSaveLinksWithNoLinks(): void @@ -209,7 +214,11 @@ public function testIsExtractAndSaveLinksApplicableWhenClickTrackIsFalse(): void ->with('click_track', false) ->willReturn(false); - $subject = new LinkTrackService($this->linkTrackRepository, $paramProvider); + $subject = new LinkTrackService( + $this->linkTrackRepository, + $paramProvider, + $this->createMock(EntityManagerInterface::class) + ); self::assertFalse($subject->isExtractAndSaveLinksApplicable()); } From ef1fabc5247325c8ed2da76d10f88224267221ea Mon Sep 17 00:00:00 2001 From: Tatevik Date: Wed, 1 Apr 2026 22:26:54 +0400 Subject: [PATCH 2/2] Add: copy as draft message functionality --- .../Analytics/Service/LinkTrackService.php | 9 ---- .../Identity/Service/PermissionChecker.php | 5 +-- .../CampaignProcessorMessageHandler.php | 3 +- src/Domain/Messaging/Model/Message.php | 6 +++ .../Messaging/Model/Message/MessageStatus.php | 5 ++- .../Service/Manager/MessageManager.php | 33 +++++++++++--- .../Service/LinkTrackServiceTest.php | 13 +----- .../Service/Manager/MessageManagerTest.php | 43 +++++++++++++++++++ .../Service/MessageDataLoaderTest.php | 18 ++++---- 9 files changed, 92 insertions(+), 43 deletions(-) diff --git a/src/Domain/Analytics/Service/LinkTrackService.php b/src/Domain/Analytics/Service/LinkTrackService.php index 62e364cc..0060eeb2 100644 --- a/src/Domain/Analytics/Service/LinkTrackService.php +++ b/src/Domain/Analytics/Service/LinkTrackService.php @@ -4,7 +4,6 @@ namespace PhpList\Core\Domain\Analytics\Service; -use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Core\ParameterProvider; use PhpList\Core\Domain\Analytics\Exception\MissingMessageIdException; use PhpList\Core\Domain\Analytics\Model\LinkTrack; @@ -16,7 +15,6 @@ class LinkTrackService public function __construct( private readonly LinkTrackRepository $linkTrackRepository, private readonly ParameterProvider $paramProvider, - private readonly EntityManagerInterface $entityManager ) { } @@ -57,8 +55,6 @@ public function extractAndSaveLinks(MessagePrecacheDto $content, int $userId, ?i $links = array_unique($links); $savedLinks = []; - $newLinksPersisted = false; - foreach ($links as $url) { $existingLinkTrack = $this->linkTrackRepository->findByUrlUserIdAndMessageId($url, $userId, $messageId); if ($existingLinkTrack !== null) { @@ -71,14 +67,9 @@ public function extractAndSaveLinks(MessagePrecacheDto $content, int $userId, ?i $linkTrack->setUrl($url); $this->linkTrackRepository->persist($linkTrack); - $newLinksPersisted = true; $savedLinks[] = $linkTrack; } - if ($newLinksPersisted) { - $this->entityManager->flush(); - } - return $savedLinks; } diff --git a/src/Domain/Identity/Service/PermissionChecker.php b/src/Domain/Identity/Service/PermissionChecker.php index 9986ff59..cce4d514 100644 --- a/src/Domain/Identity/Service/PermissionChecker.php +++ b/src/Domain/Identity/Service/PermissionChecker.php @@ -8,6 +8,7 @@ use PhpList\Core\Domain\Common\Model\Interfaces\OwnableInterface; use PhpList\Core\Domain\Identity\Model\Administrator; use PhpList\Core\Domain\Identity\Model\PrivilegeFlag; +use PhpList\Core\Domain\Messaging\Model\ListMessage; use PhpList\Core\Domain\Messaging\Model\Message; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberList; @@ -70,9 +71,7 @@ private function resolveRelatedEntity(DomainModel $resource, string $relatedClas } if ($resource instanceof Message && $relatedClass === SubscriberList::class) { - // todo: check which one is correct - // return $resource->getListMessages()->map(fn(ListMessage $lm) => $lm->getList())->toArray(); - return $resource->getListMessages()->map(fn($lm) => $lm->getSubscriberList())->toArray(); + return $resource->getListMessages()->map(fn(ListMessage $lm) => $lm->getList())->toArray(); } return []; diff --git a/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php b/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php index f576d1df..86429ffc 100644 --- a/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php +++ b/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php @@ -199,12 +199,13 @@ private function handleEmailSending( UserMessage $userMessage, MessagePrecacheDto $precachedContent, ): void { - // todo: check at which point link tracking should be applied (maybe after constructing ful text?) + // todo: check at which point link tracking should be applied (maybe after constructing full text?) $processed = $this->messagePreparator->processMessageLinks( campaignId: $campaign->getId(), cachedMessageDto: $precachedContent, subscriber: $subscriber ); + $this->entityManager->flush(); try { $result = $this->campaignEmailBuilder->buildCampaignEmail( diff --git a/src/Domain/Messaging/Model/Message.php b/src/Domain/Messaging/Model/Message.php index c70ff01c..4d5f4e8f 100644 --- a/src/Domain/Messaging/Model/Message.php +++ b/src/Domain/Messaging/Model/Message.php @@ -61,6 +61,9 @@ class Message implements DomainModel, Identity, ModificationDate, OwnableInterfa #[ORM\JoinColumn(name: 'template', referencedColumnName: 'id', nullable: true, onDelete: 'SET NULL')] private ?Template $template = null; + /** + * @var Collection + */ #[ORM\OneToMany(targetEntity: ListMessage::class, mappedBy: 'message')] private Collection $listMessages; @@ -190,6 +193,9 @@ public function setOptions(MessageOptions $options): self return $this; } + /** + * @return Collection + */ public function getListMessages(): Collection { return $this->listMessages; diff --git a/src/Domain/Messaging/Model/Message/MessageStatus.php b/src/Domain/Messaging/Model/Message/MessageStatus.php index 90b7f987..8727dd52 100644 --- a/src/Domain/Messaging/Model/Message/MessageStatus.php +++ b/src/Domain/Messaging/Model/Message/MessageStatus.php @@ -22,8 +22,9 @@ enum MessageStatus: string public function allowedTransitions(): array { return match ($this) { - self::Draft, self::Suspended => [self::Submitted], - self::Submitted => [self::Prepared, self::InProcess], + self::Draft => [self::Prepared, self::Submitted], + self::Suspended => [self::Submitted], + self::Submitted => [self::Prepared, self::InProcess, self::Suspended], self::Prepared => [self::InProcess], self::InProcess => [self::Sent, self::Suspended, self::Submitted], self::Requeued => [self::InProcess, self::Suspended], diff --git a/src/Domain/Messaging/Service/Manager/MessageManager.php b/src/Domain/Messaging/Service/Manager/MessageManager.php index c11d34d9..f2432626 100644 --- a/src/Domain/Messaging/Service/Manager/MessageManager.php +++ b/src/Domain/Messaging/Service/Manager/MessageManager.php @@ -8,18 +8,16 @@ use PhpList\Core\Domain\Messaging\Model\Dto\MessageContext; use PhpList\Core\Domain\Messaging\Model\Dto\MessageDtoInterface; use PhpList\Core\Domain\Messaging\Model\Message; +use PhpList\Core\Domain\Messaging\Model\Message\MessageMetadata; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; use PhpList\Core\Domain\Messaging\Service\Builder\MessageBuilder; class MessageManager { - private MessageRepository $messageRepository; - private MessageBuilder $messageBuilder; - - public function __construct(MessageRepository $messageRepository, MessageBuilder $messageBuilder) - { - $this->messageRepository = $messageRepository; - $this->messageBuilder = $messageBuilder; + public function __construct( + private readonly MessageRepository $messageRepository, + private readonly MessageBuilder $messageBuilder, + ) { } public function createMessage(MessageDtoInterface $createMessageDto, Administrator $authUser): Message @@ -31,6 +29,27 @@ public function createMessage(MessageDtoInterface $createMessageDto, Administrat return $message; } + public function copyAsDraftMessage(Message $message, Administrator $authUser): Message + { + $newMessage = new Message( + format: new Message\MessageFormat( + htmlFormatted: $message->getFormat()->isHtmlFormatted(), + sendFormat: $message->getFormat()->getSendFormat() + ), + schedule: clone $message->getSchedule(), + metadata: new MessageMetadata(status: Message\MessageStatus::Draft), + content: clone $message->getContent(), + options: clone $message->getOptions(), + owner: $authUser, + template: $message->getTemplate(), + ); + $newMessage->setUuid(bin2hex(random_bytes(18))); + + $this->messageRepository->persist($newMessage); + + return $newMessage; + } + public function updateMessage( MessageDtoInterface $updateMessageDto, Message $message, diff --git a/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php b/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php index 668ac2b8..a2a53892 100644 --- a/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php +++ b/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php @@ -4,7 +4,6 @@ namespace PhpList\Core\Tests\Unit\Domain\Analytics\Service; -use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Core\ParameterProvider; use PhpList\Core\Domain\Analytics\Exception\MissingMessageIdException; use PhpList\Core\Domain\Analytics\Model\LinkTrack; @@ -30,11 +29,7 @@ protected function setUp(): void ->with('click_track', false) ->willReturn(true); - $this->subject = new LinkTrackService( - $this->linkTrackRepository, - $paramProvider, - $this->createMock(EntityManagerInterface::class) - ); + $this->subject = new LinkTrackService($this->linkTrackRepository, $paramProvider); } public function testExtractAndSaveLinksWithNoLinks(): void @@ -214,11 +209,7 @@ public function testIsExtractAndSaveLinksApplicableWhenClickTrackIsFalse(): void ->with('click_track', false) ->willReturn(false); - $subject = new LinkTrackService( - $this->linkTrackRepository, - $paramProvider, - $this->createMock(EntityManagerInterface::class) - ); + $subject = new LinkTrackService($this->linkTrackRepository, $paramProvider); self::assertFalse($subject->isExtractAndSaveLinksApplicable()); } diff --git a/tests/Unit/Domain/Messaging/Service/Manager/MessageManagerTest.php b/tests/Unit/Domain/Messaging/Service/Manager/MessageManagerTest.php index cbbbcb68..dac13243 100644 --- a/tests/Unit/Domain/Messaging/Service/Manager/MessageManagerTest.php +++ b/tests/Unit/Domain/Messaging/Service/Manager/MessageManagerTest.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Tests\Unit\Domain\Messaging\Service\Manager; +use DateTime; use PhpList\Core\Domain\Identity\Model\Administrator; use PhpList\Core\Domain\Messaging\Model\Dto\CreateMessageDto; use PhpList\Core\Domain\Messaging\Model\Dto\Message\MessageContentDto; @@ -13,6 +14,10 @@ use PhpList\Core\Domain\Messaging\Model\Dto\Message\MessageScheduleDto; use PhpList\Core\Domain\Messaging\Model\Dto\UpdateMessageDto; use PhpList\Core\Domain\Messaging\Model\Message; +use PhpList\Core\Domain\Messaging\Model\Message\MessageFormat; +use PhpList\Core\Domain\Messaging\Model\Message\MessageMetadata; +use PhpList\Core\Domain\Messaging\Model\Message\MessageOptions; +use PhpList\Core\Domain\Messaging\Model\Message\MessageSchedule; use PhpList\Core\Domain\Messaging\Model\Message\MessageContent; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; use PhpList\Core\Domain\Messaging\Service\Builder\MessageBuilder; @@ -21,6 +26,44 @@ class MessageManagerTest extends TestCase { + public function testCopyAsDraftMessagePersistsClonedDraftMessage(): void + { + $messageRepository = $this->createMock(MessageRepository::class); + $messageBuilder = $this->createMock(MessageBuilder::class); + $manager = new MessageManager($messageRepository, $messageBuilder); + + $message = new Message( + format: new MessageFormat(true, 'html'), + schedule: new MessageSchedule( + repeatInterval: 0, + repeatUntil: null, + requeueInterval: 0, + requeueUntil: null, + embargo: new DateTime('2025-04-17T09:00:00+00:00') + ), + metadata: new MessageMetadata(Message\MessageStatus::Submitted), + content: new MessageContent('Subject', 'Full text', 'Short text', 'Footer'), + options: new MessageOptions('from@example.com', 'to@example.com', 'reply@example.com', 'all-users'), + owner: null + ); + + $messageRepository->expects($this->once()) + ->method('persist') + ->with($this->callback(function (Message $persistedMessage) use ($message): bool { + $this->assertNotSame($message, $persistedMessage); + $this->assertSame(Message\MessageStatus::Draft, $persistedMessage->getMetadata()->getStatus()); + $this->assertTrue($persistedMessage->getFormat()->isHtmlFormatted()); + $this->assertSame('html', $persistedMessage->getFormat()->getSendFormat()); + + return true; + })); + + $result = $manager->copyAsDraftMessage($message, $this->createMock(Administrator::class)); + + $this->assertSame(Message\MessageStatus::Draft, $result->getMetadata()->getStatus()); + $this->assertNotSame($message, $result); + } + public function testCreateMessageReturnsPersistedMessage(): void { $messageRepository = $this->createMock(MessageRepository::class); diff --git a/tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php b/tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php index c6174a44..5999ce50 100644 --- a/tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php +++ b/tests/Unit/Domain/Messaging/Service/MessageDataLoaderTest.php @@ -7,11 +7,13 @@ use Doctrine\Common\Collections\ArrayCollection; use PhpList\Core\Domain\Configuration\Model\ConfigOption; use PhpList\Core\Domain\Configuration\Service\Provider\ConfigProvider; +use PhpList\Core\Domain\Messaging\Model\ListMessage; use PhpList\Core\Domain\Messaging\Model\Message; use PhpList\Core\Domain\Messaging\Model\MessageData; use PhpList\Core\Domain\Messaging\Repository\MessageDataRepository; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; use PhpList\Core\Domain\Messaging\Service\MessageDataLoader; +use PhpList\Core\Domain\Subscription\Model\SubscriberList; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -61,6 +63,11 @@ public function testLoadsMessageDataMergesAndParses(): void $md2 = (new MessageData())->setId($messageId)->setName('criteria_match')->setData('any'); $md3 = (new MessageData())->setId($messageId)->setName('embargo')->setData('string'); + $listMock = $this->createMock(SubscriberList::class); + $listMock->method('getId')->willReturn(42); + $listMessageMock = $this->createMock(ListMessage::class); + $listMessageMock->method('getList')->willReturn($listMock); + $this->messageDataRepository ->method('getForMessage') ->with($messageId) @@ -69,16 +76,7 @@ public function testLoadsMessageDataMergesAndParses(): void // Use a Message mock instead of an anonymous stub $message = $this->createMock(Message::class); $message->method('getId')->willReturn($messageId); - $message->method('getListMessages')->willReturn( - new ArrayCollection([ - new class { - public function getListId(): int - { - return 42; - } - }, - ]) - ); + $message->method('getListMessages')->willReturn(new ArrayCollection([$listMessageMock])); $loader = new MessageDataLoader( configProvider: $this->config,