Skip to content

Commit

Permalink
MDL-49921 forum: Handle exceptions correctly in get_forums_by_courses
Browse files Browse the repository at this point in the history
  • Loading branch information
@jleyva
jleyva committed Apr 22, 2015
1 parent b0682b9 commit 852d965
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 52 deletions.
67 changes: 38 additions & 29 deletions mod/forum/externallib.php
View file
Expand Up @@ -54,7 +54,7 @@ public static function get_forums_by_courses_parameters() {
* @since Moodle 2.5
*/
public static function get_forums_by_courses($courseids = array()) {
global $CFG, $DB, $USER;
global $CFG;

require_once($CFG->dirroot . "/mod/forum/lib.php");

Expand All @@ -72,38 +72,47 @@ public static function get_forums_by_courses($courseids = array()) {

// Ensure there are courseids to loop through.
if (!empty($courseids)) {
// Array of the courses we are going to retrieve the forums from.
$dbcourses = array();
// Mod info for courses.
$modinfocourses = array();

// Go through the courseids and return the forums.
foreach ($courseids as $cid) {
// Get the course context.
$context = context_course::instance($cid);
foreach ($courseids as $courseid) {
// Check the user can function in this context.
self::validate_context($context);
// Get the forums in this course.
if ($forums = $DB->get_records('forum', array('course' => $cid))) {
try {
$context = context_course::instance($courseid);
self::validate_context($context);
// Get the modinfo for the course.
$modinfo = get_fast_modinfo($cid);
// Get the forum instances.
$foruminstances = $modinfo->get_instances_of('forum');
// Loop through the forums returned by modinfo.
foreach ($foruminstances as $forumid => $cm) {
// If it is not visible or present in the forums get_records call, continue.
if (!$cm->uservisible || !isset($forums[$forumid])) {
continue;
}
// Set the forum object.
$forum = $forums[$forumid];
// Get the module context.
$context = context_module::instance($cm->id);
// Check they have the view forum capability.
require_capability('mod/forum:viewdiscussion', $context);
// Format the intro before being returning using the format setting.
list($forum->intro, $forum->introformat) = external_format_text($forum->intro, $forum->introformat,
$context->id, 'mod_forum', 'intro', 0);
// Add the course module id to the object, this information is useful.
$forum->cmid = $cm->id;
// Add the forum to the array to return.
$arrforums[$forum->id] = (array) $forum;
$modinfocourses[$courseid] = get_fast_modinfo($courseid);
$dbcourses[$courseid] = $modinfocourses[$courseid]->get_course();

} catch (Exception $e) {
continue;
}
}

// Get the forums in this course. This function checks users visibility permissions.
if ($forums = get_all_instances_in_courses("forum", $dbcourses)) {
foreach ($forums as $forum) {

$course = $dbcourses[$forum->course];
$cm = $modinfocourses[$course->id]->get_cm($forum->coursemodule);
$context = context_module::instance($cm->id);

// Skip forums we are not allowed to see discussions.
if (!has_capability('mod/forum:viewdiscussion', $context)) {
continue;
}

// Format the intro before being returning using the format setting.
list($forum->intro, $forum->introformat) = external_format_text($forum->intro, $forum->introformat,
$context->id, 'mod_forum', 'intro', 0);

$forum->cmid = $forum->coursemodule;

// Add the forum to the array to return.
$arrforums[$forum->id] = $forum;
}
}
}
Expand Down
44 changes: 21 additions & 23 deletions mod/forum/tests/externallib_test.php
View file
Expand Up @@ -96,45 +96,43 @@ public function test_mod_forum_get_forums_by_courses() {
$roleid2 = $this->assignUserCapability('mod/forum:viewdiscussion', $context2->id, $newrole);

// Create what we expect to be returned when querying the two courses.
unset($forum1->displaywordcount);
unset($forum2->displaywordcount);

$expectedforums = array();
$expectedforums[$forum1->id] = (array) $forum1;
$expectedforums[$forum2->id] = (array) $forum2;

// Call the external function passing course ids.
$forums = mod_forum_external::get_forums_by_courses(array($course1->id, $course2->id));
external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertEquals($expectedforums, $forums);
$forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertCount(2, $forums);
foreach ($forums as $forum) {
$this->assertEquals($expectedforums[$forum['id']], $forum);
}

// Call the external function without passing course id.
$forums = mod_forum_external::get_forums_by_courses();
external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertEquals($expectedforums, $forums);
$forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertCount(2, $forums);
foreach ($forums as $forum) {
$this->assertEquals($expectedforums[$forum['id']], $forum);
}

// Unenrol user from second course and alter expected forums.
$enrol->unenrol_user($instance2, $user->id);
unset($expectedforums[$forum2->id]);

// Call the external function without passing course id.
$forums = mod_forum_external::get_forums_by_courses();
external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertEquals($expectedforums, $forums);

// Call for the second course we unenrolled the user from, ensure exception thrown.
try {
mod_forum_external::get_forums_by_courses(array($course2->id));
$this->fail('Exception expected due to being unenrolled from the course.');
} catch (moodle_exception $e) {
$this->assertEquals('requireloginerror', $e->errorcode);
}

// Call without required capability, ensure exception thrown.
$this->unassignUserCapability('mod/forum:viewdiscussion', null, null, $course1->id);
try {
$forums = mod_forum_external::get_forums_by_courses(array($course1->id));
$this->fail('Exception expected due to missing capability.');
} catch (moodle_exception $e) {
$this->assertEquals('nopermissions', $e->errorcode);
}
$forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertCount(1, $forums);
$this->assertEquals($expectedforums[$forum1->id], $forums[0]);

// Call for the second course we unenrolled the user from.
$forums = mod_forum_external::get_forums_by_courses(array($course2->id));
$forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertCount(0, $forums);
}

/**
Expand Down

0 comments on commit 852d965

Please sign in to comment.