Simplify the query to calculate lifetime emails
Also rename email_counts to email_count and update tests [MAILPOET-4177]
This commit is contained in:
@@ -249,7 +249,7 @@ class Migrator {
|
||||
'engagement_score_updated_at timestamp NULL,',
|
||||
'last_engagement_at timestamp NULL,',
|
||||
'woocommerce_synced_at timestamp NULL,',
|
||||
'emails_count int(11) unsigned NOT NULL DEFAULT 0, ',
|
||||
'email_count int(11) unsigned NOT NULL DEFAULT 0, ',
|
||||
'PRIMARY KEY (id),',
|
||||
'UNIQUE KEY email (email),',
|
||||
'UNIQUE KEY unsubscribe_token (unsubscribe_token),',
|
||||
|
@@ -154,7 +154,7 @@ class SubscriberEntity {
|
||||
* @ORM\Column(type="integer")
|
||||
* @var int
|
||||
*/
|
||||
private $emailsCount = 0;
|
||||
private $emailCount = 0;
|
||||
|
||||
/**
|
||||
* @ORM\OneToMany(targetEntity="MailPoet\Entities\SubscriberSegmentEntity", mappedBy="subscriber", orphanRemoval=true)
|
||||
@@ -476,12 +476,12 @@ class SubscriberEntity {
|
||||
return $this->woocommerceSyncedAt;
|
||||
}
|
||||
|
||||
public function getEmailsCount(): int {
|
||||
return $this->emailsCount;
|
||||
public function getEmailCount(): int {
|
||||
return $this->emailCount;
|
||||
}
|
||||
|
||||
public function setEmailsCount(int $emailsCount): void {
|
||||
$this->emailsCount = $emailsCount;
|
||||
public function setEmailCount(int $emailCount): void {
|
||||
$this->emailCount = $emailCount;
|
||||
}
|
||||
|
||||
/** @ORM\PreFlush */
|
||||
|
@@ -113,7 +113,7 @@ class InactiveSubscribersController {
|
||||
AND s.status = :status
|
||||
AND s.id >= :startId
|
||||
AND s.id < :endId
|
||||
AND s.emails_count >= {$lifetimeEmailsThreshold}
|
||||
AND s.email_count >= {$lifetimeEmailsThreshold}
|
||||
GROUP BY s.id
|
||||
HAVING count(s.id) >= :unopenedEmailsThreshold
|
||||
",
|
||||
|
@@ -4,14 +4,11 @@ namespace MailPoet\Subscribers;
|
||||
|
||||
use MailPoet\Entities\ScheduledTaskEntity;
|
||||
use MailPoet\Entities\ScheduledTaskSubscriberEntity;
|
||||
use MailPoet\Entities\SendingQueueEntity;
|
||||
use MailPoet\Entities\SubscriberEntity;
|
||||
use MailPoetVendor\Carbon\Carbon;
|
||||
use MailPoetVendor\Doctrine\ORM\EntityManager;
|
||||
|
||||
class SubscribersEmailCountsController {
|
||||
private $processedTaskIdsTableCreated = false;
|
||||
|
||||
/** @var EntityManager */
|
||||
private $entityManager;
|
||||
|
||||
@@ -26,7 +23,6 @@ class SubscribersEmailCountsController {
|
||||
}
|
||||
|
||||
public function updateSubscribersEmailCounts(?\DateTimeInterface $dateLastProcessed, int $batchSize, ?int $startId = null): array {
|
||||
$sendingQueuesTable = $this->entityManager->getClassMetadata(SendingQueueEntity::class)->getTableName();
|
||||
$scheduledTasksTable = $this->entityManager->getClassMetadata(ScheduledTaskEntity::class)->getTableName();
|
||||
$scheduledTaskSubscribersTable = $this->entityManager->getClassMetadata(ScheduledTaskSubscriberEntity::class)->getTableName();
|
||||
|
||||
@@ -42,66 +38,39 @@ class SubscribersEmailCountsController {
|
||||
return [0, 0];
|
||||
}
|
||||
|
||||
// Temporary table with processed tasks from threshold date up to yesterday
|
||||
$processedTaskIdsTable = 'processed_task_ids';
|
||||
if (!$this->processedTaskIdsTableCreated) {
|
||||
$queryParams = [];
|
||||
$processedTaskIdsTableSql = "
|
||||
CREATE TEMPORARY TABLE IF NOT EXISTS {$processedTaskIdsTable}
|
||||
(INDEX task_id_ids (id))
|
||||
SELECT DISTINCT task_id as id FROM {$sendingQueuesTable} as sq
|
||||
JOIN {$scheduledTasksTable} as st ON sq.task_id = st.id
|
||||
WHERE st.processed_at IS NOT NULL
|
||||
AND st.processed_at < :dayAgo";
|
||||
$queryParams['dayAgo'] = $dayAgoIso;
|
||||
|
||||
$queryParams = [
|
||||
'startId' => $startId,
|
||||
'endId' => $endId,
|
||||
'dayAgo' => $dayAgoIso,
|
||||
];
|
||||
if ($dateLastProcessed) {
|
||||
$processedTaskIdsTableSql .= " AND st.processed_at >= :dateFrom";
|
||||
$carbonDateLastProcessed = Carbon::createFromTimestamp($dateLastProcessed->getTimestamp());
|
||||
$dateFromIso = ($carbonDateLastProcessed->subDay())->toDateTimeString();
|
||||
$queryParams['dateFrom'] = $dateFromIso;
|
||||
}
|
||||
|
||||
$resultQuery = $connection->executeQuery($processedTaskIdsTableSql, $queryParams);
|
||||
$this->processedTaskIdsTableCreated = true;
|
||||
|
||||
if ($resultQuery->rowCount() === 0) return [0,0];
|
||||
}
|
||||
|
||||
// Temporary table needed for UPDATE query
|
||||
// mySQL does not allow to modify the same table used in the select
|
||||
$subscriberIdsEmailsCountTmpTable = 'subscribers_ids_email_counts';
|
||||
$connection->executeQuery("
|
||||
CREATE TEMPORARY TABLE IF NOT EXISTS {$subscriberIdsEmailsCountTmpTable}
|
||||
(UNIQUE subscriber_id (id))
|
||||
SELECT s.id, count(task_ids.id) as emails_count from {$this->subscribersTable} s
|
||||
JOIN {$scheduledTaskSubscribersTable} sts ON s.id = sts.subscriber_id
|
||||
JOIN {$processedTaskIdsTable} task_ids ON task_ids.id = sts.task_id
|
||||
WHERE s.id >= :startId
|
||||
AND s.id <= :endId
|
||||
GROUP BY s.id
|
||||
",
|
||||
[
|
||||
'startId' => $startId,
|
||||
'endId' => $endId,
|
||||
]
|
||||
);
|
||||
|
||||
// If $dateLastProcessed provided, increment value, otherwise count all and reset value
|
||||
$initUpdateValue = $dateLastProcessed ? 's.emails_count' : '';
|
||||
$updateQuery = $connection->executeQuery("
|
||||
$initUpdateValue = $dateLastProcessed ? 's.email_count' : '';
|
||||
$dateLastProcessedSql = $dateLastProcessed ? ' AND st.processed_at >= :dateFrom' : '';
|
||||
|
||||
$connection->executeQuery("
|
||||
UPDATE {$this->subscribersTable} as s
|
||||
JOIN {$subscriberIdsEmailsCountTmpTable} as sc ON s.id = sc.id
|
||||
SET s.emails_count = {$initUpdateValue} + IFNULL(sc.emails_count, 0)
|
||||
JOIN (
|
||||
SELECT s.id, COUNT(st.id) as email_count
|
||||
FROM {$this->subscribersTable} as s
|
||||
JOIN {$scheduledTaskSubscribersTable} as sts ON s.id = sts.subscriber_id
|
||||
JOIN {$scheduledTasksTable} as st ON st.id = sts.task_id
|
||||
WHERE s.id >= :startId
|
||||
AND s.id <= :endId
|
||||
AND st.type = 'sending'
|
||||
AND st.processed_at IS NOT NULL
|
||||
AND st.processed_at < :dayAgo
|
||||
{$dateLastProcessedSql}
|
||||
GROUP BY s.id
|
||||
) counts ON counts.id = s.id
|
||||
SET s.email_count = {$initUpdateValue} + IFNULL(counts.email_count, 0)
|
||||
",
|
||||
[
|
||||
'startId' => $startId,
|
||||
'endId' => $endId,
|
||||
]
|
||||
$queryParams
|
||||
);
|
||||
$connection->executeQuery("DROP TABLE {$subscriberIdsEmailsCountTmpTable}");
|
||||
|
||||
return [$countSubscribersToUpdate, $endId];
|
||||
}
|
||||
|
@@ -62,10 +62,10 @@ class SubscribersLifetimeEmailCountTest extends \MailPoetTest {
|
||||
$this->entityManager->clear();
|
||||
$subscriber1 = $this->subscribersRepository->findOneById($subscriber1->getId());
|
||||
assert($subscriber1 instanceof SubscriberEntity);
|
||||
expect($subscriber1->getEmailsCount())->equals(80);
|
||||
expect($subscriber1->getEmailCount())->equals(80);
|
||||
$subscriber2 = $this->subscribersRepository->findOneById($subscriber2->getId());
|
||||
assert($subscriber2 instanceof SubscriberEntity);
|
||||
expect($subscriber2->getEmailsCount())->equals(8);
|
||||
expect($subscriber2->getEmailCount())->equals(8);
|
||||
}
|
||||
|
||||
public function testItUpdatesSubscribersEmailCountsAfterFirstRun() {
|
||||
@@ -88,7 +88,7 @@ class SubscribersLifetimeEmailCountTest extends \MailPoetTest {
|
||||
$this->entityManager->clear();
|
||||
$subscriber1 = $this->subscribersRepository->findOneById($subscriber1->getId());
|
||||
assert($subscriber1 instanceof SubscriberEntity);
|
||||
expect($subscriber1->getEmailsCount())->equals(81);
|
||||
expect($subscriber1->getEmailCount())->equals(81);
|
||||
|
||||
}
|
||||
|
||||
@@ -116,14 +116,14 @@ class SubscribersLifetimeEmailCountTest extends \MailPoetTest {
|
||||
string $email,
|
||||
int $createdDaysAgo = 0,
|
||||
string $status = SubscriberEntity::STATUS_SUBSCRIBED,
|
||||
int $emailCounts = 0
|
||||
int $emailCount = 0
|
||||
): SubscriberEntity {
|
||||
$createdAt = (new Carbon())->subDays($createdDaysAgo);
|
||||
$subscriber = new SubscriberEntity();
|
||||
$subscriber->setEmail($email);
|
||||
$subscriber->setStatus($status);
|
||||
$subscriber->setCreatedAt($createdAt);
|
||||
$subscriber->setEmailsCount($emailCounts);
|
||||
$subscriber->setEmailCount($emailCount);
|
||||
$this->entityManager->persist($subscriber);
|
||||
// we need to set lastSubscribeAt after persist due to LastSubscribedAtListener
|
||||
$subscriber->setLastSubscribedAt($createdAt);
|
||||
|
@@ -271,12 +271,12 @@ class InactiveSubscribersControllerTest extends \MailPoetTest {
|
||||
string $email,
|
||||
int $createdDaysAgo = 0,
|
||||
string $status = SubscriberEntity::STATUS_SUBSCRIBED,
|
||||
int $emailsCount = InactiveSubscribersController::LIFETIME_EMAILS_THRESHOLD
|
||||
int $emailCount = InactiveSubscribersController::LIFETIME_EMAILS_THRESHOLD
|
||||
): SubscriberEntity {
|
||||
$createdAt = (new Carbon())->subDays($createdDaysAgo);
|
||||
$subscriber = new SubscriberEntity();
|
||||
$subscriber->setEmail($email);
|
||||
$subscriber->setEmailsCount($emailsCount);
|
||||
$subscriber->setEmailCount($emailCount);
|
||||
$subscriber->setStatus($status);
|
||||
$subscriber->setCreatedAt($createdAt);
|
||||
$this->entityManager->persist($subscriber);
|
||||
|
@@ -60,9 +60,9 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
assert($subscriber2 instanceof SubscriberEntity);
|
||||
assert($subscriber3 instanceof SubscriberEntity);
|
||||
|
||||
expect($subscriber1->getEmailsCount())->equals(80);
|
||||
expect($subscriber2->getEmailsCount())->equals(8);
|
||||
expect($subscriber3->getEmailsCount())->equals(0);
|
||||
expect($subscriber1->getEmailCount())->equals(80);
|
||||
expect($subscriber2->getEmailCount())->equals(8);
|
||||
expect($subscriber3->getEmailCount())->equals(0);
|
||||
}
|
||||
|
||||
public function testItIncrementsSubscribersEmailCountsWhenDateProvided(): void {
|
||||
@@ -89,9 +89,9 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
assert($subscriber2 instanceof SubscriberEntity);
|
||||
assert($subscriber3 instanceof SubscriberEntity);
|
||||
|
||||
expect($subscriber1->getEmailsCount())->equals(81);
|
||||
expect($subscriber2->getEmailsCount())->equals(9);
|
||||
expect($subscriber3->getEmailsCount())->equals(1);
|
||||
expect($subscriber1->getEmailCount())->equals(81);
|
||||
expect($subscriber2->getEmailCount())->equals(9);
|
||||
expect($subscriber3->getEmailCount())->equals(1);
|
||||
|
||||
}
|
||||
|
||||
@@ -117,9 +117,9 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
assert($subscriber2 instanceof SubscriberEntity);
|
||||
assert($subscriber3 instanceof SubscriberEntity);
|
||||
|
||||
expect($subscriber1->getEmailsCount())->equals(80);
|
||||
expect($subscriber2->getEmailsCount())->equals(8);
|
||||
expect($subscriber3->getEmailsCount())->equals(1);
|
||||
expect($subscriber1->getEmailCount())->equals(80);
|
||||
expect($subscriber2->getEmailCount())->equals(8);
|
||||
expect($subscriber3->getEmailCount())->equals(1);
|
||||
}
|
||||
|
||||
public function testItUpdatesOnlySubscribersInBatch() {
|
||||
@@ -145,13 +145,13 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
assert($subscriber2 instanceof SubscriberEntity);
|
||||
assert($subscriber3 instanceof SubscriberEntity);
|
||||
|
||||
expect($subscriber1->getEmailsCount())->equals(80);
|
||||
expect($subscriber2->getEmailsCount())->equals(8);
|
||||
expect($subscriber1->getEmailCount())->equals(80);
|
||||
expect($subscriber2->getEmailCount())->equals(8);
|
||||
// Subscriber not in batch should not be updated
|
||||
expect($subscriber3->getEmailsCount())->equals(0);
|
||||
expect($subscriber3->getEmailCount())->equals(0);
|
||||
}
|
||||
|
||||
public function testItDoesNotCountIfThereAreNoSubscribersOrTasksToUpdate() {
|
||||
public function testItDoesNotCountIfThereAreNoSubscribersToUpdate() {
|
||||
// Subscribers empty table
|
||||
[$count, $maxSubscriberId] = $this->controller->updateSubscribersEmailCounts(null, 1);
|
||||
expect($count)->equals(0);
|
||||
@@ -160,10 +160,6 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
$subscriber2 = $this->createSubscriber('s2@email.com', 20);
|
||||
$subscriber3 = $this->createSubscriber('s3@email.com', 10);
|
||||
|
||||
// Tasks empty table
|
||||
[$count, $maxSubscriberId] = $this->controller->updateSubscribersEmailCounts(null, 1);
|
||||
expect($count)->equals(0);
|
||||
|
||||
$this->createCompletedSendingTasksForSubscriber($subscriber1, 80, 90);
|
||||
$this->createCompletedSendingTasksForSubscriber($subscriber2, 8, 3);
|
||||
$this->createCompletedSendingTasksForSubscriber($subscriber3, 1, 4);
|
||||
@@ -180,9 +176,9 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
assert($subscriber2 instanceof SubscriberEntity);
|
||||
assert($subscriber3 instanceof SubscriberEntity);
|
||||
|
||||
expect($subscriber1->getEmailsCount())->equals(0);
|
||||
expect($subscriber2->getEmailsCount())->equals(0);
|
||||
expect($subscriber3->getEmailsCount())->equals(0);
|
||||
expect($subscriber1->getEmailCount())->equals(0);
|
||||
expect($subscriber2->getEmailCount())->equals(0);
|
||||
expect($subscriber3->getEmailCount())->equals(0);
|
||||
}
|
||||
|
||||
private function createCompletedSendingTasksForSubscriber(SubscriberEntity $subscriber, int $numTasks = 1, int $processedDaysAgo = 0): void {
|
||||
@@ -233,7 +229,7 @@ class SubscribersEmailCountsControllerTest extends \MailPoetTest {
|
||||
$subscriber->setEmail($email);
|
||||
$subscriber->setStatus($status);
|
||||
$subscriber->setCreatedAt($createdAt);
|
||||
$subscriber->setEmailsCount($emailCounts);
|
||||
$subscriber->setEmailCount($emailCounts);
|
||||
$this->entityManager->persist($subscriber);
|
||||
// we need to set lastSubscribeAt after persist due to LastSubscribedAtListener
|
||||
$subscriber->setLastSubscribedAt($createdAt);
|
||||
|
Reference in New Issue
Block a user