From 5658321d4d70382e7b88165b55ee274cefdf570a Mon Sep 17 00:00:00 2001 From: TatevikGr Date: Mon, 9 Feb 2026 10:42:07 +0400 Subject: [PATCH 1/6] Add forward email endpoint (#164) New Features Email forwarding functionality for campaigns/messages with recipient validation, sender details, and personal notes support Campaign message processing enhancements Refactor Service configuration simplified and consolidated Updated dependency management Co-authored-by: Tatevik --- composer.json | 5 +- config/services/normalizers.yml | 100 +-------------- config/services/services.yml | 15 ++- .../AdminAttributeDefinitionController.php | 2 +- .../AdminAttributeValueController.php | 2 +- .../Controller/AdministratorController.php | 2 +- .../Controller/PasswordResetController.php | 2 +- src/Identity/Controller/SessionController.php | 2 +- .../Controller/CampaignController.php | 9 +- .../Controller/EmailForwardController.php | 121 ++++++++++++++++++ .../Request/ForwardMessageRequest.php | 92 +++++++++++++ .../Serializer/ForwardingResultNormalizer.php | 71 ++++++++++ .../Serializer/MessageNormalizer.php | 7 - src/Messaging/Service/CampaignService.php | 18 ++- .../Validator/Constraint/MaxForwardCount.php | 13 ++ .../Constraint/MaxForwardCountValidator.php | 48 +++++++ .../Constraint/MaxPersonalNoteSize.php | 13 ++ .../MaxPersonalNoteSizeValidator.php | 41 ++++++ .../Common/AbstractTestController.php | 4 + .../Messaging/Fixtures/MessageFixture.php | 9 +- .../Serializer/MessageNormalizerTest.php | 15 ++- 21 files changed, 458 insertions(+), 133 deletions(-) create mode 100644 src/Messaging/Controller/EmailForwardController.php create mode 100644 src/Messaging/Request/ForwardMessageRequest.php create mode 100644 src/Messaging/Serializer/ForwardingResultNormalizer.php create mode 100644 src/Messaging/Validator/Constraint/MaxForwardCount.php create mode 100644 src/Messaging/Validator/Constraint/MaxForwardCountValidator.php create mode 100644 src/Messaging/Validator/Constraint/MaxPersonalNoteSize.php create mode 100644 src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php diff --git a/composer.json b/composer.json index c4c880f8..3dc5a3db 100644 --- a/composer.json +++ b/composer.json @@ -42,13 +42,14 @@ }, "require": { "php": "^8.1", - "phplist/core": "dev-main", + "phplist/core": "dev-ref/campaign-processing", "friendsofsymfony/rest-bundle": "*", "symfony/test-pack": "^1.0", "symfony/process": "^6.4", "zircote/swagger-php": "^4.11", "ext-dom": "*", - "tatevikgr/rss-feed": "dev-main as 0.1.0" + "tatevikgr/rss-feed": "dev-main as 0.1.0", + "psr/simple-cache": "^3.0" }, "require-dev": { "phpunit/phpunit": "^10.0", diff --git a/config/services/normalizers.yml b/config/services/normalizers.yml index 2179f6ad..b6176579 100644 --- a/config/services/normalizers.yml +++ b/config/services/normalizers.yml @@ -4,6 +4,10 @@ services: autoconfigure: true public: false + _instanceof: + Symfony\Component\Serializer\Normalizer\NormalizerInterface: + tags: [ 'serializer.normalizer' ] + Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter: ~ Symfony\Component\Serializer\Normalizer\ObjectNormalizer: @@ -11,97 +15,5 @@ services: $classMetadataFactory: '@?serializer.mapping.class_metadata_factory' $nameConverter: '@Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter' - PhpList\RestBundle\Subscription\Serializer\SubscriberNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscriberOnlyNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Identity\Serializer\AdministratorTokenNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscriberListNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscriberHistoryNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscriptionNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Messaging\Serializer\MessageNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Messaging\Serializer\TemplateImageNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Messaging\Serializer\TemplateNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Messaging\Serializer\ListMessageNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Identity\Serializer\AdministratorNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Identity\Serializer\AdminAttributeDefinitionNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Identity\Serializer\AdminAttributeValueNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\AttributeDefinitionNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscriberAttributeValueNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Common\Serializer\CursorPaginationNormalizer: - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscribersExportRequestNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Statistics\Serializer\CampaignStatisticsNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Statistics\Serializer\ViewOpensStatisticsNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Statistics\Serializer\TopDomainsNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Statistics\Serializer\TopLocalPartsNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\UserBlacklistNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Subscription\Serializer\SubscribePageNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true - - PhpList\RestBundle\Messaging\Serializer\BounceRegexNormalizer: - tags: [ 'serializer.normalizer' ] - autowire: true + PhpList\RestBundle\: + resource: '../../src/*/Serializer/*' diff --git a/config/services/services.yml b/config/services/services.yml index f087aa6e..17127575 100644 --- a/config/services/services.yml +++ b/config/services/services.yml @@ -1,8 +1,19 @@ services: - PhpList\RestBundle\Subscription\Service\SubscriberService: + PhpList\RestBundle\Subscription\Service\SubscriberHistoryService: autowire: true autoconfigure: true - PhpList\RestBundle\Subscription\Service\SubscriberHistoryService: + PhpList\Core\Domain\Messaging\Service\ForwardingGuard: + autowire: true + autoconfigure: true + public: false + + PhpList\Core\Domain\Messaging\Service\ForwardDeliveryService: + autowire: true + autoconfigure: true + public: false + + PhpList\Core\Domain\Messaging\Service\ForwardContentService: autowire: true autoconfigure: true + public: false diff --git a/src/Identity/Controller/AdminAttributeDefinitionController.php b/src/Identity/Controller/AdminAttributeDefinitionController.php index 4bf38643..41ba5a91 100644 --- a/src/Identity/Controller/AdminAttributeDefinitionController.php +++ b/src/Identity/Controller/AdminAttributeDefinitionController.php @@ -7,7 +7,7 @@ use Doctrine\ORM\EntityManagerInterface; use OpenApi\Attributes as OA; use PhpList\Core\Domain\Identity\Model\AdminAttributeDefinition; -use PhpList\Core\Domain\Identity\Service\AdminAttributeDefinitionManager; +use PhpList\Core\Domain\Identity\Service\Manager\AdminAttributeDefinitionManager; use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Service\Provider\PaginatedDataProvider; diff --git a/src/Identity/Controller/AdminAttributeValueController.php b/src/Identity/Controller/AdminAttributeValueController.php index b8e1b5a4..5cbea428 100644 --- a/src/Identity/Controller/AdminAttributeValueController.php +++ b/src/Identity/Controller/AdminAttributeValueController.php @@ -10,7 +10,7 @@ use PhpList\Core\Domain\Identity\Model\Administrator; use PhpList\Core\Domain\Identity\Model\AdminAttributeDefinition; use PhpList\Core\Domain\Identity\Model\AdminAttributeValue; -use PhpList\Core\Domain\Identity\Service\AdminAttributeManager; +use PhpList\Core\Domain\Identity\Service\Manager\AdminAttributeManager; use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Service\Provider\PaginatedDataProvider; diff --git a/src/Identity/Controller/AdministratorController.php b/src/Identity/Controller/AdministratorController.php index c1f173f1..83138038 100644 --- a/src/Identity/Controller/AdministratorController.php +++ b/src/Identity/Controller/AdministratorController.php @@ -7,7 +7,7 @@ use Doctrine\ORM\EntityManagerInterface; use OpenApi\Attributes as OA; use PhpList\Core\Domain\Identity\Model\Administrator; -use PhpList\Core\Domain\Identity\Service\AdministratorManager; +use PhpList\Core\Domain\Identity\Service\Manager\AdministratorManager; use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Service\Provider\PaginatedDataProvider; diff --git a/src/Identity/Controller/PasswordResetController.php b/src/Identity/Controller/PasswordResetController.php index a3527b07..b020814d 100644 --- a/src/Identity/Controller/PasswordResetController.php +++ b/src/Identity/Controller/PasswordResetController.php @@ -6,7 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use OpenApi\Attributes as OA; -use PhpList\Core\Domain\Identity\Service\PasswordManager; +use PhpList\Core\Domain\Identity\Service\Manager\PasswordManager; use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Validator\RequestValidator; diff --git a/src/Identity/Controller/SessionController.php b/src/Identity/Controller/SessionController.php index 66b49ce9..1924072e 100644 --- a/src/Identity/Controller/SessionController.php +++ b/src/Identity/Controller/SessionController.php @@ -7,7 +7,7 @@ use Doctrine\ORM\EntityManagerInterface; use OpenApi\Attributes as OA; use PhpList\Core\Domain\Identity\Model\AdministratorToken; -use PhpList\Core\Domain\Identity\Service\SessionManager; +use PhpList\Core\Domain\Identity\Service\Manager\SessionManager; use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Validator\RequestValidator; diff --git a/src/Messaging/Controller/CampaignController.php b/src/Messaging/Controller/CampaignController.php index a047c9cd..dc2ac689 100644 --- a/src/Messaging/Controller/CampaignController.php +++ b/src/Messaging/Controller/CampaignController.php @@ -29,19 +29,14 @@ #[Route('/campaigns', name: 'campaign_')] class CampaignController extends BaseController { - private CampaignService $campaignService; - private MessageBusInterface $messageBus; - public function __construct( Authentication $authentication, RequestValidator $validator, - CampaignService $campaignService, - MessageBusInterface $messageBus, + private readonly CampaignService $campaignService, + private readonly MessageBusInterface $messageBus, private readonly EntityManagerInterface $entityManager, ) { parent::__construct($authentication, $validator); - $this->campaignService = $campaignService; - $this->messageBus = $messageBus; } #[Route('', name: 'get_list', methods: ['GET'])] diff --git a/src/Messaging/Controller/EmailForwardController.php b/src/Messaging/Controller/EmailForwardController.php new file mode 100644 index 00000000..3917e8ca --- /dev/null +++ b/src/Messaging/Controller/EmailForwardController.php @@ -0,0 +1,121 @@ + + */ +#[Route('/email-forward', name: 'email_forward_')] +class EmailForwardController extends BaseController +{ + public function __construct( + Authentication $authentication, + RequestValidator $validator, + private readonly EntityManagerInterface $entityManager, + private readonly MessageForwardService $messageForwardService, + private readonly ForwardingResultNormalizer $forwardingResultNormalizer, + ) { + parent::__construct($authentication, $validator); + } + + #[Route('/{messageId}', name: 'forward', requirements: ['messageId' => '\\d+'], methods: ['POST'])] + #[OA\Post( + path: '/api/v2/campaigns/{messageId}/forward', + description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' . + 'Queues forwarding of a campaign/message to provided recipient emails.', + summary: 'Forward a message to recipients.', + requestBody: new OA\RequestBody( + description: 'Forwarding payload', + required: true, + content: new OA\JsonContent(ref: '#/components/schemas/ForwardMessageRequest') + ), + tags: ['campaigns'], + parameters: [ + new OA\Parameter( + name: 'php-auth-pw', + description: 'Session key obtained from login', + in: 'header', + required: true, + schema: new OA\Schema(type: 'string') + ), + new OA\Parameter( + name: 'messageId', + description: 'message ID', + in: 'path', + required: true, + schema: new OA\Schema(type: 'string') + ) + ], + responses: [ + new OA\Response( + response: 202, + description: 'Accepted', + content: new OA\JsonContent(ref: '#/components/schemas/ForwardResult') + ), + new OA\Response( + response: 403, + description: 'Failure', + content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse') + ), + new OA\Response( + response: 404, + description: 'Failure', + content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse') + ), + new OA\Response( + response: 422, + description: 'Failure', + content: new OA\JsonContent(ref: '#/components/schemas/ValidationErrorResponse') + ) + ] + )] + public function forwardMessage( + Request $request, + #[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null + ): JsonResponse { + if ($message === null) { + throw $this->createNotFoundException('Campaign not found.'); + } + + /** @var ForwardMessageRequest $forwardRequest */ + $forwardRequest = $this->validator->validate($request, ForwardMessageRequest::class); + + $result = $this->messageForwardService->forward( + messageForwardDto: new MessageForwardDto( + emails: $forwardRequest->recipients, + uid: $forwardRequest->uid, + fromName: $forwardRequest->fromName, + fromEmail: $forwardRequest->fromEmail, + note: $forwardRequest->note, + ), + campaign: $message, + ); + + $this->entityManager->flush(); + + return $this->json( + $this->forwardingResultNormalizer->normalize($result), + Response::HTTP_ACCEPTED + ); + } +} diff --git a/src/Messaging/Request/ForwardMessageRequest.php b/src/Messaging/Request/ForwardMessageRequest.php new file mode 100644 index 00000000..e32f75e3 --- /dev/null +++ b/src/Messaging/Request/ForwardMessageRequest.php @@ -0,0 +1,92 @@ + [ + new Assert\Email([]), + new Assert\Length(max: 255), + ], + ])] + public array $recipients = []; + + #[Assert\Length(max: 255)] + public ?string $uid = null; + + #[MaxPersonalNoteSize] + public ?string $note = null; + + #[Assert\Length(max: 255)] + public ?string $fromName = null; + + #[Assert\Email] + #[Assert\Length(max: 255)] + public ?string $fromEmail = null; + + public function getDto(): array + { + return [ + 'recipients' => $this->recipients, + 'uid' => $this->uid, + 'note' => $this->note, + 'fromName' => $this->fromName, + 'fromEmail' => $this->fromEmail, + ]; + } +} diff --git a/src/Messaging/Serializer/ForwardingResultNormalizer.php b/src/Messaging/Serializer/ForwardingResultNormalizer.php new file mode 100644 index 00000000..61b85cf7 --- /dev/null +++ b/src/Messaging/Serializer/ForwardingResultNormalizer.php @@ -0,0 +1,71 @@ + $recipient->email, + 'status' => $recipient->status, + 'reason' => $recipient->reason, + ]; + }, $object->recipients); + + return [ + 'total_requested' => $object->totalRequested, + 'total_sent' => $object->totalSent, + 'total_failed' => $object->totalFailed, + 'total_already_sent' => $object->totalAlreadySent, + 'recipients' => $recipients, + ]; + } + + /** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function supportsNormalization($data, string $format = null): bool + { + return $data instanceof ForwardingResult; + } +} diff --git a/src/Messaging/Serializer/MessageNormalizer.php b/src/Messaging/Serializer/MessageNormalizer.php index 5c1f3e60..7a2ead3b 100644 --- a/src/Messaging/Serializer/MessageNormalizer.php +++ b/src/Messaging/Serializer/MessageNormalizer.php @@ -33,12 +33,6 @@ properties: [ new OA\Property(property: 'html_formated', type: 'boolean'), new OA\Property(property: 'send_format', type: 'string', example: 'text', nullable: true), - new OA\Property( - property: 'format_options', - type: 'array', - items: new OA\Items(type: 'string'), - example: ['as_html', 'as_text'], - ), ], type: 'object' ), @@ -112,7 +106,6 @@ public function normalize($object, string $format = null, array $context = []): 'message_format' => [ 'html_formated' => $object->getFormat()->isHtmlFormatted(), 'send_format' => $object->getFormat()->getSendFormat(), - 'format_options' => $object->getFormat()->getFormatOptions() ], 'message_metadata' => [ 'status' => $object->getMetadata()->getStatus()->value, diff --git a/src/Messaging/Service/CampaignService.php b/src/Messaging/Service/CampaignService.php index 4bc9e3c6..90ec7d25 100644 --- a/src/Messaging/Service/CampaignService.php +++ b/src/Messaging/Service/CampaignService.php @@ -32,7 +32,12 @@ public function getMessages(Request $request, Administrator $administrator): arr { $filter = (new MessageFilter())->setOwner($administrator); - return $this->paginatedProvider->getPaginatedList($request, $this->normalizer, Message::class, $filter); + return $this->paginatedProvider->getPaginatedList( + request: $request, + normalizer: $this->normalizer, + className: Message::class, + filter: $filter + ); } public function getMessage(Message $message = null): array @@ -50,7 +55,10 @@ public function createMessage(CreateMessageRequest $createMessageRequest, Admini throw new AccessDeniedHttpException('You are not allowed to create campaigns.'); } - $data = $this->messageManager->createMessage($createMessageRequest->getDto(), $administrator); + $data = $this->messageManager->createMessage( + createMessageDto: $createMessageRequest->getDto(), + authUser: $administrator + ); return $this->normalizer->normalize($data); } @@ -69,9 +77,9 @@ public function updateMessage( } $data = $this->messageManager->updateMessage( - $updateMessageRequest->getDto(), - $message, - $administrator + updateMessageDto: $updateMessageRequest->getDto(), + message: $message, + authUser: $administrator ); return $this->normalizer->normalize($data); diff --git a/src/Messaging/Validator/Constraint/MaxForwardCount.php b/src/Messaging/Validator/Constraint/MaxForwardCount.php new file mode 100644 index 00000000..9c6e41db --- /dev/null +++ b/src/Messaging/Validator/Constraint/MaxForwardCount.php @@ -0,0 +1,13 @@ + $this->maxForward) { + $this->context->buildViolation($constraint->message) + ->setParameter('{{ limit }}', $this->maxForward) + ->addViolation(); + } + } +} diff --git a/src/Messaging/Validator/Constraint/MaxPersonalNoteSize.php b/src/Messaging/Validator/Constraint/MaxPersonalNoteSize.php new file mode 100644 index 00000000..1b0c519b --- /dev/null +++ b/src/Messaging/Validator/Constraint/MaxPersonalNoteSize.php @@ -0,0 +1,13 @@ +maxSize === null || $this->maxSize < 0) { + return; + } + + if (!is_string($value)) { + return; + } + + $sizeLimit = $this->maxSize * 2; + $length = function_exists('mb_strlen') ? mb_strlen($value) : strlen($value); + + if ($length > $sizeLimit) { + $this->context->buildViolation($constraint->message) + ->setParameter('{{ limit }}', (string) $this->maxSize) + ->addViolation(); + } + } +} diff --git a/tests/Integration/Common/AbstractTestController.php b/tests/Integration/Common/AbstractTestController.php index e431abbd..e8d96b85 100644 --- a/tests/Integration/Common/AbstractTestController.php +++ b/tests/Integration/Common/AbstractTestController.php @@ -60,6 +60,10 @@ protected function jsonRequest( ): Crawler { $serverWithContentType = $server; $serverWithContentType['CONTENT_TYPE'] = 'application/json'; + // Ensure the server knows the client expects JSON back as well + $serverWithContentType['HTTP_ACCEPT'] = 'application/json'; + // Mark as AJAX-style request to help some handlers choose JSON rendering + $serverWithContentType['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'; return self::getClient()->request( $method, diff --git a/tests/Integration/Messaging/Fixtures/MessageFixture.php b/tests/Integration/Messaging/Fixtures/MessageFixture.php index 42b33b51..f281546c 100644 --- a/tests/Integration/Messaging/Fixtures/MessageFixture.php +++ b/tests/Integration/Messaging/Fixtures/MessageFixture.php @@ -50,13 +50,8 @@ public function load(ObjectManager $manager): void $template = $templateRepository->find($row['template']); $format = new MessageFormat( - (bool)$row['htmlformatted'], - $row['sendformat'], - array_keys(array_filter([ - MessageFormat::FORMAT_TEXT => $row['astext'], - MessageFormat::FORMAT_HTML => $row['ashtml'], - MessageFormat::FORMAT_PDF => $row['aspdf'], - ])) + htmlFormatted: (bool)$row['htmlformatted'], + sendFormat: $row['sendformat'], ); $schedule = new MessageSchedule( diff --git a/tests/Unit/Messaging/Serializer/MessageNormalizerTest.php b/tests/Unit/Messaging/Serializer/MessageNormalizerTest.php index d90ef900..8d6856d9 100644 --- a/tests/Unit/Messaging/Serializer/MessageNormalizerTest.php +++ b/tests/Unit/Messaging/Serializer/MessageNormalizerTest.php @@ -19,7 +19,11 @@ class MessageNormalizerTest extends TestCase public function __construct(string $name) { parent::__construct($name); - $this->normalizer = new MessageNormalizer(new TemplateNormalizer(new TemplateImageNormalizer())); + $this->normalizer = new MessageNormalizer( + new TemplateNormalizer( + new TemplateImageNormalizer() + ) + ); } public function testSupportsNormalization(): void @@ -41,7 +45,6 @@ public function testNormalizeReturnsExpectedArray(): void $content = new Message\MessageContent('Subject', 'Text', 'TextMsg', 'Footer'); $format = new Message\MessageFormat(true, 'html'); - $format->setFormatOptions(['text', 'html']); $entered = new DateTime('2025-01-01T10:00:00+00:00'); $sent = new DateTime('2025-01-02T10:00:00+00:00'); @@ -61,7 +64,12 @@ public function testNormalizeReturnsExpectedArray(): void new DateTime('2025-01-01T00:00:00+00:00') ); - $options = new Message\MessageOptions('from@example.com', 'to@example.com', 'reply@example.com', 'group'); + $options = new Message\MessageOptions( + 'from@example.com', + 'to@example.com', + 'reply@example.com', + 'group' + ); $message = $this->createMock(Message::class); $message->method('getId')->willReturn(1); @@ -79,7 +87,6 @@ public function testNormalizeReturnsExpectedArray(): void $this->assertSame('uuid-123', $result['unique_id']); $this->assertSame('Test Template', $result['template']['title']); $this->assertSame('Subject', $result['message_content']['subject']); - $this->assertSame(['text', 'html'], $result['message_format']['format_options']); $this->assertSame(Message\MessageStatus::Draft->value, $result['message_metadata']['status']); $this->assertSame('from@example.com', $result['message_options']['from_field']); } From 1419d83dc441b3cf771a1ef7ebb299f0cc98e23c Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 11:11:52 +0400 Subject: [PATCH 2/6] After review 0 --- src/Messaging/Controller/EmailForwardController.php | 2 +- .../Validator/Constraint/MaxForwardCountValidator.php | 4 ++-- .../Validator/Constraint/MaxPersonalNoteSizeValidator.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Messaging/Controller/EmailForwardController.php b/src/Messaging/Controller/EmailForwardController.php index 3917e8ca..4c81c1c1 100644 --- a/src/Messaging/Controller/EmailForwardController.php +++ b/src/Messaging/Controller/EmailForwardController.php @@ -40,7 +40,7 @@ public function __construct( #[Route('/{messageId}', name: 'forward', requirements: ['messageId' => '\\d+'], methods: ['POST'])] #[OA\Post( - path: '/api/v2/campaigns/{messageId}/forward', + path: '/api/v2/email-forward/{messageId}', description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' . 'Queues forwarding of a campaign/message to provided recipient emails.', summary: 'Forward a message to recipients.', diff --git a/src/Messaging/Validator/Constraint/MaxForwardCountValidator.php b/src/Messaging/Validator/Constraint/MaxForwardCountValidator.php index 091e6ce8..052a79e4 100644 --- a/src/Messaging/Validator/Constraint/MaxForwardCountValidator.php +++ b/src/Messaging/Validator/Constraint/MaxForwardCountValidator.php @@ -11,7 +11,7 @@ class MaxForwardCountValidator extends ConstraintValidator { public function __construct( - #[Autowire('%phplist.forward_email_count%')] private readonly string $maxForward + #[Autowire('%phplist.forward_email_count%')] private readonly int $maxForward ) { } @@ -41,7 +41,7 @@ public function validate($value, Constraint $constraint): void if ($uniqueCount > $this->maxForward) { $this->context->buildViolation($constraint->message) - ->setParameter('{{ limit }}', $this->maxForward) + ->setParameter('{{ limit }}', (string) $this->maxForward) ->addViolation(); } } diff --git a/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php b/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php index d53b0ad8..77a514e4 100644 --- a/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php +++ b/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php @@ -34,7 +34,7 @@ public function validate($value, Constraint $constraint): void if ($length > $sizeLimit) { $this->context->buildViolation($constraint->message) - ->setParameter('{{ limit }}', (string) $this->maxSize) + ->setParameter('{{ limit }}', (string) $sizeLimit) ->addViolation(); } } From cd8139f44e69e29ad72e11eaaa8a1415b5a5ec70 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 11:22:46 +0400 Subject: [PATCH 3/6] Add tests --- config/services/validators.yml | 10 ++ .../EventListener/ExceptionListener.php | 9 ++ .../Controller/EmailForwardController.php | 2 +- .../Request/ForwardMessageRequest.php | 18 ++-- .../MaxPersonalNoteSizeValidator.php | 2 +- .../Controller/EmailForwardControllerTest.php | 81 ++++++++++++++++ .../Request/ForwardMessageRequestTest.php | 51 ++++++++++ .../ForwardingResultNormalizerTest.php | 92 +++++++++++++++++++ .../MaxForwardCountValidatorTest.php | 78 ++++++++++++++++ .../MaxPersonalNoteSizeValidatorTest.php | 89 ++++++++++++++++++ 10 files changed, 424 insertions(+), 8 deletions(-) create mode 100644 tests/Integration/Messaging/Controller/EmailForwardControllerTest.php create mode 100644 tests/Unit/Messaging/Request/ForwardMessageRequestTest.php create mode 100644 tests/Unit/Messaging/Serializer/ForwardingResultNormalizerTest.php create mode 100644 tests/Unit/Messaging/Validator/Constraint/MaxForwardCountValidatorTest.php create mode 100644 tests/Unit/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidatorTest.php diff --git a/config/services/validators.yml b/config/services/validators.yml index e7302414..a4fa508e 100644 --- a/config/services/validators.yml +++ b/config/services/validators.yml @@ -41,3 +41,13 @@ services: autowire: true autoconfigure: true + PhpList\RestBundle\Messaging\Validator\Constraint\MaxForwardCountValidator: + autowire: true + autoconfigure: true + tags: [ 'validator.constraint_validator' ] + + PhpList\RestBundle\Messaging\Validator\Constraint\MaxPersonalNoteSizeValidator: + autowire: true + autoconfigure: true + tags: [ 'validator.constraint_validator' ] + diff --git a/src/Common/EventListener/ExceptionListener.php b/src/Common/EventListener/ExceptionListener.php index b898defa..d66b852b 100644 --- a/src/Common/EventListener/ExceptionListener.php +++ b/src/Common/EventListener/ExceptionListener.php @@ -6,6 +6,7 @@ use Exception; use PhpList\Core\Domain\Identity\Exception\AdminAttributeCreationException; +use PhpList\Core\Domain\Messaging\Exception\MessageNotReceivedException; use PhpList\Core\Domain\Subscription\Exception\AttributeDefinitionCreationException; use PhpList\Core\Domain\Subscription\Exception\SubscriptionCreationException; use Symfony\Component\HttpFoundation\JsonResponse; @@ -17,6 +18,9 @@ class ExceptionListener { + /** + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + */ public function onKernelException(ExceptionEvent $event): void { $exception = $event->getThrowable(); @@ -58,6 +62,11 @@ public function onKernelException(ExceptionEvent $event): void 'message' => $exception->getMessage(), ], 403); $event->setResponse($response); + } elseif ($exception instanceof MessageNotReceivedException) { + $response = new JsonResponse([ + 'message' => $exception->getMessage(), + ], 422); + $event->setResponse($response); } elseif ($exception instanceof Exception) { $response = new JsonResponse([ 'message' => $exception->getMessage(), diff --git a/src/Messaging/Controller/EmailForwardController.php b/src/Messaging/Controller/EmailForwardController.php index 4c81c1c1..c7e1210d 100644 --- a/src/Messaging/Controller/EmailForwardController.php +++ b/src/Messaging/Controller/EmailForwardController.php @@ -96,7 +96,7 @@ public function forwardMessage( if ($message === null) { throw $this->createNotFoundException('Campaign not found.'); } - + // todo: per-subscriber forwarding limits /** @var ForwardMessageRequest $forwardRequest */ $forwardRequest = $this->validator->validate($request, ForwardMessageRequest::class); diff --git a/src/Messaging/Request/ForwardMessageRequest.php b/src/Messaging/Request/ForwardMessageRequest.php index e32f75e3..4ce3f4a5 100644 --- a/src/Messaging/Request/ForwardMessageRequest.php +++ b/src/Messaging/Request/ForwardMessageRequest.php @@ -24,7 +24,7 @@ property: 'uid', type: 'string', example: 'fwd-123e4567-e89b-12d3-a456-426614174000', - nullable: true + nullable: false ), new OA\Property( property: 'note', @@ -36,14 +36,14 @@ property: 'from_name', type: 'string', example: 'Alice', - nullable: true + nullable: false ), new OA\Property( property: 'from_email', type: 'string', format: 'email', example: 'alice@example.com', - nullable: true + nullable: false ), ], type: 'object' @@ -66,18 +66,24 @@ class ForwardMessageRequest implements RequestInterface ])] public array $recipients = []; + #[Assert\NotNull] + #[Assert\NotBlank] #[Assert\Length(max: 255)] - public ?string $uid = null; + public string $uid; #[MaxPersonalNoteSize] public ?string $note = null; + #[Assert\NotNull] + #[Assert\NotBlank] #[Assert\Length(max: 255)] - public ?string $fromName = null; + public string $fromName; + #[Assert\NotNull] + #[Assert\NotBlank] #[Assert\Email] #[Assert\Length(max: 255)] - public ?string $fromEmail = null; + public string $fromEmail; public function getDto(): array { diff --git a/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php b/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php index 77a514e4..18344ac3 100644 --- a/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php +++ b/src/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidator.php @@ -21,7 +21,7 @@ public function validate($value, Constraint $constraint): void return; } - if ($value === null || $value === '' || $this->maxSize === null || $this->maxSize < 0) { + if ($value === null || $value === '' || $this->maxSize === null || $this->maxSize <= 0) { return; } diff --git a/tests/Integration/Messaging/Controller/EmailForwardControllerTest.php b/tests/Integration/Messaging/Controller/EmailForwardControllerTest.php new file mode 100644 index 00000000..e3f1ac51 --- /dev/null +++ b/tests/Integration/Messaging/Controller/EmailForwardControllerTest.php @@ -0,0 +1,81 @@ +get(EmailForwardController::class) + ); + } + + public function testForwardWithInvalidPayloadReturnsUnprocessableEntity(): void + { + $this->loadFixtures([AdministratorFixture::class, AdministratorTokenFixture::class, MessageFixture::class]); + + // Missing required 'recipients' field should trigger 422 from RequestValidator + $this->authenticatedJsonRequest('POST', '/api/v2/email-forward/1', content: json_encode([ ])); + + $this->assertHttpUnprocessableEntity(); + } + + public function testForwardWithValidDataButNotReceivedEmail(): void + { + $this->loadFixtures([AdministratorFixture::class, AdministratorTokenFixture::class, MessageFixture::class]); + + $payload = json_encode([ + 'recipients' => ['friend1@example.com'], + 'uid' => 'fwd-123', + 'note' => null, + 'from_name' => 'Alice', + 'from_email' => 'alice@example.com', + ]); + + $this->authenticatedJsonRequest('POST', '/api/v2/email-forward/1', content: $payload); + + $response = self::getClient()->getResponse(); + $this->assertHttpUnprocessableEntity(); + self::assertStringContainsString('application/json', (string)$response->headers); + + $data = $this->getDecodedJsonResponseContent(); + self::assertIsArray($data); + self::assertArrayHasKey('message', $data); + self::assertStringContainsString('Cannot forward: user has not received this message', $data['message']); + } + + public function testForwardWithInvalidEmailInRecipientsReturnsUnprocessableEntity(): void + { + $this->loadFixtures([AdministratorFixture::class, AdministratorTokenFixture::class, MessageFixture::class]); + + $payload = json_encode([ + 'recipients' => ['not-an-email'], + ]); + + $this->authenticatedJsonRequest('POST', '/api/v2/email-forward/1', content: $payload); + + $this->assertHttpUnprocessableEntity(); + } + + public function testForwardWithInvalidIdReturnsNotFound(): void + { + $this->loadFixtures([AdministratorFixture::class, AdministratorTokenFixture::class]); + + $this->authenticatedJsonRequest('POST', '/api/v2/email-forward/9999', content: json_encode([ + 'recipients' => ['friend@example.com'], + ])); + + $this->assertHttpNotFound(); + } +} diff --git a/tests/Unit/Messaging/Request/ForwardMessageRequestTest.php b/tests/Unit/Messaging/Request/ForwardMessageRequestTest.php new file mode 100644 index 00000000..aeb66366 --- /dev/null +++ b/tests/Unit/Messaging/Request/ForwardMessageRequestTest.php @@ -0,0 +1,51 @@ +recipients = ['friend1@example.com', 'friend2@example.com']; + $request->uid = 'fwd-123e4567-e89b-12d3-a456-426614174000'; + $request->note = 'Thought you might like this.'; + $request->fromName = 'Alice'; + $request->fromEmail = 'alice@example.com'; + + $dto = $request->getDto(); + + $this->assertIsArray($dto); + $this->assertSame(['friend1@example.com', 'friend2@example.com'], $dto['recipients']); + $this->assertSame('fwd-123e4567-e89b-12d3-a456-426614174000', $dto['uid']); + $this->assertSame('Thought you might like this.', $dto['note']); + $this->assertSame('Alice', $dto['fromName']); + $this->assertSame('alice@example.com', $dto['fromEmail']); + } + + public function testGetDtoHandlesNullables(): void + { + $request = new ForwardMessageRequest(); + + $request->recipients = ['friend@example.com']; + $request->uid = 'fwd-uid-1'; + $request->note = null; + $request->fromName = 'Bob'; + $request->fromEmail = 'bob@example.com'; + + $dto = $request->getDto(); + + $this->assertIsArray($dto); + $this->assertSame(['friend@example.com'], $dto['recipients']); + $this->assertSame('fwd-uid-1', $dto['uid']); + $this->assertNull($dto['note']); + $this->assertSame('Bob', $dto['fromName']); + $this->assertSame('bob@example.com', $dto['fromEmail']); + } +} diff --git a/tests/Unit/Messaging/Serializer/ForwardingResultNormalizerTest.php b/tests/Unit/Messaging/Serializer/ForwardingResultNormalizerTest.php new file mode 100644 index 00000000..45e65527 --- /dev/null +++ b/tests/Unit/Messaging/Serializer/ForwardingResultNormalizerTest.php @@ -0,0 +1,92 @@ +normalizer = new ForwardingResultNormalizer(); + } + + public function testSupportsNormalizationReturnsTrueForForwardingResult(): void + { + $result = new ForwardingResult( + totalRequested: 0, + totalSent: 0, + totalFailed: 0, + totalAlreadySent: 0, + recipients: [], + ); + + $this->assertTrue($this->normalizer->supportsNormalization($result)); + } + + public function testSupportsNormalizationReturnsFalseForOtherObjects(): void + { + $this->assertFalse($this->normalizer->supportsNormalization(new \stdClass())); + } + + public function testNormalizeMapsAllTopLevelCounts(): void + { + $result = new ForwardingResult( + totalRequested: 5, + totalSent: 3, + totalFailed: 1, + totalAlreadySent: 1, + recipients: [], + ); + + $data = $this->normalizer->normalize($result); + + $this->assertIsArray($data); + $this->assertSame(5, $data['total_requested']); + $this->assertSame(3, $data['total_sent']); + $this->assertSame(1, $data['total_failed']); + $this->assertSame(1, $data['total_already_sent']); + $this->assertSame([], $data['recipients']); + } + + public function testNormalizeMapsRecipientsWithNullableReason(): void + { + $r1 = new ForwardingRecipientResult('a@example.com', 'sent', null); + $r2 = new ForwardingRecipientResult('b@example.com', 'failed', 'precache_failed'); + + $result = new ForwardingResult( + totalRequested: 2, + totalSent: 1, + totalFailed: 1, + totalAlreadySent: 0, + recipients: [$r1, $r2], + ); + + $data = $this->normalizer->normalize($result); + + $this->assertCount(2, $data['recipients']); + $this->assertSame([ + 'email' => 'a@example.com', + 'status' => 'sent', + 'reason' => null, + ], $data['recipients'][0]); + $this->assertSame([ + 'email' => 'b@example.com', + 'status' => 'failed', + 'reason' => 'precache_failed', + ], $data['recipients'][1]); + } + + public function testNormalizeReturnsEmptyArrayForNonForwardingResult(): void + { + $data = $this->normalizer->normalize(new \stdClass()); + $this->assertSame([], $data); + } +} diff --git a/tests/Unit/Messaging/Validator/Constraint/MaxForwardCountValidatorTest.php b/tests/Unit/Messaging/Validator/Constraint/MaxForwardCountValidatorTest.php new file mode 100644 index 00000000..3360749d --- /dev/null +++ b/tests/Unit/Messaging/Validator/Constraint/MaxForwardCountValidatorTest.php @@ -0,0 +1,78 @@ +createMock(ExecutionContextInterface::class); + $context->expects($this->never())->method('buildViolation'); + + $validator = new MaxForwardCountValidator(5); + $validator->initialize($context); + + $constraint = new MaxForwardCount(); + $validator->validate('not-an-array', $constraint); + + $this->assertTrue(true); + } + + public function testTriggersViolationWhenUniqueCountExceedsLimit(): void + { + $builder = $this->createMock(ConstraintViolationBuilderInterface::class); + $builder->expects($this->once()) + ->method('setParameter') + ->with('{{ limit }}', '1') + ->willReturnSelf(); + $builder->expects($this->once())->method('addViolation'); + + $context = $this->createMock(ExecutionContextInterface::class); + $context->expects($this->once()) + ->method('buildViolation') + ->with('You can forward to at most {{ limit }} recipients.') + ->willReturn($builder); + + $validator = new MaxForwardCountValidator(1); + $validator->initialize($context); + + $constraint = new MaxForwardCount(); + $emails = [ + ' A@Example.com ', + // duplicate after trim+lower + 'a@example.com', + 'b@example.com', + // ignored empty + '', + // ignored non-string + null, + // ignored non-string + 123, + ]; + + $validator->validate($emails, $constraint); + } + + public function testNoViolationWhenWithinLimit(): void + { + $context = $this->createMock(ExecutionContextInterface::class); + $context->expects($this->never())->method('buildViolation'); + + $validator = new MaxForwardCountValidator(3); + $validator->initialize($context); + + $constraint = new MaxForwardCount(); + $emails = ['a@example.com', 'b@example.com', 'a@example.com']; + + $validator->validate($emails, $constraint); + $this->assertTrue(true); + } +} diff --git a/tests/Unit/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidatorTest.php b/tests/Unit/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidatorTest.php new file mode 100644 index 00000000..dc38dcf1 --- /dev/null +++ b/tests/Unit/Messaging/Validator/Constraint/MaxPersonalNoteSizeValidatorTest.php @@ -0,0 +1,89 @@ +createMock(ExecutionContextInterface::class); + $context->expects($this->never())->method('buildViolation'); + + $validator = new MaxPersonalNoteSizeValidator(10); + $validator->initialize($context); + + $constraint = new MaxPersonalNoteSize(); + $validator->validate(null, $constraint); + $validator->validate('', $constraint); + + $this->assertTrue(true); + } + + public function testSkipsWhenMaxSizeIsNullOrNegative(): void + { + $context = $this->createMock(ExecutionContextInterface::class); + $context->expects($this->never())->method('buildViolation'); + + $validatorNull = new MaxPersonalNoteSizeValidator(null); + $validatorNull->initialize($context); + $validatorNull->validate('anything', new MaxPersonalNoteSize()); + + $validatorNeg = new MaxPersonalNoteSizeValidator(-1); + $validatorNeg->initialize($context); + $validatorNeg->validate('anything', new MaxPersonalNoteSize()); + + $this->assertTrue(true); + } + + public function testNoViolationWhenWithinOrAtLimit(): void + { + $context = $this->createMock(ExecutionContextInterface::class); + $context->expects($this->never())->method('buildViolation'); + // sizeLimit = 20 + $max = 10; + $validator = new MaxPersonalNoteSizeValidator($max); + $validator->initialize($context); + + $constraint = new MaxPersonalNoteSize(); + // exactly at limit + $within = str_repeat('a', 20); + $validator->validate($within, $constraint); + // below limit + $short = str_repeat('b', 5); + $validator->validate($short, $constraint); + + $this->assertTrue(true); + } + + public function testViolationWhenExceedsLimit(): void + { + $builder = $this->createMock(ConstraintViolationBuilderInterface::class); + $builder->expects($this->once()) + ->method('setParameter') + ->with('{{ limit }}', '4') + ->willReturnSelf(); + $builder->expects($this->once())->method('addViolation'); + + $context = $this->createMock(ExecutionContextInterface::class); + $context->expects($this->once()) + ->method('buildViolation') + ->with('Your personal note must be at most {{ limit }} characters long.') + ->willReturn($builder); + // sizeLimit = 4 + $validator = new MaxPersonalNoteSizeValidator(2); + $validator->initialize($context); + + $constraint = new MaxPersonalNoteSize(); + // length 5 > 4 + $value = str_repeat('x', 5); + $validator->validate($value, $constraint); + } +} From 8b1e424859ea47348fe675377d3ec7fbb6c1ee3e Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 13:05:28 +0400 Subject: [PATCH 4/6] Core from dev branch --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 3dc5a3db..0934eda7 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ }, "require": { "php": "^8.1", - "phplist/core": "dev-ref/campaign-processing", + "phplist/core": "dev-dev", "friendsofsymfony/rest-bundle": "*", "symfony/test-pack": "^1.0", "symfony/process": "^6.4", From 1be400678c4c8cbfc2e9a72cd5c5720aa2004f30 Mon Sep 17 00:00:00 2001 From: TatevikGr Date: Wed, 11 Feb 2026 14:45:32 +0400 Subject: [PATCH 5/6] Feat: attachement endpoint (#166) New Features Added an attachment download endpoint that streams files with correct headers (Content-Type, Content-Disposition, Content-Length). Bug Fixes Consolidated and standardized JSON error responses with proper HTTP statuses (404 for missing attachments/subscribers/files, 403 for access denied, others mapped appropriately). Tests Added integration tests and fixtures covering successful streaming downloads, header/body validation, and not-found/error scenarios. --- .../EventListener/ExceptionListener.php | 88 ++++++------ .../Controller/AttachmentController.php | 94 +++++++++++++ .../Controller/AttachmentControllerTest.php | 125 ++++++++++++++++++ .../Messaging/Fixtures/AttachmentFixture.php | 33 +++++ 4 files changed, 291 insertions(+), 49 deletions(-) create mode 100644 src/Messaging/Controller/AttachmentController.php create mode 100644 tests/Integration/Messaging/Controller/AttachmentControllerTest.php create mode 100644 tests/Integration/Messaging/Fixtures/AttachmentFixture.php diff --git a/src/Common/EventListener/ExceptionListener.php b/src/Common/EventListener/ExceptionListener.php index d66b852b..f2724628 100644 --- a/src/Common/EventListener/ExceptionListener.php +++ b/src/Common/EventListener/ExceptionListener.php @@ -6,7 +6,9 @@ use Exception; use PhpList\Core\Domain\Identity\Exception\AdminAttributeCreationException; +use PhpList\Core\Domain\Messaging\Exception\AttachmentFileNotFoundException; use PhpList\Core\Domain\Messaging\Exception\MessageNotReceivedException; +use PhpList\Core\Domain\Messaging\Exception\SubscriberNotFoundException; use PhpList\Core\Domain\Subscription\Exception\AttributeDefinitionCreationException; use PhpList\Core\Domain\Subscription\Exception\SubscriptionCreationException; use Symfony\Component\HttpFoundation\JsonResponse; @@ -18,61 +20,49 @@ class ExceptionListener { - /** - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - */ + private const EXCEPTION_STATUS_MAP = [ + SubscriptionCreationException::class => null, + AttributeDefinitionCreationException::class => null, + AdminAttributeCreationException::class => null, + ValidatorException::class => 400, + AccessDeniedException::class => 403, + AccessDeniedHttpException::class => 403, + AttachmentFileNotFoundException::class => 404, + SubscriberNotFoundException::class => 404, + MessageNotReceivedException::class => 422, + ]; + public function onKernelException(ExceptionEvent $event): void { $exception = $event->getThrowable(); - if ($exception instanceof AccessDeniedHttpException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 403); - - $event->setResponse($response); - } elseif ($exception instanceof HttpExceptionInterface) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); + foreach (self::EXCEPTION_STATUS_MAP as $class => $statusCode) { + if ($exception instanceof $class) { + $status = $statusCode ?? $exception->getStatusCode(); + $event->setResponse( + new JsonResponse([ + 'message' => $exception->getMessage() + ], $status) + ); + return; + } + } - $event->setResponse($response); - } elseif ($exception instanceof SubscriptionCreationException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); - $event->setResponse($response); - } elseif ($exception instanceof AdminAttributeCreationException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); - $event->setResponse($response); - } elseif ($exception instanceof AttributeDefinitionCreationException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); - $event->setResponse($response); - } elseif ($exception instanceof ValidatorException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 400); - $event->setResponse($response); - } elseif ($exception instanceof AccessDeniedException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 403); - $event->setResponse($response); - } elseif ($exception instanceof MessageNotReceivedException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 422); - $event->setResponse($response); - } elseif ($exception instanceof Exception) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 500); + if ($exception instanceof HttpExceptionInterface) { + $event->setResponse( + new JsonResponse([ + 'message' => $exception->getMessage() + ], $exception->getStatusCode()) + ); + return; + } - $event->setResponse($response); + if ($exception instanceof Exception) { + $event->setResponse( + new JsonResponse([ + 'message' => $exception->getMessage() + ], 500) + ); } } } diff --git a/src/Messaging/Controller/AttachmentController.php b/src/Messaging/Controller/AttachmentController.php new file mode 100644 index 00000000..b7e90fe2 --- /dev/null +++ b/src/Messaging/Controller/AttachmentController.php @@ -0,0 +1,94 @@ + '\\d+'], methods: ['GET'])] + #[OA\Get( + path: '/api/v2/attachments/download/{id}', + description: 'Download an attachment by ID. `uid` query parameter is required.', + summary: 'Download attachment', + tags: ['attachments'], + parameters: [ + new OA\Parameter( + name: 'id', + description: 'Attachment ID', + in: 'path', + required: true, + schema: new OA\Schema(type: 'integer') + ), + new OA\Parameter( + name: 'uid', + description: 'Download token (subscriber email or word "forwarded")', + in: 'query', + required: true, + schema: new OA\Schema(type: 'string') + ), + ], + responses: [ + new OA\Response(response: 200, description: 'File stream'), + new OA\Response(response: 403, description: 'Unauthorized'), + new OA\Response(response: 404, description: 'Not found'), + ] + )] + public function download( + #[MapEntity(mapping: ['id' => 'id'])] Attachment $attachment, + #[MapQueryParameter] string $uid + ): StreamedResponse { + $downloadable = $this->attachmentDownloadService->getDownloadable($attachment, $uid); + + $headers = [ + 'Content-Type' => $downloadable->mimeType, + 'Content-Disposition' => HeaderUtils::makeDisposition( + disposition: ResponseHeaderBag::DISPOSITION_ATTACHMENT, + filename: $downloadable->filename + ), + ]; + + if ($downloadable->size !== null) { + $headers['Content-Length'] = (string) $downloadable->size; + } + + return new StreamedResponse( + callback: function () use ($downloadable) { + $stream = $downloadable->content; + + if ($stream->isSeekable()) { + $stream->rewind(); + } + + while (!$stream->eof()) { + echo $stream->read(8192); + flush(); + } + }, + status: 200, + headers: $headers + ); + } +} diff --git a/tests/Integration/Messaging/Controller/AttachmentControllerTest.php b/tests/Integration/Messaging/Controller/AttachmentControllerTest.php new file mode 100644 index 00000000..e00bc7c5 --- /dev/null +++ b/tests/Integration/Messaging/Controller/AttachmentControllerTest.php @@ -0,0 +1,125 @@ +repoPath = (string) self::getContainer()->getParameter('phplist.attachment_repository_path'); + if (!is_dir($this->repoPath)) { + mkdir($this->repoPath, 0777, true); + } + } + + protected function tearDown(): void + { + // Clean up any test file we might have created + $file = $this->repoPath . DIRECTORY_SEPARATOR . AttachmentFixture::FILENAME; + if (is_file($file)) { + unlink($file); + } + + parent::tearDown(); + } + + public function testControllerIsAvailableViaContainer(): void + { + self::assertInstanceOf( + AttachmentController::class, + self::getContainer()->get(AttachmentController::class) + ); + } + + public function testDownloadReturnsFileStreamWithHeaders(): void + { + $this->loadFixtures([AttachmentFixture::class]); + + // Prepare the actual file in the repository path + $content = 'Hello Attachment'; + $file = $this->repoPath . DIRECTORY_SEPARATOR . AttachmentFixture::FILENAME; + file_put_contents($file, $content); + + self::getClient()->request( + 'GET', + sprintf( + '/api/v2/attachments/download/%d?uid=%s', + AttachmentFixture::ATTACHMENT_ID, + Attachment::FORWARD + ) + ); + + $response = self::getClient()->getResponse(); + + // StreamedResponse should be 200 with correct headers + self::assertSame(200, $response->getStatusCode()); + self::assertSame('text/plain; charset=UTF-8', $response->headers->get('Content-Type')); + self::assertStringContainsString( + 'attachment; filename=' . AttachmentFixture::FILENAME, + (string) $response->headers->get('Content-Disposition') + ); + self::assertSame((string) strlen($content), $response->headers->get('Content-Length')); + + $callback = $response->getCallback(); + ob_start(); + $callback(); + $body = ob_get_clean(); + + self::assertSame($content, $body); + } + + public function testDownloadReturnsNotFoundWhenAttachmentEntityMissing(): void + { + self::getClient()->request('GET', '/api/v2/attachments/download/999999?uid=' . Attachment::FORWARD); + $this->assertHttpNotFound(); + } + + public function testDownloadReturnsNotFoundWhenUidEmailNotFound(): void + { + $this->loadFixtures([AttachmentFixture::class]); + + // Do not create the file; the uid validation happens first and should 404 + self::getClient()->request( + 'GET', + sprintf( + '/api/v2/attachments/download/%d?uid=%s', + AttachmentFixture::ATTACHMENT_ID, + 'does-not-exist@example.com' + ) + ); + + $this->assertHttpNotFound(); + } + + public function testDownloadReturnsNotFoundWhenFileMissing(): void + { + $this->loadFixtures([AttachmentFixture::class]); + + // Ensure no file exists + $file = $this->repoPath . DIRECTORY_SEPARATOR . AttachmentFixture::FILENAME; + if (is_file($file)) { + unlink($file); + } + + self::getClient()->request( + 'GET', + sprintf( + '/api/v2/attachments/download/%d?uid=%s', + AttachmentFixture::ATTACHMENT_ID, + Attachment::FORWARD + ) + ); + + $this->assertHttpNotFound(); + } +} diff --git a/tests/Integration/Messaging/Fixtures/AttachmentFixture.php b/tests/Integration/Messaging/Fixtures/AttachmentFixture.php new file mode 100644 index 00000000..7d23319e --- /dev/null +++ b/tests/Integration/Messaging/Fixtures/AttachmentFixture.php @@ -0,0 +1,33 @@ +setSubjectId($attachment, self::ATTACHMENT_ID); + $manager->persist($attachment); + $manager->flush(); + } +} From aacaf11573cb2960c26dc743e23078f57f1ba150 Mon Sep 17 00:00:00 2001 From: TatevikGr Date: Fri, 13 Feb 2026 12:58:13 +0400 Subject: [PATCH 6/6] Feat: track message view (#167) New Features Added 1x1 pixel message-open tracking endpoint to record when messages are viewed. Tests Added integration tests validating pixel responses, headers, behavior with missing/unknown parameters, and included test fixtures linking messages and subscribers. Chores Adjusted CI/config finishing touches related to docstring handling. --- .coderabbit.yaml | 7 +- .../Controller/MessageOpenTrackController.php | 104 ++++++++++++++++++ .../MessageOpenTrackControllerTest.php | 74 +++++++++++++ .../Fixtures/UserMessageFixture.php | 43 ++++++++ 4 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 src/Statistics/Controller/MessageOpenTrackController.php create mode 100644 tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php create mode 100644 tests/Integration/Statistics/Fixtures/UserMessageFixture.php diff --git a/.coderabbit.yaml b/.coderabbit.yaml index edaddc74..0577bc26 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -9,12 +9,11 @@ reviews: high_level_summary_in_walkthrough: false changed_files_summary: false poem: false + finishing_touches: + docstrings: + enabled: false auto_review: enabled: true base_branches: - ".*" drafts: false - -checks: - docstring_coverage: - enabled: false diff --git a/src/Statistics/Controller/MessageOpenTrackController.php b/src/Statistics/Controller/MessageOpenTrackController.php new file mode 100644 index 00000000..ccd3a675 --- /dev/null +++ b/src/Statistics/Controller/MessageOpenTrackController.php @@ -0,0 +1,104 @@ +returnPixelResponse(); + } + + $metadata = [ + 'HTTP_USER_AGENT' => $request->server->get('HTTP_USER_AGENT'), + 'HTTP_REFERER' => $request->server->get('HTTP_REFERER'), + 'client_ip' => $request->getClientIp(), + ]; + + try { + $this->userMessageService->trackUserMessageView($uid, $messageId, $metadata); + $this->entityManager->flush(); + } catch (Throwable $e) { + $this->logger->error( + 'Failed to track user message view', + [ + 'exception' => $e, + 'message_id' => $messageId, + ] + ); + } + + return $this->returnPixelResponse(); + } + + private function returnPixelResponse(): Response + { + return new Response( + content: base64_decode('R0lGODlhAQABAPAAAAAAAAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='), + status: 200, + headers: [ + 'Content-Type' => 'image/gif', + 'Cache-Control' => 'no-store, no-cache, must-revalidate, max-age=0', + 'Pragma' => 'no-cache', + 'Expires' => '0', + ] + ); + } +} diff --git a/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php b/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php new file mode 100644 index 00000000..011aded2 --- /dev/null +++ b/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php @@ -0,0 +1,74 @@ +get(MessageOpenTrackController::class) + ); + } + + public function testOpenGifReturnsTransparentGifWithNoCacheHeaders(): void + { + $this->loadFixtures([ + AdministratorFixture::class, + TemplateFixture::class, + MessageFixture::class, + SubscriberFixture::class, + UserMessageFixture::class, + ]); + + self::getClient()->request( + 'GET', + sprintf('/api/v2/t/open.gif?u=%s&m=%d', self::TEST_UID, self::TEST_MESSAGE_ID) + ); + + $response = self::getClient()->getResponse(); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame('image/gif', $response->headers->get('Content-Type')); + self::assertStringContainsString('no-store', (string) $response->headers->get('Cache-Control')); + self::assertSame('no-cache', $response->headers->get('Pragma')); + self::assertSame('0', $response->headers->get('Expires')); + + $expectedGif = base64_decode('R0lGODlhAQABAPAAAAAAAAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); + self::assertSame($expectedGif, $response->getContent()); + } + + public function testOpenGifReturnsGifEvenIfUidUnknown(): void + { + // No fixtures needed for this; the controller should still return the GIF even if tracking no-ops + self::getClient()->request('GET', '/api/v2/t/open.gif?u=unknown-uid&m=999999'); + + $response = self::getClient()->getResponse(); + self::assertSame(200, $response->getStatusCode()); + self::assertSame('image/gif', $response->headers->get('Content-Type')); + } + + public function testOpenGifMissingParametersReturns200Anyway(): void + { + self::getClient()->request('GET', '/api/v2/t/open.gif'); + $status = self::getClient()->getResponse()->getStatusCode(); + + self::assertSame(200, $status); + } +} diff --git a/tests/Integration/Statistics/Fixtures/UserMessageFixture.php b/tests/Integration/Statistics/Fixtures/UserMessageFixture.php new file mode 100644 index 00000000..73acc93e --- /dev/null +++ b/tests/Integration/Statistics/Fixtures/UserMessageFixture.php @@ -0,0 +1,43 @@ +getRepository(Subscriber::class)->find(self::SUBSCRIBER_ID); + /** @var Message|null $message */ + $message = $manager->getRepository(Message::class)->find(self::MESSAGE_ID); + + // Doctrine may return null here when prerequisite fixtures are not loaded. + // PHPStan infers non-null from PHPDoc in some environments; suppress that false positive. + if ($subscriber === null || $message === null) { + // Pre-requisite fixtures aren't loaded; nothing to do. + return; + } + + $userMessage = new UserMessage($subscriber, $message); + $userMessage->setStatus(UserMessageStatus::Sent); + + $manager->persist($userMessage); + $manager->flush(); + } +}