MDL-80501 mod_h5pactivity: activity group mode

Respect activity's group mode when checking access to the report. Previously only the course's group mode was checked.
moodle · Feb 8, 2024 · fa72ebc · fa72ebc
1 parent 6d7f162
commit fa72ebc
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 1 deletion.
diff --git a/mod/h5pactivity/classes/local/manager.php b/mod/h5pactivity/classes/local/manager.php
@@ -479,7 +479,7 @@ public function get_report(int $userid = null, int $attemptid = null, $currentgr
             // Ensure user can view the attempt of specific userid, respecting access checks.
             if ($user && $user->id != $USER->id) {
                 $course = get_course($this->coursemodule->course);
-                if ($this->coursemodule->effectivegroupmode == SEPARATEGROUPS && !user_can_view_profile($user, $course)) {
+                if (!groups_user_groups_visible($course, $user->id, $this->coursemodule)) {
                     return null;
                 }
             }

diff --git a/mod/h5pactivity/tests/local/manager_test.php b/mod/h5pactivity/tests/local/manager_test.php
@@ -950,6 +950,94 @@ public function get_report_data(): array {
         ];
     }
 
+    /**
+     * Test teacher access to student reports (get_report) when course groupmode is SEPARATEGROUPS.
+     * @covers ::get_report()
+     * @dataProvider get_report_data_groupmode
+     *
+     * @param bool $activitygroupmode Course or activity groupmode
+     */
+    public function test_get_report_groupmode(bool $activitygroupmode): void {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        if ($activitygroupmode) {
+            $course = $this->getDataGenerator()->create_course(['groupmode' => NOGROUPS, 'groupmodeforce' => 0]);
+            $activitysettings = ['course' => $course, 'groupmode' => SEPARATEGROUPS];
+        } else {
+            $course = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1]);
+            $activitysettings = ['course' => $course];
+        }
+
+        $activity = $this->getDataGenerator()->create_module('h5pactivity', $activitysettings);
+
+        // Grant mod/h5pactivity:reviewattempts to non-editing teacher.
+        // At the time of writing this is not set by default (see MDL-80028).
+        $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']);
+        role_change_permission($teacherrole->id,
+            \context_course::instance($course->id), 'mod/h5pactivity:reviewattempts', CAP_ALLOW);
+
+        $manager = manager::create_from_instance($activity);
+
+        $editingteacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher');
+        $teacher1 = $this->getDataGenerator()->create_and_enrol($course, 'teacher');
+        $teacher2 = $this->getDataGenerator()->create_and_enrol($course, 'teacher');
+        $student1 = $this->getDataGenerator()->create_and_enrol($course);
+        $student2 = $this->getDataGenerator()->create_and_enrol($course);
+        $student3 = $this->getDataGenerator()->create_and_enrol($course);
+
+        $group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]);
+        $this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $teacher1->id]);
+        $this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $student1->id]);
+
+        $group2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]);
+        $this->getDataGenerator()->create_group_member(['groupid' => $group2->id, 'userid' => $student2->id]);
+
+        // Check reports.
+
+        // Editing teachers can view all users, those in any group or no group.
+        $this->setUser($editingteacher);
+        $report = $manager->get_report($student1->id);
+        $this->assertNotNull($report);
+        $report = $manager->get_report($student3->id);
+        $this->assertNotNull($report);
+
+        // Non-editing teacher can view student, both members of same group.
+        $this->setUser($teacher1);
+        $report = $manager->get_report($student1->id);
+        $this->assertNotNull($report);
+
+        // Non-editing teacher cannot view student in no group.
+        $report = $manager->get_report($student3->id);
+        $this->assertNull($report);
+
+        // Non-editing teacher cannot view student in different group.
+        $report = $manager->get_report($student2->id);
+        $this->assertNull($report);
+
+        // Non-editing teacher in no group can view no one.
+        $this->setUser($teacher2);
+        $report = $manager->get_report($student1->id);
+        $this->assertNull($report);
+        $report = $manager->get_report($student3->id);
+        $this->assertNull($report);
+    }
+
+    /**
+     * Data provider for test_get_report_groupmode.
+     *
+     * @return array
+     */
+    public function get_report_data_groupmode(): array {
+        return [
+            // No tracking scenarios.
+            'course groupmode is SEPARATEGROUPS' => [false],
+            'course groupmode is NOGROUPS, activity groupmode is SEPARATEGROUPS' => [true],
+        ];
+    }
+
     /**
      * Test get_attempt method.
      *