diff --git a/mod/forum/classes/subscriptions.php b/mod/forum/classes/subscriptions.php
index 554e65f6437..f431f2d2715 100644
--- a/mod/forum/classes/subscriptions.php
+++ b/mod/forum/classes/subscriptions.php
@@ -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);
diff --git a/mod/forum/db/install.xml b/mod/forum/db/install.xml
index 10867e81c8a..73537586623 100644
--- a/mod/forum/db/install.xml
+++ b/mod/forum/db/install.xml
@@ -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"/>
diff --git a/mod/forum/db/upgrade.php b/mod/forum/db/upgrade.php
index c241541f201..52edb2e16f0 100644
--- a/mod/forum/db/upgrade.php
+++ b/mod/forum/db/upgrade.php
@@ -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;
diff --git a/mod/forum/discuss.php b/mod/forum/discuss.php
index 30bed3c2177..000f445d63e 100644
--- a/mod/forum/discuss.php
+++ b/mod/forum/discuss.php
@@ -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);
diff --git a/mod/forum/lib.php b/mod/forum/lib.php
index 1ecd6c33623..a21b2248874 100644
--- a/mod/forum/lib.php
+++ b/mod/forum/lib.php
@@ -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) {
diff --git a/mod/forum/tests/events_test.php b/mod/forum/tests/events_test.php
index 0c017bc5d3e..22edf3bc9ea 100644
--- a/mod/forum/tests/events_test.php
+++ b/mod/forum/tests/events_test.php
@@ -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);
diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php
index 29239ecf430..959c4413df1 100644
--- a/mod/forum/tests/mail_test.php
+++ b/mod/forum/tests/mail_test.php
@@ -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);
+
+ }
}
diff --git a/mod/forum/tests/subscriptions_test.php b/mod/forum/tests/subscriptions_test.php
index f161bdba495..8f57abc1106 100644
--- a/mod/forum/tests/subscriptions_test.php
+++ b/mod/forum/tests/subscriptions_test.php
@@ -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.
diff --git a/mod/forum/version.php b/mod/forum/version.php
index fc107f3ff91..095752571ae 100644
--- a/mod/forum/version.php
+++ b/mod/forum/version.php
@@ -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)