From 6a62d17a590b2ba257bce82fdad5717ad76a5ec2 Mon Sep 17 00:00:00 2001 From: Pereyaslov Konstantin Date: Sun, 7 Dec 2014 12:21:53 +0300 Subject: [PATCH 1/7] Duplicate code removed --- src/USPS/Rate.php | 4 ---- 1 file changed, 4 deletions(-) mode change 100644 => 100755 src/USPS/Rate.php diff --git a/src/USPS/Rate.php b/src/USPS/Rate.php old mode 100644 new mode 100755 index 33b43fe..a664d5a --- a/src/USPS/Rate.php +++ b/src/USPS/Rate.php @@ -78,10 +78,6 @@ class Rate extends RateAdapter $this->password = $options['password']; } - if (isset($options['username'])) { - $this->username = $options['username']; - } - if (isset($options['approved_codes'])) { $this->approved_codes = $options['approved_codes']; } From bfc91cd7300c669e3c324f0d2dccb891e9378beb Mon Sep 17 00:00:00 2001 From: Pereyaslov Konstantin Date: Sun, 7 Dec 2014 12:30:12 +0300 Subject: [PATCH 2/7] Determine shipment size automatically for USPS --- src/USPS/Rate.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) mode change 100755 => 100644 src/USPS/Rate.php diff --git a/src/USPS/Rate.php b/src/USPS/Rate.php old mode 100755 new mode 100644 index a664d5a..e9dbc35 --- a/src/USPS/Rate.php +++ b/src/USPS/Rate.php @@ -103,6 +103,22 @@ class Rate extends RateAdapter throw new Exception('Weight missing'); } + $size = Arr::get($this->shipment, 'size'); + + // If user has not specified size, determine it automatically + // https://www.usps.com/business/web-tools-apis/rate-calculator-api.htm#_Toc378922331 + if ($size === null) { + // Size is considered large if any dimension is larger than 12 inches + foreach ($dimensions as $dimension) { + if ($dimension > 12) { + $size = 'LARGE'; + break; + } + } + if (!isset($size)) { + $size = 'REGULAR'; + } + } $this->data = ' @@ -113,7 +129,7 @@ class Rate extends RateAdapter ' . $pounds . ' ' . $ounces . ' ' . Arr::get($this->shipment, 'container') . ' - ' . Arr::get($this->shipment, 'size') . ' + ' . $size . ' ' . Arr::get($dimensions, 'width') . ' ' . Arr::get($dimensions, 'length') . ' ' . Arr::get($dimensions, 'height') . ' From 5dafdf17a0625a4ce0e00d2c1d3f8b5ddf3f5eec Mon Sep 17 00:00:00 2001 From: Pereyaslov Konstantin Date: Mon, 8 Dec 2014 18:56:58 +0300 Subject: [PATCH 3/7] broken use removed --- src/UPS/Rate.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/UPS/Rate.php b/src/UPS/Rate.php index 0e98561..b3e68cd 100644 --- a/src/UPS/Rate.php +++ b/src/UPS/Rate.php @@ -1,7 +1,6 @@ Date: Mon, 8 Dec 2014 19:09:30 +0300 Subject: [PATCH 4/7] Added validation that all necessary properties are set --- src/Fedex/Rate.php | 29 ++++++++++++++++++++++++----- src/RateAdapter.php | 5 +++++ src/UPS/Rate.php | 28 ++++++++++++++++++++++++---- src/USPS/Rate.php | 18 ++++++++++++++++-- src/Validator.php | 10 ++++++++++ 5 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 src/Validator.php diff --git a/src/Fedex/Rate.php b/src/Fedex/Rate.php index bc4ba01..33fda6f 100644 --- a/src/Fedex/Rate.php +++ b/src/Fedex/Rate.php @@ -7,6 +7,7 @@ use pdt256\Shipping\Arr; use pdt256\Shipping\Quote; use pdt256\Shipping\RateAdapter; use pdt256\Shipping\RateRequest; +use pdt256\Shipping\Validator; use DOMDocument; use Exception; @@ -15,10 +16,10 @@ class Rate extends RateAdapter private $urlDev = 'https://gatewaybeta.fedex.com/web-services/'; private $urlProd = 'https://gateway.fedex.com/web-services/'; - private $key = 'XXX'; - private $password = 'XXX'; - private $accountNumber = 'XXX'; - private $meterNumber = 'XXX'; + private $key; + private $password; + private $accountNumber; + private $meterNumber; private $dropOffType = 'BUSINESS_SERVICE_CENTER'; public $approvedCodes = [ @@ -66,7 +67,26 @@ class Rate extends RateAdapter $this->setRequestAdapter(Arr::get($options, 'requestAdapter', new RateRequest\Post())); } + protected function validate() + { + foreach ($this->shipment->getPackages() as $package) { + Validator::checkIfNull($package->getWeight(), 'weight'); + Validator::checkIfNull($package->getLength(), 'length'); + Validator::checkIfNull($package->getHeight(), 'height'); + } + Validator::checkIfNull($this->key, 'key'); + Validator::checkIfNull($this->password, 'password'); + Validator::checkIfNull($this->accountNumber, 'accountNumber'); + Validator::checkIfNull($this->meterNumber, 'meterNumber'); + Validator::checkIfNull($this->shipment->getFromPostalCode(), 'fromPostalCode'); + Validator::checkIfNull($this->shipment->getFromCountryCode(), 'fromCountryCode'); + Validator::checkIfNull($this->shipment->getFromIsResidential(), 'fromIsResidential'); + Validator::checkIfNull($this->shipment->getToPostalCode(), 'toPostalCode'); + Validator::checkIfNull($this->shipment->getToCountryCode(), 'toCountryCode'); + Validator::checkIfNull($this->shipment->getToIsResidential(), 'toIsResidential'); + return $this; + } protected function prepare() { $date = time(); @@ -103,7 +123,6 @@ class Rate extends RateAdapter '' . ''; } - $this->data = '' . '' . diff --git a/src/RateAdapter.php b/src/RateAdapter.php index 0a334cd..b4b5995 100644 --- a/src/RateAdapter.php +++ b/src/RateAdapter.php @@ -16,6 +16,10 @@ abstract class RateAdapter /** @var @var RateRequest\Adapter */ protected $rateRequest; + /** + * Make sure all necessary fields are set + */ + abstract protected function validate(); /** * Prepare XML */ @@ -47,6 +51,7 @@ abstract class RateAdapter public function getRates() { $this + ->validate() ->prepare() ->execute() ->process() diff --git a/src/UPS/Rate.php b/src/UPS/Rate.php index b3e68cd..a366688 100644 --- a/src/UPS/Rate.php +++ b/src/UPS/Rate.php @@ -5,6 +5,7 @@ use pdt256\Shipping\Arr; use pdt256\Shipping\Quote; use pdt256\Shipping\RateAdapter; use pdt256\Shipping\RateRequest; +use pdt256\Shipping\Validator; use DOMDocument; use Exception; @@ -13,10 +14,10 @@ class Rate extends RateAdapter private $urlDev = 'https://wwwcie.ups.com/ups.app/xml/Rate'; private $urlProd = 'https://www.ups.com/ups.app/xml/Rate'; - private $accessKey = 'XXX'; - private $userId = 'XXX'; - private $password = 'XXX'; - private $shipperNumber = 'XXX'; + private $accessKey; + private $userId; + private $password; + private $shipperNumber; public $approvedCodes = [ '03', @@ -94,7 +95,26 @@ class Rate extends RateAdapter $this->setRequestAdapter(Arr::get($options, 'requestAdapter', new RateRequest\Post())); } + protected function validate() + { + foreach ($this->shipment->getPackages() as $package) { + Validator::checkIfNull($package->getWeight(), 'weight'); + Validator::checkIfNull($package->getLength(), 'length'); + Validator::checkIfNull($package->getHeight(), 'height'); + } + Validator::checkIfNull($this->accessKey, 'accessKey'); + Validator::checkIfNull($this->userId, 'userId'); + Validator::checkIfNull($this->password, 'password'); + Validator::checkIfNull($this->shipperNumber, 'shipperNumber'); + Validator::checkIfNull($this->shipment->getFromPostalCode(), 'fromPostalCode'); + Validator::checkIfNull($this->shipment->getFromCountryCode(), 'fromCountryCode'); + Validator::checkIfNull($this->shipment->getFromIsResidential(), 'fromIsResidential'); + Validator::checkIfNull($this->shipment->getToPostalCode(), 'toPostalCode'); + Validator::checkIfNull($this->shipment->getToCountryCode(), 'toCountryCode'); + Validator::checkIfNull($this->shipment->getToIsResidential(), 'toIsResidential'); + return $this; + } protected function prepare() { $service_code = '03'; diff --git a/src/USPS/Rate.php b/src/USPS/Rate.php index f5e4887..91e44bf 100644 --- a/src/USPS/Rate.php +++ b/src/USPS/Rate.php @@ -6,6 +6,7 @@ use pdt256\Shipping\Arr; use pdt256\Shipping\Quote; use pdt256\Shipping\RateAdapter; use pdt256\Shipping\RateRequest; +use pdt256\Shipping\Validator; use DOMDocument; use Exception; @@ -14,8 +15,8 @@ class Rate extends RateAdapter private $urlDev = 'http://production.shippingapis.com/ShippingAPI.dll'; private $urlProd = 'http://production.shippingapis.com/ShippingAPI.dll'; - private $username = 'XXX'; - private $password = 'XXX'; + private $username; + private $password; public $approvedCodes = [ '1', @@ -76,7 +77,20 @@ class Rate extends RateAdapter $this->approvedCodes = Arr::get($options, 'approvedCodes'); $this->setRequestAdapter(Arr::get($options, 'requestAdapter', new RateRequest\Get())); } + protected function validate() + { + foreach ($this->shipment->getPackages() as $package) { + Validator::checkIfNull($package->getWeight(), 'weight'); + Validator::checkIfNull($package->getLength(), 'length'); + Validator::checkIfNull($package->getHeight(), 'height'); + } + Validator::checkIfNull($this->username, 'username'); + Validator::checkIfNull($this->password, 'password'); + Validator::checkIfNull($this->shipment->getFromPostalCode(), 'fromPostalCode'); + Validator::checkIfNull($this->shipment->getToPostalCode(), 'toPostalCode'); + return $this; + } protected function prepare() { $packages = ''; diff --git a/src/Validator.php b/src/Validator.php new file mode 100644 index 0000000..ed697f6 --- /dev/null +++ b/src/Validator.php @@ -0,0 +1,10 @@ + Date: Mon, 8 Dec 2014 19:29:23 +0300 Subject: [PATCH 5/7] Setting all options in tests --- tests/Fedex/FedexTest.php | 8 ++++++-- tests/ShipTest.php | 7 ++++--- tests/UPS/RateTest.php | 5 +++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/Fedex/FedexTest.php b/tests/Fedex/FedexTest.php index 01012ba..e68b82f 100644 --- a/tests/Fedex/FedexTest.php +++ b/tests/Fedex/FedexTest.php @@ -49,6 +49,7 @@ class RateTest extends \PHPUnit_Framework_TestCase $this->shipment->setFromStateProvinceCode('CA') ->setFromPostalCode('90401') ->setFromCountryCode('US') + ->setFromIsResidential(true) ->setToPostalCode('78703') ->setToCountryCode('US') ->setToIsResidential(true) @@ -59,12 +60,15 @@ class RateTest extends \PHPUnit_Framework_TestCase { $rateAdapter = new Rate([ 'prod' => false, - 'drop_off_type' => 'BUSINESS_SERVICE_CENTER', + 'key' => 'XXX', + 'password' => 'XXX', + 'accountNumber' => 'XXX', + 'meterNumber' => 'XXX', + 'dropOffType' => 'BUSINESS_SERVICE_CENTER', 'shipment' => $this->shipment, 'approvedCodes' => $this->approvedCodes, 'requestAdapter' => new StubFedex, ]); - $rates = $rateAdapter->getRates(); $ground = new Quote('fedex', 'GROUND_HOME_DELIVERY', 'Ground Home Delivery', 1655); diff --git a/tests/ShipTest.php b/tests/ShipTest.php index 7b8bc10..94175b3 100644 --- a/tests/ShipTest.php +++ b/tests/ShipTest.php @@ -49,6 +49,7 @@ class ShipTest extends \PHPUnit_Framework_TestCase $s->setFromStateProvinceCode('CA') ->setFromPostalCode('90401') ->setFromCountryCode('US') + ->setFromIsResidential(true) ->setToPostalCode('78703') ->setToCountryCode('US') ->setToIsResidential(true); @@ -105,9 +106,9 @@ class ShipTest extends \PHPUnit_Framework_TestCase 'prod' => false, 'key' => 'XXXX', 'password' => 'XXXX', - 'account_number' => 'XXXX', - 'meter_number' => 'XXXX', - 'drop_off_type' => 'BUSINESS_SERVICE_CENTER', + 'accountNumber' => 'XXXX', + 'meterNumber' => 'XXXX', + 'dropOffType' => 'BUSINESS_SERVICE_CENTER', 'shipment' => $this->shipment, 'approvedCodes' => $approvedCodes, 'requestAdapter' => new RateRequest\StubFedex(), diff --git a/tests/UPS/RateTest.php b/tests/UPS/RateTest.php index e16e015..4edf4ad 100644 --- a/tests/UPS/RateTest.php +++ b/tests/UPS/RateTest.php @@ -48,6 +48,7 @@ class RateTest extends \PHPUnit_Framework_TestCase $this->shipment->setFromStateProvinceCode('CA') ->setFromPostalCode('90401') ->setFromCountryCode('US') + ->setFromIsResidential(true) ->setToPostalCode('78703') ->setToCountryCode('US') ->setToIsResidential(true) @@ -57,6 +58,10 @@ class RateTest extends \PHPUnit_Framework_TestCase public function testMockRates() { $rateAdapter = new Rate([ + 'accessKey' => 'XXX', + 'userId' => 'XXX', + 'password' => 'XXX', + 'shipperNumber' => 'XXX', 'prod' => false, 'shipment' => $this->shipment, 'approvedCodes' => $this->approvedCodes, From 2938be5a2dcae4c5c94c108fd8c6b4137371aab5 Mon Sep 17 00:00:00 2001 From: Pereyaslov Konstantin Date: Mon, 8 Dec 2014 19:34:31 +0300 Subject: [PATCH 6/7] Moving default values to __construct, because otherwise they get overwritten --- src/Fedex/Rate.php | 27 ++++++++++++++++----------- src/UPS/Rate.php | 14 ++++++++------ src/USPS/Rate.php | 14 ++++++++------ 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/Fedex/Rate.php b/src/Fedex/Rate.php index 33fda6f..82f8dad 100644 --- a/src/Fedex/Rate.php +++ b/src/Fedex/Rate.php @@ -20,15 +20,14 @@ class Rate extends RateAdapter private $password; private $accountNumber; private $meterNumber; - private $dropOffType = 'BUSINESS_SERVICE_CENTER'; - - public $approvedCodes = [ - 'PRIORITY_OVERNIGHT', - 'FEDEX_2_DAY', - 'FEDEX_EXPRESS_SAVER', - 'FEDEX_GROUND', - 'GROUND_HOME_DELIVERY', - ]; + /** + * Type of Drop off, default value "BUSINESS_SERVICE_CENTER" is defined in __construct if not specified. + */ + private $dropOffType; + /** + * Codes of appropriate shipping types. Default value is specified in __construct. + */ + public $approvedCodes; private $shippingCodes = [ 'EUROPE_FIRST_INTERNATIONAL_PRIORITY' => 'Europe First International Priority', @@ -62,8 +61,14 @@ class Rate extends RateAdapter $this->password = Arr::get($options, 'password'); $this->accountNumber = Arr::get($options, 'accountNumber'); $this->meterNumber = Arr::get($options, 'meterNumber'); - $this->approvedCodes = Arr::get($options, 'approvedCodes'); - $this->dropOffType = Arr::get($options, 'dropOffType'); + $this->approvedCodes = Arr::get($options, 'approvedCodes', [ + 'PRIORITY_OVERNIGHT', + 'FEDEX_2_DAY', + 'FEDEX_EXPRESS_SAVER', + 'FEDEX_GROUND', + 'GROUND_HOME_DELIVERY', + ]); + $this->dropOffType = Arr::get($options, 'dropOffType', 'BUSINESS_SERVICE_CENTER'); $this->setRequestAdapter(Arr::get($options, 'requestAdapter', new RateRequest\Post())); } diff --git a/src/UPS/Rate.php b/src/UPS/Rate.php index a366688..cfc3170 100644 --- a/src/UPS/Rate.php +++ b/src/UPS/Rate.php @@ -18,11 +18,10 @@ class Rate extends RateAdapter private $userId; private $password; private $shipperNumber; - - public $approvedCodes = [ - '03', - '12', - ]; + /** + * Codes of appropriate shipping types. Default value is specified in __construct. + */ + public $approvedCodes; private $shippingCodes = [ 'US' => [ // United States @@ -90,7 +89,10 @@ class Rate extends RateAdapter $this->userId = Arr::get($options, 'userId'); $this->password = Arr::get($options, 'password'); $this->shipperNumber = Arr::get($options, 'shipperNumber'); - $this->approvedCodes = Arr::get($options, 'approvedCodes'); + $this->approvedCodes = Arr::get($options, 'approvedCodes',[ + '03', + '12', + ]); $this->setRequestAdapter(Arr::get($options, 'requestAdapter', new RateRequest\Post())); diff --git a/src/USPS/Rate.php b/src/USPS/Rate.php index 91e44bf..2ade0ec 100644 --- a/src/USPS/Rate.php +++ b/src/USPS/Rate.php @@ -17,11 +17,10 @@ class Rate extends RateAdapter private $username; private $password; - - public $approvedCodes = [ - '1', - '4', - ]; + /** + * Codes of appropriate shipping types. Default value is specified in __construct. + */ + public $approvedCodes; private $shipping_codes = [ 'domestic' => [ @@ -74,7 +73,10 @@ class Rate extends RateAdapter $this->username = Arr::get($options, 'username'); $this->password = Arr::get($options, 'password'); - $this->approvedCodes = Arr::get($options, 'approvedCodes'); + $this->approvedCodes = Arr::get($options, 'approvedCodes', [ + '1', + '4', + ]); $this->setRequestAdapter(Arr::get($options, 'requestAdapter', new RateRequest\Get())); } protected function validate() From 04cae3c2950c90551464562f83c1eda19717662a Mon Sep 17 00:00:00 2001 From: Pereyaslov Konstantin Date: Mon, 8 Dec 2014 20:15:36 +0300 Subject: [PATCH 7/7] Fixed formatting to match PSR-2 --- src/Ship.php | 5 ----- src/UPS/Rate.php | 2 +- src/Validator.php | 16 +++++++++------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Ship.php b/src/Ship.php index dde40f8..0ce4c50 100644 --- a/src/Ship.php +++ b/src/Ship.php @@ -58,7 +58,6 @@ class Ship // Build approvedCodes foreach ($this->shipping_options as $shipping_group => $row) { - foreach ($row as $_carrier => $row2) { if (!isset($approvedCodes[$_carrier])) { $approvedCodes[$_carrier] = []; @@ -89,9 +88,7 @@ class Ship $group_codes = array_keys($row2); if (! empty($rates[$carrier])) { - foreach ($rates[$carrier] as $row3) { - if (in_array($row3->getCode(), $group_codes)) { $row3->setCarrier($carrier); @@ -127,9 +124,7 @@ class Ship $group_codes = array_keys($row2); if (!empty($rates[$carrier])) { - foreach ($rates[$carrier] as $row3) { - if (in_array($row3->getCode(), $group_codes)) { $row3->setCarrier($carrier); $display_rates[$shipping_group][] = $row3; diff --git a/src/UPS/Rate.php b/src/UPS/Rate.php index cfc3170..c8ecf67 100644 --- a/src/UPS/Rate.php +++ b/src/UPS/Rate.php @@ -89,7 +89,7 @@ class Rate extends RateAdapter $this->userId = Arr::get($options, 'userId'); $this->password = Arr::get($options, 'password'); $this->shipperNumber = Arr::get($options, 'shipperNumber'); - $this->approvedCodes = Arr::get($options, 'approvedCodes',[ + $this->approvedCodes = Arr::get($options, 'approvedCodes', [ '03', '12', ]); diff --git a/src/Validator.php b/src/Validator.php index ed697f6..70d7acb 100644 --- a/src/Validator.php +++ b/src/Validator.php @@ -1,10 +1,12 @@