From a98f627dbff67cb3ea43b1e8b69eaf11e20b9d76 Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Tue, 22 Sep 2020 14:59:00 +0200 Subject: [PATCH] Add segment filters application to Subscribers listing repository [MAILPOET-3077] --- lib/Listing/ListingRepository.php | 5 + .../SubscriberListingRepository.php | 108 +++++++++++++++- .../SubscriberListingRepositoryTest.php | 116 +++++++++++++++++- 3 files changed, 223 insertions(+), 6 deletions(-) diff --git a/lib/Listing/ListingRepository.php b/lib/Listing/ListingRepository.php index 05117f3117..b8e13157a1 100644 --- a/lib/Listing/ListingRepository.php +++ b/lib/Listing/ListingRepository.php @@ -82,6 +82,8 @@ abstract class ListingRepository { if ($parameters) { $this->applyParameters($queryBuilder, $parameters); } + + $this->applyListingDefinition($queryBuilder, $definition); } abstract protected function applyGroup(QueryBuilder $queryBuilder, string $group); @@ -92,6 +94,9 @@ abstract class ListingRepository { abstract protected function applyParameters(QueryBuilder $queryBuilder, array $parameters); + protected function applyListingDefinition(QueryBuilder $queryBuilder, ListingDefinition $definition) { + } + protected function applySorting(QueryBuilder $queryBuilder, string $sortBy, string $sortOrder) { $alias = $this->queryBuilder->getRootAliases()[0]; $queryBuilder->addOrderBy("$alias.$sortBy", $sortOrder); diff --git a/lib/Subscribers/SubscriberListingRepository.php b/lib/Subscribers/SubscriberListingRepository.php index 140a74c4c9..985094c29d 100644 --- a/lib/Subscribers/SubscriberListingRepository.php +++ b/lib/Subscribers/SubscriberListingRepository.php @@ -2,14 +2,23 @@ namespace MailPoet\Subscribers; +use MailPoet\Entities\SegmentEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Listing\ListingDefinition; use MailPoet\Listing\ListingRepository; +use MailPoet\Segments\DynamicSegments\FilterHandler; +use MailPoet\Util\Helpers; use MailPoet\WP\Functions as WPFunctions; +use MailPoetVendor\Doctrine\DBAL\Driver\Statement; +use MailPoetVendor\Doctrine\ORM\EntityManager; use MailPoetVendor\Doctrine\ORM\Query\Expr\Join; use MailPoetVendor\Doctrine\ORM\QueryBuilder; +use function MailPoetVendor\array_column; + class SubscriberListingRepository extends ListingRepository { + const DEFAULT_SORT_BY = 'createdAt'; + private static $supportedStatuses = [ SubscriberEntity::STATUS_SUBSCRIBED, SubscriberEntity::STATUS_UNSUBSCRIBED, @@ -18,6 +27,18 @@ class SubscriberListingRepository extends ListingRepository { SubscriberEntity::STATUS_UNCONFIRMED, ]; + /** @var FilterHandler */ + private $dynamicSegmentsFilter; + + /** @var EntityManager */ + private $entityManager; + + public function __construct(EntityManager $entityManager, FilterHandler $dynamicSegmentsFilter) { + parent::__construct($entityManager); + $this->dynamicSegmentsFilter = $dynamicSegmentsFilter; + $this->entityManager = $entityManager; + } + protected function applySelectClause(QueryBuilder $queryBuilder) { $queryBuilder->select("PARTIAL s.{id,email,firstName,lastName,status,createdAt,countConfirmations,wpUserId,isWoocommerceUser}"); } @@ -44,14 +65,32 @@ class SubscriberListingRepository extends ListingRepository { } protected function applySearch(QueryBuilder $queryBuilder, string $search) { - $search = str_replace(['\\', '%', '_'], ['\\\\', '\\%', '\\_'], trim($search)); // escape for 'LIKE' + $search = $this->sanitizeSearch($search); $queryBuilder ->andWhere('s.email LIKE :search or s.firstName LIKE :search or s.lastName LIKE :search') ->setParameter('search', "%$search%"); } protected function applyFilters(QueryBuilder $queryBuilder, array $filters) { - // this is done in a different level + // Filtering for subscribers is handled in self::applyDefinition + // because other parameters are needed for performance reasons + } + + protected function applyListingDefinition(QueryBuilder $queryBuilder, ListingDefinition $definition) { + $filters = $definition->getFilters(); + if (!$filters || !isset($filters['segment'])) { + return; + } + $segment = $this->entityManager->find(SegmentEntity::class, (int)$filters['segment']); + if (!$segment instanceof SegmentEntity) { + return; + } + if ($segment->isStatic()) { + $queryBuilder->join('s.subscriberSegments', 'ss', Join::WITH, 'ss.segment = :ssSegment') + ->setParameter('ssSegment', $segment->getId()); + return; + } + $this->applyDynamicSegmentsFilter($queryBuilder, $definition, $segment); } protected function applyParameters(QueryBuilder $queryBuilder, array $parameters) { @@ -59,6 +98,9 @@ class SubscriberListingRepository extends ListingRepository { } protected function applySorting(QueryBuilder $queryBuilder, string $sortBy, string $sortOrder) { + if (!$sortBy) { + $sortBy = self::DEFAULT_SORT_BY; + } $queryBuilder->addOrderBy("s.$sortBy", $sortOrder); } @@ -183,4 +225,66 @@ class SubscriberListingRepository extends ListingRepository { } return ['segment' => $segmentList]; } + + private function sanitizeSearch(string $search): string { + return str_replace(['\\', '%', '_'], ['\\\\', '\\%', '\\_'], trim($search)); // escape for 'LIKE' + } + + private function applyDynamicSegmentsFilter( + QueryBuilder $queryBuilder, + ListingDefinition $definition, + SegmentEntity $segment + ) { + $subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName(); + $subscribersIdsQuery = $this->entityManager + ->getConnection() + ->createQueryBuilder() + ->select("DISTINCT $subscribersTable.id") + ->from($subscribersTable); + + // Apply dynamic segments filters + foreach ($segment->getDynamicFilters() as $filter) { + $subscribersIdsQuery = $this->dynamicSegmentsFilter->apply($subscribersIdsQuery, $filter); + } + + // Apply group, search, order and paging to fetch only necessary ids + // This id done for performance reasons instead of fetching all IDs in dynamic segment + if ($definition->getSearch()) { + $search = $this->sanitizeSearch((string)$definition->getSearch()); + $subscribersIdsQuery + ->andWhere("$subscribersTable.email LIKE :search or $subscribersTable.first_name LIKE :search or $subscribersTable.last_name LIKE :search") + ->setParameter('search', "%$search%"); + } + if ($definition->getGroup()) { + if ($definition->getGroup() === 'trash') { + $subscribersIdsQuery->andWhere("$subscribersTable.deleted_at IS NOT NULL"); + } else { + $subscribersIdsQuery->andWhere("$subscribersTable.deleted_at IS NULL"); + } + if (in_array($definition->getGroup(), self::$supportedStatuses)) { + $subscribersIdsQuery + ->andWhere("$subscribersTable.status = :status") + ->setParameter('status', $definition->getGroup()); + } + } + $sortBy = $definition->getSortBy() ?: self::DEFAULT_SORT_BY; + $subscribersIdsQuery->orderBy("$subscribersTable." . Helpers::camelCaseToUnderscore($sortBy), $definition->getSortOrder()); + $subscribersIdsQuery->setFirstResult($definition->getOffset()); + $subscribersIdsQuery->setMaxResults($definition->getLimit()); + + $idsStatement = $subscribersIdsQuery->execute(); + // This shouldn't happen because execute on select SQL always returns Statement, but PHPStan doesn't know that + if (!$idsStatement instanceof Statement) { + $queryBuilder->andWhere('0 = 1'); + return; + } + $result = $idsStatement->fetchAll(); + $ids = array_column($result, 'id'); + if (count($ids)) { + $queryBuilder->andWhere('s.id IN (:subscriberIds)') + ->setParameter('subscriberIds', $ids); + } else { + $queryBuilder->andWhere('0 = 1'); // Don't return any subscribers if no ids found + } + } } diff --git a/tests/integration/Subscribers/SubscriberListingRepositoryTest.php b/tests/integration/Subscribers/SubscriberListingRepositoryTest.php index 73fa2ff11b..242a8b27c9 100644 --- a/tests/integration/Subscribers/SubscriberListingRepositoryTest.php +++ b/tests/integration/Subscribers/SubscriberListingRepositoryTest.php @@ -2,10 +2,14 @@ namespace MailPoet\Subscribers; +use MailPoet\Entities\DynamicSegmentFilterEntity; use MailPoet\Entities\SegmentEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Entities\SubscriberSegmentEntity; use MailPoet\Listing\ListingDefinition; +use MailPoet\Segments\DynamicSegments\FilterHandler; + +require_once(ABSPATH . 'wp-admin/includes/user.php'); class SubscriberListingRepositoryTest extends \MailPoetTest { @@ -27,10 +31,11 @@ class SubscriberListingRepositoryTest extends \MailPoetTest { ]; public function _before() { - $this->repository = new SubscriberListingRepository($this->entityManager); - $this->truncateEntity(SegmentEntity::class); - $this->truncateEntity(SubscriberEntity::class); - $this->truncateEntity(SubscriberSegmentEntity::class); + $this->repository = new SubscriberListingRepository( + $this->entityManager, + $this->diContainer->get(FilterHandler::class) + ); + $this->cleanup(); } public function testItBuildsFilters() { @@ -116,6 +121,79 @@ class SubscriberListingRepositoryTest extends \MailPoetTest { expect($groups['6']['count'])->equals(1); } + public function testLoadAllSubscribers() { + $this->createSubscriberEntity(); // subscriber without a list + + $list = $this->createSegmentEntity(); + $subscriberUnsubscribedFromAList = $this->createSubscriberEntity(); + $subscriberSegment = $this->createSubscriberSegmentEntity($list, $subscriberUnsubscribedFromAList); + $subscriberSegment->setStatus(SubscriberEntity::STATUS_UNSUBSCRIBED); + + $regularSubscriber = $this->createSubscriberEntity(); + $regularSubscriber->setStatus(SubscriberEntity::STATUS_SUBSCRIBED); + $this->createSubscriberSegmentEntity($list, $regularSubscriber); + + $unsubscribed = $this->createSubscriberEntity(); + $unsubscribed->setStatus(SubscriberEntity::STATUS_UNSUBSCRIBED); + + $unconfirmed = $this->createSubscriberEntity(); + $unconfirmed->setStatus(SubscriberEntity::STATUS_UNCONFIRMED); + + $inactive = $this->createSubscriberEntity(); + $inactive->setStatus(SubscriberEntity::STATUS_INACTIVE); + + $bounced = $this->createSubscriberEntity(); + $bounced->setStatus(SubscriberEntity::STATUS_BOUNCED); + + $this->entityManager->flush(); + + $data = $this->repository->getData($this->getListingDefinition()); + expect(count($data))->equals(7); + } + + public function testLoadSubscribersInDefaultSegment() { + $list = $this->createSegmentEntity(); + $subscriberUnsubscribedFromAList = $this->createSubscriberEntity(); + $subscriberSegment = $this->createSubscriberSegmentEntity($list, $subscriberUnsubscribedFromAList); + $subscriberSegment->setStatus(SubscriberEntity::STATUS_UNSUBSCRIBED); + + $this->createSubscriberEntity(); // subscriber without a list + + $regularSubscriber = $this->createSubscriberEntity(); + $regularSubscriber->setStatus(SubscriberEntity::STATUS_SUBSCRIBED); + $this->createSubscriberSegmentEntity($list, $regularSubscriber); + + $this->entityManager->flush(); + + $this->listingData['filter'] = ['segment' => $list->getId()]; + $data = $this->repository->getData($this->getListingDefinition()); + expect(count($data))->equals(2); + expect($data[0]->getEmail())->equals($subscriberUnsubscribedFromAList->getEmail()); + expect($data[1]->getEmail())->equals($regularSubscriber->getEmail()); + } + + public function testLoadSubscribersInDynamicSegment() { + $wpUserEmail = 'user-role-test1@example.com'; + $this->cleanupWpUser($wpUserEmail); + wp_insert_user([ + 'user_login' => 'user-role-test1', + 'user_email' => $wpUserEmail, + 'role' => 'editor', + 'user_pass' => '12123154', + ]); + + $list = $this->createDynamicSegmentEntity(); + + $this->createSubscriberEntity(); // subscriber without a list + $this->entityManager->flush(); + + $this->listingData['filter'] = ['segment' => $list->getId()]; + $data = $this->repository->getData($this->getListingDefinition()); + expect(count($data))->equals(1); + expect($data[0]->getEmail())->equals($wpUserEmail); + $this->cleanupWpUser($wpUserEmail); + } + private function createSubscriberEntity(): SubscriberEntity { $subscriber = new SubscriberEntity(); $rand = rand(0, 100000); @@ -134,12 +212,38 @@ class SubscriberListingRepositoryTest extends \MailPoetTest { return $segment; } + private function createDynamicSegmentEntity(): SegmentEntity { + $segment = new SegmentEntity('Segment' . rand(0, 10000), SegmentEntity::TYPE_DYNAMIC, 'Segment description'); + $dynamicFilter = new DynamicSegmentFilterEntity($segment, [ + 'wordpressRole' => 'editor', + 'segmentType' => DynamicSegmentFilterEntity::TYPE_USER_ROLE, + ]); + $segment->getDynamicFilters()->add($dynamicFilter); + $this->entityManager->persist($segment); + $this->entityManager->persist($dynamicFilter); + return $segment; + } + private function createSubscriberSegmentEntity(SegmentEntity $segment, SubscriberEntity $subscriber): SubscriberSegmentEntity { $subscriberSegment = new SubscriberSegmentEntity($segment, $subscriber, SubscriberEntity::STATUS_SUBSCRIBED); $this->entityManager->persist($subscriberSegment); return $subscriberSegment; } + private function cleanupWpUser(string $email) { + $user = get_user_by('email', $email); + if ($user) { + wp_delete_user($user->ID); + } + } + + private function cleanup() { + $this->truncateEntity(SegmentEntity::class); + $this->truncateEntity(SubscriberEntity::class); + $this->truncateEntity(SubscriberSegmentEntity::class); + $this->truncateEntity(DynamicSegmentFilterEntity::class); + } + private function getListingDefinition(): ListingDefinition { return new ListingDefinition( $this->listingData['group'], @@ -153,4 +257,8 @@ class SubscriberListingRepositoryTest extends \MailPoetTest { $this->listingData['selection'] ); } + + public function _after() { + $this->cleanup(); + } }