moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-05 08:56:36 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

forum subscriptions: MDL-14876 user of deprecated functions was breaking managing subscribers.

Browse source 
This was visible when, for example, unusual role definitions meant that someone could access a forum without being enrolled in a course.

Note that one of the places that was previously broken was front page forums. Since subscribers.php does not do paging at all, the fact that I have fixed this bug makes this page dangerous on large sites. A proper solution will have to wait until bug 17550 is fixed in HEAD.

This also fixes a minor problem introduced by MDL-12979.
This commit is contained in:
tjhunt 2008-12-11 07:01:57 +00:00
parent 3c573960ec
commit 8d8d0bfaae
3 changed files with 66 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

41
mod/forum/lib.php
View file

@ -83,9 +83,13 @@ function forum_add_instance($forum) {
}
if ($forum->forcesubscribe == FORUM_INITIALSUBSCRIBE) {
// all users should be subscribed initially
$users = get_users_by_capability(get_context_instance(CONTEXT_COURSE, $forum->course),
'mod/forum:initialsubscriptions', 'u.id', '', '','','',null, false);
/// all users should be subscribed initially
/// Note: forum_get_potential_subscribers should take the forum context,
/// but that does not exist yet, becuase the forum is only half build at this
/// stage. However, because the forum is brand new, we know that there are
/// no role assignments or overrides in the forum context, so using the
/// course context gives the same list of users.
$users = forum_get_potential_subscribers(get_context_instance(CONTEXT_COURSE, $forum->course), 0, 'id, email', '');
foreach ($users as $user) {
forum_subscribe($user->id, $forum->id);
}
@ -386,7 +390,8 @@ function forum_cron() {
// caching subscribed users of each forum
if (!isset($subscribedusers[$forumid])) {
if ($subusers = forum_subscribed_users($courses[$courseid], $forums[$forumid], 0, false)) {
if ($subusers = forum_subscribed_users($courses[$courseid], $forums[$forumid], 0,
get_context_instance(CONTEXT_MODULE, $coursemodules[$forumid]))) {
foreach ($subusers as $postuser) {
// do not try to mail users with stopped email
if ($postuser->emailstop) {
@ -2661,10 +2666,28 @@ function forum_get_user_discussions($courseid, $userid, $groupid=0) {
}
/**
* Returns list of user objects that are subscribed to this forum
* Get the list of potential subscribers to a forum.
*
* @param object $forumcontext the forum context.
* @param integer $groupid the id of a group, or 0 for all groups.
* @param string $fields the list of fields to return for each user. As for get_users_by_capability.
* @param string $sort sort order. As for get_users_by_capability.
* @return array list of users.
*/
function forum_subscribed_users($course, $forum, $groupid=0) {
function forum_get_potential_subscribers($forumcontext, $groupid, $fields, $sort) {
return get_users_by_capability($forumcontext, 'mod/forum:initialsubscriptions', $fields, $sort, '', '', $groupid, '', false, true);
}
/**
* Returns list of user objects that are subscribed to this forum
*
* @param object $course the course
* @param forum $forum the forum
* @param integer $groupid group id, or 0 for all.
* @param object $context the forum context, to save re-fetching it where possible.
* @return array list of users.
*/
function forum_subscribed_users($course, $forum, $groupid=0, $context = NULL) {
global $CFG, $DB;
$params = array($forum->id);
@ -2699,9 +2722,11 @@ function forum_subscribed_users($course, $forum, $groupid=0) {
u.mnethostid";
if (forum_is_forcesubscribed($forum)) {
$context = get_context_instance(CONTEXT_COURSE, $course->id);
if (is_null($context)) {
$context = get_context_instance(CONTEXT_MODULE, get_coursemodule_from_instance('forum', $forum->id, $course->id));
}
$sort = "u.email ASC";
$results = get_users_by_capability($context, 'mod/forum:initialsubscriptions', $fields, $sort, '','','','', false, true);
$results = forum_get_potential_subscribers($context, $groupid, $fields, $sort);
} else {
$results = $DB->get_records_sql("SELECT $fields
FROM {user} u,

20
mod/forum/subscriber.html
View file

@ -1,6 +1,5 @@
<form id="subscriberform" method="post" action="subscribers.php">
<input type="hidden" name="previoussearch" value="<?php p($previoussearch) ?>" />
<input type="hidden" name="id" value="<?php echo $id?>" />
<table align="center" border="0" cellpadding="5" cellspacing="0">
<tr>
@ -14,7 +13,7 @@
</tr>
<tr>
<td valign="top">
<select name="removeselect[]" size="20" id="removeselect" multiple
<select name="removeselect[]" size="20" id="removeselect" multiple="multiple"
onFocus="getElementById('subscriberform').add.disabled=true;
getElementById('subscriberform').remove.disabled=false;
getElementById('subscriberform').addselect.selectedIndex=-1;">
@ -34,12 +33,12 @@
<br />
</td>
<td valign="top">
<select name="addselect[]" size="20" id="addselect" multiple
<select name="addselect[]" size="20" id="addselect" multiple="multiple"
onFocus="getElementById('subscriberform').add.disabled=false;
getElementById('subscriberform').remove.disabled=true;
getElementById('subscriberform').removeselect.selectedIndex=-1;">
<?php
if (!empty($searchusers)) {
if (isset($searchusers)) {
echo "<optgroup label=\"$strsearchresults (" . count($searchusers) . ")\">\n";
foreach ($searchusers as $user) {
$fullname = fullname($user, true);
@ -56,19 +55,10 @@
?>
</select>
<br />
<input type="text" name="searchtext" size="30" value="<?php p($searchtext) ?>"
onFocus ="getElementById('subscriberform').add.disabled=true;
getElementById('subscriberform').remove.disabled=true;
getElementById('subscriberform').removeselect.selectedIndex=-1;
getElementById('subscriberform').addselect.selectedIndex=-1;"
onkeydown = "var keyCode = event.which ? event.which : event.keyCode;
if (keyCode == 13) {
getElementById('subscriberform').previoussearch.value=1;
getElementById('subscriberform').submit();
} " />
<input type="text" name="searchtext" size="30" value="<?php p($searchtext, true) ?>" />
<input name="search" id="search" type="submit" value="<?php p($strsearch) ?>" />
<?php
if (!empty($searchusers)) {
if (isset($searchusers)) {
echo '<input name="showall" id="showall" type="submit" value="'.s($strshowall).'" />'."\n";
}
?>

53
mod/forum/subscribers.php
View file

@ -31,10 +31,10 @@
add_to_log($course->id, "forum", "view subscribers", "subscribers.php?id=$forum->id", $forum->id, $cm->id);
$strsubscribeall = get_string("subscribeall", "forum");
$strsubscribeall = get_string("subscribeall", "forum");
$strsubscribenone = get_string("subscribenone", "forum");
$strsubscribers = get_string("subscribers", "forum");
$strforums = get_string("forums", "forum");
$strsubscribers = get_string("subscribers", "forum");
$strforums = get_string("forums", "forum");
$navigation = build_navigation($strsubscribers, $cm);
@ -56,7 +56,7 @@
if (empty($USER->subscriptionsediting)) { /// Display an overview of subscribers
if (! $users = forum_subscribed_users($course, $forum, $currentgroup) ) {
if (! $users = forum_subscribed_users($course, $forum, $currentgroup, $context) ) {
print_heading(get_string("nosubscribers", "forum"));
@ -93,12 +93,11 @@
$strsubscribers = get_string("subscribers", "forum");
$strforums = get_string("forums", "forum");
$searchtext = optional_param('searchtext', '', PARAM_RAW);
if ($frm = data_submitted()) {
/// A form was submitted so process the input
if (!empty($frm->add) and !empty($frm->addselect)) {
foreach ($frm->addselect as $addsubscriber) {
if (! forum_subscribe($addsubscriber, $id)) {
print_error('cannotaddsubscriber', 'forum', '', $addsubscriber);
@ -111,36 +110,40 @@
}
}
} else if (!empty($frm->showall)) {
unset($frm->searchtext);
$frm->previoussearch = 0;
$searchtext = '';
}
}
$previoussearch = (!empty($frm->search) or (!empty($frm->previoussearch) && $frm->previoussearch == 1)) ;
/// Get all existing subscribers for this forum.
if (!$subscribers = forum_subscribed_users($course, $forum, $currentgroup)) {
if (!$subscribers = forum_subscribed_users($course, $forum, $currentgroup, $context)) {
$subscribers = array();
}
$subscriberarray = array();
/// Get all the potential subscribers excluding users already subscribed
$users = forum_get_potential_subscribers($context, $currentgroup, 'id,email,firstname,lastname', 'firstname ASC, lastname ASC');
if (!$users) {
$users = array();
}
foreach ($subscribers as $subscriber) {
$subscriberarray[] = $subscriber->id;
}
$subscriberlist = implode(',', $subscriberarray);
/// Get search results excluding any users already subscribed
if (!empty($frm->searchtext) and $previoussearch) {
$searchusers = search_users($course->id, $currentgroup, $frm->searchtext, 'firstname ASC, lastname ASC', $subscriberarray);
unset($users[$subscriber->id]);
}
/// If no search results then get potential subscribers for this forum excluding users already subscribed
if (empty($searchusers)) {
$users = get_users_by_capability($context, 'moodle/course:view', '', 'firstname ASC, lastname ASC', '','',$currentgroup,$subscriberlist, false);
/// This is yucky, but do the search in PHP, becuase the list we are using comes from get_users_by_capability,
/// which does not allow searching in the database. Fortunately the list is only this list of users in this
/// course, which is normally OK, except on the site course of a big site. But before you can enter a search
/// term, you have already seen a page that lists everyone, since this code never does paging, so you have probably
/// already crashed your server if you are going to. This will be fixed properly for Moodle 2.0: MDL-17550.
if ($searchtext) {
$searchusers = array();
$lcsearchtext = moodle_strtolower($searchtext);
foreach ($users as $userid => $user) {
if (strpos(moodle_strtolower($user->email), $lcsearchtext) !== false ||
strpos(moodle_strtolower($user->firstname . ' ' . $user->lastname), $lcsearchtext) !== false) {
$searchusers[$userid] = $user;
}
unset($users[$userid]);
}
}
$searchtext = (isset($frm->searchtext)) ? $frm->searchtext : "";
$previoussearch = ($previoussearch) ? '1' : '0';
print_simple_box_start('center');