Merge branch 'MDL-76221' of https://github.com/paulholden/moodle

badges/tests/reportbuilder/datasource/badges_test.php

@@ -72,16 +72,13 @@ public function test_datasource(): void {
         $report = $generator->create_report(['name' => 'Badges', 'source' => badges::class, 'default' => 0]);
 
         // Badge course.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:fullname'])
-            ->set_many(['sortenabled' => true, 'sortdirection' => SORT_ASC])->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:fullname', 'sortenabled' => 1]);
 
         // Badge name.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'badge:name'])
-            ->set_many(['sortenabled' => true, 'sortdirection' => SORT_ASC])->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'badge:name', 'sortenabled' => 1]);
 
         // User fullname.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname'])
-            ->set_many(['sortenabled' => true, 'sortdirection' => SORT_ASC])->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname', 'sortenabled' => 1]);
 
         $content = $this->get_custom_report_content($report->get('id'));
         $this->assertCount(3, $content);

course/tests/reportbuilder/datasource/participants_test.php

@@ -97,10 +97,7 @@ public function test_participants_datasource(): void {
         $generator->create_column(['reportid' => $report->get('id'),
             'uniqueidentifier' => 'user:fullname']);
         // Order by enrolment method.
-        $generator->create_column(['reportid' => $report->get('id'),
-            'uniqueidentifier' => 'enrolment:method'])
-            ->set('sortenabled', true)
-            ->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'enrolment:method', 'sortenabled' => 1]);
         $generator->create_column(['reportid' => $report->get('id'),
             'uniqueidentifier' => 'group:name']);
         $generator->create_column(['reportid' => $report->get('id'),

group/tests/reportbuilder/datasource/groups_test.php

@@ -86,8 +86,7 @@ public function test_datasource_groupings(): void {
 
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:fullname']);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'grouping:name']);
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'group:name'])
-            ->set('sortenabled', true)->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'group:name', 'sortenabled' => 1]);
 
         $content = $this->get_custom_report_content($report->get('id'));
         $this->assertCount(2, $content);

lib/classes/persistent.php

