mirror of
https://github.com/moodle/moodle.git
synced 2025-01-19 06:18:28 +01:00
MDL-65888 forum: Do not send notifications to unconfirmed users
Under certain conditions, users marked as not confirmed yet may be returned as forum notification recipients. Such users are considered as inactive though and the delivery adhoc task cannot be run as these users, throwing an exception. The solution is to make sure that fetch_subscribed_users() does not include unconfirmed users, similarly to what was done for suspended users and nologin users. The added unit test also checks for deleted users. But these should never be returned as subscribed because we consider enrolled users only and deleted users are filtered out implicitly.
This commit is contained in:
parent
f3507273e9
commit
1ecf0397bb
@ -272,7 +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
|
||||
WHERE u.auth <> 'nologin' AND u.suspended = 0 AND u.confirmed = 1
|
||||
ORDER BY $sort";
|
||||
|
||||
return $DB->get_records_sql($sql, $params);
|
||||
@ -443,7 +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
|
||||
WHERE u.auth <> 'nologin' AND u.suspended = 0 AND u.confirmed = 1
|
||||
ORDER BY u.email ASC";
|
||||
|
||||
} else {
|
||||
@ -452,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 AND u.auth <> 'nologin' AND u.suspended = 0
|
||||
s.forum = :forumid AND u.auth <> 'nologin' AND u.suspended = 0 AND u.confirmed = 1
|
||||
ORDER BY u.email ASC";
|
||||
}
|
||||
$results = $DB->get_records_sql($sql, $params);
|
||||
|
@ -137,6 +137,7 @@ class mod_forum_mail_testcase extends advanced_testcase {
|
||||
}
|
||||
|
||||
public function test_forced_subscription() {
|
||||
global $DB;
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
// Create a course, with a forum.
|
||||
@ -145,8 +146,14 @@ class mod_forum_mail_testcase extends advanced_testcase {
|
||||
$options = array('course' => $course->id, 'forcesubscribe' => FORUM_FORCESUBSCRIBE);
|
||||
$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 users enrolled in the course as students.
|
||||
list($author, $recipient, $unconfirmed, $deleted) = $this->helper_create_users($course, 4);
|
||||
|
||||
// Make the third user unconfirmed (thence inactive) to make sure it does not break the notifications.
|
||||
$DB->set_field('user', 'confirmed', 0, ['id' => $unconfirmed->id]);
|
||||
|
||||
// Mark the fourth user as deleted to make sure it does not break the notifications.
|
||||
$DB->set_field('user', 'deleted', 1, ['id' => $deleted->id]);
|
||||
|
||||
// Post a discussion to the forum.
|
||||
list($discussion, $post) = $this->helper_post_to_forum($forum, $author);
|
||||
@ -160,11 +167,21 @@ class mod_forum_mail_testcase extends advanced_testcase {
|
||||
'userid' => $recipient->id,
|
||||
'messages' => 1,
|
||||
],
|
||||
(object) [
|
||||
'userid' => $unconfirmed->id,
|
||||
'messages' => 0,
|
||||
],
|
||||
(object) [
|
||||
'userid' => $deleted->id,
|
||||
'messages' => 0,
|
||||
],
|
||||
];
|
||||
$this->queue_tasks_and_assert($expect);
|
||||
|
||||
$this->send_notifications_and_assert($author, [$post]);
|
||||
$this->send_notifications_and_assert($recipient, [$post]);
|
||||
$this->send_notifications_and_assert($unconfirmed, []);
|
||||
$this->send_notifications_and_assert($deleted, []);
|
||||
}
|
||||
|
||||
/**
|
||||
|
Loading…
x
Reference in New Issue
Block a user