Add PaginationIterator constructor with custom method

zendesk · Nov 15, 2023 · 2ce8083 · 2ce8083
1 parent 7b61553
commit 2ce8083
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 21 deletions.
diff --git a/CBP_UPGRADE_GUIDE.md b/CBP_UPGRADE_GUIDE.md
@@ -1,5 +1,7 @@
 # OBP to CBP Upgrade guide
 
+TODO version requirement, how to bump.
+
 ## Useful links
 
 * [This README](./README.md#pagination)
@@ -16,7 +18,7 @@ CBP works based on cursors, so when ordering by `id` with page size 10, if the l
 
 ## API calls
 
-URL example:
+Note the query parameters change in these two URL examples:
 
 * 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
@@ -28,11 +30,6 @@ 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 might also change** with the pagination type:
 
 https://developer.zendesk.com/api-reference/ticketing/tickets/tickets/#sorting
@@ -56,7 +53,9 @@ If this is your situation, **you will need to change the sorting order** to a su
 
 ## The new iterator
 
-Please refer to the [README](./README.md#iterator-recommended).
+The best way to upgrade your app to support CBP is to use the iterator we provide. Please refer to the [README](./README.md#iterator-recommended).
+
+**Note**: The iterator will automatically convert these parameters to work with CBP: `page_size, sort_by, sort_order`.
 
 ## Parallel requests
 

diff --git a/README.md b/README.md
@@ -137,7 +137,7 @@ foreach ($iterator as $ticket) {
 }
 ```
 
-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).
+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)).
 
 ##### Iterator with params example
 
@@ -180,6 +180,21 @@ This can be useful for filter endpoints like [active automations](https://develo
 $iterator = $client->automations()->iterator($params, 'findActive');
 ```
 
+Which is analogous to:
+
+```php
+use Zendesk\API\Traits\Utility\Pagination\PaginationIterator;
+use Zendesk\API\Traits\Utility\Pagination\CbpStrategy;
+$strategy = new CbpStrategy('automations', $params);
+$iterator = new PaginationIterator(
+    $client->automations(),
+    $strategy,
+    'findActive'
+);
+```
+
+See how the [Pagination Trait](src/Zendesk/API/Traits/Resource/Pagination.php) is used if you need more custom implementations.
+
 ##### Catching API errors
 
 This doesn't change too much:

diff --git a/src/Zendesk/API/Traits/Resource/Pagination.php b/src/Zendesk/API/Traits/Resource/Pagination.php
@@ -15,12 +15,12 @@ trait Pagination {
      *
      * @return PaginationIterator to fetch all pages.
      */
-    public function iterator($params = [])
+    public function iterator($params = [], $method = 'findAll')
     {
         $strategyClass = $this->paginationStrategyClass();
         $strategy = new $strategyClass($this->resourcesKey(), $params);
 
-        return new PaginationIterator($this, $strategy);
+        return new PaginationIterator($this, $strategy, $method);
     }
 
     /**
@@ -33,6 +33,8 @@ protected function paginationStrategyClass() {
     }
 
     /**
+     * The key in the API responses where the resources are returned
+     *
      * @return string eg: "job_statuses"
      */
     protected function resourcesKey() {

diff --git a/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php b/src/Zendesk/API/Traits/Utility/Pagination/CbpStrategy.php
@@ -4,7 +4,7 @@
 
 /**
  * Cursor Based Pagination
- * Used in paginationStrategyClass
+ * Used in PaginationIterator
  */
 class CbpStrategy extends AbstractStrategy
 {
@@ -18,7 +18,7 @@ public function page($getPageFn)
         $this->latestResponse = $getPageFn();
         if (!isset($this->latestResponse->meta->has_more)) {
             throw new PaginationError(
-                "Response is not CBP, if you think your request is correct, please open an issue at https://github.com/zendesk/zendesk_api_client_php/issues"
+                "Response not conforming to the CBP format, if you think your request is correct, please open an issue at https://github.com/zendesk/zendesk_api_client_php/issues"
             );
         }
         $this->hasMore = $this->latestResponse->meta->has_more;

diff --git a/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php b/src/Zendesk/API/Traits/Utility/Pagination/PaginationIterator.php
@@ -21,34 +21,39 @@ class PaginationIterator implements Iterator
      * @param AbstractStrategy $strategy For pagination Logic (OBP, CBP, SinglePage)
      * @param string $method used to make the API call
      */
-    public function __construct($clientList, AbstractStrategy $strategy, $method = 'findAll')
+    public function __construct($clientList, AbstractStrategy $strategy, $method)
     {
         $this->clientList = $clientList;
         $this->strategy = $strategy;
         $this->method = $method;
     }
 
+    #[\ReturnTypeWillChange]
     public function key()
     {
         return $this->position;
     }
 
+    #[\ReturnTypeWillChange]
     public function next()
     {
         ++$this->position;
     }
 
+    #[\ReturnTypeWillChange]
     public function rewind()
     {
         $this->position = 0;
     }
 
+    #[\ReturnTypeWillChange]
     public function valid()
     {
         $this->getPageIfNeeded();
         return !!$this->current();
     }
 
+    #[\ReturnTypeWillChange]
     public function current()
     {
         if (isset($this->items[$this->position])) {

diff --git a/tests/Zendesk/API/UnitTests/Core/AutomationsTest.php b/tests/Zendesk/API/UnitTests/Core/AutomationsTest.php
@@ -47,6 +47,38 @@ public function testIterator()
         $this->assertEquals($this->testResource2['anyField'], $actual[2]->anyField);
     }
 
+    public function testIteratorFindActive()
+    {
+        // CBP
+        $this->mockApiResponses([
+            new Response(200, [], json_encode([
+                'automations' => [$this->testResource0, $this->testResource1],
+                'meta' => ['after_cursor' => '<after_cursor>', 'has_more' => true],
+
+            ])),
+            new Response(200, [], json_encode([
+                'automations' => [$this->testResource2],
+                'meta' => ['has_more' => false],
+
+            ])),
+        ]);
+
+        $iterator = $this->client->automations()->iterator([], 'findActive');
+
+        $actual = iterator_to_array($iterator);
+
+        $this->assertLastRequestIs(
+            [
+                'method'      => 'GET',
+                'endpoint'    => 'automations/active.json'
+            ]
+        );
+        $this->assertCount(3, $actual);
+        $this->assertEquals($this->testResource0['anyField'], $actual[0]->anyField);
+        $this->assertEquals($this->testResource1['anyField'], $actual[1]->anyField);
+        $this->assertEquals($this->testResource2['anyField'], $actual[2]->anyField);
+    }
+
     /**
      * Test we can use endpoint to get active automations
      */

diff --git a/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationIteratorTest.php b/tests/Zendesk/API/UnitTests/Traits/Utility/PaginationIteratorTest.php
@@ -80,7 +80,7 @@ public function testFetchesTickets()
             [['id' => 3], ['id' => 4]]
         ]);
         $strategy = new CbpStrategy('tickets', ['page[size]' => 2]);
-        $iterator = new PaginationIterator($mockTickets, $strategy);
+        $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
 
         $tickets = iterator_to_array($iterator);
 
@@ -95,7 +95,7 @@ public function testFetchesUsers()
             [['id' => 3, 'name' => 'User 3'], ['id' => 4, 'name' => 'User 4']]
         ]);
         $strategy = new CbpStrategy('users', ['page[size]' => 2]);
-        $iterator = new PaginationIterator($mockUsers, $strategy);
+        $iterator = new PaginationIterator($mockUsers, $strategy, 'findAll');
 
         $users = iterator_to_array($iterator);
 
@@ -114,7 +114,7 @@ public function testFetchesCbpWithParams()
             [['id' => 3], ['id' => 4]]
         ]);
         $strategy = new CbpStrategy('tickets', ['page[size]' => 2, 'any' => 'param']);
-        $iterator = new PaginationIterator($mockTickets, $strategy);
+        $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
 
         $tickets = iterator_to_array($iterator);
 
@@ -132,7 +132,7 @@ public function testCorrectsParamsToCbp()
             [['id' => 3], ['id' => 4]]
         ]);
         $strategy = new CbpStrategy('tickets', ['per_page' => 2, 'sort_by' => 'id', 'sort_order' => 'desc']);
