From b47842c46147ba6427fbf7f5330364aae11d485f Mon Sep 17 00:00:00 2001
From: Pavel Dohnal <pavel.dohnal@automattic.com>
Date: Wed, 8 Dec 2021 14:50:26 +0100
Subject: [PATCH] Process multiple newsletters with operator ANY

[MAILPOET-3950]
---
 .../DynamicSegments/Filters/EmailAction.php   | 15 +++++++-
 .../Filters/EmailActionTest.php               | 35 +++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/lib/Segments/DynamicSegments/Filters/EmailAction.php b/lib/Segments/DynamicSegments/Filters/EmailAction.php
index 2b493bc257..2488e44ee1 100644
--- a/lib/Segments/DynamicSegments/Filters/EmailAction.php
+++ b/lib/Segments/DynamicSegments/Filters/EmailAction.php
@@ -9,6 +9,7 @@ use MailPoet\Entities\StatisticsOpenEntity;
 use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Entities\UserAgentEntity;
 use MailPoet\Util\Security;
+use MailPoetVendor\Doctrine\DBAL\Connection;
 use MailPoetVendor\Doctrine\DBAL\Query\QueryBuilder;
 use MailPoetVendor\Doctrine\ORM\EntityManager;
 
@@ -47,7 +48,12 @@ class EmailAction implements Filter {
   public function apply(QueryBuilder $queryBuilder, DynamicSegmentFilterEntity $filter): QueryBuilder {
     $filterData = $filter->getFilterData();
     $action = $filterData->getAction();
-    $newsletterId = (int)$filterData->getParam('newsletter_id');
+    $newsletterId = $filterData->getParam('newsletter_id');
+    if ($newsletterId) {
+      $newsletters = [(int)$newsletterId];
+    } else {
+      $newsletters = $filterData->getParam('newsletters');
+    }
     $linkId = $filterData->getParam('link_id') ? (int)$filterData->getParam('link_id') : null;
     $parameterSuffix = (string)($filter->getId() ?? Security::generateRandomString());
 
@@ -74,6 +80,13 @@ class EmailAction implements Filter {
         $this->createNotStatsJoinCondition($filter, $action, $linkId, $parameterSuffix)
       )->setParameter('newsletter' . $parameterSuffix, $newsletterId);
       $where .= ' AND stats.id IS NULL';
+    } elseif ($action === EmailAction::ACTION_OPENED) {
+      $queryBuilder = $queryBuilder->innerJoin(
+        $subscribersTable,
+        $statsTable,
+        'stats',
+        "stats.subscriber_id = $subscribersTable.id AND stats.newsletter_id IN (:newsletters" . $parameterSuffix . ')'
+      )->setParameter('newsletters' . $parameterSuffix, $newsletters, Connection::PARAM_INT_ARRAY);
     } else {
       $queryBuilder = $queryBuilder->innerJoin(
         $subscribersTable,
diff --git a/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php b/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php
index 95ee3fd5ee..1d164dbb4c 100644
--- a/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php
+++ b/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php
@@ -77,8 +77,9 @@ class EmailActionTest extends \MailPoetTest {
 
     $this->subscriberOpenedClicked = $this->createSubscriber('opened_clicked@example.com');
     $this->subscriberOpenedNotClicked = $this->createSubscriber('opened_not_clicked@example.com');
-    $subscriberOpenedNotClicked2 = $this->createSubscriber('opened_clicked2@example.com');
-    $subscriberOpenedNotClicked3 = $this->createSubscriber('opened_clicked3@example.com');
+    $subscriberOpenedNotClicked2 = $this->createSubscriber('opened_not_clicked2@example.com');
+    $subscriberOpenedNotClicked3 = $this->createSubscriber('opened_not_clicked3@example.com');
+    $subscriberOpenedNotClicked4 = $this->createSubscriber('opened_not_clicked4@example.com');
     $this->subscriberNotOpened = $this->createSubscriber('not_opened@example.com');
     $this->subscriberNotSent = $this->createSubscriber('not_sent@example.com');
 
@@ -86,12 +87,16 @@ class EmailActionTest extends \MailPoetTest {
     $this->createStatsNewsletter($this->subscriberOpenedNotClicked, $this->newsletter);
     $this->createStatsNewsletter($this->subscriberNotOpened, $this->newsletter);
     $this->createStatsNewsletter($subscriberOpenedNotClicked2, $this->newsletter2);
+    $this->createStatsNewsletter($subscriberOpenedNotClicked4, $this->newsletter2);
     $this->createStatsNewsletter($subscriberOpenedNotClicked3, $this->newsletter3);
+    $this->createStatsNewsletter($subscriberOpenedNotClicked4, $this->newsletter3);
 
     $this->createStatisticsOpens($this->subscriberOpenedClicked, $this->newsletter);
     $this->createStatisticsOpens($this->subscriberOpenedNotClicked, $this->newsletter);
     $this->createStatisticsOpens($subscriberOpenedNotClicked2, $this->newsletter2);
+    $this->createStatisticsOpens($subscriberOpenedNotClicked4, $this->newsletter2);
     $this->createStatisticsOpens($subscriberOpenedNotClicked3, $this->newsletter3);
+    $this->createStatisticsOpens($subscriberOpenedNotClicked4, $this->newsletter3);
 
     $this->addClickedToLink('http://example.com', $this->newsletter, $this->subscriberOpenedClicked);
   }
@@ -125,6 +130,32 @@ class EmailActionTest extends \MailPoetTest {
     expect($subscriber1->getEmail())->equals('not_opened@example.com');
   }
 
+  public function testGetOpenedOperatorAny() {
+    $segmentFilter = $this->getSegmentFilter(
+      EmailAction::ACTION_OPENED,
+      [
+        'newsletters' => [(int)$this->newsletter->getId(), (int)$this->newsletter2->getId()],
+        'operator' => DynamicSegmentFilterData::OPERATOR_ANY,
+      ]
+    );
+    $statement = $this->emailAction->apply($this->getQueryBuilder(), $segmentFilter)->execute();
+    $this->assertInstanceOf(Statement::class, $statement);
+    $result = $statement->fetchAll();
+    expect(count($result))->equals(4);
+    $subscriber1 = $this->entityManager->find(SubscriberEntity::class, $result[0]['id']);
+    $this->assertInstanceOf(SubscriberEntity::class, $subscriber1);
+    $subscriber2 = $this->entityManager->find(SubscriberEntity::class, $result[1]['id']);
+    $this->assertInstanceOf(SubscriberEntity::class, $subscriber2);
+    $subscriber3 = $this->entityManager->find(SubscriberEntity::class, $result[2]['id']);
+    $this->assertInstanceOf(SubscriberEntity::class, $subscriber3);
+    $subscriber4 = $this->entityManager->find(SubscriberEntity::class, $result[3]['id']);
+    $this->assertInstanceOf(SubscriberEntity::class, $subscriber4);
+    expect($subscriber1->getEmail())->equals('opened_clicked@example.com');
+    expect($subscriber2->getEmail())->equals('opened_not_clicked@example.com');
+    expect($subscriber3->getEmail())->equals('opened_not_clicked2@example.com');
+    expect($subscriber4->getEmail())->equals('opened_not_clicked4@example.com');
+  }
+
   public function testGetClickedWithoutLink() {
     $segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_CLICKED, [
       'newsletter_id' => (int)$this->newsletter->getId(),