Merge branch 'MDL-75855' of https://github.com/paulholden/moodle
This commit is contained in:
@@ -4610,6 +4610,9 @@
|
||||
<KEY NAME="usercreated" TYPE="foreign" FIELDS="usercreated" REFTABLE="user" REFFIELDS="id"/>
|
||||
<KEY NAME="usermodified" TYPE="foreign" FIELDS="usermodified" REFTABLE="user" REFFIELDS="id"/>
|
||||
</KEYS>
|
||||
<INDEXES>
|
||||
<INDEX NAME="report-filter" UNIQUE="true" FIELDS="reportid, uniqueidentifier, iscondition"/>
|
||||
</INDEXES>
|
||||
</TABLE>
|
||||
<TABLE NAME="reportbuilder_audience" COMMENT="Defines report audience">
|
||||
<FIELDS>
|
||||
|
||||
@@ -3471,5 +3471,35 @@ privatefiles,moodle|/user/files.php';
|
||||
upgrade_main_savepoint(true, 2023082200.02);
|
||||
}
|
||||
|
||||
if ($oldversion < 2023082200.04) {
|
||||
|
||||
// Remove any non-unique filters/conditions.
|
||||
$duplicates = $DB->get_records_sql("
|
||||
SELECT MIN(id) AS id, reportid, uniqueidentifier, iscondition
|
||||
FROM {reportbuilder_filter}
|
||||
GROUP BY reportid, uniqueidentifier, iscondition
|
||||
HAVING COUNT(*) > 1");
|
||||
|
||||
foreach ($duplicates as $duplicate) {
|
||||
$DB->delete_records_select(
|
||||
'reportbuilder_filter',
|
||||
'id <> :id AND reportid = :reportid AND uniqueidentifier = :uniqueidentifier AND iscondition = :iscondition',
|
||||
(array) $duplicate
|
||||
);
|
||||
}
|
||||
|
||||
// Define index report-filter (unique) to be added to reportbuilder_filter.
|
||||
$table = new xmldb_table('reportbuilder_filter');
|
||||
$index = new xmldb_index('report-filter', XMLDB_INDEX_UNIQUE, ['reportid', 'uniqueidentifier', 'iscondition']);
|
||||
|
||||
// Conditionally launch add index report-filter.
|
||||
if (!$dbman->index_exists($table, $index)) {
|
||||
$dbman->add_index($table, $index);
|
||||
}
|
||||
|
||||
// Main savepoint reached.
|
||||
upgrade_main_savepoint(true, 2023082200.04);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -110,6 +110,7 @@ class report {
|
||||
public static function add_report_column(int $reportid, string $uniqueidentifier): column {
|
||||
$report = manager::get_report_from_id($reportid);
|
||||
|
||||
// Ensure column is available.
|
||||
if (!array_key_exists($uniqueidentifier, $report->get_columns())) {
|
||||
throw new invalid_parameter_exception('Invalid column');
|
||||
}
|
||||
@@ -237,10 +238,16 @@ class report {
|
||||
public static function add_report_condition(int $reportid, string $uniqueidentifier): filter {
|
||||
$report = manager::get_report_from_id($reportid);
|
||||
|
||||
// Ensure condition is available.
|
||||
if (!array_key_exists($uniqueidentifier, $report->get_conditions())) {
|
||||
throw new invalid_parameter_exception('Invalid condition');
|
||||
}
|
||||
|
||||
// Ensure condition wasn't already added.
|
||||
if (array_key_exists($uniqueidentifier, $report->get_active_conditions())) {
|
||||
throw new invalid_parameter_exception('Duplicate condition');
|
||||
}
|
||||
|
||||
$condition = new filter(0, (object) [
|
||||
'reportid' => $reportid,
|
||||
'uniqueidentifier' => $uniqueidentifier,
|
||||
@@ -317,11 +324,16 @@ class report {
|
||||
public static function add_report_filter(int $reportid, string $uniqueidentifier): filter {
|
||||
$report = manager::get_report_from_id($reportid);
|
||||
|
||||
$reportfilters = $report->get_filters();
|
||||
if (!array_key_exists($uniqueidentifier, $reportfilters)) {
|
||||
// Ensure filter is available.
|
||||
if (!array_key_exists($uniqueidentifier, $report->get_filters())) {
|
||||
throw new invalid_parameter_exception('Invalid filter');
|
||||
}
|
||||
|
||||
// Ensure filter wasn't already added.
|
||||
if (array_key_exists($uniqueidentifier, $report->get_active_filters())) {
|
||||
throw new invalid_parameter_exception('Duplicate filter');
|
||||
}
|
||||
|
||||
$filter = new filter(0, (object) [
|
||||
'reportid' => $reportid,
|
||||
'uniqueidentifier' => $uniqueidentifier,
|
||||
|
||||
+1
-1
@@ -84,7 +84,7 @@ class delete_test extends externallib_advanced_testcase {
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
$condition = $generator->create_condition(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:email']);
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
|
||||
@@ -92,7 +92,7 @@ class reorder_test extends externallib_advanced_testcase {
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
$condition = $generator->create_condition(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:email']);
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
|
||||
+1
-1
@@ -49,7 +49,7 @@ class reset_test extends externallib_advanced_testcase {
|
||||
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
$generator->create_condition(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname']);
|
||||
|
||||
$instance = manager::get_report_from_persistent($report);
|
||||
|
||||
+1
-1
@@ -85,7 +85,7 @@ class delete_test extends externallib_advanced_testcase {
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
$filter = $generator->create_filter(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:email']);
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
|
||||
+1
-1
@@ -95,7 +95,7 @@ class reorder_test extends externallib_advanced_testcase {
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
$filter = $generator->create_filter(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:email']);
|
||||
|
||||
$user = $this->getDataGenerator()->create_user();
|
||||
|
||||
@@ -386,6 +386,25 @@ class report_test extends advanced_testcase {
|
||||
report::add_report_condition($report->get('id'), 'user:invalid');
|
||||
}
|
||||
|
||||
/**
|
||||
* Test adding duplicate report condition
|
||||
*/
|
||||
public function test_add_report_condition_duplicate(): void {
|
||||
$this->resetAfterTest();
|
||||
$this->setAdminUser();
|
||||
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
|
||||
// First one is fine.
|
||||
report::add_report_condition($report->get('id'), 'user:email');
|
||||
|
||||
$this->expectException(invalid_parameter_exception::class);
|
||||
$this->expectExceptionMessage('Duplicate condition');
|
||||
report::add_report_condition($report->get('id'), 'user:email');
|
||||
}
|
||||
|
||||
/**
|
||||
* Test deleting report condition
|
||||
*/
|
||||
@@ -536,6 +555,25 @@ class report_test extends advanced_testcase {
|
||||
report::add_report_filter($report->get('id'), 'user:invalid');
|
||||
}
|
||||
|
||||
/**
|
||||
* Test adding duplicate report filter
|
||||
*/
|
||||
public function test_add_report_filter_duplicate(): void {
|
||||
$this->resetAfterTest();
|
||||
$this->setAdminUser();
|
||||
|
||||
/** @var core_reportbuilder_generator $generator */
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
$report = $generator->create_report(['name' => 'My report', 'source' => users::class, 'default' => false]);
|
||||
|
||||
// First one is fine.
|
||||
report::add_report_filter($report->get('id'), 'user:email');
|
||||
|
||||
$this->expectException(invalid_parameter_exception::class);
|
||||
$this->expectExceptionMessage('Duplicate filter');
|
||||
report::add_report_filter($report->get('id'), 'user:email');
|
||||
}
|
||||
|
||||
/**
|
||||
* Test deleting report filter
|
||||
*/
|
||||
|
||||
@@ -157,9 +157,11 @@ class send_schedule_test extends advanced_testcase {
|
||||
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
|
||||
|
||||
// Create a report that won't return any data.
|
||||
$report = $generator->create_report(['name' => 'Myself', 'source' => users::class]);
|
||||
$report = $generator->create_report(['name' => 'Myself', 'source' => users::class, 'default' => false]);
|
||||
|
||||
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:username']);
|
||||
$generator->create_condition(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:username']);
|
||||
|
||||
manager::get_report_from_persistent($report)->set_condition_values([
|
||||
'user:username_operator' => text::IS_EQUAL_TO,
|
||||
'user:username_value' => 'baconlettucetomato',
|
||||
|
||||
@@ -28,6 +28,8 @@ Information provided here is intended especially for developers.
|
||||
* If a non-default column is specified in a datasource `get_default_column_sorting` method, a coding exception will be thrown
|
||||
* Trying to add/annotate duplicate entity names to a report will now throw a coding exception
|
||||
* The `get_default_entity_name` method of the base entity class is now private, and shouldn't be overridden in extending classes
|
||||
* The report helper methods `add_report_[condition|filter]` now throw an exception when trying to add duplicate conditions or
|
||||
filters to a report
|
||||
* Two new methods:
|
||||
- `get_default_no_results_notice` and
|
||||
- `set_default_no_results_notice`
|
||||
|
||||
+1
-1
@@ -29,7 +29,7 @@
|
||||
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
$version = 2023082200.03; // YYYYMMDD = weekly release date of this DEV branch.
|
||||
$version = 2023082200.04; // YYYYMMDD = weekly release date of this DEV branch.
|
||||
// RR = release increments - 00 in DEV branches.
|
||||
// .XX = incremental changes.
|
||||
$release = '4.3dev+ (Build: 20230822)'; // Human-friendly version name
|
||||
|
||||
Reference in New Issue
Block a user