From 7f7150761ed46cbf159c646b9c848f47b85dca94 Mon Sep 17 00:00:00 2001 From: Francois-Gomis Date: Wed, 24 Jul 2024 00:31:21 +0200 Subject: [PATCH 1/5] feature: create addTracking endpoint --- src/Endpoints/Orders.php | 14 +++-- src/Entities/Order.php | 36 ----------- tests/Unit/Endpoints/OrdersTest.php | 98 ++++++++++------------------- tests/Unit/Entities/OrderTest.php | 9 --- 4 files changed, 40 insertions(+), 117 deletions(-) diff --git a/src/Endpoints/Orders.php b/src/Endpoints/Orders.php index 27ff15b3..d2b0f68d 100644 --- a/src/Endpoints/Orders.php +++ b/src/Endpoints/Orders.php @@ -66,21 +66,23 @@ public function update($orderId, $orderData) /** * @param string $orderId - * @param string|null $carrier - * @param string|null $trackingNumber + * @param string $carrier + * @param string $trackingNumber * @param string|null $trackingUrl - * @return Order + * @return void * @throws AlmaException */ - public function updateTracking($orderId, $carrier = null, $trackingNumber = null, $trackingUrl = null) + public function addTracking($orderId, $carrier, $trackingNumber, $trackingUrl = null) { $trackingData = array_filter([ 'carrier' => $carrier, 'tracking_number' => $trackingNumber, 'tracking_url' => $trackingUrl ]); - $response = $this->request(self::ORDERS_PATH . "/{$orderId}")->setRequestBody($trackingData)->put(); - return new Order($response->json); + $response = $this->request(self::ORDERS_PATH_V2 . "/{$orderId}/shipment")->setRequestBody($trackingData)->post(); + if ($response->isError()) { + throw new RequestException($response->errorMessage, null, $response); + } } /** diff --git a/src/Entities/Order.php b/src/Entities/Order.php index 1aa8fbde..46fb40c1 100644 --- a/src/Entities/Order.php +++ b/src/Entities/Order.php @@ -27,15 +27,6 @@ class Order { - /** @var string | null Order carrier */ - private $carrier; - - /** @var string | null Order carrier tracking number */ - private $trackingNumber; - - /** @var string | null Order carrier tracking URL */ - private $trackingUrl; - /** @var string ID of the Payment owning this Order * @deprecated */ @@ -105,7 +96,6 @@ class Order public function __construct($orderDataArray) { - $this->carrier = $orderDataArray['carrier']; $this->comment = $orderDataArray['comment']; $this->createdAt = $orderDataArray['created']; $this->customerUrl = $orderDataArray['customer_url']; @@ -119,35 +109,9 @@ public function __construct($orderDataArray) $this->merchantUrl = $orderDataArray['merchant_url']; $this->payment = $orderDataArray['payment']; $this->paymentId = $orderDataArray['payment']; - $this->trackingNumber = $orderDataArray['tracking_number']; - $this->trackingUrl = $orderDataArray['tracking_url']; $this->updatedAt = isset($orderDataArray['updated']) ? $orderDataArray['updated'] : null; } - /** - * @return string|null - */ - public function getCarrier() - { - return $this->carrier; - } - - /** - * @return string|null - */ - public function getTrackingNumber() - { - return $this->trackingNumber; - } - - /** - * @return string|null - */ - public function getTrackingUrl() - { - return $this->trackingUrl; - } - /** * @return string */ diff --git a/tests/Unit/Endpoints/OrdersTest.php b/tests/Unit/Endpoints/OrdersTest.php index 633c35f7..3b12554f 100644 --- a/tests/Unit/Endpoints/OrdersTest.php +++ b/tests/Unit/Endpoints/OrdersTest.php @@ -186,91 +186,57 @@ public function testSendStatusWithException() )); } - public function testUpdateTrackingThrowsAlmaException() + public function testAddTrackingThrowsAlmaException() { $this->expectException(AlmaException::class); - $this->requestObject->shouldReceive('put')->andThrow(new RequestError()); + $this->requestObject->shouldReceive('post')->andThrow(new RequestError()); $this->requestObject->shouldReceive('setRequestBody')->andReturn($this->requestObject); $this->orderEndpoint->shouldReceive('request') - ->with('/v1/orders/123') + ->with('/v2/orders/123/shipment') ->once() ->andReturn($this->requestObject); - $this->orderEndpoint->updateTracking('123', 'ups', '123456', 'myUrl'); + $this->orderEndpoint->addTracking('123', 'ups', '123456', 'myUrl'); + } + public function testAddTrackingThrowsAlmaExceptionForBadData() + { + $this->expectException(RequestException::class); + $this->responseMock->shouldReceive('isError')->once()->andReturn(true); + $this->requestObject->shouldReceive('post')->once()->andReturn($this->responseMock); + $this->requestObject->shouldReceive('setRequestBody')->andReturn($this->requestObject); + $this->orderEndpoint->shouldReceive('request') + ->with('/v2/orders/123/shipment') + ->once() + ->andReturn($this->requestObject); + $this->orderEndpoint->addTracking('123', 'ups', null); } /** - * @dataProvider updateTrackingDataProvider - * @param $carrier - * @param $trackingNumber - * @param $trackingUrl - * @param $trackingData * @return void * @throws AlmaException */ - public function testUpdateTracking($carrier, $trackingNumber, $trackingUrl, $trackingData) + public function testAddTracking() { - $this->responseMock->json = $this->orderDataFactory($carrier, $trackingNumber, $trackingUrl); - $this->requestObject->shouldReceive('put')->andReturn($this->responseMock); + $trackingData = [ + 'carrier' => 'UPS', + 'tracking_number' => 'UPS_123456', + 'tracking_url' => 'https://tracking.com' + ]; + $this->responseMock->shouldReceive('isError')->once()->andReturn(false); + $this->requestObject->shouldReceive('post')->once()->andReturn($this->responseMock); $this->requestObject->shouldReceive('setRequestBody')->with($trackingData)->andReturn($this->requestObject); + $this->orderEndpoint->shouldReceive('request') - ->with('/v1/orders/123') + ->with('/v2/orders/123/shipment') ->once() ->andReturn($this->requestObject); - $order = $this->orderEndpoint->updateTracking('123', $carrier, $trackingNumber, $trackingUrl); - $this->assertInstanceOf(Order::class, $order); - $this->assertEquals($carrier, $order->getCarrier()); - $this->assertEquals($trackingNumber, $order->getTrackingNumber()); - $this->assertEquals($trackingUrl, $order->getTrackingUrl()); - } - /** - * @return array[] - */ - public static function updateTrackingDataProvider() - { - return [ - 'no data' => [ - null, - null, - null, - [] - ], - 'Only Carrier' => [ - 'ups', - null, - null, - ['carrier' => 'ups'] - ], - 'Only Url' => [ - null, - null, - 'myUrl', - ['tracking_url' => 'myUrl'] - ], - 'Carrier and Url' => [ - 'ups', - null, - 'myUrl', - ['carrier' => 'ups', 'tracking_url' => 'myUrl'] - ], - 'All params' => [ - 'ups', - '123456', - 'myUrl', - ['carrier' => 'ups', 'tracking_number' => '123456', 'tracking_url' => 'myUrl'] - ], - ]; + $this->orderEndpoint->addTracking( + '123', + $trackingData['carrier'], + $trackingData['tracking_number'], + $trackingData['tracking_url'] + ); } - public function orderDataFactory( - $carrier, - $tracking_number, - $tracking_url - ) - { - return OrderTest::orderDataFactory($carrier, $tracking_number, $tracking_url); - } - - } diff --git a/tests/Unit/Entities/OrderTest.php b/tests/Unit/Entities/OrderTest.php index b890fb7c..c0a659f1 100644 --- a/tests/Unit/Entities/OrderTest.php +++ b/tests/Unit/Entities/OrderTest.php @@ -13,9 +13,6 @@ public function testOrderGetters() $orderData = $this->orderDataFactory(); $order = new Order($orderData); - $this->assertEquals($orderData['carrier'], $order->getCarrier()); - $this->assertEquals($orderData['tracking_number'], $order->getTrackingNumber()); - $this->assertEquals($orderData['tracking_url'], $order->getTrackingUrl()); $this->assertEquals($orderData['payment'], $order->payment); $this->assertEquals($orderData['payment'], $order->getPaymentId()); $this->assertEquals($orderData['merchant_reference'], $order->merchant_reference); @@ -34,9 +31,6 @@ public function testOrderGetters() public static function orderDataFactory( - $carrier = 'ups', - $tracking_number = 'ups_123456', - $tracking_url = 'http://tracking.url', $comment = 'my comment', $created = 1715331839, $customer_url = 'http://customer.url', @@ -49,7 +43,6 @@ public static function orderDataFactory( ) { return [ - 'carrier' => $carrier, 'comment' => $comment, 'created' => $created, 'customer_url' => $customer_url, @@ -58,8 +51,6 @@ public static function orderDataFactory( 'merchant_reference' => $merchant_reference, 'merchant_url' => $merchant_url, 'payment' => $payment, - 'tracking_number' => $tracking_number, - 'tracking_url' => $tracking_url, 'updated' => $updated ]; } From d60cf8aa93101d3178be8e4058f17e1474dbcac1 Mon Sep 17 00:00:00 2001 From: Francois-Gomis Date: Wed, 24 Jul 2024 10:26:08 +0200 Subject: [PATCH 2/5] feature: create addTracking endpoint integration test --- tests/Integration/Endpoints/OrdersTest.php | 39 +++++++++------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/tests/Integration/Endpoints/OrdersTest.php b/tests/Integration/Endpoints/OrdersTest.php index e0ab2d78..506c2445 100644 --- a/tests/Integration/Endpoints/OrdersTest.php +++ b/tests/Integration/Endpoints/OrdersTest.php @@ -3,6 +3,8 @@ namespace Alma\API\Tests\Integration\Endpoints; use Alma\API\Entities\Order; +use Alma\API\Exceptions\AlmaException; +use Alma\API\Exceptions\RequestException; use Alma\API\Tests\Integration\TestHelpers\ClientTestHelper; use Alma\API\Tests\Integration\TestHelpers\PaymentTestHelper; use PHPUnit\Framework\TestCase; @@ -28,32 +30,23 @@ public function testCanCreateANewOrder() $this->assertInstanceOf(Order::class, $newOrder); $this->assertEquals('ABC-123-NEW', $newOrder->getMerchantReference()); } - - public function testCanUpdateOrderTracking() + public function testAddOrderTrackingThrowErrorWithBadData() { + $this->expectException(RequestException::class); $payment = OrdersTest::$payment; $order = $payment->orders[0]; $this->assertInstanceOf(Order::class, $order); - $this->assertNull($order->getCarrier()); - $this->assertNull($order->getTrackingUrl()); - $this->assertNull($order->getTrackingNumber()); - - $updatedOrder = OrdersTest::$almaClient->orders->updateTracking($order->getExternalId(), null,null , 'https://tracking.com'); - $this->assertInstanceOf(Order::class, $updatedOrder); - $this->assertNull($order->getCarrier()); - $this->assertNull($updatedOrder->getTrackingNumber()); - $this->assertEquals('https://tracking.com', $updatedOrder->getTrackingUrl()); - - $updatedOrder = OrdersTest::$almaClient->orders->updateTracking($order->getExternalId(), 'UPS'); - $this->assertInstanceOf(Order::class, $updatedOrder); - $this->assertEquals('UPS', $updatedOrder->getCarrier()); - $this->assertNull($updatedOrder->getTrackingNumber()); - $this->assertEquals('https://tracking.com', $updatedOrder->getTrackingUrl()); - - $updatedOrder = OrdersTest::$almaClient->orders->updateTracking($order->getExternalId(), 'LAPOSTE','123456789' , 'https://laposte.com'); - $this->assertInstanceOf(Order::class, $updatedOrder); - $this->assertEquals('LAPOSTE', $updatedOrder->getCarrier()); - $this->assertEquals('123456789', $updatedOrder->getTrackingNumber()); - $this->assertEquals('https://laposte.com', $updatedOrder->getTrackingUrl()); + $this->assertNull( + OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS',null ) + ); + } + public function testAddOrderTracking() + { + $payment = OrdersTest::$payment; + $order = $payment->orders[0]; + $this->assertInstanceOf(Order::class, $order); + $this->assertNull( + OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS','UPS_123456' , 'https://tracking.com') + ); } } \ No newline at end of file From 68747300e134c466726380e978f8847ced6396c2 Mon Sep 17 00:00:00 2001 From: Francois-Gomis Date: Wed, 24 Jul 2024 22:33:31 +0200 Subject: [PATCH 3/5] fix: orderTest and insurance lint --- src/Endpoints/Insurance.php | 2 +- tests/Integration/Endpoints/OrdersTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Endpoints/Insurance.php b/src/Endpoints/Insurance.php index bdd20c4a..9f08452a 100644 --- a/src/Endpoints/Insurance.php +++ b/src/Endpoints/Insurance.php @@ -76,7 +76,7 @@ public function getInsuranceContract( throw new RequestException($response->errorMessage, null, $response); } - // @todo is it a possible case, or do we need to throw an exception + // Is it a possible case, or do we need to throw an exception if (!$response->json) { return null; } diff --git a/tests/Integration/Endpoints/OrdersTest.php b/tests/Integration/Endpoints/OrdersTest.php index 506c2445..c1f1d21e 100644 --- a/tests/Integration/Endpoints/OrdersTest.php +++ b/tests/Integration/Endpoints/OrdersTest.php @@ -30,23 +30,23 @@ public function testCanCreateANewOrder() $this->assertInstanceOf(Order::class, $newOrder); $this->assertEquals('ABC-123-NEW', $newOrder->getMerchantReference()); } + public function testAddOrderTrackingThrowErrorWithBadData() { $this->expectException(RequestException::class); $payment = OrdersTest::$payment; $order = $payment->orders[0]; $this->assertInstanceOf(Order::class, $order); - $this->assertNull( - OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS',null ) - ); + OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS', null); } + public function testAddOrderTracking() { $payment = OrdersTest::$payment; $order = $payment->orders[0]; $this->assertInstanceOf(Order::class, $order); $this->assertNull( - OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS','UPS_123456' , 'https://tracking.com') + OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS', 'UPS_123456', 'https://tracking.com') ); } } \ No newline at end of file From 974ca6297677f1cd1bdaf044e8c0881fea6e3fc0 Mon Sep 17 00:00:00 2001 From: Francois-Gomis Date: Wed, 24 Jul 2024 22:41:39 +0200 Subject: [PATCH 4/5] fix: split 126 characters long --- src/Endpoints/Orders.php | 4 +++- tests/Integration/Endpoints/OrdersTest.php | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Endpoints/Orders.php b/src/Endpoints/Orders.php index d2b0f68d..75003ff3 100644 --- a/src/Endpoints/Orders.php +++ b/src/Endpoints/Orders.php @@ -79,7 +79,9 @@ public function addTracking($orderId, $carrier, $trackingNumber, $trackingUrl = 'tracking_number' => $trackingNumber, 'tracking_url' => $trackingUrl ]); - $response = $this->request(self::ORDERS_PATH_V2 . "/{$orderId}/shipment")->setRequestBody($trackingData)->post(); + $response = $this->request(self::ORDERS_PATH_V2 . "/{$orderId}/shipment") + ->setRequestBody($trackingData) + ->post(); if ($response->isError()) { throw new RequestException($response->errorMessage, null, $response); } diff --git a/tests/Integration/Endpoints/OrdersTest.php b/tests/Integration/Endpoints/OrdersTest.php index c1f1d21e..dd12d9e0 100644 --- a/tests/Integration/Endpoints/OrdersTest.php +++ b/tests/Integration/Endpoints/OrdersTest.php @@ -46,7 +46,12 @@ public function testAddOrderTracking() $order = $payment->orders[0]; $this->assertInstanceOf(Order::class, $order); $this->assertNull( - OrdersTest::$almaClient->orders->addTracking($order->getExternalId(), 'UPS', 'UPS_123456', 'https://tracking.com') + OrdersTest::$almaClient->orders->addTracking( + $order->getExternalId(), + 'UPS', + 'UPS_123456', + 'https://tracking.com' + ) ); } } \ No newline at end of file From 3b1e8e9c94a7d807f051b3ee3db8a735784abebd Mon Sep 17 00:00:00 2001 From: Francois-Gomis Date: Fri, 26 Jul 2024 14:58:45 +0200 Subject: [PATCH 5/5] fix: add beStrictAboutTestsThatDoNotTestAnything="false" property --- phpunit.ci.xml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/phpunit.ci.xml b/phpunit.ci.xml index 1ef46110..36343cc3 100644 --- a/phpunit.ci.xml +++ b/phpunit.ci.xml @@ -8,7 +8,9 @@ convertNoticesToExceptions = "true" convertWarningsToExceptions = "true" processIsolation = "false" - stopOnFailure = "true"> + stopOnFailure = "true" + beStrictAboutTestsThatDoNotTestAnything="false" + >