diff --git a/.upgradenotes/MDL-85852-2025071709274388.yml b/.upgradenotes/MDL-85852-2025071709274388.yml new file mode 100644 index 00000000000..f8d2093be47 --- /dev/null +++ b/.upgradenotes/MDL-85852-2025071709274388.yml @@ -0,0 +1,16 @@ +issueNumber: MDL-85852 +notes: + core_courseformat: + - message: >- + New needs_filtering_by_groups() and get_groups_for_filtering() had been + created in activityoverviewbase class for a better management of groups + filtering in Activities overview page by activities. + needs_filtering_by_groups() returns whether the user needs to filter by + groups in the current module, and get_groups_for_filtering() returns + which is the filter the user should use with groups API. + type: improved + - message: >- + A new has_error() function has been created in activityoverviewbase + class to raise when a user is trying to check information about a module + set as SEPARATE_GROUPS but the user is not in any group. + type: improved diff --git a/.upgradenotes/MDL-85852-2025071807250090.yml b/.upgradenotes/MDL-85852-2025071807250090.yml new file mode 100644 index 00000000000..8c27d9084df --- /dev/null +++ b/.upgradenotes/MDL-85852-2025071807250090.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-85852 +notes: + core_courseformat: + - message: >- + New optional $nogroupserror parameter has been added to activityname + class constructor. A set_nogroupserror() setter to change the value + after the constructor has been also added. + type: improved diff --git a/public/course/format/classes/activityoverviewbase.php b/public/course/format/classes/activityoverviewbase.php index beb95a9992f..33dc299ded6 100644 --- a/public/course/format/classes/activityoverviewbase.php +++ b/public/course/format/classes/activityoverviewbase.php @@ -46,6 +46,15 @@ abstract class activityoverviewbase { /** @var courseformat $format the course format */ protected courseformat $format; + /** @var ?bool $needsfiltering whether the current user needs to filter by groups or not in the current module */ + protected ?bool $needsfiltering = null; + + /** @var array $groupstofilterby the array of groups to use as parameter for the groups API. Empty array for all groups */ + protected array $groupstofilterby; + + /** @var bool $nogroupserror Whether the user has no permission to view any student */ + protected bool $nogroupserror; + /** * Activity Overview Base class constructor. * @@ -70,6 +79,8 @@ abstract class activityoverviewbase { $this->context = $cm->context; $this->course = $cm->get_course(); $this->format = courseformat::instance($this->course); + + $this->nogroupserror = ($this->needs_filtering_by_groups() && empty($this->get_groups_for_filtering())); } /** @@ -82,6 +93,45 @@ abstract class activityoverviewbase { redirect(overviewpage::get_modname_url($courseid, $modname)); } + /** + * Checks the module's group mode, the user's capabilities and returns + * whether overview page needs to filter by group. + * + * @return bool Whether current user needs to filter by group in the current module. + */ + public function needs_filtering_by_groups(): bool { + if ($this->needsfiltering != null) { + return $this->needsfiltering; + } + if (has_capability('moodle/site:accessallgroups', $this->context)) { + $this->needsfiltering = false; + return $this->needsfiltering; + } + $groupmode = groups_get_activity_groupmode($this->cm); + if ($groupmode != SEPARATEGROUPS) { + $this->needsfiltering = false; + return $this->needsfiltering; + } + $this->needsfiltering = true; + return $this->needsfiltering; + } + + /** + * Returns an array of the groups to filter by using groups API. + * Empty array for non-filtering by groups. + * + * @return array Groups to filter by. + */ + public function get_groups_for_filtering(): array { + if (!$this->needsfiltering) { + return []; + } + if (!isset($this->groupstofilterby)) { + $this->groupstofilterby = groups_get_activity_allowed_groups($this->cm); + } + return $this->groupstofilterby; + } + /** * Get the plugin specific overview items for the activity. * @@ -102,7 +152,7 @@ abstract class activityoverviewbase { return new overviewitem( name: get_string('name'), value: $this->cm->name, - content: new activityname($this->cm), + content: new activityname($this->cm, $this->nogroupserror), ); } @@ -255,4 +305,13 @@ abstract class activityoverviewbase { } return []; } + + /** + * Wether the activity should show an error because the user is not in any group and they should be. + * + * @return bool nogroupserror property. + */ + public function has_error(): bool { + return $this->nogroupserror; + } } diff --git a/public/course/format/classes/output/local/overview/activityname.php b/public/course/format/classes/output/local/overview/activityname.php index 2f2754b9796..07477b57176 100644 --- a/public/course/format/classes/output/local/overview/activityname.php +++ b/public/course/format/classes/output/local/overview/activityname.php @@ -39,9 +39,22 @@ class activityname implements renderable, named_templatable { public function __construct( /** @var cm_info The course module. */ protected cm_info $cm, + /** @var bool Should show no groups error */ + protected bool $nogroupserror = false, ) { } + /** + * nogroupserror property setter + * + * @param bool $nogroupserror New value fpr nogroupserror property + * @return $this + */ + public function set_nogroupserror(bool $nogroupserror): self { + $this->nogroupserror = $nogroupserror; + return $this; + } + /** * Export this data so it can be used as the context for a mustache template. * @@ -59,6 +72,7 @@ class activityname implements renderable, named_templatable { 'activityurl' => $cm->url, 'hidden' => empty($cm->visible), 'stealth' => $cm->is_stealth(), + 'nogroupserror' => $this->nogroupserror, ]; if ($format->uses_sections()) { $result->sectiontitle = $format->get_section_name($section); diff --git a/public/course/format/classes/output/local/overview/overviewtable.php b/public/course/format/classes/output/local/overview/overviewtable.php index 62199272e11..59c2b7f43e2 100644 --- a/public/course/format/classes/output/local/overview/overviewtable.php +++ b/public/course/format/classes/output/local/overview/overviewtable.php @@ -20,6 +20,7 @@ use core\output\named_templatable; use core\output\renderable; use core\output\renderer_base; use core\plugin_manager; +use core_courseformat\activityoverviewbase; use core_courseformat\local\overview\overviewitem; use core_courseformat\local\overview\overviewfactory; use cm_info; @@ -55,7 +56,7 @@ class overviewtable implements renderable, named_templatable { #[\Override] public function export_for_template(renderer_base $output): stdClass { - $activities = $this->load_all_overviews_from_each_activity($output); + $activities = $this->load_all_overviews_from_each_activity(); $headers = $this->export_headers(); $result = (object) [ 'caption' => $this->get_table_caption(), @@ -120,6 +121,7 @@ class overviewtable implements renderable, named_templatable { } $result[] = [ 'cmid' => $activity['cmid'], + 'haserror' => $activity['haserror'], 'overviews' => $items, ]; } @@ -129,18 +131,19 @@ class overviewtable implements renderable, named_templatable { /** * Loads all overviews from activities for the given course and module name. * - * @param renderer_base $output * @return array An array of overviews. */ - private function load_all_overviews_from_each_activity(renderer_base $output): array { + private function load_all_overviews_from_each_activity(): array { $result = []; foreach ($this->get_related_course_modules() as $cm) { if (!$this->is_cm_displayable($cm)) { continue; } + $overview = overviewfactory::create($cm); $result[] = [ 'cmid' => $cm->id, - 'overviews' => $this->load_overview_items_from_activity($output, $cm), + 'haserror' => $overview->has_error(), + 'overviews' => $this->load_overview_items_from_activity($overview), ]; } return $result; @@ -218,21 +221,21 @@ class overviewtable implements renderable, named_templatable { /** * Loads overview items from a given activity. * - * @param renderer_base $output - * @param cm_info $cm + * @param activityoverviewbase $overview * @return array An associative array containing the overview items for the activity. */ - private function load_overview_items_from_activity(renderer_base $output, cm_info $cm): array { - global $PAGE; - $overview = overviewfactory::create($cm); + private function load_overview_items_from_activity(activityoverviewbase $overview): array { + if ($overview->has_error()) { + return ['name' => $overview->get_name_overview()]; + } $row = [ - 'name' => $overview->get_name_overview($output), - 'duedate' => $overview->get_due_date_overview($output), - 'completion' => $overview->get_completion_overview($output), + 'name' => $overview->get_name_overview(), + 'duedate' => $overview->get_due_date_overview(), + 'completion' => $overview->get_completion_overview(), ]; - $row = array_merge($row, $overview->get_extra_overview_items($output)); + $row = array_merge($row, $overview->get_extra_overview_items()); $gradeitems = $overview->get_grades_overviews(); if (!empty($gradeitems)) { @@ -242,7 +245,7 @@ class overviewtable implements renderable, named_templatable { } // Actions are always the last column, if any. - $row['actions'] = $overview->get_actions_overview($output); + $row['actions'] = $overview->get_actions_overview(); $row = array_filter($row, function ($item) { return $item !== null; diff --git a/public/course/format/templates/local/overview/activityname.mustache b/public/course/format/templates/local/overview/activityname.mustache index 746587e0624..fe2743f9e15 100644 --- a/public/course/format/templates/local/overview/activityname.mustache +++ b/public/course/format/templates/local/overview/activityname.mustache @@ -25,7 +25,8 @@ "activityurl": "http://moodle.com", "sectiontitle": "Section title", "visible": true, - "stealth": false + "stealth": false, + "nogroupserror": false } }}
@@ -51,4 +52,9 @@ {{/stealth}}
+ {{#nogroupserror}} +
+ {{#pix}}e/cancel_solid_circle, moodle, {{#str}}overview_nogroups_title,course{{/str}}{{/pix}}{{#str}}overview_nogroups_error,course{{/str}} +
+ {{/nogroupserror}} diff --git a/public/course/format/templates/local/overview/overviewtable.mustache b/public/course/format/templates/local/overview/overviewtable.mustache index e6536672955..4e966e27338 100644 --- a/public/course/format/templates/local/overview/overviewtable.mustache +++ b/public/course/format/templates/local/overview/overviewtable.mustache @@ -37,6 +37,7 @@ "activities": [ { "cmid": 1, + "haserror": true, "overviews": [ { "overview": "name", @@ -56,6 +57,7 @@ }, { "cmid": 2, + "haserror": false, "overviews": [ { "overview": "name", @@ -95,7 +97,7 @@ {{#activities}} - + {{#overviews}} assertEmpty($result); } - /** - * Test get_grades_overviews when the grade item is hidden. - * - * @covers ::get_grades_overviews - */ /** * Test get_grades_overviews method. * @@ -331,4 +328,280 @@ final class activityoverviewbase_test extends \advanced_testcase { $this->assertEquals('-', $result[0]->get_value()); $this->assertEquals('-', $result[0]->get_content()); } + + /** + * Test needs_filtering_by_groups method. + * + * @covers ::needs_filtering_by_groups + * @dataProvider provider_needs_filtering_by_groups + * + * @param string $role of the user to test + * @param int $groupmode of the activity to test + * @param bool $expected result. + * @return void + */ + public function test_needs_filtering_by_groups(string $role, int $groupmode, bool $expected): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $activity = $this->getDataGenerator()->create_module( + 'assign', + ['course' => $course->id, 'groupmode' => $groupmode] + ); + $user = $this->getDataGenerator()->create_and_enrol($course, $role); + $this->setUser($user); + $modinfo = get_fast_modinfo($course); + $cm = $modinfo->get_cm($activity->cmid); + $overview = new \core_courseformat\fake_activityoverview($cm); + $this->assertEquals($expected, $overview->needs_filtering_by_groups()); + } + + /** + * Data provider for test_needs_filtering_by_groups. + * + * @return array the testing scenarios + */ + public static function provider_needs_filtering_by_groups(): array { + return [ + 'Editing teacher with no groups' => [ + 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, + 'expected' => false, + ], + 'Editing teacher with visible groups' => [ + 'role' => 'editingteacher', + 'groupmode' => VISIBLEGROUPS, + 'expected' => false, + ], + 'Editing teacher with separate groups' => [ + 'role' => 'editingteacher', + 'groupmode' => SEPARATEGROUPS, + 'expected' => false, + ], + 'Non-editing teacher with no groups' => [ + 'role' => 'teacher', + 'groupmode' => NOGROUPS, + 'expected' => false, + ], + 'Non-editing teacher with visible groups' => [ + 'role' => 'teacher', + 'groupmode' => VISIBLEGROUPS, + 'expected' => false, + ], + 'Non-editing teacher with separate groups' => [ + 'role' => 'teacher', + 'groupmode' => SEPARATEGROUPS, + 'expected' => true, + ], + 'Student with no groups' => [ + 'role' => 'student', + 'groupmode' => NOGROUPS, + 'expected' => false, + ], + 'Student with visible groups' => [ + 'role' => 'student', + 'groupmode' => VISIBLEGROUPS, + 'expected' => false, + ], + 'Student with separate groups' => [ + 'role' => 'student', + 'groupmode' => SEPARATEGROUPS, + 'expected' => true, + ], + ]; + } + + /** + * Test needs_filtering_by_groups method. + * + * @covers ::get_groups_for_filtering + * @dataProvider provider_get_groups_for_filtering + * + * @param string $role of the user to test + * @param int $groupmode of the activity to test + * @param array $expected result + * @return void + */ + public function test_get_groups_for_filtering(string $role, int $groupmode, array $expected): void { + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $activity = $generator->create_module( + 'assign', + ['course' => $course->id, 'groupmode' => $groupmode] + ); + $user = $generator->create_and_enrol($course, $role); + $g1 = $generator->create_group(['courseid' => $course->id, 'name' => 'g1']); + $g2 = $generator->create_group(['courseid' => $course->id, 'name' => 'g2']); + $g3 = $generator->create_group(['courseid' => $course->id, 'name' => 'g3']); + + // We add user to g1 and g2 only. + groups_add_member($g1, $user); + groups_add_member($g2, $user); + + $this->setUser($user); + $modinfo = get_fast_modinfo($course); + $cm = $modinfo->get_cm($activity->cmid); + $overview = new \core_courseformat\fake_activityoverview($cm); + $result = $overview->get_groups_for_filtering(); + if (!$expected) { + $this->assertEquals($expected, $result); + } else { + foreach ($result as $group) { + $this->assertContains($group->name, $expected); + } + } + } + + /** + * Data provider for test_get_groups_for_filtering. + * + * @return array the testing scenarios + */ + public static function provider_get_groups_for_filtering(): array { + return [ + 'Editing teacher with no groups' => [ + 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, + 'expected' => [], + ], + 'Editing teacher with visible groups' => [ + 'role' => 'editingteacher', + 'groupmode' => VISIBLEGROUPS, + 'expected' => [], + ], + 'Editing teacher with separate groups' => [ + 'role' => 'editingteacher', + 'groupmode' => SEPARATEGROUPS, + 'expected' => [], + ], + 'Non-editing teacher with no groups' => [ + 'role' => 'teacher', + 'groupmode' => NOGROUPS, + 'expected' => [], + ], + 'Non-editing teacher with visible groups' => [ + 'role' => 'teacher', + 'groupmode' => VISIBLEGROUPS, + 'expected' => ['g1', 'g2', 'g3'], + ], + 'Non-editing teacher with separate groups' => [ + 'role' => 'teacher', + 'groupmode' => SEPARATEGROUPS, + 'expected' => ['g1', 'g2'], + ], + 'Student with no groups' => [ + 'role' => 'student', + 'groupmode' => NOGROUPS, + 'expected' => [], + ], + 'Student with visible groups' => [ + 'role' => 'student', + 'groupmode' => VISIBLEGROUPS, + 'expected' => ['g1', 'g2', 'g3'], + ], + 'Student with separate groups' => [ + 'role' => 'student', + 'groupmode' => SEPARATEGROUPS, + 'expected' => ['g1', 'g2'], + ], + ]; + } + + /** + * Test has_error method. + * + * @covers ::has_error + * @dataProvider provider_has_error + * + * @param string $role of the user to test + * @param int $groupmode of the activity to test + * @param bool $expected result + * @return void + */ + public function test_has_error(string $role, int $groupmode, bool $expected): void { + $this->resetAfterTest(); + + $generator = $this->getDataGenerator(); + $course = $generator->create_course(); + $activity = $generator->create_module( + 'assign', + ['course' => $course->id, 'groupmode' => $groupmode] + ); + $user = $generator->create_and_enrol($course, $role); + $g1 = $generator->create_group(['courseid' => $course->id, 'name' => 'g1']); + $g2 = $generator->create_group(['courseid' => $course->id, 'name' => 'g2']); + + $this->setUser($user); + + $modinfo = get_fast_modinfo($course); + $cm = $modinfo->get_cm($activity->cmid); + $overview = new \core_courseformat\fake_activityoverview($cm); + $this->assertEquals($expected, $overview->has_error()); + + // We add user to g1. + groups_add_member($g1, $user); + + $modinfo = get_fast_modinfo($course); + $cm = $modinfo->get_cm($activity->cmid); + $overview = new \core_courseformat\fake_activityoverview($cm); + + $this->assertfalse($overview->has_error()); + } + + /** + * Data provider for test_has_error. + * + * @return array the testing scenarios + */ + public static function provider_has_error(): array { + return [ + 'Editing teacher with no groups' => [ + 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, + 'expected' => false, + ], + 'Editing teacher with visible groups' => [ + 'role' => 'editingteacher', + 'groupmode' => VISIBLEGROUPS, + 'expected' => false, + ], + 'Editing teacher with separate groups' => [ + 'role' => 'editingteacher', + 'groupmode' => SEPARATEGROUPS, + 'expected' => false, + ], + 'Non-editing teacher with no groups' => [ + 'role' => 'teacher', + 'groupmode' => NOGROUPS, + 'expected' => false, + ], + 'Non-editing teacher with visible groups' => [ + 'role' => 'teacher', + 'groupmode' => VISIBLEGROUPS, + 'expected' => false, + ], + 'Non-editing teacher with separate groups' => [ + 'role' => 'teacher', + 'groupmode' => SEPARATEGROUPS, + 'expected' => true, + ], + 'Student with no groups' => [ + 'role' => 'student', + 'groupmode' => NOGROUPS, + 'expected' => false, + ], + 'Student with visible groups' => [ + 'role' => 'student', + 'groupmode' => VISIBLEGROUPS, + 'expected' => false, + ], + 'Student with separate groups' => [ + 'role' => 'student', + 'groupmode' => SEPARATEGROUPS, + 'expected' => true, + ], + ]; + } } diff --git a/public/course/tests/behat/course_overview.feature b/public/course/tests/behat/course_overview.feature index 02b06649c76..6b5d6429b69 100644 --- a/public/course/tests/behat/course_overview.feature +++ b/public/course/tests/behat/course_overview.feature @@ -313,3 +313,30 @@ Feature: Users can access the course activities overview page | name | Announcements$$(a+b)=2$$$$(a+b)=2$$ | When I am on the "Course 1" "course > activities > assign" page logged in as "teacher1" Then I should not see "span" in the "assign_overview_collapsible" "region" + + @javascript + Scenario: Users in no group that cannot view all groups see an error on 'Separate groups' activities + Given the following "users" exist: + | username | firstname | lastname | email | + | nonediting | Non-editing | Teacher | nonediting1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | nonediting | C1 | teacher | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + And the following "activity" exists: + | activity | assign | + | course | C1 | + | section | 1 | + | idnumber | separate | + | name | Separate groups | + | groupmode | 1 | + When I am on the "Course 1" "course > activities > assign" page logged in as "teacher1" + Then I should not see "You are not a member of any group" in the "assign_overview_collapsible" "region" + And I log out + And I am on the "Course 1" "course > activities > assign" page logged in as "nonediting" + And I should see "You are not a member of any group" in the "assign_overview_collapsible" "region" + And I log out + And I am on the "Course 1" "course > activities > assign" page logged in as "student1" + And I should see "You are not a member of any group" in the "assign_overview_collapsible" "region" diff --git a/public/lang/en/course.php b/public/lang/en/course.php index c3a5ec862f6..b02f6020c42 100644 --- a/public/lang/en/course.php +++ b/public/lang/en/course.php @@ -124,6 +124,8 @@ $string['overview_info'] = 'An overview of all activities in the course, with da $string['overview_missing_notice'] = 'Go to the {$a}.'; $string['overview_missing_title'] = '{$a} information not available.'; $string['overview_modname'] = '{$a} overview'; +$string['overview_nogroups_error'] = 'You are not a member of any group'; +$string['overview_nogroups_title'] = 'Information not available because you are not a member of any group'; $string['overview_page_title'] = 'Course activities: {$a}'; $string['overview_table_caption'] = 'Table listing all {$a} activities'; $string['participants:perpage'] = 'Number of participants per page';