Added check for clicked_at to mail:unsubscribe-inactive-users at …

…all times remp/novydenik#1115
remp2020 · Aug 21, 2023 · 391a47f · 391a47f
1 parent d0df936
commit 391a47f
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 - Removed `HermesException` for missing `mail_sender_id` from `ValidateCrmEmailHandler`. remp/remp#1291
   - This is valid state. It was introduced as fix by commit https://github.com/remp2020/remp/commit/c2d55b5d7d56f7ba29e3977f33785d77b3ca145a
 - Added new `Mailer` segment provider which provides segments of users subscribed to mail types. remp/mnt#114
+- Added check for `clicked_at` to `mail:unsubscribe-inactive-users` at all times. remp/novydenik#1115
 
 ## Archive
 

diff --git a/Mailer/extensions/mailer-module/src/Commands/UnsubscribeInactiveUsersCommand.php b/Mailer/extensions/mailer-module/src/Commands/UnsubscribeInactiveUsersCommand.php
@@ -26,7 +26,7 @@ class UnsubscribeInactiveUsersCommand extends Command
 
     public const COMMAND_NAME = 'mail:unsubscribe-inactive-users';
     private const CRM_SEGMENT_NAME = 'unsubscribe_inactive_users_from_newsletters_list';
-    private const APPLE_BOT_EMAILS = 'apple_bot_emails';
+    public const APPLE_BOT_EMAILS = 'apple_bot_emails';
 
     private array $omitMailTypeCodes = ['system', 'system_optional'];
 
