From a84471b65c5e6e13f03aa5700a33c512aaa9185e Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Tue, 17 Aug 2021 12:53:06 +0200 Subject: [PATCH] Save user agent on open [MAILPOET-3735] --- lib/Config/Migrator.php | 1 + lib/Entities/StatisticsOpenEntity.php | 14 ++++++ lib/Router/Endpoints/Track.php | 1 + lib/Statistics/Track/Opens.php | 13 +++++- lib/Statistics/UserAgentsRepository.php | 24 +++++++++++ .../Statistics/Track/OpensTest.php | 43 ++++++++++++++----- 6 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 lib/Statistics/UserAgentsRepository.php diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php index 3b478be485..4037979e8b 100644 --- a/lib/Config/Migrator.php +++ b/lib/Config/Migrator.php @@ -443,6 +443,7 @@ class Migrator { 'newsletter_id int(11) unsigned NOT NULL,', 'subscriber_id int(11) unsigned NOT NULL,', 'queue_id int(11) unsigned NOT NULL,', + 'user_agent_id int(11) unsigned NULL,', 'created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,', 'PRIMARY KEY (id),', 'KEY newsletter_id_subscriber_id (newsletter_id, subscriber_id),', diff --git a/lib/Entities/StatisticsOpenEntity.php b/lib/Entities/StatisticsOpenEntity.php index e2714e469e..32802f7b11 100644 --- a/lib/Entities/StatisticsOpenEntity.php +++ b/lib/Entities/StatisticsOpenEntity.php @@ -37,6 +37,12 @@ class StatisticsOpenEntity { */ private $subscriber; + /** + * @ORM\ManyToOne(targetEntity="MailPoet\Entities\UserAgentEntity") + * @var UserAgentEntity|null + */ + private $userAgent; + public function __construct( NewsletterEntity $newsletter, SendingQueueEntity $queue, @@ -83,4 +89,12 @@ class StatisticsOpenEntity { public function setSubscriber($subscriber) { $this->subscriber = $subscriber; } + + public function getUserAgent(): ?UserAgentEntity { + return $this->userAgent; + } + + public function setUserAgent(?UserAgentEntity $userAgent): void { + $this->userAgent = $userAgent; + } } diff --git a/lib/Router/Endpoints/Track.php b/lib/Router/Endpoints/Track.php index 068f3e4283..201d7aced4 100644 --- a/lib/Router/Endpoints/Track.php +++ b/lib/Router/Endpoints/Track.php @@ -98,6 +98,7 @@ class Track { 'queue' => $data->queue_id, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps ]); } + $data->userAgent = $_SERVER['HTTP_USER_AGENT'] ?? null; return $this->_validateTrackData($data); } diff --git a/lib/Statistics/Track/Opens.php b/lib/Statistics/Track/Opens.php index b5f3944f29..2008baa94c 100644 --- a/lib/Statistics/Track/Opens.php +++ b/lib/Statistics/Track/Opens.php @@ -7,13 +7,21 @@ use MailPoet\Entities\SendingQueueEntity; use MailPoet\Entities\StatisticsOpenEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Statistics\StatisticsOpensRepository; +use MailPoet\Statistics\UserAgentsRepository; class Opens { /** @var StatisticsOpensRepository */ private $statisticsOpensRepository; - public function __construct(StatisticsOpensRepository $statisticsOpensRepository) { + /** @var UserAgentsRepository */ + private $userAgentsRepository; + + public function __construct( + StatisticsOpensRepository $statisticsOpensRepository, + UserAgentsRepository $userAgentsRepository + ) { $this->statisticsOpensRepository = $statisticsOpensRepository; + $this->userAgentsRepository = $userAgentsRepository; } public function track($data, $displayImage = true) { @@ -40,6 +48,9 @@ class Opens { return $this->returnResponse($displayImage); } $statistics = new StatisticsOpenEntity($newsletter, $queue, $subscriber); + if (!empty($data->userAgent)) { + $statistics->setUserAgent($this->userAgentsRepository->findOrCreate($data->userAgent)); + } $this->statisticsOpensRepository->persist($statistics); $this->statisticsOpensRepository->flush(); $this->statisticsOpensRepository->recalculateSubscriberScore($subscriber); diff --git a/lib/Statistics/UserAgentsRepository.php b/lib/Statistics/UserAgentsRepository.php new file mode 100644 index 0000000000..bf1e680db1 --- /dev/null +++ b/lib/Statistics/UserAgentsRepository.php @@ -0,0 +1,24 @@ + + */ +class UserAgentsRepository extends Repository { + protected function getEntityClassName() { + return UserAgentEntity::class; + } + + public function findOrCreate(string $userAgent): UserAgentEntity { + $hash = (string)crc32($userAgent); + $userAgentEntity = $this->findOneBy(['hash' => $hash]); + if ($userAgentEntity) return $userAgentEntity; + $userAgentEntity = new UserAgentEntity($userAgent); + $this->persist($userAgentEntity); + return $userAgentEntity; + } +} diff --git a/tests/integration/Statistics/Track/OpensTest.php b/tests/integration/Statistics/Track/OpensTest.php index be7e5195e4..3472f57aaf 100644 --- a/tests/integration/Statistics/Track/OpensTest.php +++ b/tests/integration/Statistics/Track/OpensTest.php @@ -7,17 +7,13 @@ use Codeception\Stub\Expected; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\SendingQueueEntity; +use MailPoet\Entities\StatisticsOpenEntity; use MailPoet\Entities\SubscriberEntity; -use MailPoet\Models\Newsletter; -use MailPoet\Models\ScheduledTask; -use MailPoet\Models\SendingQueue; use MailPoet\Models\StatisticsOpens; -use MailPoet\Models\Subscriber; use MailPoet\Statistics\StatisticsOpensRepository; use MailPoet\Statistics\Track\Opens; use MailPoet\Subscribers\LinkTokens; use MailPoet\Tasks\Sending as SendingTask; -use MailPoetVendor\Idiorm\ORM; class OpensTest extends \MailPoetTest { public $opens; @@ -26,6 +22,9 @@ class OpensTest extends \MailPoetTest { public $subscriber; public $newsletter; + /** @var StatisticsOpensRepository */ + private $statisticsOpensRepository; + public function _before() { parent::_before(); $this->cleanup(); @@ -68,7 +67,8 @@ class OpensTest extends \MailPoetTest { 'preview' => false, ]; // instantiate class - $this->opens = new Opens($this->diContainer->get(StatisticsOpensRepository::class)); + $this->statisticsOpensRepository = $this->diContainer->get(StatisticsOpensRepository::class); + $this->opens = new Opens($this->statisticsOpensRepository); } public function testItReturnsImageWhenTrackDataIsEmpty() { @@ -119,15 +119,36 @@ class OpensTest extends \MailPoetTest { $opens->track($this->trackData); } + public function testItSavesNewUserAgent() { + $this->trackData->userAgent = 'User agent'; + $opens = Stub::construct($this->opens, [$this->diContainer->get(StatisticsOpensRepository::class)], [ + 'returnResponse' => null, + ], $this); + $opens->track($this->trackData); + $opens = $this->statisticsOpensRepository->findAll(); + expect($opens)->count(1); + $open = $opens[0]; + $userAgent = $open->getUserAgent(); + expect($userAgent)->notNull(); + } + + public function testItSavesOpenWithExistingUserAgent() { + + } + + public function testItOverridesOldUserAgent() { + + } + public function _after() { $this->cleanup(); } public function cleanup() { - ORM::raw_execute('TRUNCATE ' . Newsletter::$_table); - ORM::raw_execute('TRUNCATE ' . Subscriber::$_table); - ORM::raw_execute('TRUNCATE ' . ScheduledTask::$_table); - ORM::raw_execute('TRUNCATE ' . SendingQueue::$_table); - ORM::raw_execute('TRUNCATE ' . StatisticsOpens::$_table); + $this->truncateEntity(NewsletterEntity::class); + $this->truncateEntity(SubscriberEntity::class); + $this->truncateEntity(ScheduledTaskEntity::class); + $this->truncateEntity(SendingQueueEntity::class); + $this->truncateEntity(StatisticsOpenEntity::class); } }