Browse Source

Merge pull request #11193 from cakephp/issue-9791

Allow nth page to sort on multiple fields
Mark Story 8 years ago
parent
commit
8972cd0322

+ 7 - 3
src/Datasource/Paginator.php

@@ -215,6 +215,7 @@ class Paginator implements PaginatorInterface
             'sortDefault' => $sortDefault,
             'directionDefault' => $directionDefault,
             'scope' => $options['scope'],
+            'completeSort' => $order,
         ];
 
         $this->_pagingParams = [$alias => $paging];
@@ -323,7 +324,7 @@ class Paginator implements PaginatorInterface
      * the model friendly order key.
      *
      * You can use the whitelist parameter to control which columns/fields are
-     * available for sorting. This helps prevent users from ordering large
+     * available for sorting via URL parameters. This helps prevent users from ordering large
      * result sets on un-indexed values.
      *
      * If you need to sort on associated columns or synthetic properties you
@@ -333,6 +334,9 @@ class Paginator implements PaginatorInterface
      * You can use this to sort on synthetic columns, or columns added in custom
      * find operations that may not exist in the schema.
      *
+     * The default order options provided to paginate() will be merged with the user's
+     * requested sorting field/direction.
+     *
      * @param \Cake\Datasource\RepositoryInterface $object Repository object.
      * @param array $options The pagination options being used for this request.
      * @return array An array of options with sort + direction removed and
@@ -348,7 +352,8 @@ class Paginator implements PaginatorInterface
             if (!in_array($direction, ['asc', 'desc'])) {
                 $direction = 'asc';
             }
-            $options['order'] = [$options['sort'] => $direction];
+            $order = (isset($options['order']) && is_array($options['order'])) ? $options['order'] : [];
+            $options['order'] = [$options['sort'] => $direction] + $order;
         }
         unset($options['sort'], $options['direction']);
 
@@ -369,7 +374,6 @@ class Paginator implements PaginatorInterface
                 return $options;
             }
         }
-
         $options['order'] = $this->_prefix($object, $options['order'], $inWhitelist);
 
         return $options;

+ 1 - 0
tests/TestCase/Controller/Component/PaginatorComponentTest.php

@@ -934,6 +934,7 @@ class PaginatorComponentTest extends TestCase
             ]
         ];
         $result = $this->Paginator->validateSort($model, $options);
+
         $expected = [
             'model.author_id' => 'asc',
             'model.title' => 'asc'

+ 64 - 46
tests/TestCase/Datasource/PaginatorTest.php

@@ -683,11 +683,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortWhitelistFailure()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->any())->method('hasField')->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'sort' => 'body',
@@ -706,13 +702,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortWhitelistTrusted()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->once())
-            ->method('hasField')
-            ->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'sort' => 'body',
@@ -736,12 +726,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortWhitelistEmpty()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->any())->method('hasField')
-            ->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'order' => [
@@ -793,13 +778,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortWhitelistMultiple()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->once())
-            ->method('hasField')
-            ->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'order' => [
@@ -817,6 +796,19 @@ class PaginatorTest extends TestCase
         $this->assertEquals($expected, $result['order']);
     }
 
+    protected function mockAliasHasFieldModel()
+    {
+        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
+        $model->expects($this->any())
+            ->method('alias')
+            ->will($this->returnValue('model'));
+        $model->expects($this->any())
+            ->method('hasField')
+            ->will($this->returnValue(true));
+
+        return $model;
+    }
+
     /**
      * test that multiple sort works.
      *
@@ -824,11 +816,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortMultiple()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->any())->method('hasField')->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'order' => [
@@ -846,17 +834,56 @@ class PaginatorTest extends TestCase
     }
 
     /**
+     * test that multiple sort adds in query data.
+     *
+     * @return void
+     */
+    public function testValidateSortMultipleWithQuery()
+    {
+        $model = $this->mockAliasHasFieldModel();
+
+        $options = [
+            'sort' => 'created',
+            'direction' => 'desc',
+            'order' => [
+                'author_id' => 'asc',
+                'title' => 'asc'
+            ]
+        ];
+        $result = $this->Paginator->validateSort($model, $options);
+
+        $expected = [
+            'model.created' => 'desc',
+            'model.author_id' => 'asc',
+            'model.title' => 'asc'
+        ];
+        $this->assertEquals($expected, $result['order']);
+
+        $options = [
+            'sort' => 'title',
+            'direction' => 'desc',
+            'order' => [
+                'author_id' => 'asc',
+                'title' => 'asc'
+            ]
+        ];
+        $result = $this->Paginator->validateSort($model, $options);
+
+        $expected = [
+            'model.title' => 'desc',
+            'model.author_id' => 'asc',
+        ];
+        $this->assertEquals($expected, $result['order']);
+    }
+
+    /**
      * Tests that order strings can used by Paginator
      *
      * @return void
      */
     public function testValidateSortWithString()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->any())->method('hasField')->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'order' => 'model.author_id DESC'
@@ -874,12 +901,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortNoSort()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->any())->method('hasField')
-            ->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = [
             'direction' => 'asc',
@@ -896,11 +918,7 @@ class PaginatorTest extends TestCase
      */
     public function testValidateSortInvalidAlias()
     {
-        $model = $this->getMockBuilder('Cake\Datasource\RepositoryInterface')->getMock();
-        $model->expects($this->any())
-            ->method('alias')
-            ->will($this->returnValue('model'));
-        $model->expects($this->any())->method('hasField')->will($this->returnValue(true));
+        $model = $this->mockAliasHasFieldModel();
 
         $options = ['sort' => 'Derp.id'];
         $result = $this->Paginator->validateSort($model, $options);