Fix code using the combined segments

[MAILPOET-3212]

lib/Segments/DynamicSegments/FilterHandler.php

@@ -75,14 +75,23 @@ class FilterHandler {
 
     if (!isset($data['connect']) || $data['connect'] === 'or') {
       // the final query: SELECT * FROM subscribers INNER JOIN (filter_select1 UNION filter_select2) filtered_subscribers ON filtered_subscribers.inner_subscriber_id = id
-      $queryBuilder->innerJoin($subscribersTable, sprintf('(%s)', join(' UNION ', $subQueries)), 'filtered_subscribers', 'filtered_subscribers.inner_subscriber_id = id');
+      $queryBuilder->innerJoin(
+        $subscribersTable,
+        sprintf('(%s)', join(' UNION ', $subQueries)),
+        'filtered_subscribers',
+        "filtered_subscribers.inner_subscriber_id = $subscribersTable.id"
+      );
       return $queryBuilder;
     }
 
     foreach ($subQueries as $subQuery) {
       // we need a unique name for each subquery so that we can join them together in the sql query - just make sure the identifier starts with a letter, not a number
       $subqueryName = 'a' . Security::generateRandomString(5);
-      $queryBuilder->innerJoin($subscribersTable, "($subQuery)", $subqueryName, "$subqueryName.inner_subscriber_id = id");
+      $queryBuilder->innerJoin(
+        $subscribersTable,
+        "($subQuery)",
+        $subqueryName,
+        "$subqueryName.inner_subscriber_id = $subscribersTable.id");
     }
     return $queryBuilder;
   }

lib/Segments/SegmentSubscribersRepository.php

