Merge branch 'MDL-61197-master' of git://github.com/jleyva/moodle
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 30 Oct 2018 23:22:30 +0000 (00:22 +0100)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 30 Oct 2018 23:22:30 +0000 (00:22 +0100)
course/upgrade.txt
group/externallib.php
group/tests/externallib_test.php

index c46cfaf..0013e74 100644 (file)
@@ -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 ===
 
index 4c04acd..9ed2492 100644 (file)
@@ -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);
+        // 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.
-        $context = context_course::instance($course->id);
-        self::validate_context($context);
+        list($courses, $warnings) = external_util::validate_courses(array_keys($courses), $courses, true);
 
-         // 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');
+        $usergroups = array();
+        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;
             }
-            // Validate if the user is enrolled in the course.
-            if (!is_enrolled($context, $user->id)) {
+
+            // 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.
-                $warning['item'] = 'course';
-                $warning['itemid'] = $course->id;
-                $warning['warningcode'] = '1';
-                $warning['message'] = "User $user->id is not enrolled in course $course->id";
-                $warnings[] = $warning;
+                $warnings[] = array(
+                    'item' => 'course',
+                    'itemid' => $course->id,
+                    'warningcode' => 'notenrolled',
+                    'message' => "User $userid is not enrolled in course $course->id",
+                );
             }
-        }
 
-        $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');
+            $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;
             }
index fff4079..287118e 100644 (file)
@@ -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']);
     }
 
     /**