From 234e7b45fe8b64815e203fce1616d7969a7bd0cb Mon Sep 17 00:00:00 2001
From: "Erik Trapin (ecoologic)" <erik.trapin@zendesk.com>
Date: Thu, 2 Nov 2023 12:49:49 +1000
Subject: [PATCH 1/4] Add CbpStrategy orderParams()

---
 README.md                                     |  2 +
 UPGRADE_GUIDE.md                              | 46 +++++++++++++++++++
 docker-compose.yml                            |  4 +-
 src/Zendesk/API/Traits/Resource/FindAll.php   |  1 -
 .../Utility/Pagination/AbstractStrategy.php   |  5 ++
 .../Traits/Utility/Pagination/CbpStrategy.php | 29 ++++++++++--
 .../Utility/Pagination/PaginationIterator.php |  7 ++-
 .../Traits/Utility/PaginationTest.php         | 21 ++++++++-
 8 files changed, 104 insertions(+), 11 deletions(-)
 create mode 100644 UPGRADE_GUIDE.md

diff --git a/README.md b/README.md
index 343299ec..94c348ec 100755
--- a/README.md
+++ b/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]`.
diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md
new file mode 100644
index 00000000..a3a37722
--- /dev/null
+++ b/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.
+
+###
diff --git a/docker-compose.yml b/docker-compose.yml
index e1ac1d78..e5a7128a 100644
--- a/docker-compose.yml
+++ b/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
 
diff --git a/src/Zendesk/API/Traits/Resource/FindAll.php b/src/Zendesk/API/Traits/Resource/FindAll.php
index 1c826cc6..a4cda69a 100644
--- a/src/Zendesk/API/Traits/Resource/FindAll.php
+++ b/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
 {
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
index c6978570..bd516c4a 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
+++ b/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);
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
index 8828c987..0b477596 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
+++ b/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;
+    }
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
index 5919af17..86c9b22e 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
+++ b/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));
diff --git a/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php b/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
index 3cc70ab0..695a64aa 100644
--- a/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
+++ b/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
@@ -86,13 +86,30 @@ 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']);
+
+        iterator_to_array($iterator);
+
+        $this->assertEquals([
+            'sort' => '-id',
             'page[size]' => 2, 'page[after]' => 'cursor_for_next_page'
         ], $mockTickets->params);
     }

From 0ad78a276aa3e52ea096a48638481c9867ea8d92 Mon Sep 17 00:00:00 2001
From: "Erik Trapin (ecoologic)" <erik.trapin@zendesk.com>
Date: Fri, 3 Nov 2023 12:30:01 +1000
Subject: [PATCH 2/4] Add Pagination/AbstractStrategy params()

---
 docker-compose.yml                            |  4 +-
 .../API/Traits/Resource/Pagination.php        | 10 ++--
 .../Utility/Pagination/AbstractStrategy.php   | 28 ++++++---
 .../Traits/Utility/Pagination/CbpStrategy.php | 58 +++++++++++++------
 .../Traits/Utility/Pagination/ObpStrategy.php | 12 ++--
 .../Utility/Pagination/PaginationIterator.php | 25 ++++----
 .../Utility/Pagination/SinglePageStrategy.php |  4 +-
 .../Traits/Utility/PaginationTest.php         | 19 +++---
 8 files changed, 98 insertions(+), 62 deletions(-)

diff --git a/docker-compose.yml b/docker-compose.yml
index e5a7128a..e1ac1d78 100644
--- a/docker-compose.yml
+++ b/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
 
diff --git a/src/Zendesk/API/Traits/Resource/Pagination.php b/src/Zendesk/API/Traits/Resource/Pagination.php
index 7847c294..ef2ec50a 100644
--- a/src/Zendesk/API/Traits/Resource/Pagination.php
+++ b/src/Zendesk/API/Traits/Resource/Pagination.php
@@ -2,15 +2,15 @@
 
 namespace Zendesk\API\Traits\Resource;
 
-use Zendesk\API\Traits\Utility\Pagination\AbstractStrategy;
 use Zendesk\API\Traits\Utility\Pagination\CbpStrategy;
 use Zendesk\API\Traits\Utility\Pagination\PaginationIterator;
 
 trait Pagination {
     /**
      * Usage:
+     * $ticketsIterator = $client->tickets()->iterator();
      * foreach ($ticketsIterator as $ticket) {
-     *     process($ticket)
+     *     process($ticket);
      * }
      *
      * @return PaginationIterator to fetch all pages.
@@ -18,8 +18,9 @@ trait Pagination {
     public function iterator($params = [])
     {
         $strategyClass = $this->paginationStrategyClass();
-        $strategy = new $strategyClass($this->resourcesKey(), AbstractStrategy::DEFAULT_PAGE_SIZE);
-        return new PaginationIterator($this, $strategy, $params);
+        $strategy = new $strategyClass($this->resourcesKey(), $params);
+
+        return new PaginationIterator($this, $strategy);
     }
 
     /**
@@ -27,7 +28,6 @@ public function iterator($params = [])
      *
      * @return string subclass of AbstractStrategy used for fetching pages
      */
-
     protected function paginationStrategyClass() {
         return CbpStrategy::class;
     }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
index bd516c4a..b766d777 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
+++ b/src/Zendesk/API/Traits/Utility/Pagination/AbstractStrategy.php
@@ -3,25 +3,39 @@
 
 abstract class AbstractStrategy
 {
-    public const DEFAULT_PAGE_SIZE = 100;
-
     /**
      * @var string The response key where the data is returned
      */
     protected $resourcesKey;
+    protected $params;
     protected $pageSize;
 
-    public function __construct($resourcesKey, $pageSize = self::DEFAULT_PAGE_SIZE)
+    public function __construct($resourcesKey, $params)
     {
         $this->resourcesKey = $resourcesKey;
-        $this->pageSize = $pageSize;
+        $this->params = $params;
     }
 
-    public function orderParams($params)
+    public function params()
     {
-        return $params;
+        return $this->params;
+    }
+
+    protected function pageSize()
+    {
+        if (isset($this->pageSize)) {
+            return $this->pageSize;
+        } else if (isset($this->params['page[size]'])) {
+            $this->pageSize = $this->params['page[size]'];
+        } else if (isset($this->params['per_page'])) {
+            $this->pageSize = $this->params['per_page'];
+        } else {
+            $this->pageSize = DEFAULT_PAGE_SIZE;
+        }
+
+        return $this->pageSize;
     }
 
-    abstract public function getPage($pageFn);
+    abstract public function page($getPageFn);
     abstract public function shouldGetPage($position);
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
index 0b477596..c1e5213f 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
+++ b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
@@ -10,14 +10,14 @@ class CbpStrategy extends AbstractStrategy
 {
     private $afterCursor = null;
     private $started = false;
+    private $sortParams;
 
-    public function getPage($pageFn)
+    public function page($getPageFn)
     {
         $this->started = true;
-
-        $response = $pageFn($this->paginationParams());
-
+        $response = $getPageFn();
         $this->afterCursor = $response->meta->has_more ? $response->meta->after_cursor : null;
+
         return $response->{$this->resourcesKey};
     }
 
@@ -25,25 +25,49 @@ public function shouldGetPage($position) {
         return !$this->started || $this->afterCursor;
     }
 
-    public function orderParams($params)
+    public function params()
+    {
+        $result = array_merge($this->params, $this->paginationParams(), $this->sortParams());
+        $result = $this->unsetObpParams($result);
+
+        return $result;
+    }
+    /**
+     * The params that are needed to ordering in CBP (eg: ["sort" => "-age"])
+     * If OBP params are passed, they are converted to CBP
+     *
+     * @return array all params with CBP sorting order
+     */
+    private function sortParams()
     {
-        if (isset($params['sort']) || !isset($params['sort_by'])) {
-            return $params;
+        if (isset($this->params['sort_by']) && !isset($this->params['sort'])) {
+            $direction = (isset($this->params['sort_order']) && strtolower($this->params['sort_order']) === 'desc') ? '-' : '';
+            return array_merge($this->params, ['sort' => $direction . $this->params['sort_by']]);
         } 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;
+            return [];
         }
     }
+    /**
+     * The params that are needed to for pagination (eg: ["page[size]" => "100"])
+     * If OBP params are passed, they are converted to CBP
+     *
+     * @return array all params with CBP sorting order
+     */
     private function paginationParams()
     {
-        // TODO: per_page...
-        $result = ['page[size]' => $this->pageSize];
-        if ($this->afterCursor) {
-            $result['page[after]'] = $this->afterCursor;
-        }
+        $result = isset($this->afterCursor) ? ['page[after]' => $this->afterCursor] : [];
 
-        return $result;
+        return array_merge(['page[size]' => $this->pageSize()], $result);
+    }
+
+    private function unsetObpParams($params)
+    {
+        unset(
+            $params['page'],
+            $params['per_page'],
+            $params['sort_by'],
+            $params['sort_order']
+        );
+        return $params;
     }
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php
index b69183f0..52c3b20c 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php
+++ b/src/Zendesk/API/Traits/Utility/Pagination/ObpStrategy.php
@@ -10,16 +10,20 @@ class ObpStrategy extends AbstractStrategy
 {
     private $pageNumber = 0;
 
-    public function getPage($pageFn)
+    public function page($getPageFn)
     {
         ++$this->pageNumber;
-        $params = ['page' => $this->pageNumber, 'page_size' => $this->pageSize];
-        $response = $pageFn($params);
+        $response = $getPageFn();
 
         return $response->{$this->resourcesKey};
     }
 
     public function shouldGetPage($position) {
-        return $this->pageNumber == 0 || $position >= $this->pageNumber * $this->pageSize;
+        return $this->pageNumber == 0 || $position >= $this->pageNumber * $this->pageSize();
+    }
+    public function paramsWithPagination()
+    {
+        return ['page' => $this->pageNumber, 'per_page' => $this->pageSize()];
+
     }
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
index 86c9b22e..6feef83b 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
+++ b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
@@ -2,26 +2,25 @@
 
 namespace Zendesk\API\Traits\Utility\Pagination;
 
+const DEFAULT_PAGE_SIZE = 100;
+
 use Iterator;
 
 class PaginationIterator implements Iterator
 {
-    private $position = 0;
-    private $page = [];
-    private $strategy;
-    private $params;
-
     /**
-     * @var mixed use trait FindAll. The object handling the list, Ie: `$client->{clientList}()`
+     * @var mixed using trait FindAll. The object handling the list, Ie: `$client->{clientList}()`
      * Eg: `$client->tickets()` which uses FindAll
      */
     private $clientList;
+    private $strategy;
+    private $position = 0;
+    private $page = [];
 
-    public function __construct($clientList, AbstractStrategy $strategy, $params = [])
+    public function __construct($clientList, AbstractStrategy $strategy)
     {
         $this->clientList = $clientList;
         $this->strategy = $strategy;
-        $this->params = $params;
     }
 
     public function key()
@@ -60,14 +59,10 @@ private function getPageIfNeeded()
             return;
         }
 
-        $pageFn = function ($paginationParams = []) {
-            return $this->clientList->findAll(
-                array_merge(
-                    $this->strategy->orderParams($this->params),
-                    $paginationParams
-                ));
+        $getPageFn = function () {
+            return $this->clientList->findAll($this->strategy->params());
         };
 
-        $this->page = array_merge($this->page, $this->strategy->getPage($pageFn));
+        $this->page = array_merge($this->page, $this->strategy->page($getPageFn));
     }
 }
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/SinglePageStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/SinglePageStrategy.php
index 648213b9..819fade4 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/SinglePageStrategy.php
+++ b/src/Zendesk/API/Traits/Utility/Pagination/SinglePageStrategy.php
@@ -11,10 +11,10 @@ class SinglePageStrategy extends AbstractStrategy
 {
     protected $started = false;
 
-    public function getPage($pageFn)
+    public function page($getPageFn)
     {
         $this->started = true;
-        $response = $pageFn();
+        $response = $getPageFn();
 
         return $response->{$this->resourcesKey};
     }
diff --git a/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php b/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
index 695a64aa..1569be61 100644
--- a/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
+++ b/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationTest.php
@@ -52,7 +52,7 @@ public function testFetchesTickets()
             [['id' => 1], ['id' => 2]],
             [['id' => 3], ['id' => 4]]
         ]);
-        $strategy = new CbpStrategy('tickets', 2);
+        $strategy = new CbpStrategy('tickets', ['page[size]' => 2]);
         $iterator = new PaginationIterator($mockTickets, $strategy);
 
         $tickets = iterator_to_array($iterator);
@@ -66,7 +66,7 @@ public function testFetchesUsers()
             [['id' => 1, 'name' => 'User 1'], ['id' => 2, 'name' => 'User 2']],
             [['id' => 3, 'name' => 'User 3'], ['id' => 4, 'name' => 'User 4']]
         ]);
-        $strategy = new CbpStrategy('users', 2);
+        $strategy = new CbpStrategy('users', ['page[size]' => 2]);
         $iterator = new PaginationIterator($mockUsers, $strategy);
 
         $users = iterator_to_array($iterator);
@@ -85,8 +85,8 @@ public function testFetchesCbpWithParams()
             [['id' => 1], ['id' => 2]],
             [['id' => 3], ['id' => 4]]
         ]);
-        $strategy = new CbpStrategy('tickets', 2);
-        $iterator = new PaginationIterator($mockTickets, $strategy, ['any' => 'param']);
+        $strategy = new CbpStrategy('tickets', ['page[size]' => 2, 'any' => 'param']);
+        $iterator = new PaginationIterator($mockTickets, $strategy);
 
         $tickets = iterator_to_array($iterator);
 
@@ -97,14 +97,14 @@ public function testFetchesCbpWithParams()
         ], $mockTickets->params);
     }
 
-    public function testFetchesCbpCorrectingOrderParams()
+    public function testCorrectsParamsToCbp()
     {
         $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']);
+        $strategy = new CbpStrategy('tickets', ['per_page' => 2, 'sort_by' => 'id', 'sort_order' => 'desc']);
+        $iterator = new PaginationIterator($mockTickets, $strategy);
 
         iterator_to_array($iterator);
 
@@ -121,8 +121,8 @@ public function testFetchesSinglePageWithParams()
         $mockResults = new MockResource($resultsKey, [
             [['id' => 1, 'name' => 'Resource 1'], ['id' => 2, 'name' => 'Resource 2']]
         ]);
-        $strategy = new SinglePageStrategy($resultsKey);
-        $iterator = new PaginationIterator($mockResults, $strategy, $userParams);
+        $strategy = new SinglePageStrategy($resultsKey, $userParams);
+        $iterator = new PaginationIterator($mockResults, $strategy);
 
         $resources = iterator_to_array($iterator);
 
@@ -132,5 +132,4 @@ public function testFetchesSinglePageWithParams()
         ], $resources);
         $this->assertEquals($mockResults->params, $userParams);
     }
-
 }

From 01a87d6fa88d86d8964e5e2ed8491070bc7e56f8 Mon Sep 17 00:00:00 2001
From: "Erik Trapin (ecoologic)" <erik.trapin@zendesk.com>
Date: Fri, 3 Nov 2023 15:11:08 +1000
Subject: [PATCH 3/4] CBP Doc

---
 README.md                                     | 24 +++++--
 UPGRADE_GUIDE.md                              | 66 +++++++++++++++----
 .../Traits/Utility/Pagination/CbpStrategy.php |  4 +-
 3 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/README.md b/README.md
index 94c348ec..2dc64ae8 100755
--- a/README.md
+++ b/README.md
@@ -132,14 +132,30 @@ There are two ways to do pagination in the Zendesk API, **CBP (Cursor Based Pagi
 The use of the correct pagination is encapsulated using the iterator pattern, which allows you to retrieve all resources in all pages, without having to deal with pagination at all:
 
 ```php
