diff --git a/lib/Segments/DynamicSegments/FilterHandler.php b/lib/Segments/DynamicSegments/FilterHandler.php index 0e4021d286..0bbc1ffda2 100644 --- a/lib/Segments/DynamicSegments/FilterHandler.php +++ b/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; } diff --git a/lib/Segments/SegmentSubscribersRepository.php b/lib/Segments/SegmentSubscribersRepository.php index d08abfeb57..c19ff04169 100644 --- a/lib/Segments/SegmentSubscribersRepository.php +++ b/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'); - } - foreach ($filters as $filter) { - $queryBuilder = $this->filterHandler->apply($queryBuilder, $filter); + } elseif ($segment instanceof SegmentEntity) { + $queryBuilder = $this->filterHandler->apply($queryBuilder, $segment); } $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName(); $queryBuilder = $queryBuilder->andWhere("$subscribersTable.deleted_at IS NULL"); diff --git a/lib/Subscribers/ImportExport/ImportExportRepository.php b/lib/Subscribers/ImportExport/ImportExportRepository.php index 8931de091c..e210600643 100644 --- a/lib/Subscribers/ImportExport/ImportExportRepository.php +++ b/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(); - foreach ($filters as $filter) { - $qb = $this->filterHandler->apply($qb, $filter->getFilterData()); - } + $qb = $this->filterHandler->apply($qb, $segment); } $statement = $qb->execute(); diff --git a/lib/Subscribers/SubscriberListingRepository.php b/lib/Subscribers/SubscriberListingRepository.php index 6f54f36c43..b34491fd5d 100644 --- a/lib/Subscribers/SubscriberListingRepository.php +++ b/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, $filter->getFilterData()); - } + $subscribersQuery = $this->dynamicSegmentsFilter->apply($subscribersQuery, $segment); // Apply group, search to fetch only necessary ids $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName(); if ($definition->getSearch()) { diff --git a/tests/integration/Segments/DynamicSegments/FilterHandlerTest.php b/tests/integration/Segments/DynamicSegments/FilterHandlerTest.php index 3e75a9ef12..b8e23bda15 100644 --- a/tests/integration/Segments/DynamicSegments/FilterHandlerTest.php +++ b/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']); - $this->subscriber2 = $subscribersRepository->findOneBy(['email' => 'user-role-test2@example.com']); + $subscriber1 = $subscribersRepository->findOneBy(['email' => 'user-role-test1@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); - $result = $queryBuilder->execute()->fetchAll(); + $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute(); + 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); - $result = $queryBuilder->execute()->fetchAll(); + $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute(); + 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); - $result = $queryBuilder->execute()->fetchAll(); + $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute(); + 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); - $result = $queryBuilder->execute()->fetchAll(); + $statement = $this->filterHandler->apply($this->getQueryBuilder(), $segment)->execute(); + assert($statement instanceof Statement); + $result = $statement->fetchAll(); expect($result)->count(1); } diff --git a/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php b/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php index 83693ce52e..a48d9b6d88 100644 --- a/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php +++ b/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 { - return new DynamicSegmentFilterData([ + private function getSegmentFilter(string $action, int $newsletterId, int $linkId = null): DynamicSegmentFilterEntity { + $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) { diff --git a/tests/integration/Segments/DynamicSegments/Filters/UserRoleTest.php b/tests/integration/Segments/DynamicSegments/Filters/UserRoleTest.php index 6bb7383899..4c685f2098 100644 --- a/tests/integration/Segments/DynamicSegments/Filters/UserRoleTest.php +++ b/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 { - return new DynamicSegmentFilterData([ + private function getSegmentFilter(string $role): DynamicSegmentFilterEntity { + $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() {