@@ -93,13 +93,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
                 $logCount = count($logs);
                 if (count($logs) >= 5) {
-                    $columnToCheck = 'opened_at';
-                    if ($this->redis()->sismember(self::APPLE_BOT_EMAILS, $userEmail)) {
-                        $columnToCheck = 'clicked_at';
-                    }
+                    $isAppleBotOpenedEmail = $this->redis()->sismember(self::APPLE_BOT_EMAILS, $userEmail);
 
                     foreach ($logs as $log) {
-                        if ($log->{$columnToCheck}) {
+                        if ($isAppleBotOpenedEmail && $log->clicked_at) {
+                            $output->writeln("Skipping, user is active.");
+                            continue 2;
+                        }
+
+                        if (!$isAppleBotOpenedEmail && ($log->opened_at || $log->clicked_at)) {
                             $output->writeln("Skipping, user is active.");
                             continue 2;
                         }

diff --git a/...tensions/mailer-module/src/Tests/Feature/Commands/UnsubscribeInactiveUsersCommandTest.php b/...tensions/mailer-module/src/Tests/Feature/Commands/UnsubscribeInactiveUsersCommandTest.php
@@ -5,6 +5,8 @@
 use Nette\Database\Table\ActiveRow;
 use Nette\Utils\DateTime;
 use Remp\MailerModule\Commands\UnsubscribeInactiveUsersCommand;
+use Remp\MailerModule\Models\RedisClientFactory;
+use Remp\MailerModule\Models\RedisClientTrait;
 use Remp\MailerModule\Models\Segment\Aggregator;
 use Remp\MailerModule\Models\Segment\ISegment;
 use Symfony\Component\Console\Input\StringInput;
@@ -14,6 +16,8 @@
 
 class UnsubscribeInactiveUsersCommandTest extends BaseFeatureTestCase
 {
+    use RedisClientTrait;
+
     protected ISegment $testSegmentProvider;
 
     protected Aggregator $segmentAggregator;
@@ -22,6 +26,8 @@ class UnsubscribeInactiveUsersCommandTest extends BaseFeatureTestCase
 
     protected ActiveRow $mailLayout;
 
+    protected RedisClientFactory $redisClientFactory;
+
     private array $user = [
         'id' => 1,
         'email' => '[email protected]',
@@ -54,6 +60,11 @@ public function setUp(): void
         $this->segmentAggregator = $this->inject(Aggregator::class);
         $this->segmentAggregator->register($this->testSegmentProvider);
 
+        $this->redisClientFactory = $this->inject(RedisClientFactory::class);
+
+        // remove email from `APPLE_BOT_EMAILS` redis key before every test
+        $this->redis()->srem(UnsubscribeInactiveUsersCommand::APPLE_BOT_EMAILS, $this->user['email']);
+
         $this->unsubscribeInactiveUsersCommand = $this->inject(UnsubscribeInactiveUsersCommand::class);
 
         $this->mailLayout = $this->createMailLayout();
@@ -81,10 +92,11 @@ public function dataProvider()
             'TooLittleDeliveredEmails_ShouldNotUnsubscribe' => [
                 'subscribe' => ['system', 'test'],
                 'logs' => [
-                    ['delivered' => '-11 days', 'opened' => null],
-                    ['delivered' => '-10 days', 'opened' => null],
-                    ['delivered' => '-9 days', 'opened' => null],
+                    ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-9 days', 'opened' => null, 'clicked' => null],
                 ],
+                'isAppleBotOpenedEmail' => false,
                 'result' => [
                     'system' => true,
                     'test' => true,
@@ -93,13 +105,14 @@ public function dataProvider()
             'TooManyNotOpenedEmails_ShouldUnsubscribe' => [
                 'subscribe' => ['system', 'test', 'test-omit'],
                 'logs' => [
-                      ['delivered' => '-11 days', 'opened' => null],
-                      ['delivered' => '-10 days', 'opened' => null],
-                      ['delivered' => '-9 days', 'opened' => null],
-                      ['delivered' => '-8 days', 'opened' => null],
-                      ['delivered' => '-7 days', 'opened' => null],
-                      ['delivered' => '-6 days', 'opened' => null],
+                      ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-9 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-8 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-7 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-6 days', 'opened' => null, 'clicked' => null],
                 ],
+                'isAppleBotOpenedEmail' => false,
                 'result' => [
                     'system' => true,
                     'test' => false,
@@ -109,13 +122,14 @@ public function dataProvider()
             'TooManyNotOpenedEmails_WithOmit_ShouldUnsubscribe' => [
                 'subscribe' => ['system', 'test', 'test-omit'],
                 'logs' => [
-                      ['delivered' => '-11 days', 'opened' => null],
-                      ['delivered' => '-10 days', 'opened' => null],
-                      ['delivered' => '-9 days', 'opened' => null],
-                      ['delivered' => '-8 days', 'opened' => null],
-                      ['delivered' => '-7 days', 'opened' => null],
-                      ['delivered' => '-6 days', 'opened' => null],
+                      ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-9 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-8 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-7 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-6 days', 'opened' => null, 'clicked' => null],
                 ],
+                'isAppleBotOpenedEmail' => false,
                 'result' => [
                     'system' => true,
                     'test' => false,
@@ -126,13 +140,14 @@ public function dataProvider()
             'TooManyNotOpenedEmails_DryRun_ShouldNotUnsubscribe' => [
                 'subscribe' => ['system', 'test'],
                 'logs' => [
-                      ['delivered' => '-11 days', 'opened' => null],
-                      ['delivered' => '-10 days', 'opened' => null],
-                      ['delivered' => '-9 days', 'opened' => null],
-                      ['delivered' => '-8 days', 'opened' => null],
-                      ['delivered' => '-7 days', 'opened' => null],
-                      ['delivered' => '-6 days', 'opened' => null],
+                      ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-9 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-8 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-7 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-6 days', 'opened' => null, 'clicked' => null],
                 ],
+                'isAppleBotOpenedEmail' => false,
                 'result' => [
                     'system' => true,
                     'test' => true,
@@ -143,28 +158,78 @@ public function dataProvider()
             'SomeOpenedDeliveries_ShouldNotUnsubscribe' => [
                 'subscribe' => ['system', 'test'],
                 'logs' => [
-                      ['delivered' => '-11 days', 'opened' => null],
-                      ['delivered' => '-10 days', 'opened' => null],
-                      ['delivered' => '-9 days', 'opened' => null],
-                      ['delivered' => '-8 days', 'opened' => '-5 days'],
-                      ['delivered' => '-7 days', 'opened' => null],
-                      ['delivered' => '-6 days', 'opened' => null],
+                      ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-9 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-8 days', 'opened' => '-5 days', 'clicked' => null],
+                      ['delivered' => '-7 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-6 days', 'opened' => null, 'clicked' => null],
+                ],
+                'isAppleBotOpenedEmail' => false,
+                'result' => [
+                    'system' => true,
+                    'test' => true,
+                ],
+            ],
+            'SomeClickedNotOpenedDeliveries_ShouldNotUnsubscribe' => [
+                'subscribe' => ['system', 'test'],
+                'logs' => [
+                      ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-9 days', 'opened' => null, 'clicked' => '-5 days'],
+                      ['delivered' => '-8 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-7 days', 'opened' => null, 'clicked' => null],
+                      ['delivered' => '-6 days', 'opened' => null, 'clicked' => null],
+                ],
+                'isAppleBotOpenedEmail' => false,
+                'result' => [
+                    'system' => true,
+                    'test' => true,
+                ],
+            ],
+            'AppleBotOpenedClickedDeliveries_ShouldNotUnsubscribe' => [
+                'subscribe' => ['system', 'test'],
+                'logs' => [
+                    ['delivered' => '-11 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-10 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-9 days', 'opened' => '-5 days', 'clicked' => '-5 days'],
+                    ['delivered' => '-8 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-7 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-6 days', 'opened' => '-5 days', 'clicked' => null],
                 ],
+                'isAppleBotOpenedEmail' => true,
                 'result' => [
                     'system' => true,
                     'test' => true,
                 ],
             ],
+            'AppleBotOpenedNotClickedDeliveries_ShouldUnsubscribe' => [
+                'subscribe' => ['system', 'test'],
+                'logs' => [
+                    ['delivered' => '-11 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-10 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-9 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-8 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-7 days', 'opened' => '-5 days', 'clicked' => null],
+                    ['delivered' => '-6 days', 'opened' => '-5 days', 'clicked' => null],
+                ],
+                'isAppleBotOpenedEmail' => true,
+                'result' => [
+                    'system' => true,
+                    'test' => false,
+                ],
+            ],
             'NotMatchedDeliveryThresholdWithinSelectedPeriod_ShouldNotUnsubscribe' => [
                 'subscribe' => ['system', 'test', 'test-omit'],
                 'logs' => [
-                    ['delivered' => '-11 days', 'opened' => null],
-                    ['delivered' => '-10 days', 'opened' => null],
-                    ['delivered' => '-9 days', 'opened' => null],
-                    ['delivered' => '-8 days', 'opened' => null],
-                    ['delivered' => '-7 days', 'opened' => null],
-                    ['delivered' => '-6 days', 'opened' => null],
+                    ['delivered' => '-11 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-10 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-9 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-8 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-7 days', 'opened' => null, 'clicked' => null],
+                    ['delivered' => '-6 days', 'opened' => null, 'clicked' => null],
                 ],
+                'isAppleBotOpenedEmail' => false,
                 'result' => [
                     'system' => true,
                     'test' => true,
@@ -180,7 +245,7 @@ public function dataProvider()
     /**
      * @dataProvider dataProvider
      */
-    public function testUnsubscribeInactive(array $subscribe, array $logs, array $result, array $omit = [], bool $dryRun = false, int $days = null)
+    public function testUnsubscribeInactive(array $subscribe, array $logs, bool $isAppleBotOpenedEmail, array $result, array $omit = [], bool $dryRun = false, int $days = null)
     {
         foreach ($subscribe as $mailTypeCode) {
             $this->susbcribeUser($this->user, $mailTypeCode);
@@ -190,7 +255,12 @@ public function testUnsubscribeInactive(array $subscribe, array $logs, array $re
             $template = $this->templatesRepository->findBy('code', 'test');
             $delivered = DateTime::from($log['delivered']);
             $opened = $log['opened'] ? DateTime::from($log['opened']) : null;
-            $this->addMailLog($this->user, $template, $delivered, $opened);
+            $clicked = $log['clicked'] ? DateTime::from($log['clicked']) : null;
+            $this->addMailLog($this->user, $template, $delivered, $opened, $clicked);
+        }
+
+        if ($isAppleBotOpenedEmail) {
+            $this->redis()->sadd(UnsubscribeInactiveUsersCommand::APPLE_BOT_EMAILS, [$this->user['email']]);
         }
 
         $input = '--segment-provider ' . TestSegmentProvider::PROVIDER_ALIAS;
@@ -220,7 +290,7 @@ private function susbcribeUser(array $user, string $mailTypeCode)
         $this->userSubscriptionsRepository->subscribeUser($list, $user['id'], $user['email']);
     }
 
-    private function addMailLog(array $user, ActiveRow $template, DateTime $deliveredAt, ?DateTime $openedAt = null)
+    private function addMailLog(array $user, ActiveRow $template, DateTime $deliveredAt, ?DateTime $openedAt = null, ?DateTime $clickedAt = null)
     {
         $mailLog = $this->mailLogsRepository->add(
             email: $user['email'],
@@ -231,7 +301,8 @@ private function addMailLog(array $user, ActiveRow $template, DateTime $delivere
 
         $this->mailLogsRepository->update($mailLog, [
             'delivered_at' => $deliveredAt,
-            'opened_at' => $openedAt
+            'opened_at' => $openedAt,
+            'clicked_at' => $clickedAt,
         ]);
     }
 }