L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-85638-main' of https://github.com/sarjona/moodle

Browse Source
This commit is contained in:
Sara Arjona
2025-07-28 16:22:54 +02:00
parent 970f206939 ee1aa205dd
commit acc4c90802
6 changed files with 268 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/mod/choice/classes/courseformat/overview.php
+44 -27
View File
@@ -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,
);
}
public/mod/choice/classes/manager.php
+19 -1
View File
@@ -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',
);
}
}
public/mod/choice/lang/en/choice.php
+2 -1
View File
@@ -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';
public/mod/choice/tests/behat/overview_report.feature
+14 -1
View File
@@ -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"
public/mod/choice/tests/courseformat/overview_test.php
+103 -24
View File
@@ -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) {
public/mod/choice/tests/manager_test.php
+86 -10
View File
@@ -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);
}
}