From 677c2df9d639c171d29f2b3082ee7c805e2af530 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Mon, 1 Nov 2021 14:01:24 +0000
Subject: [PATCH] MDL-72961 reportbuilder: more defensive usage of report
 elements.

We should gracefully handle invalid element models that refer to
columns, filters, conditions that are no longer defined within the
report/entity they are linked to.
---
 .../custom_report_columns_sorting_exporter.php   | 10 ++++++----
 .../external/custom_report_filters_exporter.php  | 16 ++++++++++------
 reportbuilder/classes/form/condition.php         |  5 ++++-
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/reportbuilder/classes/external/custom_report_columns_sorting_exporter.php b/reportbuilder/classes/external/custom_report_columns_sorting_exporter.php
index 7eaef089b93..0d6719b6e96 100644
--- a/reportbuilder/classes/external/custom_report_columns_sorting_exporter.php
+++ b/reportbuilder/classes/external/custom_report_columns_sorting_exporter.php
@@ -93,11 +93,13 @@ class custom_report_columns_sorting_exporter extends exporter {
 
         $reportid = $report->get_report_persistent()->get('id');
         $activecolumns = column::get_records(['reportid' => $reportid], 'sortorder');
-        $sortablecolumns = array_filter($activecolumns, function(column $persistent) use($report) {
-            $column = $report->get_column($persistent->get('uniqueidentifier'))
-                ->set_aggregation($persistent->get('aggregation'));
+        $sortablecolumns = array_filter($activecolumns, function(column $persistent) use($report): bool {
+            $column = $report->get_column($persistent->get('uniqueidentifier'));
+            if ($column === null) {
+                return false;
+            }
 
-            return $column && $column->get_is_sortable();
+            return $column->set_aggregation($persistent->get('aggregation'))->get_is_sortable();
         });
 
         $sortablecolumns = array_map(function(column $persistent) use ($report) {
diff --git a/reportbuilder/classes/external/custom_report_filters_exporter.php b/reportbuilder/classes/external/custom_report_filters_exporter.php
index f05219cb960..53be1597234 100644
--- a/reportbuilder/classes/external/custom_report_filters_exporter.php
+++ b/reportbuilder/classes/external/custom_report_filters_exporter.php
@@ -103,11 +103,11 @@ class custom_report_filters_exporter extends exporter {
         /** @var base $report */
         $report = $this->related['report'];
 
-        // Current filter instances contained in the report.
-        $filterinstances = filter::get_filter_records($report->get_report_persistent()->get('id'), 'filterorder');
+        // Current filters added to the report.
+        $filters = filter::get_filter_records($report->get_report_persistent()->get('id'), 'filterorder');
         $filteridentifiers = array_map(static function(filter $filter): string {
             return $filter->get('uniqueidentifier');
-        }, $filterinstances);
+        }, $filters);
 
         $availablefilters = $activefilters = [];
 
@@ -134,12 +134,16 @@ class custom_report_filters_exporter extends exporter {
         }
 
         // Populate active filters.
-        foreach ($filterinstances as $filter) {
-            $editable = new filter_heading_editable($filter->get('id'));
+        $filterinstances = $report->get_filter_instances();
+        foreach ($filters as $filter) {
+            $filterinstance = $filterinstances[$filter->get('uniqueidentifier')] ?? null;
+            if ($filterinstance === null) {
+                continue;
+            }
 
-            $filterinstance = $report->get_filter($filter->get('uniqueidentifier'));
             $entityname = $filterinstance->get_entity_name();
             $displayvalue = $filterinstance->get_header();
+            $editable = new filter_heading_editable($filter->get('id'));
 
             $activefilters[] = [
                 'id' => $filter->get('id'),
diff --git a/reportbuilder/classes/form/condition.php b/reportbuilder/classes/form/condition.php
index e51907719f7..867c73f340f 100644
--- a/reportbuilder/classes/form/condition.php
+++ b/reportbuilder/classes/form/condition.php
@@ -124,7 +124,10 @@ class condition extends dynamic_form {
 
         $conditions = filter::get_condition_records($this->get_report()->get_report_persistent()->get('id'), 'filterorder');
         foreach ($conditions as $condition) {
-            $conditioninstance = $conditioninstances[$condition->get('uniqueidentifier')];
+            $conditioninstance = $conditioninstances[$condition->get('uniqueidentifier')] ?? null;
+            if ($conditioninstance === null) {
+                continue;
+            }
 
             $entityname = $conditioninstance->get_entity_name();
             $displayvalue = $conditioninstance->get_header();