MDL-80943 reportbuilder: select-type customfields should be text.

The column type determines the available aggregation options present
for the column. Custom fields of type "select" should be considered as
text fields for this purpose, because their stored value represents
the index to their available options, rather than having any distinct
meaning of it's own for display.
This commit is contained in:
Paul Holden
2024-03-07 09:47:53 +00:00
parent 970fe4d26d
commit bf0849d9a4
4 changed files with 70 additions and 21 deletions
+21
View File
@@ -3719,5 +3719,26 @@ privatefiles,moodle|/user/files.php';
upgrade_main_savepoint(true, 2023100902.07);
}
if ($oldversion < 2023100903.05) {
// Get all "select" custom field shortnames.
$fieldshortnames = $DB->get_fieldset_select('customfield_field', 'shortname', 'type = :type', ['type' => 'select']);
// Ensure any used in custom reports columns are not using integer type aggregation.
foreach ($fieldshortnames as $fieldshortname) {
$DB->execute("
UPDATE {reportbuilder_column}
SET aggregation = NULL
WHERE " . $DB->sql_like('uniqueidentifier', ':uniqueidentifier', false) . "
AND aggregation IN ('avg', 'max', 'min', 'sum')
", [
'uniqueidentifier' => '%' . $DB->sql_like_escape(":customfield_{$fieldshortname}"),
]);
}
// Main savepoint reached.
upgrade_main_savepoint(true, 2023100903.05);
}
return true;
}
@@ -212,6 +212,10 @@ class custom_fields {
return column::TYPE_TIMESTAMP;
}
if ($field->get('type') === 'select') {
return column::TYPE_TEXT;
}
if ($datafield === 'intvalue') {
return column::TYPE_INTEGER;
}
@@ -97,16 +97,23 @@ class custom_fields_test extends core_reportbuilder_testcase {
$this->assertCount(5, $columns);
$this->assertContainsOnlyInstancesOf(column::class, $columns);
[$column0, $column1, $column2, $column3, $column4] = $columns;
$this->assertEqualsCanonicalizing(['Text', 'Textarea', 'Checkbox', 'Date', 'Select'], [
$column0->get_title(), $column1->get_title(), $column2->get_title(), $column3->get_title(), $column4->get_title()
]);
// Column titles.
$this->assertEquals(
['Text', 'Textarea', 'Checkbox', 'Date', 'Select'],
array_map(fn(column $column) => $column->get_title(), $columns)
);
$this->assertEquals(column::TYPE_TEXT, $column0->get_type());
$this->assertEquals('course', $column0->get_entity_name());
$this->assertStringStartsWith('LEFT JOIN {customfield_data}', $column0->get_joins()[0]);
// Column of type TEXT is sortable.
$this->assertTrue($column0->get_is_sortable());
// Column types.
$this->assertEquals(
[column::TYPE_TEXT, column::TYPE_LONGTEXT, column::TYPE_BOOLEAN, column::TYPE_TIMESTAMP, column::TYPE_TEXT],
array_map(fn(column $column) => $column->get_type(), $columns)
);
// Column sortable.
$this->assertEquals(
[true, false, true, true, true],
array_map(fn(column $column) => $column->get_is_sortable(), $columns)
);
}
/**
@@ -116,12 +123,23 @@ class custom_fields_test extends core_reportbuilder_testcase {
$this->resetAfterTest();
$customfields = $this->generate_customfields();
$columns = $customfields->get_columns();
$this->assertCount(1, ($columns[0])->get_joins());
$customfields->add_join('JOIN {test} t ON t.id = id');
// By default, we always join on the customfield data table.
$columns = $customfields->get_columns();
$this->assertCount(2, ($columns[0])->get_joins());
$joins = $columns[0]->get_joins();
$this->assertCount(1, $joins);
$this->assertStringStartsWith('LEFT JOIN {customfield_data}', $joins[0]);
// Add additional join.
$customfields->add_join('JOIN {test} t ON t.id = id');
$columns = $customfields->get_columns();
$joins = $columns[0]->get_joins();
$this->assertCount(2, $joins);
$this->assertEquals('JOIN {test} t ON t.id = id', $joins[0]);
$this->assertStringStartsWith('LEFT JOIN {customfield_data}', $joins[1]);
}
/**
@@ -131,12 +149,17 @@ class custom_fields_test extends core_reportbuilder_testcase {
$this->resetAfterTest();
$customfields = $this->generate_customfields();
$columns = $customfields->get_columns();
$this->assertCount(1, ($columns[0])->get_joins());
// Add additional joins.
$customfields->add_joins(['JOIN {test} t ON t.id = id', 'JOIN {test2} t2 ON t2.id = id']);
$columns = $customfields->get_columns();
$this->assertCount(3, ($columns[0])->get_joins());
$joins = $columns[0]->get_joins();
$this->assertCount(3, $joins);
$this->assertEquals('JOIN {test} t ON t.id = id', $joins[0]);
$this->assertEquals('JOIN {test2} t2 ON t2.id = id', $joins[1]);
$this->assertStringStartsWith('LEFT JOIN {customfield_data}', $joins[2]);
}
/**
@@ -151,10 +174,11 @@ class custom_fields_test extends core_reportbuilder_testcase {
$this->assertCount(5, $filters);
$this->assertContainsOnlyInstancesOf(filter::class, $filters);
[$filter0, $filter1, $filter2, $filter3, $filter4] = $filters;
$this->assertEqualsCanonicalizing(['Text', 'Textarea', 'Checkbox', 'Date', 'Select'], [
$filter0->get_header(), $filter1->get_header(), $filter2->get_header(), $filter3->get_header(), $filter4->get_header()
]);
// Filter titles.
$this->assertEquals(
['Text', 'Textarea', 'Checkbox', 'Date', 'Select'],
array_map(fn(filter $filter) => $filter->get_header(), $filters)
);
}
/**
+1 -1
View File
@@ -29,7 +29,7 @@
defined('MOODLE_INTERNAL') || die();
$version = 2023100903.04; // 20231009 = branching date YYYYMMDD - do not modify!
$version = 2023100903.05; // 20231009 = branching date YYYYMMDD - do not modify!
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.
$release = '4.3.3+ (Build: 20240305)'; // Human-friendly version name