Skip to content

Commit

Permalink
Fix sorting on users fields not using another table (#16872)
Browse files Browse the repository at this point in the history 
* Fix sorting on users fields not using another table

closes #16855

* Fix tests
  • Loading branch information
@trasher
trasher committed Apr 8, 2024
1 parent 07493e4 commit 3abeb83
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 82 deletions.
26 changes: 17 additions & 9 deletions src/Search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3755,15 +3755,23 @@ public static function addOrderBy($itemtype, $sort_fields, $_id = 'ASC')
$addaltemail = ",
IFNULL(`$ticket_user_table`.`alternative_email`, '')";
}
$criterion = "GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table$addtable`.`$name1`, ''),
IFNULL(`$table$addtable`.`$name2`, ''),
IFNULL(`$table$addtable`.`name`, '')$addaltemail
) ORDER BY CONCAT(
IFNULL(`$table$addtable`.`$name1`, ''),
IFNULL(`$table$addtable`.`$name2`, ''),
IFNULL(`$table$addtable`.`name`, '')$addaltemail) ASC
) $order";
if ($searchopt[$ID]['linkfield'] != 'users_id_lastupdater' && $searchopt[$ID]['linkfield'] != 'users_id_recipient') {
$criterion = "GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table$addtable`.`$name1`, ''),
IFNULL(`$table$addtable`.`$name2`, ''),
IFNULL(`$table$addtable`.`name`, '')$addaltemail
) ORDER BY CONCAT(
IFNULL(`$table$addtable`.`$name1`, ''),
IFNULL(`$table$addtable`.`$name2`, ''),
IFNULL(`$table$addtable`.`name`, '')$addaltemail) ASC
) $order";
} else {
$criterion = "CONCAT(
IFNULL(`$table$addtable`.`$name1`, ''),
IFNULL(`$table$addtable`.`$name2`, ''),
IFNULL(`$table$addtable`.`name`, '')$addaltemail
) $order";
}
} else {
$criterion = "`" . $table . $addtable . "`.`name` $order";
}
Expand Down
142 changes: 69 additions & 73 deletions tests/functional/Search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1392,16 +1392,16 @@ function () use (&$user_order_1) {
->withMessage('The parameters for Search::addOrderBy have changed to allow sorting by multiple fields. Please update your calling code.')
->exists();
$this->string($user_order_1)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) ASC ");
) ASC ");

$user_order_2 = null;
$this->when(
Expand All @@ -1413,16 +1413,16 @@ function () use (&$user_order_2) {
->withMessage('The parameters for Search::addOrderBy have changed to allow sorting by multiple fields. Please update your calling code.')
->exists();
$this->string($user_order_2)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) DESC ");
) DESC ");

$_SESSION['glpinames_format'] = \User::REALNAME_BEFORE;
$user_order_3 = null;
Expand All @@ -1435,16 +1435,16 @@ function () use (&$user_order_3) {
->withMessage('The parameters for Search::addOrderBy have changed to allow sorting by multiple fields. Please update your calling code.')
->exists();
$this->string($user_order_3)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) ASC ");
) ASC ");

$user_order_4 = null;
$this->when(
Expand All @@ -1456,16 +1456,16 @@ function () use (&$user_order_4) {
->withMessage('The parameters for Search::addOrderBy have changed to allow sorting by multiple fields. Please update your calling code.')
->exists();
$this->string($user_order_4)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) DESC ");
) DESC ");
}

/**
Expand All @@ -1488,33 +1488,33 @@ public function testAddOrderBy($itemtype, $sort_fields, $expected)
]
]);
$this->string($user_order_1)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) ASC ");
) ASC ");
$user_order_2 = \Search::addOrderBy('Ticket', [
[
'searchopt_id' => 4,
'order' => 'DESC'
]
]);
$this->string($user_order_2)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) DESC ");
) DESC ");

$_SESSION['glpinames_format'] = \User::REALNAME_BEFORE;
$user_order_3 = \Search::addOrderBy('Ticket', [
Expand All @@ -1524,33 +1524,33 @@ public function testAddOrderBy($itemtype, $sort_fields, $expected)
]
]);
$this->string($user_order_3)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) ASC ");
) ASC ");
$user_order_4 = \Search::addOrderBy('Ticket', [
[
'searchopt_id' => 4,
'order' => 'DESC'
]
]);
$this->string($user_order_4)->isEqualTo(" ORDER BY GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
) ORDER BY CONCAT(
IFNULL(`$table_addtable`.`realname`, ''),
IFNULL(`$table_addtable`.`firstname`, ''),
IFNULL(`$table_addtable`.`name`, ''),
IFNULL(`$table_ticket_user`.`alternative_email`, '')) ASC
) DESC ");
) DESC ");
}

/**
Expand Down Expand Up @@ -2191,15 +2191,11 @@ public function testSearchWithMultipleFkeysOnSameTable()
->contains("`glpi_users_users_id_recipient`.`id` = '{$user_normal_id}'")

// Check that ORDER applies on corresponding table alias
->contains("GROUP_CONCAT(DISTINCT CONCAT(
IFNULL(`glpi_users_users_id_recipient`.`realname`, ''),
IFNULL(`glpi_users_users_id_recipient`.`firstname`, ''),
IFNULL(`glpi_users_users_id_recipient`.`name`, '')
) ORDER BY CONCAT(
IFNULL(`glpi_users_users_id_recipient`.`realname`, ''),
IFNULL(`glpi_users_users_id_recipient`.`firstname`, ''),
IFNULL(`glpi_users_users_id_recipient`.`name`, '')) ASC
) ASC");
->contains("CONCAT(
IFNULL(`glpi_users_users_id_recipient`.`realname`, ''),
IFNULL(`glpi_users_users_id_recipient`.`firstname`, ''),
IFNULL(`glpi_users_users_id_recipient`.`name`, '')
) ASC");
}

public function testSearchAllAssets()
Expand Down

0 comments on commit 3abeb83

Please sign in to comment.