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

MDL-49921 forum: Handle exceptions correctly in get_forums_by_courses

Browse source
This commit is contained in:
Juan Leyva 2015-04-20 12:23:20 +02:00
parent d302ba231f
commit c8f1d8a026
2 changed files with 57 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

54
mod/forum/externallib.php
View file

@ -54,7 +54,7 @@ class mod_forum_external extends external_api {
* @since Moodle 2.5 * @since Moodle 2.5
*/ */
public static function get_forums_by_courses($courseids = array()) { public static function get_forums_by_courses($courseids = array()) {
global $CFG, $DB, $USER; global $CFG;
require_once($CFG->dirroot . "/mod/forum/lib.php"); require_once($CFG->dirroot . "/mod/forum/lib.php");
@ -72,44 +72,48 @@ class mod_forum_external extends external_api {
// Ensure there are courseids to loop through. // Ensure there are courseids to loop through.
if (!empty($courseids)) { 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. // Go through the courseids and return the forums.
foreach ($courseids as $cid) { foreach ($courseids as $courseid) {
// Get the course context.
$context = context_course::instance($cid);
// Check the user can function in this context. // Check the user can function in this context.
try {
$context = context_course::instance($courseid);
self::validate_context($context); self::validate_context($context);
// Get the forums in this course.
if ($forums = $DB->get_records('forum', array('course' => $cid))) {
// Get the modinfo for the course. // Get the modinfo for the course.
$modinfo = get_fast_modinfo($cid); $modinfocourses[$courseid] = get_fast_modinfo($courseid);
// Get the course object. $dbcourses[$courseid] = $modinfocourses[$courseid]->get_course();
$course = $modinfo->get_course();
// Get the forum instances. } catch (Exception $e) {
$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; continue;
} }
// Set the forum object. }
$forum = $forums[$forumid];
// Get the module context. // 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); $context = context_module::instance($cm->id);
// Check they have the view forum capability.
require_capability('mod/forum:viewdiscussion', $context); // 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. // Format the intro before being returning using the format setting.
list($forum->intro, $forum->introformat) = external_format_text($forum->intro, $forum->introformat, list($forum->intro, $forum->introformat) = external_format_text($forum->intro, $forum->introformat,
$context->id, 'mod_forum', 'intro', 0); $context->id, 'mod_forum', 'intro', 0);
// Add the course module id to the object, this information is useful.
$forum->cmid = $cm->id;
// Discussions count. This function does static request cache. // Discussions count. This function does static request cache.
$forum->numdiscussions = forum_count_discussions($forum, $cm, $course); $forum->numdiscussions = forum_count_discussions($forum, $cm, $course);
$forum->cmid = $forum->coursemodule;
// Add the forum to the array to return. // Add the forum to the array to return.
$arrforums[$forum->id] = (array) $forum; $arrforums[$forum->id] = $forum;
}
} }
} }
} }

42
mod/forum/tests/externallib_test.php
View file

@ -124,19 +124,28 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
$roleid2 = $this->assignUserCapability('mod/forum:viewdiscussion', $context2->id, $newrole); $roleid2 = $this->assignUserCapability('mod/forum:viewdiscussion', $context2->id, $newrole);
// Create what we expect to be returned when querying the two courses. // Create what we expect to be returned when querying the two courses.
unset($forum1->displaywordcount);
unset($forum2->displaywordcount);
$expectedforums = array(); $expectedforums = array();
$expectedforums[$forum1->id] = (array) $forum1; $expectedforums[$forum1->id] = (array) $forum1;
$expectedforums[$forum2->id] = (array) $forum2; $expectedforums[$forum2->id] = (array) $forum2;
// Call the external function passing course ids. // Call the external function passing course ids.
$forums = mod_forum_external::get_forums_by_courses(array($course1->id, $course2->id)); $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); $forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertEquals($expectedforums, $forums); $this->assertCount(2, $forums);
foreach ($forums as $forum) {
$this->assertEquals($expectedforums[$forum['id']], $forum);
}
// Call the external function without passing course id. // Call the external function without passing course id.
$forums = mod_forum_external::get_forums_by_courses(); $forums = mod_forum_external::get_forums_by_courses();
external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums); $forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertEquals($expectedforums, $forums); $this->assertCount(2, $forums);
foreach ($forums as $forum) {
$this->assertEquals($expectedforums[$forum['id']], $forum);
}
// Unenrol user from second course and alter expected forums. // Unenrol user from second course and alter expected forums.
$enrol->unenrol_user($instance2, $user->id); $enrol->unenrol_user($instance2, $user->id);
@ -144,25 +153,14 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
// Call the external function without passing course id. // Call the external function without passing course id.
$forums = mod_forum_external::get_forums_by_courses(); $forums = mod_forum_external::get_forums_by_courses();
external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums); $forums = external_api::clean_returnvalue(mod_forum_external::get_forums_by_courses_returns(), $forums);
$this->assertEquals($expectedforums, $forums); $this->assertCount(1, $forums);
$this->assertEquals($expectedforums[$forum1->id], $forums[0]);
// Call for the second course we unenrolled the user from, ensure exception thrown. // Call for the second course we unenrolled the user from.
try { $forums = mod_forum_external::get_forums_by_courses(array($course2->id));
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->fail('Exception expected due to being unenrolled from the course.'); $this->assertCount(0, $forums);
} 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);
}
} }
/** /**