MDL-74714 reportbuilder: clarify optional properties of exporters.

The custom report exporter should define its own editable element properties (conditions, filters, etc) as optional, rather than each of those element exporters defining their properties as optional.
2022-08-05 10:25:06 +01:00
parent 35b993b694
commit 411269b529
9 changed files with 58 additions and 45 deletions
@@ -52,11 +52,9 @@ class custom_report_card_view_exporter extends exporter {
        return [
            'form' => [
                'type' => PARAM_RAW,
-                'optional' => true,
            ],
            'helpicon' => [
                'type' => PARAM_RAW,
-                'optional' => true,
            ],
        ];
    }
@@ -53,7 +53,6 @@ class custom_report_columns_sorting_exporter extends exporter {
        return [
            'hassortablecolumns' => [
                'type' => PARAM_BOOL,
-                'optional' => true,
            ],
            'sortablecolumns' => [
                'type' => [
@@ -73,12 +72,10 @@ class custom_report_columns_sorting_exporter extends exporter {
                    'movetitle' => ['type' => PARAM_TEXT],
                    'sortenabledtitle' => ['type' => PARAM_TEXT],
                ],
-                'optional' => true,
                'multiple' => true,
            ],
            'helpicon' => [
                'type' => PARAM_RAW,
-                'optional' => true,
            ],
        ];
    }
@@ -53,7 +53,6 @@ class custom_report_conditions_exporter extends exporter {
        return [
            'hasavailableconditions' => [
                'type' => PARAM_BOOL,
-                'optional' => true,
            ],
            'availableconditions' => [
                'type' => [
@@ -71,19 +70,15 @@ class custom_report_conditions_exporter extends exporter {
                    ],
                ],
                'multiple' => true,
-                'optional' => true
            ],
            'hasactiveconditions' => [
                'type' => PARAM_BOOL,
-                'optional' => true,
            ],
            'activeconditionsform' => [
                'type' => PARAM_RAW,
-                'optional' => true,
            ],
            'helpicon' => [
                'type' => PARAM_RAW,
-                'optional' => true,
            ],
            'javascript' => [
                'type' => PARAM_RAW,
@@ -90,15 +90,30 @@ class custom_report_exporter extends persistent_exporter {
    protected static function define_other_properties(): array {
        return [
            'table' => ['type' => PARAM_RAW],
-            'sidebarmenucards' => ['type' => custom_report_column_cards_exporter::read_properties_definition()],
-            'conditions' => ['type' => custom_report_conditions_exporter::read_properties_definition()],
-            'filters' => ['type' => custom_report_filters_exporter::read_properties_definition()],
-            'sorting' => ['type' => custom_report_columns_sorting_exporter::read_properties_definition()],
-            'cardview' => ['type' => custom_report_card_view_exporter::read_properties_definition()],
            'filtersapplied' => ['type' => PARAM_INT],
            'filterspresent' => ['type' => PARAM_BOOL],
            'filtersform' => ['type' => PARAM_RAW],
            'editmode' => ['type' => PARAM_BOOL],
+            'sidebarmenucards' => [
+                'type' => custom_report_column_cards_exporter::read_properties_definition(),
+                'optional' => true,
+            ],
+            'conditions' => [
+                'type' => custom_report_conditions_exporter::read_properties_definition(),
+                'optional' => true,
+            ],
+            'filters' => [
+                'type' => custom_report_filters_exporter::read_properties_definition(),
+                'optional' => true,
+            ],
+            'sorting' => [
+                'type' => custom_report_columns_sorting_exporter::read_properties_definition(),
+                'optional' => true,
+            ],
+            'cardview' => [
+                'type' => custom_report_card_view_exporter::read_properties_definition(),
+                'optional' => true,
+            ],
            'javascript' => ['type' => PARAM_RAW],
        ];
    }
@@ -138,32 +153,32 @@ class custom_report_exporter extends persistent_exporter {
        $report = manager::get_report_from_persistent($this->persistent);

        // If we are editing we need all this information for the template.
+        $editordata = [];
        if ($this->editmode) {
            $menucardsexporter = new custom_report_column_cards_exporter(null, ['report' => $report]);
-            $menucards = (array) $menucardsexporter->export($output);
+            $editordata['sidebarmenucards'] = (array) $menucardsexporter->export($output);
+
            $conditionsexporter = new custom_report_conditions_exporter(null, ['report' => $report]);
-            $conditions = (array) $conditionsexporter->export($output);
+            $editordata['conditions'] = (array) $conditionsexporter->export($output);
+
            $filtersexporter = new custom_report_filters_exporter(null, ['report' => $report]);
-            $filters = (array) $filtersexporter->export($output);
+            $editordata['filters'] = (array) $filtersexporter->export($output);
+
            $sortingexporter = new custom_report_columns_sorting_exporter(null, ['report' => $report]);
-            $sorting = (array) $sortingexporter->export($output);
+            $editordata['sorting'] = (array) $sortingexporter->export($output);
+
            $cardviewexporter = new custom_report_card_view_exporter(null, ['report' => $report]);
-            $cardview = (array) $cardviewexporter->export($output);
+            $editordata['cardview'] = (array) $cardviewexporter->export($output);
        }

        return [
            'table' => $output->render($table),
-            'sidebarmenucards' => $menucards ?? [],
-            'conditions' => $conditions ?? [],
-            'filters' => $filters ?? [],
-            'sorting' => $sorting ?? [],
-            'cardview' => $cardview ?? [],
            'filtersapplied' => $report->get_applied_filter_count(),
            'filterspresent' => $filterspresent,
            'filtersform' => $filtersform,
            'editmode' => $this->editmode,
            'javascript' => '',
-        ];
+        ] + $editordata;
    }

    /**
@@ -53,7 +53,6 @@ class custom_report_filters_exporter extends exporter {
        return [
            'hasavailablefilters' => [
                'type' => PARAM_BOOL,
-                'optional' => true,
            ],
            'availablefilters' => [
                'type' => [
@@ -71,11 +70,9 @@ class custom_report_filters_exporter extends exporter {
                    ],
                ],
                'multiple' => true,
-                'optional' => true
            ],
            'hasactivefilters' => [
                'type' => PARAM_BOOL,
-                'optional' => true,
            ],
            'activefilters' => [
                'type' => [
@@ -87,11 +84,9 @@ class custom_report_filters_exporter extends exporter {
                    'entityname' => ['type' => PARAM_TEXT],
                ],
                'multiple' => true,
-                'optional' => true
            ],
            'helpicon' => [
                'type' => PARAM_RAW,
-                'optional' => true,
            ],
        ];
    }
@@ -69,10 +69,7 @@ class system_report_exporter extends persistent_exporter {
            'parameters' => ['type' => PARAM_RAW],
            'filterspresent' => ['type' => PARAM_BOOL],
            'filtersapplied' => ['type' => PARAM_INT],
-            'filtersform' => [
-                'type' => PARAM_RAW,
-                'optional' => true,
-            ],
+            'filtersform' => ['type' => PARAM_RAW],
        ];
    }

@@ -89,12 +89,12 @@ class custom_report_exporter_test extends advanced_testcase {
        $this->assertEmpty($export->filtersform);
        $this->assertFalse($export->editmode);

-        // The following are all generated by additional exporters.
-        $this->assertEmpty($export->sidebarmenucards);
-        $this->assertEmpty($export->conditions);
-        $this->assertEmpty($export->filters);
-        $this->assertEmpty($export->sorting);
-        $this->assertEmpty($export->cardview);
+        // The following are all generated by additional exporters, and should not be present when not editing.
+        $this->assertObjectNotHasAttribute('sidebarmenucards', $export);
+        $this->assertObjectNotHasAttribute('conditions', $export);
+        $this->assertObjectNotHasAttribute('filters', $export);
+        $this->assertObjectNotHasAttribute('sorting', $export);
+        $this->assertObjectNotHasAttribute('cardview', $export);
    }

    /**
@@ -71,11 +71,18 @@ class get_test extends externallib_advanced_testcase {
        $this->assertFalse($result['filterspresent']);
        $this->assertEmpty($result['filtersform']);
        $this->assertTrue($result['editmode']);
+
+        // Confirm editor-specific data is returned.
+        $this->assertNotEmpty($result['sidebarmenucards']);
+        $this->assertNotEmpty($result['conditions']);
+        $this->assertNotEmpty($result['filters']);
        $this->assertTrue($result['filters']['hasavailablefilters']);
        $this->assertNotEmpty($result['filters']['availablefilters']);
        $this->assertTrue($result['filters']['hasactivefilters']);
        $this->assertEquals($filterfullname->get('id'), $result['filters']['activefilters'][0]['id']);
        $this->assertEquals($filteremail->get('id'), $result['filters']['activefilters'][1]['id']);
+        $this->assertNotEmpty($result['sorting']);
+        $this->assertNotEmpty($result['cardview']);
    }

    /**
@@ -109,10 +116,13 @@ class get_test extends externallib_advanced_testcase {
        $this->assertTrue($result['filterspresent']);
        $this->assertNotEmpty($result['filtersform']);
        $this->assertFalse($result['editmode']);
-        $this->assertEmpty($result['filters']);
-        $this->assertEmpty($result['conditions']);
-        $this->assertEmpty($result['sorting']);
-        $this->assertEmpty($result['cardview']);
+
+        // Confirm editor-specific data is not returned.
+        $this->assertArrayNotHasKey('sidebarmenucards', $result);
+        $this->assertArrayNotHasKey('conditions', $result);
+        $this->assertArrayNotHasKey('filters', $result);
+        $this->assertArrayNotHasKey('sorting', $result);
+        $this->assertArrayNotHasKey('cardview', $result);
    }

    /**
@@ -23,6 +23,12 @@ 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 `custom_report_exporter` class now defines its editor element properties as optional, rather than each of those exporters
+  defining their own properties as optional. In turn, this means the structure of the following external methods will always be
+  present and consistent:
+  - `core_reportbuilder_columns_*`
+  - `core_reportbuilder_conditions_*`
+  - `core_reportbuilder_filters_*`
 * The following permission methods now accept an optional `$context` parameter (default system context):
  - `[require_]can_view_reports_list`
  - `[require_]can_create_report`