MDL-83181 reportbuilder: ensure audience correctness before delete.

reportbuilder/classes/external/audiences/delete.php

@@ -18,10 +18,10 @@ declare(strict_types=1);
 
 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 @@ class delete extends external_api {
         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_collator;
 use core_component;
 use core_reportbuilder\local\audiences\base;
 use core_reportbuilder\local\models\audience as audience_model;
+use invalid_parameter_exception;
 
 /**
  * Class containing report audience helper methods
@@ -232,6 +233,30 @@ class audience {
         return [$wheres, $params];
     }
 
+    /**
+     * 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;
+    }
+
     /**
      * Returns the list of audiences types in the system.
      *

reportbuilder/tests/local/helpers/audience_test.php

@@ -22,7 +22,9 @@ use advanced_testcase;
 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
@@ -297,4 +299,51 @@ class audience_test extends advanced_testcase {
         $reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params);
         $this->assertEmpty($reports);
     }
+
+    /**
+     * 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'));
+    }
 }