MDL-47903 mod_forum: Add discussion subscription time

This helps us to only send notifications for messages the user should have
already seen.
This commit is contained in:
Andrew Nicols 2014-10-28 08:31:31 +08:00
parent 4f82670250
commit eb451c7981
9 changed files with 148 additions and 41 deletions

View File

@ -34,13 +34,6 @@ defined('MOODLE_INTERNAL') || die();
*/
class subscriptions {
/**
* The status value for a subscribed discussion.
*
* @const int
*/
const FORUM_DISCUSSION_SUBSCRIBED = 1;
/**
* The status value for an unsubscribed discussion.
*
@ -126,7 +119,7 @@ class subscriptions {
// Check whether there is a record for this discussion subscription.
if (isset($subscriptions[$discussionid])) {
return ($subscriptions[$discussionid] == self::FORUM_DISCUSSION_SUBSCRIBED);
return ($subscriptions[$discussionid] != self::FORUM_DISCUSSION_UNSUBSCRIBED);
}
return self::is_subscribed_to_forum($userid, $forum);
@ -605,14 +598,18 @@ class subscriptions {
if ($userrequest) {
$discussionsubscriptions = $DB->get_recordset('forum_discussion_subs', array('userid' => $userid, 'forum' => $forum->id));
$DB->delete_records('forum_discussion_subs',
array('userid' => $userid, 'forum' => $forum->id, 'preference' => self::FORUM_DISCUSSION_SUBSCRIBED));
$DB->delete_records_select('forum_discussion_subs',
'userid = :userid AND forum = :forumid AND preference <> :preference', array(
'userid' => $userid,
'forumid' => $forum->id,
'preference' => self::FORUM_DISCUSSION_UNSUBSCRIBED,
));
// Reset the subscription caches for this forum.
// We know that the there were previously entries and there aren't any more.
if (isset(self::$forumdiscussioncache[$userid]) && isset(self::$forumdiscussioncache[$userid][$forum->id])) {
foreach (self::$forumdiscussioncache[$userid][$forum->id] as $discussionid => $preference) {
if ($preference == self::FORUM_DISCUSSION_SUBSCRIBED) {
if ($preference != self::FORUM_DISCUSSION_UNSUBSCRIBED) {
unset(self::$forumdiscussioncache[$userid][$forum->id][$discussionid]);
}
}
@ -716,7 +713,7 @@ class subscriptions {
// First check whether the user is subscribed to the discussion already.
$subscription = $DB->get_record('forum_discussion_subs', array('userid' => $userid, 'discussion' => $discussion->id));
if ($subscription) {
if ($subscription->preference == self::FORUM_DISCUSSION_SUBSCRIBED) {
if ($subscription->preference != self::FORUM_DISCUSSION_UNSUBSCRIBED) {
// The user is already subscribed to the discussion. Ignore.
return false;
}
@ -733,17 +730,17 @@ class subscriptions {
}
} else {
if ($subscription) {
$subscription->preference = self::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$DB->update_record('forum_discussion_subs', $subscription);
} else {
$subscription = new \stdClass();
$subscription->userid = $userid;
$subscription->forum = $discussion->forum;
$subscription->discussion = $discussion->id;
$subscription->preference = self::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
self::$forumdiscussioncache[$userid][$discussion->forum][$discussion->id] = true;
self::$forumdiscussioncache[$userid][$discussion->forum][$discussion->id] = $subscription->preference;
}
}
@ -785,7 +782,7 @@ class subscriptions {
}
// No discussion-level preference. Check for a forum level subscription.
if (!$DB->record_exists('forum_subscriptions', array('userid' => $userid, 'forum' => $discussion->forum))) {
if ($subscription && $subscription->preference == self::FORUM_DISCUSSION_SUBSCRIBED) {
if ($subscription && $subscription->preference != self::FORUM_DISCUSSION_UNSUBSCRIBED) {
// The user is not subscribed to the forum, but subscribed from the discussion, delete the discussion subscription.
$DB->delete_records('forum_discussion_subs', array('id' => $subscription->id));
unset(self::$forumdiscussioncache[$userid][$discussion->forum][$discussion->id]);
@ -806,7 +803,7 @@ class subscriptions {
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
}
self::$forumdiscussioncache[$userid][$discussion->forum][$discussion->id] = false;
self::$forumdiscussioncache[$userid][$discussion->forum][$discussion->id] = $subscription->preference;
}
$context = forum_get_context($discussion->forum, $context);

View File

@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8" ?>
<XMLDB PATH="mod/forum/db" VERSION="20140815" COMMENT="XMLDB file for Moodle mod/forum"
<XMLDB PATH="mod/forum/db" VERSION="20141028" COMMENT="XMLDB file for Moodle mod/forum"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
>
@ -173,7 +173,7 @@
<FIELD NAME="forum" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="userid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="discussion" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="preference" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="1" SEQUENCE="false"/>
<FIELD NAME="preference" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="1" SEQUENCE="false"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>

View File

@ -182,7 +182,7 @@ function xmldb_forum_upgrade($oldversion) {
$table->add_field('forum', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
$table->add_field('userid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
$table->add_field('discussion', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
$table->add_field('preference', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '1');
$table->add_field('preference', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '1');
// Adding keys to table forum_discussion_subs.
$table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
@ -197,7 +197,20 @@ function xmldb_forum_upgrade($oldversion) {
}
// Forum savepoint reached.
upgrade_mod_savepoint(true, 2014081900, 'forum');
upgrade_mod_savepoint(true, 2014102800, 'forum');
}
if ($oldversion > 2014081900 && $oldversion < 2014102800) {
// Changing precision of field preference on table forum_discussion_subs to (10).
$table = new xmldb_table('forum_discussion_subs');
$field = new xmldb_field('preference', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '1', 'discussion');
// Launch change of precision for field preference.
$dbman->change_field_precision($table, $field);
// Forum savepoint reached.
upgrade_mod_savepoint(true, 2014102800, 'forum');
}
return true;

View File

@ -114,12 +114,13 @@
// And also for the discussion being moved.
\mod_forum\subscriptions::fill_subscription_cache($forum->id);
$subscriptionchanges = array();
$subscriptiontime = time();
foreach ($potentialsubscribers as $subuser) {
$userid = $subuser->id;
$targetsubscription = \mod_forum\subscriptions::is_subscribed($userid, $forumto, null, $cmto);
if (\mod_forum\subscriptions::is_subscribed($userid, $forum, $discussion->id)) {
if (!$targetsubscription) {
$subscriptionchanges[$userid] = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscriptionchanges[$userid] = $subscriptiontime;
}
} else {
if ($targetsubscription) {
@ -136,7 +137,7 @@
$newdiscussion = clone $discussion;
$newdiscussion->forum = $forumto->id;
foreach ($subscriptionchanges as $userid => $preference) {
if ($preference === \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED) {
if ($preference != \mod_forum\subscriptions::FORUM_DISCUSSION_UNSUBSCRIBED) {
// Users must have viewdiscussion to a discussion.
if (has_capability('mod/forum:viewdiscussion', $destinationctx, $userid)) {
\mod_forum\subscriptions::subscribe_user_to_discussion($userid, $newdiscussion, $destinationctx);

View File

@ -630,6 +630,13 @@ function forum_cron() {
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;
}
}
// 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) {

View File

@ -1914,7 +1914,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -1973,7 +1973,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2027,7 +2027,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2078,7 +2078,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2129,7 +2129,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2317,7 +2317,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2371,7 +2371,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2422,7 +2422,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);
@ -2473,7 +2473,7 @@ class mod_forum_events_testcase extends advanced_testcase {
$subscription->userid = $user->id;
$subscription->forum = $forum->id;
$subscription->discussion = $discussion->id;
$subscription->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$subscription->preference = time();
$subscription->id = $DB->insert_record('forum_discussion_subs', $subscription);

View File

@ -132,6 +132,7 @@ class mod_forum_mail_testcase extends advanced_testcase {
* @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();
@ -149,6 +150,35 @@ class mod_forum_mail_testcase extends advanced_testcase {
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_forced_subscription() {
$this->resetAfterTest(true);
@ -409,6 +439,9 @@ class mod_forum_mail_testcase extends advanced_testcase {
// Unsubscribe the 'author' user from the discussion.
\mod_forum\subscriptions::unsubscribe_user_from_discussion($author->id, $discussion);
$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;
@ -547,4 +580,64 @@ class mod_forum_mail_testcase extends advanced_testcase {
// Run cron and check that the expected number of users received the notification.
$messages = $this->helper_run_cron_check_count($post, $expected);
}
/**
* Test that a user unsubscribed from a forum who has subscribed to a discussion, only receives posts made after
* they subscribed to the discussion.
*/
public function test_forum_discussion_subscription_forum_unsubscribed_discussion_subscribed_after_post() {
$this->resetAfterTest(true);
global $DB;
// 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);
$expectedmessages = array();
// Create a user enrolled in the course as a student.
list($author) = $this->helper_create_users($course, 1);
// Post a discussion to the forum.
list($discussion, $post) = $this->helper_post_to_forum($forum, $author);
// Update the original post to have a timecreated in the past by a few minutes.
$post->created = $post->created - 600;
$DB->update_record('forum_posts', $post);
$expectedmessages[] = array(
'id' => $post->id,
'subject' => $post->subject,
'count' => 0,
);
// Then subscribe the user to the discussion.
$this->assertTrue(\mod_forum\subscriptions::subscribe_user_to_discussion($author->id, $discussion));
// Then post a reply to the first discussion.
$record = new stdClass();
$record->course = $forum->course;
$record->userid = $author->id;
$record->forum = $forum->id;
$record->discussion = $discussion->id;
$record->mailnow = 1;
$record->created = time();
$generator = $this->getDataGenerator()->get_plugin_generator('mod_forum');
$reply = $generator->create_post($record);
$expectedmessages[] = array(
'id' => $reply->id,
'subject' => $reply->subject,
'count' => 1,
);
// We expect there to be two notifications, and not three.
$expectedcount = 1;
// Run cron and check that the expected number of users received the notification.
$messages = $this->helper_run_cron_check_counts($expectedmessages, $expectedcount);
}
}

