From 335012580bfa355a673d5dfd8a93a93fa2fdce81 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 18 Jan 2022 18:04:37 +0000 Subject: [PATCH] MDL-73598 reportbuilder: feature switch for custom reports. --- admin/settings/reportbuilder.php | 11 +++- admin/settings/subsystems.php | 6 ++ lang/en/reportbuilder.php | 2 + reportbuilder/classes/permission.php | 19 ++++-- reportbuilder/classes/task/send_schedule.php | 7 +- reportbuilder/tests/permission_test.php | 68 ++++++++++++++++++++ 6 files changed, 105 insertions(+), 8 deletions(-) diff --git a/admin/settings/reportbuilder.php b/admin/settings/reportbuilder.php index e9ccdc70d75..f28475f4e83 100644 --- a/admin/settings/reportbuilder.php +++ b/admin/settings/reportbuilder.php @@ -30,7 +30,13 @@ use core_reportbuilder\permission; defined('MOODLE_INTERNAL') || die; /** @var admin_root $ADMIN */ -$ADMIN->add('reports', new admin_category('reportbuilder', new lang_string('reportbuilder', 'core_reportbuilder'))); +$ADMIN->add( + 'reports', new admin_category( + 'reportbuilder', + new lang_string('reportbuilder', 'core_reportbuilder'), + empty($CFG->enablecustomreports) + ) +); $ADMIN->add( 'reportbuilder', new accesscallback( @@ -39,6 +45,7 @@ $ADMIN->add( (new moodle_url('/reportbuilder/index.php'))->out(), static function(accesscallback $accesscallback): bool { return permission::can_view_reports_list(); - } + }, + empty($CFG->enablecustomreports) ) ); diff --git a/admin/settings/subsystems.php b/admin/settings/subsystems.php index 10e06c639e9..8500721a478 100644 --- a/admin/settings/subsystems.php +++ b/admin/settings/subsystems.php @@ -64,6 +64,12 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page 1) ); + $optionalsubsystems->add(new admin_setting_configcheckbox('enablecustomreports', + new lang_string('enablecustomreports', 'core_reportbuilder'), + new lang_string('enablecustomreports_desc', 'core_reportbuilder'), + 1 + )); + $fullunicodesupport = true; if ($DB->get_dbfamily() == 'mysql') { $collation = $DB->get_dbcollation(); diff --git a/lang/en/reportbuilder.php b/lang/en/reportbuilder.php index fa095f57f78..c929e32598d 100644 --- a/lang/en/reportbuilder.php +++ b/lang/en/reportbuilder.php @@ -100,6 +100,8 @@ $string['editreportdetails'] = 'Edit report details'; $string['editreportname'] = 'Edit report name'; $string['editscheduledetails'] = 'Edit schedule details'; $string['editschedulename'] = 'Edit schedule name'; +$string['enablecustomreports'] = 'Enable custom reports'; +$string['enablecustomreports_desc'] = 'Allow users to create and view Report builder custom reports'; $string['entitycourse'] = 'Course'; $string['entityuser'] = 'User'; $string['errorreportcreate'] = 'You cannot create a new report'; diff --git a/reportbuilder/classes/permission.php b/reportbuilder/classes/permission.php index 4a6423ce9a6..21774f68330 100644 --- a/reportbuilder/classes/permission.php +++ b/reportbuilder/classes/permission.php @@ -51,7 +51,9 @@ class permission { * @return bool */ public static function can_view_reports_list(?int $userid = null): bool { - return has_any_capability([ + global $CFG; + + return !empty($CFG->enablecustomreports) && has_any_capability([ 'moodle/reportbuilder:editall', 'moodle/reportbuilder:edit', 'moodle/reportbuilder:view', @@ -96,7 +98,6 @@ class permission { * * @param report $report * @param int|null $userid User ID to check, or the current user if omitted - * @return void * @throws report_access_exception */ public static function require_can_edit_report(report $report, ?int $userid = null): void { @@ -113,7 +114,11 @@ class permission { * @return bool */ public static function can_edit_report(report $report, ?int $userid = null): bool { - global $USER; + global $CFG, $USER; + + if (empty($CFG->enablecustomreports)) { + return false; + } // We can only edit custom reports. if ($report->get('type') !== base::TYPE_CUSTOM_REPORT) { @@ -135,8 +140,12 @@ class permission { * @return bool */ public static function can_create_report(?int $userid = null): bool { - $capabilities = ['moodle/reportbuilder:edit', 'moodle/reportbuilder:editall']; - return has_any_capability($capabilities, context_system::instance(), $userid); + global $CFG; + + return !empty($CFG->enablecustomreports) && has_any_capability([ + 'moodle/reportbuilder:edit', + 'moodle/reportbuilder:editall', + ], context_system::instance(), $userid); } /** diff --git a/reportbuilder/classes/task/send_schedule.php b/reportbuilder/classes/task/send_schedule.php index 00682625a34..fb7968f1379 100644 --- a/reportbuilder/classes/task/send_schedule.php +++ b/reportbuilder/classes/task/send_schedule.php @@ -38,13 +38,18 @@ class send_schedule extends adhoc_task { * Execute the task */ public function execute(): void { - global $USER, $DB; + global $CFG, $USER, $DB; [ 'reportid' => $reportid, 'scheduleid' => $scheduleid, ] = (array) $this->get_custom_data(); + // Custom reports are disabled. + if (empty($CFG->enablecustomreports)) { + return; + } + $schedule = schedule::get_record(['id' => $scheduleid, 'reportid' => $reportid]); if ($schedule === false) { $this->log('Invalid schedule', 0); diff --git a/reportbuilder/tests/permission_test.php b/reportbuilder/tests/permission_test.php index 5e093f92d97..718d58b22f4 100644 --- a/reportbuilder/tests/permission_test.php +++ b/reportbuilder/tests/permission_test.php @@ -63,6 +63,20 @@ class permission_test extends advanced_testcase { permission::require_can_view_reports_list(); } + /** + * Test whether user can view reports list when custom reports are disabled + */ + public function test_require_can_view_reports_list_disabled(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + set_config('enablecustomreports', 0); + + $this->expectException(report_access_exception::class); + $this->expectExceptionMessage('You cannot view this report'); + permission::require_can_view_reports_list(); + } + /** * Test whether user can view specific report */ @@ -129,6 +143,24 @@ class permission_test extends advanced_testcase { permission::require_can_view_report($report); } + /** + * Test whether user can view report when custom reports are disabled + */ + public function test_require_can_view_report_disabled(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + set_config('enablecustomreports', 0); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + $this->expectException(report_access_exception::class); + $this->expectExceptionMessage('You cannot view this report'); + permission::require_can_view_report($report); + } + /** * Test that user cannot edit system reports */ @@ -206,6 +238,24 @@ class permission_test extends advanced_testcase { permission::require_can_edit_report($reportadmin); } + /** + * Test whether user can edit report when custom reports are disabled + */ + public function test_require_can_edit_report_disabled(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + set_config('enablecustomreports', 0); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + $this->expectException(report_access_exception::class); + $this->expectExceptionMessage('You cannot edit this report'); + permission::require_can_edit_report($report); + } + /** * Test that user can create a new report */ @@ -248,4 +298,22 @@ class permission_test extends advanced_testcase { $this->expectExceptionMessage('You cannot create a new report'); permission::require_can_create_report((int)$user3->id); } + + /** + * Test whether user can create report when custom reports are disabled + */ + public function test_require_can_create_report_disabled(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + set_config('enablecustomreports', 0); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + $this->expectException(report_access_exception::class); + $this->expectExceptionMessage('You cannot create a new report'); + permission::require_can_create_report(); + } }