MDL-87451 reportbuilder: accept entity instances when adding elements.

Where the report already has an instance of the entity, it should be
able to just pass that, avoiding a lookup of the same.
This commit is contained in:
Paul Holden
2026-02-27 10:25:19 +00:00
parent dcf942624d
commit 33dc3d8102
4 changed files with 205 additions and 43 deletions
@@ -0,0 +1,13 @@
issueNumber: MDL-87451
notes:
core_reportbuilder:
- message: >-
The following methods now support both string or entity instance types
for parameters referring to entities by name, which prevents lookups of
instances which most report sources will already have:
- `add_all_from_[entity|entities]()`
- `add_[columns|filters|conditions]_from_entity()`
type: changed
+14 -21
View File
@@ -19,10 +19,10 @@ declare(strict_types=1);
namespace core_reportbuilder;
use coding_exception;
use core_reportbuilder\local\entities\base as entity_base;
use core_reportbuilder\local\helpers\report;
use core_reportbuilder\local\models\{column as column_model, filter as filter_model};
use core_reportbuilder\local\report\base;
use core_reportbuilder\local\report\{column, filter};
use core_reportbuilder\local\report\{base, column, filter};
/**
* Class datasource
@@ -207,17 +207,21 @@ abstract class datasource extends base {
*
* Wildcard matching is supported with '*' in both $include and $exclude, e.g. ['customfield*']
*
* @param string $entityname
* @param string|entity_base $entityname
* @param string[] $include Include only these conditions, if omitted then include all
* @param string[] $exclude Exclude these conditions, if omitted then exclude none
* @throws coding_exception If both $include and $exclude are non-empty
*/
final protected function add_conditions_from_entity(string $entityname, array $include = [], array $exclude = []): void {
final protected function add_conditions_from_entity(
string|entity_base $entityname,
array $include = [],
array $exclude = [],
): void {
if (!empty($include) && !empty($exclude)) {
throw new coding_exception('Cannot specify conditions to include and exclude simultaneously');
}
$entity = $this->get_entity($entityname);
$entity = $this->normalise_entity($entityname);
// Retrieve filtered conditions from entity, respecting given $include/$exclude parameters.
$conditions = array_filter($entity->get_conditions(), function (filter $condition) use ($include, $exclude): bool {
@@ -312,13 +316,13 @@ abstract class datasource extends base {
/**
* Adds all columns/filters/conditions from the given entity to the report at once
*
* @param string $entityname
* @param string|entity_base $entityname
* @param string[] $limitcolumns Include only these columns
* @param string[] $limitfilters Include only these filters
* @param string[] $limitconditions Include only these conditions
*/
final protected function add_all_from_entity(
string $entityname,
string|entity_base $entityname,
array $limitcolumns = [],
array $limitfilters = [],
array $limitconditions = [],
@@ -331,23 +335,12 @@ abstract class datasource extends base {
/**
* Adds all columns/filters/conditions from all the entities added to the report at once
*
* @param string[] $entitynames If specified, then only these entity elements are added in the order that they are specified
* @param string[]|entity_base[] $entitynames Limit to only these entity elements, in the order that they are specified
*/
final protected function add_all_from_entities(array $entitynames = []): void {
if (empty($entitynames)) {
$entities = $this->get_entities();
} else {
$entities = array_combine(
$entitynames,
array_map(
fn(string $entityname) => $this->get_entity($entityname),
$entitynames,
),
);
}
$entities = empty($entitynames) ? $this->get_entities() : $entitynames;
foreach ($entities as $entity) {
$this->add_all_from_entity($entity->get_entity_name());
$this->add_all_from_entity($entity);
}
}
@@ -305,6 +305,17 @@ abstract class base {
return $this->entities;
}
/**
* Helper to normalise entity instance based on type of passed parameter
*
* @param string|entity_base $entity
* @return entity_base
*/
final protected function normalise_entity(string|entity_base $entity): entity_base {
$entityname = $entity instanceof entity_base ? $entity->get_entity_name() : $entity;
return $this->get_entity($entityname);
}
/**
* Define a new entity for the report
*
@@ -385,17 +396,21 @@ abstract class base {
*
* Wildcard matching is supported with '*' in both $include and $exclude, e.g. ['customfield*']
*
* @param string $entityname
* @param string|entity_base $entityname
* @param string[] $include Include only these columns, if omitted then include all
* @param string[] $exclude Exclude these columns, if omitted then exclude none
* @throws coding_exception If both $include and $exclude are non-empty
*/
final protected function add_columns_from_entity(string $entityname, array $include = [], array $exclude = []): void {
final protected function add_columns_from_entity(
string|entity_base $entityname,
array $include = [],
array $exclude = [],
): void {
if (!empty($include) && !empty($exclude)) {
throw new coding_exception('Cannot specify columns to include and exclude simultaneously');
}
$entity = $this->get_entity($entityname);
$entity = $this->normalise_entity($entityname);
// Retrieve filtered columns from entity, respecting given $include/$exclude parameters.
$columns = array_filter($entity->get_columns(), function(column $column) use ($include, $exclude): bool {
@@ -683,17 +698,21 @@ abstract class base {
*
* Wildcard matching is supported with '*' in both $include and $exclude, e.g. ['customfield*']
*
* @param string $entityname
* @param string|entity_base $entityname
* @param string[] $include Include only these filters, if omitted then include all
* @param string[] $exclude Exclude these filters, if omitted then exclude none
* @throws coding_exception If both $include and $exclude are non-empty
*/
final protected function add_filters_from_entity(string $entityname, array $include = [], array $exclude = []): void {
final protected function add_filters_from_entity(
string|entity_base $entityname,
array $include = [],
array $exclude = [],
): void {
if (!empty($include) && !empty($exclude)) {
throw new coding_exception('Cannot specify filters to include and exclude simultaneously');
}
$entity = $this->get_entity($entityname);
$entity = $this->normalise_entity($entityname);
// Retrieve filtered filters from entity, respecting given $include/$exclude parameters.
$filters = array_filter($entity->get_filters(), function(filter $filter) use ($include, $exclude): bool {
+153 -16
View File
@@ -27,6 +27,7 @@ declare(strict_types=1);
namespace core_reportbuilder;
use advanced_testcase;
use core\exception\coding_exception;
use core\lang_string;
use core_reportbuilder_generator;
use core_reportbuilder\local\entities\base;
@@ -40,7 +41,7 @@ defined('MOODLE_INTERNAL') || die();
* Unit tests for base datasource
*
* @package core_reportbuilder
* @coversDefaultClass \core_reportbuilder\datasource
* @covers \core_reportbuilder\datasource
* @copyright 2023 Paul Holden <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
@@ -91,8 +92,6 @@ final class datasource_test extends advanced_testcase {
* @param string[] $exclude
* @param string[] $expectedcolumns
*
* @covers ::add_columns_from_entity
*
* @dataProvider add_columns_from_entity_provider
*/
public function test_add_columns_from_entity(
@@ -102,10 +101,10 @@ final class datasource_test extends advanced_testcase {
): void {
$instance = $this->get_datasource_test_source();
// Assert we can pass the entity name when adding columns.
$method = (new ReflectionClass($instance))->getMethod('add_columns_from_entity');
$method->invoke($instance, 'entityone', $include, $exclude);
// Get all our entity columns.
$this->assertEquals(
$expectedcolumns,
array_map(
@@ -115,6 +114,56 @@ final class datasource_test extends advanced_testcase {
);
}
/**
* Test adding columns from entity instance
*/
public function test_add_columns_from_entity_instance(): void {
$instance = $this->get_datasource_test_source();
// Get the entity instance.
$method = (new ReflectionClass($instance))->getMethod('get_entity');
$entity = $method->invoke($instance, 'entityone');
// Assert we can pass the entity instance itself when adding columns.
$method = (new ReflectionClass($instance))->getMethod('add_columns_from_entity');
$method->invoke($instance, $entity, ['first']);
$this->assertEquals(
[
'dummy:test',
'entityone:first',
],
array_map(
fn(column $column) => $column->get_unique_identifier(),
array_values($instance->get_columns()),
),
);
}
/**
* Test adding columns from entity that has not been added to report
*/
public function test_add_columns_from_entity_invalid(): void {
$instance = $this->get_datasource_test_source();
$method = (new ReflectionClass($instance))->getMethod('add_columns_from_entity');
// Invalid entity name.
try {
$method->invoke($instance, 'invalid');
$this->fail('Exception expected');
} catch (coding_exception $exception) {
$this->assertStringContainsString("Invalid entity name (invalid)", $exception->getMessage());
}
// Invalid entity instance.
try {
$method->invoke($instance, new datasource_test_entity());
$this->fail('Exception expected');
} catch (coding_exception $exception) {
$this->assertStringContainsString("Invalid entity name (datasource_test_entity)", $exception->getMessage());
}
}
/**
* Data provider for {@see test_add_filters_from_entity}
*
@@ -158,8 +207,6 @@ final class datasource_test extends advanced_testcase {
* @param string[] $exclude
* @param string[] $expectedfilters
*
* @covers ::add_filters_from_entity
*
* @dataProvider add_filters_from_entity_provider
*/
public function test_add_filters_from_entity(
@@ -169,10 +216,10 @@ final class datasource_test extends advanced_testcase {
): void {
$instance = $this->get_datasource_test_source();
// Assert we can pass the entity name when adding filters.
$method = (new ReflectionClass($instance))->getMethod('add_filters_from_entity');
$method->invoke($instance, 'entityone', $include, $exclude);
// Get all our entity filters.
$this->assertEquals(
$expectedfilters,
array_map(
@@ -182,6 +229,55 @@ final class datasource_test extends advanced_testcase {
);
}
/**
* Test adding filters from entity instance
*/
public function test_add_filters_from_entity_instance(): void {
$instance = $this->get_datasource_test_source();
// Get the entity instance.
$method = (new ReflectionClass($instance))->getMethod('get_entity');
$entity = $method->invoke($instance, 'entityone');
// Assert we can pass the entity instance itself when adding filters.
$method = (new ReflectionClass($instance))->getMethod('add_filters_from_entity');
$method->invoke($instance, $entity, ['first']);
$this->assertEquals(
[
'entityone:first',
],
array_map(
fn(filter $filter) => $filter->get_unique_identifier(),
array_values($instance->get_filters()),
),
);
}
/**
* Test adding filters from entity that has not been added to report
*/
public function test_add_filters_from_entity_invalid(): void {
$instance = $this->get_datasource_test_source();
$method = (new ReflectionClass($instance))->getMethod('add_filters_from_entity');
// Invalid entity name.
try {
$method->invoke($instance, 'invalid');
$this->fail('Exception expected');
} catch (coding_exception $exception) {
$this->assertStringContainsString("Invalid entity name (invalid)", $exception->getMessage());
}
// Invalid entity instance.
try {
$method->invoke($instance, new datasource_test_entity());
$this->fail('Exception expected');
} catch (coding_exception $exception) {
$this->assertStringContainsString("Invalid entity name (datasource_test_entity)", $exception->getMessage());
}
}
/**
* Data provider for {@see test_add_conditions_from_entity}
*
@@ -225,8 +321,6 @@ final class datasource_test extends advanced_testcase {
* @param string[] $exclude
* @param string[] $expectedconditions
*
* @covers ::add_conditions_from_entity
*
* @dataProvider add_conditions_from_entity_provider
*/
public function test_add_conditions_from_entity(
@@ -236,10 +330,10 @@ final class datasource_test extends advanced_testcase {
): void {
$instance = $this->get_datasource_test_source();
// Assert we can pass the entity name when adding conditions.
$method = (new ReflectionClass($instance))->getMethod('add_conditions_from_entity');
$method->invoke($instance, 'entityone', $include, $exclude);
// Get all our entity conditions.
$this->assertEquals(
$expectedconditions,
array_map(
@@ -249,10 +343,57 @@ final class datasource_test extends advanced_testcase {
);
}
/**
* Test adding conditions from entity instance
*/
public function test_add_conditions_from_entity_instance(): void {
$instance = $this->get_datasource_test_source();
// Get the entity instance.
$method = (new ReflectionClass($instance))->getMethod('get_entity');
$entity = $method->invoke($instance, 'entityone');
// Assert we can pass the entity instance itself when adding conditions.
$method = (new ReflectionClass($instance))->getMethod('add_conditions_from_entity');
$method->invoke($instance, $entity, ['first']);
$this->assertEquals(
[
'entityone:first',
],
array_map(
fn(filter $condition) => $condition->get_unique_identifier(),
array_values($instance->get_conditions()),
),
);
}
/**
* Test adding conditions from entity that has not been added to report
*/
public function test_add_conditions_from_entity_invalid(): void {
$instance = $this->get_datasource_test_source();
$method = (new ReflectionClass($instance))->getMethod('add_conditions_from_entity');
// Invalid entity name.
try {
$method->invoke($instance, 'invalid');
$this->fail('Exception expected');
} catch (coding_exception $exception) {
$this->assertStringContainsString("Invalid entity name (invalid)", $exception->getMessage());
}
// Invalid entity instance.
try {
$method->invoke($instance, new datasource_test_entity());
$this->fail('Exception expected');
} catch (coding_exception $exception) {
$this->assertStringContainsString("Invalid entity name (datasource_test_entity)", $exception->getMessage());
}
}
/**
* Test adding all from entity
*
* @covers ::add_all_from_entity
*/
public function test_add_all_from_entity(): void {
$instance = $this->get_datasource_test_source();
@@ -393,8 +534,6 @@ final class datasource_test extends advanced_testcase {
* @param string[] $expectedfilters
* @param string[] $expectedconditions
*
* @covers ::add_all_from_entities
*
* @dataProvider add_all_from_entities_provider
*/
public function test_add_all_from_entities(
@@ -438,8 +577,6 @@ final class datasource_test extends advanced_testcase {
/**
* Test getting active conditions
*
* @covers ::get_active_conditions
*/
public function test_get_active_conditions(): void {
$instance = $this->get_datasource_test_source();