From 7a0a94b6b2bc6c47b6efdf298fefb9a7c072b3c5 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Wed, 13 Dec 2023 17:10:46 +0000 Subject: [PATCH 1/2] MDL-80412 reportbuilder: remove duplicate columns from order clause. This works around a problem in SQL Server, which caused it to throw a DML exception if the same column is duplicated in `ORDER BY` clause. Backported from MDL-79270. --- .../classes/table/base_report_table.php | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/reportbuilder/classes/table/base_report_table.php b/reportbuilder/classes/table/base_report_table.php index a2cc69612ef..6bbbae37d92 100644 --- a/reportbuilder/classes/table/base_report_table.php +++ b/reportbuilder/classes/table/base_report_table.php @@ -186,14 +186,24 @@ abstract class base_report_table extends table_sql implements dynamic, renderabl /** * Override parent method of the same, to ensure that any columns with custom sort fields are accounted for * + * Because the base table_sql has "special" handling of fullname columns {@see table_sql::contains_fullname_columns}, we need + * to handle that here to ensure that any that are being sorted take priority over reportbuilders own aliases of the same + * columns. This prevents them appearing multiple times in a query, which SQL Server really doesn't like + * * @return string */ public function get_sql_sort() { $columnsbyalias = $this->report->get_active_columns_by_alias(); $columnsortby = []; + // First pass over sorted columns, to extract all the fullname fields from table_sql. + $sortedcolumns = $this->get_sort_columns(); + $sortedcolumnsfullname = array_filter($sortedcolumns, static function(string $alias): bool { + return !preg_match('/^c[\d]+_/', $alias); + }, ARRAY_FILTER_USE_KEY); + // Iterate over all sorted report columns, replace with columns own fields if applicable. - foreach ($this->get_sort_columns() as $alias => $order) { + foreach ($sortedcolumns as $alias => $order) { $column = $columnsbyalias[$alias] ?? null; // If the column is not being aggregated and defines custom sort fields, then use them. @@ -208,6 +218,14 @@ abstract class base_report_table extends table_sql implements dynamic, renderabl } } + // Now ensure that any fullname sorted columns have duplicated aliases removed. + $columnsortby = array_filter($columnsortby, static function(string $alias) use ($sortedcolumnsfullname): bool { + if (preg_match('/^c[\d]+_(?.*)$/', $alias, $matches)) { + return !array_key_exists($matches['column'], $sortedcolumnsfullname); + } + return true; + }, ARRAY_FILTER_USE_KEY); + return static::construct_order_by($columnsortby); } From a432e1767f7a8cedd0dc27694b63defcd43d7582 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 15 Dec 2023 10:48:29 +0000 Subject: [PATCH 2/2] MDL-80412 reportbuilder: Behat scenario to view/sort additional names. Related to fixes from 8f32e623. --- reportbuilder/tests/behat/audience.feature | 33 +++++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/reportbuilder/tests/behat/audience.feature b/reportbuilder/tests/behat/audience.feature index 66f0d4d9b7d..115f08db891 100644 --- a/reportbuilder/tests/behat/audience.feature +++ b/reportbuilder/tests/behat/audience.feature @@ -8,10 +8,10 @@ Feature: Configure access to reports based on intended audience | datatype | shortname | name | | text | fruit | Fruit | And the following "users" exist: - | username | firstname | lastname | email | profile_field_fruit | - | user1 | User | 1 | user1@example.com | Apple | - | user2 | User | 2 | user2@example.com | Banana | - | user3 | User | 3 | user3@example.com | Banana | + | username | firstname | middlename | lastname | email | profile_field_fruit | + | user1 | User | One | 1 | user1@example.com | Apple | + | user2 | User | Two | 2 | user2@example.com | Banana | + | user3 | User | Three | 3 | user3@example.com | Banana | And the following "core_reportbuilder > Reports" exist: | name | source | default | | My report | core_user\reportbuilder\datasource\users | 1 | @@ -216,6 +216,31 @@ Feature: Configure access to reports based on intended audience And I should see "User 2" in the "reportbuilder-table" "table" And I should not see "User 3" in the "reportbuilder-table" "table" + Scenario: View configured user additional names on the access tab + Given the following config values are set as admin: + | alternativefullnameformat | firstname middlename lastname | + And the following "core_reportbuilder > Audiences" exist: + | report | configdata | + | My report | | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + When I click on the "Access" dynamic tab + Then I should see "User One 1" in the "reportbuilder-table" "table" + And I should see "User Two 2" in the "reportbuilder-table" "table" + And I should see "User Three 3" in the "reportbuilder-table" "table" + # Now sort each of them. + And I click on "First name" "link" in the "reportbuilder-table" "table" + And "Admin User" "table_row" should appear before "User One 1" "table_row" + And I click on "First name" "link" in the "reportbuilder-table" "table" + And "User One 1" "table_row" should appear before "Admin User" "table_row" + And I click on "Middle name" "link" in the "reportbuilder-table" "table" + And "User Two 2" "table_row" should appear before "User One 1" "table_row" + And I click on "Middle name" "link" in the "reportbuilder-table" "table" + And "User One 1" "table_row" should appear before "User Two 2" "table_row" + And I click on "Last name" "link" in the "reportbuilder-table" "table" + And "User One 1" "table_row" should appear before "User Two 2" "table_row" + And I click on "Last name" "link" in the "reportbuilder-table" "table" + And "User Two 2" "table_row" should appear before "User One 1" "table_row" + Scenario: View configured user identity fields on the access tab Given the following config values are set as admin: | showuseridentity | email,profile_field_fruit |