New service for handling setting changes

[MAILPOET-4061]

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) {

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);
-
+      $this->settingsChangeHandler->onInactiveSubscribersIntervalChange();
-      /**
-       * 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();
     }
 
     return true;

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

mailpoet/lib/Settings/SettingsChangeHandler.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace MailPoet\Settings;
+
+use MailPoet\Cron\Workers\InactiveSubscribers;
+use MailPoet\Cron\Workers\WooCommerceSync;
+use MailPoet\Entities\ScheduledTaskEntity;
+use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
+use MailPoet\WP\Functions as WPFunctions;
+use MailPoetVendor\Carbon\Carbon;
+
+class SettingsChangeHandler {
+
+  /**
+   * @var ScheduledTasksRepository
+   */
+  private $scheduledTasksRepository;
+
+  public function __construct(
+    ScheduledTasksRepository $scheduledTasksRepository
+  ) {
+    $this->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;
+  }
+}

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);

mailpoet/tests/integration/Settings/SettingsChangeHandlerTest.php

@@ -0,0 +1,88 @@
+<?php
+
+namespace MailPoet\Settings;
+
+use MailPoet\Cron\Workers\InactiveSubscribers;
+use MailPoet\Cron\Workers\WooCommerceSync;
+use MailPoet\Entities\ScheduledTaskEntity;
+use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
+use MailPoet\WP\Functions as WPFunctions;
+use MailPoetVendor\Carbon\Carbon;
+
+class SettingsChangeHandlerTest extends \MailPoetTest {
+  /** @var ScheduledTasksRepository */
+  private $tasksRepository;
+
+  /** @var SettingsChangeHandler */
+  private $settingsChangeHandler;
+
+  public function _before() {
+    parent::_before();
+    $this->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,
+    ]);
+  }
+}