Skip to content

Commit

Permalink
fix(search): order by actor
Browse files Browse the repository at this point in the history
  • Loading branch information
@Rom1-B
Rom1-B authored Feb 5, 2024
1 parent 8026e4e commit 7e05bc0
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 28 deletions.
19 changes: 16 additions & 3 deletions src/Search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3744,9 +3744,22 @@ public static function addOrderBy($itemtype, $sort_fields, $_id = 'ASC')
$name1 = 'realname';
$name2 = 'firstname';
}
$criterion = "`" . $table . $addtable . "`.`$name1` $order,
`" . $table . $addtable . "`.`$name2` $order,
`" . $table . $addtable . "`.`name` $order";
$addaltemail = "";
if (
in_array($itemtype, ['Ticket', 'Change', 'Problem'])
&& isset($searchopt[$ID]['joinparams']['beforejoin']['table'])
&& in_array($searchopt[$ID]['joinparams']['beforejoin']['table'], ['glpi_tickets_users', 'glpi_changes_users', 'glpi_problems_users'])
) { // For tickets_users
$ticket_user_table = $searchopt[$ID]['joinparams']['beforejoin']['table'] . "_" .
self::computeComplexJoinID($searchopt[$ID]['joinparams']['beforejoin']['joinparams']);
$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";
} else {
$criterion = "`" . $table . $addtable . "`.`name` $order";
}
Expand Down
212 changes: 187 additions & 25 deletions tests/functional/Search.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,7 @@ function () use (&$result, $itemtype, $id, $order) {

// Complex cases
$table_addtable = 'glpi_users_af1042e23ce6565cfe58c6db91f84692';
$table_ticket_user = 'glpi_tickets_users_019878060c6d5f06cbe3c4d7c31dec24';

$_SESSION['glpinames_format'] = \User::FIRSTNAME_BEFORE;
$user_order_1 = null;
Expand All @@ -1388,9 +1389,12 @@ function () use (&$user_order_1) {
->withType(E_USER_DEPRECATED)
->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 `$table_addtable`.`firstname` ASC,
`$table_addtable`.`realname` ASC,
`$table_addtable`.`name` ASC ");
$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_ticket_user`.`alternative_email`, '')
)) ASC ");

$user_order_2 = null;
$this->when(
Expand All @@ -1401,9 +1405,12 @@ function () use (&$user_order_2) {
->withType(E_USER_DEPRECATED)
->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 `$table_addtable`.`firstname` DESC,
`$table_addtable`.`realname` DESC,
`$table_addtable`.`name` 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_ticket_user`.`alternative_email`, '')
)) DESC ");

$_SESSION['glpinames_format'] = \User::REALNAME_BEFORE;
$user_order_3 = null;
Expand All @@ -1415,9 +1422,13 @@ function () use (&$user_order_3) {
->withType(E_USER_DEPRECATED)
->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 `$table_addtable`.`realname` ASC,
`$table_addtable`.`firstname` ASC,
`$table_addtable`.`name` ASC ");
$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_ticket_user`.`alternative_email`, '')
)) ASC ");

$user_order_4 = null;
$this->when(
function () use (&$user_order_4) {
Expand All @@ -1427,9 +1438,12 @@ function () use (&$user_order_4) {
->withType(E_USER_DEPRECATED)
->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 `$table_addtable`.`realname` DESC,
`$table_addtable`.`firstname` DESC,
`$table_addtable`.`name` 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_ticket_user`.`alternative_email`, '')
)) DESC ");
}

