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-02-14 15:31:08 +00:00
parent b2fa19f45d
commit 4bd8f3a0dc
No known key found for this signature in database
GPG Key ID: A81A96D6045F6164
4 changed files with 70 additions and 21 deletions

View File

@ -1093,5 +1093,26 @@ function xmldb_main_upgrade($oldversion) {
upgrade_main_savepoint(true, 2024022300.02);
}
if ($oldversion < 2024030500.01) {
// Get all "select" custom field shortnames.
$fieldshortnames = $DB->get_fieldset('customfield_field', 'shortname', ['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, 2024030500.01);
}
return true;
}

View File

@ -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;
}

View File

@ -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)
);
}
/**

View File

@ -29,7 +29,7 @@
defined('MOODLE_INTERNAL') || die();
$version = 2024030500.00; // YYYYMMDD = weekly release date of this DEV branch.
$version = 2024030500.01; // YYYYMMDD = weekly release date of this DEV branch.
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.
$release = '4.4dev+ (Build: 20240305)'; // Human-friendly version name