From fe66e31b5f57784a7f381c3ab3b22b9d1969e01b Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Thu, 1 Aug 2024 13:45:33 +0200 Subject: [PATCH] Add check for orphaned subscriptions [MAILPOET-1587] --- .../js/src/help/data-inconsistencies.tsx | 1 + .../DataInconsistencyController.php | 5 +++ .../DataInconsistencyRepository.php | 28 +++++++++++++++++ .../DataInconsistencyRepositoryTest.php | 31 +++++++++++++++++++ 4 files changed, 65 insertions(+) diff --git a/mailpoet/assets/js/src/help/data-inconsistencies.tsx b/mailpoet/assets/js/src/help/data-inconsistencies.tsx index eb529f1d1d..0840e887a0 100644 --- a/mailpoet/assets/js/src/help/data-inconsistencies.tsx +++ b/mailpoet/assets/js/src/help/data-inconsistencies.tsx @@ -27,6 +27,7 @@ export function DataInconsistencies({ dataInconsistencies }: Props) { 'Sending Queues without Newsletter', 'mailpoet', ), + orphaned_subscriptions: __('Orphaned Subscriptions', 'mailpoet'), }), [], ); diff --git a/mailpoet/lib/Util/DataInconsistency/DataInconsistencyController.php b/mailpoet/lib/Util/DataInconsistency/DataInconsistencyController.php index e13135ab5b..d8e8cfa61f 100644 --- a/mailpoet/lib/Util/DataInconsistency/DataInconsistencyController.php +++ b/mailpoet/lib/Util/DataInconsistency/DataInconsistencyController.php @@ -8,11 +8,13 @@ class DataInconsistencyController { const ORPHANED_SENDING_TASKS = 'orphaned_sending_tasks'; const ORPHANED_SENDING_TASK_SUBSCRIBERS = 'orphaned_sending_task_subscribers'; const SENDING_QUEUE_WITHOUT_NEWSLETTER = 'sending_queue_without_newsletter'; + const ORPHANED_SUBSCRIPTIONS = 'orphaned_subscriptions'; const SUPPORTED_INCONSISTENCY_CHECKS = [ self::ORPHANED_SENDING_TASKS, self::ORPHANED_SENDING_TASK_SUBSCRIBERS, self::SENDING_QUEUE_WITHOUT_NEWSLETTER, + self::ORPHANED_SUBSCRIPTIONS, ]; private DataInconsistencyRepository $repository; @@ -28,6 +30,7 @@ class DataInconsistencyController { self::ORPHANED_SENDING_TASKS => $this->repository->getOrphanedSendingTasksCount(), self::ORPHANED_SENDING_TASK_SUBSCRIBERS => $this->repository->getOrphanedScheduledTasksSubscribersCount(), self::SENDING_QUEUE_WITHOUT_NEWSLETTER => $this->repository->getSendingQueuesWithoutNewsletterCount(), + self::ORPHANED_SUBSCRIPTIONS => $this->repository->getOrphanedSubscriptionsCount(), ]; $result['total'] = array_sum($result); return $result; @@ -43,6 +46,8 @@ class DataInconsistencyController { $this->repository->cleanupOrphanedScheduledTaskSubscribers(); } elseif ($inconsistency === self::SENDING_QUEUE_WITHOUT_NEWSLETTER) { $this->repository->cleanupSendingQueuesWithoutNewsletter(); + } elseif ($inconsistency === self::ORPHANED_SUBSCRIPTIONS) { + $this->repository->cleanupOrphanedSubscriptions(); } } } diff --git a/mailpoet/lib/Util/DataInconsistency/DataInconsistencyRepository.php b/mailpoet/lib/Util/DataInconsistency/DataInconsistencyRepository.php index 6cd9f38a21..a97a43ad79 100644 --- a/mailpoet/lib/Util/DataInconsistency/DataInconsistencyRepository.php +++ b/mailpoet/lib/Util/DataInconsistency/DataInconsistencyRepository.php @@ -6,7 +6,10 @@ use MailPoet\Cron\Workers\SendingQueue\SendingQueue; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\ScheduledTaskSubscriberEntity; +use MailPoet\Entities\SegmentEntity; use MailPoet\Entities\SendingQueueEntity; +use MailPoet\Entities\SubscriberEntity; +use MailPoet\Entities\SubscriberSegmentEntity; use MailPoetVendor\Doctrine\ORM\EntityManager; use MailPoetVendor\Doctrine\ORM\Query; use MailPoetVendor\Doctrine\ORM\QueryBuilder; @@ -48,6 +51,19 @@ class DataInconsistencyRepository { return intval($count); } + public function getOrphanedSubscriptionsCount(): int { + $subscriberTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName(); + $segmentTable = $this->entityManager->getClassMetadata(SegmentEntity::class)->getTableName(); + $subscriberSegmentTable = $this->entityManager->getClassMetadata(SubscriberSegmentEntity::class)->getTableName(); + $count = $this->entityManager->getConnection()->executeQuery(" + SELECT count(distinct ss.`id`) FROM $subscriberSegmentTable ss + LEFT JOIN $segmentTable seg ON seg.`id` = ss.`segment_id` + LEFT JOIN $subscriberTable sub ON sub.`id` = ss.`subscriber_id` + WHERE seg.`id` IS NULL OR sub.`id` IS NULL + ")->fetchOne(); + return intval($count); + } + public function cleanupOrphanedSendingTasks(): int { $ids = $this->buildOrphanedSendingTasksQuery( $this->entityManager->createQueryBuilder() @@ -100,6 +116,18 @@ class DataInconsistencyRepository { return $deletedQueuesCount; } + public function cleanupOrphanedSubscriptions(): int { + $subscriberTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName(); + $segmentTable = $this->entityManager->getClassMetadata(SegmentEntity::class)->getTableName(); + $subscriberSegmentTable = $this->entityManager->getClassMetadata(SubscriberSegmentEntity::class)->getTableName(); + return (int)$this->entityManager->getConnection()->executeStatement(" + DELETE ss FROM $subscriberSegmentTable ss + LEFT JOIN $segmentTable seg ON seg.`id` = ss.`segment_id` + LEFT JOIN $subscriberTable sub ON sub.`id` = ss.`subscriber_id` + WHERE seg.`id` IS NULL OR sub.`id` IS NULL + "); + } + private function buildOrphanedSendingTasksQuery(QueryBuilder $queryBuilder): Query { return $queryBuilder ->from(ScheduledTaskEntity::class, 'st') diff --git a/mailpoet/tests/integration/Util/DataInconsistency/DataInconsistencyRepositoryTest.php b/mailpoet/tests/integration/Util/DataInconsistency/DataInconsistencyRepositoryTest.php index fe3e32d351..543daf76ba 100644 --- a/mailpoet/tests/integration/Util/DataInconsistency/DataInconsistencyRepositoryTest.php +++ b/mailpoet/tests/integration/Util/DataInconsistency/DataInconsistencyRepositoryTest.php @@ -5,10 +5,12 @@ namespace MailPoet\Util\DataInconsistency; use MailPoet\Cron\Workers\SendingQueue\SendingQueue as SendingQueueWorker; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\ScheduledTaskSubscriberEntity; +use MailPoet\Entities\SegmentEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Test\DataFactories\Newsletter; use MailPoet\Test\DataFactories\ScheduledTask; use MailPoet\Test\DataFactories\ScheduledTaskSubscriber; +use MailPoet\Test\DataFactories\Segment; use MailPoet\Test\DataFactories\SendingQueue; use MailPoet\Test\DataFactories\Subscriber; @@ -98,4 +100,33 @@ class DataInconsistencyRepositoryTest extends \MailPoetTest { verify($this->repository->getOrphanedSendingTasksCount())->equals(0); verify($this->repository->getOrphanedScheduledTasksSubscribersCount())->equals(0); } + + public function testItHandlesOrphanedSubscriptions(): void { + $segmentToDelete = (new Segment())->create(); + $segmentToKeep = (new Segment())->create(); + + $subscriberToDelete = (new Subscriber())->withSegments([$segmentToDelete, $segmentToKeep])->create(); + $subscriberToKeep = (new Subscriber())->withSegments([$segmentToDelete, $segmentToKeep])->create(); + + $subscriberTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName(); + $this->entityManager->getConnection() + ->executeQuery("DELETE s FROM $subscriberTable s WHERE id = :id", ['id' => $subscriberToDelete->getId()]); + + $segmentTable = $this->entityManager->getClassMetadata(SegmentEntity::class)->getTableName(); + $this->entityManager->getConnection() + ->executeQuery("DELETE s FROM $segmentTable s WHERE id = :id", ['id' => $segmentToDelete->getId()]); + + // Expect 3 because both subscribers were associated to the deleted segment + deleted subscriber to segment we kept + verify($this->repository->getOrphanedSubscriptionsCount())->equals(3); + $this->repository->cleanupOrphanedSubscriptions(); + verify($this->repository->getOrphanedSubscriptionsCount())->equals(0); + + $this->entityManager->detach($subscriberToKeep); + $subscriberToKeep = $this->entityManager->find(SubscriberEntity::class, $subscriberToKeep->getId()); + $this->assertInstanceOf(SubscriberEntity::class, $subscriberToKeep); + + $this->entityManager->detach($segmentToKeep); + $segmentToKeep = $this->entityManager->find(SegmentEntity::class, $segmentToKeep->getId()); + $this->assertInstanceOf(SegmentEntity::class, $segmentToKeep); + } }