Merge branch 'MDL-74599' of https://github.com/paulholden/moodle

reportbuilder/classes/external/audiences/delete.php

@@ -18,7 +18,6 @@ declare(strict_types=1);
 
 namespace core_reportbuilder\external\audiences;
 
-use context_system;
 use core_reportbuilder\local\audiences\base;
 use external_api;
 use external_function_parameters;
@@ -72,7 +71,7 @@ class delete extends external_api {
 
         $report = manager::get_report_from_id($reportid);
 
-        self::validate_context(context_system::instance());
+        self::validate_context($report->get_context());
         permission::require_can_edit_report($report->get_report_persistent());
 
         $baseinstance = base::instance($instanceid);

reportbuilder/classes/external/filters/reset.php

@@ -18,10 +18,10 @@ declare(strict_types=1);
 
 namespace core_reportbuilder\external\filters;
 
-use context_system;
 use external_api;
 use external_function_parameters;
 use external_value;
+use core_reportbuilder\manager;
 use core_reportbuilder\local\helpers\user_filter_manager;
 
 defined('MOODLE_INTERNAL') || die();
@@ -62,8 +62,8 @@ class reset extends external_api {
             'reportid' => $reportid,
         ]);
 
-        $context = context_system::instance();
-        self::validate_context($context);
+        $report = manager::get_report_from_id($reportid);
+        self::validate_context($report->get_context());
 
         return user_filter_manager::reset_all($reportid);
     }

reportbuilder/classes/form/audience.php

@@ -19,13 +19,10 @@ declare(strict_types=1);
 namespace core_reportbuilder\form;
 
 use context;
-use context_system;
 use core_form\dynamic_form;
 use core_reportbuilder\local\audiences\base;
 use core_reportbuilder\output\audience_heading_editable;
-use core_reportbuilder\manager;
 use core_reportbuilder\permission;
-use moodle_exception;
 use moodle_url;
 use stdClass;
 
