Merge pull request #316 from EasyPost/fix_pagination

fix: pagination errors with user-supplied params
EasyPost · Nov 28, 2023 · 5e4c554 · 5e4c554
2 parents 5bcbc4b + bc5a28a
commit 5e4c554
Show file tree

Hide file tree

Showing 26 changed files with 552 additions and 478 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Next Major Release
 
 - Removed `withCarbonOffset` parameter from `create`, `buy`, and `regenerateRates` functions of the Shipment service as EasyPost now offers Carbon Neutral shipments by default for free
+- Fixes a bug where the original filtering criteria of `all` calls wasn't passed along to `getNextPage` calls. Now, these are persisted via a `_params` key on response objects locally
 
 ## v6.9.1 (2023-11-20)
 

diff --git a/lib/EasyPost/EasyPostClient.php b/lib/EasyPost/EasyPostClient.php
@@ -141,9 +141,13 @@ public function __get($serviceName)
         if (array_key_exists($serviceName, $serviceClassMap)) {
             return new $serviceClassMap[$serviceName]($this);
         } else {
-            throw new EasyPostException(
-                sprintf(Constants::UNDEFINED_PROPERTY_ERROR, 'EasyPostClient', $serviceName)
-            );
+            // TODO: checking for `_parent` is a hack and should be fixed when we revisit the
+            // (de)serialization of objects in this lib.
+            if ($serviceName != '_parent') {
+                throw new EasyPostException(
+                    sprintf(Constants::UNDEFINED_PROPERTY_ERROR, 'EasyPostClient', $serviceName)
+                );
+            }
         }
     }
 

diff --git a/lib/EasyPost/EasyPostObject.php b/lib/EasyPost/EasyPostObject.php
@@ -143,7 +143,12 @@ public static function constructFrom($client, $values, $class)
     public function convertEach($client, $values)
     {
         foreach ($values as $k => $v) {
-            $this->_values[$k] = InternalUtil::convertToEasyPostObject($client, $v);
+            // We don't want `_params` to become the default `EasyPostObject` since it needs to remain a normal array
+            if ($k == '_params') {
+                $this->_values[$k] = $v;
+            } else {
+                $this->_values[$k] = InternalUtil::convertToEasyPostObject($client, $v);
+            }
         }
     }
 

diff --git a/lib/EasyPost/Service/BaseService.php b/lib/EasyPost/Service/BaseService.php
@@ -3,14 +3,12 @@
 namespace EasyPost\Service;
 
 use EasyPost\Constant\Constants;
+use EasyPost\Exception\General\EndOfPaginationException;
 use EasyPost\Exception\General\InvalidObjectException;
 use EasyPost\Exception\General\InvalidParameterException;
-use EasyPost\Exception\General\EndOfPaginationException;
 use EasyPost\Http\Requestor;
 use EasyPost\Util\InternalUtil;
 
-use function PHPUnit\Framework\assertTrue;
-
 class BaseService
 {
     protected $client;
@@ -137,6 +135,9 @@ protected function allResources($class, $params = null, $beta = false)
         self::validate($params);
         $url = self::classUrl($class);
         $response = Requestor::request($this->client, 'get', $url, $params, $beta);
+        if (isset($params)) {
+            $response['_params'] = $params;
+        }
 
         return InternalUtil::convertToEasyPostObject($this->client, $response);
     }
@@ -148,13 +149,13 @@ protected function allResources($class, $params = null, $beta = false)
      * @param string $class
      * @param mixed $collection
      * @param int $pageSize
-     * @param mixed $optionalParams
      * @return mixed
      */