@@ -14,26 +14,18 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
-/**
- * Abstract class for objects saved to the DB.
- *
- * @package    core
- * @copyright  2015 Damyon Wiese
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
 namespace core;
-defined('MOODLE_INTERNAL') || die();
 
 use coding_exception;
 use invalid_parameter_exception;
 use lang_string;
 use ReflectionMethod;
 use stdClass;
-use renderer_base;
 
 /**
  * Abstract class for core objects saved to the DB.
  *
+ * @package    core
  * @copyright  2015 Damyon Wiese
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -328,6 +320,16 @@ final public static function properties_definition() {
         return $def;
     }
 
+    /**
+     * For a given record, return an array containing only those properties that are defined by the persistent
+     *
+     * @param stdClass $record
+     * @return array
+     */
+    final public static function properties_filter(stdClass $record): array {
+        return array_intersect_key((array) $record, static::properties_definition());
+    }
+
     /**
      * Gets all the formatted properties.
      *
@@ -421,8 +423,7 @@ final public static function is_property_required($property) {
      * @return static
      */
     final public function from_record(stdClass $record) {
-        $properties = static::properties_definition();
-        $record = array_intersect_key((array) $record, $properties);
+        $record = static::properties_filter($record);
         foreach ($record as $property => $value) {
             $this->raw_set($property, $value);
         }

lib/tests/persistent_test.php

@@ -14,14 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
-/**
- * Persistent class tests.
- *
- * @package    core
- * @copyright  2015 Frédéric Massart - FMCorz.net
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
 namespace core;
 
 use advanced_testcase;
@@ -30,8 +22,6 @@
 use lang_string;
 use xmldb_table;
 
-defined('MOODLE_INTERNAL') || die();
-
 /**
  * Persistent testcase.
  *
@@ -173,6 +163,23 @@ public function test_properties_definition() {
         $this->assertEquals($expected, core_testable_persistent::properties_definition());
     }
 
+    /**
+     * Test filtering record properties returns only those defined by the persistent
+     */
+    public function test_properties_filter(): void {
+        $result = core_testable_persistent::properties_filter((object) [
+            'idnumber' => '123',
+            'sortorder' => 1,
+            'invalidparam' => 'abc',
+        ]);
+
+        // We should get back all data except invalid param.
+        $this->assertEquals([
+            'idnumber' => '123',
+            'sortorder' => 1,
+        ], $result);
+    }
+
     /**
      * Test creating persistent instance by specifying record ID in constructor
      */

lib/upgrade.txt

@@ -17,6 +17,7 @@ information provided here is intended especially for developers.
 * The useexternalyui configuration setting has been removed as a part of the migration away from YUI.
   It only worked with http sites and did not officially support SSL, and is at risk of disappearing should Yahoo! decide
   to remove it.
+* New `properties_filter` method of persistent class for filtering properties of a record against persistent definition
 
 === 4.1 ===
 

reportbuilder/tests/external/custom_report_data_exporter_test.php

@@ -48,8 +48,12 @@ public function test_export(): void {
         $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
 
         $report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname'])
-            ->set_many(['heading' => 'Lovely user', 'sortenabled' => true])->update();
+        $generator->create_column([
+            'reportid' => $report->get('id'),
+            'uniqueidentifier' => 'user:fullname',
+            'heading' => 'Lovely user',
+            'sortenabled' => 1,
+        ]);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:email']);
 
         $reportinstance = manager::get_report_from_persistent($report);

reportbuilder/tests/external/reports/retrieve_test.php

@@ -53,8 +53,7 @@ public function test_execute(): void {
         $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
 
         $report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname'])
-            ->set('sortenabled', true)->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname', 'sortenabled' => 1]);
         $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:email']);
 
         // There are three users (admin plus the two previouly created), but we're paging the first two only.

reportbuilder/tests/generator/lib.php

@@ -80,7 +80,15 @@ public function create_column($record): column {
             throw new coding_exception('Record must contain \'uniqueidentifier\' property');
         }
 
-        return helper::add_report_column($record['reportid'], $record['uniqueidentifier']);
+        $column = helper::add_report_column($record['reportid'], $record['uniqueidentifier']);
+
+        // Update additional record properties.
+        unset($record['reportid'], $record['uniqueidentifier']);
+        if ($properties = column::properties_filter((object) $record)) {
+            $column->set_many($properties)->update();
+        }
+
+        return $column;
     }
 
     /**
@@ -100,7 +108,15 @@ public function create_filter($record): filter {
             throw new coding_exception('Record must contain \'uniqueidentifier\' property');
         }
 
-        return helper::add_report_filter($record['reportid'], $record['uniqueidentifier']);
+        $filter = helper::add_report_filter($record['reportid'], $record['uniqueidentifier']);
+
+        // Update additional record properties.
+        unset($record['reportid'], $record['uniqueidentifier']);
+        if ($properties = filter::properties_filter((object) $record)) {
+            $filter->set_many($properties)->update();
+        }
+
+        return $filter;
     }
 
     /**
@@ -120,7 +136,15 @@ public function create_condition($record): filter {
             throw new coding_exception('Record must contain \'uniqueidentifier\' property');
         }
 
-        return helper::add_report_condition($record['reportid'], $record['uniqueidentifier']);
+        $condition = helper::add_report_condition($record['reportid'], $record['uniqueidentifier']);
+
+        // Update additional record properties.
+        unset($record['reportid'], $record['uniqueidentifier']);
+        if ($properties = filter::properties_filter((object) $record)) {
+            $condition->set_many($properties)->update();
+        }
+
+        return $condition;
     }
 
     /**

reportbuilder/tests/generator_test.php

@@ -64,6 +64,27 @@ public function test_create_column(): void {
         $this->assertTrue(column::record_exists($column->get('id')));
     }
 
+    /**
+     * Test creating a column, specifying additional properties
+     */
+    public function test_create_column_additional_properties(): void {
+        $this->resetAfterTest();
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+
+        $report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => 0]);
+        $column = $generator->create_column([
+            'reportid' => $report->get('id'),
+            'uniqueidentifier' => 'user:lastname',
+            'heading' => 'My pants',
+            'sortenabled' => 1,
+        ]);
+
+        $this->assertEquals('My pants', $column->get('heading'));
+        $this->assertTrue($column->get('sortenabled'));
+    }
+
     /**
      * Test creating a filter
      */
