mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-05-03 23:07:27 +02:00
Code Issues Projects Releases Wiki Activity

MDL-47107 mod_forum Optimise forum subscription checks

Browse Source 
Supply a coursemodule where relevant to the forum subscription checks.
This commit is contained in:
Andrew Nicols 2014-09-04 20:06:55 +08:00
parent 457f818026
commit 4238983e2a
9 changed files with 68 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
mod/forum/classes/observer.php
View File

@ -72,7 +72,7 @@ class mod_forum_observer {
require_once($CFG->dirroot . '/mod/forum/lib.php');
$userid = $event->relateduserid;
$sql = "SELECT f.id, cm.id AS cmid, f.forcesubscribe
$sql = "SELECT f.id, f.course as course, cm.id AS cmid, f.forcesubscribe
FROM {forum} f
JOIN {course_modules} cm ON (cm.instance = f.id)
JOIN {modules} m ON (m.id = cm.module)

14
mod/forum/classes/subscriptions.php
View File

@ -104,14 +104,18 @@ class subscriptions {
* @param int $userid The user ID
* @param \stdClass $forum The record of the forum to test
* @param int $discussionid The ID of the discussion to check
* @param \cm_info $cm The coursemodule record. If not supplied, this will be calculated using get_fast_modinfo instead.
* @return boolean
*/
public static function is_subscribed($userid, $forum, $discussionid = null) {
public static function is_subscribed($userid, $forum, $discussionid = null, cm_info $cm = null) {
// If forum is force subscribed and has allowforcesubscribe, then user is subscribed.
$cm = get_coursemodule_from_instance('forum', $forum->id);
if ($cm && self::is_forcesubscribed($forum) &&
has_capability('mod/forum:allowforcesubscribe', \context_module::instance($cm->id), $userid)) {
return true;
if (self::is_forcesubscribed($forum)) {
if (!$cm) {
$cm = get_fast_modinfo($forum->course)->instances['forum'][$forum->id];
}
if (has_capability('mod/forum:allowforcesubscribe', \context_module::instance($cm->id), $userid)) {
return true;
}
}
if ($discussionid === null) {

2
mod/forum/discuss.php
View File

@ -114,7 +114,7 @@
$subscriptionchanges = array();
foreach ($potentialsubscribers as $subuser) {
$userid = $subuser->id;
$targetsubscription = \mod_forum\subscriptions::is_subscribed($userid, $forumto);
$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;

2
mod/forum/index.php
View File

@ -192,7 +192,7 @@ if (!is_null($subscribe)) {
$cansub = false;
}
if (!\mod_forum\subscriptions::is_forcesubscribed($forum)) {
$subscribed = \mod_forum\subscriptions::is_subscribed($USER->id, $forum);
$subscribed = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, null, $cm);
$canmanageactivities = has_capability('moodle/course:manageactivities', $coursecontext, $USER->id);
if (($canmanageactivities || \mod_forum\subscriptions::is_subscribable($forum)) && $subscribe && !$subscribed && $cansub) {
\mod_forum\subscriptions::subscribe_user($USER->id, $forum, $modcontext, true);

2
mod/forum/lib.php
View File

@ -615,7 +615,7 @@ function forum_cron() {
continue;
}
if (!\mod_forum\subscriptions::is_subscribed($userto->id, $forum, $post->discussion)) {
if (!\mod_forum\subscriptions::is_subscribed($userto->id, $forum, $post->discussion, $coursemodules[$forum->id])) {
// The user does not subscribe to this forum, or to this specific discussion.
continue;
}

5
mod/forum/post.php
View File

@ -573,7 +573,8 @@ $mform_post = new mod_forum_post_form('post.php', array('course' => $course,
'modcontext' => $modcontext,
'forum' => $forum,
'post' => $post,
'subscribe' => \mod_forum\subscriptions::is_subscribed($USER->id, $forum),
'subscribe' => \mod_forum\subscriptions::is_subscribed($USER->id, $forum,
null, $cm),
'thresholdwarning' => $thresholdwarning,
'edit' => $edit), 'post', '', array('id' => 'mformforum'));
@ -616,7 +617,7 @@ $currenttext = file_prepare_draft_area($draftid_editor, $modcontext->id, 'mod_fo
// which case use their existing preference.
$discussionsubscribe = true;
if (isset($discussion) && forum_user_has_posted($forum->id, $discussion->id, $USER->id)) {
$discussionsubscribe = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id);
$discussionsubscribe = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id, $cm);
}
$mform_post->set_data(array( 'attachments'=>$draftitemid,
'general'=>$heading,

4
mod/forum/subscribe.php
View File

@ -73,7 +73,7 @@ if (isset($cm->groupmode) && empty($course->groupmodeforce)) {
} else {
$groupmode = $course->groupmode;
}
if ($groupmode && !\mod_forum\subscriptions::is_subscribed($user->id, $forum) && !has_capability('moodle/site:accessallgroups', $context)) {
if ($groupmode && !\mod_forum\subscriptions::is_subscribed($user->id, $forum, null, $cm) && !has_capability('moodle/site:accessallgroups', $context)) {
if (!groups_get_all_groups($course->id, $USER->id)) {
print_error('cannotsubscribe', 'forum');
}
@ -142,7 +142,7 @@ $info = new stdClass();
$info->name = fullname($user);
$info->forum = format_string($forum->name);
if (\mod_forum\subscriptions::is_subscribed($user->id, $forum, $discussionid)) {
if (\mod_forum\subscriptions::is_subscribed($user->id, $forum, $discussionid, $cm)) {
if (is_null($sesskey)) { // we came here via link in email
$PAGE->set_title($course->shortname);
$PAGE->set_heading($course->fullname);

2
mod/forum/subscribe_ajax.php
View File

@ -46,7 +46,7 @@ if (!\mod_forum\subscriptions::is_subscribable($forum)) {
die;
}
if (\mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id)) {
if (\mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id, $cm)) {
// The user is subscribed, unsubscribe them.
\mod_forum\subscriptions::unsubscribe_user_from_discussion($USER->id, $discussion, $context);
} else {

49
mod/forum/tests/subscriptions_test.php
View File

@ -1433,4 +1433,53 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
)));
}
/**
* Test that providing a context_module instance to is_subscribed does not result in additional lookups to retrieve
* the context_module.
*/
public function test_is_subscribed_cm() {
global $DB;
$this->resetAfterTest(true);
// Create a course, with a forum.
$course = $this->getDataGenerator()->create_course();
$options = array('course' => $course->id, 'forcesubscribe' => FORUM_FORCESUBSCRIBE);
$forum = $this->getDataGenerator()->create_module('forum', $options);
// Create a user enrolled in the course as a student.
list($user) = $this->helper_create_users($course, 1);
// Retrieve the $cm now.
$cm = get_fast_modinfo($forum->course)->instances['forum'][$forum->id];
// Reset get_fast_modinfo.
get_fast_modinfo(0, 0, true);
// Call is_subscribed without passing the $cmid - this should result in a lookup and filling of some of the
// caches. This provides us with consistent data to start from.
$this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum));
$this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum));
// Make a note of the number of DB calls.
$basecount = $DB->perf_get_reads();
// Call is_subscribed - it should give return the correct result (False), and result in no additional queries.
$this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum, null, $cm));
// The capability check does require some queries, so we don't test it directly.
// We don't assert here because this is dependant upon linked code which could change at any time.
$suppliedcmcount = $DB->perf_get_reads() - $basecount;
// Call is_subscribed without passing the $cmid now - this should result in a lookup.
get_fast_modinfo(0, 0, true);
$basecount = $DB->perf_get_reads();
$this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum));
$calculatedcmcount = $DB->perf_get_reads() - $basecount;
// There should be more queries than when we performed the same check a moment ago.
$this->assertGreaterThan($suppliedcmcount, $calculatedcmcount);
}
}