diff --git a/course/upgrade.txt b/course/upgrade.txt index c46cfafae91..0013e7412ca 100644 --- a/course/upgrade.txt +++ b/course/upgrade.txt @@ -11,6 +11,7 @@ information provided here is intended especially for developers. - customdata (module custom data (JSON encoded)) - completion (to indicate if completion is enabled or not) - completiondata (completion status for the current user in the module) + * External function core_group_external::get_course_user_groups now can return all user courses group information. === 3.5 === diff --git a/group/externallib.php b/group/externallib.php index 4c04acd41fc..9ed2492e122 100644 --- a/group/externallib.php +++ b/group/externallib.php @@ -1180,8 +1180,9 @@ class core_group_external extends external_api { public static function get_course_user_groups_parameters() { return new external_function_parameters( array( - 'courseid' => new external_value(PARAM_INT, 'id of course'), - 'userid' => new external_value(PARAM_INT, 'id of user'), + 'courseid' => new external_value(PARAM_INT, + 'Id of course (empty or 0 for all the courses where the user is enrolled).', VALUE_DEFAULT, 0), + 'userid' => new external_value(PARAM_INT, 'Id of user (empty or 0 for current user).', VALUE_DEFAULT, 0), 'groupingid' => new external_value(PARAM_INT, 'returns only groups in the specified grouping', VALUE_DEFAULT, 0) ) ); @@ -1197,7 +1198,7 @@ class core_group_external extends external_api { * @return array of group objects (id, name, description, format) and possible warnings. * @since Moodle 2.9 */ - public static function get_course_user_groups($courseid, $userid, $groupingid = 0) { + public static function get_course_user_groups($courseid = 0, $userid = 0, $groupingid = 0) { global $USER; // Warnings array, it can be empty at the end but is mandatory. @@ -1209,43 +1210,62 @@ class core_group_external extends external_api { 'groupingid' => $groupingid ); $params = self::validate_parameters(self::get_course_user_groups_parameters(), $params); + $courseid = $params['courseid']; $userid = $params['userid']; $groupingid = $params['groupingid']; - // Validate course and user. get_course throws an exception if the course does not exists. - $course = get_course($courseid); - $user = core_user::get_user($userid, '*', MUST_EXIST); - core_user::require_active_user($user); - - // Security checks. - $context = context_course::instance($course->id); - self::validate_context($context); - - // Check if we have permissions for retrieve the information. - if ($user->id != $USER->id) { - if (!has_capability('moodle/course:managegroups', $context)) { - throw new moodle_exception('accessdenied', 'admin'); - } - // Validate if the user is enrolled in the course. - if (!is_enrolled($context, $user->id)) { - // We return a warning because the function does not fail for not enrolled users. - $warning['item'] = 'course'; - $warning['itemid'] = $course->id; - $warning['warningcode'] = '1'; - $warning['message'] = "User $user->id is not enrolled in course $course->id"; - $warnings[] = $warning; - } + // Validate user. + if (empty($userid)) { + $userid = $USER->id; + } else { + $user = core_user::get_user($userid, '*', MUST_EXIST); + core_user::require_active_user($user); } + // Get courses. + if (empty($courseid)) { + $courses = enrol_get_users_courses($userid, true); + $checkenrolments = false; // No need to check enrolments here since they are my courses. + } else { + $courses = array($courseid => get_course($courseid)); + $checkenrolments = true; + } + + // Security checks. + list($courses, $warnings) = external_util::validate_courses(array_keys($courses), $courses, true); + $usergroups = array(); - if (empty($warnings)) { - $groups = groups_get_all_groups($course->id, $user->id, 0, 'g.id, g.name, g.description, g.descriptionformat, g.idnumber'); + foreach ($courses as $course) { + // Check if we have permissions for retrieve the information. + if ($userid != $USER->id && !has_capability('moodle/course:managegroups', $course->context)) { + $warnings[] = array( + 'item' => 'course', + 'itemid' => $course->id, + 'warningcode' => 'cannotmanagegroups', + 'message' => "User $USER->id cannot manage groups in course $course->id", + ); + continue; + } + + // Check if the user being check is enrolled in the given course. + if ($checkenrolments && !is_enrolled($course->context, $userid)) { + // We return a warning because the function does not fail for not enrolled users. + $warnings[] = array( + 'item' => 'course', + 'itemid' => $course->id, + 'warningcode' => 'notenrolled', + 'message' => "User $userid is not enrolled in course $course->id", + ); + } + + $groups = groups_get_all_groups($course->id, $userid, $groupingid, + 'g.id, g.name, g.description, g.descriptionformat, g.idnumber'); foreach ($groups as $group) { list($group->description, $group->descriptionformat) = external_format_text($group->description, $group->descriptionformat, - $context->id, 'group', 'description', $group->id); + $course->context->id, 'group', 'description', $group->id); $group->courseid = $course->id; $usergroups[] = $group; } diff --git a/group/tests/externallib_test.php b/group/tests/externallib_test.php index fff4079f1e7..287118e7b96 100644 --- a/group/tests/externallib_test.php +++ b/group/tests/externallib_test.php @@ -378,10 +378,12 @@ class core_group_externallib_testcase extends externallib_advanced_testcase { $teacher = self::getDataGenerator()->create_user(); $course = self::getDataGenerator()->create_course(); + $anothercourse = self::getDataGenerator()->create_course(); $emptycourse = self::getDataGenerator()->create_course(); $studentrole = $DB->get_record('role', array('shortname' => 'student')); $this->getDataGenerator()->enrol_user($student1->id, $course->id, $studentrole->id); + $this->getDataGenerator()->enrol_user($student1->id, $anothercourse->id, $studentrole->id); $this->getDataGenerator()->enrol_user($student2->id, $course->id, $studentrole->id); $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher')); @@ -397,12 +399,30 @@ class core_group_externallib_testcase extends externallib_advanced_testcase { $group2data['courseid'] = $course->id; $group2data['name'] = 'Group Test 2'; $group2data['description'] = 'Group Test 2 description'; + $group3data = array(); + $group3data['courseid'] = $anothercourse->id; + $group3data['name'] = 'Group Test 3'; + $group3data['description'] = 'Group Test 3 description'; + $group3data['idnumber'] = 'TEST3'; $group1 = self::getDataGenerator()->create_group($group1data); $group2 = self::getDataGenerator()->create_group($group2data); + $group3 = self::getDataGenerator()->create_group($group3data); groups_add_member($group1->id, $student1->id); groups_add_member($group1->id, $student2->id); groups_add_member($group2->id, $student1->id); + groups_add_member($group3->id, $student1->id); + + // Create a grouping. + $groupingdata = array(); + $groupingdata['courseid'] = $course->id; + $groupingdata['name'] = 'Grouping Test'; + $groupingdata['description'] = 'Grouping Test description'; + $groupingdata['descriptionformat'] = FORMAT_MOODLE; + + $grouping = self::getDataGenerator()->create_grouping($groupingdata); + // Grouping only containing group1. + groups_assign_grouping($grouping->id, $group1->id); $this->setUser($student1); @@ -410,6 +430,22 @@ class core_group_externallib_testcase extends externallib_advanced_testcase { $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); // Check that I see my groups. $this->assertCount(2, $groups['groups']); + $this->assertEquals($course->id, $groups['groups'][0]['courseid']); + $this->assertEquals($course->id, $groups['groups'][1]['courseid']); + + // Check that I only see my groups inside the given grouping. + $groups = core_group_external::get_course_user_groups($course->id, $student1->id, $grouping->id); + $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); + // Check that I see my groups in the grouping. + $this->assertCount(1, $groups['groups']); + $this->assertEquals($group1->id, $groups['groups'][0]['id']); + + + // Check optional parameters (all student 1 courses and current user). + $groups = core_group_external::get_course_user_groups(); + $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); + // Check that I see my groups in all my courses. + $this->assertCount(3, $groups['groups']); $this->setUser($student2); $groups = core_group_external::get_course_user_groups($course->id, $student2->id); @@ -424,34 +460,49 @@ class core_group_externallib_testcase extends externallib_advanced_testcase { $this->setUser($teacher); $groups = core_group_external::get_course_user_groups($course->id, $student1->id); $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); - // Check that a teacher can see student groups. + // Check that a teacher can see student groups in given course. $this->assertCount(2, $groups['groups']); $groups = core_group_external::get_course_user_groups($course->id, $student2->id); $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); - // Check that a teacher can see student groups. + // Check that a teacher can see student groups in given course. $this->assertCount(1, $groups['groups']); + $groups = core_group_external::get_course_user_groups(0, $student1->id); + $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); + // Check that a teacher can see student groups in all the user courses if the teacher is enrolled in the course. + $this->assertCount(2, $groups['groups']); // Teacher only see groups in first course. + $this->assertCount(1, $groups['warnings']); // Enrolment warnings. + $this->assertEquals('1', $groups['warnings'][0]['warningcode']); + + // Enrol teacher in second course. + $this->getDataGenerator()->enrol_user($teacher->id, $anothercourse->id, $teacherrole->id); + $groups = core_group_external::get_course_user_groups(0, $student1->id); + $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); + // Check that a teacher can see student groups in all the user courses if the teacher is enrolled in the course. + $this->assertCount(3, $groups['groups']); + // Check permissions. $this->setUser($student1); - try { - $groups = core_group_external::get_course_user_groups($course->id, $student2->id); - } catch (moodle_exception $e) { - $this->assertEquals('accessdenied', $e->errorcode); - } - try { - $groups = core_group_external::get_course_user_groups($emptycourse->id, $student2->id); - } catch (moodle_exception $e) { - $this->assertEquals('requireloginerror', $e->errorcode); - } + // Student can's see other students group. + $groups = core_group_external::get_course_user_groups($course->id, $student2->id); + $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); + $this->assertCount(1, $groups['warnings']); + $this->assertEquals('cannotmanagegroups', $groups['warnings'][0]['warningcode']); + + // Not enrolled course. + $groups = core_group_external::get_course_user_groups($emptycourse->id, $student2->id); + $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); + $this->assertCount(1, $groups['warnings']); + $this->assertEquals('1', $groups['warnings'][0]['warningcode']); $this->setUser($teacher); - // Check warnings. + // Check user checking not enrolled in given course. $groups = core_group_external::get_course_user_groups($emptycourse->id, $student1->id); $groups = external_api::clean_returnvalue(core_group_external::get_course_user_groups_returns(), $groups); $this->assertCount(1, $groups['warnings']); - + $this->assertEquals('notenrolled', $groups['warnings'][0]['warningcode']); } /**