From d548cb851337151862f518b9b349f8f0e1b34907 Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Mon, 31 Jul 2023 08:42:50 +0200 Subject: [PATCH 1/5] MOL-1196: Tracking --- .../MollieOrderShipmentTrackingEvent.php | 128 ------------------ src/Facade/MollieShipment.php | 19 ++- src/Resources/config/services/subscriber.xml | 5 - src/Service/MollieApi/Order.php | 12 +- src/Service/OrderDeliveryService.php | 1 + src/Subscriber/ShippingSubscriber.php | 79 ----------- .../Facade/MollieShipment/SetShipmentTest.php | 3 +- 7 files changed, 29 insertions(+), 218 deletions(-) delete mode 100644 src/Event/MollieOrderShipmentTrackingEvent.php delete mode 100644 src/Subscriber/ShippingSubscriber.php diff --git a/src/Event/MollieOrderShipmentTrackingEvent.php b/src/Event/MollieOrderShipmentTrackingEvent.php deleted file mode 100644 index 645816ba2..000000000 --- a/src/Event/MollieOrderShipmentTrackingEvent.php +++ /dev/null @@ -1,128 +0,0 @@ -orderId = $orderId; - $this->context = $context; - $this->trackingCarrier = $trackingCarrier; - $this->trackingCode = $trackingCode; - $this->trackingUrl = $trackingUrl; - } - - /** - * @return string - */ - public function getOrderId(): string - { - return $this->orderId; - } - - /** - * @param string $orderId - */ - public function setOrderId(string $orderId): void - { - $this->orderId = $orderId; - } - - /** - * @return Context - */ - public function getContext(): Context - { - return $this->context; - } - - /** - * @param Context $context - */ - public function setContext(Context $context): void - { - $this->context = $context; - } - - /** - * @return string - */ - public function getTrackingCarrier(): string - { - return $this->trackingCarrier; - } - - /** - * @param string $trackingCarrier - */ - public function setTrackingCarrier(string $trackingCarrier): void - { - $this->trackingCarrier = $trackingCarrier; - } - - /** - * @return string - */ - public function getTrackingCode(): string - { - return $this->trackingCode; - } - - /** - * @param string $trackingCode - */ - public function setTrackingCode(string $trackingCode): void - { - $this->trackingCode = $trackingCode; - } - - /** - * @return string - */ - public function getTrackingUrl(): string - { - return $this->trackingUrl; - } - - /** - * @param string $trackingUrl - */ - public function setTrackingUrl(string $trackingUrl): void - { - $this->trackingUrl = $trackingUrl; - } -} diff --git a/src/Facade/MollieShipment.php b/src/Facade/MollieShipment.php index b6bf37498..cdf26a1f5 100644 --- a/src/Facade/MollieShipment.php +++ b/src/Facade/MollieShipment.php @@ -20,6 +20,7 @@ use Shopware\Core\Checkout\Order\Aggregate\OrderLineItem\OrderLineItemEntity; use Shopware\Core\Checkout\Order\Aggregate\OrderTransaction\OrderTransactionEntity; use Shopware\Core\Checkout\Order\OrderEntity; +use Shopware\Core\Checkout\Shipping\ShippingMethodEntity; use Shopware\Core\Content\Product\ProductEntity; use Shopware\Core\Framework\Context; use Shopware\Core\Framework\Uuid\Uuid; @@ -169,8 +170,8 @@ public function setShipment(string $orderDeliveryId, Context $context): bool return false; } - - $addedMollieShipment = $this->mollieApiOrderService->setShipment($mollieOrderId, $order->getSalesChannelId()); + $trackingInfoStruct = $this->createTrackingInfoStructFromDelivery($delivery); + $addedMollieShipment = $this->mollieApiOrderService->setShipment($mollieOrderId, $trackingInfoStruct, $order->getSalesChannelId()); if ($addedMollieShipment) { $values = [CustomFieldsInterface::DELIVERY_SHIPPED => true]; @@ -463,6 +464,15 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie return false; }); } + private function createTrackingInfoStructFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?ShipmentTrackingInfoStruct{ + $trackingCodes = $orderDeliveryEntity->getTrackingCodes(); + $shippingMethod = $orderDeliveryEntity->getShippingMethod(); + if (count($trackingCodes) !== 1 && ! $shippingMethod instanceof ShippingMethodEntity) { + return null; + } + + return $this->createTrackingInfoStruct($shippingMethod->getName(),$trackingCodes[0], $shippingMethod->getTrackingUrl()); + } private function createTrackingInfoStruct(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct { @@ -478,6 +488,11 @@ private function createTrackingInfoStruct(string $trackingCarrier, string $track throw new \InvalidArgumentException('Missing Argument for Tracking Code!'); } + $trackingUrl = trim(sprintf($trackingUrl, $trackingCode)); + if (filter_var($trackingUrl, FILTER_VALIDATE_URL) === false) { + $trackingUrl = ''; + } + return new ShipmentTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl); } } diff --git a/src/Resources/config/services/subscriber.xml b/src/Resources/config/services/subscriber.xml index eb89fc689..e76ae36f7 100644 --- a/src/Resources/config/services/subscriber.xml +++ b/src/Resources/config/services/subscriber.xml @@ -21,11 +21,6 @@ - - - - - diff --git a/src/Service/MollieApi/Order.php b/src/Service/MollieApi/Order.php index 140c87c47..cbd4fcab2 100644 --- a/src/Service/MollieApi/Order.php +++ b/src/Service/MollieApi/Order.php @@ -14,6 +14,7 @@ use Kiener\MolliePayments\Service\MollieApi\RequestAnonymizer\MollieRequestAnonymizer; use Kiener\MolliePayments\Service\Router\RoutingBuilder; use Kiener\MolliePayments\Service\SettingsService; +use Kiener\MolliePayments\Struct\MollieApi\ShipmentTrackingInfoStruct; use Mollie\Api\Exceptions\ApiException; use Mollie\Api\Resources\Order as MollieOrder; use Mollie\Api\Resources\OrderLine; @@ -25,7 +26,9 @@ use Psr\Log\LoggerInterface; use RuntimeException; use Shopware\Core\Checkout\Customer\CustomerEntity; +use Shopware\Core\Checkout\Order\Aggregate\OrderDelivery\OrderDeliveryEntity; use Shopware\Core\Checkout\Order\OrderEntity; +use Shopware\Core\Checkout\Shipping\ShippingMethodEntity; use Shopware\Core\System\SalesChannel\SalesChannelContext; class Order @@ -378,14 +381,17 @@ public function getPaymentUrl(string $mollieOrderId, string $salesChannelId): ?s return $mollieOrder->status === 'created' ? $mollieOrder->getCheckoutUrl() : null; } - public function setShipment(string $mollieOrderId, string $salesChannelId): bool + public function setShipment(string $mollieOrderId, ?ShipmentTrackingInfoStruct $trackingInfoStruct, string $salesChannelId): bool { $mollieOrder = $this->getMollieOrder($mollieOrderId, $salesChannelId); - + $shipment = []; + if($trackingInfoStruct instanceof ShipmentTrackingInfoStruct){ + $shipment['tracking'] = $trackingInfoStruct->toArray(); + } /** @var OrderLine $orderLine */ foreach ($mollieOrder->lines() as $orderLine) { if ($orderLine->shippableQuantity > 0) { - $mollieOrder->shipAll(); + $mollieOrder->shipAll($shipment); return true; } diff --git a/src/Service/OrderDeliveryService.php b/src/Service/OrderDeliveryService.php index 0d50472c0..fb3eaad9e 100644 --- a/src/Service/OrderDeliveryService.php +++ b/src/Service/OrderDeliveryService.php @@ -27,6 +27,7 @@ public function getDelivery(string $orderDeliveryId, Context $context): ?OrderDe { $criteria = new Criteria([$orderDeliveryId]); $criteria->addAssociation('order.transactions.paymentMethod'); + $criteria->addAssociation('shippingMethod'); $result = $this->orderDeliveryRepository->search($criteria, $context); return $result->first(); diff --git a/src/Subscriber/ShippingSubscriber.php b/src/Subscriber/ShippingSubscriber.php deleted file mode 100644 index 0afdd4f33..000000000 --- a/src/Subscriber/ShippingSubscriber.php +++ /dev/null @@ -1,79 +0,0 @@ -shipmentFacade = $shipmentFacade; - $this->logger = $logger; - } - - public static function getSubscribedEvents() - { - return [ - MollieOrderShipmentTrackingEvent::class => 'onShipOrderWithTracking', - ]; - } - - /** - * @param MollieOrderShipmentTrackingEvent $event - */ - public function onShipOrderWithTracking(MollieOrderShipmentTrackingEvent $event): void - { - try { - $this->shipmentFacade->shipOrderByOrderId( - $event->getOrderId(), - $event->getTrackingCarrier(), - $event->getTrackingCode(), - $event->getTrackingUrl(), - $event->getContext() - ); - } catch (CouldNotExtractMollieOrderIdException $e) { - // We need to catch CouldNotExtractMollieOrderIdException, because if it's not a Mollie Order - // it obviously cannot get shipped with Mollie. We also don't have to log this, except for debugging. - // But if we don't catch it, the rest of the process might break. - $this->logger->debug($e->getMessage(), [ - 'orderId' => $event->getOrderId(), - 'trackingCarrier' => $event->getTrackingCarrier(), - 'trackingCode' => $event->getTrackingCode(), - 'trackingUrl' => $event->getTrackingUrl(), - ]); - } catch (\Exception $e) { - // We log the error, but don't rethrow so the rest of the proces can continue. - $this->logger->error( - sprintf( - "Error when shipping order from Mollie Event: \"%s\" in \"%s\" on line %s", - $e->getMessage(), - $e->getFile(), - $e->getLine() - ), - [ - 'orderId' => $event->getOrderId(), - 'trackingCarrier' => $event->getTrackingCarrier(), - 'trackingCode' => $event->getTrackingCode(), - 'trackingUrl' => $event->getTrackingUrl(), - ] - ); - } - } -} diff --git a/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php b/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php index 3530ddde3..3b321651b 100644 --- a/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php +++ b/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php @@ -211,6 +211,7 @@ public function testThatOrderDeliveryCustomFieldsAreWrittenWhenApiCallSuccessful $order->setSalesChannel($salesChannel); $order->setSalesChannelId($salesChannelId); $delivery = $this->createDelivery($order); + $deliveryId = $delivery->getId(); $this->orderDeliveryService->method('getDelivery')->willReturn($delivery); $this->mollieApiOrderService->method('setShipment') @@ -236,7 +237,7 @@ private function createDelivery(?OrderEntity $order): OrderDeliveryEntity { $delivery = new OrderDeliveryEntity(); $delivery->setId(Uuid::randomHex()); - + $delivery->setTrackingCodes([]); if ($order instanceof OrderEntity) { $delivery->setOrder($order); } From d5d81db32c31cbac2123f1b9fd5f1d204d2bfeda Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Mon, 27 Nov 2023 14:29:03 +0100 Subject: [PATCH 2/5] MOL-1196: fix pipeline --- src/Facade/MollieShipment.php | 10 ++++++++-- .../PHPUnit/Facade/MollieShipment/SetShipmentTest.php | 5 +++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Facade/MollieShipment.php b/src/Facade/MollieShipment.php index cdf26a1f5..7fb507aec 100644 --- a/src/Facade/MollieShipment.php +++ b/src/Facade/MollieShipment.php @@ -161,6 +161,7 @@ public function setShipment(string $orderDeliveryId, Context $context): bool $lastTransaction = $this->extractor->extractLastMolliePayment($order->getTransactions()); if (!$lastTransaction instanceof OrderTransactionEntity) { + $this->logger->info( sprintf( 'The last transaction of the order (%s) is not a mollie payment! No shipment will be sent to mollie', @@ -170,7 +171,9 @@ public function setShipment(string $orderDeliveryId, Context $context): bool return false; } + $trackingInfoStruct = $this->createTrackingInfoStructFromDelivery($delivery); + $addedMollieShipment = $this->mollieApiOrderService->setShipment($mollieOrderId, $trackingInfoStruct, $order->getSalesChannelId()); if ($addedMollieShipment) { @@ -467,11 +470,14 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie private function createTrackingInfoStructFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?ShipmentTrackingInfoStruct{ $trackingCodes = $orderDeliveryEntity->getTrackingCodes(); $shippingMethod = $orderDeliveryEntity->getShippingMethod(); - if (count($trackingCodes) !== 1 && ! $shippingMethod instanceof ShippingMethodEntity) { + if($shippingMethod === null){ + return null; + } + if (count($trackingCodes) !== 1) { return null; } - return $this->createTrackingInfoStruct($shippingMethod->getName(),$trackingCodes[0], $shippingMethod->getTrackingUrl()); + return $this->createTrackingInfoStruct((string)$shippingMethod->getName(),$trackingCodes[0], (string)$shippingMethod->getTrackingUrl()); } private function createTrackingInfoStruct(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct diff --git a/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php b/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php index 3b321651b..deee49636 100644 --- a/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php +++ b/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php @@ -188,7 +188,7 @@ public function testThatOrderDeliveryCustomFieldsAreNotWrittenWhenApiCallUnsucce $deliveryId = $delivery->getId(); $this->orderDeliveryService->method('getDelivery')->willReturn($delivery); $this->mollieApiOrderService->method('setShipment') - ->with($mollieOrderId, $salesChannelId) + ->with($mollieOrderId, null, $salesChannelId) ->willReturn(false); // custom fields for shipping are never written @@ -201,6 +201,7 @@ public function testThatOrderDeliveryCustomFieldsAreNotWrittenWhenApiCallUnsucce public function testThatOrderDeliveryCustomFieldsAreWrittenWhenApiCallSuccessful(): void { $transaction = $this->createTransaction('Kiener\MolliePayments\Handler\Method\FooMethod'); + $order = $this->createOrder($transaction); $mollieOrderId = 'foo'; $customFields[CustomFieldsInterface::MOLLIE_KEY][CustomFieldsInterface::ORDER_KEY] = $mollieOrderId; @@ -215,7 +216,7 @@ public function testThatOrderDeliveryCustomFieldsAreWrittenWhenApiCallSuccessful $deliveryId = $delivery->getId(); $this->orderDeliveryService->method('getDelivery')->willReturn($delivery); $this->mollieApiOrderService->method('setShipment') - ->with($mollieOrderId, $salesChannelId) + ->with($mollieOrderId, null,$salesChannelId) ->willReturn(true); // custom fields for shipping are written From 8f63450b225e13541fed965e4b538104a33c6706 Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Mon, 27 Nov 2023 15:41:20 +0100 Subject: [PATCH 3/5] MOL-1196: extract to factory --- src/Facade/MollieShipment.php | 51 +++++------------- src/Resources/config/services.xml | 3 ++ src/Resources/config/services/facades.xml | 2 + src/Service/MollieApi/Order.php | 2 +- src/Service/TrackingInfoStructFactory.php | 54 +++++++++++++++++++ .../CreateTrackingStructTest.php | 3 ++ .../Facade/MollieShipment/SetShipmentTest.php | 4 ++ .../Service/TrackingInfoStructFactoryTest.php | 46 ++++++++++++++++ 8 files changed, 125 insertions(+), 40 deletions(-) create mode 100644 src/Service/TrackingInfoStructFactory.php create mode 100644 tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php diff --git a/src/Facade/MollieShipment.php b/src/Facade/MollieShipment.php index 7fb507aec..1021bf181 100644 --- a/src/Facade/MollieShipment.php +++ b/src/Facade/MollieShipment.php @@ -11,6 +11,7 @@ use Kiener\MolliePayments\Service\MolliePaymentExtractor; use Kiener\MolliePayments\Service\OrderDeliveryService; use Kiener\MolliePayments\Service\OrderService; +use Kiener\MolliePayments\Service\TrackingInfoStructFactory; use Kiener\MolliePayments\Service\Transition\DeliveryTransitionServiceInterface; use Kiener\MolliePayments\Struct\MollieApi\ShipmentTrackingInfoStruct; use Psr\Log\LoggerInterface; @@ -62,11 +63,17 @@ class MollieShipment implements MollieShipmentInterface */ private $orderDataExtractor; + /** + * @var TrackingInfoStructFactory + */ + private $trackingInfoStructFactory; + /** * @var LoggerInterface */ private $logger; + /** * @param MolliePaymentExtractor $extractor * @param DeliveryTransitionServiceInterface $deliveryTransitionService @@ -77,7 +84,7 @@ class MollieShipment implements MollieShipmentInterface * @param OrderDataExtractor $orderDataExtractor * @param LoggerInterface $logger */ - public function __construct(MolliePaymentExtractor $extractor, DeliveryTransitionServiceInterface $deliveryTransitionService, Order $mollieApiOrderService, Shipment $mollieApiShipmentService, OrderDeliveryService $orderDeliveryService, OrderService $orderService, OrderDataExtractor $orderDataExtractor, LoggerInterface $logger) + public function __construct(MolliePaymentExtractor $extractor, DeliveryTransitionServiceInterface $deliveryTransitionService, Order $mollieApiOrderService, Shipment $mollieApiShipmentService, OrderDeliveryService $orderDeliveryService, OrderService $orderService, OrderDataExtractor $orderDataExtractor, TrackingInfoStructFactory $trackingInfoStructFactory, LoggerInterface $logger) { $this->extractor = $extractor; $this->deliveryTransitionService = $deliveryTransitionService; @@ -87,6 +94,7 @@ public function __construct(MolliePaymentExtractor $extractor, DeliveryTransitio $this->orderService = $orderService; $this->orderDataExtractor = $orderDataExtractor; $this->logger = $logger; + $this->trackingInfoStructFactory = $trackingInfoStructFactory; } /** @@ -161,7 +169,6 @@ public function setShipment(string $orderDeliveryId, Context $context): bool $lastTransaction = $this->extractor->extractLastMolliePayment($order->getTransactions()); if (!$lastTransaction instanceof OrderTransactionEntity) { - $this->logger->info( sprintf( 'The last transaction of the order (%s) is not a mollie payment! No shipment will be sent to mollie', @@ -172,7 +179,7 @@ public function setShipment(string $orderDeliveryId, Context $context): bool return false; } - $trackingInfoStruct = $this->createTrackingInfoStructFromDelivery($delivery); + $trackingInfoStruct = $this->trackingInfoStructFactory->createFromDelivery($delivery); $addedMollieShipment = $this->mollieApiOrderService->setShipment($mollieOrderId, $trackingInfoStruct, $order->getSalesChannelId()); @@ -254,7 +261,7 @@ public function shipOrder( $shipment = $this->mollieApiShipmentService->shipOrder( $mollieOrderId, $order->getSalesChannelId(), - $this->createTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl) + $this->trackingInfoStructFactory->create($trackingCarrier, $trackingCode, $trackingUrl) ); $delivery = $this->orderDataExtractor->extractDelivery($order, $context); @@ -375,7 +382,7 @@ public function shipItem( $order->getSalesChannelId(), $mollieOrderLineId, $quantity, - $this->createTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl) + $this->trackingInfoStructFactory->create($trackingCarrier, $trackingCode, $trackingUrl) ); $delivery = $this->orderDataExtractor->extractDelivery($order, $context); @@ -467,38 +474,4 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie return false; }); } - private function createTrackingInfoStructFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?ShipmentTrackingInfoStruct{ - $trackingCodes = $orderDeliveryEntity->getTrackingCodes(); - $shippingMethod = $orderDeliveryEntity->getShippingMethod(); - if($shippingMethod === null){ - return null; - } - if (count($trackingCodes) !== 1) { - return null; - } - - return $this->createTrackingInfoStruct((string)$shippingMethod->getName(),$trackingCodes[0], (string)$shippingMethod->getTrackingUrl()); - } - - private function createTrackingInfoStruct(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct - { - if (empty($trackingCarrier) && empty($trackingCode)) { - return null; - } - - if (empty($trackingCarrier)) { - throw new \InvalidArgumentException('Missing Argument for Tracking Carrier!'); - } - - if (empty($trackingCode)) { - throw new \InvalidArgumentException('Missing Argument for Tracking Code!'); - } - - $trackingUrl = trim(sprintf($trackingUrl, $trackingCode)); - if (filter_var($trackingUrl, FILTER_VALIDATE_URL) === false) { - $trackingUrl = ''; - } - - return new ShipmentTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl); - } } diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index f822b00ad..1c529d7fd 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -178,6 +178,9 @@ %env(default::APP_URL)% + + + diff --git a/src/Resources/config/services/facades.xml b/src/Resources/config/services/facades.xml index b9b93894d..941f4edef 100644 --- a/src/Resources/config/services/facades.xml +++ b/src/Resources/config/services/facades.xml @@ -13,9 +13,11 @@ + + diff --git a/src/Service/MollieApi/Order.php b/src/Service/MollieApi/Order.php index b377fe896..3e1a7e145 100644 --- a/src/Service/MollieApi/Order.php +++ b/src/Service/MollieApi/Order.php @@ -398,7 +398,7 @@ public function setShipment(string $mollieOrderId, ?ShipmentTrackingInfoStruct $ { $mollieOrder = $this->getMollieOrder($mollieOrderId, $salesChannelId); $shipment = []; - if($trackingInfoStruct instanceof ShipmentTrackingInfoStruct){ + if ($trackingInfoStruct instanceof ShipmentTrackingInfoStruct) { $shipment['tracking'] = $trackingInfoStruct->toArray(); } /** @var OrderLine $orderLine */ diff --git a/src/Service/TrackingInfoStructFactory.php b/src/Service/TrackingInfoStructFactory.php new file mode 100644 index 000000000..06cec1949 --- /dev/null +++ b/src/Service/TrackingInfoStructFactory.php @@ -0,0 +1,54 @@ +getTrackingCodes(); + $shippingMethod = $orderDeliveryEntity->getShippingMethod(); + if ($shippingMethod === null) { + return null; + } + if (count($trackingCodes) !== 1) { + return null; + } + + return $this->create((string)$shippingMethod->getName(), $trackingCodes[0], (string)$shippingMethod->getTrackingUrl()); + } + + public function create(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct + { + if (empty($trackingCarrier) && empty($trackingCode)) { + return null; + } + + if (empty($trackingCarrier)) { + throw new \InvalidArgumentException('Missing Argument for Tracking Carrier!'); + } + + if (empty($trackingCode)) { + throw new \InvalidArgumentException('Missing Argument for Tracking Code!'); + } + + $trackingUrl = trim($trackingUrl . $trackingCode); + + if (filter_var($trackingUrl, FILTER_VALIDATE_URL) === false) { + $trackingUrl = ''; + } + + /** + * following characters are not allowed in the tracking URL {,},<,>,# + */ + if (preg_match_all('/[{}<>#]/m', $trackingUrl)) { + $trackingUrl = ''; + } + + return new ShipmentTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl); + } +} diff --git a/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php b/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php index 0664c1208..5ca7dc9ae 100644 --- a/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php +++ b/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php @@ -10,6 +10,7 @@ use Kiener\MolliePayments\Service\MolliePaymentExtractor; use Kiener\MolliePayments\Service\OrderDeliveryService; use Kiener\MolliePayments\Service\OrderService; +use Kiener\MolliePayments\Service\TrackingInfoStructFactory; use Kiener\MolliePayments\Service\Transition\DeliveryTransitionService; use Kiener\MolliePayments\Struct\MollieApi\ShipmentTrackingInfoStruct; use Mollie\Api\Resources\Shipment as ShipmentResource; @@ -71,6 +72,7 @@ public function setUp(): void 'extractDelivery' => $this->delivery ]); + $this->shipmentFacade = new MollieShipment( $this->createMock(MolliePaymentExtractor::class), $this->createMock(DeliveryTransitionService::class), @@ -79,6 +81,7 @@ public function setUp(): void $this->createMock(OrderDeliveryService::class), $this->orderService, $this->orderDataExtractor, + new TrackingInfoStructFactory(), new NullLogger(), ); diff --git a/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php b/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php index deee49636..bef93d763 100644 --- a/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php +++ b/tests/PHPUnit/Facade/MollieShipment/SetShipmentTest.php @@ -11,6 +11,7 @@ use Kiener\MolliePayments\Service\MolliePaymentExtractor; use Kiener\MolliePayments\Service\OrderDeliveryService; use Kiener\MolliePayments\Service\OrderService; +use Kiener\MolliePayments\Service\TrackingInfoStructFactory; use Kiener\MolliePayments\Service\Transition\DeliveryTransitionService; use Monolog\Logger; use PHPUnit\Framework\TestCase; @@ -100,6 +101,8 @@ public function setup(): void $this->createMock(CustomerService::class) ); + $trackingStructFactory = new TrackingInfoStructFactory(); + $this->mollieShipment = new MollieShipment( $this->extractor, $this->deliveryTransitionService, @@ -108,6 +111,7 @@ public function setup(): void $this->orderDeliveryService, $this->orderService, $this->orderDataExtractor, + $trackingStructFactory, $this->logger ); $this->orderNumber = 'fooOrderNumber'; diff --git a/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php new file mode 100644 index 000000000..4fee1cdcf --- /dev/null +++ b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php @@ -0,0 +1,46 @@ +factory = new TrackingInfoStructFactory(); + } + + /** + * @dataProvider invalidCodes + * @param string $url + * @param string $trackingCode + * @return void + */ + public function testInvalidTrackingCodeCharacter(string $trackingCode):void{ + + $trackingInfoStruct = $this->factory->create('test',$trackingCode,'https://foo.bar'); + $expected =''; + $this->assertSame($expected,$trackingInfoStruct->getUrl()); + + } + public function invalidCodes():array{ + return [ + ['some{code'], + ['some}code'], + ['somecode'], + ['some#code'], + ]; + } +} \ No newline at end of file From 2dbf5af176989072a63fde850d0079b0f84ab11f Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Tue, 28 Nov 2023 13:38:10 +0100 Subject: [PATCH 4/5] MOL-1196: check tracking code lenght --- src/Service/TrackingInfoStructFactory.php | 20 ++++- .../CreateTrackingStructTest.php | 2 +- .../Service/TrackingInfoStructFactoryTest.php | 84 +++++++++++++++++-- 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/Service/TrackingInfoStructFactory.php b/src/Service/TrackingInfoStructFactory.php index 06cec1949..e7d732a48 100644 --- a/src/Service/TrackingInfoStructFactory.php +++ b/src/Service/TrackingInfoStructFactory.php @@ -8,6 +8,11 @@ class TrackingInfoStructFactory { + /** + * Mollie throws an error with length >= 100 + */ + const MAX_TRACKING_CODE_LENGTH = 99; + public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?ShipmentTrackingInfoStruct { $trackingCodes = $orderDeliveryEntity->getTrackingCodes(); @@ -15,6 +20,11 @@ public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?Sh if ($shippingMethod === null) { return null; } + /** + * mollie accepts only one tracking struct with code per shippment + * + * https://docs.mollie.com/reference/v2/shipments-api/create-shipment + */ if (count($trackingCodes) !== 1) { return null; } @@ -36,12 +46,20 @@ public function create(string $trackingCarrier, string $trackingCode, string $tr throw new \InvalidArgumentException('Missing Argument for Tracking Code!'); } - $trackingUrl = trim($trackingUrl . $trackingCode); + if (strpos($trackingUrl, '%s') === false) { + throw new \InvalidArgumentException('Missing %s as code placeholder in Tracking URL'); + } + + $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/Facade/MollieShipment/CreateTrackingStructTest.php b/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php index 5ca7dc9ae..49c03c301 100644 --- a/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php +++ b/tests/PHPUnit/Facade/MollieShipment/CreateTrackingStructTest.php @@ -123,6 +123,6 @@ public function testTrackingInfoStructWithCorrectData() $this->assertInstanceOf(ShipmentTrackingInfoStruct::class, $trackingInfoStruct); }); - $this->shipmentFacade->shipOrder($this->order, 'Mollie', '123456789', '', $this->context); + $this->shipmentFacade->shipOrder($this->order, 'Mollie', '123456789', 'https://foo.bar?code=%s', $this->context); } } diff --git a/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php index 4fee1cdcf..5ff22ff39 100644 --- a/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php +++ b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php @@ -5,6 +5,8 @@ use Kiener\MolliePayments\Service\TrackingInfoStructFactory; use PHPUnit\Framework\TestCase; +use Shopware\Core\Checkout\Order\Aggregate\OrderDelivery\OrderDeliveryEntity; +use Shopware\Core\Checkout\Shipping\ShippingMethodEntity; /** * @coversDefaultClass \Kiener\MolliePayments\Service\TrackingInfoStructFactory @@ -21,26 +23,98 @@ public function setUp(): void $this->factory = new TrackingInfoStructFactory(); } + + public function testInfoStructCreatedByDelivery(): void + { + $expectedCode = '1234'; + $expectedCarrier = 'Test carrier'; + $expectedUrl = 'https://test.foo?code=1234'; + $deliveryEntity = new OrderDeliveryEntity(); + $deliveryEntity->setUniqueIdentifier('testDelivery'); + $deliveryEntity->setTrackingCodes([ + $expectedCode + ]); + + $shippingMethod = new ShippingMethodEntity(); + $shippingMethod->setName($expectedCarrier); + $shippingMethod->setUniqueIdentifier('testShippingMethod'); + $shippingMethod->setTrackingUrl('https://test.foo?code=%s'); + + $deliveryEntity->setShippingMethod($shippingMethod); + $trackingInfoStruct = $this->factory->createFromDelivery($deliveryEntity); + + $this->assertNotNull($trackingInfoStruct); + $this->assertSame($expectedCode, $trackingInfoStruct->getCode()); + $this->assertSame($expectedUrl, $trackingInfoStruct->getUrl()); + $this->assertSame($expectedCarrier, $trackingInfoStruct->getCarrier()); + } + + public function testOnlyOneCodeAccepted(): void + { + + $deliveryEntity = new OrderDeliveryEntity(); + $deliveryEntity->setUniqueIdentifier('testDelivery'); + $deliveryEntity->setTrackingCodes([ + '1234', + 'test' + ]); + + $shippingMethod = new ShippingMethodEntity(); + $shippingMethod->setName('Test carrier'); + $shippingMethod->setUniqueIdentifier('testShippingMethod'); + $shippingMethod->setTrackingUrl('https://test.foo?code=%s'); + + $deliveryEntity->setShippingMethod($shippingMethod); + $trackingInfoStruct = $this->factory->createFromDelivery($deliveryEntity); + $this->assertNull($trackingInfoStruct); + } + + public function testInfoStructCreatedByArguments(): void + { + $expectedCode = '1234'; + $expectedCarrier = 'Test carrier'; + $trackingInfoStruct = $this->factory->create($expectedCarrier, $expectedCode, '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 testExceptionIsThrownWithoutPlaceholderInUrl(): void + { + $this->expectException(\InvalidArgumentException::class); + $trackingInfoStruct = $this->factory->create('Test', 'testCode', 'https://test.foo?code'); + + $this->assertNull($trackingInfoStruct); + } + /** * @dataProvider invalidCodes * @param string $url * @param string $trackingCode * @return void */ - public function testInvalidTrackingCodeCharacter(string $trackingCode):void{ + public function testInvalidTrackingCodeCharacter(string $trackingCode): void + { - $trackingInfoStruct = $this->factory->create('test',$trackingCode,'https://foo.bar'); - $expected =''; - $this->assertSame($expected,$trackingInfoStruct->getUrl()); + $trackingInfoStruct = $this->factory->create('test', $trackingCode, 'https://foo.bar/%s'); + $expected = ''; + $this->assertSame($expected, $trackingInfoStruct->getUrl()); } - public function invalidCodes():array{ + + public function invalidCodes(): array + { return [ ['some{code'], ['some}code'], ['somecode'], ['some#code'], + [str_repeat('1', 200)], ]; } } \ No newline at end of file From c947525f5ae7736aa18240324e4267a372373eb8 Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Fri, 1 Dec 2023 13:31:31 +0100 Subject: [PATCH 5/5] MOL-1196: update comment --- src/Service/TrackingInfoStructFactory.php | 32 +++++++--- .../Service/TrackingInfoStructFactoryTest.php | 58 +++++++++++++++++-- 2 files changed, 77 insertions(+), 13 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..b747f326a 100644 --- a/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php +++ b/tests/PHPUnit/Service/TrackingInfoStructFactoryTest.php @@ -66,6 +66,7 @@ public function testOnlyOneCodeAccepted(): void $deliveryEntity->setShippingMethod($shippingMethod); $trackingInfoStruct = $this->factory->createFromDelivery($deliveryEntity); + $this->assertNull($trackingInfoStruct); } @@ -83,12 +84,59 @@ public function testInfoStructCreatedByArguments(): void } - public function testExceptionIsThrownWithoutPlaceholderInUrl(): void + public function testUrlWithCodeIsInvalid(): void { - $this->expectException(\InvalidArgumentException::class); - $trackingInfoStruct = $this->factory->create('Test', 'testCode', 'https://test.foo?code'); + $expectedCode = '/123 4%foo=bar?test'; + $expectedCarrier = 'Test carrier'; + $trackingInfoStruct = $this->factory->create($expectedCarrier, $expectedCode, 'https://test.foo?code=%s'); + $expectedUrl = ''; - $this->assertNull($trackingInfoStruct); + $this->assertNotNull($trackingInfoStruct); + $this->assertSame($expectedCode, $trackingInfoStruct->getCode()); + $this->assertSame($expectedUrl, $trackingInfoStruct->getUrl()); + $this->assertSame($expectedCarrier, $trackingInfoStruct->getCarrier()); + } + + 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->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()); } /** @@ -102,6 +150,7 @@ public function testInvalidTrackingCodeCharacter(string $trackingCode): void $trackingInfoStruct = $this->factory->create('test', $trackingCode, 'https://foo.bar/%s'); $expected = ''; + $this->assertSame($expected, $trackingInfoStruct->getUrl()); } @@ -114,6 +163,7 @@ public function invalidCodes(): array ['somecode'], ['some#code'], + ['some#<>{},' . str_repeat('1', 200)], [str_repeat('1', 200)], ]; }