@@ -37,6 +34,7 @@ use stdClass;
  * @license     http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class audience extends dynamic_form {
+
     /**
      * Audience we work with
      *
@@ -95,13 +93,13 @@ class audience extends dynamic_form {
      * @return context
      */
     protected function get_context_for_dynamic_submission(): context {
-        return context_system::instance();
+        return $this->get_audience()->get_persistent()->get_report()->get_context();
     }
 
     /**
-     * Checks if current user has access to this form, otherwise throws exception
+     * Ensure current user is able to use this form
      *
-     * @throws moodle_exception
+     * A {@see \core_reportbuilder\report_access_exception} will be thrown if they can't
      */
     protected function check_access_for_dynamic_submission(): void {
         $audience = $this->get_audience();

reportbuilder/classes/form/schedule.php

@@ -19,7 +19,6 @@ declare(strict_types=1);
 namespace core_reportbuilder\form;
 
 use context;
-use context_system;
 use core_user;
 use html_writer;
 use moodle_url;

reportbuilder/classes/permission.php

@@ -18,6 +18,7 @@ declare(strict_types=1);
 
 namespace core_reportbuilder;
 
+use context;
 use context_system;
 use core_reportbuilder\local\helpers\audience;
 use core_reportbuilder\local\models\report;
@@ -36,10 +37,11 @@ class permission {
      * Require given user can view reports list
      *
      * @param int|null $userid User ID to check, or the current user if omitted
+     * @param context|null $context
      * @throws report_access_exception
      */
-    public static function require_can_view_reports_list(?int $userid = null): void {
-        if (!static::can_view_reports_list($userid)) {
+    public static function require_can_view_reports_list(?int $userid = null, ?context $context = null): void {
+        if (!static::can_view_reports_list($userid, $context)) {
             throw new report_access_exception();
         }
     }
@@ -48,16 +50,21 @@ class permission {
      * Whether given user can view reports list
      *
      * @param int|null $userid User ID to check, or the current user if omitted
+     * @param context|null $context
      * @return bool
      */
-    public static function can_view_reports_list(?int $userid = null): bool {
+    public static function can_view_reports_list(?int $userid = null, ?context $context = null): bool {
         global $CFG;
 
+        if ($context === null) {
+            $context = context_system::instance();
+        }
+
         return !empty($CFG->enablecustomreports) && has_any_capability([
             'moodle/reportbuilder:editall',
             'moodle/reportbuilder:edit',
             'moodle/reportbuilder:view',
-        ], context_system::instance(), $userid);
+        ], $context, $userid);
     }
 
     /**
@@ -81,7 +88,7 @@ class permission {
      * @return bool
      */
     public static function can_view_report(report $report, ?int $userid = null): bool {
-        if (!static::can_view_reports_list($userid)) {
+        if (!static::can_view_reports_list($userid, $report->get_context())) {
             return false;
         }
 
@@ -132,9 +139,9 @@ class permission {
             return has_any_capability([
                 'moodle/reportbuilder:edit',
                 'moodle/reportbuilder:editall',
-            ], context_system::instance(), $userid);
+            ], $report->get_context(), $userid);
         } else {
-            return has_capability('moodle/reportbuilder:editall', context_system::instance(), $userid);
+            return has_capability('moodle/reportbuilder:editall', $report->get_context(), $userid);
         }
     }
 
@@ -142,25 +149,31 @@ class permission {
      * Whether given user can create a new report
      *
      * @param int|null $userid User ID to check, or the current user if omitted
+     * @param context|null $context
      * @return bool
      */
-    public static function can_create_report(?int $userid = null): bool {
+    public static function can_create_report(?int $userid = null, ?context $context = null): bool {
         global $CFG;
 
+        if ($context === null) {
+            $context = context_system::instance();
+        }
+
         return !empty($CFG->enablecustomreports) && has_any_capability([
             'moodle/reportbuilder:edit',
             'moodle/reportbuilder:editall',
-        ], context_system::instance(), $userid);
+        ], $context, $userid);
     }
 
     /**
      * Require given user can create a new report
      *
      * @param int|null $userid User ID to check, or the current user if omitted
+     * @param context|null $context
      * @throws report_access_exception
      */
-    public static function require_can_create_report(?int $userid = null): void {
-        if (!static::can_create_report($userid)) {
+    public static function require_can_create_report(?int $userid = null, ?context $context = null): void {
+        if (!static::can_create_report($userid, $context)) {
             throw new report_access_exception('errorreportcreate');
         }
     }

reportbuilder/tests/external/filters/reset_test.php

@@ -18,9 +18,11 @@ declare(strict_types=1);
 
 namespace core_reportbuilder\external\filters;
 
+use core_reportbuilder_generator;
 use external_api;
 use externallib_advanced_testcase;
 use core_reportbuilder\local\helpers\user_filter_manager;
+use core_user\reportbuilder\datasource\users;
 
 defined('MOODLE_INTERNAL') || die();
 
@@ -44,18 +46,23 @@ class reset_test extends externallib_advanced_testcase {
         $this->resetAfterTest();
         $this->setAdminUser();
 
-        user_filter_manager::set(5, [
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+        $report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
+
+        user_filter_manager::set($report->get('id'), [
             'entity:filter_name' => 'something',
         ]);
 
-        $this->assertCount(1, user_filter_manager::get(5));
+        // Sanity check that we get back the filter we just set.
+        $this->assertCount(1, user_filter_manager::get($report->get('id')));
 
-        $result = reset::execute(5);
+        $result = reset::execute($report->get('id'));
         $result = external_api::clean_returnvalue(reset::execute_returns(), $result);
 
         $this->assertTrue($result);
 
         // We should get an empty array back.
-        $this->assertEquals([], user_filter_manager::get(5));
+        $this->assertEquals([], user_filter_manager::get($report->get('id')));
     }
 }

reportbuilder/upgrade.txt

@@ -14,3 +14,6 @@ Information provided here is intended especially for developers.
 * The following local helper methods have been deprecated, their implementation moved to exporters:
   - `audience::get_all_audiences_menu_types` -> `custom_report_audience_cards_exporter`
   - `report::get_available_columns` -> `custom_report_column_cards_exporter`
+* The following permission methods now accept an optional `$context` parameter (default system context):
+  - `[require_]can_view_reports_list`
+  - `[require_]can_create_report`