fix(search): order by actor

[Rom1-B]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	!31353

For ticket searches, when sorting users by requester, the result did not take anonymous users into account.

The same applies to other types of actor and to changes and problems.

[AdrienClairembault]

Tested functionally and I confirm it works perfectly 👍

I am not sure about the tests part tho, it looks very technical as it is checking the exact expected SQL query.

I think having another test a bit more on the functional side (where we add a few actors on a few tickets and sort them using the search engine) would be a great addition.

[trasher]

I totally agree on the tests parts... Current tests does not really check the feature; but it also may hard to achieve properly (I think it's why it's currently done that way :/).

[AdrienClairembault]

The test could be simplified by using a data provider.
This way we can easily add new cases in the future :)

Suggestion:

diff --git a/tests/functional/Search.php b/tests/functional/Search.php
index a4d671940b..06bd0d13b5 100644
--- a/tests/functional/Search.php
+++ b/tests/functional/Search.php
@@ -1511,7 +1511,10 @@ class Search extends DbTestCase
                             )) DESC ");
     }
 
-    public function testAddOrderByUser()
+    /**
+     * Data provider for testAddOrderByUser
+     */
+    protected function testAddOrderByUserProvider(): iterable
     {
         $this->login('glpi', 'glpi');
 
@@ -1520,7 +1523,7 @@ class Search extends DbTestCase
         $group_1 = getItemByTypeName('Group', '_test_group_1')->getID();
 
         // Creates Changes with different requesters
-        $t = $this->createItems('Change', [
+        $this->createItems('Change', [
             // Test set on requester
             [
                 'name' => 'testAddOrderByUser user 1 (R)',
@@ -1579,105 +1582,64 @@ class Search extends DbTestCase
             ],
         ]);
 
-        // get the Changes filtered title containing 'testAddOrderByUser' and ordered by requester name in ASC
-        $search_params = [
-            'is_deleted' => 0,
-            'start' => 0,
-            'criteria[0][field]' => 1,
-            'criteria[0][searchtype]' => 'contains',
-            'criteria[0][value]' => 'testAddOrderByUser',
-            'sort' => 4,
-            'order' => 'ASC',
+        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)',       //  myemail@email.com
+            ]
         ];
-        $data = $this->doSearch('Change', $search_params);
 
-        $this->integer($data['data']['totalcount'])->isEqualTo(6);
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[0]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser group 1 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[1]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 1 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[2]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 1 (R) + group 1 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[3]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 1 (R) + user 2 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[4]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 2 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[5]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser anonymous user (R)');
-
-        // get the Changes filtered title containing 'testAddOrderByUser' and sort by requester name in DESC
-        $search_params = [
-            'is_deleted' => 0,
-            'start' => 0,
-            'criteria[0][field]' => 1,
-            'criteria[0][searchtype]' => 'contains',
-            'criteria[0][value]' => 'testAddOrderByUser',
-            'sort' => 4,
-            'order' => 'DESC',
+        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)',       //  myemail@email.com
+                '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);
 
-        $this->integer($data['data']['totalcount'])->isEqualTo(6);
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[0]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser anonymous user (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[1]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 2 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[2]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 1 (R) + user 2 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[3]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 1 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[4]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser user 1 (R) + group 1 (R)');
-        $this->array($data)
-            ->array['data']
-            ->array['rows']
-            ->array[5]
-            ->array['raw']
-            ->string['ITEM_Change_1']->isEqualTo('testAddOrderByUser group 1 (R)');
+        // 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)