From 6857dfe857ed5bb568d7ecb00e8ae19de1a99165 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Thu, 12 Feb 2026 12:59:36 +0400 Subject: [PATCH 1/8] MessageOpenTrackController --- composer.json | 2 +- .../Controller/MessageOpenTrackController.php | 83 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 src/Statistics/Controller/MessageOpenTrackController.php diff --git a/composer.json b/composer.json index 0934eda7..1313a572 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ }, "require": { "php": "^8.1", - "phplist/core": "dev-dev", + "phplist/core": "dev-feat/user-message-tracking", "friendsofsymfony/rest-bundle": "*", "symfony/test-pack": "^1.0", "symfony/process": "^6.4", diff --git a/src/Statistics/Controller/MessageOpenTrackController.php b/src/Statistics/Controller/MessageOpenTrackController.php new file mode 100644 index 00000000..fbd8027f --- /dev/null +++ b/src/Statistics/Controller/MessageOpenTrackController.php @@ -0,0 +1,83 @@ + $request->server->get('HTTP_USER_AGENT'), + 'HTTP_REFERER' => $request->server->get('HTTP_REFERER'), + 'client_ip' => $request->getClientIp(), + ]; + + $this->userMessageService->trackUserMessageView($uid, $messageId, $metadata); + + $gifData = base64_decode('R0lGODlhAQABAPAAAAAAAAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); + + return new Response( + content: $gifData, + status: 200, + headers: [ + 'Content-Type' => 'image/gif', + 'Cache-Control' => 'no-store, no-cache, must-revalidate, max-age=0', + 'Pragma' => 'no-cache', + 'Expires' => '0', + ] + ); + } +} From a94a0eb02411ed018794c738ff89df6309b6fa75 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Thu, 12 Feb 2026 13:33:32 +0400 Subject: [PATCH 2/8] Add tests --- .../Controller/MessageOpenTrackController.php | 37 +++++++-- .../MessageOpenTrackControllerTest.php | 75 +++++++++++++++++++ .../Fixtures/UserMessageFixture.php | 41 ++++++++++ 3 files changed, 146 insertions(+), 7 deletions(-) create mode 100644 tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php create mode 100644 tests/Integration/Statistics/Fixtures/UserMessageFixture.php diff --git a/src/Statistics/Controller/MessageOpenTrackController.php b/src/Statistics/Controller/MessageOpenTrackController.php index fbd8027f..5811e759 100644 --- a/src/Statistics/Controller/MessageOpenTrackController.php +++ b/src/Statistics/Controller/MessageOpenTrackController.php @@ -4,15 +4,18 @@ namespace PhpList\RestBundle\Statistics\Controller; +use Doctrine\ORM\EntityManagerInterface; use OpenApi\Attributes as OA; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\Core\Domain\Analytics\Service\UserMessageService; use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Validator\RequestValidator; +use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Attribute\MapQueryParameter; use Symfony\Component\Routing\Attribute\Route; +use Throwable; #[Route('/t', name: 'tracks_')] class MessageOpenTrackController extends BaseController @@ -21,6 +24,8 @@ public function __construct( Authentication $authentication, RequestValidator $validator, private readonly UserMessageService $userMessageService, + private readonly EntityManagerInterface $entityManager, + private readonly LoggerInterface $logger, ) { parent::__construct($authentication, $validator); } @@ -50,27 +55,45 @@ public function __construct( ], responses: [ new OA\Response(response: 200, description: 'Transparent 1x1 GIF'), - new OA\Response(response: 400, description: 'Bad request'), - new OA\Response(response: 500, description: 'Server error'), ] )] public function trackUserMessageView( Request $request, - #[MapQueryParameter(name: 'u')] string $uid, - #[MapQueryParameter(name: 'm')] int $messageId, + #[MapQueryParameter(name: 'u')] ?string $uid = null, + #[MapQueryParameter(name: 'm')] ?int $messageId = null, ): Response { + if (!$uid || !$messageId) { + return $this->returnPixelResponse(); + } + $metadata = [ 'HTTP_USER_AGENT' => $request->server->get('HTTP_USER_AGENT'), 'HTTP_REFERER' => $request->server->get('HTTP_REFERER'), 'client_ip' => $request->getClientIp(), ]; - $this->userMessageService->trackUserMessageView($uid, $messageId, $metadata); + 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, + 'subscriber_uid' => $uid, + 'metadata' => $metadata + ] + ); + } - $gifData = base64_decode('R0lGODlhAQABAPAAAAAAAAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); + return $this->returnPixelResponse(); + } + private function returnPixelResponse(): Response + { return new Response( - content: $gifData, + content: base64_decode('R0lGODlhAQABAPAAAAAAAAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='), status: 200, headers: [ 'Content-Type' => 'image/gif', diff --git a/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php b/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php new file mode 100644 index 00000000..f37f2aef --- /dev/null +++ b/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php @@ -0,0 +1,75 @@ +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(); + + // MapQueryParameter with the required non-nullable argument should yield 400 Bad Request + 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..be027242 --- /dev/null +++ b/tests/Integration/Statistics/Fixtures/UserMessageFixture.php @@ -0,0 +1,41 @@ +getRepository(Subscriber::class)->find(self::SUBSCRIBER_ID); + /** @var Message $message */ + $message = $manager->getRepository(Message::class)->find(self::MESSAGE_ID); + + if ($subscriber === null || $message === null) { + // Pre-requisite fixtures not loaded; nothing to do. + return; + } + + $userMessage = new UserMessage($subscriber, $message); + $userMessage->setStatus(UserMessageStatus::Sent); + + $manager->persist($userMessage); + $manager->flush(); + } +} From 5dfbf76652d250711bd7127cfe8e2d8c7b781751 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Thu, 12 Feb 2026 14:05:43 +0400 Subject: [PATCH 3/8] Fix phpstan --- .../Statistics/Fixtures/UserMessageFixture.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/Integration/Statistics/Fixtures/UserMessageFixture.php b/tests/Integration/Statistics/Fixtures/UserMessageFixture.php index be027242..73acc93e 100644 --- a/tests/Integration/Statistics/Fixtures/UserMessageFixture.php +++ b/tests/Integration/Statistics/Fixtures/UserMessageFixture.php @@ -22,13 +22,15 @@ class UserMessageFixture extends Fixture public function load(ObjectManager $manager): void { - /** @var Subscriber $subscriber */ + /** @var Subscriber|null $subscriber */ $subscriber = $manager->getRepository(Subscriber::class)->find(self::SUBSCRIBER_ID); - /** @var Message $message */ + /** @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 not loaded; nothing to do. + // Pre-requisite fixtures aren't loaded; nothing to do. return; } From 0777fc6738614d063d2b823714f6c4a819e0aada Mon Sep 17 00:00:00 2001 From: Tatevik Date: Fri, 13 Feb 2026 12:25:03 +0400 Subject: [PATCH 4/8] dev branch --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 1313a572..0934eda7 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ }, "require": { "php": "^8.1", - "phplist/core": "dev-feat/user-message-tracking", + "phplist/core": "dev-dev", "friendsofsymfony/rest-bundle": "*", "symfony/test-pack": "^1.0", "symfony/process": "^6.4", From 514057ac59b991ab571c846ea9461a8b2054d65d Mon Sep 17 00:00:00 2001 From: Tatevik Date: Fri, 13 Feb 2026 12:35:42 +0400 Subject: [PATCH 5/8] Fix: coderabbitai config --- .coderabbit.yaml | 7 +++---- .../Controller/MessageOpenTrackControllerTest.php | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) 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/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php b/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php index f37f2aef..011aded2 100644 --- a/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php +++ b/tests/Integration/Statistics/Controller/MessageOpenTrackControllerTest.php @@ -69,7 +69,6 @@ public function testOpenGifMissingParametersReturns200Anyway(): void self::getClient()->request('GET', '/api/v2/t/open.gif'); $status = self::getClient()->getResponse()->getStatusCode(); - // MapQueryParameter with the required non-nullable argument should yield 400 Bad Request self::assertSame(200, $status); } } From cb691919bfbec40d7e02329fe2f3d1a7e25610a3 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Fri, 13 Feb 2026 12:37:48 +0400 Subject: [PATCH 6/8] After review 0 --- src/Statistics/Controller/MessageOpenTrackController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Statistics/Controller/MessageOpenTrackController.php b/src/Statistics/Controller/MessageOpenTrackController.php index 5811e759..b204d036 100644 --- a/src/Statistics/Controller/MessageOpenTrackController.php +++ b/src/Statistics/Controller/MessageOpenTrackController.php @@ -82,7 +82,6 @@ public function trackUserMessageView( 'exception' => $e, 'message_id' => $messageId, 'subscriber_uid' => $uid, - 'metadata' => $metadata ] ); } From 3e84af66a0f40c6387baefcbf298db14215cec85 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Fri, 13 Feb 2026 12:46:13 +0400 Subject: [PATCH 7/8] After review 1 --- src/Statistics/Controller/MessageOpenTrackController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Statistics/Controller/MessageOpenTrackController.php b/src/Statistics/Controller/MessageOpenTrackController.php index b204d036..3e2401b0 100644 --- a/src/Statistics/Controller/MessageOpenTrackController.php +++ b/src/Statistics/Controller/MessageOpenTrackController.php @@ -81,7 +81,6 @@ public function trackUserMessageView( [ 'exception' => $e, 'message_id' => $messageId, - 'subscriber_uid' => $uid, ] ); } From f0f9d95e7f42632d8930483493ead718c943cb10 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Fri, 13 Feb 2026 12:52:29 +0400 Subject: [PATCH 8/8] After review 2 --- src/Statistics/Controller/MessageOpenTrackController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Statistics/Controller/MessageOpenTrackController.php b/src/Statistics/Controller/MessageOpenTrackController.php index 3e2401b0..ccd3a675 100644 --- a/src/Statistics/Controller/MessageOpenTrackController.php +++ b/src/Statistics/Controller/MessageOpenTrackController.php @@ -62,7 +62,7 @@ public function trackUserMessageView( #[MapQueryParameter(name: 'u')] ?string $uid = null, #[MapQueryParameter(name: 'm')] ?int $messageId = null, ): Response { - if (!$uid || !$messageId) { + if ($uid === null || $messageId === null || $messageId <= 0) { return $this->returnPixelResponse(); }