From ff717ac87f65b9dbe02320d77227d03ce3c84c48 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Thu, 28 Jul 2022 22:14:34 +0100
Subject: [PATCH] MDL-75447 reportbuilder: return only valid audience
 instances.

---
 .../classes/local/helpers/audience.php        | 10 +++++++---
 .../tests/local/helpers/audience_test.php     | 20 ++++++++++++++-----
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/reportbuilder/classes/local/helpers/audience.php b/reportbuilder/classes/local/helpers/audience.php
index de50cb9b223..8dc3f3843a4 100644
--- a/reportbuilder/classes/local/helpers/audience.php
+++ b/reportbuilder/classes/local/helpers/audience.php
@@ -35,16 +35,20 @@ use core_reportbuilder\local\models\audience as audience_model;
 class audience {
 
     /**
-     * Return audience base records for a given report
+     * Return audience instances for a given report. Note that any records pointing to invalid audience types will be excluded
      *
      * @param int $reportid
      * @return base[]
      */
     public static function get_base_records(int $reportid): array {
         $records = audience_model::get_records(['reportid' => $reportid], 'id');
-        return array_map(function ($record) {
-            return base::instance(0, $record->to_record());
+
+        $instances = array_map(static function(audience_model $audience): ?base {
+            return base::instance(0, $audience->to_record());
         }, $records);
+
+        // Filter and remove null elements (invalid audience types).
+        return array_filter($instances);
     }
 
     /**
diff --git a/reportbuilder/tests/local/helpers/audience_test.php b/reportbuilder/tests/local/helpers/audience_test.php
index fd31e191428..ac4bfb84404 100644
--- a/reportbuilder/tests/local/helpers/audience_test.php
+++ b/reportbuilder/tests/local/helpers/audience_test.php
@@ -50,10 +50,10 @@ class audience_test extends advanced_testcase {
     public function test_get_base_records(): void {
         $this->resetAfterTest();
 
-        // Report with no audiences.
         /** @var core_reportbuilder_generator $generator */
         $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
 
+        // Report with no audiences.
         $report = $generator->create_report([
             'name' => 'My report',
             'source' => users::class,
@@ -65,13 +65,14 @@ class audience_test extends advanced_testcase {
         // Create a couple of manual audience types.
         $user1 = $this->getDataGenerator()->create_user();
         $user2 = $this->getDataGenerator()->create_user();
-        $generator->create_audience([
+        $audience1 = $generator->create_audience([
             'reportid' => $report->get('id'),
             'classname' => manual::class,
             'configdata' => ['users' => [$user1->id, $user2->id]],
         ]);
+
         $user3 = $this->getDataGenerator()->create_user();
-        $generator->create_audience([
+        $audience2 = $generator->create_audience([
             'reportid' => $report->get('id'),
             'classname' => manual::class,
             'configdata' => ['users' => [$user3->id]],
@@ -79,8 +80,17 @@ class audience_test extends advanced_testcase {
 
         $baserecords = audience::get_base_records($report->get('id'));
         $this->assertCount(2, $baserecords);
-        $this->assertInstanceOf(manual::class, $baserecords[0]);
-        $this->assertInstanceOf(manual::class, $baserecords[1]);
+        $this->assertContainsOnlyInstancesOf(manual::class, $baserecords);
+
+        // Set invalid classname of first audience, should be excluded in subsequent request.
+        $audience1->get_persistent()->set('classname', '\invalid')->save();
+
+        $baserecords = audience::get_base_records($report->get('id'));
+        $this->assertCount(1, $baserecords);
+
+        $baserecord = reset($baserecords);
+        $this->assertInstanceOf(manual::class, $baserecord);
+        $this->assertEquals($audience2->get_persistent()->get('id'), $baserecord->get_persistent()->get('id'));
     }
 
     /**