MDL-84464 report_log: Hide report for teachers without groups

* Teachers without groups in SEPARATEGROUP mode should not see the report log page nor access it
This commit is contained in:
Laurent David
2025-08-06 10:23:12 +07:00
committed by Huong Nguyen
parent 7c3b3af98c
commit edd9c78756
8 changed files with 365 additions and 15 deletions
+40
View File
@@ -103,4 +103,44 @@ class report_helper {
}
$USER->course_last_report[$id] = $url;
}
/**
* Check if the user is in a valid group for the course (i.e. if the user is in a group in SEPARATEGROUPS mode)
*
* @param context $context context for the course or module: if context is a course context, the course group mode is used,
* if it is a module context, the module effective group mode is used (combined with the current user).
* @param int|null $userid user id to check, if null the current user is used
* @return bool true if the user is in a valid group (i.e. belongs to a group in SEPARATEGROUPS MODE), false otherwise
*/
public static function has_valid_group(\context $context, ?int $userid = null): bool {
global $USER;
$userid = $userid ?? $USER->id;
if ($context instanceof \context_course) {
$courseid = $context->instanceid;
$course = get_course($courseid);
$groupmode = $course->groupmode;
} else if ($context instanceof \context_module) {
$courseid = $context->get_course_context()->instanceid;
$modinfo = get_fast_modinfo($courseid);
$cm = $modinfo->get_cm($context->instanceid);
$groupmode = $cm->effectivegroupmode;
} else {
return true; // No groups in system context.
}
if ($groupmode != SEPARATEGROUPS) {
return true; // No groups or visible all groups.
}
if (!has_capability('moodle/site:accessallgroups', $context, $userid)) {
$usergroups = groups_get_all_groups($courseid, $userid);
if (empty($usergroups)) {
return false;
}
}
return true;
}
}
+144 -1
View File
@@ -26,7 +26,6 @@
namespace core;
use moodle_url;
use core\report_helper;
/**
* Tests the functions for report_helper class.
@@ -113,4 +112,148 @@ final class report_helper_test extends \advanced_testcase {
$this->assertStringContainsString($loglive, $output);
$this->assertStringContainsString($participation, $output);
}
/**
* Tests {@see report_helper::has_valid_group()}.
*
* @param int $groupmode Group mode for the course
* @param string $username Username of the user to check
* @param array $expected Expected result of the check, with 3 boolean values depending on the context:
* - Course context
* - Module context
* - System context
*
* @covers \core\report_helper::has_valid_group
* @dataProvider has_valid_group_provider
*/
public function test_has_valid_group(int $groupmode, string $username, array $expected): void {
$this->resetAfterTest();
// Create some test course, groups, and users.
$generator = self::getDataGenerator();
$course = $generator->create_course(['groupmode' => $groupmode, 'groupmodeforce' => 1]);
$assign = $generator->create_module('assign', ['course' => $course->id]);
$g1 = $generator->create_group(['courseid' => $course->id]);
$this->userids = [];
$data = [
's1' => ['role' => 'student', 'group' => $g1->id],
's2' => ['role' => 'student', 'group' => null],
't1' => ['role' => 'teacher', 'group' => $g1->id],
't2' => ['role' => 'teacher', 'group' => null],
'et1' => ['role' => 'editingteacher', 'group' => null],
];
foreach ($data as $key => $value) {
['group' => $groupid, 'role' => $role] = $value;
$this->userids[$key] = $generator->create_user(['username' => $key]);
$generator->enrol_user($this->userids[$key]->id, $course->id, $role);
if ($groupid) {
groups_add_member($groupid, $this->userids[$key]->id);
}
}
$coursecontext = \context_course::instance($course->id);
[$course, $cm] = get_course_and_cm_from_instance($assign->id, 'assign');
$modulecontext = \context_module::instance($cm->id);
[$hasvalidgroupcourse, $hasvalidgroupmodule, $hasvalidgroupsystem] = $expected;
$this->assertEquals(
$hasvalidgroupcourse,
report_helper::has_valid_group($coursecontext, $this->userids[$username]->id),
"Failed for user $username in course context"
);
$this->assertEquals(
$hasvalidgroupmodule,
report_helper::has_valid_group($modulecontext, $this->userids[$username]->id),
'Failed for user ' . $username . ' in module context'
);
$this->assertEquals(
$hasvalidgroupsystem,
report_helper::has_valid_group(\context_system::instance(), $this->userids[$username]->id),
'Failed for user ' . $username . ' in system context'
);
}
/**
* Data provider for test_has_valid_group.
*
* @return array
*/
public static function has_valid_group_provider(): array {
return [
'student 1 - g1 - separate group' => [
'groupmode' => SEPARATEGROUPS,
'username' => 's1',
'expected' => [true, true, true],
],
'student 2 - no group - separate group' => [
'groupmode' => SEPARATEGROUPS,
'username' => 's2',
'expected' => [false, false, true],
],
'teacher 1 - g1 - separate group' => [
'groupmode' => SEPARATEGROUPS,
'username' => 't1',
'expected' => [true, true, true],
],
'teacher 2 - no group - separate group' => [
'groupmode' => SEPARATEGROUPS,
'username' => 't2',
'expected' => [false, false, true],
],
'editing teacher - no group - separate group' => [
'groupmode' => SEPARATEGROUPS,
'username' => 'et1',
'expected' => [true, true, true],
],
'student 1 - g1 - no group' => [
'groupmode' => NOGROUPS,
'username' => 's1',
'expected' => [true, true, true],
],
'student 2 - no group - no group' => [
'groupmode' => NOGROUPS,
'username' => 's2',
'expected' => [true, true, true],
],
'teacher 1 - g1 - no group' => [
'groupmode' => NOGROUPS,
'username' => 't1',
'expected' => [true, true, true],
],
'teacher 2 - no group - no group' => [
'groupmode' => NOGROUPS,
'username' => 't2',
'expected' => [true, true, true],
],
'editing teacher - no group - no group' => [
'groupmode' => NOGROUPS,
'username' => 'et1',
'expected' => [true, true, true],
],
'student 1 - g1 - visible group' => [
'groupmode' => VISIBLEGROUPS,
'username' => 's1',
'expected' => [true, true, true],
],
'student 2 - no group - visible group' => [
'groupmode' => VISIBLEGROUPS,
'username' => 's2',
'expected' => [true, true, true],
],
'teacher 1 - g1 - visible group' => [
'groupmode' => VISIBLEGROUPS,
'username' => 't1',
'expected' => [true, true, true],
],
'teacher 2 - no group - visible group' => [
'groupmode' => VISIBLEGROUPS,
'username' => 't2',
'expected' => [true, true, true],
],
'editing teacher - visible group - no group' => [
'groupmode' => VISIBLEGROUPS,
'username' => 'et1',
'expected' => [true, true, true],
],
];
}
}
+5
View File
@@ -1,6 +1,11 @@
This files describes API changes in core libraries and APIs,
information provided here is intended especially for developers.
=== 4.1.20 ===
* Add a new method has_valid_group in \core\report_helper that will return true or false depending if the user has a valid group. This is mainly
false in case the user is not in any group in SEPARATEGROUPS. Used in report_log and report_loglive.
=== 4.1.18 ===
* A new method, `core_text::trim_ctrl_chars()`, has been introduced to clean control characters from text.
This ensures cleaner input handling and prevents issues caused by invisible or non-printable characters
+9 -2
View File
@@ -150,11 +150,18 @@ if ($course->id == $SITE->id) {
$PAGE->set_heading($course->fullname);
}
$output = $PAGE->get_renderer('report_log');
if (!report_helper::has_valid_group($context)) {
echo $output->header();
echo $output->notification(get_string('notingroup'));
echo $output->footer();
exit();
}
$reportlog = new report_log_renderable($logreader, $course, $user, $modid, $modaction, $group, $edulevel, $showcourses, $showusers,
$chooselog, true, $url, $date, $logformat, $page, $perpage, 'timecreated DESC', $origin);
$readers = $reportlog->get_readers();
$output = $PAGE->get_renderer('report_log');
$readers = $reportlog->get_readers();
if (empty($readers)) {
echo $output->header();
echo $output->heading(get_string('nologreaderenabled', 'report_log'));
+10 -2
View File
@@ -24,6 +24,8 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
use core\report_helper;
defined('MOODLE_INTERNAL') || die;
/**
@@ -34,7 +36,10 @@ defined('MOODLE_INTERNAL') || die;
* @param stdClass $context The context of the course
*/
function report_log_extend_navigation_course($navigation, $course, $context) {
if (has_capability('report/log:view', $context)) {
if (
has_capability('report/log:view', $context)
&& report_helper::has_valid_group($context)
) {
$url = new moodle_url('/report/log/index.php', array('id'=>$course->id));
$navigation->add(get_string('pluginname', 'report_log'), $url, navigation_node::TYPE_SETTING, null, null, new pix_icon('i/report', ''));
}
@@ -124,7 +129,10 @@ function report_log_can_access_user_report($user, $course) {
* @param stdClass $cm
*/
function report_log_extend_navigation_module($navigation, $cm) {
if (has_capability('report/log:view', context_course::instance($cm->course))) {
if (
has_capability('report/log:view', context_course::instance($cm->course))
&& report_helper::has_valid_group(context_module::instance($cm->id))
) {
$url = new moodle_url('/report/log/index.php', array('chooselog'=>'1','id'=>$cm->course,'modid'=>$cm->id));
$navigation->add(get_string('logs'), $url, navigation_node::TYPE_SETTING, null, 'logreport', new pix_icon('i/report', ''))
->set_show_in_secondary_navigation(false);
@@ -0,0 +1,51 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
require_once(__DIR__ . '/../../../../lib/behat/behat_base.php');
/**
* Step definition for report_log behat tests.
*
* @package report_log
* @category test
* @copyright 2025 Laurent David <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class behat_report_log extends behat_base {
/**
* Convert page names to URLs for steps like 'When I am on the "[identifier]" "[page type]" page'.
*
* Recognised page names are:
* | pagetype | name meaning | description |
* | logs | Course name | The course report logs page |
*
* @param string $page identifies which type of page this is, e.g. 'Logs'.
* @param string $identifier identifies the particular page, e.g. 'C1'.
* @return moodle_url the corresponding URL.
* @throws Exception with a meaningful error message if the specified page cannot be found.
*/
protected function resolve_page_instance_url(string $page, string $identifier): moodle_url {
switch (strtolower($page)) {
case 'logs':
$courseid = $this->get_course_id($identifier);
return new moodle_url('/report/log/index.php', [
'id' => $courseid,
]);
default:
throw new Exception("Unrecognised page type '{$page}'");
}
}
}
@@ -0,0 +1,95 @@
@report @report_log
Feature: In a course with group mode, I can view the group report log page or not
depending on the group I am in.
Background:
Given the following "courses" exist:
| fullname | shortname | category | groupmode |
| Course separate group | C1 | 0 | 1 |
| Course visible group | C2 | 0 | 2 |
| Course no group | C3 | 0 | 0 |
And the following "users" exist:
| username | firstname | lastname | email |
| student1 | Student | 1 | student1@example.com |
| student2 | Student | 2 | student2@example.com |
| teacher1 | Teacher | 1 | teacher1@example.com |
| teacher2 | Teacher | 2 | teacher2@example.com |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | teacher |
| teacher2 | C1 | teacher |
| student1 | C1 | student |
| student2 | C1 | student |
| teacher1 | C2 | teacher |
| teacher2 | C2 | teacher |
| student1 | C2 | student |
| student2 | C2 | student |
| teacher1 | C3 | teacher |
| teacher2 | C3 | teacher |
| student1 | C3 | student |
| student2 | C3 | student |
And the following "groups" exist:
| course | name | idnumber |
| C1 | Group C1.1 | group11 |
| C2 | Group C2.1 | group21 |
| C3 | Group C3.1 | group31 |
And the following "group members" exist:
| group | user |
| group11 | student1 |
| group11 | teacher1 |
| group21 | student1 |
| group21 | teacher1 |
| group31 | student1 |
| group31 | teacher1 |
And the following "activities" exist:
| activity | name | intro | course | idnumber |
| page | Page11 | Page11 | C1 | page1 |
| page | Page21 | Page21 | C2 | page1 |
| page | Page31 | Page31 | C3 | page1 |
# Generate logs for the pages.
And I am on the "Page11" "page activity" page logged in as student1
And I am on "Course separate group" course homepage
And I log out
And I am on the "Page11" "page activity" page logged in as student2
And I am on "Course separate group" course homepage
And I log out
And I am on the "Page21" "page activity" page logged in as student1
And I am on "Course visible group" course homepage
And I log out
And I am on the "Page21" "page activity" page logged in as student2
And I am on "Course visible group" course homepage
And I log out
And I am on the "Page31" "page activity" page logged in as student1
And I am on "Course no group" course homepage
And I log out
And I am on the "Page31" "page activity" page logged in as student2
And I am on "Course no group" course homepage
And I log out
Scenario Outline: As a user in a course, I can view a link to the report logs if I am in the right group.
Given I log in as "<user>"
And I am on "<course>" course homepage
When I navigate to "Reports" in current page administration
And "Logs" "link" <shouldexist>
And I log out
Examples:
| course | user | shouldexist |
| Course separate group | teacher1 | should exist |
| Course separate group | teacher2 | should not exist |
| Course visible group | teacher1 | should exist |
| Course visible group | teacher2 | should exist |
| Course no group | teacher1 | should exist |
| Course no group | teacher2 | should exist |
Scenario Outline: As a non editing teacher not in a group, I can not view the report logs.
Given I log in as "<user>"
When I am on the "<course>" "report_log > Logs" page
Then I <shouldsee> "you need to be part of a group to see this page."
Examples:
| course | user | shouldsee |
| Course separate group | teacher1 | should not see |
| Course separate group | teacher2 | should see |
| Course visible group | teacher1 | should not see |
| Course visible group | teacher2 | should not see |
| Course no group | teacher1 | should not see |
| Course no group | teacher2 | should not see |
+11 -10
View File
@@ -29,6 +29,7 @@ use core\report_helper;
require('../../config.php');
require_once($CFG->libdir.'/adminlib.php');
require_once($CFG->dirroot.'/course/lib.php');
global $USER, $SITE, $PAGE;
$id = optional_param('id', 0, PARAM_INT);
$page = optional_param('page', 0, PARAM_INT);
@@ -60,23 +61,23 @@ $url = new moodle_url("/report/loglive/index.php", $params);
$PAGE->set_url($url);
$PAGE->set_pagelayout('report');
$PAGE->set_context($context);
$strlivelogs = get_string('livelogs', 'report_loglive');
$PAGE->set_title("$coursename: $strlivelogs");
$output = $PAGE->get_renderer('report_loglive');
echo $output->header();
if (!report_helper::has_valid_group($context)) {
echo $output->notification(get_string('notingroup'));
echo $output->footer();
exit();
}
$renderable = new report_loglive_renderable($logreader, $id, $url, 0, $page);
$refresh = $renderable->get_refresh_rate();
$logreader = $renderable->selectedlogreader;
$strlivelogs = get_string('livelogs', 'report_loglive');
$strupdatesevery = get_string('updatesevery', 'moodle', $refresh);
$PAGE->set_url($url);
$PAGE->set_context($context);
$PAGE->set_title("$coursename: $strlivelogs");
$PAGE->set_heading($coursename);
$output = $PAGE->get_renderer('report_loglive');
echo $output->header();
// Print selector dropdown.
$pluginname = get_string('pluginname', 'report_loglive');
report_helper::print_report_selector($pluginname);