MDL-56287 gradereport_history: Show users from groups that can be viewed
authorJun Pataleta <jun@moodle.com>
Thu, 27 Feb 2020 08:48:32 +0000 (16:48 +0800)
committerSara Arjona <sara@moodle.com>
Wed, 4 Mar 2020 10:46:52 +0000 (11:46 +0100)
grade/report/history/classes/helper.php
grade/report/history/classes/output/tablelog.php
grade/report/history/index.php
grade/report/history/tests/report_test.php

index b0a3ff9..0a50fb8 100644 (file)
@@ -128,6 +128,7 @@ class helper {
      * @return array sql and params list
      */
     protected static function get_users_sql_and_params($context, $search = '', $count = false) {
+        global $DB, $USER;
 
         // Fields we need from the user table.
         $extrafields = get_extra_user_fields($context);
@@ -147,13 +148,38 @@ class helper {
             $select = "SELECT DISTINCT $ufields ";
             $orderby = " ORDER BY u.lastname ASC, u.firstname ASC";
         }
+
+        $groupjoinsql = '';
+        $groupwheresql = '';
+        $courseid = $context->instanceid;
+        $course = $DB->get_record('course', ['id' => $courseid]);
+        $groupmode = groups_get_course_groupmode($course);
+
+        // We're only interested in separate groups mode because it's the only group mode that requires the user to be a member of
+        // specific group(s), except when they have the 'moodle/site:accessallgroups' capability.
+        if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) {
+            // Fetch the groups that the user can see.
+            $groups = groups_get_all_groups($courseid, $USER->id, 0, 'g.id');
+            if (empty($groups)) {
+                // The user's not in any group and they don't have the capability to access all groups. So throw an error.
+                throw new \moodle_exception('notingroup');
+            }
+
+            // Add join condition to include users that only belong to the same group as the user.
+            list($insql, $inparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED, 'gid');
+            $groupjoinsql = " JOIN {groups_members} gm ON gm.userid = u.id ";
+            $groupwheresql = " AND gm.groupid $insql ";
+            $params = array_merge($params, $inparams);
+        }
+
         $sql = "$select
                  FROM {user} u
                  JOIN {grade_grades_history} ggh ON u.id = ggh.userid
                  JOIN {grade_items} gi ON gi.id = ggh.itemid
-                WHERE $filtersql gi.courseid = :courseid";
+                 $groupjoinsql
+                WHERE $filtersql gi.courseid = :courseid $groupwheresql";
         $sql .= $orderby;
-        $params['courseid'] = $context->instanceid;
+        $params['courseid'] = $courseid;
 
         return array($sql, $params);
     }