@@ -79,6 +100,25 @@ public function test_create_filter(): void {
         $this->assertTrue(filter::record_exists($filter->get('id')));
     }
 
+    /**
+     * Test creating a filter, specifying additional properties
+     */
+    public function test_create_filter_additional_properties(): void {
+        $this->resetAfterTest();
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+
+        $report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => 0]);
+        $filter = $generator->create_filter([
+            'reportid' => $report->get('id'),
+            'uniqueidentifier' => 'user:lastname',
+            'heading' => 'My pants',
+        ]);
+
+        $this->assertEquals('My pants', $filter->get('heading'));
+    }
+
     /**
      * Test creating a condition
      */
@@ -94,6 +134,25 @@ public function test_create_condition(): void {
         $this->assertTrue(filter::record_exists($condition->get('id')));
     }
 
+    /**
+     * Test creating a condition, specifying additional properties
+     */
+    public function test_create_condition_additional_properties(): void {
+        $this->resetAfterTest();
+
+        /** @var core_reportbuilder_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
+
+        $report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => 0]);
+        $condition = $generator->create_condition([
+            'reportid' => $report->get('id'),
+            'uniqueidentifier' => 'user:lastname',
+            'heading' => 'My pants',
+        ]);
+
+        $this->assertEquals('My pants', $condition->get('heading'));
+    }
+
     /**
      * Test creating an audience
      */

reportbuilder/tests/local/aggregation/avg_test.php

@@ -54,14 +54,12 @@ public function test_column_aggregation(): void {
         $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]);
 
         // First column, sorted.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname'])
-            ->set('sortenabled', true)
-            ->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname', 'sortenabled' => 1]);
 
         // This is the column we'll aggregate.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:suspended'])
-            ->set('aggregation', avg::get_class_name())
-            ->update();
+        $generator->create_column(
+            ['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:suspended', 'aggregation' => avg::get_class_name()]
+        );
 
         $content = $this->get_custom_report_content($report->get('id'));
         $this->assertEquals([
@@ -91,14 +89,12 @@ public function test_column_aggregation_with_callback(): void {
         $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]);
 
         // First column, sorted.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname'])
-            ->set('sortenabled', true)
-            ->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname', 'sortenabled' => 1]);
 
         // This is the column we'll aggregate.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:suspended'])
-            ->set('aggregation', avg::get_class_name())
-            ->update();
+        $generator->create_column(
+            ['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:suspended', 'aggregation' => avg::get_class_name()]
+        );
 
         // Set callback to format the column (hack column definition to ensure callbacks are executed).
         $instance = manager::get_report_from_persistent($report);

reportbuilder/tests/local/aggregation/count_test.php

@@ -54,14 +54,12 @@ public function test_column_aggregation(): void {
         $report = $generator->create_report(['name' => 'Users', 'source' => users::class, 'default' => 0]);
 
         // First column, sorted.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname'])
-            ->set('sortenabled', true)
-            ->update();
+        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:firstname', 'sortenabled' => 1]);
 
         // This is the column we'll aggregate.
-        $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:lastname'])
-            ->set('aggregation', count::get_class_name())
-            ->update();
+        $generator->create_column([
+            'reportid' => $report->get('id'), 'uniqueidentifier' => 'user:lastname', 'aggregation' => count::get_class_name()]
+        );
 
         $content = $this->get_custom_report_content($report->get('id'));
         $this->assertEquals([