From 1cd8adcd2abde7b00ceade638a33cad96455e7ef Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Wed, 6 Aug 2025 17:02:32 +0200 Subject: [PATCH] MDL-85850 feedback: Implement group based information in overview page --- .upgradenotes/MDL-85850-2025080710081091.yml | 9 + .../classes/courseformat/overview.php | 5 +- public/mod/feedback/lib.php | 83 ++++--- .../tests/behat/overview_report.feature | 28 +-- .../tests/courseformat/overview_test.php | 213 ++++++++++++++++++ public/mod/feedback/tests/lib_test.php | 167 ++++++++++++++ 6 files changed, 457 insertions(+), 48 deletions(-) create mode 100644 .upgradenotes/MDL-85850-2025080710081091.yml diff --git a/.upgradenotes/MDL-85850-2025080710081091.yml b/.upgradenotes/MDL-85850-2025080710081091.yml new file mode 100644 index 00000000000..421a0e195e9 --- /dev/null +++ b/.upgradenotes/MDL-85850-2025080710081091.yml @@ -0,0 +1,9 @@ +issueNumber: MDL-85850 +notes: + mod_feedback: + - message: >- + Two new methods, `feedback_get_completeds` and + `feedback_get_completeds_count`, have been added to the feedback API. + These methods allow you to retrieve completed items based on multiple + groups. + type: improved diff --git a/public/mod/feedback/classes/courseformat/overview.php b/public/mod/feedback/classes/courseformat/overview.php index e05433442ba..de180b7f6c0 100644 --- a/public/mod/feedback/classes/courseformat/overview.php +++ b/public/mod/feedback/classes/courseformat/overview.php @@ -96,8 +96,9 @@ class overview extends \core_courseformat\activityoverviewbase { require_once($CFG->dirroot . '/mod/feedback/lib.php'); - $submissions = feedback_get_completeds_group_count( - $this->cm->get_instance_record() + $submissions = feedback_get_completeds_count( + $this->cm->get_instance_record(), + $this->get_groups_for_filtering(), ); // Normalize the value. if (!$submissions) { diff --git a/public/mod/feedback/lib.php b/public/mod/feedback/lib.php index 66fc211e998..bf4ab1ce994 100644 --- a/public/mod/feedback/lib.php +++ b/public/mod/feedback/lib.php @@ -2131,6 +2131,44 @@ function feedback_is_already_submitted($feedbackid, $courseid = false) { return $DB->record_exists('feedback_completed', $params); } +/** + * Get the completeds depending on the given groups. + * This method doesn't check if the user has the capability to view the defined groups, + * so this should be checked before calling this function. + * + * @param stdClass $feedback The feedback object. + * @param array $groups Identifiers of the groups to filter by. + * @return array Array of completed records. + */ +function feedback_get_completeds(stdClass $feedback, array $groups = []) { + $db = \core\di::get(\moodle_database::class); + + if (empty($groups)) { + // If no groups are specified, return all completeds for the feedback. + return $db->get_records('feedback_completed', ['feedback' => $feedback->id]); + } + + [$sql, $params] = $db->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $query = 'SELECT fbc.* + FROM {feedback_completed} fbc, {groups_members} gm + WHERE fbc.feedback = :feedbackid + AND (gm.groupid ' . $sql . ' OR gm.groupid = 0) + AND fbc.userid = gm.userid'; + $params['feedbackid'] = $feedback->id; + return $db->get_records_sql($query, $params); +} + +/** + * Get the count of completeds depending on the given group identifiers. + * + * @param stdClass $feedback The feedback object. + * @param array $groups Identifiers of the groups to filter by. + * @return int Count of completeds. + */ +function feedback_get_completeds_count(stdClass $feedback, array $groups = []): int { + return count(feedback_get_completeds($feedback, $groups)); +} + /** * get the completeds depending on the given groupid. * @@ -2142,39 +2180,26 @@ function feedback_is_already_submitted($feedbackid, $courseid = false) { * @return mixed array of found completeds otherwise false */ function feedback_get_completeds_group($feedback, $groupid = false, $courseid = false) { - global $CFG, $DB; + global $DB; - if (intval($groupid) > 0) { - $query = "SELECT fbc.* - FROM {feedback_completed} fbc, {groups_members} gm - WHERE fbc.feedback = ? - AND gm.groupid = ? - AND fbc.userid = gm.userid"; - if ($values = $DB->get_records_sql($query, array($feedback->id, $groupid))) { - return $values; - } else { + if (intval($groupid) > 0 || !$courseid) { + $values = feedback_get_completeds($feedback, [$groupid]); + if (empty($values)) { return false; } + return $values; + } + + $query = "SELECT DISTINCT fbc.* + FROM {feedback_completed} fbc, {feedback_value} fbv + WHERE fbc.id = fbv.completed + AND fbc.feedback = ? + AND fbv.course_id = ? + ORDER BY random_response"; + if ($values = $DB->get_records_sql($query, [$feedback->id, $courseid])) { + return $values; } else { - if ($courseid) { - $query = "SELECT DISTINCT fbc.* - FROM {feedback_completed} fbc, {feedback_value} fbv - WHERE fbc.id = fbv.completed - AND fbc.feedback = ? - AND fbv.course_id = ? - ORDER BY random_response"; - if ($values = $DB->get_records_sql($query, array($feedback->id, $courseid))) { - return $values; - } else { - return false; - } - } else { - if ($values = $DB->get_records('feedback_completed', array('feedback'=>$feedback->id))) { - return $values; - } else { - return false; - } - } + return false; } } diff --git a/public/mod/feedback/tests/behat/overview_report.feature b/public/mod/feedback/tests/behat/overview_report.feature index e38905288ff..9731891552c 100644 --- a/public/mod/feedback/tests/behat/overview_report.feature +++ b/public/mod/feedback/tests/behat/overview_report.feature @@ -15,7 +15,7 @@ Feature: Testing overview integration in mod_feedback | student6 | Username | 6 | | student7 | Username | 7 | | student8 | Username | 8 | - | teacher1 | Teacher | T | + | teacher1 | Teacher | T | And the following "courses" exist: | fullname | shortname | groupmode | | Course 1 | C1 | 1 | @@ -55,29 +55,23 @@ Feature: Testing overview integration in mod_feedback Scenario: Teacher can see the feedback relevant information in the feedback overview When I am on the "Course 1" "course > activities > feedback" page logged in as "teacher1" - Then I should see "Responses" in the "feedback_overview_collapsible" "region" + Then the following should exist in the "Table listing all Feedback activities" table: + | Name | Due date | Responses | Actions | + | Date feedback | 1 January 2040 | 4 | View | + | Not responded feedback | Tomorrow | 0 | View | + | No date feedback | - | 3 | View | And I should not see "Responded" in the "feedback_overview_collapsible" "region" - And I should see "Due date" in the "feedback_overview_collapsible" "region" - And I should see "1 January 2040" in the "Date feedback" "table_row" - And I should see "4" in the "Date feedback" "table_row" - And I should see "Tomorrow" in the "Not responded feedback" "table_row" - And I should see "0" in the "Not responded feedback" "table_row" - And I should see "-" in the "No date feedback" "table_row" - And I should see "3" in the "No date feedback" "table_row" And I click on "View" "link" in the "Date feedback" "table_row" And I should see "Show responses" Scenario: Students can see the feedback relevant information in the feedback overview When I am on the "Course 1" "course > activities > feedback" page logged in as "student1" - Then I should not see "Responses" in the "feedback_overview_collapsible" "region" - And I should not see "Actions" in the "feedback_overview_collapsible" "region" - And I should see "Responded" in the "feedback_overview_collapsible" "region" - And I should see "Due date" in the "feedback_overview_collapsible" "region" - And I should see "1 January 2040" in the "Date feedback" "table_row" + Then the following should exist in the "Table listing all Feedback activities" table: + | Name | Due date | Responded | + | Date feedback | 1 January 2040 | | + | Not responded feedback | Tomorrow | - | + | No date feedback | - | | And "You have already submitted this feedback" "icon" should exist in the "Date feedback" "table_row" - And I should see "Tomorrow" in the "Not responded feedback" "table_row" - And I should see "-" in the "Not responded feedback" "table_row" - And I should see "-" in the "No date feedback" "table_row" And "You have already submitted this feedback" "icon" should exist in the "No date feedback" "table_row" Scenario: The feedback overview report should generate log events diff --git a/public/mod/feedback/tests/courseformat/overview_test.php b/public/mod/feedback/tests/courseformat/overview_test.php index c114b03bdb2..ed2e6145b4b 100644 --- a/public/mod/feedback/tests/courseformat/overview_test.php +++ b/public/mod/feedback/tests/courseformat/overview_test.php @@ -240,6 +240,219 @@ final class overview_test extends \advanced_testcase { ]; } + /** + * Test get_extra_responses_overview_with_groups(). + * + * @dataProvider provider_feedback_get_extra_responses_overview_with_groups + * @param int $groupmode The group mode of the course. + * @param string $currentuser The user to set for the test. + * @param int $expectedcount The expected number of completeds. + * + * @covers ::get_extra_responses_overview + */ + public function test_get_extra_responses_overview_with_groups( + int $groupmode, + string $currentuser, + int $expectedcount, + ): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course([ + 'groupmode' => $groupmode, + 'groupmodeforce' => true, + ]); + $allgroups = [ + 'groupa' => $this->getDataGenerator()->create_group(['courseid' => $course->id]), + 'groupb' => $this->getDataGenerator()->create_group(['courseid' => $course->id]), + 'groupc' => $this->getDataGenerator()->create_group(['courseid' => $course->id]), + ]; + + // Participant: Role: Groups: + // student1a student groupa + // student2a student groupa + // student3b student groupb + // teacher1 editingteacher groupa + // teacher2 teacher groupa + // teacher3 teacher groupb + // teacher4 teacher groupc + // teacher5 teacher (no group) + // teacher6 editingteacher (no group) . + $student1a = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupa']->id, + 'userid' => $student1a->id, + ]); + $student2a = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupa']->id, + 'userid' => $student2a->id, + ]); + $student3b = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupb']->id, + 'userid' => $student3b->id, + ]); + $teachers['teacher1'] = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupa']->id, + 'userid' => $teachers['teacher1']->id, + ]); + $teachers['teacher2'] = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupa']->id, + 'userid' => $teachers['teacher2']->id, + ]); + $teachers['teacher3'] = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupb']->id, + 'userid' => $teachers['teacher3']->id, + ]); + $teachers['teacher4'] = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupc']->id, + 'userid' => $teachers['teacher4']->id, + ]); + $teachers['teacher5'] = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $teachers['teacher6'] = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher'); + + $activity = $this->getDataGenerator()->create_module('feedback', ['course' => $course->id]); + $cm = get_fast_modinfo($course)->get_cm($activity->cmid); + + // Add a multichoice item to the feedback and create responses for it. + /** @var \mod_feedback_generator $feedbackgenerator */ + $feedbackgenerator = $this->getDataGenerator()->get_plugin_generator('mod_feedback'); + $item = $feedbackgenerator->create_item_multichoice($activity, ['values' => "y\nn"]); + $feedbackgenerator->create_response([ + 'userid' => $student1a->id, + 'cmid' => $cm->id, + 'anonymous' => false, + $item->name => 'y', + ]); + $feedbackgenerator->create_response([ + 'userid' => $student2a->id, + 'cmid' => $cm->id, + 'anonymous' => false, + $item->name => 'n', + ]); + $feedbackgenerator->create_response([ + 'userid' => $student3b->id, + 'cmid' => $cm->id, + 'anonymous' => false, + $item->name => 'y', + ]); + + $this->setUser($teachers[$currentuser]); + + $overview = overviewfactory::create($cm); + $reflection = new \ReflectionClass($overview); + $method = $reflection->getMethod('get_extra_responses_overview'); + $method->setAccessible(true); + $item = $method->invoke($overview); + + $this->assertEquals(get_string('responses', 'mod_feedback'), $item->get_name()); + $this->assertEquals($expectedcount, $item->get_value()); + } + + /** + * Data provider for feedback_get_extra_responses_overview_with_groups. + * + * @return array + */ + public static function provider_feedback_get_extra_responses_overview_with_groups(): array { + return [ + 'Separate groups - Editing teacher' => [ + 'groupmode' => SEPARATEGROUPS, + 'currentuser' => 'teacher1', + 'expectedcount' => 3, + ], + 'Separate groups - Non-editing teacher (groupa)' => [ + 'groupmode' => SEPARATEGROUPS, + 'currentuser' => 'teacher2', + 'expectedcount' => 2, + ], + 'Separate groups - Non-editing teacher (groupb)' => [ + 'groupmode' => SEPARATEGROUPS, + 'currentuser' => 'teacher3', + 'expectedcount' => 1, + ], + 'Separate groups - Non-editing teacher (groupc)' => [ + 'groupmode' => SEPARATEGROUPS, + 'currentuser' => 'teacher4', + 'expectedcount' => 0, + ], + 'Separate groups - Non-editing teacher (no group)' => [ + 'groupmode' => SEPARATEGROUPS, + 'currentuser' => 'teacher5', + 'expectedcount' => 3, // Although the expected count should be 0, this information will never be shown to the user. + ], + 'Separate groups - Editing teacher (no group)' => [ + 'groupmode' => SEPARATEGROUPS, + 'currentuser' => 'teacher6', + 'expectedcount' => 3, + ], + 'Visible groups - Editing teacher' => [ + 'groupmode' => VISIBLEGROUPS, + 'currentuser' => 'teacher1', + 'expectedcount' => 3, + ], + 'Visible groups - Non-editing teacher (groupa)' => [ + 'groupmode' => VISIBLEGROUPS, + 'currentuser' => 'teacher2', + 'expectedcount' => 3, + ], + 'Visible groups - Non-editing teacher (groupb)' => [ + 'groupmode' => VISIBLEGROUPS, + 'currentuser' => 'teacher3', + 'expectedcount' => 3, + ], + 'Visible groups - Non-editing teacher (groupc)' => [ + 'groupmode' => VISIBLEGROUPS, + 'currentuser' => 'teacher4', + 'expectedcount' => 3, + ], + 'Visible groups - Non-editing teacher (no group)' => [ + 'groupmode' => VISIBLEGROUPS, + 'currentuser' => 'teacher5', + 'expectedcount' => 3, + ], + 'Visible groups - Editing teacher (no group)' => [ + 'groupmode' => VISIBLEGROUPS, + 'currentuser' => 'teacher6', + 'expectedcount' => 3, + ], + 'No groups - Editing teacher' => [ + 'groupmode' => NOGROUPS, + 'currentuser' => 'teacher1', + 'expectedcount' => 3, + ], + 'No groups - Non-editing teacher (groupa)' => [ + 'groupmode' => NOGROUPS, + 'currentuser' => 'teacher2', + 'expectedcount' => 3, + ], + 'No groups - Non-editing teacher (groupb)' => [ + 'groupmode' => NOGROUPS, + 'currentuser' => 'teacher3', + 'expectedcount' => 3, + ], + 'No groups - Non-editing teacher (groupc)' => [ + 'groupmode' => NOGROUPS, + 'currentuser' => 'teacher4', + 'expectedcount' => 3, + ], + 'No groups - Non-editing teacher (no group)' => [ + 'groupmode' => NOGROUPS, + 'currentuser' => 'teacher5', + 'expectedcount' => 3, + ], + 'No groups - Editing teacher (no group)' => [ + 'groupmode' => NOGROUPS, + 'currentuser' => 'teacher6', + 'expectedcount' => 3, + ], + ]; + } + /** * Test get_extra_submitted_overview. * diff --git a/public/mod/feedback/tests/lib_test.php b/public/mod/feedback/tests/lib_test.php index 24625409fb1..26aa41a4c35 100644 --- a/public/mod/feedback/tests/lib_test.php +++ b/public/mod/feedback/tests/lib_test.php @@ -1209,4 +1209,171 @@ final class lib_test extends \advanced_testcase { $this->assertNotEmpty($teacherwithnogroup[$teacher['teacher3']->id]); $this->assertNotEmpty($teacherwithnogroup[$teacher['teacher4']->id]); } + + /** + * Test feedback_get_completeds(). + * + * @covers ::feedback_get_completeds + * @covers ::feedback_get_completeds_count + * @dataProvider provider_feedback_get_completeds + * + * @param int $groupmode The group mode of the course. + * @param array $selectedgroups The groups selected for filtering. + * @param int $expectedcount The expected number of completeds. + */ + public function test_feedback_get_completeds( + int $groupmode, + array $selectedgroups, + int $expectedcount, + ): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course([ + 'groupmode' => $groupmode, + 'groupmodeforce' => true, + ]); + $allgroups = [ + 'groupa' => $this->getDataGenerator()->create_group(['courseid' => $course->id]), + 'groupb' => $this->getDataGenerator()->create_group(['courseid' => $course->id]), + 'groupc' => $this->getDataGenerator()->create_group(['courseid' => $course->id]), + ]; + + // Participant: Role: Groups: + // student1a student groupa + // student2a student groupa + // student3b student groupb + // teacher editingteacher no group . + $student1a = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupa']->id, + 'userid' => $student1a->id, + ]); + $student2a = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupa']->id, + 'userid' => $student2a->id, + ]); + $student3b = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $allgroups['groupb']->id, + 'userid' => $student3b->id, + ]); + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher'); + + $activity = $this->getDataGenerator()->create_module('feedback', ['course' => $course->id]); + $cm = get_fast_modinfo($course)->get_cm($activity->cmid); + + // Add a multichoice item to the feedback and create responses for it. + /** @var \mod_feedback_generator $feedbackgenerator */ + $feedbackgenerator = $this->getDataGenerator()->get_plugin_generator('mod_feedback'); + $item = $feedbackgenerator->create_item_multichoice($activity, ['values' => "y\nn"]); + $feedbackgenerator->create_response([ + 'userid' => $student1a->id, + 'cmid' => $cm->id, + 'anonymous' => false, + $item->name => 'y', + ]); + $feedbackgenerator->create_response([ + 'userid' => $student2a->id, + 'cmid' => $cm->id, + 'anonymous' => false, + $item->name => 'n', + ]); + $feedbackgenerator->create_response([ + 'userid' => $student3b->id, + 'cmid' => $cm->id, + 'anonymous' => false, + $item->name => 'y', + ]); + + $this->setUser($teacher); + $groups = []; + if (!empty($selectedgroups)) { + foreach ($selectedgroups as $group) { + if ($group === 'unexisting') { + $groups[666] = new \stdClass(); + } else { + $groups[$allgroups[$group]->id] = $allgroups[$group]; + } + } + } + + $result = feedback_get_completeds($activity, $groups); + $count = feedback_get_completeds_count($activity, $groups); + $this->assertCount($expectedcount, $result); + $this->assertEquals($expectedcount, $count); + foreach ($result as $completed) { + $this->assertEquals($activity->id, $completed->feedback); + } + } + + /** + * Data provider for test_feedback_get_completeds(). + * + * @return array + */ + public static function provider_feedback_get_completeds(): array { + return [ + 'Separate groups - All completeds, without filtering by groups' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => [], + 'expectedcount' => 3, + ], + 'Separate groups - Filter by groupa' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => ['groupa'], + 'expectedcount' => 2, + ], + 'Separate groups - Filter by groupb' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => ['groupb'], + 'expectedcount' => 1, + ], + 'Separate groups - Filter by groupc' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => ['groupc'], + 'expectedcount' => 0, + ], + 'Separate groups - Filter by groupa, groupb and groupc' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => ['groupa', 'groupb', 'groupc'], + 'expectedcount' => 3, + ], + 'Separate groups - Unexisting groupid' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => ['unexisting'], + 'expectedcount' => 0, + ], + 'Visible groups - All completeds, without filtering by groups' => [ + 'groupmode' => VISIBLEGROUPS, + 'selectedgroups' => [], + 'expectedcount' => 3, + ], + 'Visible groups - Filter by groupa' => [ + 'groupmode' => VISIBLEGROUPS, + 'selectedgroups' => ['groupa'], + 'expectedcount' => 2, + ], + 'Visible groups - Filter by groupb' => [ + 'groupmode' => VISIBLEGROUPS, + 'selectedgroups' => ['groupb'], + 'expectedcount' => 1, + ], + 'Visible groups - Filter by groupc' => [ + 'groupmode' => VISIBLEGROUPS, + 'selectedgroups' => ['groupc'], + 'expectedcount' => 0, + ], + 'Visible groups - Filter by groupa, groupb and groupc' => [ + 'groupmode' => VISIBLEGROUPS, + 'selectedgroups' => ['groupa', 'groupb', 'groupc'], + 'expectedcount' => 3, + ], + 'Visible groups - Unexisting groupid' => [ + 'groupmode' => SEPARATEGROUPS, + 'selectedgroups' => ['unexisting'], + 'expectedcount' => 0, + ], + ]; + } }