Remove logic to handle MP2 subscribers when deactivating subscribers

This commit removes the logic that was added in #2045 to handle
subscribers migrated from MP2 when deactivating subscribers. Without it,
MP3 would deactive all subscribers imported from MP2 as the subscribe
date is migrated but the stats are not (see
https://mailpoet.atlassian.net/browse/MAILPOET-2040) for more details.

This code is not necessary anymore as we are removing all the MP2 migration
related code.

[MAILPOET-4376]

mailpoet/lib/Subscribers/InactiveSubscribersController.php

@@ -2,12 +2,10 @@
 
 namespace MailPoet\Subscribers;
 
-use MailPoet\Config\MP2Migrator;
 use MailPoet\Entities\ScheduledTaskEntity;
 use MailPoet\Entities\ScheduledTaskSubscriberEntity;
 use MailPoet\Entities\SendingQueueEntity;
 use MailPoet\Entities\SubscriberEntity;
-use MailPoet\Settings\SettingsRepository;
 use MailPoetVendor\Carbon\Carbon;
 use MailPoetVendor\Doctrine\DBAL\Connection;
 use MailPoetVendor\Doctrine\ORM\EntityManager;
@@ -19,17 +17,12 @@ class InactiveSubscribersController {
 
   private $processedTaskIdsTableCreated = false;
 
-  /** @var SettingsRepository */
-  private $settingsRepository;
-
   /** @var EntityManager */
   private $entityManager;
 
   public function __construct(
-    EntityManager $entityManager,
+    EntityManager $entityManager
-    SettingsRepository $settingsRepository
   ) {
-    $this->settingsRepository = $settingsRepository;
     $this->entityManager = $entityManager;
   }
 
@@ -60,7 +53,7 @@ class InactiveSubscribersController {
   }
 
   /**
-   * @return int|bool
+   * @return int
    */
   private function deactivateSubscribers(Carbon $thresholdDate, int $batchSize, ?int $startId = null, ?int $unopenedEmails = self::UNOPENED_EMAILS_THRESHOLD) {
     $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName();
@@ -73,13 +66,6 @@ class InactiveSubscribersController {
     $dayAgo = new Carbon();
     $dayAgoIso = $dayAgo->subDay()->toDateTimeString();
 
-    // If MP2 migration occurred during detection interval we can't deactivate subscribers
-    // because they are imported with original subscription date but they were not present in a list for whole period
-    $mp2MigrationDate = $this->getMP2MigrationDate();
-    if ($mp2MigrationDate && $mp2MigrationDate > $thresholdDate) {
-      return false;
-    }
-
     // Temporary table with processed tasks from threshold date up to yesterday
     $processedTaskIdsTable = 'inactive_task_ids';
     if (!$this->processedTaskIdsTableCreated) {
@@ -160,40 +146,24 @@ class InactiveSubscribersController {
     $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName();
     $connection = $this->entityManager->getConnection();
 
-    $mp2MigrationDate = $this->getMP2MigrationDate();
+    $idsToActivate = $connection->executeQuery("
-    if ($mp2MigrationDate && $mp2MigrationDate > $thresholdDate) {
+      SELECT s.id
-      // If MP2 migration occurred during detection interval re-activate all subscribers created before migration
+      FROM {$subscribersTable} s
-      $idsToActivate = $connection->executeQuery("
+      LEFT OUTER JOIN {$subscribersTable} s2 ON s.id = s2.id AND GREATEST(
-        SELECT id
+        COALESCE(s2.last_engagement_at, '0'),
-        FROM {$subscribersTable}
+        COALESCE(s2.last_subscribed_at, '0'),
-        WHERE created_at < :migrationDate
+        COALESCE(s2.created_at, '0')
-          AND status = :statusInactive
+      ) > :thresholdDate
-        LIMIT :batchSize
+      WHERE s.last_subscribed_at < :thresholdDate
-        ", [
+        AND s.status = :statusInactive
-          'migrationDate' => $mp2MigrationDate,
+        AND s2.id IS NOT NULL
-          'statusInactive' => SubscriberEntity::STATUS_INACTIVE,
+      GROUP BY s.id
-          'batchSize' => $batchSize,
+      LIMIT :batchSize
-      ], ['batchSize' => \PDO::PARAM_INT])->fetchAllAssociative();
+    ", [
-    } else {
+      'thresholdDate' => $thresholdDate,
-      $idsToActivate = $connection->executeQuery("
+      'statusInactive' => SubscriberEntity::STATUS_INACTIVE,
-        SELECT s.id
+      'batchSize' => $batchSize,
-        FROM {$subscribersTable} s
+    ], ['batchSize' => \PDO::PARAM_INT])->fetchAllAssociative();
-        LEFT OUTER JOIN {$subscribersTable} s2 ON s.id = s2.id AND GREATEST(
-          COALESCE(s2.last_engagement_at, '0'),
-          COALESCE(s2.last_subscribed_at, '0'),
-          COALESCE(s2.created_at, '0')
-        ) > :thresholdDate
-        WHERE s.last_subscribed_at < :thresholdDate
-          AND s.status = :statusInactive
-          AND s2.id IS NOT NULL
-        GROUP BY s.id
-        LIMIT :batchSize
-      ", [
-        'thresholdDate' => $thresholdDate,
-        'statusInactive' => SubscriberEntity::STATUS_INACTIVE,
-        'batchSize' => $batchSize,
-      ], ['batchSize' => \PDO::PARAM_INT])->fetchAllAssociative();
-    }
 
     $idsToActivate = array_map(
       function($id) {
@@ -209,9 +179,4 @@ class InactiveSubscribersController {
     ], ['idsToActivate' => Connection::PARAM_INT_ARRAY]);
     return count($idsToActivate);
   }
-
-  private function getMP2MigrationDate() {
-    $setting = $this->settingsRepository->findOneByName(MP2Migrator::MIGRATION_COMPLETE_SETTING_KEY);
-    return $setting ? Carbon::instance($setting->getCreatedAt()) : null;
-  }
 }

mailpoet/tests/integration/Subscribers/InactiveSubscribersControllerTest.php

@@ -2,7 +2,6 @@
 
 namespace MailPoet\Subscribers;
 
-use MailPoet\Config\MP2Migrator;
 use MailPoet\Entities\NewsletterEntity;
 use MailPoet\Entities\ScheduledTaskEntity;
 use MailPoet\Entities\ScheduledTaskSubscriberEntity;
@@ -10,7 +9,6 @@ use MailPoet\Entities\SendingQueueEntity;
 use MailPoet\Entities\SettingEntity;
 use MailPoet\Entities\StatisticsOpenEntity;
 use MailPoet\Entities\SubscriberEntity;
-use MailPoet\Settings\SettingsRepository;
 use MailPoet\Tasks\Sending;
 use MailPoetVendor\Carbon\Carbon;
 use MailPoetVendor\Doctrine\ORM\EntityManager;
@@ -32,8 +30,7 @@ class InactiveSubscribersControllerTest extends \MailPoetTest {
 
   public function _before() {
     $this->controller = new InactiveSubscribersController(
-      $this->diContainer->get(EntityManager::class),
+      $this->diContainer->get(EntityManager::class)
-      $this->diContainer->get(SettingsRepository::class)
     );
     $this->subscribersRepository = $this->diContainer->get(SubscribersRepository::class);
     $this->truncateEntity(SubscriberEntity::class);
@@ -169,20 +166,6 @@ class InactiveSubscribersControllerTest extends \MailPoetTest {
     expect($subscriber1->getStatus())->equals(SubscriberEntity::STATUS_SUBSCRIBED);
   }
 
-  public function testItDoesNotDeactivatesSubscribersWhenMP2MigrationHappenedWithinInterval(): void {
-    $this->createSetting(MP2Migrator::MIGRATION_COMPLETE_SETTING_KEY, true, (new Carbon())->subDays(3));
-
-    $subscriber = $this->createSubscriber('s1@email.com', 10);
-    $this->createCompletedSendingTasksForSubscriber($subscriber, self::UNOPENED_EMAILS_THRESHOLD, 3);
-
-    $result = $this->controller->markInactiveSubscribers(self::INACTIVITY_DAYS_THRESHOLD, self::PROCESS_BATCH_SIZE);
-    expect($result)->equals(0);
-    $subscriber = $this->subscribersRepository->findOneById($subscriber->getId());
-    assert($subscriber instanceof SubscriberEntity);
-    expect($subscriber->getStatus())->equals(SubscriberEntity::STATUS_SUBSCRIBED);
-    $this->removeSetting(MP2Migrator::MIGRATION_COMPLETE_SETTING_KEY);
-  }
-
   public function testItActivatesSubscriberWhoRecentlyOpenedEmail(): void {
     [$task, $queue] = $this->createCompletedSendingTask(2);
     $subscriber = $this->createSubscriber('s1@email.com', 10, SubscriberEntity::STATUS_INACTIVE);
@@ -239,23 +222,6 @@ class InactiveSubscribersControllerTest extends \MailPoetTest {
     expect($subscriber->getStatus())->equals(SubscriberEntity::STATUS_INACTIVE);
   }
 
-  public function testItActivatesSubscribersWhenMP2MigrationHappenedWithinInterval(): void {
-    [$task] = $this->createCompletedSendingTask(3);
-
-    $this->createSetting(MP2Migrator::MIGRATION_COMPLETE_SETTING_KEY, true, (new Carbon())->subDays(3));
-
-    $subscriber = $this->createSubscriber('s1@email.com', 10, SubscriberEntity::STATUS_INACTIVE);
-    $this->addSubscriberToTask($subscriber, $task);
-
-    $result = $this->controller->markActiveSubscribers(self::INACTIVITY_DAYS_THRESHOLD, self::PROCESS_BATCH_SIZE);
-    $this->entityManager->clear();
-    expect($result)->equals(1);
-    $subscriber = $this->subscribersRepository->findOneById($subscriber->getId());
-    assert($subscriber instanceof SubscriberEntity);
-    expect($subscriber->getStatus())->equals(SubscriberEntity::STATUS_SUBSCRIBED);
-    $this->removeSetting(MP2Migrator::MIGRATION_COMPLETE_SETTING_KEY);
-  }
-
   public function testItDoesReactivateInactiveSubscribers(): void {
     [$task] = $this->createCompletedSendingTask(2);
     $subscriber = $this->createSubscriber('s1@email.com', 10, SubscriberEntity::STATUS_INACTIVE);