From e31f62a788121279ae3dec57830b0983ac9c6385 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Mon, 20 May 2019 18:39:13 +0200
Subject: [PATCH] MDL-65655 forum: Do not send notifications to inactive users

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 |  4 ++-
 mod/forum/tests/mail_test.php       | 47 +++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/mod/forum/classes/subscriptions.php b/mod/forum/classes/subscriptions.php
index 5f50eb8de25..74181b6e982 100644
--- a/mod/forum/classes/subscriptions.php
+++ b/mod/forum/classes/subscriptions.php
@@ -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);
diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php
index e6ecae3b791..4b1654ce868 100644
--- a/mod/forum/tests/mail_test.php
+++ b/mod/forum/tests/mail_test.php
@@ -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);