From 42171eb4352e315ea7ea103bf3bd43ec2ba63d13 Mon Sep 17 00:00:00 2001 From: Neill Magill Date: Fri, 8 Oct 2021 13:39:00 +0100 Subject: [PATCH] MDL-72767 forum: Ensure digests are sent Before this change if a new post was made after the digest time but before todays daily digest had been sent the run time on the digest would be set to the next day resulting in no digest being sent to the user. By adding the server midnight to the custom data on the task we ensure that in this case a new adhoc task is created for the next day leaving the current one to get processed. Server midnight is used so that if the digest time setting is changed we would not get two tasks queued for the same day. --- mod/forum/classes/task/cron_task.php | 7 ++- mod/forum/tests/maildigest_test.php | 67 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/mod/forum/classes/task/cron_task.php b/mod/forum/classes/task/cron_task.php index e3627c551c4..ade297fd42f 100644 --- a/mod/forum/classes/task/cron_task.php +++ b/mod/forum/classes/task/cron_task.php @@ -336,16 +336,19 @@ class cron_task extends \core\task\scheduled_task { if (!empty($digestpostdata)) { // Insert all of the records for the digest. $DB->insert_records('forum_queue', $digestpostdata); - $digesttime = usergetmidnight($timenow, $sitetimezone) + ($CFG->digestmailtime * 3600); + $servermidnight = usergetmidnight($timenow, $sitetimezone); + $digesttime = $servermidnight + ($CFG->digestmailtime * 3600); if ($digesttime < $timenow) { // Digest time is in the past. Get a new time for tomorrow. - $digesttime = usergetmidnight($timenow + DAYSECS, $sitetimezone) + ($CFG->digestmailtime * 3600); + $servermidnight = usergetmidnight($timenow + DAYSECS, $sitetimezone); + $digesttime = $servermidnight + ($CFG->digestmailtime * 3600); } $task = new \mod_forum\task\send_user_digests(); $task->set_userid($user->id); $task->set_component('mod_forum'); + $task->set_custom_data(['servermidnight' => $servermidnight]); $task->set_next_run_time($digesttime); \core\task\manager::reschedule_or_queue_adhoc_task($task); $usercounts['digests']++; diff --git a/mod/forum/tests/maildigest_test.php b/mod/forum/tests/maildigest_test.php index a5d70f84dd1..e0cecbe3863 100644 --- a/mod/forum/tests/maildigest_test.php +++ b/mod/forum/tests/maildigest_test.php @@ -695,6 +695,73 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $this->assertLessThanOrEqual($digesttime, $task->nextruntime); } + /** + * Tests that if a new message is posted after the days digest time, + * but before that days digests are sent a new task is created. + */ + public function test_cron_digest_queue_next_before_current_processed() { + global $DB, $CFG; + + $this->resetAfterTest(true); + + // Set up a basic user enrolled in a course. + $userhelper = $this->helper_setup_user_in_course(); + $user = $userhelper->user; + $forum1 = $userhelper->forums->forum1; + + // Add 1 discussions to forum 1. + $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + + // Set the tested user's default maildigest setting. + $DB->set_field('user', 'maildigest', 1, ['id' => $user->id]); + + // Set the digest time to the future (magic, shouldn't work). + $CFG->digestmailtime = 25; + // One digest e-mail should be sent, and no individual notifications. + $expect = [ + (object) [ + 'userid' => $user->id, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // Set the digest time to midnight. + $CFG->digestmailtime = 0; + + // Add another discussions to forum 1. + $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + + // One digest e-mail should be sent, and no individual notifications. + $expect = [ + (object) [ + 'userid' => $user->id, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // There should now be two tasks queued. + $tasks = $DB->get_records('task_adhoc'); + $this->assertCount(2, $tasks); + + // Add yet another another discussions to forum 1. + $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + + // One digest e-mail should be sent, and no individual notifications. + $expect = [ + (object) [ + 'userid' => $user->id, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // There should still be two tasks queued. + $tasks = $DB->get_records('task_adhoc'); + $this->assertCount(2, $tasks); + } + /** * The sending of a digest marks posts as read if automatic message read marking is set. */