index 4d9c9fc..9ff4748 100644 (file)
@@ -63,6 +63,9 @@ class tablelog extends \table_sql implements \renderable {
      */
     protected $cms;
 
+    /** @var array Only fetch users belonging to these groups if defined. */
+    protected $groups;
+
     /**
      * @var int The default number of decimal points to use in this course
      * when a grade item does not itself define the number of decimal points.
@@ -99,6 +102,8 @@ class tablelog extends \table_sql implements \renderable {
         $this->courseid = $this->context->instanceid;
         $this->pagesize = $perpage;
         $this->page = $page;
+        $this->groups = $filters['groups'];
+        unset($filters['groups']);
         $this->filters = (object)$filters;
         $this->gradeitems = \grade_item::fetch_all(array('courseid' => $this->courseid));
         $this->cms = get_fast_modinfo($this->courseid);
@@ -379,6 +384,8 @@ class tablelog extends \table_sql implements \renderable {
      * @return array containing sql to use and an array of params.
      */
     protected function get_sql_and_params($count = false) {
+        global $DB;
+
         $fields = 'ggh.id, ggh.timemodified, ggh.itemid, ggh.userid, ggh.finalgrade, ggh.usermodified,
                    ggh.source, ggh.overridden, ggh.locked, ggh.excluded, ggh.feedback, ggh.feedbackformat,
                    gi.itemtype, gi.itemmodule, gi.iteminstance, gi.itemnumber, ';
@@ -424,12 +431,24 @@ class tablelog extends \table_sql implements \renderable {
 
         list($where, $params) = $this->get_filters_sql_and_params();
 
+        $groupjoinsql = '';
+        $groupwheresql = '';
+        // Generate group filters when necessary.
+        if ($this->groups) {
+            // Add join condition to include users that only belong to the same group as the user.
+            list($insql, $inparams) = $DB->get_in_or_equal(array_keys($this->groups), SQL_PARAMS_NAMED, 'gid');
+            $groupjoinsql = " JOIN {groups_members} gm ON gm.userid = u.id ";
+            $groupwheresql = " AND gm.groupid $insql ";
+            $params = array_merge($params, $inparams);
+        }
+
         $sql =  "SELECT $select
                    FROM {grade_grades_history} ggh
                    JOIN {grade_items} gi ON gi.id = ggh.itemid
                    JOIN {user} u ON u.id = ggh.userid
+                   $groupjoinsql
               LEFT JOIN {user} ug ON ug.id = ggh.usermodified
-                  WHERE $where";
+                  WHERE $where $groupwheresql";
 
         // As prevgrade is a dynamic field, we need to wrap the query. This is the only filtering
         // that should be defined outside the method self::get_filters_sql_and_params().
index 127e106..d58bb64 100644 (file)
@@ -43,6 +43,19 @@ $context = context_course::instance($course->id);
 require_capability('gradereport/history:view', $context);
 require_capability('moodle/grade:viewall', $context);
 
+$groupmode = groups_get_course_groupmode($course);
+$groups = [];
+// We're only interested in separate groups mode because it's the only group mode that requires the user to be a member of
+// specific group(s), except when they have the 'moodle/site:accessallgroups' capability.
+if ($groupmode == SEPARATEGROUPS && !has_capability('moodle/site:accessallgroups', $context)) {
+    // Fetch the groups that the user can see.
+    $groups = groups_get_all_groups($courseid, $USER->id, 0, 'g.id');
+    if (empty($groups)) {
+        // The user's not in any group and they don't have the capability to access all groups. So throw an error.
+        throw new \moodle_exception('notingroup');
+    }
+}
+
 // Last selected report session tracking.
 if (!isset($USER->grade_last_report)) {
     $USER->grade_last_report = array();
@@ -78,6 +91,7 @@ if ($data = $mform->get_data()) {
         'revisedonly' => optional_param('revisedonly', 0, PARAM_INT),
     );
 }
+$filters['groups'] = $groups;
 
 $table = new \gradereport_history\output\tablelog('gradereport_history', $context, $url, $filters, $download, $page);
 
index 7a3959d..4775e76 100644 (file)
@@ -219,6 +219,115 @@ class gradereport_history_report_testcase extends advanced_testcase {
         $this->assertEquals(1, \gradereport_history\helper::get_users_count($c1ctx, 'c'));
     }
 
+    /**
+     * Data provider method for \gradereport_history_report_testcase::test_get_users_with_groups()
+     */
+    public function get_users_provider() {
+        return [
+            'Visible groups, non-editing teacher, not in any group' => [
+                VISIBLEGROUPS, 'teacher', ['g1', 'g2'], false, ['s1', 's2', 's3', 's4']
+            ],
+            'Visible groups, non-editing teacher' => [
+                VISIBLEGROUPS, 'teacher', [], false, ['s1', 's2', 's3', 's4']
+            ],
+            'Visible groups, editing teacher' => [
+                VISIBLEGROUPS, 'editingteacher', ['g1', 'g2'], false, ['s1', 's2', 's3', 's4']
+            ],
+            'Separate groups, non-editing teacher' => [
+                SEPARATEGROUPS, 'teacher', ['g1', 'g2'], false, ['s1', 's2']
+            ],
+            'Separate groups, non-editing teacher, not in any group' => [
+                SEPARATEGROUPS, 'teacher', [], true, []
+            ],
+            'Separate groups, editing teacher' => [
+                SEPARATEGROUPS, 'editingteacher', ['g1', 'g2'], false, ['s1', 's2', 's3', 's4']
+            ],
+        ];
+    }
+
+    /**
+     * Test for helper::get_users() with course group mode set.
+     *
+     * @dataProvider get_users_provider
+     * @param $groupmode
+     * @param $teacherrole
+     * @param $teachergroups
+     * @param $exceptionexpected
+     * @param $expectedusers
+     */
+    public function test_get_users_with_groups($groupmode, $teacherrole, $teachergroups, $exceptionexpected, $expectedusers) {
+        global $DB;
+        $this->resetAfterTest();
+
+        $generator = $this->getDataGenerator();
+
+        // Create a test course.
+        $course = $generator->create_course(['groupmode' => $groupmode]);
+
+        // Create an assignment module.
+        $assign = $generator->create_module('assign', ['course' => $course]);
+
+        // Fetch roles.
+        $role = $DB->get_record('role', ['shortname' => $teacherrole], '*', MUST_EXIST);
+        $studentrole =  $DB->get_record('role', ['shortname' => 'student'], '*', MUST_EXIST);
+
+        // Create users.
+        $t1 = $generator->create_user(['username' => 't1', 'email' => 't1@example.com']);
+        $s1 = $generator->create_user(['username' => 's1', 'email' => 's1@example.com']);
+        $s2 = $generator->create_user(['username' => 's2', 'email' => 's2@example.com']);
+        $s3 = $generator->create_user(['username' => 's3', 'email' => 's3@example.com']);
+        $s4 = $generator->create_user(['username' => 's4', 'email' => 's4@example.com']);
+
+        // Enrol users.
+        $generator->enrol_user($t1->id, $course->id, $role->id);
+        $generator->enrol_user($s1->id, $course->id, $studentrole->id);
+        $generator->enrol_user($s2->id, $course->id, $studentrole->id);
+        $generator->enrol_user($s3->id, $course->id, $studentrole->id);
+        $generator->enrol_user($s4->id, $course->id, $studentrole->id);
+
+        // Create groups.
+        $groups = [];
+        $groups['g1'] = $generator->create_group(['courseid' => $course->id, 'name' => 'g1']);
+        $groups['g2'] = $generator->create_group(['courseid' => $course->id, 'name' => 'g2']);
+        $groups['g3'] = $generator->create_group(['courseid' => $course->id, 'name' => 'g3']);
+
+        // Add teacher to the assigned groups.
+        foreach ($teachergroups as $groupname) {
+            $group = $groups[$groupname];
+            $generator->create_group_member(['groupid' => $group->id, 'userid' => $t1->id]);
+        }
+
+        // Add students to groups.
+        $generator->create_group_member(['groupid' => $groups['g1']->id, 'userid' => $s1->id]);
+        $generator->create_group_member(['groupid' => $groups['g2']->id, 'userid' => $s2->id]);
+        $generator->create_group_member(['groupid' => $groups['g3']->id, 'userid' => $s3->id]);
+
+        // Creating grade history for the students.
+        $gi = grade_item::fetch(['iteminstance' => $assign->id, 'itemtype' => 'mod', 'itemmodule' => 'assign']);
+        $this->create_grade_history(['itemid' => $gi->id, 'userid' => $s1->id]);
+        $this->create_grade_history(['itemid' => $gi->id, 'userid' => $s2->id]);
+        $this->create_grade_history(['itemid' => $gi->id, 'userid' => $s3->id]);
+        $this->create_grade_history(['itemid' => $gi->id, 'userid' => $s4->id]);
+
+        // Log in as the teacher.
+        $this->setUser($t1);
+
+        // Declare when we expect to get an exception.
+        if ($exceptionexpected) {
+            $this->expectException(moodle_exception::class);
+            $this->expectExceptionMessage('notingroup');
+        }
+
+        // Fetch the users.
+        $users = \gradereport_history\helper::get_users(context_course::instance($course->id));
+        // Confirm that the number of users fetched is the same as the count of expected users.
+        $this->assertCount(count($expectedusers), $users);
+        foreach ($users as $user) {
+            // Confirm that each user returned is in the list of expected users.
+            $this->assertTrue(in_array($user->username, $expectedusers));
+        }
+    }
+
     /**
      * Test the get graders helper method.
      */