From 1df5d7191c37e2f8faaa71ccb5851e3b8db67935 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 29 Jul 2015 16:27:46 +0200 Subject: [PATCH] MDL-50537 mod_chat: Use the utility class and several fixes --- mod/chat/classes/external.php | 67 ++++++++++++----------------- mod/chat/db/services.php | 8 ++++ mod/chat/tests/externallib_test.php | 25 +++++++++-- 3 files changed, 56 insertions(+), 44 deletions(-) diff --git a/mod/chat/classes/external.php b/mod/chat/classes/external.php index 311c2a0edfcf2..56d70c69cc499 100644 --- a/mod/chat/classes/external.php +++ b/mod/chat/classes/external.php @@ -493,6 +493,7 @@ public static function view_chat_returns() { ); } + /** * Describes the parameters for get_chats_by_courses. * @@ -503,12 +504,12 @@ public static function get_chats_by_courses_parameters() { return new external_function_parameters ( array( 'courseids' => new external_multiple_structure( - new external_value(PARAM_INT, 'course id'), - 'Array of course ids', VALUE_DEFAULT, array() + new external_value(PARAM_INT, 'course id'), 'Array of course ids', VALUE_DEFAULT, array() ), ) ); } + /** * Returns a list of chats in a provided list of courses, * if no list is provided all chats that the user can view will be returned. @@ -519,44 +520,24 @@ public static function get_chats_by_courses_parameters() { */ public static function get_chats_by_courses($courseids = array()) { global $CFG; - $params = self::validate_parameters(self::get_chats_by_courses_parameters(), array('courseids' => $courseids)); + + $returnedchats = array(); $warnings = array(); - if (!empty($params['courseids'])) { - $courses = array(); - $courseids = $params['courseids']; - } else { - $courses = enrol_get_my_courses(); - $courseids = array_keys($courses); + + $params = self::validate_parameters(self::get_chats_by_courses_parameters(), array('courseids' => $courseids)); + + if (empty($params['courseids'])) { + $params['courseids'] = array_keys(enrol_get_my_courses()); } - // Array to store the chats to return. - $arrchats = array(); + // Ensure there are courseids to loop through. - if (!empty($courseids)) { - // Array of the courses we are going to retrieve the chats from. - $arraycourses = array(); - // Go through the courseids. - foreach ($courseids as $cid) { - // Check the user can function in this context. - try { - $context = context_course::instance($cid); - self::validate_context($context); - // Check if this course was already loaded (by enrol_get_my_courses). - if (!isset($courses[$cid])) { - $courses[$cid] = get_course($cid); - } - $arraycourses[$cid] = $courses[$cid]; - } catch (Exception $e) { - $warnings[] = array( - 'item' => 'course', - 'itemid' => $cid, - 'warningcode' => '1', - 'message' => 'No access rights in course context '.$e->getMessage() - ); - } - } + if (!empty($params['courseids'])) { + + list($courses, $warnings) = external_util::validate_courses($params['courseids']); + // Get the chats in this course, this function checks users visibility permissions. // We can avoid then additional validate_context calls. - $chats = get_all_instances_in_courses("chat", $arraycourses); + $chats = get_all_instances_in_courses("chat", $courses); foreach ($chats as $chat) { $chatcontext = context_module::instance($chat->coursemodule); // Entry to return. @@ -568,27 +549,32 @@ public static function get_chats_by_courses($courseids = array()) { $chatdetails['name'] = $chat->name; // Format intro. list($chatdetails['intro'], $chatdetails['introformat']) = - external_format_text($chat->intro, $chat->introformat, - $chatcontext->id, 'mod_chat', 'intro', null); - if (has_capability('moodle/course:manageactivities', $chatcontext)) { + external_format_text($chat->intro, $chat->introformat, $chatcontext->id, 'mod_chat', 'intro', null); + + if (has_capability('mod/chat:chat', $chatcontext)) { + $chatdetails['chatmethod'] = $CFG->chat_method; $chatdetails['keepdays'] = $chat->keepdays; $chatdetails['studentlogs'] = $chat->studentlogs; $chatdetails['chattime'] = $chat->chattime; $chatdetails['schedule'] = $chat->schedule; + } + + if (has_capability('moodle/course:manageactivities', $chatcontext)) { $chatdetails['timemodified'] = $chat->timemodified; $chatdetails['section'] = $chat->section; $chatdetails['visible'] = $chat->visible; $chatdetails['groupmode'] = $chat->groupmode; $chatdetails['groupingid'] = $chat->groupingid; } - $arrchats[] = $chatdetails; + $returnedchats[] = $chatdetails; } } $result = array(); - $result['chats'] = $arrchats; + $result['chats'] = $returnedchats; $result['warnings'] = $warnings; return $result; } + /** * Describes the get_chats_by_courses return value. * @@ -607,6 +593,7 @@ public static function get_chats_by_courses_returns() { 'name' => new external_value(PARAM_TEXT, 'Chat name'), 'intro' => new external_value(PARAM_RAW, 'The Chat intro'), 'introformat' => new external_format_value('intro'), + 'chatmethod' => new external_value(PARAM_ALPHA, 'chat method (sockets, daemon)', VALUE_OPTIONAL), 'keepdays' => new external_value(PARAM_INT, 'keep days', VALUE_OPTIONAL), 'studentlogs' => new external_value(PARAM_INT, 'student logs visible to everyone', VALUE_OPTIONAL), 'chattime' => new external_value(PARAM_RAW, 'chat time', VALUE_OPTIONAL), diff --git a/mod/chat/db/services.php b/mod/chat/db/services.php index 750943fba92a8..2361aace2dee7 100644 --- a/mod/chat/db/services.php +++ b/mod/chat/db/services.php @@ -68,4 +68,12 @@ 'capabilities' => 'mod/chat:chat' ), + 'mod_chat_get_chats_by_courses' => array( + 'classname' => 'mod_chat_external', + 'methodname' => 'get_chats_by_courses', + 'description' => 'Returns a list of chat instances in a provided set of courses, + if no courses are provided then all the chat instances the user has access to will be returned.', + 'type' => 'read', + 'capabilities' => '' + ) ); diff --git a/mod/chat/tests/externallib_test.php b/mod/chat/tests/externallib_test.php index d8265b92427d1..cbd335f99f305 100644 --- a/mod/chat/tests/externallib_test.php +++ b/mod/chat/tests/externallib_test.php @@ -240,32 +240,49 @@ public function test_get_chats_by_courses() { $chat2 = self::getDataGenerator()->create_module('chat', $chatoptions2); $student1 = $this->getDataGenerator()->create_user(); $studentrole = $DB->get_record('role', array('shortname' => 'student')); + // Enroll Student1 in Course1. self::getDataGenerator()->enrol_user($student1->id, $course1->id, $studentrole->id); $this->setUser($student1); - $chats = mod_chat_external::get_chats_by_courses(array()); + + $chats = mod_chat_external::get_chats_by_courses(); // We need to execute the return values cleaning process to simulate the web service server. $chats = external_api::clean_returnvalue(mod_chat_external::get_chats_by_courses_returns(), $chats); $this->assertCount(1, $chats['chats']); $this->assertEquals('First Chat', $chats['chats'][0]['name']); - // As Student you cannot see some chat properties like 'showunanswered'. + // We see 11 fields. + $this->assertCount(11, $chats['chats'][0]); + + // As Student you cannot see some chat properties like 'section'. $this->assertFalse(isset($chats['chats'][0]['section'])); - // Student1 is not enrolled in this Course. - // The webservice will give a warning! + + // Student1 is not enrolled in course2. The webservice will return a warning! $chats = mod_chat_external::get_chats_by_courses(array($course2->id)); // We need to execute the return values cleaning process to simulate the web service server. $chats = external_api::clean_returnvalue(mod_chat_external::get_chats_by_courses_returns(), $chats); $this->assertCount(0, $chats['chats']); $this->assertEquals(1, $chats['warnings'][0]['warningcode']); + // Now as admin. $this->setAdminUser(); // As Admin we can see this chat. $chats = mod_chat_external::get_chats_by_courses(array($course2->id)); // We need to execute the return values cleaning process to simulate the web service server. $chats = external_api::clean_returnvalue(mod_chat_external::get_chats_by_courses_returns(), $chats); + $this->assertCount(1, $chats['chats']); $this->assertEquals('Second Chat', $chats['chats'][0]['name']); + // We see 16 fields. + $this->assertCount(16, $chats['chats'][0]); // As an Admin you can see some chat properties like 'section'. $this->assertEquals(0, $chats['chats'][0]['section']); + + // Enrol student in the second course. + self::getDataGenerator()->enrol_user($student1->id, $course2->id, $studentrole->id); + $this->setUser($student1); + $chats = mod_chat_external::get_chats_by_courses(); + $chats = external_api::clean_returnvalue(mod_chat_external::get_chats_by_courses_returns(), $chats); + $this->assertCount(2, $chats['chats']); + } }