Reset last run state when enabling email sending (#96)

* Refactored last run setting and checking into a service
* Reset last run state when enabling email sending #94

modules/localgov_workflows_notifications/localgov_workflows_notifications.install

@@ -27,19 +27,16 @@ function localgov_workflows_notifications_install($is_syncing) {
     }
   }
 
-  // Initialise last_email_run state variable.
-  $request_time = \Drupal::time()->getRequestTime();
-  \Drupal::state()->set('localgov_workflows_notifications.last_email_run', $request_time);
+  // Initialise notification timer.
+  \Drupal::service('localgov_workflows_notifications.notification_timer')->update();
 }
 
 /**
- * Initialise last_email_run state variable.
+ * Update notification timer if not already.
  */
 function localgov_workflows_notifications_update_10001(&$sandbox) {
-
-  $last_run = \Drupal::state()->get('localgov_workflows_notifications.last_email_run');
-  if (is_null($last_run)) {
-    $request_time = \Drupal::time()->getRequestTime();
-    \Drupal::state()->set('localgov_workflows_notifications.last_email_run', $request_time);
+  $timer = \Drupal::service('localgov_workflows_notifications.notification_timer');
+  if (is_null($timer->getLastRun())) {
+    $timer->update();
   }
 }

modules/localgov_workflows_notifications/localgov_workflows_notifications.module

@@ -130,12 +130,12 @@ function localgov_workflows_notifications_cron(): void {
   }
 
   // Enqueue email notification for content past its needs review date.
-  $request_time = \Drupal::time()->getRequestTime();
-  $last_run = \Drupal::state()->get('localgov_workflows_notifications.last_email_run', $request_time);
-  $next_run = $last_run + 86400 * $settings->get('email_frequency');
-  if ($request_time >= $next_run) {
+  $notification_timer = \Drupal::service('localgov_workflows_notifications.notification_timer');
+  if ($notification_timer->trigger()) {
 
     // Get review date IDs that need have passed their review date.
+    $request_time = \Drupal::time()->getRequestTime();
+    $last_run = $notification_timer->getLastRun();
     $storage = \Drupal::entityTypeManager()->getStorage('review_date');
     $review_date_ids = $storage->getQuery()
       ->condition('active', 1)
@@ -156,7 +156,7 @@ function localgov_workflows_notifications_cron(): void {
       }
     }
 
-    \Drupal::state()->set('localgov_workflows_notifications.last_email_run', $request_time);
+    $notification_timer->update();
   }
 }
 

modules/localgov_workflows_notifications/localgov_workflows_notifications.services.yml

@@ -2,3 +2,7 @@ services:
   localgov_workflows_notifications.notifier:
     class: Drupal\localgov_workflows_notifications\WorkflowNotification
     arguments: ['@queue']
+
+  localgov_workflows_notifications.notification_timer:
+    class: Drupal\localgov_workflows_notifications\NotificationTimer
+    arguments: ['@config.factory', '@state', '@datetime.time']

modules/localgov_workflows_notifications/src/Form/LocalgovWorkflowsNotificationsSettingsForm.php