-    protected function getNextPageResources($class, $collection, $pageSize = null, $optionalParams = null)
+    protected function getNextPageResources($class, $collection, $pageSize = null)
     {
         $objectName = substr(self::classUrl($class), 1);
         $collectionArray = $collection[$objectName];
+        $userParams = $collection['_params'] ?? null;
 
         if (empty($collectionArray) || !$collection['has_more']) {
             throw new EndOfPaginationException();
@@ -165,13 +166,17 @@ protected function getNextPageResources($class, $collection, $pageSize = null, $
             'before_id' => $collectionArray[count($collectionArray) - 1]['id']
         ];
 
-        if (isset($optionalParams)) {
-            $params = array_merge($params, $optionalParams);
+        if (isset($userParams)) {
+            $params = array_merge($params, $userParams);
         }
 
         $response = $this->allResources($class, $params);
 
-        if (empty($response[$objectName]) || !$response['has_more']) {
+        if (isset($userParams)) {
+            $response['_params'] = $userParams;
+        }
+
+        if (empty($response[$objectName])) {
             throw new EndOfPaginationException();
         }
 

diff --git a/lib/EasyPost/Service/ReportService.php b/lib/EasyPost/Service/ReportService.php
@@ -3,8 +3,8 @@
 namespace EasyPost\Service;
 
 use EasyPost\Constant\Constants;
-use EasyPost\Exception\General\MissingParameterException;
 use EasyPost\Exception\General\EndOfPaginationException;
+use EasyPost\Exception\General\MissingParameterException;
 use EasyPost\Http\Requestor;
 use EasyPost\Util\InternalUtil;
 
@@ -57,6 +57,7 @@ public function all($params = null)
     public function getNextPage($reports, $pageSize = null)
     {
         $reportArray = $reports['reports'];
+        $userParams = $reports['_params'] ?? null;
 
         if (empty($reportArray) || !$reports['has_more']) {
             throw new EndOfPaginationException();
@@ -66,10 +67,19 @@ public function getNextPage($reports, $pageSize = null)
             'page_size' => $pageSize,
             'before_id' => $reportArray[count($reportArray) - 1]['id']
         ];
-        $url = self::reportUrl($reports->type);
+
+        if (isset($userParams)) {
+            $params = array_merge($params, $userParams);
+        }
+
+        $url = self::reportUrl($reportArray[0]->object);
         $response = Requestor::request($this->client, 'get', $url, $params);
 
-        if (empty($response['reports']) || !$response['has_more']) {
+        if (isset($userParams)) {
+            $response['_params'] = $userParams;
+        }
+
+        if (empty($response['reports'])) {
             throw new EndOfPaginationException();
         }
 

diff --git a/lib/EasyPost/Service/ShipmentService.php b/lib/EasyPost/Service/ShipmentService.php
@@ -30,12 +30,7 @@ public function retrieve($id)
      */
     public function all($params = null)
     {
-        self::validate($params);
-        $response = Requestor::request($this->client, 'get', '/shipments', $params);
-        $response['purchased'] = $params['purchased'] ?? null;
-        $response['include_children'] = $params['include_children'] ?? null;
-
-        return InternalUtil::convertToEasyPostObject($this->client, $response);
+        return self::allResources(self::serviceModelClassName(self::class), $params);
     }
 
     /**
@@ -47,16 +42,7 @@ public function all($params = null)
      */
     public function getNextPage($shipments, $pageSize = null)
     {
-        $params = [];
-
-        if (isset($shipments->purchased)) {
-            $params['purchased'] = $shipments->purchased;
-        }
-
-        if (isset($shipments->include_children)) {
-            $params['include_children'] = $shipments->include_children;
-        }
-        return $this->getNextPageResources(self::serviceModelClassName(self::class), $shipments, $pageSize, $params);
+        return $this->getNextPageResources(self::serviceModelClassName(self::class), $shipments, $pageSize);
     }
 
     /**

diff --git a/lib/EasyPost/Service/TrackerService.php b/lib/EasyPost/Service/TrackerService.php
@@ -29,12 +29,7 @@ public function retrieve($id)
      */
     public function all($params = null)
     {
-        self::validate($params);
-        $response = Requestor::request($this->client, 'get', '/trackers', $params);
-        $response['tracking_code'] = $params['tracking_code'] ?? null;
-        $response['carrier'] = $params['carrier'] ?? null;
-
-        return InternalUtil::convertToEasyPostObject($this->client, $response);
+        return self::allResources(self::serviceModelClassName(self::class), $params);
     }
 
     /**
@@ -46,16 +41,7 @@ public function all($params = null)
      */
     public function getNextPage($trackers, $pageSize = null)
     {
-        $params = [];
-
-        if (isset($trackers->tracking_code)) {
-            $params['tracking_code'] = $trackers->tracking_code;
-        }
-
-        if (isset($trackers->carrier)) {
-            $params['carrier'] = $trackers->carrier;
-        }
-        return $this->getNextPageResources(self::serviceModelClassName(self::class), $trackers, $pageSize, $params);
+        return $this->getNextPageResources(self::serviceModelClassName(self::class), $trackers, $pageSize);
     }
 
     /**

diff --git a/test/EasyPost/AddressTest.php b/test/EasyPost/AddressTest.php
@@ -148,11 +148,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['addresses'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 

diff --git a/test/EasyPost/EventTest.php b/test/EasyPost/EventTest.php
@@ -6,9 +6,9 @@
 use EasyPost\Event;
 use EasyPost\Exception\General\EasyPostException;
 use EasyPost\Exception\General\EndOfPaginationException;
-use Exception;
 use EasyPost\Payload;
 use EasyPost\Util\Util;
+use Exception;
 
 class EventTest extends \PHPUnit\Framework\TestCase
 {
@@ -66,11 +66,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['events'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 

diff --git a/test/EasyPost/InsuranceTest.php b/test/EasyPost/InsuranceTest.php
@@ -101,11 +101,12 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['insurances'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+            $this->assertNotNull($nextPage['_params']);
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 }
diff --git a/test/EasyPost/PickupTest.php b/test/EasyPost/PickupTest.php
@@ -3,10 +3,10 @@
 namespace EasyPost\Test;
 
 use EasyPost\EasyPostClient;
-use EasyPost\Exception\General\FilteringException;
 use EasyPost\Exception\General\EndOfPaginationException;
-use Exception;
+use EasyPost\Exception\General\FilteringException;
 use EasyPost\Pickup;
+use Exception;
 
 class PickupTest extends \PHPUnit\Framework\TestCase
 {
@@ -102,11 +102,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['pickups'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 

diff --git a/test/EasyPost/ReferralCustomerTest.php b/test/EasyPost/ReferralCustomerTest.php
@@ -86,11 +86,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['referral_customers'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 

diff --git a/test/EasyPost/RefundTest.php b/test/EasyPost/RefundTest.php
@@ -86,11 +86,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['refunds'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 

diff --git a/test/EasyPost/ReportTest.php b/test/EasyPost/ReportTest.php
@@ -3,10 +3,10 @@
 namespace EasyPost\Test;
 
 use EasyPost\EasyPostClient;
-use EasyPost\Exception\General\MissingParameterException;
 use EasyPost\Exception\General\EndOfPaginationException;
-use Exception;
+use EasyPost\Exception\General\MissingParameterException;
 use EasyPost\Report;
+use Exception;
 
 class ReportTest extends \PHPUnit\Framework\TestCase
 {
@@ -143,11 +143,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['reports'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 

diff --git a/test/EasyPost/ScanFormTest.php b/test/EasyPost/ScanFormTest.php
@@ -3,8 +3,8 @@
 namespace EasyPost\Test;
 
 use EasyPost\EasyPostClient;
-use EasyPost\ScanForm;
 use EasyPost\Exception\General\EndOfPaginationException;
+use EasyPost\ScanForm;
 use Exception;
 
 class ScanFormTest extends \PHPUnit\Framework\TestCase
@@ -99,11 +99,11 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['scan_forms'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }
 }
diff --git a/test/EasyPost/ShipmentTest.php b/test/EasyPost/ShipmentTest.php
@@ -99,11 +99,12 @@ public function testGetNextPage()
             $secondIdOfSecondPage = $nextPage['shipments'][0]->id;
 
             $this->assertNotEquals($firstIdOfFirstPage, $secondIdOfSecondPage);
-        } catch (Exception $error) {
-            if (!($error instanceof EndOfPaginationException)) {
-                throw new Exception('Test failed intentionally');
-            }
+            $this->assertNotNull($nextPage['_params']);
+        } catch (EndOfPaginationException $error) {
+            // There's no second page, that's not a failure
             $this->assertTrue(true);
+        } catch (Exception $error) {
+            throw $error;
         }
     }