Skip to content

Commit

Permalink
Merge branch 'MDL-80619' of https://github.com/paulholden/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@sarjona
sarjona committed Jan 22, 2024
2 parents 0d8535f + 27cb750 commit 9e4366a
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 46 deletions.
1 change: 1 addition & 0 deletions lang/en/role.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@
$string['reportbuilder:editall'] = 'Edit all custom reports';
$string['reportbuilder:scheduleviewas'] = 'Schedule reports to be viewed as other users';
$string['reportbuilder:view'] = 'View custom reports';
$string['reportbuilder:viewall'] = 'View all custom reports';
$string['resetrole'] = 'Reset';
$string['resettingrole'] = 'Resetting role \'{$a}\'';
$string['restore:configure'] = 'Configure restore options';
Expand Down
7 changes: 7 additions & 0 deletions lib/db/access.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2686,6 +2686,13 @@
],
],

// Allow users to view all custom reports.
'moodle/reportbuilder:viewall' => [
'captype' => 'read',
'contextlevel' => CONTEXT_SYSTEM,
'archetypes' => [],
],

// Allow users to create/edit their own custom reports.
'moodle/reportbuilder:edit' => [
'captype' => 'write',
Expand Down
46 changes: 22 additions & 24 deletions reportbuilder/classes/local/helpers/audience.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ public static function user_reports_list(?int $userid = null): array {
/**
* Returns SQL to limit the list of reports to those that the given user has access to
*
* - A user with 'editall' capability will have access to all reports
* - A user with 'viewall/editall' capability will have access to all reports
* - A user with 'edit' capability will have access to:
* - Those reports this user has created
* - Those reports this user is in audience of
* - A user with 'view' capability will have access to:
* - Otherwise:
* - Those reports this user is in audience of
*
* @param string $reporttablealias
Expand All @@ -182,31 +182,29 @@ public static function user_reports_list_access_sql(
$context = context_system::instance();
}

// If user can't view all reports, limit the returned list to those reports they can see.
if (!has_capability('moodle/reportbuilder:editall', $context, $userid)) {

// Select all reports accessible to the user based on audience.
[$reportselect, $params] = $DB->get_in_or_equal(
self::user_reports_list($userid),
SQL_PARAMS_NAMED,
database::generate_param_name('_'),
true,
null,
);

$where = "{$reporttablealias}.id {$reportselect}";

// User can also see any reports that they can edit.
if (has_capability('moodle/reportbuilder:edit', $context, $userid)) {
$paramuserid = database::generate_param_name();
$where = "({$reporttablealias}.usercreated = :{$paramuserid} OR {$where})";
$params[$paramuserid] = $userid ?? $USER->id;
}
if (has_any_capability(['moodle/reportbuilder:editall', 'moodle/reportbuilder:viewall'], $context, $userid)) {
return ['1=1', []];
}

// Limit the returned list to those reports the user can see, by selecting based on report audience.
[$reportselect, $params] = $DB->get_in_or_equal(
self::user_reports_list($userid),
SQL_PARAMS_NAMED,
database::generate_param_name('_'),
true,
null,
);

return [$where, $params];
$where = "{$reporttablealias}.id {$reportselect}";

// User can also see any reports that they can edit.
if (has_capability('moodle/reportbuilder:edit', $context, $userid)) {
$paramuserid = database::generate_param_name();
$where = "({$reporttablealias}.usercreated = :{$paramuserid} OR {$where})";
$params[$paramuserid] = $userid ?? $USER->id;
}

return ['1=1', []];
return [$where, $params];
}

/**
Expand Down
7 changes: 6 additions & 1 deletion reportbuilder/classes/permission.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ public static function can_view_reports_list(?int $userid = null, ?context $cont
}

return !empty($CFG->enablecustomreports) && has_any_capability([
'moodle/reportbuilder:editall',
'moodle/reportbuilder:edit',
'moodle/reportbuilder:editall',
'moodle/reportbuilder:view',
'moodle/reportbuilder:viewall',
], $context, $userid);
}

Expand Down Expand Up @@ -92,6 +93,10 @@ public static function can_view_report(report $report, ?int $userid = null): boo
return false;
}

if (has_capability('moodle/reportbuilder:viewall', $report->get_context(), $userid)) {
return true;
}

if (self::can_edit_report($report, $userid)) {
return true;
}
Expand Down
53 changes: 43 additions & 10 deletions reportbuilder/tests/local/helpers/audience_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,16 +267,6 @@ public function test_user_reports_list_access_sql(): void {
$this->setUser($usertwo);
$usertworeport = $generator->create_report(['name' => 'User two report', 'source' => users::class]);

// Admin user sees all reports.
$this->setAdminUser();
[$where, $params] = audience::user_reports_list_access_sql('r');
$reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params);
$this->assertEqualsCanonicalizing([
$useradminreport->get('id'),
$useronereport->get('id'),
$usertworeport->get('id'),
], $reports);

// User one sees only the report they created.
[$where, $params] = audience::user_reports_list_access_sql('r', (int) $userone->id);
$reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params);
Expand All @@ -298,6 +288,49 @@ public function test_user_reports_list_access_sql(): void {
$this->assertEmpty($reports);
}

/**
* Data provider for {@see test_user_reports_list_access_sql_with_capability}
*
* @return array[]
*/
public static function user_reports_list_access_sql_with_capability_provider(): array {
return [
['moodle/reportbuilder:editall'],
['moodle/reportbuilder:viewall'],
];
}

/**
* Test retrieving list of reports that user can access observes capability to view all reports
*
* @param string $capability
*
* @dataProvider user_reports_list_access_sql_with_capability_provider
*/
public function test_user_reports_list_access_sql_with_capability(string $capability): void {
global $DB;

$this->resetAfterTest();

// Admin creates a report, no audience.
$this->setAdminUser();

/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'Admin report', 'source' => users::class]);

// Switch to new user, assign capability.
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

$userrole = $DB->get_field('role', 'id', ['shortname' => 'user']);
assign_capability($capability, CAP_ALLOW, $userrole, context_system::instance());

[$where, $params] = audience::user_reports_list_access_sql('r');
$reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params);
$this->assertEquals([$report->get('id')], $reports);
}

