MDL-83181 reportbuilder: ensure audience correctness before delete.

reportbuilder/classes/external/audiences/delete.php

@@ -18,10 +18,10 @@
 
 namespace core_reportbuilder\external\audiences;
 
-use core_reportbuilder\local\audiences\base;
 use core_external\external_api;
 use core_external\external_value;
 use core_external\external_function_parameters;
+use core_reportbuilder\local\helpers\audience;
 use core_reportbuilder\manager;
 use core_reportbuilder\permission;
 
@@ -69,14 +69,7 @@ public static function execute(int $reportid, int $instanceid): bool {
         self::validate_context($report->get_context());
         permission::require_can_edit_report($report->get_report_persistent());
 
-        $baseinstance = base::instance($instanceid);
-        if ($baseinstance && $baseinstance->user_can_edit()) {
-            $persistent = $baseinstance->get_persistent();
-            $persistent->delete();
-            return true;
-        }
-
-        return false;
+        return audience::delete_report_audience($reportid, $instanceid);
     }
 
     /**

reportbuilder/classes/local/helpers/audience.php

@@ -25,6 +25,7 @@
 use core_component;
 use core_reportbuilder\local\audiences\base;
 use core_reportbuilder\local\models\{audience as audience_model, schedule};
+use invalid_parameter_exception;
 
 /**
  * Class containing report audience helper methods
@@ -254,6 +255,30 @@ public static function get_audiences_for_report_schedules(int $reportid): array
         return array_unique($audienceids, SORT_NUMERIC);
     }
 
+    /**
+     * Delete given audience from report
+     *
+     * @param int $reportid
+     * @param int $audienceid
+     * @return bool
+     * @throws invalid_parameter_exception
+     */
+    public static function delete_report_audience(int $reportid, int $audienceid): bool {
+        $audience = audience_model::get_record(['id' => $audienceid, 'reportid' => $reportid]);
+        if ($audience === false) {
+            throw new invalid_parameter_exception('Invalid audience');
+        }
+
+        $instance = base::instance(0, $audience->to_record());
+        if ($instance && $instance->user_can_edit()) {
+            $persistent = $instance->get_persistent();
+            $persistent->delete();
+            return true;
+        }
+
+        return false;
+    }
+
     /**
      * @deprecated since Moodle 4.1 - please do not use this function any more, {@see custom_report_audience_cards_exporter}
      */

reportbuilder/tests/local/helpers/audience_test.php

@@ -22,7 +22,9 @@
 use context_system;
 use core_reportbuilder_generator;
 use core_reportbuilder\reportbuilder\audience\manual;
+use core_reportbuilder\local\models\audience as audience_model;
 use core_user\reportbuilder\datasource\users;
+use invalid_parameter_exception;
 
 /**
  * Unit tests for audience helper
@@ -362,4 +364,51 @@ public function test_get_audiences_for_report_schedules(): void {
             $audiencetwo->get_persistent()->get('id'),
         ], $audiences);
     }
+
+    /**
+     * Testing deleting report audience
+     */
+    public function test_delete_report_audience(): void {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+
+        $report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
+        $audienceone = $generator->create_audience(['reportid' => $report->get('id'), 'configdata' => []]);
+        $audiencetwo = $generator->create_audience(['reportid' => $report->get('id'), 'configdata' => []]);
+
+        $audiences = audience_model::get_records(['reportid' => $report->get('id')]);
+        $this->assertCount(2, $audiences);
+
+        // Delete the first audience.
+        $result = audience::delete_report_audience($report->get('id'), $audienceone->get_persistent()->get('id'));
+        $this->assertTrue($result);
+
+        // We should be left with only the second audience.
+        $audiences = audience_model::get_records(['reportid' => $report->get('id')]);
+        $this->assertCount(1, $audiences);
+        $this->assertEquals($audiencetwo->get_persistent()->get('id'), reset($audiences)->get('id'));
+    }
+
+    /**
+     * Testing deleting invalid report audience
+     */
+    public function test_delete_report_audience_invalid(): void {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+
+        $reportone = $generator->create_report(['name' => 'Report one', 'source' => users::class, 'default' => false]);
+        $audienceone = $generator->create_audience(['reportid' => $reportone->get('id'), 'configdata' => []]);
+
+        $reporttwo = $generator->create_report(['name' => 'Report two', 'source' => users::class, 'default' => false]);
+
+        $this->expectException(invalid_parameter_exception::class);
+        $this->expectExceptionMessage('Invalid audience');
+        audience::delete_report_audience($reporttwo->get('id'), $audienceone->get_persistent()->get('id'));
+    }
 }