diff --git a/mailpoet/lib/API/JSON/v1/Settings.php b/mailpoet/lib/API/JSON/v1/Settings.php index b851d91375..a1b65e6b48 100644 --- a/mailpoet/lib/API/JSON/v1/Settings.php +++ b/mailpoet/lib/API/JSON/v1/Settings.php @@ -6,9 +6,7 @@ use MailPoet\API\JSON\Endpoint as APIEndpoint; use MailPoet\API\JSON\Error as APIError; use MailPoet\Config\AccessControl; use MailPoet\Config\ServicesChecker; -use MailPoet\Cron\Workers\InactiveSubscribers; use MailPoet\Cron\Workers\SubscribersEngagementScore; -use MailPoet\Cron\Workers\WooCommerceSync; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Form\FormMessageController; @@ -18,6 +16,7 @@ use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Segments\SegmentsRepository; use MailPoet\Services\AuthorizedEmailsController; use MailPoet\Services\Bridge; +use MailPoet\Settings\SettingsChangeHandler; use MailPoet\Settings\SettingsController; use MailPoet\Settings\TrackingConfig; use MailPoet\Statistics\StatisticsOpensRepository; @@ -74,6 +73,9 @@ class Settings extends APIEndpoint { /** @var TrackingConfig */ private $trackingConfig; + /** @var SettingsChangeHandler */ + private $settingsChangeHandler; + public function __construct( SettingsController $settings, Bridge $bridge, @@ -87,6 +89,7 @@ class Settings extends APIEndpoint { FormMessageController $messageController, ServicesChecker $servicesChecker, SegmentsRepository $segmentsRepository, + SettingsChangeHandler $settingsChangeHandler, SubscribersCountsController $subscribersCountsController, TrackingConfig $trackingConfig ) { @@ -102,6 +105,7 @@ class Settings extends APIEndpoint { $this->scheduledTasksRepository = $scheduledTasksRepository; $this->messageController = $messageController; $this->segmentsRepository = $segmentsRepository; + $this->settingsChangeHandler = $settingsChangeHandler; $this->subscribersCountsController = $subscribersCountsController; $this->trackingConfig = $trackingConfig; } @@ -198,7 +202,7 @@ class Settings extends APIEndpoint { $oldInactivationInterval = $oldSettings['deactivate_subscriber_after_inactive_days']; $newInactivationInterval = $newSettings['deactivate_subscriber_after_inactive_days']; if ($oldInactivationInterval !== $newInactivationInterval) { - $this->onInactiveSubscribersIntervalChange(); + $this->settingsChangeHandler->onInactiveSubscribersIntervalChange(); } $oldSendingMethod = $oldSettings['mta_group']; @@ -215,7 +219,7 @@ class Settings extends APIEndpoint { ? $newSettings['mailpoet_subscribe_old_woocommerce_customers']['enabled'] : '0'; if ($oldSubscribeOldWoocommerceCustomers !== $newSubscribeOldWoocommerceCustomers) { - $this->onSubscribeOldWoocommerceCustomersChange(); + $this->settingsChangeHandler->onSubscribeOldWoocommerceCustomersChange(); } if (!empty($newSettings['woocommerce']['use_mailpoet_editor'])) { @@ -240,41 +244,6 @@ class Settings extends APIEndpoint { } } - public function onSubscribeOldWoocommerceCustomersChange(): void { - $task = $this->scheduledTasksRepository->findOneBy([ - 'type' => WooCommerceSync::TASK_TYPE, - 'status' => ScheduledTaskEntity::STATUS_SCHEDULED, - ]); - if (!($task instanceof ScheduledTaskEntity)) { - $task = $this->createScheduledTask(WooCommerceSync::TASK_TYPE); - } - $datetime = Carbon::createFromTimestamp((int)WPFunctions::get()->currentTime('timestamp')); - $task->setScheduledAt($datetime->subMinute()); - $this->scheduledTasksRepository->persist($task); - $this->scheduledTasksRepository->flush(); - } - - public function onInactiveSubscribersIntervalChange(): void { - $task = $this->scheduledTasksRepository->findOneBy([ - 'type' => InactiveSubscribers::TASK_TYPE, - 'status' => ScheduledTaskEntity::STATUS_SCHEDULED, - ]); - if (!($task instanceof ScheduledTaskEntity)) { - $task = $this->createScheduledTask(InactiveSubscribers::TASK_TYPE); - } - $datetime = Carbon::createFromTimestamp((int)WPFunctions::get()->currentTime('timestamp')); - $task->setScheduledAt($datetime->subMinute()); - $this->scheduledTasksRepository->persist($task); - $this->scheduledTasksRepository->flush(); - } - - private function createScheduledTask(string $type): ScheduledTaskEntity { - $task = new ScheduledTaskEntity(); - $task->setType($type); - $task->setStatus(ScheduledTaskEntity::STATUS_SCHEDULED); - return $task; - } - public function recalculateSubscribersCountsCache() { $segments = $this->segmentsRepository->findAll(); foreach ($segments as $segment) { diff --git a/mailpoet/lib/Config/Migrator.php b/mailpoet/lib/Config/Migrator.php index b7dd86a660..fef3eb39a7 100644 --- a/mailpoet/lib/Config/Migrator.php +++ b/mailpoet/lib/Config/Migrator.php @@ -2,8 +2,6 @@ namespace MailPoet\Config; -use MailPoet\API\JSON\v1\Settings; -use MailPoet\DI\ContainerWrapper; use MailPoet\Entities\DynamicSegmentFilterData; use MailPoet\Entities\FormEntity; use MailPoet\Models\Newsletter; @@ -13,6 +11,7 @@ use MailPoet\Segments\DynamicSegments\Filters\UserRole; use MailPoet\Segments\DynamicSegments\Filters\WooCommerceCategory; use MailPoet\Segments\DynamicSegments\Filters\WooCommerceProduct; use MailPoet\Segments\DynamicSegments\Filters\WooCommerceSubscription; +use MailPoet\Settings\SettingsChangeHandler; use MailPoet\Settings\SettingsController; use MailPoet\Util\Helpers; @@ -28,10 +27,15 @@ class Migrator { /** @var SettingsController */ private $settings; + /** @var SettingsChangeHandler */ + private $settingsChangeHandler; + public function __construct( - SettingsController $settings + SettingsController $settings, + SettingsChangeHandler $settingsChangeHandler ) { $this->settings = $settings; + $this->settingsChangeHandler = $settingsChangeHandler; $this->prefix = Env::$dbPrefix; $this->charsetCollate = Env::$dbCharsetCollate; $this->models = [ @@ -950,17 +954,7 @@ class Migrator { $currentValue = (int)$this->settings->get('deactivate_subscriber_after_inactive_days'); if ($currentValue === 180) { $this->settings->set('deactivate_subscriber_after_inactive_days', 365); - - /** - * Ensure that inactive subscribers are recalculated as soon after the setting change as possible. - * - * This behavior is normally triggered when the setting is changed through the JSON API, i.e. when a user has - * changed the setting themselves through the UI. The `onInactiveSubscribersIntervalChange` method doesn't - * depend on any state, and calling it directly means avoiding code duplication. - */ - $diContainer = ContainerWrapper::getInstance(); - $apiSettings = $diContainer->get(Settings::class); - $apiSettings->onInactiveSubscribersIntervalChange(); + $this->settingsChangeHandler->onInactiveSubscribersIntervalChange(); } return true; diff --git a/mailpoet/lib/DI/ContainerConfigurator.php b/mailpoet/lib/DI/ContainerConfigurator.php index a475ecc1b2..756885a8f8 100644 --- a/mailpoet/lib/DI/ContainerConfigurator.php +++ b/mailpoet/lib/DI/ContainerConfigurator.php @@ -326,6 +326,7 @@ class ContainerConfigurator implements IContainerConfigurator { $container->autowire(\MailPoet\Tasks\State::class); // Settings $container->autowire(\MailPoet\Settings\SettingsController::class)->setPublic(true); + $container->autowire(\MailPoet\Settings\SettingsChangeHandler::class)->setPublic(true); $container->autowire(\MailPoet\Settings\SettingsRepository::class)->setPublic(true); $container->autowire(\MailPoet\Settings\TrackingConfig::class)->setPublic(true); // User Flags diff --git a/mailpoet/lib/Settings/SettingsChangeHandler.php b/mailpoet/lib/Settings/SettingsChangeHandler.php new file mode 100644 index 0000000000..10164b7b0a --- /dev/null +++ b/mailpoet/lib/Settings/SettingsChangeHandler.php @@ -0,0 +1,59 @@ +scheduledTasksRepository = $scheduledTasksRepository; + } + + public function onSubscribeOldWoocommerceCustomersChange(): void { + $task = $this->scheduledTasksRepository->findOneBy([ + 'type' => WooCommerceSync::TASK_TYPE, + 'status' => ScheduledTaskEntity::STATUS_SCHEDULED, + ]); + if (!($task instanceof ScheduledTaskEntity)) { + $task = $this->createScheduledTask(WooCommerceSync::TASK_TYPE); + } + $datetime = Carbon::createFromTimestamp((int)WPFunctions::get()->currentTime('timestamp')); + $task->setScheduledAt($datetime->subMinute()); + $this->scheduledTasksRepository->persist($task); + $this->scheduledTasksRepository->flush(); + } + + public function onInactiveSubscribersIntervalChange(): void { + $task = $this->scheduledTasksRepository->findOneBy([ + 'type' => InactiveSubscribers::TASK_TYPE, + 'status' => ScheduledTaskEntity::STATUS_SCHEDULED, + ]); + if (!($task instanceof ScheduledTaskEntity)) { + $task = $this->createScheduledTask(InactiveSubscribers::TASK_TYPE); + } + $datetime = Carbon::createFromTimestamp((int)WPFunctions::get()->currentTime('timestamp')); + $task->setScheduledAt($datetime->subMinute()); + $this->scheduledTasksRepository->persist($task); + $this->scheduledTasksRepository->flush(); + } + + private function createScheduledTask(string $type): ScheduledTaskEntity { + $task = new ScheduledTaskEntity(); + $task->setType($type); + $task->setStatus(ScheduledTaskEntity::STATUS_SCHEDULED); + return $task; + } +} diff --git a/mailpoet/tests/integration/API/JSON/v1/SettingsTest.php b/mailpoet/tests/integration/API/JSON/v1/SettingsTest.php index d4980c62fb..0d8b7663fb 100644 --- a/mailpoet/tests/integration/API/JSON/v1/SettingsTest.php +++ b/mailpoet/tests/integration/API/JSON/v1/SettingsTest.php @@ -9,7 +9,6 @@ use MailPoet\API\JSON\Response as APIResponse; use MailPoet\API\JSON\v1\Settings; use MailPoet\Config\ServicesChecker; use MailPoet\Cron\Workers\InactiveSubscribers; -use MailPoet\Cron\Workers\WooCommerceSync; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Form\FormMessageController; @@ -21,6 +20,7 @@ use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Segments\SegmentsRepository; use MailPoet\Services\AuthorizedEmailsController; use MailPoet\Services\Bridge; +use MailPoet\Settings\SettingsChangeHandler; use MailPoet\Settings\SettingsController; use MailPoet\Settings\SettingsRepository; use MailPoet\Settings\TrackingConfig; @@ -40,16 +40,12 @@ class SettingsTest extends \MailPoetTest { /** @var SettingsController */ private $settings; - /** @var ScheduledTasksRepository */ - private $tasksRepository; - /* @var NewslettersRepository */ private $newsletterRepository; public function _before() { parent::_before(); ORM::raw_execute('TRUNCATE ' . ScheduledTask::$_table); - $this->tasksRepository = $this->diContainer->get(ScheduledTasksRepository::class); $this->newsletterRepository = $this->diContainer->get(NewslettersRepository::class); $this->settings = SettingsController::getInstance(); $this->settings->set('some.setting.key', true); @@ -66,6 +62,7 @@ class SettingsTest extends \MailPoetTest { $this->diContainer->get(FormMessageController::class), $this->make(ServicesChecker::class, ['isMailPoetAPIKeyPendingApproval' => false]), $this->diContainer->get(SegmentsRepository::class), + $this->diContainer->get(SettingsChangeHandler::class), $this->diContainer->get(SubscribersCountsController::class), $this->diContainer->get(TrackingConfig::class) ); @@ -108,6 +105,7 @@ class SettingsTest extends \MailPoetTest { $this->diContainer->get(FormMessageController::class), $this->make(ServicesChecker::class, ['isMailPoetAPIKeyPendingApproval' => false]), $this->diContainer->get(SegmentsRepository::class), + $this->diContainer->get(SettingsChangeHandler::class), $this->diContainer->get(SubscribersCountsController::class), $this->diContainer->get(TrackingConfig::class) ); @@ -141,6 +139,7 @@ class SettingsTest extends \MailPoetTest { $this->diContainer->get(FormMessageController::class), $this->make(ServicesChecker::class, ['isMailPoetAPIKeyPendingApproval' => false]), $this->diContainer->get(SegmentsRepository::class), + $this->diContainer->get(SettingsChangeHandler::class), $this->diContainer->get(SubscribersCountsController::class), $this->diContainer->get(TrackingConfig::class) ); @@ -169,6 +168,7 @@ class SettingsTest extends \MailPoetTest { $this->diContainer->get(FormMessageController::class), $this->make(ServicesChecker::class, ['isMailPoetAPIKeyPendingApproval' => false]), $this->diContainer->get(SegmentsRepository::class), + $this->diContainer->get(SettingsChangeHandler::class), $this->diContainer->get(SubscribersCountsController::class), $this->diContainer->get(TrackingConfig::class) ); @@ -199,6 +199,7 @@ class SettingsTest extends \MailPoetTest { $this->diContainer->get(FormMessageController::class), $this->make(ServicesChecker::class), $this->diContainer->get(SegmentsRepository::class), + $this->diContainer->get(SettingsChangeHandler::class), $this->diContainer->get(SubscribersCountsController::class), $this->diContainer->get(TrackingConfig::class) ); @@ -247,54 +248,6 @@ class SettingsTest extends \MailPoetTest { expect($this->settings->get('reply_to'))->isEmpty(); } - public function testItReschedulesScheduledTaskForWoocommerceSync(): void { - $newTask = $this->createScheduledTask(WooCommerceSync::TASK_TYPE); - assert($newTask instanceof ScheduledTaskEntity); - - $this->endpoint->onSubscribeOldWoocommerceCustomersChange(); - - $this->entityManager->clear(); - $task = $this->getScheduledTaskByType(WooCommerceSync::TASK_TYPE); - assert($task instanceof ScheduledTaskEntity); - $scheduledAt = $task->getScheduledAt(); - assert($scheduledAt instanceof \DateTime); - $expectedScheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); - $expectedScheduledAt->subMinute(); - expect($scheduledAt)->equals($expectedScheduledAt); - expect($newTask->getId())->equals($task->getId()); - } - - public function testItCreatesScheduledTaskForWoocommerceSync(): void { - $task = $this->getScheduledTaskByType(WooCommerceSync::TASK_TYPE); - expect($task)->null(); - $this->endpoint->onSubscribeOldWoocommerceCustomersChange(); - $task = $this->getScheduledTaskByType(WooCommerceSync::TASK_TYPE); - expect($task)->isInstanceOf(ScheduledTaskEntity::class); - } - - public function testItReschedulesScheduledTaskForInactiveSubscribers(): void { - $newTask = $this->createScheduledTask(InactiveSubscribers::TASK_TYPE); - assert($newTask instanceof ScheduledTaskEntity); - $this->endpoint->onInactiveSubscribersIntervalChange(); - - $task = $this->getScheduledTaskByType(InactiveSubscribers::TASK_TYPE); - assert($task instanceof ScheduledTaskEntity); - $scheduledAt = $task->getScheduledAt(); - assert($scheduledAt instanceof \DateTime); - $expectedScheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); - $expectedScheduledAt->subMinute(); - expect($scheduledAt)->equals($expectedScheduledAt); - expect($newTask->getId())->equals($task->getId()); - } - - public function testItCreatesScheduledTaskForInactiveSubscribers(): void { - $task = $this->getScheduledTaskByType(InactiveSubscribers::TASK_TYPE); - expect($task)->null(); - $this->endpoint->onInactiveSubscribersIntervalChange(); - $task = $this->getScheduledTaskByType(InactiveSubscribers::TASK_TYPE); - expect($task)->isInstanceOf(ScheduledTaskEntity::class); - } - public function testItDeactivatesReEngagementEmailsIfTrackingDisabled(): void { $this->createNewsletter(NewsletterEntity::TYPE_RE_ENGAGEMENT, NewsletterEntity::STATUS_ACTIVE); $this->settings->set('tracking', ['level' => TrackingConfig::LEVEL_PARTIAL]); @@ -321,22 +274,6 @@ class SettingsTest extends \MailPoetTest { expect($response->meta['showNotice'])->equals(false); } - private function createScheduledTask(string $type): ScheduledTaskEntity { - $task = new ScheduledTaskEntity(); - $task->setType($type); - $task->setStatus(ScheduledTaskEntity::STATUS_SCHEDULED); - $this->tasksRepository->persist($task); - $this->tasksRepository->flush(); - return $task; - } - - private function getScheduledTaskByType(string $type): ?ScheduledTaskEntity { - return $this->tasksRepository->findOneBy([ - 'type' => $type, - 'status' => ScheduledTaskEntity::STATUS_SCHEDULED, - ]); - } - private function createNewsletter(string $type, string $status = NewsletterEntity::STATUS_DRAFT, $parent = null): NewsletterEntity { $newsletter = new NewsletterEntity(); $newsletter->setType($type); diff --git a/mailpoet/tests/integration/Settings/SettingsChangeHandlerTest.php b/mailpoet/tests/integration/Settings/SettingsChangeHandlerTest.php new file mode 100644 index 0000000000..a6d3cf97bf --- /dev/null +++ b/mailpoet/tests/integration/Settings/SettingsChangeHandlerTest.php @@ -0,0 +1,88 @@ +tasksRepository = $this->diContainer->get(ScheduledTasksRepository::class); + $this->settingsChangeHandler = $this->diContainer->get(SettingsChangeHandler::class); + } + + public function testItReschedulesScheduledTaskForWoocommerceSync(): void { + $newTask = $this->createScheduledTask(WooCommerceSync::TASK_TYPE); + assert($newTask instanceof ScheduledTaskEntity); + + $this->settingsChangeHandler->onSubscribeOldWoocommerceCustomersChange(); + + $this->entityManager->clear(); + $task = $this->getScheduledTaskByType(WooCommerceSync::TASK_TYPE); + assert($task instanceof ScheduledTaskEntity); + $scheduledAt = $task->getScheduledAt(); + assert($scheduledAt instanceof \DateTime); + $expectedScheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); + $expectedScheduledAt->subMinute(); + expect($scheduledAt)->equals($expectedScheduledAt); + expect($newTask->getId())->equals($task->getId()); + } + + public function testItCreatesScheduledTaskForWoocommerceSync(): void { + $task = $this->getScheduledTaskByType(WooCommerceSync::TASK_TYPE); + expect($task)->null(); + $this->settingsChangeHandler->onSubscribeOldWoocommerceCustomersChange(); + $task = $this->getScheduledTaskByType(WooCommerceSync::TASK_TYPE); + expect($task)->isInstanceOf(ScheduledTaskEntity::class); + } + + public function testItReschedulesScheduledTaskForInactiveSubscribers(): void { + $newTask = $this->createScheduledTask(InactiveSubscribers::TASK_TYPE); + assert($newTask instanceof ScheduledTaskEntity); + $this->settingsChangeHandler->onInactiveSubscribersIntervalChange(); + + $task = $this->getScheduledTaskByType(InactiveSubscribers::TASK_TYPE); + assert($task instanceof ScheduledTaskEntity); + $scheduledAt = $task->getScheduledAt(); + assert($scheduledAt instanceof \DateTime); + $expectedScheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); + $expectedScheduledAt->subMinute(); + expect($scheduledAt)->equals($expectedScheduledAt); + expect($newTask->getId())->equals($task->getId()); + } + + public function testItCreatesScheduledTaskForInactiveSubscribers(): void { + $task = $this->getScheduledTaskByType(InactiveSubscribers::TASK_TYPE); + expect($task)->null(); + $this->settingsChangeHandler->onInactiveSubscribersIntervalChange(); + $task = $this->getScheduledTaskByType(InactiveSubscribers::TASK_TYPE); + expect($task)->isInstanceOf(ScheduledTaskEntity::class); + } + + private function createScheduledTask(string $type): ScheduledTaskEntity { + $task = new ScheduledTaskEntity(); + $task->setType($type); + $task->setStatus(ScheduledTaskEntity::STATUS_SCHEDULED); + $this->tasksRepository->persist($task); + $this->tasksRepository->flush(); + return $task; + } + + private function getScheduledTaskByType(string $type): ?ScheduledTaskEntity { + return $this->tasksRepository->findOneBy([ + 'type' => $type, + 'status' => ScheduledTaskEntity::STATUS_SCHEDULED, + ]); + } +}