diff --git a/mod/quiz/classes/local/override_manager.php b/mod/quiz/classes/local/override_manager.php index a79c739b1a5..4bd5081a53f 100644 --- a/mod/quiz/classes/local/override_manager.php +++ b/mod/quiz/classes/local/override_manager.php @@ -447,6 +447,22 @@ class override_manager { } } + /** + * Determine whether user can view a given override record + * + * @param \stdClass $override + * @param \stdClass $course + * @param \cm_info $cm + * @return bool + */ + public function can_view_override(\stdClass $override, \stdClass $course, \cm_info $cm): bool { + if ($override->groupid) { + return groups_group_visible($override->groupid, $course, $cm); + } else { + return groups_user_groups_visible($course, $override->userid, $cm); + } + } + /** * Builds common event data * diff --git a/mod/quiz/overridedelete.php b/mod/quiz/overridedelete.php index aa42d2c7a89..985d3dba388 100644 --- a/mod/quiz/overridedelete.php +++ b/mod/quiz/overridedelete.php @@ -44,15 +44,8 @@ require_login($course, false, $cm); // Check the user has the required capabilities to modify an override. $manager->require_manage_capability(); - -if ($override->groupid) { - if (!groups_group_visible($override->groupid, $course, $cm)) { - throw new \moodle_exception('invalidoverrideid', 'quiz'); - } -} else { - if (!groups_user_groups_visible($course, $override->userid, $cm)) { - throw new \moodle_exception('invalidoverrideid', 'quiz'); - } +if (!$manager->can_view_override($override, $course, $cm)) { + throw new \moodle_exception('invalidoverrideid', 'quiz'); } $url = new moodle_url('/mod/quiz/overridedelete.php', ['id' => $override->id]); diff --git a/mod/quiz/overrideedit.php b/mod/quiz/overrideedit.php index 878e2637705..492fb290189 100644 --- a/mod/quiz/overrideedit.php +++ b/mod/quiz/overrideedit.php @@ -72,14 +72,8 @@ if ($overrideid) { // Editing an override. $data = clone $override; - if ($override->groupid) { - if (!groups_group_visible($override->groupid, $course, $cm)) { - throw new \moodle_exception('invalidoverrideid', 'quiz'); - } - } else { - if (!groups_user_groups_visible($course, $override->userid, $cm)) { - throw new \moodle_exception('invalidoverrideid', 'quiz'); - } + if (!$manager->can_view_override($override, $course, $cm)) { + throw new \moodle_exception('invalidoverrideid', 'quiz'); } } else { // Creating a new override. diff --git a/mod/quiz/tests/local/override_manager_test.php b/mod/quiz/tests/local/override_manager_test.php index 2690513a2b0..83b8fcfe187 100644 --- a/mod/quiz/tests/local/override_manager_test.php +++ b/mod/quiz/tests/local/override_manager_test.php @@ -46,7 +46,7 @@ final class override_manager_test extends \advanced_testcase { * @return array containing quiz object and course */ private function create_quiz_and_course(): array { - $course = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS]); + $course = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1]); $quizparams = array_merge(self::TEST_QUIZ_SETTINGS, ['course' => $course->id]); $quiz = $this->getDataGenerator()->create_module('quiz', $quizparams); $quizobj = quiz_settings::create($quiz->id); @@ -123,6 +123,89 @@ final class override_manager_test extends \advanced_testcase { ]; } + /** + * Data provider for {@see test_can_view_override} + * + * @return array[] + */ + public static function can_view_override_provider(): array { + return [ + ['admin', true, true, true, true], + ['teacher', true, false, true, false], + ]; + } + + /** + * Test whether user can view given override + * + * @param string $currentuser + * @param bool $grouponeview + * @param bool $grouptwoview + * @param bool $studentoneview + * @param bool $studenttwoview + * + * @dataProvider can_view_override_provider + */ + public function test_can_view_override( + string $currentuser, + bool $grouponeview, + bool $grouptwoview, + bool $studentoneview, + bool $studenttwoview, + ): void { + global $DB; + + $this->resetAfterTest(); + + [$quizobj, $course] = $this->create_quiz_and_course(); + + // Teacher cannot view all groups. + $roleid = $DB->get_field('role', 'id', ['shortname' => 'editingteacher']); + assign_capability('moodle/site:accessallgroups', CAP_PROHIBIT, $roleid, $quizobj->get_context()->id); + + // Group one will contain our teacher and another student. + $groupone = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher', ['username' => 'teacher']); + $this->getDataGenerator()->create_group_member(['groupid' => $groupone->id, 'userid' => $teacher->id]); + $studentone = $this->getDataGenerator()->create_and_enrol($course); + $this->getDataGenerator()->create_group_member(['groupid' => $groupone->id, 'userid' => $studentone->id]); + + // Group two will contain a solitary student. + $grouptwo = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $studenttwo = $this->getDataGenerator()->create_and_enrol($course); + $this->getDataGenerator()->create_group_member(['groupid' => $grouptwo->id, 'userid' => $studenttwo->id]); + + $user = \core_user::get_user_by_username($currentuser); + $this->setUser($user); + + /** @var override_manager $manager */ + $manager = $quizobj->get_override_manager(); + + $this->assertEquals($grouponeview, $manager->can_view_override( + (object) ['groupid' => $groupone->id, 'userid' => null], + $course, + $quizobj->get_cm(), + )); + + $this->assertEquals($grouptwoview, $manager->can_view_override( + (object) ['groupid' => $grouptwo->id, 'userid' => null], + $course, + $quizobj->get_cm(), + )); + + $this->assertEquals($studentoneview, $manager->can_view_override( + (object) ['userid' => $studentone->id, 'groupid' => null], + $course, + $quizobj->get_cm(), + )); + + $this->assertEquals($studenttwoview, $manager->can_view_override( + (object) ['userid' => $studenttwo->id, 'groupid' => null], + $course, + $quizobj->get_cm(), + )); + } + /** * Provides values to test_save_and_get_override *