/**
Expand All @@ -1442,6 +1456,7 @@ public function testAddOrderBy($itemtype, $sort_fields, $expected)

// Complex cases
$table_addtable = 'glpi_users_af1042e23ce6565cfe58c6db91f84692';
$table_ticket_user = 'glpi_tickets_users_019878060c6d5f06cbe3c4d7c31dec24';

$_SESSION['glpinames_format'] = \User::FIRSTNAME_BEFORE;
$user_order_1 = \Search::addOrderBy('Ticket', [
Expand All @@ -1450,18 +1465,24 @@ public function testAddOrderBy($itemtype, $sort_fields, $expected)
'order' => 'ASC'
]
]);
$this->string($user_order_1)->isEqualTo(" ORDER BY `$table_addtable`.`firstname` ASC,
`$table_addtable`.`realname` ASC,
`$table_addtable`.`name` ASC ");
$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_ticket_user`.`alternative_email`, '')
)) ASC ");
$user_order_2 = \Search::addOrderBy('Ticket', [
[
'searchopt_id' => 4,
'order' => 'DESC'
]
]);
$this->string($user_order_2)->isEqualTo(" ORDER BY `$table_addtable`.`firstname` DESC,
`$table_addtable`.`realname` DESC,
`$table_addtable`.`name` 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_ticket_user`.`alternative_email`, '')
)) DESC ");

$_SESSION['glpinames_format'] = \User::REALNAME_BEFORE;
$user_order_3 = \Search::addOrderBy('Ticket', [
Expand All @@ -1470,18 +1491,155 @@ public function testAddOrderBy($itemtype, $sort_fields, $expected)
'order' => 'ASC'
]
]);
$this->string($user_order_3)->isEqualTo(" ORDER BY `$table_addtable`.`realname` ASC,
`$table_addtable`.`firstname` ASC,
`$table_addtable`.`name` ASC ");
$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_ticket_user`.`alternative_email`, '')
)) ASC ");
$user_order_4 = \Search::addOrderBy('Ticket', [
[
'searchopt_id' => 4,
'order' => 'DESC'
]
]);
$this->string($user_order_4)->isEqualTo(" ORDER BY `$table_addtable`.`realname` DESC,
`$table_addtable`.`firstname` DESC,
`$table_addtable`.`name` 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_ticket_user`.`alternative_email`, '')
)) DESC ");
}

/**
* Data provider for testAddOrderByUser
*/
protected function testAddOrderByUserProvider(): iterable
{
$this->login('glpi', 'glpi');

$user_1 = getItemByTypeName('User', TU_USER)->getID();
$user_2 = getItemByTypeName('User', 'glpi')->getID();
$group_1 = getItemByTypeName('Group', '_test_group_1')->getID();

// Creates Changes with different requesters
$this->createItems('Change', [
// Test set on requester
[
'name' => 'testAddOrderByUser user 1 (R)',
'content' => '',
'_actors' => [
'requester' => [['itemtype' => 'User', 'items_id' => $user_1]],
]
],
[
'name' => 'testAddOrderByUser user 2 (R)',
'content' => '',
'_actors' => [
'requester' => [['itemtype' => 'User', 'items_id' => $user_2]],
]
],
[
'name' => 'testAddOrderByUser user 1 (R) + user 2 (R)',
'content' => '',
'_actors' => [
'requester' => [
['itemtype' => 'User', 'items_id' => $user_1],
['itemtype' => 'User', 'items_id' => $user_2],
],
]
],
[
'name' => 'testAddOrderByUser anonymous user (R)',
'content' => '',
'_actors' => [
'requester' => [
[
'itemtype' => 'User',
'items_id' => 0,
"alternative_email" => "[email protected]",
'use_notification' => true
]
],
]
],
[
'name' => 'testAddOrderByUser group 1 (R)',
'content' => '',
'_actors' => [
'requester' => [['itemtype' => 'Group', 'items_id' => $group_1]],
]
],
[
'name' => 'testAddOrderByUser user 1 (R) + group 1 (R)',
'content' => '',
'_actors' => [
'requester' => [
['itemtype' => 'User', 'items_id' => $user_1],
['itemtype' => 'Group', 'items_id' => $group_1],
],
]
],
]);

yield [
'search_params' => [
'is_deleted' => 0,
'start' => 0,
'criteria[0][field]' => 1,
'criteria[0][searchtype]' => 'contains',
'criteria[0][value]' => 'testAddOrderByUser',
'sort' => 4,
'order' => 'ASC',
],
'expected_order' => [
'testAddOrderByUser group 1 (R)', // no requester
'testAddOrderByUser user 1 (R)', // _test_user
'testAddOrderByUser user 1 (R) + group 1 (R)', // _test_user
'testAddOrderByUser user 1 (R) + user 2 (R)', // _test_user, glpi
'testAddOrderByUser user 2 (R)', // glpi
'testAddOrderByUser anonymous user (R)', // [email protected]
]
];

yield [
'search_params' => [
'is_deleted' => 0,
'start' => 0,
'criteria[0][field]' => 1,
'criteria[0][searchtype]' => 'contains',
'criteria[0][value]' => 'testAddOrderByUser',
'sort' => 4,
'order' => 'DESC',
],
'expected_order' => [
'testAddOrderByUser anonymous user (R)', // [email protected]
'testAddOrderByUser user 2 (R)', // glpi
'testAddOrderByUser user 1 (R) + user 2 (R)', // _test_user, glpi
'testAddOrderByUser user 1 (R)', // _test_user
'testAddOrderByUser user 1 (R) + group 1 (R)', // _test_user
'testAddOrderByUser group 1 (R)', // no requester
]
];
}

/**
* @dataProvider testAddOrderByUserProvider
*/
public function testAddOrderByUser(
array $search_params,
array $expected_order
) {
$data = $this->doSearch('Change', $search_params);

// Extract items names
$items = [];
foreach ($data['data']['rows'] as $row) {
$items[] = $row['raw']['ITEM_Change_1'];
}

// Validate order
$this->array($items)->isEqualTo($expected_order);
}

private function cleanSQL($sql)
Expand Down Expand Up @@ -1991,7 +2149,11 @@ public function testSearchWithMultipleFkeysOnSameTable()
->contains("`glpi_users_users_id_recipient`.`id` = '{$user_normal_id}'")

// Check that ORDER applies on corresponding table alias
->contains("`glpi_users_users_id_recipient`.`name` ASC");
->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`, '')
)) ASC");
}

public function testSearchAllAssets()
Expand Down

0 comments on commit 7e05bc0

Please sign in to comment.