Fix sort roles by name (#5285)

fixes #4789
Icinga · Nov 5, 2024 · 9f64509 · 9f64509
2 parents eea50d7 + 8551fff
commit 9f64509
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 20 deletions.
diff --git a/application/controllers/RoleController.php b/application/controllers/RoleController.php
@@ -25,8 +25,6 @@
 
 /**
  * Manage user permissions and restrictions based on roles
- *
- * @TODO(el): Rename to RolesController: https://dev.icinga.com/issues/10015
  */
 class RoleController extends AuthBackendController
 {
@@ -53,8 +51,6 @@ public function indexAction()
 
     /**
      * List roles
-     *
-     * @TODO(el): Rename to indexAction()
      */
     public function listAction()
     {
@@ -63,22 +59,25 @@ public function listAction()
             ->select();
 
         $sortAndFilterColumns = [
-            'name'        => $this->translate('Name'),
-            'users'       => $this->translate('Users'),
-            'groups'      => $this->translate('Groups'),
-            'permissions' => $this->translate('Permissions')
+            'name'   => $this->translate('Name'),
+            'users'  => $this->translate('Users'),
+            'groups' => $this->translate('Groups'),
+            'parent' => $this->translate('Inherits From')
         ];
 
-        $this->setupFilterControl($this->view->roles, $sortAndFilterColumns, ['name']);
+        $this->setupFilterControl(
+            $this->view->roles,
+            $sortAndFilterColumns + [
+                'permissions' => $this->translate('Permissions')
+            ]
+        );
         $this->setupLimitControl();
         $this->setupPaginationControl($this->view->roles);
-        $this->setupSortControl($sortAndFilterColumns, $this->view->roles, ['name']);
+        $this->setupSortControl($sortAndFilterColumns, $this->view->roles);
     }
 
     /**
      * Create a new role
-     *
-     * @TODO(el): Rename to newAction()
      */
     public function addAction()
     {
@@ -93,8 +92,6 @@ public function addAction()
 
     /**
      * Update a role
-     *
-     * @TODO(el): Rename to updateAction()
      */
     public function editAction()
     {

diff --git a/library/Icinga/Authentication/RolesConfig.php b/library/Icinga/Authentication/RolesConfig.php
@@ -8,6 +8,8 @@
 
 class RolesConfig extends IniRepository
 {
+    protected $sortRules = ['name' => ['order' => 'asc']];
+
     protected $configs = [
         'roles' => [
             'name'      => 'roles',

diff --git a/library/Icinga/Data/DataArray/ArrayDatasource.php b/library/Icinga/Data/DataArray/ArrayDatasource.php
@@ -196,7 +196,16 @@ protected function createResult(SimpleQuery $query)
         $filter = $query->getFilter();
         $offset = $query->hasOffset() ? $query->getOffset() : 0;
         $limit = $query->hasLimit() ? $query->getLimit() : 0;
-        $data = $this->data;
+
+        $data = [];
+        foreach ($this->data as $key => $row) {
+            if ($this->keyColumn !== null && ! isset($row->{$this->keyColumn})) {
+                $row = clone $row; // Make sure that this won't affect the actual data
+                $row->{$this->keyColumn} = $key;
+            }
+
+            $data[$key] = $row;
+        }
 
         if ($query->hasOrder()) {
             uasort($data, [$query, 'compare']);
@@ -206,11 +215,6 @@ protected function createResult(SimpleQuery $query)
         $result = [];
         $skipped = 0;
         foreach ($data as $key => $row) {
-            if ($this->keyColumn !== null && !isset($row->{$this->keyColumn})) {
-                $row = clone $row; // Make sure that this won't affect the actual data
-                $row->{$this->keyColumn} = $key;
-            }
-
             if (! $filter->matches($row)) {
                 continue;
             } elseif ($skipped < $offset) {

diff --git a/test/php/library/Icinga/Data/DataArray/ArrayDatasourceTest.php b/test/php/library/Icinga/Data/DataArray/ArrayDatasourceTest.php
@@ -132,4 +132,31 @@ public function testOrderIsCorrectWithLimitAndOffset()
             'ArrayDatasource does not sort limited queries correctly'
         );
     }
+
+    public function testOrderByKeyColumnIsCorrect()
+    {
+        $result = (new ArrayDatasource([
+            'a' => (object) [
+                'foo' => 'bar',
+                'baz' => 'qux'
+            ],
+            'b' => (object) [
+                'foo' => 'bar',
+                'baz' => 'qux'
+            ],
+            'c' => (object) [
+                'foo' => 'bar',
+                'baz' => 'qux'
+            ]
+        ]))->setKeyColumn('name')
+            ->select()
+            ->order('name', 'desc')
+            ->fetchAll();
+
+        $this->assertSame(
+            ['c', 'b', 'a'],
+            array_keys($result),
+            'ArrayDatasource does not sort queries correctly by key column'
+        );
+    }
 }