/**
* Test getting list of audiences in use within schedules for a report
*/
Expand Down
100 changes: 90 additions & 10 deletions reportbuilder/tests/permission_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,17 @@ public function test_require_can_view_reports_list(): void {

$this->resetAfterTest();

// User with permission.
$this->setAdminUser();
// User with default permission.
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

try {
permission::require_can_view_reports_list();
} catch (Throwable $exception) {
$this->fail($exception->getMessage());
}

// User without permission.
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

$userrole = $DB->get_field('role', 'id', ['shortname' => 'user']);
unassign_capability('moodle/reportbuilder:view', $userrole, context_system::instance());

Expand All @@ -63,6 +62,47 @@ public function test_require_can_view_reports_list(): void {
permission::require_can_view_reports_list();
}

/**
* Data provider for {@see test_require_can_view_reports_list_with_capability}
*
* @return array[]
*/
public static function require_can_view_reports_list_with_capability_provider(): array {
return [
['moodle/reportbuilder:edit'],
['moodle/reportbuilder:editall'],
['moodle/reportbuilder:viewall'],
];
}

/**
* Test that viewing reports list observes capability to do so
*
* @param string $capability
*
* @dataProvider require_can_view_reports_list_with_capability_provider
*/
public function test_require_can_view_reports_list_with_capability(string $capability): void {
global $DB;

$this->resetAfterTest();

$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

$userrole = $DB->get_field('role', 'id', ['shortname' => 'user']);

// Remove default capability, allow additional.
unassign_capability('moodle/reportbuilder:view', $userrole, context_system::instance());
assign_capability($capability, CAP_ALLOW, $userrole, context_system::instance());

try {
permission::require_can_view_reports_list();
} catch (Throwable $exception) {
$this->fail($exception->getMessage());
}
}

/**
* Test whether user can view reports list when custom reports are disabled
*/
Expand All @@ -81,8 +121,6 @@ public function test_require_can_view_reports_list_disabled(): void {
* Test whether user can view specific report
*/
public function test_require_can_view_report(): void {
global $DB;

$this->resetAfterTest();

/** @var core_reportbuilder_generator $generator */
Expand All @@ -101,14 +139,56 @@ public function test_require_can_view_report(): void {
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

$userrole = $DB->get_field('role', 'id', ['shortname' => 'user']);
unassign_capability('moodle/reportbuilder:view', $userrole, context_system::instance());

$this->expectException(report_access_exception::class);
$this->expectExceptionMessage('You cannot view this report');
permission::require_can_view_report($report);
}

/**
* Data provider for {@see test_require_can_view_report_with_capability}
*
* @return array[]
*/
public static function require_can_view_report_with_capability_provider(): array {
return [
['moodle/reportbuilder:editall'],
['moodle/reportbuilder:viewall'],
];
}

/**
* Test whether user can view specific report when they have capability to view all reports
*
* @param string $capability
*
* @dataProvider require_can_view_report_with_capability_provider
*/
public function test_require_can_view_report_with_capability(string $capability): void {
global $DB;

$this->resetAfterTest();

// Admin creates a report, no audience.
$this->setAdminUser();

/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'Admin report', 'source' => users::class]);

// Switch to new user, assign capability.
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

$userrole = $DB->get_field('role', 'id', ['shortname' => 'user']);
assign_capability($capability, CAP_ALLOW, $userrole, context_system::instance());

try {
permission::require_can_view_report($report);
} catch (Throwable $exception) {
$this->fail($exception->getMessage());
}
}

/**
* Test whether user can view specific report when it belongs to an audience
*/
Expand Down
2 changes: 1 addition & 1 deletion version.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

defined('MOODLE_INTERNAL') || die();

$version = 2024011900.00; // YYYYMMDD = weekly release date of this DEV branch.
$version = 2024011900.01; // YYYYMMDD = weekly release date of this DEV branch.
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.
$release = '4.4dev (Build: 20240119)'; // Human-friendly version name
Expand Down

0 comments on commit 9e4366a

Please sign in to comment.