Add CbpStrategy orderParams()

README.md

@@ -139,6 +139,8 @@ foreach ($ticketsIterator as $ticket) {
 }
 ```
 
+If you want to customise your sort order, please refer to the sorting section in the documentation ([Tickets, for example](https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting)). If you are upgrading from OBP, please refer to [the upgrade guide](./UPGRADE_GUIDE.md).
+
 #### Find All using CBP (fine)
 
 If you still want use `findAll()`, until CBP becomes the default API response, you must explicitly request CBP responses by using the param `page[size]`.

UPGRADE_GUIDE.md

@@ -0,0 +1,46 @@
+# Upgrade guide
+
+TODO: review and provide examples.
+
+## Gotchas moving from OBP to CBP
+
+### Useful links
+
+* [Pagination](https://developer.zendesk.com/api-reference/introduction/pagination)
+* [Ticketing sorting](https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting)
+
+### CBP ordering
+
+When moving from OBP to CBP sorting params change as well:
+
+* From: `?sort_name=updated_at&sort_order=desc`
+* To: `sort=-updated_at`
+
+Ticket example:
+
+* OBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort_order=desc&sort_by=updated_at&per_page=2
+* CBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort=-updated_at&page[size]=2
+
+However, the list of attributes you can sort by is longer in OBP:
+
+https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting
+
+* OBP: `"assignee", "assignee.name", "created_at", "group", "id", "locale", "requester"`
+* CBP: `"updated_at", "id", "status"`
+
+Example:
+
+* OBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort_order=desc&sort_by=assignee.name&per_page=2 HTTP 200, works
+* CBP: https://{subdomain}.zendesk.com/api/v2/tickets?sort=assignee.name&page[size]=2 HTTP 400
+
+```json
+{
+    "error": "InvalidPaginationParameter",
+    "description": "sort is not valid"
+}
+```
+
+TODO: confirm.
+If this is your situation, you need to change sorting order to a supported one.
+
+###

docker-compose.yml

@@ -7,8 +7,8 @@ services:
     volumes:
       - .:/app
       - vendor:/app/vendor
-    command: vendor/bin/phpunit --testsuite "Zendesk API Unit Test Suites"
-    # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
+    # command: vendor/bin/phpunit --testsuite "Zendesk API Unit Test Suites"
+    command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
     # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Core/TicketsTest.php
     # command: vendor/bin/phpunit tests/Zendesk/API/UnitTests/Core/OrganizationMembershipsTest.php
 

src/Zendesk/API/Traits/Resource/FindAll.php

@@ -3,7 +3,6 @@
 namespace Zendesk\API\Traits\Resource;
 
 use Zendesk\API\Exceptions\RouteException;
-use Zendesk\API\Traits\Utility\Pagination\CbpStrategy;
 
 trait FindAll
 {

src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php

@@ -17,6 +17,11 @@ public function __construct($resourcesKey, $pageSize = self::DEFAULT_PAGE_SIZE)
         $this->pageSize = $pageSize;
     }
 
+    public function orderParams($params)
+    {
+        return $params;
+    }
+
     abstract public function getPage($pageFn);
     abstract public function shouldGetPage($position);
 }

src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php

@@ -14,11 +14,8 @@ class CbpStrategy extends AbstractStrategy
     public function getPage($pageFn)
     {
         $this->started = true;
-        $params = ['page[size]' => $this->pageSize];
-        if ($this->afterCursor) {
-            $params['page[after]'] = $this->afterCursor;
-        }
-        $response = $pageFn($params);
+
+        $response = $pageFn($this->paginationParams());
 
         $this->afterCursor = $response->meta->has_more ? $response->meta->after_cursor : null;
         return $response->{$this->resourcesKey};
@@ -27,4 +24,26 @@ public function getPage($pageFn)
     public function shouldGetPage($position) {
         return !$this->started || $this->afterCursor;
     }
+
+    public function orderParams($params)
+    {
+        if (isset($params['sort']) || !isset($params['sort_by'])) {
+            return $params;
+        } else {
+            $direction = (isset($params['sort_order']) && strtolower($params['sort_order']) === 'desc') ? '-' : '';
+            $result = array_merge($params, ['sort' => $direction . $params['sort_by']]);
+            unset($result['sort_by'], $result['sort_order']);
+            return $result;
+        }
+    }
+    private function paginationParams()
+    {
+        // TODO: per_page...
+        $result = ['page[size]' => $this->pageSize];
+        if ($this->afterCursor) {
+            $result['page[after]'] = $this->afterCursor;
+        }
+
+        return $result;
+    }
 }

src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php

@@ -13,6 +13,7 @@ class PaginationIterator implements Iterator
 
     /**
      * @var mixed use trait FindAll. The object handling the list, Ie: `$client->{clientList}()`
+     * Eg: `$client->tickets()` which uses FindAll
      */
     private $clientList;
 
@@ -60,7 +61,11 @@ private function getPageIfNeeded()
         }
 
         $pageFn = function ($paginationParams = []) {
-            return $this->clientList->findAll(array_merge($this->params, $paginationParams));
+            return $this->clientList->findAll(
+                array_merge(
+                    $this->strategy->orderParams($this->params),
+                    $paginationParams
+                ));
         };
 
         $this->page = array_merge($this->page, $this->strategy->getPage($pageFn));

tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php

@@ -86,13 +86,31 @@ public function testFetchesCbpWithParams()
             [['id' => 3], ['id' => 4]]
         ]);
         $strategy = new CbpStrategy('tickets', 2);
-        $iterator = new PaginationIterator($mockTickets, $strategy, ['sort_name' => 'id', 'sort_order' => 'desc']);
+        $iterator = new PaginationIterator($mockTickets, $strategy, ['any' => 'param']);
 
         $tickets = iterator_to_array($iterator);
 
         $this->assertEquals([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]], $tickets);
         $this->assertEquals([
-            'sort_name' => 'id', 'sort_order' => 'desc',
+            'any' => 'param',
+            'page[size]' => 2, 'page[after]' => 'cursor_for_next_page'
+        ], $mockTickets->params);
+    }
+
+    public function testFetchesCbpCorrectingOrderParams()
+    {
+        $mockTickets = new MockResource('tickets', [
+            [['id' => 1], ['id' => 2]],
+            [['id' => 3], ['id' => 4]]
+        ]);
+        $strategy = new CbpStrategy('tickets', 2);
+        $iterator = new PaginationIterator($mockTickets, $strategy, ['sort_by' => 'id', 'sort_order' => 'desc']);
+
+        $tickets = iterator_to_array($iterator);
+
+        $this->assertEquals([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]], $tickets);
+        $this->assertEquals([
+            'sort' => '-id',
             'page[size]' => 2, 'page[after]' => 'cursor_for_next_page'
         ], $mockTickets->params);
     }