MDL-65655 forum: Do not send notifications to inactive users
authorDavid Mudrák <david@moodle.com>
Mon, 20 May 2019 16:39:13 +0000 (18:39 +0200)
committerDavid Mudrák <david@moodle.com>
Mon, 20 May 2019 16:46:35 +0000 (18:46 +0200)
We cannot deliver notifications to users who had subscribed to a forum
or discussion and were later inactivated either by suspending or setting
the auth method to nologin. The deliver adhoc task cannot be run as
these users, throwing the "Suspended account" exception.

The solution is to make sure that fetch_subscribed_users() does not
include those inactive users.

mod/forum/classes/subscriptions.php
mod/forum/tests/mail_test.php

index 5f50eb8..74181b6 100644 (file)
@@ -272,6 +272,7 @@ class subscriptions {
         $sql = "SELECT $fields
                 FROM {user} u
                 JOIN ($esql) je ON je.id = u.id
+               WHERE u.auth <> 'nologin' AND u.suspended = 0
             ORDER BY $sort";
 
         return $DB->get_records_sql($sql, $params);
@@ -442,6 +443,7 @@ class subscriptions {
                         ) subscriptions
                         JOIN {user} u ON u.id = subscriptions.userid
                         JOIN ($esql) je ON je.id = u.id
+                        WHERE u.auth <> 'nologin' AND u.suspended = 0
                         ORDER BY u.email ASC";
 
             } else {
@@ -450,7 +452,7 @@ class subscriptions {
                         JOIN ($esql) je ON je.id = u.id
                         JOIN {forum_subscriptions} s ON s.userid = u.id
                         WHERE
-                          s.forum = :forumid
+                          s.forum = :forumid AND u.auth <> 'nologin' AND u.suspended = 0
                         ORDER BY u.email ASC";
             }
             $results = $DB->get_records_sql($sql, $params);
index e6ecae3..4b1654c 100644 (file)
@@ -764,6 +764,53 @@ class mod_forum_mail_testcase extends advanced_testcase {
         $this->send_notifications_and_assert($author, [$reply]);
     }
 
+    public function test_subscription_by_inactive_users() {
+        global $DB;
+        $this->resetAfterTest(true);
+
+        $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, $u1, $u2, $u3) = $this->helper_create_users($course, 4);
+
+        // Subscribe the three users to the forum.
+        \mod_forum\subscriptions::subscribe_user($u1->id, $forum);
+        \mod_forum\subscriptions::subscribe_user($u2->id, $forum);
+        \mod_forum\subscriptions::subscribe_user($u3->id, $forum);
+
+        // Make the first user inactive - suspended.
+        $DB->set_field('user', 'suspended', 1, ['id' => $u1->id]);
+
+        // Make the second user inactive - unable to log in.
+        $DB->set_field('user', 'auth', 'nologin', ['id' => $u2->id]);
+
+        // Post a discussion to the forum.
+        list($discussion, $post) = $this->helper_post_to_forum($forum, $author);
+
+        $expect = [
+            (object) [
+                'userid' => $u1->id,
+                'messages' => 0,
+            ],
+            (object) [
+                'userid' => $u2->id,
+                'messages' => 0,
+            ],
+            (object) [
+                'userid' => $u3->id,
+                'messages' => 1,
+            ],
+        ];
+
+        $this->queue_tasks_and_assert($expect);
+        $this->send_notifications_and_assert($u1, []);
+        $this->send_notifications_and_assert($u2, []);
+        $this->send_notifications_and_assert($u3, [$post]);
+    }
+
     public function test_forum_message_inbound_multiple_posts() {
         $this->resetAfterTest(true);