@@ -189,9 +189,8 @@ class SegmentSubscribersRepository {
     // For BC compatibility fetching an empty result
     if (count($filters) === 0) {
       return $queryBuilder->andWhere('0 = 1');
-    }
+    } elseif ($segment instanceof SegmentEntity) {
-    foreach ($filters as $filter) {
+      $queryBuilder = $this->filterHandler->apply($queryBuilder, $segment);
-      $queryBuilder = $this->filterHandler->apply($queryBuilder, $filter);
     }
     $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName();
     $queryBuilder = $queryBuilder->andWhere("$subscribersTable.deleted_at IS NULL");

lib/Subscribers/ImportExport/ImportExportRepository.php

@@ -185,10 +185,7 @@ class ImportExportRepository {
       // So we need to use a placeholder
       $qb->addSelect(":segmentName AS segment_name")
         ->setParameter('segmentName', $segment->getName());
-      $filters = $segment->getDynamicFilters();
+      $qb = $this->filterHandler->apply($qb, $segment);
-      foreach ($filters as $filter) {
-        $qb = $this->filterHandler->apply($qb, $filter->getFilterData());
-      }
     }
 
     $statement = $qb->execute();

lib/Subscribers/SubscriberListingRepository.php

@@ -334,9 +334,7 @@ class SubscriberListingRepository extends ListingRepository {
     SegmentEntity $segment
   ) {
     // Apply dynamic segments filters
-    foreach ($segment->getDynamicFilters() as $filter) {
+    $subscribersQuery = $this->dynamicSegmentsFilter->apply($subscribersQuery, $segment);
-      $subscribersQuery = $this->dynamicSegmentsFilter->apply($subscribersQuery, $filter->getFilterData());
-    }
     // Apply group, search to fetch only necessary ids
     $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName();
     if ($definition->getSearch()) {

tests/integration/Segments/DynamicSegments/FilterHandlerTest.php

@@ -12,6 +12,7 @@ use MailPoet\Entities\StatisticsOpenEntity;
 use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Segments\DynamicSegments\Filters\EmailAction;
 use MailPoet\Subscribers\SubscribersRepository;
+use MailPoetVendor\Doctrine\DBAL\Driver\Statement;
 
 class FilterHandlerTest extends \MailPoetTest {
 
@@ -46,17 +47,24 @@ class FilterHandlerTest extends \MailPoetTest {
     // fetch entities
     /** @var SubscribersRepository $subscribersRepository */
     $subscribersRepository = $this->diContainer->get(SubscribersRepository::class);
-    $this->subscriber1 = $subscribersRepository->findOneBy(['email' => 'user-role-test1@example.com']);
+    $subscriber1 = $subscribersRepository->findOneBy(['email' => 'user-role-test1@example.com']);
-    $this->subscriber2 = $subscribersRepository->findOneBy(['email' => 'user-role-test2@example.com']);
+    assert($subscriber1 instanceof SubscriberEntity);
+    $this->subscriber1 = $subscriber1;
+    $subscriber2 = $subscribersRepository->findOneBy(['email' => 'user-role-test2@example.com']);
+    assert($subscriber2 instanceof SubscriberEntity);
+    $this->subscriber2 = $subscriber2;
   }
 
   public function testItAppliesFilter() {
     $segment = $this->getSegment('editor');
-    $queryBuilder = $this->filterHandler->apply($this->getQueryBuilder(), $segment);
+    $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute();
-    $result = $queryBuilder->execute()->fetchAll();
+    assert($statement instanceof Statement);
+    $result = $statement->fetchAll();
     expect($result)->count(2);
     $subscriber1 = $this->entityManager->find(SubscriberEntity::class, $result[0]['id']);
+    assert($subscriber1 instanceof SubscriberEntity);
     $subscriber2 = $this->entityManager->find(SubscriberEntity::class, $result[1]['id']);
+    assert($subscriber2 instanceof SubscriberEntity);
     expect($subscriber1->getEmail())->equals('user-role-test1@example.com');
     expect($subscriber2->getEmail())->equals('user-role-test3@example.com');
   }
@@ -71,8 +79,9 @@ class FilterHandlerTest extends \MailPoetTest {
     $this->entityManager->persist($dynamicSegmentFilter);
     $segment->addDynamicFilter($dynamicSegmentFilter);
     $this->entityManager->flush();
-    $queryBuilder = $this->filterHandler->apply($this->getQueryBuilder(), $segment);
+    $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute();
-    $result = $queryBuilder->execute()->fetchAll();
+    assert($statement instanceof Statement);
+    $result = $statement->fetchAll();
     expect($result)->count(3);
   }
 
@@ -96,8 +105,9 @@ class FilterHandlerTest extends \MailPoetTest {
     $this->entityManager->persist($dynamicSegmentFilter);
     $segment->addDynamicFilter($dynamicSegmentFilter);
     $this->entityManager->flush();
-    $queryBuilder = $this->filterHandler->apply($this->getQueryBuilder(), $segment);
+    $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute();
-    $result = $queryBuilder->execute()->fetchAll();
+    assert($statement instanceof Statement);
+    $result = $statement->fetchAll();
     expect($result)->count(3);
   }
 
@@ -143,8 +153,9 @@ class FilterHandlerTest extends \MailPoetTest {
     $segment->addDynamicFilter($filterOpened);
     $this->entityManager->flush();
 
-    $queryBuilder = $this->filterHandler->apply($this->getQueryBuilder(), $segment);
+    $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute();
-    $result = $queryBuilder->execute()->fetchAll();
+    assert($statement instanceof Statement);
+    $result = $statement->fetchAll();
     expect($result)->count(1);
   }
 

tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php

@@ -3,9 +3,11 @@
 namespace MailPoet\Segments\DynamicSegments\Filters;
 
 use MailPoet\Entities\DynamicSegmentFilterData;
+use MailPoet\Entities\DynamicSegmentFilterEntity;
 use MailPoet\Entities\NewsletterEntity;
 use MailPoet\Entities\NewsletterLinkEntity;
 use MailPoet\Entities\ScheduledTaskEntity;
+use MailPoet\Entities\SegmentEntity;
 use MailPoet\Entities\SendingQueueEntity;
 use MailPoet\Entities\StatisticsClickEntity;
 use MailPoet\Entities\StatisticsNewsletterEntity;
@@ -177,13 +179,19 @@ class EmailActionTest extends \MailPoetTest {
       ->from($subscribersTable);
   }
 
-  private function getSegmentFilter(string $action, int $newsletterId, int $linkId = null): DynamicSegmentFilterData {
+  private function getSegmentFilter(string $action, int $newsletterId, int $linkId = null): DynamicSegmentFilterEntity {
-    return new DynamicSegmentFilterData([
+    $segmentFilterData = new DynamicSegmentFilterData([
       'segmentType' => DynamicSegmentFilterData::TYPE_EMAIL,
       'action' => $action,
       'newsletter_id' => $newsletterId,
       'link_id' => $linkId,
     ]);
+    $segment = new SegmentEntity('Dynamic Segment', SegmentEntity::TYPE_DYNAMIC, 'description');
+    $this->entityManager->persist($segment);
+    $dynamicSegmentFilter = new DynamicSegmentFilterEntity($segment, $segmentFilterData);
+    $this->entityManager->persist($dynamicSegmentFilter);
+    $segment->addDynamicFilter($dynamicSegmentFilter);
+    return $dynamicSegmentFilter;
   }
 
   private function createSubscriber(string $email) {

tests/integration/Segments/DynamicSegments/Filters/UserRoleTest.php

@@ -3,6 +3,8 @@
 namespace MailPoet\Segments\DynamicSegments\Filters;
 
 use MailPoet\Entities\DynamicSegmentFilterData;
+use MailPoet\Entities\DynamicSegmentFilterEntity;
+use MailPoet\Entities\SegmentEntity;
 use MailPoet\Entities\SubscriberEntity;
 
 class UserRoleTest extends \MailPoetTest {
@@ -47,11 +49,17 @@ class UserRoleTest extends \MailPoetTest {
       ->from($subscribersTable);
   }
 
-  private function getSegmentFilter(string $role): DynamicSegmentFilterData {
+  private function getSegmentFilter(string $role): DynamicSegmentFilterEntity {
-    return new DynamicSegmentFilterData([
+    $data = new DynamicSegmentFilterData([
       'segmentType' => DynamicSegmentFilterData::TYPE_USER_ROLE,
       'wordpressRole' => $role,
     ]);
+    $segment = new SegmentEntity('Dynamic Segment', SegmentEntity::TYPE_DYNAMIC, 'description');
+    $this->entityManager->persist($segment);
+    $dynamicSegmentFilter = new DynamicSegmentFilterEntity($segment, $data);
+    $this->entityManager->persist($dynamicSegmentFilter);
+    $segment->addDynamicFilter($dynamicSegmentFilter);
+    return $dynamicSegmentFilter;
   }
 
   public function _after() {