-        $iterator = new PaginationIterator($mockTickets, $strategy);
+        $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
 
         iterator_to_array($iterator);
 
@@ -150,7 +150,7 @@ public function testFetchesSinglePageWithParams()
             [['id' => 1, 'name' => 'Resource 1'], ['id' => 2, 'name' => 'Resource 2']]
         ]);
         $strategy = new SinglePageStrategy($resultsKey, $userParams);
-        $iterator = new PaginationIterator($mockResults, $strategy);
+        $iterator = new PaginationIterator($mockResults, $strategy, 'findAll');
 
         $resources = iterator_to_array($iterator);
 
@@ -188,7 +188,7 @@ public function testHandlesError()
         $mockResults = new MockResource($resultsKey, []);
         $mockResults->errorMessage = $expectedErrorMessage;
         $strategy = new CbpStrategy($resultsKey, $userParams);
-        $iterator = new PaginationIterator($mockResults, $strategy);
+        $iterator = new PaginationIterator($mockResults, $strategy, 'findAll');
 
         try {
             iterator_to_array($iterator);
@@ -208,14 +208,17 @@ public function testErrorsForWrongPagination()
         ]);
         $mockTickets->isObp = true;
         $strategy = new CbpStrategy('tickets', ['page[size]' => 2]);
-        $iterator = new PaginationIterator($mockTickets, $strategy);
+        $iterator = new PaginationIterator($mockTickets, $strategy, 'findAll');
 
         try {
             iterator_to_array($iterator);
         } catch (PaginationError $e) {
             $error = $e;
         }
 
-        $this->assertEquals("Response is not CBP, if you think your request is correct, please open an issue at https://github.com/zendesk/zendesk_api_client_php/issues", $error->getMessage());
+        $this->assertEquals(
+            "Response not conforming to the CBP format, if you think your request is correct, please open an issue at https://github.com/zendesk/zendesk_api_client_php/issues",
+            $error->getMessage()
+        );
     }
 }