@@ -2,14 +2,50 @@
 
 namespace Drupal\localgov_workflows_notifications\Form;
 
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Form\ConfigFormBase;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\localgov_workflows_notifications\NotificationTimer;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Configure LocalGov Workflows Notifications settings for the site.
  */
 final class LocalgovWorkflowsNotificationsSettingsForm extends ConfigFormBase {
 
+  /**
+   * The notification timer.
+   *
+   * @var \Drupal\localgov_workflows_notifications\NotificationTimer
+   */
+  protected NotificationTimer $timer;
+
+  /**
+   * Constructs a \Drupal\system\ConfigFormBase object.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The factory for configuration objects.
+   * @param \Drupal\localgov_workflows_notifications\NotificationTimer $notification_timer
+   *   The notification timer service.
+   * @param \Drupal\Core\Config\TypedConfigManagerInterface|null $typedConfigManager
+   *   The typed config manager.
+   */
+  public function __construct(ConfigFactoryInterface $config_factory, NotificationTimer $notification_timer, protected $typedConfigManager = NULL) {
+    $this->timer = $notification_timer;
+    parent::__construct($config_factory, $typedConfigManager);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('config.factory'),
+      $container->get('localgov_workflows_notifications.notification_timer'),
+      $container->get('config.typed')
+    );
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -49,11 +85,19 @@ public function buildForm(array $form, FormStateInterface $form_state): array {
    * {@inheritdoc}
    */
   public function submitForm(array &$form, FormStateInterface $form_state): void {
+    $settings = $this->config('localgov_workflows_notifications.settings');
+
+    // If email notifications are being enabled reset timer last run.
+    if ($settings->get('email_enabled') === FALSE && $form_state->getValue('email_enabled') === 1) {
+      $this->timer->update();
+    }
 
-    $this->config('localgov_workflows_notifications.settings')
+    // Save settings.
+    $settings
       ->set('email_enabled', $form_state->getValue('email_enabled'))
       ->set('email_frequency', $form_state->getValue('email_frequency'))
       ->save();
+
     parent::submitForm($form, $form_state);
   }
 

modules/localgov_workflows_notifications/src/NotificationTimer.php

@@ -0,0 +1,74 @@
+<?php
+
+namespace Drupal\localgov_workflows_notifications;
+
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Config\ImmutableConfig;
+use Drupal\Core\State\StateInterface;
+
+/**
+ * Handle timing when to send notifications.
+ */
+class NotificationTimer implements NotificationTimerInterface {
+
+  /**
+   * Module settings.
+   *
+   * @var \Drupal\Core\Config\ImmutableConfig
+   */
+  protected ImmutableConfig $settings;
+
+  /**
+   * The state service.
+   *
+   * @var \Drupal\Core\State\StateInterface
+   */
+  protected StateInterface $state;
+
+  /**
+   * The time service.
+   *
+   * @var \Drupal\Component\Datetime\TimeInterface
+   */
+  protected TimeInterface $time;
+
+  /**
+   * Constructs a NotificationTimer object.
+   */
+  public function __construct(ConfigFactoryInterface $config_factory, StateInterface $state, TimeInterface $time) {
+    $this->settings = $config_factory->get('localgov_workflows_notifications.settings');
+    $this->state = $state;
+    $this->time = $time;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getLastRun(): ?int {
+    return $this->state->get(self::LAST_RUN);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function update(): void {
+    $request_time = $this->time->getRequestTime();
+    $this->state->set(self::LAST_RUN, $request_time);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function trigger(): bool {
+    $request_time = $this->time->getRequestTime();
+    $last_run = $this->state->get(self::LAST_RUN, $request_time);
+    $next_run = $last_run + 86400 * $this->settings->get('email_frequency');
+    if ($request_time >= $next_run) {
+      return TRUE;
+    }
+
+    return FALSE;
+  }
+
+}

modules/localgov_workflows_notifications/src/NotificationTimerInterface.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace Drupal\localgov_workflows_notifications;
+
+/**
+ * Interface for handling notification timing.
+ */
+interface NotificationTimerInterface {
+
+  /**
+   * Last notification state variable name.
+   *
+   * @var string
+   */
+  const LAST_RUN = 'localgov_workflows_notifications.last_email_run';
+
+  /**
+   * Get the last time notifications were triggered.
+   *
+   * @return int|null
+   *   Timestamp of last notification.
+   */
+  public function getLastRun(): ?int;
+
+  /**
+   * Update the notification timer.
+   */
+  public function update(): void;
+
+  /**
+   * Is it time to trigger notification sending?
+   */
+  public function trigger(): bool;
+
+}

modules/localgov_workflows_notifications/tests/src/Functional/WorkflowNotificationSettingsTest.php

@@ -0,0 +1,72 @@
+<?php
+
+namespace Drupal\Tests\localgov_workflows_notifications\Functional;
+
+use Drupal\Core\State\StateInterface;
+use Drupal\localgov_workflows_notifications\NotificationTimerInterface;
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Test settings form.
+ */
+class WorkflowNotificationSettingsTest extends BrowserTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $profile = 'testing';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'localgov_workflows_notifications',
+  ];
+
+  /**
+   * The state service.
+   *
+   * @var \Drupal\Core\State\StateInterface
+   */
+  protected StateInterface $state;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    $this->drupalLogin($this->rootUser);
+  }
+
+  /**
+   * Test the Localgov Workflows Notifications settings form.
+   */
+  public function testSettingsForm() {
+    $last_run = \Drupal::state()->get(NotificationTimerInterface::LAST_RUN);
+
+    // Test settings get saved.
+    $this->drupalGet('/admin/config/workflow/localgov-workflows-notifications');
+    $this->submitForm([
+      'email_enabled' => FALSE,
+      'email_frequency' => 2,
+    ], 'Save configuration');
+    $settings = \Drupal::config('localgov_workflows_notifications.settings');
+    $this->assertFalse($settings->get('email_enabled'));
+    $this->assertEquals(2, $settings->get('email_frequency'));
+
+    // Test notification timer gets reset when email notifications are enabled.
+    $this->drupalGet('/admin/config/workflow/localgov-workflows-notifications');
+    $this->submitForm([
+      'email_enabled' => TRUE,
+    ], 'Save configuration');
+    $new_last_run = \Drupal::state()->get(NotificationTimerInterface::LAST_RUN);
+    $this->assertGreaterThan($last_run, $new_last_run);
+  }
+
+}

modules/localgov_workflows_notifications/tests/src/Kernel/ReviewNotificationCronHookTest.php

@@ -5,6 +5,7 @@
 use Drupal\Core\Queue\QueueInterface;
 use Drupal\KernelTests\KernelTestBase;
 use Drupal\localgov_workflows_notifications\Entity\LocalgovServiceContact;
+use Drupal\localgov_workflows_notifications\NotificationTimerInterface;
 use Drupal\localgov_workflows_notifications\Plugin\QueueWorker\EmailNotificationQueueWorker;
 use Drupal\node\Entity\Node;
 use Drupal\Tests\node\Traits\ContentTypeCreationTrait;
@@ -98,7 +99,7 @@ protected function setUp(): void {
     ]);
 
     $this->queue = \Drupal::queue(EmailNotificationQueueWorker::QUEUE_NAME);
-    \Drupal::state()->set('localgov_workflows_notifications.last_email_run', 0);
+    \Drupal::state()->set(NotificationTimerInterface::LAST_RUN, 0);
   }
 
   /**
@@ -125,7 +126,7 @@ public function testEnqueueNodes() {
     // Check notification queue after cron hook with state config.
     $request_time = \Drupal::time()->getRequestTime();
     $reviewed = $request_time - 3600;
-    \Drupal::state()->set('localgov_workflows_notifications.last_email_run', $reviewed - 86400);
+    \Drupal::state()->set(NotificationTimerInterface::LAST_RUN, $reviewed - 86400);
     $review_date = \Drupal::entityTypeManager()->getStorage('review_date')->create([
       'entity' => [
         ['target_id' => $this->node->id()],