-$ticketsIterator = $client->tickets()->iterator();
+$iterator = $client->tickets()->iterator();
 
-foreach ($ticketsIterator as $ticket) {
-    process($ticket); // Your implementation
+foreach ($iterator as $ticket) {
+    echo($ticket->id . " ");
 }
 ```
 
-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).
+If you want a specific 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).
+
+##### Iterator with params example
+
+```php
+$params = ['my' => 'param1', 'extra' => 'param2'];
+$iterator = $client->tickets()->iterator($params);
+
+foreach ($iterator as $ticket) {
+    echo($ticket->id . " ");
+}
+```
+
+* Change page size with: `$params = ['page[size]' => 5];`
+* Change sorting with: `$params = ['sort' => '-updated_at'];`
+  * Refer to the docs for details, including allowed sort fields
+* Combine everything: `$params = ['page[size]' => 2, 'sort' => 'updated_at', 'extra' => 'param'];`
 
 #### Find All using CBP (fine)
 
diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md
index a3a37722..7a6131c0 100644
--- a/UPGRADE_GUIDE.md
+++ b/UPGRADE_GUIDE.md
@@ -1,14 +1,25 @@
 # Upgrade guide
 
-TODO: review and provide examples.
-
-## Gotchas moving from OBP to CBP
-
-### Useful links
+## Useful links
 
 * [Pagination](https://developer.zendesk.com/api-reference/introduction/pagination)
 * [Ticketing sorting](https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting)
 
+## CBP Basics
+
+**OBP (Offset Based Pagination)** is quite inefficient, and increasingly so the higher the page you fetch. Switching to **CBP (Cursor Based Pagination)** will improve your application performance. OBP will eventually be subject to limits.
+
+When in OBP you request page 100, the DB will need to index all 100 pages of records before it can load the rows for the page you requested.
+
+CBP works based on a cursors, so if the order is `id`, 10 elements per page and the last item on your page is 111, the response includes a link to the next page "next 10 elements after id 111", and only your page is indexed before the rows are fetched.
+
+## API calls
+
+URL 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
+
 ### CBP ordering
 
 When moving from OBP to CBP sorting params change as well:
@@ -21,7 +32,7 @@ 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:
+However, the list of **attributes you can sort by might also change** with the pagination type:
 
 https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting
 
@@ -30,8 +41,8 @@ https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting
 
 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
+* 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
 {
@@ -40,7 +51,40 @@ Example:
 }
 ```
 
-TODO: confirm.
-If this is your situation, you need to change sorting order to a supported one.
+If this is your situation, **you need to change sorting order** to a supported one.
+
+## The new iterator
+
+Your best solution to implement CBP is to use the newly provided iterator on your resources, for example:
+
+```php
+$params = ['my' => 'param1', 'extra' => 'param2'];
+$iterator = $client->tickets()->iterator($params);
+
+foreach ($iterator as $ticket) {
+    echo($ticket->id . " ");
+}
+```
+
+This will choose the right type of pagination and adapt your parameters for pagination and ordering to work with CBP.
+
+##### Iterator with params example
+
+```php
+$params = ['my' => 'param1', 'extra' => 'param2'];
+$iterator = $client->tickets()->iterator($params);
+
+foreach ($iterator as $ticket) {
+    echo($ticket->id . " ");
+}
+```
+
+* Change page size with: `$params = ['page[size]' => 5];`
+* Change sorting with: `$params = ['sort' => '-updated_at'];`
+  * Refer to the docs for details, including allowed sort fields
+* Combine everything: `$params = ['page[size]' => 2, 'sort' => 'updated_at', 'extra' => 'param'];`
+
+
+## Parallel requests
 
-###
+If you are fetching multiple pages in parallel using OBP, you need to refactor to a serial execution, and fetch one page at a time.
diff --git a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
index c1e5213f..92bfa538 100644
--- a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
+++ b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
@@ -10,7 +10,6 @@ class CbpStrategy extends AbstractStrategy
 {
     private $afterCursor = null;
     private $started = false;
-    private $sortParams;
 
     public function page($getPageFn)
     {
@@ -36,6 +35,9 @@ public function params()
      * The params that are needed to ordering in CBP (eg: ["sort" => "-age"])
      * If OBP params are passed, they are converted to CBP
      *
+     * 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
+     *
      * @return array all params with CBP sorting order
      */
     private function sortParams()

From 902e7bc2e86a60ed8c55b7d02eed303eecedd2fc Mon Sep 17 00:00:00 2001
From: "Erik Trapin (ecoologic)" <erik.trapin@zendesk.com>
Date: Wed, 8 Nov 2023 15:47:47 +1000
Subject: [PATCH 4/4] Docs

---
 UPGRADE_GUIDE.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md
index 7a6131c0..6010fe9b 100644
--- a/UPGRADE_GUIDE.md
+++ b/UPGRADE_GUIDE.md
@@ -9,9 +9,9 @@
 
 **OBP (Offset Based Pagination)** is quite inefficient, and increasingly so the higher the page you fetch. Switching to **CBP (Cursor Based Pagination)** will improve your application performance. OBP will eventually be subject to limits.
 
-When in OBP you request page 100, the DB will need to index all 100 pages of records before it can load the rows for the page you requested.
+When using OBP, if you request page 100, the DB will need to index all records that proceed it before it can load the rows for the page you requested.
 
-CBP works based on a cursors, so if the order is `id`, 10 elements per page and the last item on your page is 111, the response includes a link to the next page "next 10 elements after id 111", and only your page is indexed before the rows are fetched.
+CBP works based on cursors, so when ordering by `id` with page size 10, if the last item on your page has id 111, the response includes a link to the next page (`links.next`) which can be used to request the next 10 elements after id 111, and only the requested rows are indexed before fetching. When in the response `meta.has_more` is `false` you are done.
 
 ## API calls
 
@@ -51,11 +51,11 @@ Example:
 }
 ```
 
-If this is your situation, **you need to change sorting order** to a supported one.
+If this is your situation, **you will need to change the sorting order** to a supported one.
 
 ## The new iterator
 
-Your best solution to implement CBP is to use the newly provided iterator on your resources, for example:
+The most efficient and elegant way to implement CBP is to use the newly provided iterator on your resources, for example:
 
 ```php
 $params = ['my' => 'param1', 'extra' => 'param2'];