diff --git a/public/mod/choice/classes/courseformat/overview.php b/public/mod/choice/classes/courseformat/overview.php index 4e2272a676e..7ade606f0ab 100644 --- a/public/mod/choice/classes/courseformat/overview.php +++ b/public/mod/choice/classes/courseformat/overview.php @@ -16,14 +16,15 @@ namespace mod_choice\courseformat; -use core\activity_dates; -use core_calendar\output\humandate; use cm_info; -use core_courseformat\local\overview\overviewitem; -use core\output\action_link; -use core\output\local\properties\text_align; -use core\output\local\properties\button; use mod_choice\manager; +use core\activity_dates; +use core\output\action_link; +use core_calendar\output\humandate; +use core\output\local\properties\button; +use core\output\local\properties\text_align; +use core_courseformat\local\overview\overviewitem; +use core_courseformat\output\local\overview\overviewdialog; /** * Choice overview integration. @@ -43,14 +44,11 @@ class overview extends \core_courseformat\activityoverviewbase { * * @param cm_info $cm the course module instance. * @param \core\output\renderer_helper $rendererhelper the renderer helper. - * @param \core_string_manager $sm the string manager. */ public function __construct( cm_info $cm, /** @var \core\output\renderer_helper $rendererhelper the renderer helper */ protected readonly \core\output\renderer_helper $rendererhelper, - /** @var \core_string_manager $sm the string manager */ - protected readonly \core_string_manager $sm, ) { parent::__construct($cm); $this->manager = manager::create_from_coursemodule($cm); @@ -70,7 +68,7 @@ class overview extends \core_courseformat\activityoverviewbase { } if (empty($closedate)) { return new overviewitem( - name: $this->sm->get_string('duedate', 'choice'), + name: get_string('duedate', 'choice'), value: null, content: '-', ); @@ -79,7 +77,7 @@ class overview extends \core_courseformat\activityoverviewbase { $content = humandate::create_from_timestamp($closedate); return new overviewitem( - name: $this->sm->get_string('duedate', 'choice'), + name: get_string('duedate', 'choice'), value: $closedate, content: $content, ); @@ -100,8 +98,8 @@ class overview extends \core_courseformat\activityoverviewbase { ); return new overviewitem( - name: $this->sm->get_string('actions'), - value: $this->sm->get_string('viewallresponses', 'choice', $currentanswerscount), + name: get_string('actions'), + value: get_string('viewallresponses', 'choice', $currentanswerscount), content: $content, textalign: text_align::CENTER, ); @@ -117,12 +115,8 @@ class overview extends \core_courseformat\activityoverviewbase { #[\Override] public function get_extra_overview_items(): array { - if (has_capability('mod/choice:readresponses', $this->cm->context)) { - return [ - 'studentwhoresponded' => $this->get_students_who_responded(), - ]; // If the user can read responses, we don't show the submission status as it is for the student only. - } return [ + 'studentwhoresponded' => $this->get_students_who_responded(), 'responded' => $this->get_extra_status_for_user(), ]; } @@ -130,13 +124,17 @@ class overview extends \core_courseformat\activityoverviewbase { /** * Get the response status overview item. * - * @return overviewitem An overview item + * @return overviewitem|null An overview item or null for teachers. */ - private function get_extra_status_for_user(): overviewitem { + private function get_extra_status_for_user(): ?overviewitem { + if (has_capability('mod/choice:readresponses', $this->cm->context)) { + return null; + } + $status = $this->manager->has_answered(); - $statustext = $this->sm->get_string('notanswered', 'choice'); + $statustext = get_string('notanswered', 'choice'); if ($status) { - $statustext = $this->sm->get_string('answered', 'choice'); + $statustext = get_string('answered', 'choice'); } $corerenderer = $this->rendererhelper->get_core_renderer(); $submittedstatuscontent = "-"; @@ -149,7 +147,7 @@ class overview extends \core_courseformat\activityoverviewbase { ); } return new overviewitem( - name: $this->sm->get_string('responded', 'choice'), + name: get_string('responded', 'choice'), value: $status, content: $submittedstatuscontent, textalign: text_align::CENTER, @@ -159,14 +157,33 @@ class overview extends \core_courseformat\activityoverviewbase { /** * Get the count of student who responded. * - * @return overviewitem An overview item + * @return overviewitem|null An overview item or null if for students. */ - private function get_students_who_responded(): overviewitem { + private function get_students_who_responded(): ?overviewitem { + if (!has_capability('mod/choice:readresponses', $this->cm->context)) { + return null; + } + $studentwhoanswered = $this->manager->count_all_users_answered(); + $overviewdialog = new overviewdialog( + buttoncontent: $studentwhoanswered, + title: get_string('totalresponses', 'mod_choice'), + description: $this->cm->get_instance_record()->allowmultiple ? get_string('allowmultiple', 'mod_choice') : '', + definition: ['buttonclasses' => button::BODY_OUTLINE->classes()], + ); + + $options = $this->manager->get_options(); + foreach ($options as $option) { + $overviewdialog->add_item( + $option->text, + $this->manager->count_all_users_answered($option->id), + ); + } + return new overviewitem( - name: $this->sm->get_string('studentwhoresponded', 'choice'), + name: get_string('studentwhoresponded', 'choice'), value: $studentwhoanswered, - content: $studentwhoanswered, + content: $overviewdialog, textalign: text_align::CENTER, ); } diff --git a/public/mod/choice/classes/manager.php b/public/mod/choice/classes/manager.php index 4c938073553..82cd609f6bf 100644 --- a/public/mod/choice/classes/manager.php +++ b/public/mod/choice/classes/manager.php @@ -121,14 +121,19 @@ class manager { /** * Return the current count of users who have answered this choice module, that the current user can see. * + * @param int|null $optionid the option ID to filter by, or null to count all answers * @return int the number of answers that the user can see */ - public function count_all_users_answered(): int { + public function count_all_users_answered(?int $optionid = null): int { if (!has_any_capability(['mod/choice:view', 'mod/choice:readresponses'], $this->context)) { return 0; } $where = ' WHERE ca.choiceid = :choiceid'; $params = ['choiceid' => $this->instance->id]; + if ($optionid) { + $where .= ' AND ca.optionid = :optionid'; + $params['optionid'] = $optionid; + } return $this->db->count_records_sql( 'SELECT COUNT(DISTINCT ca.userid) FROM {choice_answers} ca' . $where, $params @@ -147,4 +152,17 @@ class manager { $conditions = ['choiceid' => $this->instance->id, 'userid' => $USER->id]; return $this->db->record_exists('choice_answers', $conditions); } + + /** + * Get the options for this choice activity. + * + * @return array of choice options + */ + public function get_options(): array { + return $this->db->get_records( + 'choice_options', + ['choiceid' => $this->instance->id], + 'id ASC', + ); + } } diff --git a/public/mod/choice/lang/en/choice.php b/public/mod/choice/lang/en/choice.php index dc0940c9b51..ca57bfaf198 100644 --- a/public/mod/choice/lang/en/choice.php +++ b/public/mod/choice/lang/en/choice.php @@ -157,8 +157,9 @@ $string['spaceleft'] = 'space available'; $string['spacesleft'] = 'spaces available'; $string['studentwhoresponded'] = 'Students who responded'; $string['taken'] = 'Taken'; +$string['totalresponses'] = 'Total responses'; +$string['userchoosethisoption'] = 'Users who chose this option'; $string['viewallresponses'] = 'View {$a} responses'; $string['viewchoices'] = 'View choices'; $string['withselected'] = 'With selected'; -$string['userchoosethisoption'] = 'Users who chose this option'; $string['yourselection'] = 'Your selection'; diff --git a/public/mod/choice/tests/behat/overview_report.feature b/public/mod/choice/tests/behat/overview_report.feature index 6e8977d4706..3d6732a5fbd 100644 --- a/public/mod/choice/tests/behat/overview_report.feature +++ b/public/mod/choice/tests/behat/overview_report.feature @@ -24,7 +24,8 @@ Feature: Testing overview integration in mod_choice | activity | name | intro | course | idnumber | option | section | completion | allowmultiple | timeclose | | choice | Choice 1 | Choice Description 1 | C1 | choice1 | Option 1, Option 2 | 1 | 1 | 1 | 1 January 2040 | | choice | Choice 2 | Choice Description 2 | C1 | choice2 | Option A, Option B | 1 | 0 | 0 | | - | choice | Choice 3 | Choice Description 3 | C1 | choice3 | Option A | 1 | 0 | 0 | | + | choice | Choice 3 | Choice Description 3 | C1 | choice3 | Option A | 1 | 0 | 1 | | + | choice | Choice 4 | Choice Description 4 | C1 | choice4 | Option Z | 1 | 0 | 0 | | And the following "mod_choice > responses" exist: | choice | user | responses | | choice1 | student1 | Option 1, Option 2 | @@ -56,7 +57,9 @@ Feature: Testing overview integration in mod_choice | Choice 1 | 1 January 2040 | | Choice 2 | - | | Choice 3 | - | + | Choice 4 | - | + @javascript Scenario: Teachers can see relevant columns and content in the choice overview Given I am on the "Course 1" "course > activities > choice" page logged in as "teacher1" # Check columns. @@ -70,6 +73,16 @@ Feature: Testing overview integration in mod_choice | Choice 1 | 2 | View | | Choice 2 | 1 | View | | Choice 3 | 0 | View | + | Choice 4 | 0 | View | + And I click on "2" "button" in the "Choice 1" "table_row" + And I should see "Allow more than one choice to be selected" + And I should see "Option 1: 1" + And I should see "Option 2: 2" + And I press the escape key + And I click on "0" "button" in the "Choice 4" "table_row" + And I should not see "Allow more than one choice to be selected" + And I should see "Option Z: 0" + And I press the escape key Scenario: The choice index redirect to the activities overview When I log in as "admin" diff --git a/public/mod/choice/tests/courseformat/overview_test.php b/public/mod/choice/tests/courseformat/overview_test.php index 5814db53917..8781a61f2b7 100644 --- a/public/mod/choice/tests/courseformat/overview_test.php +++ b/public/mod/choice/tests/courseformat/overview_test.php @@ -17,6 +17,7 @@ namespace mod_choice\courseformat; use core_courseformat\local\overview\overviewfactory; +use core_courseformat\output\local\overview\overviewdialog; /** * Tests for Choice integration. @@ -40,15 +41,14 @@ final class overview_test extends \advanced_testcase { public function test_get_extra_status_for_user(string $user, ?bool $answered): void { $this->resetAfterTest(); ['users' => $users, 'course' => $course, 'instance' => $instance] = - $this->setup_users_and_activity($answered ?? false); + $this->setup_users_and_activity(false, $answered ?? false); $cm = get_fast_modinfo($course)->get_cm($instance->cmid); $this->setUser($users[$user]); $overview = overviewfactory::create($cm); $actionoverview = $overview->get_extra_overview_items(); if ($answered === null) { - $this->assertArrayNotHasKey('responded', $actionoverview); + $this->assertNull($actionoverview['responded']); } else { - $this->assertArrayHasKey('responded', $actionoverview); $this->assertEquals($answered, $actionoverview['responded']->get_value()); if (!$answered) { $this->assertEquals('-', $actionoverview['responded']->get_content()); @@ -137,11 +137,11 @@ final class overview_test extends \advanced_testcase { /** * Test get_actions_overview method. * - * @param string $username - * @param int|null $expectedcount + * @param string $username The username of the user to test. + * @param int|null $expectedcount the expected count of users who responded * * @covers ::get_actions_overview - * @dataProvider data_provider_get_student_responded_count + * @dataProvider provider_test_get_actions_overview */ public function test_get_actions_overview(string $username, ?int $expectedcount = null): void { $this->resetAfterTest(); @@ -161,43 +161,118 @@ final class overview_test extends \advanced_testcase { } } + /** + * Data provider for test_get_actions_overview. + * + * @return array the data provider array + */ + public static function provider_test_get_actions_overview(): array { + return [ + 'Student' => [ + 'username' => 's1', + 'expectedcount' => null, + ], + 'Teacher' => [ + 'username' => 't1', + 'expectedcount' => 2, + ], + ]; + } + /** * Test get_actions_overview method. * - * @param string $username - * @param int|null $expectedcount + * @param string $username The username of the user to test. + * @param bool $allowmultiple whether the choice allows multiple answers + * @param bool $withanswers whether the choice will be created with answers + * @param int|null $expectedcount the expected count of users who responded * * @covers ::get_actions_overview - * @dataProvider data_provider_get_student_responded_count + * @dataProvider provider_get_student_responded_count */ - public function test_get_students_who_responded(string $username, ?int $expectedcount = null): void { + public function test_get_students_who_responded( + string $username, + bool $allowmultiple = false, + bool $withanswers = true, + ?int $expectedcount = null, + ): void { $this->resetAfterTest(); - ['users' => $users, 'course' => $course, 'instance' => $instance] = $this->setup_users_and_activity(); + [ + 'users' => $users, + 'course' => $course, + 'instance' => $instance + ] = $this->setup_users_and_activity($allowmultiple, $withanswers); $cm = get_fast_modinfo($course)->get_cm($instance->cmid); $this->setUser($users[$username]); $overview = overviewfactory::create($cm); - $actionoverview = $overview->get_extra_overview_items(); + $result = $overview->get_extra_overview_items(); if (is_null($expectedcount)) { - $this->assertArrayNotHasKey('studentwhoresponded', $actionoverview); + $this->assertNull($result['studentwhoresponded']); } else { - $this->assertArrayHasKey('studentwhoresponded', $actionoverview); $this->assertEquals( $expectedcount, - $actionoverview['studentwhoresponded']->get_value(), + $result['studentwhoresponded']->get_value(), + ); + /** @var \core_courseformat\output\local\overview\overviewdialog $content */ + $content = $result['studentwhoresponded']->get_content(); + $reflection = new \ReflectionClass($content); + $description = $reflection->getProperty('description'); + $description->setAccessible(true); + if ($allowmultiple) { + $this->assertEquals( + get_string('allowmultiple', 'mod_choice'), + $description->getValue($content), + ); + } else { + $this->assertEmpty($description->getValue($content)); + } + + $reflection = new \ReflectionClass($content); + $items = $reflection->getProperty('items'); + $items->setAccessible(true); + $this->assertEquals( + 3, // A, B, C. + count($items->getValue($content)), ); } } + /** - * Data provider for get_actions_overview. + * Data provider for test_get_students_who_responded. * - * @return array + * @return array the data provider array */ - public static function data_provider_get_student_responded_count(): array { - return [ - 'teacher 1' => ['t1', 2], - 'student 1' => ['s1', null], - ]; + public static function provider_get_student_responded_count(): array { + return [ + 'Student' => [ + 'username' => 's1', + 'expectedcount' => null, + ], + 'Teacher - With answers - No multiple' => [ + 'username' => 't1', + 'allowmultiple' => false, + 'withanswers' => true, + 'expectedcount' => 2, + ], + 'Teacher - Without answers - No multiple' => [ + 'username' => 't1', + 'withanswers' => false, + 'expectedcount' => 0, + ], + 'Teacher - With answers - Multiple' => [ + 'username' => 't1', + 'allowmultiple' => true, + 'withanswers' => true, + 'expectedcount' => 2, + ], + 'Teacher - Without answers - Multiple' => [ + 'username' => 't1', + 'allowmultiple' => true, + 'withanswers' => false, + 'expectedcount' => 0, + ], + ]; } /** @@ -207,7 +282,10 @@ final class overview_test extends \advanced_testcase { * * @return array */ - private function setup_users_and_activity(bool $withanswers = true): array { + private function setup_users_and_activity( + bool $allowmultiple = false, + bool $withanswers = true + ): array { $this->setAdminUser(); $db = \core\di::get(\moodle_database::class); $generator = $this->getDataGenerator(); @@ -217,7 +295,8 @@ final class overview_test extends \advanced_testcase { } $instance = $generator->create_module('choice', [ 'course' => $course, - 'option' => ['A', 'B'], + 'option' => ['A', 'B', 'C'], + 'allowmultiple' => $allowmultiple, ]); if ($withanswers) { diff --git a/public/mod/choice/tests/manager_test.php b/public/mod/choice/tests/manager_test.php index ceb56b3b39f..91fe065dfa2 100644 --- a/public/mod/choice/tests/manager_test.php +++ b/public/mod/choice/tests/manager_test.php @@ -156,7 +156,7 @@ final class manager_test extends \advanced_testcase { * @param int $expectedcount the expected count of answers for the user. * * @covers \mod_choice\manager::count_all_users_answered - * @dataProvider count_all_answers_provider + * @dataProvider provider_count_all_answers */ public function test_count_all_users_answered( string $username, @@ -165,6 +165,8 @@ final class manager_test extends \advanced_testcase { int $expectedcount ): void { global $SESSION; + + $db = \core\di::get(\moodle_database::class); [ 'users' => $users, 'instance' => $instance, @@ -179,6 +181,21 @@ final class manager_test extends \advanced_testcase { } $count = $manager->count_all_users_answered(); $this->assertEquals($expectedcount, $count); + + // Check answers count for each option. + $options = $db->get_records_menu('choice_options', ['choiceid' => $instance->id], '', 'id, text'); + foreach ($options as $optionid => $optiontext) { + $count = $manager->count_all_users_answered($optionid); + if ($optiontext === 'A') { + $this->assertEquals(1, $count); + } else if ($optiontext === 'B') { + $this->assertEquals(2, $count); + } else { + // Option C has no answers. + $this->assertEquals(0, $count); + } + } + } /** @@ -186,18 +203,53 @@ final class manager_test extends \advanced_testcase { * * @return array */ - public static function count_all_answers_provider(): array { + public static function provider_count_all_answers(): array { return [ - 'teacher 1 (no group mode)' => ['t1', NOGROUPS, null, 2], + 'Teacher in a group - No group mode' => [ + 'username' => 't1', + 'coursegroupmode' => NOGROUPS, + 'currentgroup' => null, + 'expectedcount' => 2, + ], // This test about SEPARATEGROUPS it will be the subject of an follow up ticket (MDL-85852). - 'teacher 1 (separate group mode) - group 1 set' => ['t1', SEPARATEGROUPS, null, 2], - 'teacher 1 (separate group mode)' => ['t1', SEPARATEGROUPS, 'g1', 2], - 'teacher 1 (visible group mode)' => ['t1', VISIBLEGROUPS, null, 2], + 'Teacher in a group - Separate group mode' => [ + 'username' => 't1', + 'coursegroupmode' => SEPARATEGROUPS, + 'currentgroup' => null, + 'expectedcount' => 2, + ], + 'Teacher in a group - Separate group mode - Group1' => [ + 'username' => 't1', + 'coursegroupmode' => SEPARATEGROUPS, + 'currentgroup' => 'g1', + 'expectedcount' => 2, + ], + 'Teacher in a group - Visible group mode' => [ + 'username' => 't1', + 'coursegroupmode' => VISIBLEGROUPS, + 'currentgroup' => null, + 'expectedcount' => 2, + ], // Teacher 2 does not belong to any group. - 'teacher 2 (no group mode)' => ['t2', NOGROUPS, null, 2], - // This test about SEPARATEGROUPS it will be the subject of an follow up ticket (MDL-85852). - 'teacher 2 (separate group mode)' => ['t2', SEPARATEGROUPS, null, 2], - 'teacher 2 (visible group mode)' => ['t2', VISIBLEGROUPS, null, 2], + 'Teacher without group - No group mode' => [ + 'username' => 't2', + 'coursegroupmode' => NOGROUPS, + 'currentgroup' => null, + 'expectedcount' => 2, + ], + // These tests about SEPARATEGROUPS will be the subject of an follow up ticket (MDL-85852). + 'Teacher without group - Separate group mode' => [ + 'username' => 't2', + 'coursegroupmode' => SEPARATEGROUPS, + 'currentgroup' => null, + 'expectedcount' => 2, + ], + 'Teacher without group - Visible group mode' => [ + 'username' => 't2', + 'coursegroupmode' => VISIBLEGROUPS, + 'currentgroup' => null, + 'expectedcount' => 2, + ], ]; } @@ -214,4 +266,28 @@ final class manager_test extends \advanced_testcase { $this->setUser($users['s3']); $this->assertFalse($manager->has_answered()); } + + /** + * Test get_options method. + * + * @covers \mod_choice\manager::get_options + */ + public function test_get_options(): void { + $course = $this->getDataGenerator()->create_course(); + $instance1 = $this->getDataGenerator()->create_module('choice', [ + 'course' => $course, + 'option' => ['A', 'B', 'C'], + ]); + $manager = \mod_choice\manager::create_from_instance($instance1); + $options = $manager->get_options(); + $this->assertCount(3, $options); + + $instance2 = $this->getDataGenerator()->create_module('choice', [ + 'course' => $course, + 'option' => ['111'], + ]); + $manager = \mod_choice\manager::create_from_instance($instance2); + $options = $manager->get_options(); + $this->assertCount(1, $options); + } }