From 390333268c7c8f75d91ab8d13e7446f4d55bf0ee Mon Sep 17 00:00:00 2001 From: Laurent David Date: Mon, 14 Apr 2025 15:36:34 +0200 Subject: [PATCH] MDL-84463 mod_feedback: Fix show entries and nonrespondents * Non editing teachers not in a groups should not see entries from other users --- mod/feedback/classes/manager.php | 18 +++ mod/feedback/lib.php | 12 +- mod/feedback/show_nonrespondents.php | 9 +- .../tests/behat/show_responses.feature | 72 +++++++++++ mod/feedback/tests/manager_test.php | 115 ++++++++++++++++++ 5 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 mod/feedback/tests/behat/show_responses.feature create mode 100644 mod/feedback/tests/manager_test.php diff --git a/mod/feedback/classes/manager.php b/mod/feedback/classes/manager.php index afc17c5c84f..07827031840 100644 --- a/mod/feedback/classes/manager.php +++ b/mod/feedback/classes/manager.php @@ -16,6 +16,7 @@ namespace mod_feedback; +use cm_info; use stdClass; /** @@ -38,4 +39,21 @@ class manager { return $DB->get_record('feedback_template', ['id' => $templateid], '*', MUST_EXIST); } + + /** + * Check if the current user can see other users if in groups + * + * @param cm_info $cm + * @return bool + */ + public static function can_see_others_in_groups(cm_info $cm): bool { + $canaccessallgroups = has_capability('moodle/site:accessallgroups', $cm->context); + if ($canaccessallgroups) { + return true; + } + $course = $cm->get_course(); + $groupmode = groups_get_activity_groupmode($cm, $course); + $usergroups = groups_get_user_groups($course->id); + return ($groupmode != SEPARATEGROUPS || !empty($usergroups['0'])); + } } diff --git a/mod/feedback/lib.php b/mod/feedback/lib.php index 3e097a1c3e3..ae282bc0742 100644 --- a/mod/feedback/lib.php +++ b/mod/feedback/lib.php @@ -23,6 +23,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use mod_feedback\manager; + defined('MOODLE_INTERNAL') || die(); // Include forms lib. @@ -2692,10 +2694,12 @@ function feedback_extend_settings_navigation(settings_navigation $settings, navi } if (has_capability('mod/feedback:viewreports', $context)) { - $feedbacknode->add_node($analysisnode); - $feedbacknode->add(get_string(($hassecondary ? 'responses' : 'show_entries'), 'feedback'), - new moodle_url('/mod/feedback/show_entries.php', ['id' => $settings->get_page()->cm->id]), - navigation_node::TYPE_CUSTOM, null, 'responses'); + if (manager::can_see_others_in_groups($settings->get_page()->cm)) { + $feedbacknode->add_node($analysisnode); + $feedbacknode->add(get_string(($hassecondary ? 'responses' : 'show_entries'), 'feedback'), + new moodle_url('/mod/feedback/show_entries.php', ['id' => $settings->get_page()->cm->id]), + navigation_node::TYPE_CUSTOM, null, 'responses'); + } } else { $feedbackcompletion = new mod_feedback_completion($feedback, $context, $settings->get_page()->course->id); if ($feedbackcompletion->can_view_analysis()) { diff --git a/mod/feedback/show_nonrespondents.php b/mod/feedback/show_nonrespondents.php index 5bc66868387..2e8b3176f49 100644 --- a/mod/feedback/show_nonrespondents.php +++ b/mod/feedback/show_nonrespondents.php @@ -22,6 +22,8 @@ * @package mod_feedback */ +use mod_feedback\manager; + require_once("../../config.php"); require_once("lib.php"); require_once($CFG->libdir.'/tablelib.php'); @@ -153,7 +155,12 @@ echo $OUTPUT->header(); /** @var \mod_feedback\output\renderer $renderer */ $renderer = $PAGE->get_renderer('mod_feedback'); echo $renderer->main_action_bar($actionbar); - +if (!manager::can_see_others_in_groups($cm)) { + // The user is not in a group so show message and exit. + echo $OUTPUT->notification(get_string('notingroup')); + echo $OUTPUT->footer(); + exit(); +} /// Print the main part of the page /////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////// diff --git a/mod/feedback/tests/behat/show_responses.feature b/mod/feedback/tests/behat/show_responses.feature new file mode 100644 index 00000000000..eb0e908cde1 --- /dev/null +++ b/mod/feedback/tests/behat/show_responses.feature @@ -0,0 +1,72 @@ +@mod @mod_feedback +Feature: As a teacher, I can see users who have reponded or not responded to a feedback activity. + As a non editing teacher not in a group I cannot see the responses. + + Background: + Given the following "users" exist: + | username | firstname | lastname | + | teacher1 | Teacher | 1 | + | teacher2 | Teacher | 2 | + | teacher3 | Teacher | 3 | + | student1 | Student | 1 | + | student2 | Student | 2 | + | student3 | Student | 3 | + And the following "courses" exist: + | fullname | shortname | groupmode | + | Course 1 | C1 | 1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | teacher2 | C1 | teacher | + | teacher3 | C1 | teacher | + | student1 | C1 | student | + | student2 | C1 | student | + | student3 | C1 | student | + And the following "groups" exist: + | course | name | idnumber | + | C1 | G1 | GI1 | + And the following "group members" exist: + | user | group | + | student1 | GI1 | + | student2 | GI1 | + | teacher2 | GI1 | + When I log in as "teacher1" + And I add a feedback activity to course "Course 1" section "1" and I fill the form with: + | Name | Frogs | + | Description | x | + | Record user names | User's name will be logged and shown with answers | + And I am on the Frogs "feedback activity" page + And I navigate to "Questions" in current page administration + And I add a "Short text answer" question to the feedback with: + | Question | Y/N? | + And I log out + + # Go in as student 1 and do the feedback. + And I am on the Frogs "feedback activity" page logged in as student1 + And I follow "Answer the questions" + And I set the field "Y/N?" to "Y" + And I press "Submit your answers" + And I log out + + Scenario Outline: If a teacher or non editing teacher is in a group, they can see the responses in separate group mode. + # Go in as teacher and check the users who haven't completed it. + Given I am on the Frogs "feedback activity" page logged in as + Then "Responses" "link" in current page administration + Examples: + | user | existsornot | + | teacher1 | should exist | + | teacher2 | should exist | + | teacher3 | should not exist | + + Scenario Outline: Teachers and non editing teachers in a group can see the responses + # Go in as teacher and check the users who haven't completed it. + Given I am on the Frogs "feedback activity" page logged in as + And I navigate to "Responses" in current page administration + And I select "Show non-respondents" from the "jump" singleselect + # Should only show student 2; not student 1 (they did it) or 3 (not in grouping). + Then I + And I + Examples: + | user | studentshouldsee | studentshouldnotsee | + | teacher1 | should see "Student 2" | should see "Student 3" | + | teacher2 | should see "Student 2" | should not see "Student 3" | diff --git a/mod/feedback/tests/manager_test.php b/mod/feedback/tests/manager_test.php new file mode 100644 index 00000000000..a52492042c8 --- /dev/null +++ b/mod/feedback/tests/manager_test.php @@ -0,0 +1,115 @@ +. + +namespace mod_feedback; + +use advanced_testcase; + +/** + * Class for unit testing mod_feedback\dates. + * + * @category test + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright 2025 Laurent David + * @package mod_feedback + * @covers \mod_feedback\manager + */ +final class manager_test extends advanced_testcase { + /** + * Data provider for test_can_see_others_in_groups. + * + * @return array[] + */ + public static function can_see_others_in_groups_provider(): array { + return [ + 'student no groups, separate group' => [ + 'mode' => SEPARATEGROUPS, 'username' => 's1', 'expected' => false, + ], + 'student in groups, separate group' => [ + 'mode' => SEPARATEGROUPS, 'username' => 's2', 'expected' => true, + ], + 'non editing teacher no groups, separate group' => [ + 'mode' => SEPARATEGROUPS, 'username' => 't1', 'expected' => false, + ], + 'non editing teacher in groups, separate group' => [ + 'mode' => SEPARATEGROUPS, 'username' => 't2', 'expected' => true, + ], + 'editing teacher no groups, separate group' => [ + 'mode' => SEPARATEGROUPS, 'username' => 't3', 'expected' => true, + ], + 'editing teacher in groups, separate group' => [ + 'mode' => SEPARATEGROUPS, 'username' => 't4', 'expected' => true, + ], + 'student no groups, visible group' => [ + 'mode' => VISIBLEGROUPS, 'username' => 's1', 'expected' => true, + ], + 'student in groups, visible group' => [ + 'mode' => VISIBLEGROUPS, 'username' => 's2', 'expected' => true, + ], + 'non editing teacher no groups, visible group' => [ + 'mode' => VISIBLEGROUPS, 'username' => 't1', 'expected' => true, + ], + 'non editing teacher in groups, visible group' => [ + 'mode' => VISIBLEGROUPS, 'username' => 't2', 'expected' => true, + ], + 'editing teacher no groups, visible group' => [ + 'mode' => VISIBLEGROUPS, 'username' => 't3', 'expected' => true, + ], + 'editing teacher in groups, visible group' => [ + 'mode' => VISIBLEGROUPS, 'username' => 't4', 'expected' => true, + ], + ]; + } + + /** + * Test if we can see or not others in groups. + * + * @param int $mode The group mode. + * @param string $username The username of the user to test. + * @param bool $expected The expected result. + * + * @covers ::can_see_others_in_groups + * @dataProvider can_see_others_in_groups_provider + */ + public function test_can_see_others_in_groups(int $mode, string $username, bool $expected): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(['groupmode' => $mode, 'groupmodeforce' => 1]); + $group = $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'Group 1']); + $users = []; + $data = [ + 's1' => 'student', + 's2' => 'student', + 't1' => 'teacher', + 't2' => 'teacher', + 't3' => 'editingteacher', + 't4' => 'editingteacher', + ]; + foreach ($data as $user => $role) { + $users[$user] = $this->getDataGenerator()->create_and_enrol($course, $role, $user); + } + foreach (['s2', 't2', 't3'] as $uname) { + $user = $users[$uname]; + $this->getDataGenerator()->create_group_member( + ['groupid' => $group->id, 'userid' => $user->id] + ); + } + $feedback = $this->getDataGenerator()->create_module('feedback', ['course' => $course]); + $this->setUser($users[$username]); + $cm = get_fast_modinfo($course)->cms[$feedback->cmid]; + $this->assertEquals($expected, manager::can_see_others_in_groups($cm)); + } +}