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); } - }