Replace Segment model with Doctrine in \MailPoet\Segments\WP

[MAILPOET-5752]

mailpoet/lib/Segments/WP.php

@@ -6,7 +6,6 @@ use MailPoet\Config\SubscriberChangesNotifier;
 use MailPoet\DI\ContainerWrapper;
 use MailPoet\Entities\SegmentEntity;
 use MailPoet\Entities\SubscriberEntity;
-use MailPoet\Models\Segment;
 use MailPoet\Models\Subscriber;
 use MailPoet\Models\SubscriberSegment;
 use MailPoet\Newsletter\Scheduler\WelcomeScheduler;
@@ -43,6 +42,8 @@ class WP {
   /** @var Validator */
   private $validator;
 
+  private $segmentsRepository;
+
   public function __construct(
     WPFunctions $wp,
     WelcomeScheduler $welcomeScheduler,
@@ -50,7 +51,8 @@ class WP {
     SubscribersRepository $subscribersRepository,
     SubscriberSegmentRepository $subscriberSegmentRepository,
     SubscriberChangesNotifier $subscriberChangesNotifier,
-    Validator $validator
+    Validator $validator,
+    SegmentsRepository $segmentsRepository
   ) {
     $this->wp = $wp;
     $this->welcomeScheduler = $welcomeScheduler;
@@ -59,6 +61,7 @@ class WP {
     $this->subscriberSegmentRepository = $subscriberSegmentRepository;
     $this->subscriberChangesNotifier = $subscriberChangesNotifier;
     $this->validator = $validator;
+    $this->segmentsRepository = $segmentsRepository;
   }
 
   /**
@@ -102,8 +105,7 @@ class WP {
    */
   private function createOrUpdateSubscriber(string $currentFilter, \WP_User $wpUser, $subscriber = false, $oldWpUserData = false): void {
     // Add or update
-    $wpSegment = Segment::getWPSegment();
+    $wpSegment = $this->segmentsRepository->getWPUsersSegment();
-    if (!$wpSegment) return;
 
     // find subscriber by email when is false
     if (!$subscriber) {
@@ -138,7 +140,7 @@ class WP {
       unset($data['source']); // don't override status for existing users
     }
 
-    $addingNewUserToDisabledWPSegment = $wpSegment->deletedAt !== null && $currentFilter === 'user_register';
+    $addingNewUserToDisabledWPSegment = $wpSegment->getDeletedAt() !== null && $currentFilter === 'user_register';
 
     $otherActiveSegments = [];
     if ($subscriber) {
@@ -160,13 +162,13 @@ class WP {
       // add subscriber to the WP Users segment
       SubscriberSegment::subscribeToSegments(
         $subscriber,
-        [$wpSegment->id]
+        [$wpSegment->getId()]
       );
 
       if (!$signupConfirmationEnabled && $subscriber->status === Subscriber::STATUS_SUBSCRIBED && $currentFilter === 'user_register') {
         $subscriberSegment = $this->subscriberSegmentRepository->findOneBy([
           'subscriber' => $subscriber->id(),
-          'segment' => $wpSegment->id(),
+          'segment' => $wpSegment->getId(),
         ]);
 
         if (!is_null($subscriberSegment)) {
@@ -264,9 +266,9 @@ class WP {
 
   private function insertSubscribers(): array {
     global $wpdb;
-    $wpSegment = Segment::getWPSegment();
+    $wpSegment = $this->segmentsRepository->getWPUsersSegment();
-    if (!$wpSegment) return [];
+
-    if ($wpSegment->deletedAt !== null) {
+    if ($wpSegment->getDeletedAt() !== null) {
       $subscriberStatus = SubscriberEntity::STATUS_UNCONFIRMED;
       $deletedAt = 'CURRENT_TIMESTAMP()';
     } else {
@@ -338,28 +340,17 @@ class WP {
   }
 
   private function insertUsersToSegment(): void {
-    $wpSegment = Segment::getWPSegment();
+    $wpSegment = $this->segmentsRepository->getWPUsersSegment();
     $subscribersTable = Subscriber::$_table;
     $wpMailpoetSubscriberSegmentTable = SubscriberSegment::$_table;
     Subscriber::rawExecute(sprintf('
      INSERT IGNORE INTO %s(subscriber_id, segment_id, created_at)
       SELECT mps.id, "%s", CURRENT_TIMESTAMP() FROM %s mps
         WHERE mps.wp_user_id > 0
-    ', $wpMailpoetSubscriberSegmentTable, $wpSegment->id, $subscribersTable));
+    ', $wpMailpoetSubscriberSegmentTable, $wpSegment->getId(), $subscribersTable));
   }
 
   private function removeOrphanedSubscribers(): void {
-    // remove orphaned wp segment subscribers (not having a matching wp user id),
+    $this->subscribersRepository->removeOrphanedSubscribersFromWpSegment();
-    // e.g. if wp users were deleted directly from the database
-    global $wpdb;
-
-    $wpSegment = Segment::getWPSegment();
-
-    $wpSegment->subscribers()
-      ->leftOuterJoin($wpdb->users, [MP_SUBSCRIBERS_TABLE . '.wp_user_id', '=', 'wu.id'], 'wu')
-      ->whereRaw('(wu.id IS NULL OR ' . MP_SUBSCRIBERS_TABLE . '.email = "")')
-      ->findResultSet()
-      ->set('wp_user_id', null)
-      ->delete();
   }
 }

mailpoet/lib/Subscribers/SubscribersRepository.php

@@ -12,6 +12,7 @@ use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Entities\SubscriberSegmentEntity;
 use MailPoet\Entities\SubscriberTagEntity;
 use MailPoet\Entities\TagEntity;
+use MailPoet\Segments\SegmentsRepository;
 use MailPoet\Util\License\Features\Subscribers;
 use MailPoet\WP\Functions as WPFunctions;
 use MailPoetVendor\Carbon\Carbon;
@@ -38,14 +39,19 @@ class SubscribersRepository extends Repository {
   /** @var SubscriberChangesNotifier */
   private $changesNotifier;
 
+  /** @var SegmentsRepository */
+  private $segmentsRepository;
+
   public function __construct(
     EntityManager $entityManager,
     SubscriberChangesNotifier $changesNotifier,
-    WPFunctions $wp
+    WPFunctions $wp,
+    SegmentsRepository $segmentsRepository
   ) {
     $this->wp = $wp;
     parent::__construct($entityManager);
     $this->changesNotifier = $changesNotifier;
+    $this->segmentsRepository = $segmentsRepository;
   }
 
   protected function getEntityClassName() {
@@ -553,6 +559,24 @@ class SubscribersRepository extends Repository {
     return $count;
   }
 
+  public function removeOrphanedSubscribersFromWpSegment(): void {
+    global $wpdb;
+
+    $segmentId = $this->segmentsRepository->getWpUsersSegment()->getId();
+
+    $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName();
+    $subscriberSegmentsTable = $this->entityManager->getClassMetadata(SubscriberSegmentEntity::class)->getTableName();
+
+    $this->entityManager->getConnection()->executeStatement(
+      "DELETE s
+       FROM {$subscribersTable} s
+       INNER JOIN {$subscriberSegmentsTable} ss ON s.id = ss.subscriber_id
+       LEFT JOIN {$wpdb->users} u ON s.wp_user_id = u.id
+       WHERE ss.segment_id = :segmentId AND (u.id IS NULL OR s.email = '')",
+      ['segmentId' => $segmentId], ['segmentId' => \PDO::PARAM_INT]
+    );
+  }
+
   /**
    * @return int - number of processed ids
    */

mailpoet/tests/_support/IntegrationTester.php

@@ -83,6 +83,19 @@ class IntegrationTester extends \Codeception\Actor {
     return $userId;
   }
 
+  /**
+   * Deletes a WP user directly from the database without triggering any hooks.
+   * Needed to be able to test deleting orphaned subscribers.
+   */
+  public function deleteWPUserFromDatabase(int $id): void {
+    global $wpdb;
+
+    $this->entityManager->getConnection()->executeStatement(
+      "DELETE FROM {$wpdb->users} WHERE id = :id",
+      ['id' => $id], ['id' => \PDO::PARAM_INT]
+    );
+  }
+
   public function createWordPressTerm(string $term, string $taxonomy, array $args = []): int {
     $term = wp_insert_term($term, $taxonomy, $args);
     if ($term instanceof WP_Error) {

mailpoet/tests/integration/Segments/WPTest.php

@@ -246,7 +246,7 @@ class WPTest extends \MailPoetTest {
     $this->updateWPUserEmail($id, '');
     $this->wpSegment->synchronizeUsers();
     verify($this->subscribersRepository->findOneBy(['wpUserId' => $id]))->null();
-    $this->deleteWPUser($id);
+    $this->tester->deleteWPUserFromDatabase($id);
   }
 
   public function testRemovesUsersWithInvalidEmailsFromSunscribersDuringSynchronization(): void {
@@ -255,7 +255,7 @@ class WPTest extends \MailPoetTest {
     $this->updateWPUserEmail($id, 'ivalid.@email.com');
     $this->wpSegment->synchronizeUsers();
     verify($this->subscribersRepository->findOneBy(['wpUserId' => $id]))->null();
-    $this->deleteWPUser($id);
+    $this->tester->deleteWPUserFromDatabase($id);
   }
 
   public function testItDoesNotSynchronizeEmptyEmailsForNewUsers(): void {
@@ -263,7 +263,7 @@ class WPTest extends \MailPoetTest {
     $this->updateWPUserEmail($id, '');
     $this->wpSegment->synchronizeUsers();
     verify($this->subscribersRepository->findOneBy(['wpUserId' => $id]))->null();
-    $this->deleteWPUser($id);
+    $this->tester->deleteWPUserFromDatabase($id);
   }
 
   public function testItSynchronizeFirstNames(): void {
@@ -372,7 +372,7 @@ class WPTest extends \MailPoetTest {
     $this->insertUser();
     $id = $this->insertUser();
     $this->wpSegment->synchronizeUsers();
-    $this->deleteWPUser($id);
+    $this->tester->deleteWPUserFromDatabase($id);
     $this->wpSegment->synchronizeUsers();
     $subscribers = $this->subscribersRepository->findBy(['wpUserId' => $this->userIds]);
     verify(count($subscribers))->equals(1);
@@ -722,17 +722,6 @@ class WPTest extends \MailPoetTest {
     ', $wpdb->users, $name, $id));
   }
 
-  private function deleteWPUser(int $id): void {
-    global $wpdb;
-    $db = ORM::getDb();
-    $db->exec(sprintf('
-       DELETE FROM
-         %s
-       WHERE
-         id = %s
-    ', $wpdb->users, $id));
-  }
-
   private function clearEmail(SubscriberEntity $subscriber): void {
     $this->connection->executeStatement(
       'UPDATE ' . MP_SUBSCRIBERS_TABLE . '

mailpoet/tests/integration/Statistics/Track/ClicksTest.php

@@ -509,7 +509,12 @@ class ClicksTest extends \MailPoetTest {
     $clicksRepository = $this->diContainer->get(StatisticsClicksRepository::class);
     $data = $this->trackData;
     $data->userAgent = 'User Agent';
-    $subscribersRepository = new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($wpMock), $wpMock);
+    $subscribersRepository = $this->getServiceWithOverrides(SubscribersRepository::class,
+      [
+        'changesNotifier' => new SubscriberChangesNotifier($wpMock),
+        'wp' => $wpMock,
+      ]
+    );
     $statisticsOpensRepository = $this->diContainer->get(StatisticsOpensRepository::class);
     $opens = new Opens(
       $statisticsOpensRepository,
@@ -547,7 +552,12 @@ class ClicksTest extends \MailPoetTest {
     $clicksRepository = $this->diContainer->get(StatisticsClicksRepository::class);
     $data = $this->trackData;
     $data->userAgent = null;
-    $subscribersRepository = new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($wpMock), $wpMock);
+    $subscribersRepository = $this->getServiceWithOverrides(SubscribersRepository::class,
+      [
+        'changesNotifier' => new SubscriberChangesNotifier($wpMock),
+        'wp' => $wpMock,
+      ]
+    );
     $statisticsOpensRepository = $this->diContainer->get(StatisticsOpensRepository::class);
     $opens = new Opens(
       $statisticsOpensRepository,
@@ -585,7 +595,12 @@ class ClicksTest extends \MailPoetTest {
     $clicksRepository = $this->diContainer->get(StatisticsClicksRepository::class);
     $data = $this->trackData;
     $data->userAgent = UserAgentEntity::MACHINE_USER_AGENTS[0];
-    $subscribersRepository = new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($wpMock), $wpMock);
+    $subscribersRepository = $this->getServiceWithOverrides(SubscribersRepository::class,
+      [
+        'changesNotifier' => new SubscriberChangesNotifier($wpMock),
+        'wp' => $wpMock,
+      ]
+    );
     $statisticsOpensRepository = $this->diContainer->get(StatisticsOpensRepository::class);
     $opens = new Opens(
       $statisticsOpensRepository,

mailpoet/tests/integration/Statistics/Track/OpensTest.php

@@ -366,7 +366,12 @@ class OpensTest extends \MailPoetTest {
     $opens = Stub::construct($this->opens, [
       $this->diContainer->get(StatisticsOpensRepository::class),
       $this->diContainer->get(UserAgentsRepository::class),
-      new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($wpMock), $wpMock),
+      $this->getServiceWithOverrides(SubscribersRepository::class,
+        [
+          'changesNotifier' => new SubscriberChangesNotifier($wpMock),
+          'wp' => $wpMock,
+        ]
+      ),
     ], [
       'returnResponse' => null,
     ], $this);
@@ -390,7 +395,12 @@ class OpensTest extends \MailPoetTest {
     $opens = Stub::construct($this->opens, [
       $this->diContainer->get(StatisticsOpensRepository::class),
       $this->diContainer->get(UserAgentsRepository::class),
-      new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($wpMock), $wpMock),
+      $this->getServiceWithOverrides(SubscribersRepository::class,
+        [
+          'changesNotifier' => new SubscriberChangesNotifier($wpMock),
+          'wp' => $wpMock,
+        ]
+      ),
     ], [
       'returnResponse' => null,
     ], $this);
@@ -414,7 +424,12 @@ class OpensTest extends \MailPoetTest {
     $opens = Stub::construct($this->opens, [
       $this->diContainer->get(StatisticsOpensRepository::class),
       $this->diContainer->get(UserAgentsRepository::class),
-      new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($wpMock), $wpMock),
+      $this->getServiceWithOverrides(SubscribersRepository::class,
+        [
+          'changesNotifier' => new SubscriberChangesNotifier($wpMock),
+          'wp' => $wpMock,
+        ]
+      ),
     ], [
       'returnResponse' => null,
     ], $this);

mailpoet/tests/integration/Subscribers/SubscribersRepositoryTest.php

@@ -398,6 +398,23 @@ class SubscribersRepositoryTest extends \MailPoetTest {
     verify($this->repository->getMaxSubscriberId())->equals($subscriberTwo->getId());
   }
 
+  public function testRemoveOrphanedSubscribersFromWpSegment(): void {
+    $wpUserId1 = $this->tester->createWordPressUser('subscriber1@email.com', 'author');
+    $wpUserId2 = $this->tester->createWordPressUser('subscriber2@email.com', 'author');
+    $wpUserId3 = $this->tester->createWordPressUser('subscriber3@email.com', 'author');
+    $subscriber2 = $this->repository->findOneBy(['wpUserId' => $wpUserId2]);
+    $subscriber3 = $this->repository->findOneBy(['wpUserId' => $wpUserId3]);
+
+    $this->tester->deleteWPUserFromDatabase($wpUserId1);
+
+    $this->repository->removeOrphanedSubscribersFromWpSegment();
+
+    $subscribers = $this->repository->findAll();
+    $this->assertCount(2, $subscribers);
+    $this->assertSame($subscribers[0], $subscriber2);
+    $this->assertSame($subscribers[1], $subscriber3);
+  }
+
   private function createSubscriber(string $email, ?DateTimeImmutable $deletedAt = null): SubscriberEntity {
     $subscriber = new SubscriberEntity();
     $subscriber->setEmail($email);

mailpoet/tests/integration/WooCommerce/SubscriberEngagementTest.php

@@ -28,9 +28,15 @@ class SubscriberEngagementTest extends \MailPoetTest {
   public function _before() {
     $this->wooCommerceHelperMock = $this->createMock(Helper::class);
     $this->wpMock = $this->createMock(WPFunctions::class);
+    $subscribersRepository = $this->getServiceWithOverrides(SubscribersRepository::class,
+      [
+        'changesNotifier' => new SubscriberChangesNotifier($this->wpMock),
+        'wp' => $this->wpMock,
+      ]
+    );
     $this->subscriberEngagement = new SubscriberEngagement(
       $this->wooCommerceHelperMock,
-      new SubscribersRepository($this->entityManager, new SubscriberChangesNotifier($this->wpMock), $this->wpMock)
+      $subscribersRepository
     );
   }