From e71e063ef0904fcee70c00fe2fbf43966274ce53 Mon Sep 17 00:00:00 2001
From: wxa <wxa@users.noreply.github.com>
Date: Wed, 10 Mar 2021 10:44:21 +0300
Subject: [PATCH] Use same query logic for counting and exporting subs w/o
 lists

[MAILPOET-3462]
---
 .../ImportExport/ImportExportRepository.php    |  8 +++++++-
 .../ImportExportRepositoryTest.php             | 18 +++++++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/lib/Subscribers/ImportExport/ImportExportRepository.php b/lib/Subscribers/ImportExport/ImportExportRepository.php
index 26ad9e3ea5..8b78beea91 100644
--- a/lib/Subscribers/ImportExport/ImportExportRepository.php
+++ b/lib/Subscribers/ImportExport/ImportExportRepository.php
@@ -160,6 +160,7 @@ class ImportExportRepository {
 
   public function getSubscribersBatchBySegment(?SegmentEntity $segment, int $limit, int $offset = 0): array {
     $subscriberSegmentTable = $this->getTableName(SubscriberSegmentEntity::class);
+    $subscriberTable = $this->getTableName(SubscriberEntity::class);
     $segmentTable = $this->getTableName(SegmentEntity::class);
 
     $qb = $this->createSubscribersQueryBuilder($limit, $offset);
@@ -169,7 +170,12 @@ class ImportExportRepository {
       // if there are subscribers who do not belong to any segment, use
       // a CASE function to group them under "Not In Segment"
       $qb->addSelect("'" . WPFunctions::get()->__('Not In Segment', 'mailpoet') . "' AS segment_name")
-        ->andWhere("{$subscriberSegmentTable}.segment_id IS NULL");
+        ->leftJoin($subscriberTable, $subscriberTable, 's2', "{$subscriberTable}.id = s2.id")
+        ->leftJoin('s2', $subscriberSegmentTable, 'ssg2', "s2.id = ssg2.subscriber_id AND ssg2.status = :statusSubscribed AND {$segmentTable}.id <> ssg2.segment_id")
+        ->leftJoin('ssg2', $segmentTable, 'sg2', 'ssg2.segment_id = sg2.id AND sg2.deleted_at IS NULL')
+        ->andWhere("({$subscriberSegmentTable}.status != :statusSubscribed OR {$subscriberSegmentTable}.id IS NULL OR {$segmentTable}.deleted_at IS NOT NULL)")
+        ->andWhere('sg2.id IS NULL')
+        ->setParameter('statusSubscribed', SubscriberEntity::STATUS_SUBSCRIBED);
     } elseif ($segment->isStatic()) {
       $qb->addSelect("{$segmentTable}.name AS segment_name")
         ->andWhere("{$subscriberSegmentTable}.segment_id = :segmentId")
diff --git a/tests/integration/Subscribers/ImportExport/ImportExportRepositoryTest.php b/tests/integration/Subscribers/ImportExport/ImportExportRepositoryTest.php
index 1c6079fb71..37138c6146 100644
--- a/tests/integration/Subscribers/ImportExport/ImportExportRepositoryTest.php
+++ b/tests/integration/Subscribers/ImportExport/ImportExportRepositoryTest.php
@@ -247,13 +247,25 @@ class ImportExportRepositoryTest extends \MailPoetTest {
     $user2 = $this->createSubscriber('user2@export-test.com', 'Two', 'User');
     $user3 = $this->createSubscriber('user3@export-test.com', 'Three', 'User');
     $user4 = $this->createSubscriber('user4@export-test.com', 'Four', 'User');
+    $user5 = $this->createSubscriber('user5@export-test.com', 'Five', 'User');
     $segment1 = $this->createSegment('First', SegmentEntity::TYPE_DEFAULT);
     $segment2 = $this->createSegment('Two', SegmentEntity::TYPE_DEFAULT);
+    $segment3 = $this->createSegment('Three', SegmentEntity::TYPE_DEFAULT);
+    $segment3->setDeletedAt(new \DateTime());
+    $this->entityManager->persist($segment3);
+    // Subscribed to segment, shouldn't be exported
     $this->createSubscriberSegment($user1, $segment1, SubscriberEntity::STATUS_SUBSCRIBED);
+    // A mix of subscribed and unsubscribed, shouldn't be exported
     $this->createSubscriberSegment($user3, $segment2, SubscriberEntity::STATUS_SUBSCRIBED);
+    $this->createSubscriberSegment($user3, $segment1, SubscriberEntity::STATUS_UNSUBSCRIBED);
+    // Unsubscribed from segment, should be exported
+    $this->createSubscriberSegment($user2, $segment2, SubscriberEntity::STATUS_UNSUBSCRIBED);
+    // Subscribed to trashed segment, should be exported
+    $this->createSubscriberSegment($user4, $segment3, SubscriberEntity::STATUS_SUBSCRIBED);
+    // User5 is not subscribed to any segment, should be exported
 
     $exported = $this->repository->getSubscribersBatchBySegment(null, 100);
-    expect($exported)->count(2);
+    expect($exported)->count(3);
     expect($exported[0]['first_name'])->equals('Two');
     expect($exported[0]['last_name'])->equals('User');
     expect($exported[0]['email'])->equals('user2@export-test.com');
@@ -262,6 +274,10 @@ class ImportExportRepositoryTest extends \MailPoetTest {
     expect($exported[1]['last_name'])->equals('User');
     expect($exported[1]['email'])->equals('user4@export-test.com');
     expect($exported[1]['segment_name'])->equals('Not In Segment');
+    expect($exported[2]['first_name'])->equals('Five');
+    expect($exported[2]['last_name'])->equals('User');
+    expect($exported[2]['email'])->equals('user5@export-test.com');
+    expect($exported[2]['segment_name'])->equals('Not In Segment');
   }
 
   public function testItGetSubscribersByDynamicSegment(): void {