From 28fd710f8406fab5dff858046dc4cc86dd7276c3 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 1 Jun 2018 11:37:35 +0800 Subject: [PATCH 01/10] MDL-46881 core_task: Add a logging trait --- lib/classes/task/logging_trait.php | 113 +++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 lib/classes/task/logging_trait.php diff --git a/lib/classes/task/logging_trait.php b/lib/classes/task/logging_trait.php new file mode 100644 index 00000000000..ac2d4014a0b --- /dev/null +++ b/lib/classes/task/logging_trait.php @@ -0,0 +1,113 @@ +. + +/** + * This file defines a trait to assist with logging in tasks. + * + * @package core + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * This trait includes functions to assist with logging in tasks. + * + * @package core + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +trait logging_trait { + + /** + * @var \progress_trace + */ + protected $_trace = null; + + /** + * @var \stdClass + */ + protected $_tracestats = null; + + /** + * Get the progress_trace. + * + * @return \progress_trace + */ + protected function get_trace() { + if (null === $this->_trace) { + $this->_trace = new \text_progress_trace(); + $this->_tracestats = (object) []; + } + + return $this->_trace; + } + + /** + * Log a message to the progress tracer. + * + * @param string $message + * @param int $depth + */ + protected function log($message, $depth = 1) { + $this->get_trace() + ->output($message, $depth); + } + + /** + * Log a start message to the progress tracer. + * + * @param string $message + * @param int $depth + */ + protected function log_start($message, $depth = 0) { + $this->get_trace(); + + if (defined('MDL_PERFTOLOG') && MDL_PERFTOLOG) { + $this->_tracestats->$depth = [ + 'mem' => memory_get_usage(), + 'time' => microtime(), + ]; + } + + $this->log($message, $depth); + } + + /** + * Log an end message to the progress tracer. + * + * @param string $message + * @param int $depth + */ + protected function log_finish($message, $depth = 0) { + $this->log($message, $depth); + if (isset($this->_tracestats->$depth)) { + $startstats = $this->_tracestats->$depth; + $this->log( + sprintf("Time taken %s, memory total: %s, Memory growth: %s, Memory peak: %s", + microtime_diff($startstats['time'], microtime()), + display_size(memory_get_usage()), + display_size(memory_get_usage() - $startstats['mem']), + display_size(memory_get_peak_usage()) + ), + $depth + 1 + ); + } + } +} From 4459ad290d8d156d32df2af2fcb251ae51bb8735 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 1 Jun 2018 11:39:39 +0800 Subject: [PATCH 02/10] MDL-46881 mod_forum: Move cron around --- mod/forum/classes/task/cron_task.php | 564 ++++++++- mod/forum/classes/task/send_user_digests.php | 599 +++++++++ .../classes/task/send_user_notifications.php | 534 ++++++++ mod/forum/deprecatedlib.php | 38 + mod/forum/lib.php | 823 ------------- ...um_post_emaildigestfull_textemail.mustache | 3 +- mod/forum/tests/cron_trait.php | 129 ++ mod/forum/tests/generator_trait.php | 159 +++ mod/forum/tests/mail_group_test.php | 246 ++++ mod/forum/tests/mail_test.php | 1083 ++++++++++------- mod/forum/tests/maildigest_test.php | 236 ++-- mod/forum/tests/qanda_test.php | 153 +++ 12 files changed, 3166 insertions(+), 1401 deletions(-) create mode 100644 mod/forum/classes/task/send_user_digests.php create mode 100644 mod/forum/classes/task/send_user_notifications.php create mode 100644 mod/forum/tests/cron_trait.php create mode 100644 mod/forum/tests/generator_trait.php create mode 100644 mod/forum/tests/mail_group_test.php create mode 100644 mod/forum/tests/qanda_test.php diff --git a/mod/forum/classes/task/cron_task.php b/mod/forum/classes/task/cron_task.php index f232c622c8b..ee2d2961934 100644 --- a/mod/forum/classes/task/cron_task.php +++ b/mod/forum/classes/task/cron_task.php @@ -17,16 +17,68 @@ /** * A scheduled task for forum cron. * - * @todo MDL-44734 This job will be split up properly. - * * @package mod_forum * @copyright 2014 Dan Poltawski * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ namespace mod_forum\task; +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/mod/forum/lib.php'); + +/** + * The main scheduled task for the forum. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ class cron_task extends \core\task\scheduled_task { + // Use the logging trait to get some nice, juicy, logging. + use \core\task\logging_trait; + + /** + * @var The list of courses which contain posts to be sent. + */ + protected $courses = []; + + /** + * @var The list of forums which contain posts to be sent. + */ + protected $forums = []; + + /** + * @var The list of discussions which contain posts to be sent. + */ + protected $discussions = []; + + /** + * @var The list of posts to be sent. + */ + protected $posts = []; + + /** + * @var The list of post authors. + */ + protected $users = []; + + /** + * @var The list of subscribed users. + */ + protected $subscribedusers = []; + + /** + * @var The list of digest users. + */ + protected $digestusers = []; + + /** + * @var The list of adhoc data for sending. + */ + protected $adhocdata = []; + /** * Get a descriptive name for this task (shown to admins). * @@ -37,12 +89,512 @@ class cron_task extends \core\task\scheduled_task { } /** - * Run forum cron. + * Execute the scheduled task. */ public function execute() { - global $CFG; - require_once($CFG->dirroot . '/mod/forum/lib.php'); - forum_cron(); + global $CFG, $DB; + + $timenow = time(); + + // Delete any really old posts in the digest queue. + $weekago = $timenow - (7 * 24 * 3600); + $this->log_start("Removing old digest records from 7 days ago."); + $DB->delete_records_select('forum_queue', "timemodified < ?", array($weekago)); + $this->log_finish("Removed all old digest records."); + + $endtime = $timenow - $CFG->maxeditingtime; + $starttime = $endtime - (2 * DAYSECS); + $this->log_start("Fetching unmailed posts."); + if (!$posts = $this->get_unmailed_posts($starttime, $endtime, $timenow)) { + $this->log_finish("No posts found.", 1); + return false; + } + $this->log_finish("Done"); + + // Process post data and turn into adhoc tasks. + $this->process_post_data($posts); + + // Mark posts as read. + list($in, $params) = $DB->get_in_or_equal(array_keys($posts)); + $DB->set_field_select('forum_posts', 'mailed', 1, "id {$in}", $params); } + /** + * Process all posts and convert to appropriated hoc tasks. + * + * @param \stdClass[] $posts + */ + protected function process_post_data($posts) { + $trace = $this->get_trace(); + + $discussionids = []; + $forumids = []; + $courseids = []; + + $this->log_start("Processing post information"); + + $start = microtime(true); + foreach ($posts as $id => $post) { + $discussionids[$post->discussion] = true; + $forumids[$post->forum] = true; + $courseids[$post->course] = true; + $this->add_data_for_post($post); + $this->posts[$id] = $post; + } + $this->log_finish(sprintf("Processed %s posts", count($this->posts))); + + if (empty($this->posts)) { + $this->log("No posts found. Returning early."); + return; + } + + // Please note, this order is intentional. + // The forum cache makes use of the course. + $this->log_start("Filling caches"); + + $start = microtime(true); + $this->log_start("Filling course cache", 1); + $this->fill_course_cache(array_keys($courseids)); + $this->log_finish("Done", 1); + + $this->log_start("Filling forum cache", 1); + $this->fill_forum_cache(array_keys($forumids)); + $this->log_finish("Done", 1); + + $this->log_start("Filling discussion cache", 1); + $this->fill_discussion_cache(array_keys($discussionids)); + $this->log_finish("Done", 1); + + $this->log_start("Filling user subscription cache", 1); + $this->fill_user_subscription_cache(); + $this->log_finish("Done", 1); + + $this->log_start("Filling digest cache", 1); + $this->fill_digest_cache(); + $this->log_finish("Done", 1); + + $this->log_start("Filling user posted cache for Q&A forums", 1); + $this->fetch_user_has_posted(); + $this->log_finish("Done", 1); + + $this->log_start("Queueing user tasks."); + $this->queue_user_tasks(); + $this->log_finish("All tasks queued.", 1); + + $this->log_finish("All caches filled"); + } + + /** + * Fill the course cache. + * + * @param int[] $courseids + */ + protected function fill_course_cache($courseids) { + global $DB; + + list($in, $params) = $DB->get_in_or_equal($courseids); + $this->courses = $DB->get_records_select('course', "id $in", $params); + } + + /** + * Fill the forum cache. + * + * @param int[] $forumids + */ + protected function fill_forum_cache($forumids) { + global $DB; + + $requiredfields = [ + 'id', + 'course', + 'forcesubscribe', + 'type', + ]; + list($in, $params) = $DB->get_in_or_equal($forumids); + $this->forums = $DB->get_records_select('forum', "id $in", $params, '', implode(', ', $requiredfields)); + foreach ($this->forums as $id => $forum) { + \mod_forum\subscriptions::fill_subscription_cache($id); + \mod_forum\subscriptions::fill_discussion_subscription_cache($id); + } + } + + /** + * Fill the discussion cache. + * + * @param int[] $discussionids + */ + protected function fill_discussion_cache($discussionids) { + global $DB; + + if (empty($discussionids)) { + $this->discussion = []; + } else { + + $requiredfields = [ + 'id', + 'groupid', + 'firstpost', + 'timestart', + 'timeend', + ]; + + list($in, $params) = $DB->get_in_or_equal($discussionids); + $this->discussions = $DB->get_records_select( + 'forum_discussions', "id $in", $params, '', implode(', ', $requiredfields)); + } + } + + /** + * Fill the cache of user digest preferences. + */ + protected function fill_digest_cache() { + global $DB; + + if (empty($this->users)) { + return; + } + // Get the list of forum subscriptions for per-user per-forum maildigest settings. + list($in, $params) = $DB->get_in_or_equal(array_keys($this->users)); + $digestspreferences = $DB->get_recordset_select( + 'forum_digests', "userid $in", $params, '', 'id, userid, forum, maildigest'); + foreach ($digestspreferences as $digestpreference) { + if (!isset($this->digestusers[$digestpreference->forum])) { + $this->digestusers[$digestpreference->forum] = []; + } + $this->digestusers[$digestpreference->forum][$digestpreference->userid] = $digestpreference->maildigest; + } + $digestspreferences->close(); + } + + /** + * Add dsta for the current forum post to the structure of adhoc data. + * + * @param \stdClass $post + */ + protected function add_data_for_post($post) { + if (!isset($this->adhocdata[$post->course])) { + $this->adhocdata[$post->course] = []; + } + + if (!isset($this->adhocdata[$post->course][$post->forum])) { + $this->adhocdata[$post->course][$post->forum] = []; + } + + if (!isset($this->adhocdata[$post->course][$post->forum][$post->discussion])) { + $this->adhocdata[$post->course][$post->forum][$post->discussion] = []; + } + + $this->adhocdata[$post->course][$post->forum][$post->discussion][$post->id] = $post->id; + } + + /** + * Fill the cache of user subscriptions. + */ + protected function fill_user_subscription_cache() { + foreach ($this->forums as $forum) { + $cm = get_fast_modinfo($this->courses[$forum->course])->instances['forum'][$forum->id]; + $modcontext = \context_module::instance($cm->id); + + $this->subscribedusers[$forum->id] = []; + if ($users = \mod_forum\subscriptions::fetch_subscribed_users($forum, 0, $modcontext, 'u.id, u.maildigest', true)) { + foreach ($users as $user) { + // This user is subscribed to this forum. + $this->subscribedusers[$forum->id][$user->id] = $user->id; + if (!isset($this->users[$user->id])) { + // Store minimal user info. + $this->users[$user->id] = $user; + } + } + // Release memory. + unset($users); + } + } + } + + /** + * Fill the cache for Q&A forums. + */ + protected function fetch_user_has_posted() { + global $DB; + + $forums = array_filter($this->forums, function($forum) { + return $forum->type === 'qanda'; + }); + + if (empty($forums)) { + return; + } + + list($in, $params) = $DB->get_in_or_equal(array_keys($forums)); + + $sql = "SELECT d.forum, d.firstpost, p.userid + FROM {forum} f + INNER JOIN {forum_discussions} d ON d.forum = f.id + LEFT JOIN {forum_posts} p ON p.discussion = d.id + WHERE f.type = 'qanda' + AND f.id {$in} + GROUP BY p.userid, d.forum, d.firstpost"; + + $rs = $DB->get_recordset_sql($sql, $params); + foreach ($rs as $row) { + if (empty($this->qandametadata[$row->forum])) { + $this->qandametadata[$row->forum] = (object) [ + 'users' => [], + 'firstpost' => [$row->firstpost], + ]; + } + $this->qandametadata[$row->forum]->users[$row->userid] = true; + } + $rs->close(); + + } + + /** + * Queue the user tasks. + */ + protected function queue_user_tasks() { + global $CFG, $DB; + + $trace = $this->get_trace(); + + $timenow = time(); + $sitetimezone = \core_date::get_server_timezone(); + $counts = [ + 'digests' => 0, + 'individuals' => 0, + 'users' => 0, + 'ignored' => 0, + 'messages' => 0, + ]; + $this->log("Processing " . count($this->users) . " users", 1); + foreach ($this->users as $user) { + $usercounts = [ + 'digests' => 0, + 'messages' => 0, + ]; + + $send = false; + // Setup this user so that the capabilities are cached, and environment matches receiving user. + cron_setup_user($user); + + list($individualpostdata, $digestpostdata) = $this->fetch_posts_for_user($user); + + if (!empty($digestpostdata)) { + // Insert all of the records for the digest. + $DB->insert_records('forum_queue', $digestpostdata); + $digesttime = usergetmidnight($timenow, $sitetimezone) + ($CFG->digestmailtime * 3600); + + $task = new \mod_forum\task\send_user_digests(); + $task->set_userid($user->id); + $task->set_component('mod_forum'); + $task->set_next_run_time($digesttime); + \core\task\manager::queue_adhoc_task($task, true); + $usercounts['digests']++; + $send = true; + } + + if (!empty($individualpostdata)) { + $usercounts['messages'] += count($individualpostdata); + + $task = new \mod_forum\task\send_user_notifications(); + $task->set_userid($user->id); + $task->set_custom_data($individualpostdata); + $task->set_component('mod_forum'); + \core\task\manager::queue_adhoc_task($task); + $counts['individuals']++; + $send = true; + } + + if ($send) { + $counts['users']++; + $counts['messages'] += $usercounts['messages']; + $counts['digests'] += $usercounts['digests']; + } else { + $counts['ignored']++; + } + + $this->log(sprintf("Queued %d digests and %d messages for %s", + $usercounts['digests'], + $usercounts['messages'], + $user->id + ), 2); + } + $this->log( + sprintf( + "Queued %d digests, and %d individual tasks for %d post mails. " . + "Unique users: %d (%d ignored)", + $counts['digests'], + $counts['individuals'], + $counts['messages'], + $counts['users'], + $counts['ignored'] + ), 1); + } + + /** + * Fetch posts for this user. + * + * @param \stdClass $user The user to fetch posts for. + */ + protected function fetch_posts_for_user($user) { + // We maintain a mapping of user groups for each forum. + $usergroups = []; + $digeststructure = []; + + $poststructure = $this->adhocdata; + $poststosend = []; + foreach ($poststructure as $courseid => $forumids) { + $course = $this->courses[$courseid]; + foreach ($forumids as $forumid => $discussionids) { + $forum = $this->forums[$forumid]; + $maildigest = forum_get_user_maildigest_bulk($this->digestusers, $user, $forumid); + + if (!isset($this->subscribedusers[$forumid][$user->id])) { + // This user has no subscription of any kind to this forum. + // Do not send them any posts at all. + unset($poststructure[$courseid][$forumid]); + continue; + } + + $subscriptiontime = \mod_forum\subscriptions::fetch_discussion_subscription($forum->id, $user->id); + + $cm = get_fast_modinfo($course)->instances['forum'][$forumid]; + foreach ($discussionids as $discussionid => $postids) { + $discussion = $this->discussions[$discussionid]; + if (!\mod_forum\subscriptions::is_subscribed($user->id, $forum, $discussionid, $cm)) { + // The user does not subscribe to this forum as a whole, or to this specific discussion. + unset($poststructure[$courseid][$forumid][$discussionid]); + continue; + } + + if ($discussion->groupid > 0 and $groupmode = groups_get_activity_groupmode($cm, $course)) { + // This discussion has a groupmode set (SEPARATEGROUPS or VISIBLEGROUPS). + // Check whether the user can view it based on their groups. + if (!isset($usergroups[$forum->id])) { + $usergroups[$forum->id] = groups_get_all_groups($courseid, $user->id, $cm->groupingid); + } + + if (!isset($usergroups[$forum->id][$discussion->groupid])) { + // This user is not a member of this group, or the group no longer exists. + + $modcontext = \context_module::instance($cm->id); + if (!has_capability('moodle/site:accessallgroups', $modcontext, $user)) { + // This user does not have the accessallgroups and is not a member of the group. + // Do not send posts from other groups when in SEPARATEGROUPS or VISIBLEGROUPS. + unset($poststructure[$courseid][$forumid][$discussionid]); + continue; + } + } + } + + foreach ($postids as $postid) { + $post = $this->posts[$postid]; + if ($subscriptiontime) { + // Skip posts if the user subscribed to the discussion after it was created. + $subscribedafter = isset($subscriptiontime[$post->discussion]); + $subscribedafter = $subscribedafter && ($subscriptiontime[$post->discussion] > $post->created); + if ($subscribedafter) { + // The user subscribed to the discussion/forum after this post was created. + unset($poststructure[$courseid][$forumid][$discussionid]); + continue; + } + } + + if ($forum->type === 'qanda' && $postid != $discussion->firstpost) { + $hasqandapost = isset($this->qandametadata[$forumid]); + $hasqandapost = $hasqandapost && isset($this->qandametadata[$forumid]->users[$user->id]); + if (!$hasqandapost) { + // The user has not posted to this qanda forum. + unset($poststructure[$courseid][$forumid][$discussionid]); + continue; + } + } + + if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { + // The user is not allowed to see the post for some other reason. + unset($poststructure[$courseid][$forumid][$discussionid][$postid]); + continue; + } + + if ($maildigest > 0) { + // This user wants the mails to be in digest form. + $digeststructure[] = (object) [ + 'userid' => $user->id, + 'discussionid' => $discussion->id, + 'postid' => $post->id, + 'timemodified' => $post->created, + ]; + unset($poststructure[$courseid][$forumid][$discussionid][$postid]); + continue; + } else { + // Add this post to the list of postids to be sent. + $poststosend[] = $postid; + } + } + } + + if (empty($poststructure[$courseid][$forumid])) { + // This user is not subscribed to any discussions in this forum at all. + unset($poststructure[$courseid][$forumid]); + continue; + } + } + if (empty($poststructure[$courseid])) { + // This user is not subscribed to any forums in this course. + unset($poststructure[$courseid]); + } + } + + return [$poststosend, $digeststructure]; + } + + /** + * Returns a list of all new posts that have not been mailed yet + * + * @param int $starttime posts created after this time + * @param int $endtime posts created before this + * @param int $now used for timed discussions only + * @return array + */ + protected function get_unmailed_posts($starttime, $endtime, $now = null) { + global $CFG, $DB; + + $params = array(); + $params['mailed'] = FORUM_MAILED_PENDING; + $params['ptimestart'] = $starttime; + $params['ptimeend'] = $endtime; + $params['mailnow'] = 1; + + if (!empty($CFG->forum_enabletimedposts)) { + if (empty($now)) { + $now = time(); + } + $selectsql = "AND (p.created >= :ptimestart OR d.timestart >= :pptimestart)"; + $params['pptimestart'] = $starttime; + $timedsql = "AND (d.timestart < :dtimestart AND (d.timeend = 0 OR d.timeend > :dtimeend))"; + $params['dtimestart'] = $now; + $params['dtimeend'] = $now; + } else { + $timedsql = ""; + $selectsql = "AND p.created >= :ptimestart"; + } + + return $DB->get_records_sql( + "SELECT + p.id, + p.discussion, + d.forum, + d.course, + p.created, + p.parent, + p.userid + FROM {forum_posts} p + JOIN {forum_discussions} d ON d.id = p.discussion + WHERE p.mailed = :mailed + $selectsql + AND (p.created < :ptimeend OR p.mailnow = :mailnow) + $timedsql + ORDER BY p.modified ASC", + $params); + } } diff --git a/mod/forum/classes/task/send_user_digests.php b/mod/forum/classes/task/send_user_digests.php new file mode 100644 index 00000000000..816ad76d738 --- /dev/null +++ b/mod/forum/classes/task/send_user_digests.php @@ -0,0 +1,599 @@ +. + +/** + * This file defines an adhoc task to send notifications. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace mod_forum\task; + +defined('MOODLE_INTERNAL') || die(); + +use html_writer; +require_once($CFG->dirroot . '/mod/forum/lib.php'); + +/** + * Adhoc task to send moodle forum digests for the specified user. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class send_user_digests extends \core\task\adhoc_task { + + // Use the logging trait to get some nice, juicy, logging. + use \core\task\logging_trait; + + /** + * @var \stdClass A shortcut to $USER. + */ + protected $recipient; + + /** + * @var bool[] Whether the user can view fullnames for each forum. + */ + protected $viewfullnames = []; + + /** + * @var bool[] Whether the user can post in each forum. + */ + protected $canpostto = []; + + /** + * @var \stdClass[] Courses with posts them. + */ + protected $courses = []; + + /** + * @var \stdClass[] Forums with posts them. + */ + protected $forums = []; + + /** + * @var \stdClass[] Discussions with posts them. + */ + protected $discussions = []; + + /** + * @var \stdClass[] The posts to be sent. + */ + protected $posts = []; + + /** + * @var \stdClass[] The various authors. + */ + protected $users = []; + + /** + * @var \stdClass[] A list of any per-forum digest preference that this user holds. + */ + protected $forumdigesttypes = []; + + /** + * @var bool Whether the user has requested HTML or not. + */ + protected $allowhtml = true; + + /** + * @var string The subject of the message. + */ + protected $postsubject = ''; + + /** + * @var string The plaintext content of the whole message. + */ + protected $notificationtext = ''; + + /** + * @var string The HTML content of the whole message. + */ + protected $notificationhtml = ''; + + /** + * @var string The plaintext content for the current discussion being processed. + */ + protected $discussiontext = ''; + + /** + * @var string The HTML content for the current discussion being processed. + */ + protected $discussionhtml = ''; + + /** + * @var int The number of messages sent in this digest. + */ + protected $sentcount = 0; + + /** + * @var \renderer[][] A cache of the different types of renderer, stored both by target (HTML, or Text), and type. + */ + protected $renderers = [ + 'html' => [], + 'text' => [], + ]; + + /** + * @var int[] A list of post IDs to be marked as read for this user. + */ + protected $markpostsasread = []; + + /** + * Send out messages. + */ + public function execute() { + // Terminate if not able to fetch all digests in 5 minutes. + \core_php_time_limit::raise(300); + + $starttime = time(); + + $this->recipient = \core_user::get_user($this->get_userid()); + $this->log_start("Sending forum digests for {$this->recipient->username} ({$this->recipient->id})"); + + if (empty($this->recipient->mailformat) || $this->recipient->mailformat != 1) { + // This user does not want to receive HTML. + $this->allowhtml = false; + } + + // Fetch all of the data we need to mail these posts. + $this->prepare_data($starttime); + + if (empty($this->posts) || empty($this->discussions) || empty($this->forums)) { + $this->log_finish("No messages found to send."); + return; + } + + // Add the message headers. + $this->add_message_header(); + + foreach ($this->discussions as $discussion) { + // Raise the time limit for each discussion. + \core_php_time_limit::raise(120); + + // Grab the data pertaining to this discussion. + $forum = $this->forums[$discussion->forum]; + $course = $this->courses[$forum->course]; + $cm = get_fast_modinfo($course)->instances['forum'][$forum->id]; + $modcontext = \context_module::instance($cm->id); + $coursecontext = \context_course::instance($course->id); + + if (empty($this->posts[$discussion->id])) { + // Somehow there are no posts. + // This should not happen but better safe than sorry. + continue; + } + + if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + // The course is hidden and the user does not have access to it. + // Permissions may have changed since it was queued. + continue; + } + + if (!forum_user_can_see_discussion($forum, $discussion, $modcontext, $this->recipient)) { + // User cannot see this discussion. + // Permissions may have changed since it was queued. + continue; + } + + if (!\mod_forum\subscriptions::is_subscribed($this->recipient->id, $forum, $discussion->id, $cm)) { + // The user does not subscribe to this forum as a whole, or to this specific discussion. + continue; + } + + // Fetch additional values relating to this forum. + if (!isset($this->canpostto[$discussion->id])) { + $this->canpostto[$discussion->id] = forum_user_can_post( + $forum, $discussion, $this->recipient, $cm, $course, $modcontext); + } + + if (!isset($this->viewfullnames[$forum->id])) { + $this->viewfullnames = has_capability('moodle/site:viewfullnames', $modcontext, $this->recipient->id); + } + + // Set the discussion storage values. + $discussionpostcount = 0; + $this->discussiontext = ''; + $this->discussionhtml = ''; + + // Add the header for this discussion. + $this->add_discussion_header($discussion, $forum, $course); + $this->log_start("Adding messages in discussion {$discussion->id} (forum {$forum->id})", 1); + + // Add all posts in this forum. + foreach ($this->posts[$discussion->id] as $post) { + $author = $this->get_post_author($post->userid, $course, $forum, $cm, $modcontext); + if (empty($author)) { + // Unable to find the author. Skip to avoid errors. + continue; + } + + if (!forum_user_can_see_post($forum, $discussion, $post, $this->recipient, $cm)) { + // User cannot see this post. + // Permissions may have changed since it was queued. + continue; + } + + $this->add_post_body($author, $post, $discussion, $forum, $cm, $course); + $discussionpostcount++; + } + + // Add the forum footer. + $this->add_discussion_footer($discussion, $forum, $course); + + // Add the data for this discussion to the notification body. + if ($discussionpostcount) { + $this->sentcount += $discussionpostcount; + $this->notificationtext .= $this->discussiontext; + $this->notificationhtml .= $this->discussionhtml; + $this->log_finish("Added {$discussionpostcount} messages to discussion {$discussion->id}", 1); + } else { + $this->log_finish("No messages found in discussion {$discussion->id} - skipped.", 1); + } + } + + if ($this->sentcount) { + // This digest has at least one post and should therefore be sent. + if ($this->send_mail()) { + $this->log_finish("Digest sent with {$this->sentcount} messages."); + if (get_user_preferences('forum_markasreadonnotification', 1, $this->recipient->id) == 1) { + forum_tp_mark_posts_read($this->recipient, $this->markpostsasread); + } + } else { + $this->log_finish("Issue sending digest. Skipping."); + } + } else { + $this->log_finish("No messages found to send."); + } + + // We have finishied all digest emails, update $CFG->digestmailtimelast. + set_config('digestmailtimelast', $starttime); + } + + /** + * Prepare the data for this run. + * + * Note: This will also remove posts from the queue. + * + * @param int $timenow + */ + protected function prepare_data(int $timenow) { + global $DB; + + $sql = "SELECT p.*, f.id AS forum, f.course + FROM {forum_queue} q + INNER JOIN {forum_posts} p ON p.id = q.postid + INNER JOIN {forum_discussions} d ON d.id = p.discussion + INNER JOIN {forum} f ON f.id = d.forum + WHERE q.userid = :userid + AND q.timemodified < :timemodified + ORDER BY d.id, q.timemodified ASC"; + + $queueparams = [ + 'userid' => $this->recipient->id, + 'timemodified' => $timenow, + ]; + + $posts = $DB->get_recordset_sql($sql, $queueparams); + $discussionids = []; + $forumids = []; + $courseids = []; + $userids = []; + foreach ($posts as $post) { + $discussionids[] = $post->discussion; + $forumids[] = $post->forum; + $courseids[] = $post->course; + $userids[] = $post->userid; + unset($post->forum); + if (!isset($this->posts[$post->discussion])) { + $this->posts[$post->discussion] = []; + } + $this->posts[$post->discussion][$post->id] = $post; + } + $posts->close(); + + list($in, $params) = $DB->get_in_or_equal($discussionids); + $this->discussions = $DB->get_records_select('forum_discussions', "id {$in}", $params); + + list($in, $params) = $DB->get_in_or_equal($forumids); + $this->forums = $DB->get_records_select('forum', "id {$in}", $params); + + list($in, $params) = $DB->get_in_or_equal($courseids); + $this->courses = $DB->get_records_select('course', "id $in", $params); + + list($in, $params) = $DB->get_in_or_equal($userids); + $this->users = $DB->get_records_select('user', "id $in", $params); + + $this->fill_digest_cache(); + + $DB->delete_records_select('forum_queue', "userid = :userid AND timemodified < :timemodified", $queueparams); + } + + /** + * Fill the cron digest cache. + */ + protected function fill_digest_cache() { + global $DB; + + $this->forumdigesttypes = $DB->get_records_menu('forum_digests', [ + 'userid' => $this->recipient->id, + ], '', 'forum, maildigest'); + } + + /** + * Fetch and initialise the post author. + * + * @param int $userid The id of the user to fetch + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $cm + * @param \context $context + * @return \stdClass + */ + protected function get_post_author($userid, $course, $forum, $cm, $context) { + if (!isset($this->users[$userid])) { + // This user no longer exists. + return false; + } + + $user = $this->users[$userid]; + + if (!isset($user->groups)) { + // Initialise the groups list. + $user->groups = []; + } + + if (!isset($user->groups[$forum->id])) { + $user->groups[$forum->id] = groups_get_all_groups($course->id, $user->id, $cm->groupingid); + } + + // Clone the user object to prevent leaks between messages. + return (object) (array) $user; + } + + /** + * Add the header to this message. + */ + protected function add_message_header() { + $site = get_site(); + + // Set the subject of the message. + $this->postsubject = get_string('digestmailsubject', 'forum', format_string($site->shortname, true)); + + // And the content of the header in body. + $headerdata = (object) [ + 'sitename' => format_string($site->fullname, true), + 'userprefs' => (new \moodle_url('/user/forum.php', [ + 'id' => $this->recipient->id, + 'course' => $site->id, + ]))->out(false), + ]; + + $this->notificationtext .= get_string('digestmailheader', 'forum', $headerdata) . "\n"; + + if ($this->allowhtml) { + $headerdata->userprefs = html_writer::link($headerdata->userprefs, get_string('digestmailprefs', 'forum'), [ + 'target' => '_blank', + ]); + + $this->notificationhtml .= html_writer::tag('p', get_string('digestmailheader', 'forum', $headerdata)); + $this->notificationhtml .= html_writer::empty_tag('br'); + $this->notificationhtml .= html_writer::empty_tag('hr', [ + 'size' => 1, + 'noshade' => 'noshade', + ]); + } + } + + /** + * Add the header for this discussion. + * + * @param \stdClass $discussion The discussion to add the footer for + * @param \stdClass $forum The forum that the discussion belongs to + * @param \stdClass $course The course that the forum belongs to + */ + protected function add_discussion_header($discussion, $forum, $course) { + global $CFG; + + $shortname = format_string($course->shortname, true, [ + 'context' => \context_course::instance($course->id), + ]); + + $strforums = get_string('forums', 'forum'); + + $this->discussiontext .= "\n=====================================================================\n\n"; + $this->discussiontext .= "$shortname -> $strforums -> " . format_string($forum->name, true); + if ($discussion->name != $forum->name) { + $this->discussiontext .= " -> " . format_string($discussion->name, true); + } + $this->discussiontext .= "\n"; + $this->discussiontext .= new \moodle_url('/mod/forum/discuss.php', [ + 'd' => $discussion->id, + ]); + $this->discussiontext .= "\n"; + + if ($this->allowhtml) { + $this->discussionhtml .= "

". + "wwwroot/course/view.php?id=$course->id\">$shortname -> ". + "wwwroot/mod/forum/index.php?id=$course->id\">$strforums -> ". + "wwwroot/mod/forum/view.php?f=$forum->id\">" . + format_string($forum->name, true).""; + if ($discussion->name == $forum->name) { + $this->discussionhtml .= "

"; + } else { + $this->discussionhtml .= + " -> wwwroot/mod/forum/discuss.php?d=$discussion->id\">" . + format_string($discussion->name, true)."

"; + } + $this->discussionhtml .= '

'; + } + + } + + /** + * Add the body of this post. + * + * @param \stdClass $author The author of the post + * @param \stdClass $post The post being sent + * @param \stdClass $discussion The discussion that the post is in + * @param \stdClass $forum The forum that the discussion belongs to + * @param \cminfo $cm The cminfo object for the forum + * @param \stdClass $course The course that the forum belongs to + */ + protected function add_post_body($author, $post, $discussion, $forum, $cm, $course) { + global $CFG; + + $canreply = $this->canpostto[$discussion->id]; + + $data = new \mod_forum\output\forum_post_email( + $course, + $cm, + $forum, + $discussion, + $post, + $author, + $this->recipient, + $canreply + ); + + // Override the viewfullnames value. + $data->viewfullnames = $this->viewfullnames[$forum->id]; + + // Determine the type of digest being sent. + $maildigest = $this->get_maildigest($forum->id); + + $textrenderer = $this->get_renderer($maildigest); + $this->discussiontext .= $textrenderer->render($data); + $this->discussiontext .= "\n"; + if ($this->allowhtml) { + $htmlrenderer = $this->get_renderer($maildigest, true); + $this->discussionhtml .= $htmlrenderer->render($data); + $this->log("Adding post {$post->id} in format {$maildigest} with HTML", 2); + } else { + $this->log("Adding post {$post->id} in format {$maildigest} without HTML", 2); + } + + if ($maildigest == 1 && $CFG->forum_usermarksread) { + // Create an array of postid's for this user to mark as read. + $this->markpostsasread[] = $post->id; + } + + } + + /** + * Add the footer for this discussion. + * + * @param \stdClass $discussion The discussion to add the footer for + */ + protected function add_discussion_footer($discussion) { + global $CFG; + + if ($this->allowhtml) { + $footerlinks = []; + + $forum = $this->forums[$discussion->forum]; + if (\mod_forum\subscriptions::is_forcesubscribed($forum)) { + // This forum is force subscribed. The user cannot unsubscribe. + $footerlinks[] = get_string("everyoneissubscribed", "forum"); + } else { + $footerlinks[] = "wwwroot/mod/forum/subscribe.php?id=$forum->id\">" . + get_string("unsubscribe", "forum") . ""; + } + $footerlinks[] = "" . + get_string("digestmailpost", "forum") . ''; + + $this->discussionhtml .= "\n

" . + implode(' ', $footerlinks) . '
'; + $this->discussionhtml .= '

'; + } + } + + /** + * Get the forum digest type for the specified forum, failing back to + * the default setting for the current user if not specified. + * + * @param int $forumid + * @return int + */ + protected function get_maildigest($forumid) { + $maildigest = -1; + + if (isset($this->forumdigesttypes[$forumid])) { + $maildigest = $this->forumdigesttypes[$forumid]; + } + + if ($maildigest === -1 && !empty($this->recipient->maildigest)) { + $maildigest = $this->recipient->maildigest; + } + + if ($maildigest === -1) { + // There is no maildigest type right now. + $maildigest = 1; + } + + return $maildigest; + } + + /** + * Send the composed message to the user. + */ + protected function send_mail() { + // Headers to help prevent auto-responders. + $userfrom = \core_user::get_noreply_user(); + $userfrom->customheaders = array( + "Precedence: Bulk", + 'X-Auto-Response-Suppress: All', + 'Auto-Submitted: auto-generated', + ); + + $eventdata = new \core\message\message(); + $eventdata->courseid = SITEID; + $eventdata->component = 'mod_forum'; + $eventdata->name = 'digests'; + $eventdata->userfrom = $userfrom; + $eventdata->userto = $this->recipient; + $eventdata->subject = $this->postsubject; + $eventdata->fullmessage = $this->notificationtext; + $eventdata->fullmessageformat = FORMAT_PLAIN; + $eventdata->fullmessagehtml = $this->notificationhtml; + $eventdata->notification = 1; + $eventdata->smallmessage = get_string('smallmessagedigest', 'forum', $this->sentcount); + + return message_send($eventdata); + } + + /** + * Helper to fetch the required renderer, instantiating as required. + * + * @param int $maildigest The type of mail digest being sent + * @param bool $html Whether to fetch the HTML renderer + * @return \core_renderer + */ + protected function get_renderer($maildigest, $html = false) { + global $PAGE; + + $type = $maildigest == 2 ? 'emaildigestbasic' : 'emaildigestfull'; + $target = $html ? 'htmlemail' : 'textemail'; + + if (!isset($this->renderers[$target][$type])) { + $this->renderers[$target][$type] = $PAGE->get_renderer('mod_forum', $type, $target); + } + + return $this->renderers[$target][$type]; + } +} diff --git a/mod/forum/classes/task/send_user_notifications.php b/mod/forum/classes/task/send_user_notifications.php new file mode 100644 index 00000000000..cd146d57b58 --- /dev/null +++ b/mod/forum/classes/task/send_user_notifications.php @@ -0,0 +1,534 @@ +. + +/** + * This file defines an adhoc task to send notifications. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace mod_forum\task; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Adhoc task to send user forum notifications. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class send_user_notifications extends \core\task\adhoc_task { + + // Use the logging trait to get some nice, juicy, logging. + use \core\task\logging_trait; + + /** + * @var \stdClass A shortcut to $USER. + */ + protected $recipient; + + /** + * @var \stdClass[] List of courses the messages are in, indexed by courseid. + */ + protected $courses = []; + + /** + * @var \stdClass[] List of forums the messages are in, indexed by courseid. + */ + protected $forums = []; + + /** + * @var int[] List of IDs for forums in each course. + */ + protected $courseforums = []; + + /** + * @var \stdClass[] List of discussions the messages are in, indexed by forumid. + */ + protected $discussions = []; + + /** + * @var \stdClass[] List of IDs for discussions in each forum. + */ + protected $forumdiscussions = []; + + /** + * @var \stdClass[] List of posts the messages are in, indexed by discussionid. + */ + protected $posts = []; + + /** + * @var bool[] Whether the user can view fullnames for each forum. + */ + protected $viewfullnames = []; + + /** + * @var bool[] Whether the user can post in each discussion. + */ + protected $canpostto = []; + + /** + * @var \renderer[] The renderers. + */ + protected $renderers = []; + + /** + * @var \core\message\inbound\address_manager The inbound message address manager. + */ + protected $inboundmanager; + + /** + * Send out messages. + */ + public function execute() { + global $CFG; + + $this->recipient = \core_user::get_user($this->get_userid()); + + // Create the generic messageinboundgenerator. + $this->inboundmanager = new \core\message\inbound\address_manager(); + $this->inboundmanager->set_handler('\mod_forum\message\inbound\reply_handler'); + + $data = $this->get_custom_data(); + + $this->prepare_data((array) $data); + + $markposts = []; + $errorcount = 0; + $sentcount = 0; + $this->log_start("Sending messages to {$this->recipient->username} ({$this->recipient->id})"); + foreach ($this->courses as $course) { + $coursecontext = \context_course::instance($course->id); + if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $coursecontext)) { + // The course is hidden and the user does not have access to it. + // Permissions may have changed since it was queued. + continue; + } + foreach ($this->courseforums[$course->id] as $forumid) { + $forum = $this->forums[$forumid]; + + $cm = get_fast_modinfo($course)->instances['forum'][$forumid]; + $modcontext = \context_module::instance($cm->id); + + foreach (array_values($this->forumdiscussions[$forumid]) as $discussionid) { + $discussion = $this->discussions[$discussionid]; + + if (!forum_user_can_see_discussion($forum, $discussion, $modcontext, $this->recipient)) { + // User cannot see this discussion. + // Permissions may have changed since it was queued. + continue; + } + + if (!\mod_forum\subscriptions::is_subscribed($this->recipient->id, $forum, $discussionid, $cm)) { + // The user does not subscribe to this forum as a whole, or to this specific discussion. + continue; + } + + foreach ($this->posts[$discussionid] as $post) { + if (!forum_user_can_see_post($forum, $discussion, $post, $this->recipient, $cm)) { + // User cannot see this post. + // Permissions may have changed since it was queued. + continue; + } + + if ($this->send_post($course, $forum, $discussion, $post, $cm, $modcontext)) { + $this->log("Post {$post->id} sent", 1); + // Mark post as read if forum_usermarksread is set off. + if (!$CFG->forum_usermarksread) { + $markposts[$post->id] = true; + } + $sentcount++; + } else { + $this->log("Failed to send post {$post->id}", 1); + $errorcount++; + } + } + } + } + } + + $this->log_finish("Sent {$sentcount} messages with {$errorcount} failures"); + if (!empty($markposts)) { + if (get_user_preferences('forum_markasreadonnotification', 1, $this->recipient->id) == 1) { + $this->log_start("Marking posts as read"); + $count = count($markposts); + forum_tp_mark_posts_read($this->recipient, $markposts); + $this->log_finish("Marked {$count} posts as read"); + } + } + } + + /** + * Prepare all data for this run. + * + * Take all post ids, and fetch the relevant authors, discussions, forums, and courses for them. + * + * @param int[] $postids The list of post IDs + */ + protected function prepare_data(array $postids) { + global $DB; + + if (empty($postids)) { + return; + } + + list($in, $params) = $DB->get_in_or_equal(array_values($postids)); + $sql = "SELECT p.*, f.id AS forum, f.course + FROM {forum_posts} p + INNER JOIN {forum_discussions} d ON d.id = p.discussion + INNER JOIN {forum} f ON f.id = d.forum + WHERE p.id {$in}"; + + $posts = $DB->get_recordset_sql($sql, $params); + $discussionids = []; + $forumids = []; + $courseids = []; + $userids = []; + foreach ($posts as $post) { + $discussionids[] = $post->discussion; + $forumids[] = $post->forum; + $courseids[] = $post->course; + $userids[] = $post->userid; + unset($post->forum); + if (!isset($this->posts[$post->discussion])) { + $this->posts[$post->discussion] = []; + } + $this->posts[$post->discussion][$post->id] = $post; + } + $posts->close(); + + // Fetch all discussions. + list($in, $params) = $DB->get_in_or_equal(array_values($discussionids)); + $this->discussions = $DB->get_records_select('forum_discussions', "id {$in}", $params); + foreach ($this->discussions as $discussion) { + if (empty($this->forumdiscussions[$discussion->forum])) { + $this->forumdiscussions[$discussion->forum] = []; + } + $this->forumdiscussions[$discussion->forum][] = $discussion->id; + } + + // Fetch all forums. + list($in, $params) = $DB->get_in_or_equal(array_values($forumids)); + $this->forums = $DB->get_records_select('forum', "id {$in}", $params); + foreach ($this->forums as $forum) { + if (empty($this->courseforums[$forum->course])) { + $this->courseforums[$forum->course] = []; + } + $this->courseforums[$forum->course][] = $forum->id; + } + + // Fetch all courses. + list($in, $params) = $DB->get_in_or_equal(array_values($courseids)); + $this->courses = $DB->get_records_select('course', "id $in", $params); + + // Fetch all authors. + list($in, $params) = $DB->get_in_or_equal(array_values($userids)); + $users = $DB->get_recordset_select('user', "id $in", $params); + foreach ($users as $user) { + $this->minimise_user_record($user); + $this->users[$user->id] = $user; + } + $users->close(); + + // Fill subscription caches for each forum. + // These are per-user. + foreach (array_values($forumids) as $id) { + \mod_forum\subscriptions::fill_subscription_cache($id); + \mod_forum\subscriptions::fill_discussion_subscription_cache($id); + } + } + + /** + * Send the specified post for the current user. + * + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $discussion + * @param \stdClass $post + * @param \stdClass $cm + * @param \context $context + */ + protected function send_post($course, $forum, $discussion, $post, $cm, $context) { + global $CFG; + + $author = $this->get_post_author($post->userid, $course, $forum, $cm, $context); + if (empty($author)) { + return false; + } + + // Prepare to actually send the post now, and build up the content. + $cleanforumname = str_replace('"', "'", strip_tags(format_string($forum->name))); + + $shortname = format_string($course->shortname, true, [ + 'context' => \context_course::instance($course->id), + ]); + + // Generate a reply-to address from using the Inbound Message handler. + $replyaddress = $this->get_reply_address($course, $forum, $discussion, $post, $cm, $context); + + $data = new \mod_forum\output\forum_post_email( + $course, + $cm, + $forum, + $discussion, + $post, + $author, + $this->recipient, + $this->can_post($course, $forum, $discussion, $post, $cm, $context) + ); + $data->viewfullnames = $this->can_view_fullnames($course, $forum, $discussion, $post, $cm, $context); + + // Not all of these variables are used in the default string but are made available to support custom subjects. + $site = get_site(); + $a = (object) [ + 'subject' => $data->get_subject(), + 'forumname' => $cleanforumname, + 'sitefullname' => format_string($site->fullname), + 'siteshortname' => format_string($site->shortname), + 'courseidnumber' => $data->get_courseidnumber(), + 'coursefullname' => $data->get_coursefullname(), + 'courseshortname' => $data->get_coursename(), + ]; + $postsubject = html_to_text(get_string('postmailsubject', 'forum', $a), 0); + + // Message headers are stored against the message author. + $author->customheaders = $this->get_message_headers($course, $forum, $discussion, $post, $a, $data); + + $eventdata = new \core\message\message(); + $eventdata->courseid = $course->id; + $eventdata->component = 'mod_forum'; + $eventdata->name = 'posts'; + $eventdata->userfrom = $author; + $eventdata->userto = $this->recipient; + $eventdata->subject = $postsubject; + $eventdata->fullmessage = $this->get_renderer()->render($data); + $eventdata->fullmessageformat = FORMAT_PLAIN; + $eventdata->fullmessagehtml = $this->get_renderer(true)->render($data); + $eventdata->notification = 1; + $eventdata->replyto = $replyaddress; + if (!empty($replyaddress)) { + // Add extra text to email messages if they can reply back. + $eventdata->set_additional_content('email', [ + 'fullmessage' => [ + 'footer' => "\n\n" . get_string('replytopostbyemail', 'mod_forum'), + ], + 'fullmessagehtml' => [ + 'footer' => \html_writer::tag('p', get_string('replytopostbyemail', 'mod_forum')), + ] + ]); + } + + $eventdata->smallmessage = get_string('smallmessage', 'forum', (object) [ + 'user' => fullname($author), + 'forumname' => "$shortname: " . format_string($forum->name, true) . ": " . $discussion->name, + 'message' => $post->message, + ]); + + $contexturl = new \moodle_url('/mod/forum/discuss.php', ['d' => $discussion->id], "p{$post->id}"); + $eventdata->contexturl = $contexturl->out(); + $eventdata->contexturlname = $discussion->name; + + return message_send($eventdata); + } + + /** + * Fetch and initialise the post author. + * + * @param int $userid The id of the user to fetch + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $cm + * @param \context $context + * @return \stdClass + */ + protected function get_post_author($userid, $course, $forum, $cm, $context) { + if (!isset($this->users[$userid])) { + // This user no longer exists. + return false; + } + + $user = $this->users[$userid]; + + if (!isset($user->groups)) { + // Initialise the groups list. + $user->groups = []; + } + + if (!isset($user->groups[$forum->id])) { + $user->groups[$forum->id] = groups_get_all_groups($course->id, $user->id, $cm->groupingid); + } + + // Clone the user object to prevent leaks between messages. + return (object) (array) $user; + } + + /** + * Helper to fetch the required renderer, instantiating as required. + * + * @param bool $html Whether to fetch the HTML renderer + * @return \core_renderer + */ + protected function get_renderer($html = false) { + global $PAGE; + + $target = $html ? 'htmlemail' : 'textemail'; + + if (!isset($this->renderers[$target])) { + $this->renderers[$target] = $PAGE->get_renderer('mod_forum', 'email', $target); + } + + return $this->renderers[$target]; + } + + /** + * Get the list of message headers. + * + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $discussion + * @param \stdClass $post + * @param \stdClass $a The list of strings for this post + * @param \core\message\message $message The message to be sent + * @return \stdClass + */ + protected function get_message_headers($course, $forum, $discussion, $post, $a, $message) { + $cleanforumname = str_replace('"', "'", strip_tags(format_string($forum->name))); + $viewurl = new \moodle_url('/mod/forum/view.php', ['f' => $forum->id]); + + $headers = [ + // Headers to make emails easier to track. + 'List-Id: "' . $cleanforumname . '" ' . generate_email_messageid('moodleforum' . $forum->id), + 'List-Help: ' . $viewurl->out(), + 'Message-ID: ' . forum_get_email_message_id($post->id, $this->recipient->id), + 'X-Course-Id: ' . $course->id, + 'X-Course-Name: '. format_string($course->fullname, true), + + // Headers to help prevent auto-responders. + 'Precedence: Bulk', + 'X-Auto-Response-Suppress: All', + 'Auto-Submitted: auto-generated', + 'List-Unsubscribe: <' . $message->get_unsubscribediscussionlink() . '>', + ]; + + $rootid = forum_get_email_message_id($discussion->firstpost, $this->recipient->id); + + if ($post->parent) { + // This post is a reply, so add reply header (RFC 2822). + $parentid = forum_get_email_message_id($post->parent, $this->recipient->id); + $headers[] = "In-Reply-To: $parentid"; + + // If the post is deeply nested we also reference the parent message id and + // the root message id (if different) to aid threading when parts of the email + // conversation have been deleted (RFC1036). + if ($post->parent != $discussion->firstpost) { + $headers[] = "References: $rootid $parentid"; + } else { + $headers[] = "References: $parentid"; + } + } + + // MS Outlook / Office uses poorly documented and non standard headers, including + // Thread-Topic which overrides the Subject and shouldn't contain Re: or Fwd: etc. + $aclone = (object) (array) $a; + $aclone->subject = $discussion->name; + $threadtopic = html_to_text(get_string('postmailsubject', 'forum', $aclone), 0); + $headers[] = "Thread-Topic: $threadtopic"; + $headers[] = "Thread-Index: " . substr($rootid, 1, 28); + + return $headers; + } + + /** + * Get a no-reply address for this user to reply to the current post. + * + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $discussion + * @param \stdClass $post + * @param \stdClass $cm + * @param \context $context + * @return string + */ + protected function get_reply_address($course, $forum, $discussion, $post, $cm, $context) { + if ($this->can_post($course, $forum, $discussion, $post, $cm, $context)) { + // Generate a reply-to address from using the Inbound Message handler. + $this->inboundmanager->set_data($post->id); + return $this->inboundmanager->generate($this->recipient->id); + } + + // TODO Check if we can return a string. + // This will be controlled by the event. + return null; + } + + /** + * Check whether the user can post. + * + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $discussion + * @param \stdClass $post + * @param \stdClass $cm + * @param \context $context + * @return bool + */ + protected function can_post($course, $forum, $discussion, $post, $cm, $context) { + if (!isset($this->canpostto[$discussion->id])) { + $this->canpostto[$discussion->id] = forum_user_can_post($forum, $discussion, $this->recipient, $cm, $course, $context); + } + return $this->canpostto[$discussion->id]; + } + + /** + * Check whether the user can view full names of other users. + * + * @param \stdClass $course + * @param \stdClass $forum + * @param \stdClass $discussion + * @param \stdClass $post + * @param \stdClass $cm + * @param \context $context + * @return bool + */ + protected function can_view_fullnames($course, $forum, $discussion, $post, $cm, $context) { + if (!isset($this->viewfullnames[$forum->id])) { + $this->viewfullnames[$forum->id] = has_capability('moodle/site:viewfullnames', $context, $this->recipient->id); + } + + return $this->viewfullnames[$forum->id]; + } + + /** + * Removes properties from user record that are not necessary for sending post notifications. + * + * @param \stdClass $user + */ + protected function minimise_user_record(\stdClass $user) { + // We store large amount of users in one huge array, make sure we do not store info there we do not actually + // need in mail generation code or messaging. + unset($user->institution); + unset($user->department); + unset($user->address); + unset($user->city); + unset($user->url); + unset($user->currentlogin); + unset($user->description); + unset($user->descriptionformat); + } +} diff --git a/mod/forum/deprecatedlib.php b/mod/forum/deprecatedlib.php index be682bd5fa4..2ca2ee7a56a 100644 --- a/mod/forum/deprecatedlib.php +++ b/mod/forum/deprecatedlib.php @@ -372,3 +372,41 @@ function forum_make_mail_post($course, $cm, $forum, $discussion, $post, $userfro return $renderer->render($renderable); } + +/** + * Removes properties from user record that are not necessary for sending post notifications. + * + * @param stdClass $user + * @return void, $user parameter is modified + * @deprecated since Moodle 3.7 + */ +function forum_cron_minimise_user_record(stdClass $user) { + debugging("forum_cron_minimise_user_record() has been deprecated and has not been replaced.", + DEBUG_DEVELOPER); + + // We store large amount of users in one huge array, + // make sure we do not store info there we do not actually need + // in mail generation code or messaging. + + unset($user->institution); + unset($user->department); + unset($user->address); + unset($user->city); + unset($user->url); + unset($user->currentlogin); + unset($user->description); + unset($user->descriptionformat); +} + +/** + * Function to be run periodically according to the scheduled task. + * + * Finds all posts that have yet to be mailed out, and mails them out to all subscribers as well as other maintance + * tasks. + * + * @deprecated since Moodle 3.7 + */ +function forum_cron() { + debugging("forum_cron() has been deprecated and replaced with new tasks. Please uses these instead.", + DEBUG_DEVELOPER); +} diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 50e28663a48..39c27ccd95e 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -423,829 +423,6 @@ function forum_get_email_message_id($postid, $usertoid) { return generate_email_messageid(hash('sha256', $postid . 'to' . $usertoid)); } -/** - * Removes properties from user record that are not necessary - * for sending post notifications. - * @param stdClass $user - * @return void, $user parameter is modified - */ -function forum_cron_minimise_user_record(stdClass $user) { - - // We store large amount of users in one huge array, - // make sure we do not store info there we do not actually need - // in mail generation code or messaging. - - unset($user->institution); - unset($user->department); - unset($user->address); - unset($user->city); - unset($user->url); - unset($user->currentlogin); - unset($user->description); - unset($user->descriptionformat); -} - -/** - * Function to be run periodically according to the scheduled task. - * - * Finds all posts that have yet to be mailed out, and mails them - * out to all subscribers as well as other maintance tasks. - * - * NOTE: Since 2.7.2 this function is run by scheduled task rather - * than standard cron. - * - * @todo MDL-44734 The function will be split up into seperate tasks. - */ -function forum_cron() { - global $CFG, $USER, $DB, $PAGE; - - $site = get_site(); - - // The main renderers. - $htmlout = $PAGE->get_renderer('mod_forum', 'email', 'htmlemail'); - $textout = $PAGE->get_renderer('mod_forum', 'email', 'textemail'); - $htmldigestfullout = $PAGE->get_renderer('mod_forum', 'emaildigestfull', 'htmlemail'); - $textdigestfullout = $PAGE->get_renderer('mod_forum', 'emaildigestfull', 'textemail'); - $htmldigestbasicout = $PAGE->get_renderer('mod_forum', 'emaildigestbasic', 'htmlemail'); - $textdigestbasicout = $PAGE->get_renderer('mod_forum', 'emaildigestbasic', 'textemail'); - - // All users that are subscribed to any post that needs sending, - // please increase $CFG->extramemorylimit on large sites that - // send notifications to a large number of users. - $users = array(); - $userscount = 0; // Cached user counter - count($users) in PHP is horribly slow!!! - - // Status arrays. - $mailcount = array(); - $errorcount = array(); - - // caches - $discussions = array(); - $forums = array(); - $courses = array(); - $coursemodules = array(); - $subscribedusers = array(); - $messageinboundhandlers = array(); - - // Posts older than 2 days will not be mailed. This is to avoid the problem where - // cron has not been running for a long time, and then suddenly people are flooded - // with mail from the past few weeks or months - $timenow = time(); - $endtime = $timenow - $CFG->maxeditingtime; - $starttime = $endtime - 48 * 3600; // Two days earlier - - // Get the list of forum subscriptions for per-user per-forum maildigest settings. - $digestsset = $DB->get_recordset('forum_digests', null, '', 'id, userid, forum, maildigest'); - $digests = array(); - foreach ($digestsset as $thisrow) { - if (!isset($digests[$thisrow->forum])) { - $digests[$thisrow->forum] = array(); - } - $digests[$thisrow->forum][$thisrow->userid] = $thisrow->maildigest; - } - $digestsset->close(); - - // Create the generic messageinboundgenerator. - $messageinboundgenerator = new \core\message\inbound\address_manager(); - $messageinboundgenerator->set_handler('\mod_forum\message\inbound\reply_handler'); - - if ($posts = forum_get_unmailed_posts($starttime, $endtime, $timenow)) { - // Mark them all now as being mailed. It's unlikely but possible there - // might be an error later so that a post is NOT actually mailed out, - // but since mail isn't crucial, we can accept this risk. Doing it now - // prevents the risk of duplicated mails, which is a worse problem. - - if (!forum_mark_old_posts_as_mailed($endtime)) { - mtrace('Errors occurred while trying to mark some posts as being mailed.'); - return false; // Don't continue trying to mail them, in case we are in a cron loop - } - - // checking post validity, and adding users to loop through later - foreach ($posts as $pid => $post) { - - $discussionid = $post->discussion; - if (!isset($discussions[$discussionid])) { - if ($discussion = $DB->get_record('forum_discussions', array('id'=> $post->discussion))) { - $discussions[$discussionid] = $discussion; - \mod_forum\subscriptions::fill_subscription_cache($discussion->forum); - \mod_forum\subscriptions::fill_discussion_subscription_cache($discussion->forum); - - } else { - mtrace('Could not find discussion ' . $discussionid); - unset($posts[$pid]); - continue; - } - } - $forumid = $discussions[$discussionid]->forum; - if (!isset($forums[$forumid])) { - if ($forum = $DB->get_record('forum', array('id' => $forumid))) { - $forums[$forumid] = $forum; - } else { - mtrace('Could not find forum '.$forumid); - unset($posts[$pid]); - continue; - } - } - $courseid = $forums[$forumid]->course; - if (!isset($courses[$courseid])) { - if ($course = $DB->get_record('course', array('id' => $courseid))) { - $courses[$courseid] = $course; - } else { - mtrace('Could not find course '.$courseid); - unset($posts[$pid]); - continue; - } - } - if (!isset($coursemodules[$forumid])) { - if ($cm = get_coursemodule_from_instance('forum', $forumid, $courseid)) { - $coursemodules[$forumid] = $cm; - } else { - mtrace('Could not find course module for forum '.$forumid); - unset($posts[$pid]); - continue; - } - } - - $modcontext = context_module::instance($coursemodules[$forumid]->id); - - // Save the Inbound Message datakey here to reduce DB queries later. - $messageinboundgenerator->set_data($pid); - $messageinboundhandlers[$pid] = $messageinboundgenerator->fetch_data_key(); - - // Caching subscribed users of each forum. - if (!isset($subscribedusers[$forumid])) { - if ($subusers = \mod_forum\subscriptions::fetch_subscribed_users($forums[$forumid], 0, $modcontext, 'u.*', true)) { - - foreach ($subusers as $postuser) { - // this user is subscribed to this forum - $subscribedusers[$forumid][$postuser->id] = $postuser->id; - $userscount++; - if ($userscount > FORUM_CRON_USER_CACHE) { - // Store minimal user info. - $minuser = new stdClass(); - $minuser->id = $postuser->id; - $users[$postuser->id] = $minuser; - } else { - // Cache full user record. - forum_cron_minimise_user_record($postuser); - $users[$postuser->id] = $postuser; - } - } - // Release memory. - unset($subusers); - unset($postuser); - } - } - $mailcount[$pid] = 0; - $errorcount[$pid] = 0; - } - } - - if ($users && $posts) { - - foreach ($users as $userto) { - // Terminate if processing of any account takes longer than 2 minutes. - core_php_time_limit::raise(120); - - mtrace('Processing user ' . $userto->id); - - // Init user caches - we keep the cache for one cycle only, otherwise it could consume too much memory. - if (isset($userto->username)) { - $userto = clone($userto); - } else { - $userto = $DB->get_record('user', array('id' => $userto->id)); - forum_cron_minimise_user_record($userto); - } - $userto->viewfullnames = array(); - $userto->canpost = array(); - $userto->markposts = array(); - - // Setup this user so that the capabilities are cached, and environment matches receiving user. - cron_setup_user($userto); - - // Reset the caches. - foreach ($coursemodules as $forumid => $unused) { - $coursemodules[$forumid]->cache = new stdClass(); - $coursemodules[$forumid]->cache->caps = array(); - unset($coursemodules[$forumid]->uservisible); - } - - foreach ($posts as $pid => $post) { - $discussion = $discussions[$post->discussion]; - $forum = $forums[$discussion->forum]; - $course = $courses[$forum->course]; - $cm =& $coursemodules[$forum->id]; - - // Do some checks to see if we can bail out now. - - // Only active enrolled users are in the list of subscribers. - // This does not necessarily mean that the user is subscribed to the forum or to the discussion though. - if (!isset($subscribedusers[$forum->id][$userto->id])) { - // The user does not subscribe to this forum. - continue; - } - - if (!\mod_forum\subscriptions::is_subscribed($userto->id, $forum, $post->discussion, $coursemodules[$forum->id])) { - // The user does not subscribe to this forum, or to this specific discussion. - continue; - } - - if ($subscriptiontime = \mod_forum\subscriptions::fetch_discussion_subscription($forum->id, $userto->id)) { - // Skip posts if the user subscribed to the discussion after it was created. - if (isset($subscriptiontime[$post->discussion]) && ($subscriptiontime[$post->discussion] > $post->created)) { - continue; - } - } - - $coursecontext = context_course::instance($course->id); - if (!$course->visible and !has_capability('moodle/course:viewhiddencourses', $coursecontext, $userto->id)) { - // The course is hidden and the user does not have access to it. - continue; - } - - // Don't send email if the forum is Q&A and the user has not posted. - // Initial topics are still mailed. - if ($forum->type == 'qanda' && !forum_get_user_posted_time($discussion->id, $userto->id) && $pid != $discussion->firstpost) { - mtrace('Did not email ' . $userto->id.' because user has not posted in discussion'); - continue; - } - - // Get info about the sending user. - if (array_key_exists($post->userid, $users)) { - // We might know the user already. - $userfrom = $users[$post->userid]; - if (!isset($userfrom->idnumber)) { - // Minimalised user info, fetch full record. - $userfrom = $DB->get_record('user', array('id' => $userfrom->id)); - forum_cron_minimise_user_record($userfrom); - } - - } else if ($userfrom = $DB->get_record('user', array('id' => $post->userid))) { - forum_cron_minimise_user_record($userfrom); - // Fetch only once if possible, we can add it to user list, it will be skipped anyway. - if ($userscount <= FORUM_CRON_USER_CACHE) { - $userscount++; - $users[$userfrom->id] = $userfrom; - } - } else { - mtrace('Could not find user ' . $post->userid . ', author of post ' . $post->id . '. Unable to send message.'); - continue; - } - - // Note: If we want to check that userto and userfrom are not the same person this is probably the spot to do it. - - // Setup global $COURSE properly - needed for roles and languages. - cron_setup_user($userto, $course); - - // Fill caches. - if (!isset($userto->viewfullnames[$forum->id])) { - $modcontext = context_module::instance($cm->id); - $userto->viewfullnames[$forum->id] = has_capability('moodle/site:viewfullnames', $modcontext); - } - if (!isset($userto->canpost[$discussion->id])) { - $modcontext = context_module::instance($cm->id); - $userto->canpost[$discussion->id] = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext); - } - if (!isset($userfrom->groups[$forum->id])) { - if (!isset($userfrom->groups)) { - $userfrom->groups = array(); - if (isset($users[$userfrom->id])) { - $users[$userfrom->id]->groups = array(); - } - } - $userfrom->groups[$forum->id] = groups_get_all_groups($course->id, $userfrom->id, $cm->groupingid); - if (isset($users[$userfrom->id])) { - $users[$userfrom->id]->groups[$forum->id] = $userfrom->groups[$forum->id]; - } - } - - // Make sure groups allow this user to see this email. - if ($discussion->groupid > 0 and $groupmode = groups_get_activity_groupmode($cm, $course)) { - // Groups are being used. - if (!groups_group_exists($discussion->groupid)) { - // Can't find group - be safe and don't this message. - continue; - } - - if (!groups_is_member($discussion->groupid) and !has_capability('moodle/site:accessallgroups', $modcontext)) { - // Do not send posts from other groups when in SEPARATEGROUPS or VISIBLEGROUPS. - continue; - } - } - - // Make sure we're allowed to see the post. - if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { - mtrace('User ' . $userto->id .' can not see ' . $post->id . '. Not sending message.'); - continue; - } - - // OK so we need to send the email. - - // Does the user want this post in a digest? If so postpone it for now. - $maildigest = forum_get_user_maildigest_bulk($digests, $userto, $forum->id); - - if ($maildigest > 0) { - // This user wants the mails to be in digest form. - $queue = new stdClass(); - $queue->userid = $userto->id; - $queue->discussionid = $discussion->id; - $queue->postid = $post->id; - $queue->timemodified = $post->created; - $DB->insert_record('forum_queue', $queue); - continue; - } - - // Prepare to actually send the post now, and build up the content. - - $cleanforumname = str_replace('"', "'", strip_tags(format_string($forum->name))); - - $userfrom->customheaders = array ( - // Headers to make emails easier to track. - 'List-Id: "' . $cleanforumname . '" ' . generate_email_messageid('moodleforum' . $forum->id), - 'List-Help: ' . $CFG->wwwroot . '/mod/forum/view.php?f=' . $forum->id, - 'Message-ID: ' . forum_get_email_message_id($post->id, $userto->id), - 'X-Course-Id: ' . $course->id, - 'X-Course-Name: ' . format_string($course->fullname, true), - - // Headers to help prevent auto-responders. - 'Precedence: Bulk', - 'X-Auto-Response-Suppress: All', - 'Auto-Submitted: auto-generated', - ); - - $shortname = format_string($course->shortname, true, array('context' => context_course::instance($course->id))); - - // Generate a reply-to address from using the Inbound Message handler. - $replyaddress = null; - if ($userto->canpost[$discussion->id] && array_key_exists($post->id, $messageinboundhandlers)) { - $messageinboundgenerator->set_data($post->id, $messageinboundhandlers[$post->id]); - $replyaddress = $messageinboundgenerator->generate($userto->id); - } - - if (!isset($userto->canpost[$discussion->id])) { - $canreply = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext); - } else { - $canreply = $userto->canpost[$discussion->id]; - } - - $data = new \mod_forum\output\forum_post_email( - $course, - $cm, - $forum, - $discussion, - $post, - $userfrom, - $userto, - $canreply - ); - - $userfrom->customheaders[] = sprintf('List-Unsubscribe: <%s>', - $data->get_unsubscribediscussionlink()); - - if (!isset($userto->viewfullnames[$forum->id])) { - $data->viewfullnames = has_capability('moodle/site:viewfullnames', $modcontext, $userto->id); - } else { - $data->viewfullnames = $userto->viewfullnames[$forum->id]; - } - - // Not all of these variables are used in the default language - // string but are made available to support custom subjects. - $a = new stdClass(); - $a->subject = $data->get_subject(); - $a->forumname = $cleanforumname; - $a->sitefullname = format_string($site->fullname); - $a->siteshortname = format_string($site->shortname); - $a->courseidnumber = $data->get_courseidnumber(); - $a->coursefullname = $data->get_coursefullname(); - $a->courseshortname = $data->get_coursename(); - $postsubject = html_to_text(get_string('postmailsubject', 'forum', $a), 0); - - $rootid = forum_get_email_message_id($discussion->firstpost, $userto->id); - - if ($post->parent) { - // This post is a reply, so add reply header (RFC 2822). - $parentid = forum_get_email_message_id($post->parent, $userto->id); - $userfrom->customheaders[] = "In-Reply-To: $parentid"; - - // If the post is deeply nested we also reference the parent message id and - // the root message id (if different) to aid threading when parts of the email - // conversation have been deleted (RFC1036). - if ($post->parent != $discussion->firstpost) { - $userfrom->customheaders[] = "References: $rootid $parentid"; - } else { - $userfrom->customheaders[] = "References: $parentid"; - } - } - - // MS Outlook / Office uses poorly documented and non standard headers, including - // Thread-Topic which overrides the Subject and shouldn't contain Re: or Fwd: etc. - $a->subject = $discussion->name; - $threadtopic = html_to_text(get_string('postmailsubject', 'forum', $a), 0); - $userfrom->customheaders[] = "Thread-Topic: $threadtopic"; - $userfrom->customheaders[] = "Thread-Index: " . substr($rootid, 1, 28); - - // Send the post now! - mtrace('Sending ', ''); - - $eventdata = new \core\message\message(); - $eventdata->courseid = $course->id; - $eventdata->component = 'mod_forum'; - $eventdata->name = 'posts'; - $eventdata->userfrom = $userfrom; - $eventdata->userto = $userto; - $eventdata->subject = $postsubject; - $eventdata->fullmessage = $textout->render($data); - $eventdata->fullmessageformat = FORMAT_PLAIN; - $eventdata->fullmessagehtml = $htmlout->render($data); - $eventdata->notification = 1; - $eventdata->replyto = $replyaddress; - if (!empty($replyaddress)) { - // Add extra text to email messages if they can reply back. - $textfooter = "\n\n" . get_string('replytopostbyemail', 'mod_forum'); - $htmlfooter = html_writer::tag('p', get_string('replytopostbyemail', 'mod_forum')); - $additionalcontent = array('fullmessage' => array('footer' => $textfooter), - 'fullmessagehtml' => array('footer' => $htmlfooter)); - $eventdata->set_additional_content('email', $additionalcontent); - } - - $smallmessagestrings = new stdClass(); - $smallmessagestrings->user = fullname($userfrom); - $smallmessagestrings->forumname = "$shortname: " . format_string($forum->name, true) . ": " . $discussion->name; - $smallmessagestrings->message = $post->message; - - // Make sure strings are in message recipients language. - $eventdata->smallmessage = get_string_manager()->get_string('smallmessage', 'forum', $smallmessagestrings, $userto->lang); - - $contexturl = new moodle_url('/mod/forum/discuss.php', array('d' => $discussion->id), 'p' . $post->id); - $eventdata->contexturl = $contexturl->out(); - $eventdata->contexturlname = $discussion->name; - - $mailresult = message_send($eventdata); - if (!$mailresult) { - mtrace("Error: mod/forum/lib.php forum_cron(): Could not send out mail for id $post->id to user $userto->id". - " ($userto->email) .. not trying again."); - $errorcount[$post->id]++; - } else { - $mailcount[$post->id]++; - - // Mark post as read if forum_usermarksread is set off. - if (!$CFG->forum_usermarksread) { - $userto->markposts[$post->id] = $post->id; - } - } - - mtrace('post ' . $post->id . ': ' . $post->subject); - } - - // Mark processed posts as read. - if (get_user_preferences('forum_markasreadonnotification', 1, $userto->id) == 1) { - forum_tp_mark_posts_read($userto, $userto->markposts); - } - - unset($userto); - } - } - - if ($posts) { - foreach ($posts as $post) { - mtrace($mailcount[$post->id]." users were sent post $post->id, '$post->subject'"); - if ($errorcount[$post->id]) { - $DB->set_field('forum_posts', 'mailed', FORUM_MAILED_ERROR, array('id' => $post->id)); - } - } - } - - // release some memory - unset($subscribedusers); - unset($mailcount); - unset($errorcount); - - cron_setup_user(); - - $sitetimezone = core_date::get_server_timezone(); - - // Now see if there are any digest mails waiting to be sent, and if we should send them - - mtrace('Starting digest processing...'); - - core_php_time_limit::raise(300); // terminate if not able to fetch all digests in 5 minutes - - if (!isset($CFG->digestmailtimelast)) { // To catch the first time - set_config('digestmailtimelast', 0); - } - - $timenow = time(); - $digesttime = usergetmidnight($timenow, $sitetimezone) + ($CFG->digestmailtime * 3600); - - // Delete any really old ones (normally there shouldn't be any) - $weekago = $timenow - (7 * 24 * 3600); - $DB->delete_records_select('forum_queue', "timemodified < ?", array($weekago)); - mtrace ('Cleaned old digest records'); - - if ($CFG->digestmailtimelast < $digesttime and $timenow > $digesttime) { - - mtrace('Sending forum digests: '.userdate($timenow, '', $sitetimezone)); - - $digestposts_rs = $DB->get_recordset_select('forum_queue', "timemodified < ?", array($digesttime)); - - if ($digestposts_rs->valid()) { - - // We have work to do - $usermailcount = 0; - - //caches - reuse the those filled before too - $discussionposts = array(); - $userdiscussions = array(); - - foreach ($digestposts_rs as $digestpost) { - if (!isset($posts[$digestpost->postid])) { - if ($post = $DB->get_record('forum_posts', array('id' => $digestpost->postid))) { - $posts[$digestpost->postid] = $post; - } else { - continue; - } - } - $discussionid = $digestpost->discussionid; - if (!isset($discussions[$discussionid])) { - if ($discussion = $DB->get_record('forum_discussions', array('id' => $discussionid))) { - $discussions[$discussionid] = $discussion; - } else { - continue; - } - } - $forumid = $discussions[$discussionid]->forum; - if (!isset($forums[$forumid])) { - if ($forum = $DB->get_record('forum', array('id' => $forumid))) { - $forums[$forumid] = $forum; - } else { - continue; - } - } - - $courseid = $forums[$forumid]->course; - if (!isset($courses[$courseid])) { - if ($course = $DB->get_record('course', array('id' => $courseid))) { - $courses[$courseid] = $course; - } else { - continue; - } - } - - if (!isset($coursemodules[$forumid])) { - if ($cm = get_coursemodule_from_instance('forum', $forumid, $courseid)) { - $coursemodules[$forumid] = $cm; - } else { - continue; - } - } - $userdiscussions[$digestpost->userid][$digestpost->discussionid] = $digestpost->discussionid; - $discussionposts[$digestpost->discussionid][$digestpost->postid] = $digestpost->postid; - } - $digestposts_rs->close(); /// Finished iteration, let's close the resultset - - // Data collected, start sending out emails to each user - foreach ($userdiscussions as $userid => $thesediscussions) { - - core_php_time_limit::raise(120); // terminate if processing of any account takes longer than 2 minutes - - cron_setup_user(); - - mtrace(get_string('processingdigest', 'forum', $userid), '... '); - - // First of all delete all the queue entries for this user - $DB->delete_records_select('forum_queue', "userid = ? AND timemodified < ?", array($userid, $digesttime)); - - // Init user caches - we keep the cache for one cycle only, - // otherwise it would unnecessarily consume memory. - if (array_key_exists($userid, $users) and isset($users[$userid]->username)) { - $userto = clone($users[$userid]); - } else { - $userto = $DB->get_record('user', array('id' => $userid)); - forum_cron_minimise_user_record($userto); - } - $userto->viewfullnames = array(); - $userto->canpost = array(); - $userto->markposts = array(); - - // Override the language and timezone of the "current" user, so that - // mail is customised for the receiver. - cron_setup_user($userto); - - $postsubject = get_string('digestmailsubject', 'forum', format_string($site->shortname, true)); - - $headerdata = new stdClass(); - $headerdata->sitename = format_string($site->fullname, true); - $headerdata->userprefs = $CFG->wwwroot.'/user/forum.php?id='.$userid.'&course='.$site->id; - - $posttext = get_string('digestmailheader', 'forum', $headerdata)."\n\n"; - $headerdata->userprefs = ''.get_string('digestmailprefs', 'forum').''; - - $posthtml = '

'.get_string('digestmailheader', 'forum', $headerdata).'

' - . '
'; - - foreach ($thesediscussions as $discussionid) { - - core_php_time_limit::raise(120); // to be reset for each post - - $discussion = $discussions[$discussionid]; - $forum = $forums[$discussion->forum]; - $course = $courses[$forum->course]; - $cm = $coursemodules[$forum->id]; - - //override language - cron_setup_user($userto, $course); - - // Fill caches - if (!isset($userto->viewfullnames[$forum->id])) { - $modcontext = context_module::instance($cm->id); - $userto->viewfullnames[$forum->id] = has_capability('moodle/site:viewfullnames', $modcontext); - } - if (!isset($userto->canpost[$discussion->id])) { - $modcontext = context_module::instance($cm->id); - $userto->canpost[$discussion->id] = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext); - } - - $strforums = get_string('forums', 'forum'); - $canunsubscribe = ! \mod_forum\subscriptions::is_forcesubscribed($forum); - $canreply = $userto->canpost[$discussion->id]; - $shortname = format_string($course->shortname, true, array('context' => context_course::instance($course->id))); - - $posttext .= "\n \n"; - $posttext .= '====================================================================='; - $posttext .= "\n \n"; - $posttext .= "$shortname -> $strforums -> ".format_string($forum->name,true); - if ($discussion->name != $forum->name) { - $posttext .= " -> ".format_string($discussion->name,true); - } - $posttext .= "\n"; - $posttext .= $CFG->wwwroot.'/mod/forum/discuss.php?d='.$discussion->id; - $posttext .= "\n"; - - $posthtml .= "

". - "wwwroot/course/view.php?id=$course->id\">$shortname -> ". - "wwwroot/mod/forum/index.php?id=$course->id\">$strforums -> ". - "wwwroot/mod/forum/view.php?f=$forum->id\">".format_string($forum->name,true).""; - if ($discussion->name == $forum->name) { - $posthtml .= "

"; - } else { - $posthtml .= " -> wwwroot/mod/forum/discuss.php?d=$discussion->id\">".format_string($discussion->name,true)."

"; - } - $posthtml .= '

'; - - $postsarray = $discussionposts[$discussionid]; - sort($postsarray); - $sentcount = 0; - - foreach ($postsarray as $postid) { - $post = $posts[$postid]; - - if (array_key_exists($post->userid, $users)) { // we might know him/her already - $userfrom = $users[$post->userid]; - if (!isset($userfrom->idnumber)) { - $userfrom = $DB->get_record('user', array('id' => $userfrom->id)); - forum_cron_minimise_user_record($userfrom); - } - - } else if ($userfrom = $DB->get_record('user', array('id' => $post->userid))) { - forum_cron_minimise_user_record($userfrom); - if ($userscount <= FORUM_CRON_USER_CACHE) { - $userscount++; - $users[$userfrom->id] = $userfrom; - } - - } else { - mtrace('Could not find user '.$post->userid); - continue; - } - - if (!isset($userfrom->groups[$forum->id])) { - if (!isset($userfrom->groups)) { - $userfrom->groups = array(); - if (isset($users[$userfrom->id])) { - $users[$userfrom->id]->groups = array(); - } - } - $userfrom->groups[$forum->id] = groups_get_all_groups($course->id, $userfrom->id, $cm->groupingid); - if (isset($users[$userfrom->id])) { - $users[$userfrom->id]->groups[$forum->id] = $userfrom->groups[$forum->id]; - } - } - - // Headers to help prevent auto-responders. - $userfrom->customheaders = array( - "Precedence: Bulk", - 'X-Auto-Response-Suppress: All', - 'Auto-Submitted: auto-generated', - ); - - $maildigest = forum_get_user_maildigest_bulk($digests, $userto, $forum->id); - if (!isset($userto->canpost[$discussion->id])) { - $canreply = forum_user_can_post($forum, $discussion, $userto, $cm, $course, $modcontext); - } else { - $canreply = $userto->canpost[$discussion->id]; - } - - $data = new \mod_forum\output\forum_post_email( - $course, - $cm, - $forum, - $discussion, - $post, - $userfrom, - $userto, - $canreply - ); - - if (!isset($userto->viewfullnames[$forum->id])) { - $data->viewfullnames = has_capability('moodle/site:viewfullnames', $modcontext, $userto->id); - } else { - $data->viewfullnames = $userto->viewfullnames[$forum->id]; - } - - if ($maildigest == 2) { - // Subjects and link only. - $posttext .= $textdigestbasicout->render($data); - $posthtml .= $htmldigestbasicout->render($data); - } else { - // The full treatment. - $posttext .= $textdigestfullout->render($data); - $posthtml .= $htmldigestfullout->render($data); - - // Create an array of postid's for this user to mark as read. - if (!$CFG->forum_usermarksread) { - $userto->markposts[$post->id] = $post->id; - } - } - $sentcount++; - } - $footerlinks = array(); - if ($canunsubscribe) { - $footerlinks[] = "wwwroot/mod/forum/subscribe.php?id=$forum->id\">" . get_string("unsubscribe", "forum") . ""; - } else { - $footerlinks[] = get_string("everyoneissubscribed", "forum"); - } - $footerlinks[] = "" . get_string("digestmailpost", "forum") . ''; - $posthtml .= "\n

" . implode(' ', $footerlinks) . '
'; - $posthtml .= '

'; - } - - if (empty($userto->mailformat) || $userto->mailformat != 1) { - // This user DOESN'T want to receive HTML - $posthtml = ''; - } - - $eventdata = new \core\message\message(); - $eventdata->courseid = SITEID; - $eventdata->component = 'mod_forum'; - $eventdata->name = 'digests'; - $eventdata->userfrom = core_user::get_noreply_user(); - $eventdata->userto = $userto; - $eventdata->subject = $postsubject; - $eventdata->fullmessage = $posttext; - $eventdata->fullmessageformat = FORMAT_PLAIN; - $eventdata->fullmessagehtml = $posthtml; - $eventdata->notification = 1; - $eventdata->smallmessage = get_string('smallmessagedigest', 'forum', $sentcount); - $mailresult = message_send($eventdata); - - if (!$mailresult) { - mtrace("ERROR: mod/forum/cron.php: Could not send out digest mail to user $userto->id ". - "($userto->email)... not trying again."); - } else { - mtrace("success."); - $usermailcount++; - - // Mark post as read if forum_usermarksread is set off - if (get_user_preferences('forum_markasreadonnotification', 1, $userto->id) == 1) { - forum_tp_mark_posts_read($userto, $userto->markposts); - } - } - } - } - /// We have finishied all digest emails, update $CFG->digestmailtimelast - set_config('digestmailtimelast', $timenow); - } - - cron_setup_user(); - - if (!empty($usermailcount)) { - mtrace(get_string('digestsentusers', 'forum', $usermailcount)); - } - - if (!empty($CFG->forum_lastreadclean)) { - $timenow = time(); - if ($CFG->forum_lastreadclean + (24*3600) < $timenow) { - set_config('forum_lastreadclean', $timenow); - mtrace('Removing old forum read tracking info...'); - forum_tp_clean_read_records(); - } - } else { - set_config('forum_lastreadclean', time()); - } - - return true; -} - /** * * @param object $course diff --git a/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache b/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache index 2cec6e7ebec..1c2251a0dc7 100644 --- a/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache +++ b/mod/forum/templates/forum_post_emaildigestfull_textemail.mustache @@ -44,7 +44,8 @@ } {{/ str }} --------------------------------------------------------------------- {{{ message }}} - +{{# attachments }} {{{ attachments }}} +{{/ attachments }} --------------------------------------------------------------------- {{# str }} digestmailpostlink, forum, {{{ forumindexlink }}} {{/ str }} diff --git a/mod/forum/tests/cron_trait.php b/mod/forum/tests/cron_trait.php new file mode 100644 index 00000000000..c68306fe7c1 --- /dev/null +++ b/mod/forum/tests/cron_trait.php @@ -0,0 +1,129 @@ +. + +/** + * The forum module cron trait. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +trait mod_forum_tests_cron_trait { + /** + * Run the main cron task to queue all tasks, and ensure that posts + * were sent to the correct users. + * + * @param \stdClass[] $expectations The list of users, along with their expected count of messages and digests. + */ + protected function queue_tasks_and_assert($expectations = []) { + global $DB; + + // Note, we cannot use expectOutputRegex because it only allows for a single RegExp. + ob_start(); + cron_setup_user(); + $cron = new \mod_forum\task\cron_task(); + $cron->execute(); + $output = ob_get_contents(); + ob_end_clean(); + + $uniqueusers = 0; + foreach ($expectations as $expect) { + $expect->digests = isset($expect->digests) ? $expect->digests : 0; + $expect->messages = isset($expect->messages) ? $expect->messages : 0; + $expect->mentioned = isset($expect->mentioned) ? $expect->mentioned : false; + if ($expect->digests || $expect->messages) { + $expect->mentioned = true; + } + if (!$expect->mentioned) { + $this->assertNotRegExp("/Queued 0 for {$expect->userid}/", $output); + } else { + $uniqueusers++; + $this->assertRegExp( + "/Queued {$expect->digests} digests and {$expect->messages} messages for {$expect->userid}/", + $output + ); + } + } + + if (empty($expectations)) { + $this->assertRegExp("/No posts found./", $output); + } else { + $this->assertRegExp("/Unique users: {$uniqueusers}/", $output); + } + + // Update the forum queue for digests. + $DB->execute("UPDATE {forum_queue} SET timemodified = timemodified - 1"); + } + + /** + * Run any send_user_notifications tasks for the specified user, and + * ensure that the posts specified were sent. + * + * @param \stdClass $user + * @param \stdClass[] $posts + * @param bool $ignoreemptyposts + */ + protected function send_notifications_and_assert($user, $posts = [], $ignoreemptyposts = false) { + ob_start(); + $this->runAdhocTasks(\mod_forum\task\send_user_notifications::class, $user->id); + $output = ob_get_contents(); + ob_end_clean(); + + if (empty($posts) && !$ignoreemptyposts) { + $this->assertEquals('', $output); + } else { + $this->assertRegExp("/Sending messages to {$user->username}/", $output); + foreach ($posts as $post) { + $this->assertRegExp("/Post {$post->id} sent/", $output); + } + $count = count($posts); + $this->assertRegExp("/Sent {$count} messages with 0 failures/", $output); + } + } + + /** + * Run any send_user_digests tasks for the specified user, and + * ensure that the posts specified were sent. + * + * @param \stdClass $user + * @param \stdClass[] $fullposts + * @param \stdClass[] $shortposts + */ + protected function send_digests_and_assert($user, $fullposts = [], $shortposts = []) { + ob_start(); + $this->runAdhocTasks(\mod_forum\task\send_user_digests::class, $user->id); + $output = ob_get_contents(); + ob_end_clean(); + + if (empty($shortposts) && empty($fullposts)) { + $this->assertEquals('', $output); + $this->assertRegExp("/Digest sent with 0 messages./", $output); + } else { + $this->assertRegExp("/Sending forum digests for {$user->username}/", $output); + foreach ($fullposts as $post) { + $this->assertRegExp("/Adding post {$post->id} in format 1/", $output); + } + foreach ($shortposts as $post) { + $this->assertRegExp("/Adding post {$post->id} in format 2/", $output); + } + $count = count($fullposts) + count($shortposts); + $this->assertRegExp("/Digest sent with {$count} messages./", $output); + } + } +} diff --git a/mod/forum/tests/generator_trait.php b/mod/forum/tests/generator_trait.php new file mode 100644 index 00000000000..4c536ab0b7f --- /dev/null +++ b/mod/forum/tests/generator_trait.php @@ -0,0 +1,159 @@ +. + +/** + * The forum module trait with additional generator helpers. + * + * @package mod_forum + * @copyright 2018 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +trait mod_forum_tests_generator_trait { + + /** + * Helper to create the required number of users in the specified course. + * Users are enrolled as students by default. + * + * @param stdClass $course The course object + * @param integer $count The number of users to create + * @param string $role The role to assign users as + * @return array The users created + */ + protected function helper_create_users($course, $count, $role = null) { + $users = array(); + + for ($i = 0; $i < $count; $i++) { + $user = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user->id, $course->id, $role); + $users[] = $user; + } + + return $users; + } + + /** + * Create a new discussion and post within the specified forum, as the + * specified author. + * + * @param stdClass $forum The forum to post in + * @param stdClass $author The author to post as + * @param array $fields any other fields in discussion (name, message, messageformat, ...) + * @return array An array containing the discussion object, and the post object + */ + protected function helper_post_to_forum($forum, $author, $fields = array()) { + global $DB; + $generator = $this->getDataGenerator()->get_plugin_generator('mod_forum'); + + // Create a discussion in the forum, and then add a post to that discussion. + $record = (object)$fields; + $record->course = $forum->course; + $record->userid = $author->id; + $record->forum = $forum->id; + $discussion = $generator->create_discussion($record); + + // Retrieve the post which was created by create_discussion. + $post = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); + + return array($discussion, $post); + } + + /** + * Update the post time for the specified post by $factor. + * + * @param stdClass $post The post to update + * @param int $factor The amount to update by + */ + protected function helper_update_post_time($post, $factor) { + global $DB; + + // Update the post to have a created in the past. + $DB->set_field('forum_posts', 'created', $post->created + $factor, array('id' => $post->id)); + } + + /** + * Update the subscription time for the specified user/discussion by $factor. + * + * @param stdClass $user The user to update + * @param stdClass $discussion The discussion to update for this user + * @param int $factor The amount to update by + */ + protected function helper_update_subscription_time($user, $discussion, $factor) { + global $DB; + + $sub = $DB->get_record('forum_discussion_subs', array('userid' => $user->id, 'discussion' => $discussion->id)); + + // Update the subscription to have a preference in the past. + $DB->set_field('forum_discussion_subs', 'preference', $sub->preference + $factor, array('id' => $sub->id)); + } + + /** + * Create a new post within an existing discussion, as the specified author. + * + * @param stdClass $forum The forum to post in + * @param stdClass $discussion The discussion to post in + * @param stdClass $author The author to post as + * @return stdClass The forum post + */ + protected function helper_post_to_discussion($forum, $discussion, $author) { + global $DB; + + $generator = $this->getDataGenerator()->get_plugin_generator('mod_forum'); + + // Add a post to the discussion. + $record = new stdClass(); + $record->course = $forum->course; + $strre = get_string('re', 'forum'); + $record->subject = $strre . ' ' . $discussion->subject; + $record->userid = $author->id; + $record->forum = $forum->id; + $record->discussion = $discussion->id; + $record->mailnow = 1; + + $post = $generator->create_post($record); + + return $post; + } + + /** + * Create a new post within an existing discussion, as the specified author. + * + * @param stdClass $parent The post being replied to + * @param stdClass $author The author to post as + * @return stdClass The forum post + */ + protected function helper_reply_to_post($parent, $author) { + global $DB; + + $generator = $this->getDataGenerator()->get_plugin_generator('mod_forum'); + + // Add a post to the discussion. + $strre = get_string('re', 'forum'); + $record = (object) [ + 'discussion' => $parent->discussion, + 'parent' => $parent->id, + 'userid' => $author->id, + 'mailnow' => 1, + 'subject' => $strre . ' ' . $parent->subject, + ]; + + $post = $generator->create_post($record); + + return $post; + } +} diff --git a/mod/forum/tests/mail_group_test.php b/mod/forum/tests/mail_group_test.php new file mode 100644 index 00000000000..904151f01cd --- /dev/null +++ b/mod/forum/tests/mail_group_test.php @@ -0,0 +1,246 @@ +. + +/** + * The forum module mail generation tests for groups. + * + * @package mod_forum + * @copyright 2013 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/forum/lib.php'); +require_once(__DIR__ . '/cron_trait.php'); +require_once(__DIR__ . '/generator_trait.php'); + +/** + * The forum module mail generation tests for groups. + * + * @copyright 2013 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_forum_mail_group_testcase extends advanced_testcase { + // Make use of the cron tester trait. + use mod_forum_tests_cron_trait; + + // Make use of the test generator trait. + use mod_forum_tests_generator_trait; + + /** + * @var \phpunit_message_sink + */ + protected $messagesink; + + /** + * @var \phpunit_mailer_sink + */ + protected $mailsink; + + public function setUp() { + global $CFG; + + // We must clear the subscription caches. This has to be done both before each test, and after in case of other + // tests using these functions. + \mod_forum\subscriptions::reset_forum_cache(); + \mod_forum\subscriptions::reset_discussion_cache(); + + // Messaging is not compatible with transactions... + $this->preventResetByRollback(); + + // Catch all messages. + $this->messagesink = $this->redirectMessages(); + $this->mailsink = $this->redirectEmails(); + + // Forcibly reduce the maxeditingtime to a second in the past to + // ensure that messages are sent out. + $CFG->maxeditingtime = -1; + } + + public function tearDown() { + // We must clear the subscription caches. This has to be done both before each test, and after in case of other + // tests using these functions. + \mod_forum\subscriptions::reset_forum_cache(); + + $this->messagesink->clear(); + $this->messagesink->close(); + unset($this->messagesink); + + $this->mailsink->clear(); + $this->mailsink->close(); + unset($this->mailsink); + } + + /** + * Ensure that posts written in a forum marked for separate groups includes notifications for the members of that + * group, and any user with accessallgroups. + */ + public function test_separate_group() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $forum = $this->getDataGenerator()->create_module('forum', [ + 'course' => $course->id, + 'forcesubscribe' => FORUM_INITIALSUBSCRIBE, + 'groupmode' => SEPARATEGROUPS, + ]); + + // Create three students: + // - author, enrolled in group A; and + // - recipient, enrolled in group B; and + // - other, enrolled in the course, but no groups. + list($author, $recipient, $otheruser) = $this->helper_create_users($course, 3); + + // Create one teacher, not in any group and no accessallgroups capability. + list($teacher) = $this->helper_create_users($course, 1, 'teacher'); + + // Create one editing teacher, not in any group but with accessallgroups capability. + list($editingteacher) = $this->helper_create_users($course, 1, 'editingteacher'); + + $groupa = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $groupb = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $groupa->id, + 'userid' => $author->id, + ]); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $groupb->id, + 'userid' => $recipient->id, + ]); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author, [ + 'groupid' => $groupa->id, + ]); + + // Only the author should receive. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + 'otheruser' => (object) [ + 'userid' => $otheruser->id, + 'messages' => 0, + ], + 'teacher' => (object) [ + 'userid' => $teacher->id, + 'messages' => 0, + ], + 'editingteacher' => (object) [ + 'userid' => $editingteacher->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // No notifications should be queued. + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($recipient, []); + $this->send_notifications_and_assert($otheruser, []); + $this->send_notifications_and_assert($teacher, []); + $this->send_notifications_and_assert($editingteacher, [$post]); + } + + /** + * Ensure that posts written in a forum marked for visible groups includes notifications for the members of that + * group, and any user with accessallgroups. + */ + public function test_visible_group() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $forum = $this->getDataGenerator()->create_module('forum', [ + 'course' => $course->id, + 'forcesubscribe' => FORUM_INITIALSUBSCRIBE, + 'groupmode' => VISIBLEGROUPS, + ]); + + // Create three students: + // - author, enrolled in group A; and + // - recipient, enrolled in group B; and + // - other, enrolled in the course, but no groups. + list($author, $recipient, $otheruser) = $this->helper_create_users($course, 3); + + // Create one teacher, not in any group and no accessallgroups capability. + list($teacher) = $this->helper_create_users($course, 1, 'teacher'); + + // Create one editing teacher, not in any group but with accessallgroups capability. + list($editingteacher) = $this->helper_create_users($course, 1, 'editingteacher'); + + $groupa = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $groupb = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $groupa->id, + 'userid' => $author->id, + ]); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $groupb->id, + 'userid' => $recipient->id, + ]); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author, [ + 'groupid' => $groupa->id, + ]); + + // Only the author should receive. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + 'otheruser' => (object) [ + 'userid' => $otheruser->id, + 'messages' => 0, + ], + 'teacher' => (object) [ + 'userid' => $teacher->id, + 'messages' => 0, + ], + 'editingteacher' => (object) [ + 'userid' => $editingteacher->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // No notifications should be queued. + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($recipient, []); + $this->send_notifications_and_assert($otheruser, []); + $this->send_notifications_and_assert($teacher, []); + $this->send_notifications_and_assert($editingteacher, [$post]); + } +} diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php index d233252f84d..957a4edcc9d 100644 --- a/mod/forum/tests/mail_test.php +++ b/mod/forum/tests/mail_test.php @@ -26,42 +26,45 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; +require_once($CFG->dirroot . '/mod/forum/lib.php'); +require_once(__DIR__ . '/cron_trait.php'); +require_once(__DIR__ . '/generator_trait.php'); class mod_forum_mail_testcase extends advanced_testcase { + // Make use of the cron tester trait. + use mod_forum_tests_cron_trait; + // Make use of the test generator trait. + use mod_forum_tests_generator_trait; - protected $helper; + /** + * @var \phpunit_message_sink + */ + protected $messagesink; + + /** + * @var \phpunit_mailer_sink + */ + protected $mailsink; public function setUp() { + global $CFG; + // We must clear the subscription caches. This has to be done both before each test, and after in case of other // tests using these functions. \mod_forum\subscriptions::reset_forum_cache(); \mod_forum\subscriptions::reset_discussion_cache(); - global $CFG; - require_once($CFG->dirroot . '/mod/forum/lib.php'); - - $helper = new stdClass(); - // Messaging is not compatible with transactions... $this->preventResetByRollback(); // Catch all messages. - $helper->messagesink = $this->redirectMessages(); - $helper->mailsink = $this->redirectEmails(); - - // Confirm that we have an empty message sink so far. - $messages = $helper->messagesink->get_messages(); - $this->assertEquals(0, count($messages)); - - $messages = $helper->mailsink->get_messages(); - $this->assertEquals(0, count($messages)); + $this->messagesink = $this->redirectMessages(); + $this->mailsink = $this->redirectEmails(); // Forcibly reduce the maxeditingtime to a second in the past to // ensure that messages are sent out. $CFG->maxeditingtime = -1; - - $this->helper = $helper; } public function tearDown() { @@ -69,11 +72,13 @@ class mod_forum_mail_testcase extends advanced_testcase { // tests using these functions. \mod_forum\subscriptions::reset_forum_cache(); - $this->helper->messagesink->clear(); - $this->helper->messagesink->close(); + $this->messagesink->clear(); + $this->messagesink->close(); + unset($this->messagesink); - $this->helper->mailsink->clear(); - $this->helper->mailsink->close(); + $this->mailsink->clear(); + $this->mailsink->close(); + unset($this->mailsink); } /** @@ -93,166 +98,6 @@ class mod_forum_mail_testcase extends advanced_testcase { $record->id = $DB->update_record('messageinbound_handlers', $record); } - /** - * Helper to create the required number of users in the specified - * course. - * Users are enrolled as students. - * - * @param stdClass $course The course object - * @param integer $count The number of users to create - * @return array The users created - */ - protected function helper_create_users($course, $count) { - $users = array(); - - for ($i = 0; $i < $count; $i++) { - $user = $this->getDataGenerator()->create_user(); - $this->getDataGenerator()->enrol_user($user->id, $course->id); - $users[] = $user; - } - - return $users; - } - - /** - * Create a new discussion and post within the specified forum, as the - * specified author. - * - * @param stdClass $forum The forum to post in - * @param stdClass $author The author to post as - * @param array $fields any other fields in discussion (name, message, messageformat, ...) - * @param array An array containing the discussion object, and the post object - */ - protected function helper_post_to_forum($forum, $author, $fields = array()) { - global $DB; - $generator = $this->getDataGenerator()->get_plugin_generator('mod_forum'); - - // Create a discussion in the forum, and then add a post to that discussion. - $record = (object)$fields; - $record->course = $forum->course; - $record->userid = $author->id; - $record->forum = $forum->id; - $discussion = $generator->create_discussion($record); - - // Retrieve the post which was created by create_discussion. - $post = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); - - return array($discussion, $post); - } - - /** - * Update the post time for the specified post by $factor. - * - * @param stdClass $post The post to update - * @param int $factor The amount to update by - */ - protected function helper_update_post_time($post, $factor) { - global $DB; - - // Update the post to have a created in the past. - $DB->set_field('forum_posts', 'created', $post->created + $factor, array('id' => $post->id)); - } - - /** - * Update the subscription time for the specified user/discussion by $factor. - * - * @param stdClass $user The user to update - * @param stdClass $discussion The discussion to update for this user - * @param int $factor The amount to update by - */ - protected function helper_update_subscription_time($user, $discussion, $factor) { - global $DB; - - $sub = $DB->get_record('forum_discussion_subs', array('userid' => $user->id, 'discussion' => $discussion->id)); - - // Update the subscription to have a preference in the past. - $DB->set_field('forum_discussion_subs', 'preference', $sub->preference + $factor, array('id' => $sub->id)); - } - - /** - * Create a new post within an existing discussion, as the specified author. - * - * @param stdClass $forum The forum to post in - * @param stdClass $discussion The discussion to post in - * @param stdClass $author The author to post as - * @return stdClass The forum post - */ - protected function helper_post_to_discussion($forum, $discussion, $author) { - global $DB; - - $generator = $this->getDataGenerator()->get_plugin_generator('mod_forum'); - - // Add a post to the discussion. - $record = new stdClass(); - $record->course = $forum->course; - $strre = get_string('re', 'forum'); - $record->subject = $strre . ' ' . $discussion->subject; - $record->userid = $author->id; - $record->forum = $forum->id; - $record->discussion = $discussion->id; - $record->mailnow = 1; - - $post = $generator->create_post($record); - - return $post; - } - - /** - * Run the forum cron, and check that the specified post was sent the - * specified number of times. - * - * @param stdClass $post The forum post object - * @param integer $expected The number of times that the post should have been sent - * @return array An array of the messages caught by the message sink - */ - protected function helper_run_cron_check_count($post, $expected) { - - // Clear the sinks before running cron. - $this->helper->messagesink->clear(); - $this->helper->mailsink->clear(); - - // Cron daily uses mtrace, turn on buffering to silence output. - $this->expectOutputRegex("/{$expected} users were sent post {$post->id}, '{$post->subject}'/"); - forum_cron(); - - // Now check the results in the message sink. - $messages = $this->helper->messagesink->get_messages(); - - // There should be the expected number of messages. - $this->assertEquals($expected, count($messages)); - - return $messages; - } - - /** - * Run the forum cron, and check that the specified posts were sent the - * specified number of times. - * - * @param stdClass $post The forum post object - * @param integer $expected The number of times that the post should have been sent - * @return array An array of the messages caught by the message sink - */ - protected function helper_run_cron_check_counts($posts, $expected) { - - // Clear the sinks before running cron. - $this->helper->messagesink->clear(); - $this->helper->mailsink->clear(); - - // Cron daily uses mtrace, turn on buffering to silence output. - foreach ($posts as $post) { - $this->expectOutputRegex("/{$post['count']} users were sent post {$post['id']}, '{$post['subject']}'/"); - } - forum_cron(); - - // Now check the results in the message sink. - $messages = $this->helper->messagesink->get_messages(); - - // There should be the expected number of messages. - $this->assertEquals($expected, count($messages)); - - return $messages; - } - public function test_cron_message_includes_courseid() { $this->resetAfterTest(true); @@ -268,25 +113,22 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // Run cron and check that \core\event\message_sent contains the course id. - // Close the message sink so that message_send is run. - $this->helper->messagesink->close(); + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Catch just the cron events. For each message sent two events are fired: - // core\event\message_sent - // core\event\message_viewed. - $this->helper->eventsink = $this->redirectEvents(); - $this->expectOutputRegex('/Processing user/'); - - forum_cron(); - - // Get the events and close the sink so that remaining events can be triggered. - $events = $this->helper->eventsink->get_events(); - $this->helper->eventsink->close(); - - // Reset the message sink for other tests. - $this->helper->messagesink = $this->redirectMessages(); - // Notification has been marked as read, so now first event should be a 'notification_viewed' one. + $this->messagesink->close(); + $this->eventsink = $this->redirectEvents(); + $this->send_notifications_and_assert($author, [$post]); + $events = $this->eventsink->get_events(); $event = reset($events); $this->assertInstanceOf('\core\event\notification_viewed', $event); @@ -294,6 +136,8 @@ class mod_forum_mail_testcase extends advanced_testcase { $event = $events[1]; $this->assertInstanceOf('\core\event\notification_sent', $event); $this->assertEquals($course->id, $event->other['courseid']); + + $this->send_notifications_and_assert($recipient, [$post]); } public function test_forced_subscription() { @@ -311,31 +155,26 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // We expect both users to receive this post. - $expected = 2; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we saw messages for both users. - $this->assertTrue($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($recipient, [$post]); } - public function test_subscription_disabled() { + /** + * Ensure that for a forum with subscription disabled that standard users will not receive posts. + */ + public function test_subscription_disabled_standard_users() { global $DB; $this->resetAfterTest(true); @@ -352,50 +191,119 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // We expect both users to receive this post. - $expected = 0; - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); + } + + /** + * Ensure that for a forum with subscription disabled that a user subscribed to the forum will receive the post. + */ + public function test_subscription_disabled_user_subscribed_forum() { + global $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_DISALLOWSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); // A user with the manageactivities capability within the course can subscribe. - $expected = 1; $roleids = $DB->get_records_menu('role', null, '', 'shortname, id'); assign_capability('moodle/course:manageactivities', CAP_ALLOW, $roleids['student'], context_course::instance($course->id)); + + // Suscribe the recipient only. \mod_forum\subscriptions::subscribe_user($recipient->id, $forum); - $this->assertEquals($expected, $DB->count_records('forum_subscriptions', array( + $this->assertEquals(1, $DB->count_records('forum_subscriptions', array( 'userid' => $recipient->id, 'forum' => $forum->id, ))); + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + // Run cron and check that the expected number of users received the notification. - list($discussion, $post) = $this->helper_post_to_forum($forum, $recipient); - $messages = $this->helper_run_cron_check_count($post, $expected); + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Unsubscribe the user again. - \mod_forum\subscriptions::unsubscribe_user($recipient->id, $forum); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$post]); + } - $expected = 0; - $this->assertEquals($expected, $DB->count_records('forum_subscriptions', array( - 'userid' => $recipient->id, - 'forum' => $forum->id, - ))); + /** + * Ensure that for a forum with subscription disabled that a user subscribed to the discussion will receive the + * post. + */ + public function test_subscription_disabled_user_subscribed_discussion() { + global $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_DISALLOWSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // A user with the manageactivities capability within the course can subscribe. + $roleids = $DB->get_records_menu('role', null, '', 'shortname, id'); + assign_capability('moodle/course:manageactivities', CAP_ALLOW, $roleids['student'], context_course::instance($course->id)); // Run cron and check that the expected number of users received the notification. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - $messages = $this->helper_run_cron_check_count($post, $expected); // Subscribe the user to the discussion. \mod_forum\subscriptions::subscribe_user_to_discussion($recipient->id, $discussion); $this->helper_update_subscription_time($recipient, $discussion, -60); - $reply = $this->helper_post_to_discussion($forum, $discussion, $author); - $this->helper_update_post_time($reply, -30); + // Run cron and check that the expected number of users received the notification. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - $messages = $this->helper_run_cron_check_count($reply, $expected); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$post]); } + /** + * Ensure that for a forum with automatic subscription that users receive posts. + */ public function test_automatic() { $this->resetAfterTest(true); @@ -411,28 +319,20 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // We expect both users to receive this post. - $expected = 2; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we saw messages for both users. - $this->assertTrue($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($recipient, [$post]); } public function test_optional() { @@ -450,11 +350,20 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // We expect both users to receive this post. - $expected = 0; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); } public function test_automatic_with_unsubscribed_user() { @@ -475,28 +384,20 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // We expect only one user to receive this post. - $expected = 1; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we only saw one user. - $this->assertFalse($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$post]); } public function test_optional_with_subscribed_user() { @@ -517,28 +418,20 @@ class mod_forum_mail_testcase extends advanced_testcase { // Post a discussion to the forum. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - // We expect only one user to receive this post. - $expected = 1; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we only saw one user. - $this->assertFalse($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$post]); } public function test_automatic_with_unsubscribed_discussion() { @@ -562,28 +455,20 @@ class mod_forum_mail_testcase extends advanced_testcase { $this->assertFalse(\mod_forum\subscriptions::is_subscribed($author->id, $forum, $discussion->id)); $this->assertTrue(\mod_forum\subscriptions::is_subscribed($recipient->id, $forum, $discussion->id)); - // We expect only one user to receive this post. - $expected = 1; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we only saw one user. - $this->assertFalse($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$post]); } public function test_optional_with_subscribed_discussion() { @@ -608,37 +493,40 @@ class mod_forum_mail_testcase extends advanced_testcase { // Initially we don't expect any user to receive this post as you cannot subscribe to a discussion until after // you have read it. - $expected = 0; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); // Have a user reply to the discussion. $reply = $this->helper_post_to_discussion($forum, $discussion, $author); $this->helper_update_post_time($reply, -30); // We expect only one user to receive this post. - $expected = 1; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($reply, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we only saw one user. - $this->assertFalse($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$reply]); } public function test_automatic_with_subscribed_discussion_in_unsubscribed_forum() { @@ -664,56 +552,39 @@ class mod_forum_mail_testcase extends advanced_testcase { \mod_forum\subscriptions::subscribe_user_to_discussion($author->id, $discussion); $this->helper_update_subscription_time($author, $discussion, -60); - // We expect just the user subscribed to the forum to receive this post at the moment as the discussion - // subscription time is after the post time. - $expected = 1; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we only saw one user. - $this->assertFalse($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$post]); // Now post a reply to the original post. $reply = $this->helper_post_to_discussion($forum, $discussion, $author); $this->helper_update_post_time($reply, -30); - // We expect two users to receive this post. - $expected = 2; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($reply, $expected); - - $seenauthor = false; - $seenrecipient = false; - foreach ($messages as $message) { - // They should both be from our user. - $this->assertEquals($author->id, $message->useridfrom); - - if ($message->useridto == $author->id) { - $seenauthor = true; - } else if ($message->useridto = $recipient->id) { - $seenrecipient = true; - } - } - - // Check we saw both users. - $this->assertTrue($seenauthor); - $this->assertTrue($seenrecipient); + $this->send_notifications_and_assert($author, [$reply]); + $this->send_notifications_and_assert($recipient, [$reply]); } public function test_optional_with_unsubscribed_discussion_in_subscribed_forum() { @@ -738,10 +609,20 @@ class mod_forum_mail_testcase extends advanced_testcase { \mod_forum\subscriptions::unsubscribe_user_from_discussion($recipient->id, $discussion); // We don't expect any users to receive this post. - $expected = 0; + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + ]; + $this->queue_tasks_and_assert($expect); - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, $expected); + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); } /** @@ -780,16 +661,15 @@ class mod_forum_mail_testcase extends advanced_testcase { $reply = $this->helper_post_to_discussion($forum, $discussion, $author); $this->helper_update_post_time($reply, -30); - $expectedmessages[] = array( - 'id' => $reply->id, - 'subject' => $reply->subject, - 'count' => 1, - ); + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); - $expectedcount = 1; - - // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_counts($expectedmessages, $expectedcount); + $this->send_notifications_and_assert($author, [$reply]); } public function test_forum_message_inbound_multiple_posts() { @@ -809,23 +689,21 @@ class mod_forum_mail_testcase extends advanced_testcase { list($discussion, $post) = $this->helper_post_to_forum($forum, $author); $this->helper_update_post_time($post, -90); - $expectedmessages[] = array( + $expectedmessages[] = (object) [ 'id' => $post->id, 'subject' => $post->subject, 'count' => 0, - ); + ]; // Then post a reply to the first discussion. $reply = $this->helper_post_to_discussion($forum, $discussion, $author); $this->helper_update_post_time($reply, -60); - $expectedmessages[] = array( + $expectedmessages[] = (object) [ 'id' => $reply->id, 'subject' => $reply->subject, 'count' => 1, - ); - - $expectedcount = 2; + ]; // Ensure that messageinbound is enabled and configured for the forum handler. $this->helper_spoof_message_inbound_setup(); @@ -836,19 +714,22 @@ class mod_forum_mail_testcase extends advanced_testcase { // Run cron and check that the expected number of users received the notification. // Clear the mailsink, and close the messagesink. - $this->helper->mailsink->clear(); - $this->helper->messagesink->close(); + $this->mailsink->clear(); + $this->messagesink->close(); - // Cron daily uses mtrace, turn on buffering to silence output. - foreach ($expectedmessages as $post) { - $this->expectOutputRegex("/{$post['count']} users were sent post {$post['id']}, '{$post['subject']}'/"); - } + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => count($expectedmessages), + ], + ]; + $this->queue_tasks_and_assert($expect); - forum_cron(); - $messages = $this->helper->mailsink->get_messages(); + $this->send_notifications_and_assert($author, $expectedmessages); + $messages = $this->mailsink->get_messages(); // There should be the expected number of messages. - $this->assertEquals($expectedcount, count($messages)); + $this->assertEquals(2, count($messages)); foreach ($messages as $message) { $this->assertRegExp('/Reply-To: moodlemoodle123\+[^@]*@example.com/', $message->header); @@ -874,7 +755,16 @@ class mod_forum_mail_testcase extends advanced_testcase { list($discussion, $post) = $this->helper_post_to_forum($forum, $author, array('name' => $subject)); // Run cron and check that the expected number of users received the notification. - $messages = $this->helper_run_cron_check_count($post, 1); + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($author, [$post]); + $messages = $this->messagesink->get_messages(); $message = reset($messages); $this->assertEquals($author->id, $message->useridfrom); $this->assertEquals($expectedsubject, $message->subject); @@ -898,13 +788,44 @@ class mod_forum_mail_testcase extends advanced_testcase { // New posts should not have Re: in the subject. list($discussion, $post) = $this->helper_post_to_forum($forum, $author); - $messages = $this->helper_run_cron_check_count($post, 2); + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'commenter' => (object) [ + 'userid' => $commenter->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($commenter, [$post]); + $messages = $this->messagesink->get_messages(); $this->assertNotContains($strre, $messages[0]->subject); + $this->messagesink->clear(); // Replies should have Re: in the subject. $reply = $this->helper_post_to_discussion($forum, $discussion, $commenter); - $messages = $this->helper_run_cron_check_count($reply, 2); + + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'commenter' => (object) [ + 'userid' => $commenter->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($commenter, [$reply]); + $this->send_notifications_and_assert($author, [$reply]); + $messages = $this->messagesink->get_messages(); $this->assertContains($strre, $messages[0]->subject); + $this->assertContains($strre, $messages[1]->subject); } /** @@ -1127,17 +1048,21 @@ class mod_forum_mail_testcase extends advanced_testcase { // Clear the mailsink and close the messagesink. // (surely setup should provide us this cleared but...) - $this->helper->mailsink->clear(); - $this->helper->messagesink->close(); + $this->mailsink->clear(); + $this->messagesink->close(); - // Capture and silence cron output, verifying contents. - foreach ($posts as $post) { - $this->expectOutputRegex("/1 users were sent post {$post->id}, '{$post->subject}'/"); - } - forum_cron(); // It's really annoying that we have to run cron to test this. + $expect = [ + 'author' => (object) [ + 'userid' => $user->id, + 'messages' => count($posts), + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($user, $posts); // Get the mails. - $mails = $this->helper->mailsink->get_messages(); + $mails = $this->mailsink->get_messages(); // Start testing the expectations. $expectations = $data['expectations']; @@ -1182,7 +1107,301 @@ class mod_forum_mail_testcase extends advanced_testcase { } } } + // Finished, there should not be remaining expectations. $this->assertCount(0, $expectations); } + + /** + * Ensure that posts already mailed are not re-sent. + */ + public function test_already_mailed() { + global $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + $DB->set_field('forum_posts', 'mailed', 1); + + // No posts shoudl be considered. + $this->queue_tasks_and_assert([]); + + // No notifications should be queued. + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); + } + + /** + * Ensure that posts marked mailnow are not suspect to the maxeditingtime. + */ + public function test_mailnow() { + global $CFG, $DB; + + // Update the maxeditingtime to 1 day so that posts won't be sent. + $CFG->maxeditingtime = DAYSECS; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + + // Post a discussion to the forum. + list($discussion, $postmailednow) = $this->helper_post_to_forum($forum, $author, ['mailnow' => 1]); + + // Only the mailnow post should be considered. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // No notifications should be queued. + $this->send_notifications_and_assert($author, [$postmailednow]); + $this->send_notifications_and_assert($recipient, [$postmailednow]); + } + + /** + * Ensure that if a user has no permission to view a post, then it is not sent. + */ + public function test_access_coursemodule_hidden() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Create one users enrolled in the course as an editing teacher. + list($editor) = $this->helper_create_users($course, 1, 'editingteacher'); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + + // Hide the coursemodule. + set_coursemodule_visible($forum->cmid, 0); + + // Only the mailnow post should be considered. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 0, + ], + 'editor' => (object) [ + 'userid' => $editor->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // No notifications should be queued. + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); + $this->send_notifications_and_assert($editor, [$post], true); + } + + /** + * Ensure that if a user loses permission to view a post after it is queued, that it is not sent. + */ + public function test_access_coursemodule_hidden_after_queue() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Create one users enrolled in the course as an editing teacher. + list($editor) = $this->helper_create_users($course, 1, 'editingteacher'); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + + // Only the mailnow post should be considered. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + 'editor' => (object) [ + 'userid' => $editor->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // Hide the coursemodule. + set_coursemodule_visible($forum->cmid, 0); + + // No notifications should be queued for the students. + $this->send_notifications_and_assert($author, [], true); + $this->send_notifications_and_assert($recipient, [], true); + + // The editing teacher should still receive the post. + $this->send_notifications_and_assert($editor, [$post]); + } + + /** + * Ensure that messages are not sent until the timestart. + */ + public function test_access_before_timestart() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Create one users enrolled in the course as an editing teacher. + list($editor) = $this->helper_create_users($course, 1, 'editingteacher'); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + + // Update the discussion to have a timestart in the future. + $DB->set_field('forum_discussions', 'timestart', time() + DAYSECS); + + // None should be sent. + $this->queue_tasks_and_assert([]); + + // No notifications should be queued for any user. + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); + $this->send_notifications_and_assert($editor, []); + + // Update the discussion to have a timestart in the past. + $DB->set_field('forum_discussions', 'timestart', time() - DAYSECS); + + // Now should be sent to all. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + 'editor' => (object) [ + 'userid' => $editor->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // No notifications should be queued for any user. + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($recipient, [$post]); + $this->send_notifications_and_assert($editor, [$post]); + } + + /** + * Ensure that messages are not sent after the timeend. + */ + public function test_access_after_timeend() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_INITIALSUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Create one users enrolled in the course as an editing teacher. + list($editor) = $this->helper_create_users($course, 1, 'editingteacher'); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + + // Update the discussion to have a timestart in the past. + $DB->set_field('forum_discussions', 'timeend', time() - DAYSECS); + + // None should be sent. + $this->queue_tasks_and_assert([]); + + // No notifications should be queued for any user. + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, []); + $this->send_notifications_and_assert($editor, []); + + // Update the discussion to have a timestart in the past. + $DB->set_field('forum_discussions', 'timeend', time() + DAYSECS); + + // Now should be sent to all. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 1, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + 'editor' => (object) [ + 'userid' => $editor->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + // No notifications should be queued for any user. + $this->send_notifications_and_assert($author, [$post]); + $this->send_notifications_and_assert($recipient, [$post]); + $this->send_notifications_and_assert($editor, [$post]); + } } diff --git a/mod/forum/tests/maildigest_test.php b/mod/forum/tests/maildigest_test.php index 8dbb5e1738f..86a3613d46f 100644 --- a/mod/forum/tests/maildigest_test.php +++ b/mod/forum/tests/maildigest_test.php @@ -26,17 +26,16 @@ defined('MOODLE_INTERNAL') || die(); -global $CFG; +require_once(__DIR__ . '/cron_trait.php'); +require_once(__DIR__ . '/generator_trait.php'); class mod_forum_maildigest_testcase extends advanced_testcase { - /** - * Keep track of the message and mail sinks that we set up for each - * test. - * - * @var stdClass $helper - */ - protected $helper; + // Make use of the cron tester trait. + use mod_forum_tests_cron_trait; + + // Make use of the test generator trait. + use mod_forum_tests_generator_trait; /** * Set up message and mail sinks, and set up other requirements for the @@ -45,20 +44,18 @@ class mod_forum_maildigest_testcase extends advanced_testcase { public function setUp() { global $CFG; - $this->helper = new stdClass(); - // Messaging is not compatible with transactions... $this->preventResetByRollback(); // Catch all messages - $this->helper->messagesink = $this->redirectMessages(); - $this->helper->mailsink = $this->redirectEmails(); + $this->messagesink = $this->redirectMessages(); + $this->mailsink = $this->redirectEmails(); // Confirm that we have an empty message sink so far. - $messages = $this->helper->messagesink->get_messages(); + $messages = $this->messagesink->get_messages(); $this->assertEquals(0, count($messages)); - $messages = $this->helper->mailsink->get_messages(); + $messages = $this->mailsink->get_messages(); $this->assertEquals(0, count($messages)); // Tell Moodle that we've not sent any digest messages out recently. @@ -82,11 +79,11 @@ class mod_forum_maildigest_testcase extends advanced_testcase { * Clear the message sinks set up in this test. */ public function tearDown() { - $this->helper->messagesink->clear(); - $this->helper->messagesink->close(); + $this->messagesink->clear(); + $this->messagesink->close(); - $this->helper->mailsink->clear(); - $this->helper->mailsink->close(); + $this->mailsink->clear(); + $this->mailsink->close(); } /** @@ -131,52 +128,6 @@ class mod_forum_maildigest_testcase extends advanced_testcase { return $return; } - /** - * Helper to falsify all forum post records for a digest run. - */ - protected function helper_force_digest_mail_times() { - global $CFG, $DB; - // Fake all of the post editing times because digests aren't sent until - // the start of an hour where the modification time on the message is before - // the start of that hour - $sitetimezone = core_date::get_server_timezone(); - $digesttime = usergetmidnight(time(), $sitetimezone) + ($CFG->digestmailtime * 3600) - (60 * 60); - $DB->set_field('forum_posts', 'modified', $digesttime, array('mailed' => 0)); - $DB->set_field('forum_posts', 'created', $digesttime, array('mailed' => 0)); - } - - /** - * Run the forum cron, and check that the specified post was sent the - * specified number of times. - * - * @param integer $expected The number of times that the post should have been sent - * @param integer $individualcount The number of individual messages sent - * @param integer $digestcount The number of digest messages sent - */ - protected function helper_run_cron_check_count($expected, $individualcount, $digestcount) { - if ($expected === 0) { - $this->expectOutputRegex('/(Email digests successfully sent to .* users.){0}/'); - } else { - $this->expectOutputRegex("/Email digests successfully sent to {$expected} users/"); - } - forum_cron(); - - // Now check the results in the message sink. - $messages = $this->helper->messagesink->get_messages(); - - $counts = (object) array('digest' => 0, 'individual' => 0); - foreach ($messages as $message) { - if (strpos($message->subject, 'forum digest') !== false) { - $counts->digest++; - } else { - $counts->individual++; - } - } - - $this->assertEquals($digestcount, $counts->digest); - $this->assertEquals($individualcount, $counts->individual); - } - public function test_set_maildigest() { global $DB; @@ -302,10 +253,9 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $this->resetAfterTest(true); - $this->helper_force_digest_mail_times(); - // Initially the forum cron should generate no messages as we've made no posts. - $this->helper_run_cron_check_count(0, 0, 0); + $expect = []; + $this->queue_tasks_and_assert($expect); } /** @@ -324,27 +274,19 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $forum1 = $userhelper->forums->forum1; $forum2 = $userhelper->forums->forum2; - // Add some discussions to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user->id; - $record->mailnow = 1; - // Add 5 discussions to forum 1. - $record->forum = $forum1->id; + $posts = []; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + $posts[] = $post; } // Add 5 discussions to forum 2. - $record->forum = $forum2->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum2, $user, ['mailnow' => 1]); + $posts[] = $post; } - // Ensure that the creation times mean that the messages will be sent. - $this->helper_force_digest_mail_times(); - // Set the tested user's default maildigest setting. $DB->set_field('user', 'maildigest', 0, array('id' => $user->id)); @@ -355,7 +297,16 @@ class mod_forum_maildigest_testcase extends advanced_testcase { forum_set_user_maildigest($forum2, -1, $user); // No digests mails should be sent, but 10 forum mails will be sent. - $this->helper_run_cron_check_count(0, 10, 0); + $expect = [ + (object) [ + 'userid' => $user->id, + 'messages' => 10, + 'digests' => 0, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($user, $posts); } /** @@ -373,28 +324,20 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $course1 = $userhelper->courses->course1; $forum1 = $userhelper->forums->forum1; $forum2 = $userhelper->forums->forum2; - - // Add a discussion to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user->id; - $record->mailnow = 1; + $posts = []; // Add 5 discussions to forum 1. - $record->forum = $forum1->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + $posts[] = $post; } // Add 5 discussions to forum 2. - $record->forum = $forum2->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum2, $user, ['mailnow' => 1]); + $posts[] = $post; } - // Ensure that the creation times mean that the messages will be sent. - $this->helper_force_digest_mail_times(); - // Set the tested user's default maildigest setting. $DB->set_field('user', 'maildigest', 1, array('id' => $user->id)); @@ -404,8 +347,17 @@ class mod_forum_maildigest_testcase extends advanced_testcase { // Set the maildigest preference for forum2 to default. forum_set_user_maildigest($forum2, -1, $user); - // One digest mail should be sent, with no notifications, and one e-mail. - $this->helper_run_cron_check_count(1, 0, 1); + // No digests mails should be sent, but 10 forum mails will be sent. + $expect = [ + (object) [ + 'userid' => $user->id, + 'messages' => 0, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_digests_and_assert($user, $posts); } /** @@ -424,28 +376,21 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $course1 = $userhelper->courses->course1; $forum1 = $userhelper->forums->forum1; $forum2 = $userhelper->forums->forum2; - - // Add a discussion to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user->id; - $record->mailnow = 1; + $posts = []; + $digests = []; // Add 5 discussions to forum 1. - $record->forum = $forum1->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + $digests[] = $post; } // Add 5 discussions to forum 2. - $record->forum = $forum2->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum2, $user, ['mailnow' => 1]); + $posts[] = $post; } - // Ensure that the creation times mean that the messages will be sent. - $this->helper_force_digest_mail_times(); - // Set the tested user's default maildigest setting. $DB->set_field('user', 'maildigest', 0, array('id' => $user->id)); @@ -456,7 +401,17 @@ class mod_forum_maildigest_testcase extends advanced_testcase { forum_set_user_maildigest($forum2, -1, $user); // One digest e-mail should be sent, and five individual notifications. - $this->helper_run_cron_check_count(1, 5, 1); + $expect = [ + (object) [ + 'userid' => $user->id, + 'messages' => 5, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($user, $posts); + $this->send_digests_and_assert($user, $digests); } /** @@ -475,28 +430,21 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $course1 = $userhelper->courses->course1; $forum1 = $userhelper->forums->forum1; $forum2 = $userhelper->forums->forum2; - - // Add a discussion to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user->id; - $record->mailnow = 1; + $posts = []; + $digests = []; // Add 5 discussions to forum 1. - $record->forum = $forum1->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + $digests[] = $post; } // Add 5 discussions to forum 2. - $record->forum = $forum2->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum2, $user, ['mailnow' => 1]); + $posts[] = $post; } - // Ensure that the creation times mean that the messages will be sent. - $this->helper_force_digest_mail_times(); - // Set the tested user's default maildigest setting. $DB->set_field('user', 'maildigest', 1, array('id' => $user->id)); @@ -507,7 +455,17 @@ class mod_forum_maildigest_testcase extends advanced_testcase { forum_set_user_maildigest($forum2, 0, $user); // One digest e-mail should be sent, and five individual notifications. - $this->helper_run_cron_check_count(1, 5, 1); + $expect = [ + (object) [ + 'userid' => $user->id, + 'messages' => 5, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($user, $posts); + $this->send_digests_and_assert($user, $digests); } /** @@ -525,28 +483,21 @@ class mod_forum_maildigest_testcase extends advanced_testcase { $course1 = $userhelper->courses->course1; $forum1 = $userhelper->forums->forum1; $forum2 = $userhelper->forums->forum2; - - // Add a discussion to the forums. - $record = new stdClass(); - $record->course = $course1->id; - $record->userid = $user->id; - $record->mailnow = 1; + $fulldigests = []; + $shortdigests = []; // Add 5 discussions to forum 1. - $record->forum = $forum1->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum1, $user, ['mailnow' => 1]); + $fulldigests[] = $post; } // Add 5 discussions to forum 2. - $record->forum = $forum2->id; for ($i = 0; $i < 5; $i++) { - $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + list($discussion, $post) = $this->helper_post_to_forum($forum2, $user, ['mailnow' => 1]); + $shortdigests[] = $post; } - // Ensure that the creation times mean that the messages will be sent. - $this->helper_force_digest_mail_times(); - // Set the tested user's default maildigest setting. $DB->set_field('user', 'maildigest', 0, array('id' => $user->id)); @@ -557,7 +508,14 @@ class mod_forum_maildigest_testcase extends advanced_testcase { forum_set_user_maildigest($forum2, 2, $user); // One digest e-mail should be sent, and no individual notifications. - $this->helper_run_cron_check_count(1, 0, 1); - } + $expect = [ + (object) [ + 'userid' => $user->id, + 'digests' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + $this->send_digests_and_assert($user, $fulldigests, $shortdigests); + } } diff --git a/mod/forum/tests/qanda_test.php b/mod/forum/tests/qanda_test.php new file mode 100644 index 00000000000..417ea2a8482 --- /dev/null +++ b/mod/forum/tests/qanda_test.php @@ -0,0 +1,153 @@ +. + +/** + * The forum module mail generation tests for groups. + * + * @package mod_forum + * @copyright 2013 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/forum/lib.php'); +require_once(__DIR__ . '/cron_trait.php'); +require_once(__DIR__ . '/generator_trait.php'); + +/** + * The forum module mail generation tests for groups. + * + * @copyright 2013 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_forum_qanda_testcase extends advanced_testcase { + // Make use of the cron tester trait. + use mod_forum_tests_cron_trait; + + // Make use of the test generator trait. + use mod_forum_tests_generator_trait; + + /** + * @var \phpunit_message_sink + */ + protected $messagesink; + + /** + * @var \phpunit_mailer_sink + */ + protected $mailsink; + + public function setUp() { + global $CFG; + + // We must clear the subscription caches. This has to be done both before each test, and after in case of other + // tests using these functions. + \mod_forum\subscriptions::reset_forum_cache(); + \mod_forum\subscriptions::reset_discussion_cache(); + + // Messaging is not compatible with transactions... + $this->preventResetByRollback(); + + // Catch all messages. + $this->messagesink = $this->redirectMessages(); + $this->mailsink = $this->redirectEmails(); + + // Forcibly reduce the maxeditingtime to a second in the past to + // ensure that messages are sent out. + $CFG->maxeditingtime = -1; + } + + public function tearDown() { + // We must clear the subscription caches. This has to be done both before each test, and after in case of other + // tests using these functions. + \mod_forum\subscriptions::reset_forum_cache(); + + $this->messagesink->clear(); + $this->messagesink->close(); + unset($this->messagesink); + + $this->mailsink->clear(); + $this->mailsink->close(); + unset($this->mailsink); + } + + /** + * Test that a user who has not posted in a q&a forum does not receive + * notificatinos. + */ + public function test_user_has_not_posted() { + global $CFG, $DB; + + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $forum = $this->getDataGenerator()->create_module('forum', [ + 'course' => $course->id, + 'forcesubscribe' => FORUM_INITIALSUBSCRIBE, + 'groupmode' => SEPARATEGROUPS, + 'type' => 'qanda', + ]); + + // Create three students: + // - author, enrolled in group A; and + // - recipient, enrolled in group B; and + // - other, enrolled in the course, but no groups. + list($author, $recipient, $otheruser) = $this->helper_create_users($course, 3); + + // Create one editing teacher, not in any group but with accessallgroups capability. + list($editingteacher) = $this->helper_create_users($course, 1, 'editingteacher'); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $editingteacher); + $reply = $this->helper_reply_to_post($post, $author); + $otherreply = $this->helper_reply_to_post($post, $recipient); + $DB->execute("UPDATE {forum_posts} SET modified = modified - 1"); + $DB->execute("UPDATE {forum_posts} SET created = created - 1"); + $DB->execute("UPDATE {forum_discussions} SET timemodified = timemodified - 1"); + + // Only the author, recipient, and teachers should receive. + $expect = [ + 'author' => (object) [ + 'userid' => $author->id, + 'messages' => 3, + ], + 'recipient' => (object) [ + 'userid' => $recipient->id, + 'messages' => 3, + ], + 'otheruser' => (object) [ + 'userid' => $otheruser->id, + 'messages' => 1, + ], + 'editingteacher' => (object) [ + 'userid' => $editingteacher->id, + 'messages' => 3, + ], + ]; + $this->queue_tasks_and_assert($expect); + $posts = [$post, $reply, $otherreply]; + + // No notifications should be queued. + $this->send_notifications_and_assert($author, $posts); + $this->send_notifications_and_assert($recipient, $posts); + $this->send_notifications_and_assert($otheruser, [$post]); + $this->send_notifications_and_assert($editingteacher, $posts); + } +} From b231aabb2d3a33c95f666a7a2ccead6afeb48386 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 10 Jun 2018 21:34:24 +0800 Subject: [PATCH 03/10] MDL-46881 mod_forum: Remove unused functions Note: I have not applied the deprecation policy to these functions as they are extremely core to the cron functionality and were never intended to be used outside of cron. --- mod/forum/lib.php | 80 ------------- mod/forum/tests/lib_test.php | 216 ----------------------------------- 2 files changed, 296 deletions(-) diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 39c27ccd95e..83a7bc22ca0 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -1338,86 +1338,6 @@ function forum_search_posts($searchterms, $courseid=0, $limitfrom=0, $limitnum=5 return $DB->get_records_sql($searchsql, $params, $limitfrom, $limitnum); } -/** - * Returns a list of all new posts that have not been mailed yet - * - * @param int $starttime posts created after this time - * @param int $endtime posts created before this - * @param int $now used for timed discussions only - * @return array - */ -function forum_get_unmailed_posts($starttime, $endtime, $now=null) { - global $CFG, $DB; - - $params = array(); - $params['mailed'] = FORUM_MAILED_PENDING; - $params['ptimestart'] = $starttime; - $params['ptimeend'] = $endtime; - $params['mailnow'] = 1; - - if (!empty($CFG->forum_enabletimedposts)) { - if (empty($now)) { - $now = time(); - } - $selectsql = "AND (p.created >= :ptimestart OR d.timestart >= :pptimestart)"; - $params['pptimestart'] = $starttime; - $timedsql = "AND (d.timestart < :dtimestart AND (d.timeend = 0 OR d.timeend > :dtimeend))"; - $params['dtimestart'] = $now; - $params['dtimeend'] = $now; - } else { - $timedsql = ""; - $selectsql = "AND p.created >= :ptimestart"; - } - - return $DB->get_records_sql("SELECT p.*, d.course, d.forum - FROM {forum_posts} p - JOIN {forum_discussions} d ON d.id = p.discussion - WHERE p.mailed = :mailed - $selectsql - AND (p.created < :ptimeend OR p.mailnow = :mailnow) - $timedsql - ORDER BY p.modified ASC", $params); -} - -/** - * Marks posts before a certain time as being mailed already - * - * @global object - * @global object - * @param int $endtime - * @param int $now Defaults to time() - * @return bool - */ -function forum_mark_old_posts_as_mailed($endtime, $now=null) { - global $CFG, $DB; - - if (empty($now)) { - $now = time(); - } - - $params = array(); - $params['mailedsuccess'] = FORUM_MAILED_SUCCESS; - $params['now'] = $now; - $params['endtime'] = $endtime; - $params['mailnow'] = 1; - $params['mailedpending'] = FORUM_MAILED_PENDING; - - if (empty($CFG->forum_enabletimedposts)) { - return $DB->execute("UPDATE {forum_posts} - SET mailed = :mailedsuccess - WHERE (created < :endtime OR mailnow = :mailnow) - AND mailed = :mailedpending", $params); - } else { - return $DB->execute("UPDATE {forum_posts} - SET mailed = :mailedsuccess - WHERE discussion NOT IN (SELECT d.id - FROM {forum_discussions} d - WHERE d.timestart > :now) - AND (created < :endtime OR mailnow = :mailnow) - AND mailed = :mailedpending", $params); - } -} - /** * Get all the posts for a user in a forum suitable for forum_print_post * diff --git a/mod/forum/tests/lib_test.php b/mod/forum/tests/lib_test.php index cce78611b44..8099d0cffa7 100644 --- a/mod/forum/tests/lib_test.php +++ b/mod/forum/tests/lib_test.php @@ -2961,65 +2961,6 @@ class mod_forum_lib_testcase extends advanced_testcase { $this->assertEmpty($neighbours['next']); } - /** - * @dataProvider forum_get_unmailed_posts_provider - */ - public function test_forum_get_unmailed_posts($discussiondata, $enabletimedposts, $expectedcount, $expectedreplycount) { - global $CFG, $DB; - - $this->resetAfterTest(); - - // Configure timed posts. - $CFG->forum_enabletimedposts = $enabletimedposts; - - $course = $this->getDataGenerator()->create_course(); - $forum = $this->getDataGenerator()->create_module('forum', ['course' => $course->id]); - $user = $this->getDataGenerator()->create_user(); - $forumgen = $this->getDataGenerator()->get_plugin_generator('mod_forum'); - - // Keep track of the start time of the test. Do not use time() after this point to prevent random failures. - $time = time(); - - $record = new stdClass(); - $record->course = $course->id; - $record->userid = $user->id; - $record->forum = $forum->id; - if (isset($discussiondata['timecreated'])) { - $record->timemodified = $time + $discussiondata['timecreated']; - } - if (isset($discussiondata['timestart'])) { - $record->timestart = $time + $discussiondata['timestart']; - } - if (isset($discussiondata['timeend'])) { - $record->timeend = $time + $discussiondata['timeend']; - } - if (isset($discussiondata['mailed'])) { - $record->mailed = $discussiondata['mailed']; - } - - $discussion = $forumgen->create_discussion($record); - - // Fetch the unmailed posts. - $timenow = $time; - $endtime = $timenow - $CFG->maxeditingtime; - $starttime = $endtime - 2 * DAYSECS; - - $unmailed = forum_get_unmailed_posts($starttime, $endtime, $timenow); - $this->assertCount($expectedcount, $unmailed); - - // Add a reply just outside the maxeditingtime. - $replyto = $DB->get_record('forum_posts', array('discussion' => $discussion->id)); - $reply = new stdClass(); - $reply->userid = $user->id; - $reply->discussion = $discussion->id; - $reply->parent = $replyto->id; - $reply->created = max($replyto->created, $endtime - 1); - $forumgen->create_post($reply); - - $unmailed = forum_get_unmailed_posts($starttime, $endtime, $timenow); - $this->assertCount($expectedreplycount, $unmailed); - } - /** * Test for forum_is_author_hidden. */ @@ -3055,163 +2996,6 @@ class mod_forum_lib_testcase extends advanced_testcase { forum_is_author_hidden($post, $forum); } - public function forum_get_unmailed_posts_provider() { - return [ - 'Untimed discussion; Single post; maxeditingtime not expired' => [ - 'discussion' => [ - ], - 'timedposts' => false, - 'postcount' => 0, - 'replycount' => 0, - ], - 'Untimed discussion; Single post; maxeditingtime expired' => [ - 'discussion' => [ - 'timecreated' => - DAYSECS, - ], - 'timedposts' => false, - 'postcount' => 1, - 'replycount' => 2, - ], - 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime not expired' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => 0, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 0, - ], - 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => - DAYSECS, - ], - 'timedposts' => true, - 'postcount' => 1, - 'replycount' => 2, - ], - 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend not reached' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => - DAYSECS, - 'timeend' => + DAYSECS - ], - 'timedposts' => true, - 'postcount' => 1, - 'replycount' => 2, - ], - 'Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend passed' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => - DAYSECS, - 'timeend' => - HOURSECS, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 0, - ], - 'Timed discussion; Single post; Posted 1 week ago; timeend not reached' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timeend' => + DAYSECS - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 1, - ], - 'Timed discussion; Single post; Posted 1 week ago; timeend passed' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timeend' => - DAYSECS, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 0, - ], - - 'Previously mailed; Untimed discussion; Single post; maxeditingtime not expired' => [ - 'discussion' => [ - 'mailed' => 1, - ], - 'timedposts' => false, - 'postcount' => 0, - 'replycount' => 0, - ], - - 'Previously mailed; Untimed discussion; Single post; maxeditingtime expired' => [ - 'discussion' => [ - 'timecreated' => - DAYSECS, - 'mailed' => 1, - ], - 'timedposts' => false, - 'postcount' => 0, - 'replycount' => 1, - ], - 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime not expired' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => 0, - 'mailed' => 1, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 0, - ], - 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => - DAYSECS, - 'mailed' => 1, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 1, - ], - 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend not reached' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => - DAYSECS, - 'timeend' => + DAYSECS, - 'mailed' => 1, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 1, - ], - 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timestart maxeditingtime expired; timeend passed' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timestart' => - DAYSECS, - 'timeend' => - HOURSECS, - 'mailed' => 1, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 0, - ], - 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timeend not reached' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timeend' => + DAYSECS, - 'mailed' => 1, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 1, - ], - 'Previously mailed; Timed discussion; Single post; Posted 1 week ago; timeend passed' => [ - 'discussion' => [ - 'timecreated' => - WEEKSECS, - 'timeend' => - DAYSECS, - 'mailed' => 1, - ], - 'timedposts' => true, - 'postcount' => 0, - 'replycount' => 0, - ], - ]; - } - /** * Test the forum_discussion_is_locked function. * From c0284ccbc625b71ab4a92ea22c55517a6f4c9a76 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 23 Jan 2019 11:15:20 +0800 Subject: [PATCH 04/10] MDL-46881 forum: Ensure that there are posts to remove --- mod/forum/classes/task/send_user_digests.php | 22 ++++++++++++++++++- .../classes/task/send_user_notifications.php | 5 +++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/mod/forum/classes/task/send_user_digests.php b/mod/forum/classes/task/send_user_digests.php index 816ad76d738..395e61fe624 100644 --- a/mod/forum/classes/task/send_user_digests.php +++ b/mod/forum/classes/task/send_user_digests.php @@ -307,6 +307,12 @@ class send_user_digests extends \core\task\adhoc_task { } $posts->close(); + if (empty($discussionids)) { + // All posts have been removed since the task was queued. + $this->empty_queue($this->recipient->id, $timenow); + return; + } + list($in, $params) = $DB->get_in_or_equal($discussionids); $this->discussions = $DB->get_records_select('forum_discussions', "id {$in}", $params); @@ -321,7 +327,21 @@ class send_user_digests extends \core\task\adhoc_task { $this->fill_digest_cache(); - $DB->delete_records_select('forum_queue', "userid = :userid AND timemodified < :timemodified", $queueparams); + $this->empty_queue($this->recipient->id, $timenow); + } + + /** + * Empty the queue of posts for this user. + * + * @param array $queueparams The list of + */ + protected function empty_queue(int $userid, int $timemodified) : void { + global $DB; + + $DB->delete_records_select('forum_queue', "userid = :userid AND timemodified < :timemodified", [ + 'userid' => $userid, + 'timemodified' => $timemodified, + ]); } /** diff --git a/mod/forum/classes/task/send_user_notifications.php b/mod/forum/classes/task/send_user_notifications.php index cd146d57b58..0b6bd9eff4e 100644 --- a/mod/forum/classes/task/send_user_notifications.php +++ b/mod/forum/classes/task/send_user_notifications.php @@ -213,6 +213,11 @@ class send_user_notifications extends \core\task\adhoc_task { } $posts->close(); + if (empty($discussionids)) { + // All posts have been removed since the task was queued. + return; + } + // Fetch all discussions. list($in, $params) = $DB->get_in_or_equal(array_values($discussionids)); $this->discussions = $DB->get_records_select('forum_discussions', "id {$in}", $params); From af48d8bdd660e6531842766fc91037ebd2b2a16b Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 23 Jan 2019 10:08:53 +0800 Subject: [PATCH 05/10] MDL-46881 forum: Defer post visibility check to send time --- mod/forum/classes/task/cron_task.php | 58 ---------------------------- mod/forum/tests/mail_test.php | 8 ++-- mod/forum/tests/qanda_test.php | 2 +- 3 files changed, 5 insertions(+), 63 deletions(-) diff --git a/mod/forum/classes/task/cron_task.php b/mod/forum/classes/task/cron_task.php index ee2d2961934..2d0fd60a803 100644 --- a/mod/forum/classes/task/cron_task.php +++ b/mod/forum/classes/task/cron_task.php @@ -173,10 +173,6 @@ class cron_task extends \core\task\scheduled_task { $this->fill_digest_cache(); $this->log_finish("Done", 1); - $this->log_start("Filling user posted cache for Q&A forums", 1); - $this->fetch_user_has_posted(); - $this->log_finish("Done", 1); - $this->log_start("Queueing user tasks."); $this->queue_user_tasks(); $this->log_finish("All tasks queued.", 1); @@ -311,44 +307,6 @@ class cron_task extends \core\task\scheduled_task { } } - /** - * Fill the cache for Q&A forums. - */ - protected function fetch_user_has_posted() { - global $DB; - - $forums = array_filter($this->forums, function($forum) { - return $forum->type === 'qanda'; - }); - - if (empty($forums)) { - return; - } - - list($in, $params) = $DB->get_in_or_equal(array_keys($forums)); - - $sql = "SELECT d.forum, d.firstpost, p.userid - FROM {forum} f - INNER JOIN {forum_discussions} d ON d.forum = f.id - LEFT JOIN {forum_posts} p ON p.discussion = d.id - WHERE f.type = 'qanda' - AND f.id {$in} - GROUP BY p.userid, d.forum, d.firstpost"; - - $rs = $DB->get_recordset_sql($sql, $params); - foreach ($rs as $row) { - if (empty($this->qandametadata[$row->forum])) { - $this->qandametadata[$row->forum] = (object) [ - 'users' => [], - 'firstpost' => [$row->firstpost], - ]; - } - $this->qandametadata[$row->forum]->users[$row->userid] = true; - } - $rs->close(); - - } - /** * Queue the user tasks. */ @@ -500,22 +458,6 @@ class cron_task extends \core\task\scheduled_task { } } - if ($forum->type === 'qanda' && $postid != $discussion->firstpost) { - $hasqandapost = isset($this->qandametadata[$forumid]); - $hasqandapost = $hasqandapost && isset($this->qandametadata[$forumid]->users[$user->id]); - if (!$hasqandapost) { - // The user has not posted to this qanda forum. - unset($poststructure[$courseid][$forumid][$discussionid]); - continue; - } - } - - if (!forum_user_can_see_post($forum, $discussion, $post, null, $cm)) { - // The user is not allowed to see the post for some other reason. - unset($poststructure[$courseid][$forumid][$discussionid][$postid]); - continue; - } - if ($maildigest > 0) { // This user wants the mails to be in digest form. $digeststructure[] = (object) [ diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php index 957a4edcc9d..8bf818f94ba 100644 --- a/mod/forum/tests/mail_test.php +++ b/mod/forum/tests/mail_test.php @@ -1215,11 +1215,11 @@ class mod_forum_mail_testcase extends advanced_testcase { $expect = [ 'author' => (object) [ 'userid' => $author->id, - 'messages' => 0, + 'messages' => 1, ], 'recipient' => (object) [ 'userid' => $recipient->id, - 'messages' => 0, + 'messages' => 1, ], 'editor' => (object) [ 'userid' => $editor->id, @@ -1229,8 +1229,8 @@ class mod_forum_mail_testcase extends advanced_testcase { $this->queue_tasks_and_assert($expect); // No notifications should be queued. - $this->send_notifications_and_assert($author, []); - $this->send_notifications_and_assert($recipient, []); + $this->send_notifications_and_assert($author, [], true); + $this->send_notifications_and_assert($recipient, [], true); $this->send_notifications_and_assert($editor, [$post], true); } diff --git a/mod/forum/tests/qanda_test.php b/mod/forum/tests/qanda_test.php index 417ea2a8482..3e4795250ab 100644 --- a/mod/forum/tests/qanda_test.php +++ b/mod/forum/tests/qanda_test.php @@ -134,7 +134,7 @@ class mod_forum_qanda_testcase extends advanced_testcase { ], 'otheruser' => (object) [ 'userid' => $otheruser->id, - 'messages' => 1, + 'messages' => 3, ], 'editingteacher' => (object) [ 'userid' => $editingteacher->id, From 0dc54176ddc3379bbe9b92543af3a484b0e1e9f2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 23 Jan 2019 11:33:17 +0800 Subject: [PATCH 06/10] MDL-46881 forum: Mark posts as ready properly --- mod/forum/classes/task/send_user_notifications.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod/forum/classes/task/send_user_notifications.php b/mod/forum/classes/task/send_user_notifications.php index 0b6bd9eff4e..552177a6307 100644 --- a/mod/forum/classes/task/send_user_notifications.php +++ b/mod/forum/classes/task/send_user_notifications.php @@ -168,7 +168,7 @@ class send_user_notifications extends \core\task\adhoc_task { if (get_user_preferences('forum_markasreadonnotification', 1, $this->recipient->id) == 1) { $this->log_start("Marking posts as read"); $count = count($markposts); - forum_tp_mark_posts_read($this->recipient, $markposts); + forum_tp_mark_posts_read($this->recipient, array_keys($markposts)); $this->log_finish("Marked {$count} posts as read"); } } From 4129d898f75bea31b800c8ad3bd996d74bcc1601 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 29 Jan 2019 09:16:47 +0800 Subject: [PATCH 07/10] MDL-46881 forum: Tidy up trace variables --- lib/classes/task/logging_trait.php | 23 +++++++++++------------ mod/forum/classes/task/cron_task.php | 10 +++------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/lib/classes/task/logging_trait.php b/lib/classes/task/logging_trait.php index ac2d4014a0b..8701c2a5ae2 100644 --- a/lib/classes/task/logging_trait.php +++ b/lib/classes/task/logging_trait.php @@ -38,12 +38,12 @@ trait logging_trait { /** * @var \progress_trace */ - protected $_trace = null; + protected $trace = null; /** * @var \stdClass */ - protected $_tracestats = null; + protected $tracestats = null; /** * Get the progress_trace. @@ -51,12 +51,12 @@ trait logging_trait { * @return \progress_trace */ protected function get_trace() { - if (null === $this->_trace) { - $this->_trace = new \text_progress_trace(); - $this->_tracestats = (object) []; + if (null === $this->trace) { + $this->trace = new \text_progress_trace(); + $this->tracestats = new \stdClass(); } - return $this->_trace; + return $this->trace; } /** @@ -77,16 +77,14 @@ trait logging_trait { * @param int $depth */ protected function log_start($message, $depth = 0) { - $this->get_trace(); + $this->log($message, $depth); if (defined('MDL_PERFTOLOG') && MDL_PERFTOLOG) { - $this->_tracestats->$depth = [ + $this->tracestats->$depth = [ 'mem' => memory_get_usage(), 'time' => microtime(), ]; } - - $this->log($message, $depth); } /** @@ -97,8 +95,9 @@ trait logging_trait { */ protected function log_finish($message, $depth = 0) { $this->log($message, $depth); - if (isset($this->_tracestats->$depth)) { - $startstats = $this->_tracestats->$depth; + + if (isset($this->tracestats->$depth)) { + $startstats = $this->tracestats->$depth; $this->log( sprintf("Time taken %s, memory total: %s, Memory growth: %s, Memory peak: %s", microtime_diff($startstats['time'], microtime()), diff --git a/mod/forum/classes/task/cron_task.php b/mod/forum/classes/task/cron_task.php index 2d0fd60a803..74fdfd73230 100644 --- a/mod/forum/classes/task/cron_task.php +++ b/mod/forum/classes/task/cron_task.php @@ -125,8 +125,6 @@ class cron_task extends \core\task\scheduled_task { * @param \stdClass[] $posts */ protected function process_post_data($posts) { - $trace = $this->get_trace(); - $discussionids = []; $forumids = []; $courseids = []; @@ -173,11 +171,11 @@ class cron_task extends \core\task\scheduled_task { $this->fill_digest_cache(); $this->log_finish("Done", 1); + $this->log_finish("All caches filled"); + $this->log_start("Queueing user tasks."); $this->queue_user_tasks(); - $this->log_finish("All tasks queued.", 1); - - $this->log_finish("All caches filled"); + $this->log_finish("All tasks queued."); } /** @@ -313,8 +311,6 @@ class cron_task extends \core\task\scheduled_task { protected function queue_user_tasks() { global $CFG, $DB; - $trace = $this->get_trace(); - $timenow = time(); $sitetimezone = \core_date::get_server_timezone(); $counts = [ From b51b696927c8861b0aa2382626be85768ba0c96f Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 29 Jan 2019 09:17:19 +0800 Subject: [PATCH 08/10] MDL-46881 core: Allow adhoc tasks to be rescheduled --- lib/classes/task/manager.php | 39 +++++++++- lib/tests/adhoc_task_test.php | 108 +++++++++++++++++++++++++++ lib/upgrade.txt | 2 + mod/forum/classes/task/cron_task.php | 2 +- 4 files changed, 148 insertions(+), 3 deletions(-) diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index 59520182142..87fe34287ac 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -129,7 +129,18 @@ class manager { * @return bool */ protected static function task_is_scheduled($task) { + return false !== self::get_queued_adhoc_task_record($task); + } + + /** + * Checks if the task with the same classname, component and customdata is already scheduled + * + * @param adhoc_task $task + * @return bool + */ + protected static function get_queued_adhoc_task_record($task) { global $DB; + $record = self::record_from_adhoc_task($task); $params = [$record->classname, $record->component, $record->customdata]; $sql = 'classname = ? AND component = ? AND ' . @@ -139,14 +150,38 @@ class manager { $params[] = $record->userid; $sql .= " AND userid = ? "; } - return $DB->record_exists_select('task_adhoc', $sql, $params); + return $DB->get_record_select('task_adhoc', $sql, $params); + } + + /** + * Schedule a new task, or reschedule an existing adhoc task which has matching data. + * + * Only a task matching the same user, classname, component, and customdata will be rescheduled. + * If these values do not match exactly then a new task is scheduled. + * + * @param \core\task\adhoc_task $task - The new adhoc task information to store. + * @since Moodle 3.7 + */ + public static function reschedule_or_queue_adhoc_task(adhoc_task $task) : void { + global $DB; + + if ($existingrecord = self::get_queued_adhoc_task_record($task)) { + // Only update the next run time if it is explicitly set on the task. + $nextruntime = $task->get_next_run_time(); + if ($nextruntime && ($existingrecord->nextruntime != $nextruntime)) { + $DB->set_field('task_adhoc', 'nextruntime', $nextruntime, ['id' => $existingrecord->id]); + } + } else { + // There is nothing queued yet. Just queue as normal. + self::queue_adhoc_task($task); + } } /** * Queue an adhoc task to run in the background. * * @param \core\task\adhoc_task $task - The new adhoc task information to store. - * @param bool $checkforexisting - If set to true and the task with the same classname, component and customdata + * @param bool $checkforexisting - If set to true and the task with the same user, classname, component and customdata * is already scheduled then it will not schedule a new task. Can be used only for ASAP tasks. * @return boolean - True if the config was saved. */ diff --git a/lib/tests/adhoc_task_test.php b/lib/tests/adhoc_task_test.php index 2b8b7d1e19d..c5d796102f8 100644 --- a/lib/tests/adhoc_task_test.php +++ b/lib/tests/adhoc_task_test.php @@ -150,6 +150,114 @@ class core_adhoc_task_testcase extends advanced_testcase { } } + /** + * Ensure that the reschedule_or_queue_adhoc_task function will schedule a new task if no tasks exist. + */ + public function test_reschedule_or_queue_adhoc_task_no_existing() { + $this->resetAfterTest(true); + + // Schedule adhoc task. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + $this->assertEquals(1, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + } + + /** + * Ensure that the reschedule_or_queue_adhoc_task function will schedule a new task if a task for the same user does + * not exist. + */ + public function test_reschedule_or_queue_adhoc_task_different_user() { + $this->resetAfterTest(true); + $user = \core_user::get_user_by_username('admin'); + + // Schedule adhoc task. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + // Schedule adhoc task for a different user. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + $task->set_userid($user->id); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + $this->assertEquals(2, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + } + + /** + * Ensure that the reschedule_or_queue_adhoc_task function will schedule a new task if a task with different custom + * data exists. + */ + public function test_reschedule_or_queue_adhoc_task_different_data() { + $this->resetAfterTest(true); + + // Schedule adhoc task. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + // Schedule adhoc task for a different user. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 11]); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + $this->assertEquals(2, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + } + + /** + * Ensure that the reschedule_or_queue_adhoc_task function will not make any change for matching data if no time was + * specified. + */ + public function test_reschedule_or_queue_adhoc_task_match_no_change() { + $this->resetAfterTest(true); + + // Schedule adhoc task. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + $task->set_next_run_time(time() + DAYSECS); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + $before = \core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'); + + // Schedule the task again but do not specify a time. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + $this->assertEquals(1, count(\core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'))); + $this->assertEquals($before, \core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task')); + } + + /** + * Ensure that the reschedule_or_queue_adhoc_task function will update the run time if there are planned changes. + */ + public function test_reschedule_or_queue_adhoc_task_match_update_runtime() { + $this->resetAfterTest(true); + $initialruntime = time() + DAYSECS; + $newruntime = time() + WEEKSECS; + + // Schedule adhoc task. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + $task->set_next_run_time($initialruntime); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + $before = \core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'); + + // Schedule the task again. + $task = new \core\task\adhoc_test_task(); + $task->set_custom_data(['courseid' => 10]); + $task->set_next_run_time($newruntime); + \core\task\manager::reschedule_or_queue_adhoc_task($task); + + $tasks = \core\task\manager::get_adhoc_tasks('core\task\adhoc_test_task'); + $this->assertEquals(1, count($tasks)); + $this->assertNotEquals($before, $tasks); + $firsttask = reset($tasks); + $this->assertEquals($newruntime, $firsttask->get_next_run_time()); + } + /** * Test queue_adhoc_task "if not scheduled". */ diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 85225fdfb9d..54e55231053 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -6,6 +6,8 @@ information provided here is intended especially for developers. * The method core_user::is_real_user() now returns false for userid = 0 parameter * 'mform1' dependencies (in themes, js...) will stop working because a randomly generated string has been added to the id attribute on forms to avoid collisions in forms loaded in AJAX requests. +* A new method to allow queueing or rescheduling of an existing scheduled task was added. This allows an existing task + to be updated or queued as required. This new functionality can be found in \core\task\manager::reschedule_or_queue_adhoc_task. === 3.6 === diff --git a/mod/forum/classes/task/cron_task.php b/mod/forum/classes/task/cron_task.php index 74fdfd73230..5b88606c059 100644 --- a/mod/forum/classes/task/cron_task.php +++ b/mod/forum/classes/task/cron_task.php @@ -342,7 +342,7 @@ class cron_task extends \core\task\scheduled_task { $task->set_userid($user->id); $task->set_component('mod_forum'); $task->set_next_run_time($digesttime); - \core\task\manager::queue_adhoc_task($task, true); + \core\task\manager::reschedule_or_queue_adhoc_task($task); $usercounts['digests']++; $send = true; } From ba1a8ef7829e42c52f9777defd78f6db0659ec4c Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 29 Jan 2019 09:36:21 +0800 Subject: [PATCH 09/10] MDL-46881 forum: Adjust limits for tasks --- mod/forum/classes/task/send_user_digests.php | 3 --- mod/forum/classes/task/send_user_notifications.php | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mod/forum/classes/task/send_user_digests.php b/mod/forum/classes/task/send_user_digests.php index 395e61fe624..30fd7380cf0 100644 --- a/mod/forum/classes/task/send_user_digests.php +++ b/mod/forum/classes/task/send_user_digests.php @@ -138,9 +138,6 @@ class send_user_digests extends \core\task\adhoc_task { * Send out messages. */ public function execute() { - // Terminate if not able to fetch all digests in 5 minutes. - \core_php_time_limit::raise(300); - $starttime = time(); $this->recipient = \core_user::get_user($this->get_userid()); diff --git a/mod/forum/classes/task/send_user_notifications.php b/mod/forum/classes/task/send_user_notifications.php index 552177a6307..acc40344aae 100644 --- a/mod/forum/classes/task/send_user_notifications.php +++ b/mod/forum/classes/task/send_user_notifications.php @@ -99,6 +99,9 @@ class send_user_notifications extends \core\task\adhoc_task { public function execute() { global $CFG; + // Raise the time limit for each discussion. + \core_php_time_limit::raise(120); + $this->recipient = \core_user::get_user($this->get_userid()); // Create the generic messageinboundgenerator. From cd029f936df79cc9f4ecd5d77f207dc02040aa16 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 30 Jan 2019 08:10:10 +0800 Subject: [PATCH 10/10] MDL-46881 forum: Only skip post, not discussion --- mod/forum/classes/task/cron_task.php | 2 +- mod/forum/tests/mail_test.php | 46 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/mod/forum/classes/task/cron_task.php b/mod/forum/classes/task/cron_task.php index 5b88606c059..ca3b41e3a8e 100644 --- a/mod/forum/classes/task/cron_task.php +++ b/mod/forum/classes/task/cron_task.php @@ -449,7 +449,7 @@ class cron_task extends \core\task\scheduled_task { $subscribedafter = $subscribedafter && ($subscriptiontime[$post->discussion] > $post->created); if ($subscribedafter) { // The user subscribed to the discussion/forum after this post was created. - unset($poststructure[$courseid][$forumid][$discussionid]); + unset($poststructure[$courseid][$forumid][$discussionid][$postid]); continue; } } diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php index 8bf818f94ba..9e1afdfceef 100644 --- a/mod/forum/tests/mail_test.php +++ b/mod/forum/tests/mail_test.php @@ -529,6 +529,52 @@ class mod_forum_mail_testcase extends advanced_testcase { $this->send_notifications_and_assert($recipient, [$reply]); } + public function test_optional_with_subscribed_discussion_and_post() { + $this->resetAfterTest(true); + + // Create a course, with a forum. + $course = $this->getDataGenerator()->create_course(); + + $options = array('course' => $course->id, 'forcesubscribe' => FORUM_CHOOSESUBSCRIBE); + $forum = $this->getDataGenerator()->create_module('forum', $options); + + // Create two users enrolled in the course as students. + list($author, $recipient) = $this->helper_create_users($course, 2); + + // Post a discussion to the forum. + list($discussion, $post) = $this->helper_post_to_forum($forum, $author); + $this->helper_update_post_time($post, -90); + + // Have a user reply to the discussion before we subscribed. + $reply = $this->helper_post_to_discussion($forum, $discussion, $author); + $this->helper_update_post_time($reply, -75); + + // Subscribe the 'recipient' user to the discussion. + \mod_forum\subscriptions::subscribe_user_to_discussion($recipient->id, $discussion); + $this->helper_update_subscription_time($recipient, $discussion, -60); + + // Have a user reply to the discussion. + $reply = $this->helper_post_to_discussion($forum, $discussion, $author); + $this->helper_update_post_time($reply, -30); + + // We expect only one user to receive this post. + // The original post won't be received as it was written before the user subscribed. + $expect = [ + (object) [ + 'userid' => $author->id, + 'messages' => 0, + ], + (object) [ + 'userid' => $recipient->id, + 'messages' => 1, + ], + ]; + $this->queue_tasks_and_assert($expect); + + $this->send_notifications_and_assert($author, []); + $this->send_notifications_and_assert($recipient, [$reply]); + } + public function test_automatic_with_subscribed_discussion_in_unsubscribed_forum() { $this->resetAfterTest(true);