View File

@ -79,6 +79,7 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
* @param array An array containing the discussion object, and the post object
*/
protected function helper_post_to_forum($forum, $author) {
global $DB;
$generator = $this->getDataGenerator()->get_plugin_generator('mod_forum');
// Create a discussion in the forum, and then add a post to that discussion.
@ -88,13 +89,8 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
$record->forum = $forum->id;
$discussion = $generator->create_discussion($record);
$record = new stdClass();
$record->course = $forum->course;
$record->userid = $author->id;
$record->forum = $forum->id;
$record->discussion = $discussion->id;
$record->mailnow = 1;
$post = $generator->create_post($record);
// Retrieve the post which was created by create_discussion.
$post = $DB->get_record('forum_posts', array('discussion' => $discussion->id));
return array($discussion, $post);
}
@ -1046,7 +1042,7 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
$record->userid = $users[2]->id;
$record->forum = $forum->id;
$record->discussion = $discussion->id;
$record->preference = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
$record->preference = time();
$DB->insert_record('forum_discussion_subs', $record);
// The discussion count should not have changed.

View File

@ -24,6 +24,6 @@
defined('MOODLE_INTERNAL') || die();
$plugin->version = 2014082101; // The current module version (Date: YYYYMMDDXX)
$plugin->version = 2014102800; // The current module version (Date: YYYYMMDDXX)
$plugin->requires = 2014050800; // Requires this Moodle version
$plugin->component = 'mod_forum'; // Full name of the plugin (used for diagnostics)