From 6757864857e3f9a573b844e6b13b1ad5eca29bb4 Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Fri, 1 Dec 2023 13:31:31 +0100 Subject: [PATCH] MOL-1196: update comment --- src/Service/TrackingInfoStructFactory.php | 32 ++++++++++---- .../Service/TrackingInfoStructFactoryTest.php | 42 ++++++++++++++++--- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/Service/TrackingInfoStructFactory.php b/src/Service/TrackingInfoStructFactory.php index e7d732a48..2a5158a58 100644 --- a/src/Service/TrackingInfoStructFactory.php +++ b/src/Service/TrackingInfoStructFactory.php @@ -13,7 +13,7 @@ class TrackingInfoStructFactory */ const MAX_TRACKING_CODE_LENGTH = 99; - public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?ShipmentTrackingInfoStruct + public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity): ?ShipmentTrackingInfoStruct { $trackingCodes = $orderDeliveryEntity->getTrackingCodes(); $shippingMethod = $orderDeliveryEntity->getShippingMethod(); @@ -21,7 +21,8 @@ public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?Sh return null; } /** - * mollie accepts only one tracking struct with code per shippment + * Currently we create one shipping in mollie for one order. one shipping object can have only one tracking code. + * When we have multiple Tracking Codes, we do not know which tracking code we should send to mollie. So we just dont send any tracking information at all * * https://docs.mollie.com/reference/v2/shipments-api/create-shipment */ @@ -29,10 +30,15 @@ public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?Sh return null; } - return $this->create((string)$shippingMethod->getName(), $trackingCodes[0], (string)$shippingMethod->getTrackingUrl()); + return $this->createInfoStruct((string)$shippingMethod->getName(), $trackingCodes[0], (string)$shippingMethod->getTrackingUrl()); } public function create(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct + { + return $this->createInfoStruct($trackingCarrier, $trackingCode, $trackingUrl); + } + + private function createInfoStruct(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct { if (empty($trackingCarrier) && empty($trackingCode)) { return null; @@ -46,20 +52,28 @@ public function create(string $trackingCarrier, string $trackingCode, string $tr throw new \InvalidArgumentException('Missing Argument for Tracking Code!'); } - if (strpos($trackingUrl, '%s') === false) { - throw new \InvalidArgumentException('Missing %s as code placeholder in Tracking URL'); + # we just have to completely remove those codes, so that no tracking happens, but a shipping works. + # still, if we find multiple codes (because separators exist), then we use the first one only + if (mb_strlen($trackingCode) > self::MAX_TRACKING_CODE_LENGTH) { + if (strpos($trackingCode, ',') !== false) { + $trackingCode = trim(explode(',', $trackingCode)[0]); + } elseif (strpos($trackingCode, ';') !== false) { + $trackingCode = trim(explode(';', $trackingCode)[0]); + } + + # if we are still too long, then simply remove the code + if (mb_strlen($trackingCode) > self::MAX_TRACKING_CODE_LENGTH) { + return new ShipmentTrackingInfoStruct($trackingCarrier, '', ''); + } } + $trackingUrl = trim(sprintf($trackingUrl, $trackingCode)); if (filter_var($trackingUrl, FILTER_VALIDATE_URL) === false) { $trackingUrl = ''; } - if (mb_strlen($trackingCode) > self::MAX_TRACKING_CODE_LENGTH) { - $trackingUrl = ''; - } - /** * following characters are not allowed in the tracking URL {,},<,>,# */ diff --git a/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php index 5ff22ff39..c4910c011 100644 --- a/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php +++ b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php @@ -83,12 +83,43 @@ public function testInfoStructCreatedByArguments(): void } - public function testExceptionIsThrownWithoutPlaceholderInUrl(): void - { - $this->expectException(\InvalidArgumentException::class); - $trackingInfoStruct = $this->factory->create('Test', 'testCode', 'https://test.foo?code'); + public function testInfoStructWithCommaSeparator():void{ + $expectedCode = '1234'; + $givenCode = $expectedCode.','.str_repeat('-',100); + $expectedCarrier = 'Test carrier'; + $trackingInfoStruct = $this->factory->create($expectedCarrier, $givenCode, 'https://test.foo?code=%s'); + $expectedUrl = 'https://test.foo?code=1234'; - $this->assertNull($trackingInfoStruct); + $this->assertNotNull($trackingInfoStruct); + $this->assertSame($expectedCode, $trackingInfoStruct->getCode()); + $this->assertSame($expectedUrl, $trackingInfoStruct->getUrl()); + $this->assertSame($expectedCarrier, $trackingInfoStruct->getCarrier()); + } + + public function testInfoStructWithSemicolonSeparator():void{ + $expectedCode = '1234'; + $givenCode = $expectedCode.';'.str_repeat('-',100); + $expectedCarrier = 'Test carrier'; + $trackingInfoStruct = $this->factory->create($expectedCarrier, $givenCode, 'https://test.foo?code=%s'); + $expectedUrl = 'https://test.foo?code=1234'; + + $this->assertNotNull($trackingInfoStruct); + $this->assertSame($expectedCode, $trackingInfoStruct->getCode()); + $this->assertSame($expectedUrl, $trackingInfoStruct->getUrl()); + $this->assertSame($expectedCarrier, $trackingInfoStruct->getCarrier()); + } + + public function testCommaSeparatorHasHigherPriority():void{ + $expectedCode = '1234'; + $givenCode = $expectedCode.',5678;'.str_repeat('-',100); + $expectedCarrier = 'Test carrier'; + $trackingInfoStruct = $this->factory->create($expectedCarrier, $givenCode, 'https://test.foo?code=%s'); + $expectedUrl = 'https://test.foo?code=1234'; + + $this->assertNotNull($trackingInfoStruct); + $this->assertSame($expectedCode, $trackingInfoStruct->getCode()); + $this->assertSame($expectedUrl, $trackingInfoStruct->getUrl()); + $this->assertSame($expectedCarrier, $trackingInfoStruct->getCarrier()); } /** @@ -114,6 +145,7 @@ public function invalidCodes(): array ['somecode'], ['some#code'], + ['some#<>{},'.str_repeat('1',200)], [str_repeat('1', 200)], ]; }