From ccde869b6e096604e0fe4d939cbedd7b64e709e7 Mon Sep 17 00:00:00 2001 From: Vitalij Mik Date: Mon, 9 Dec 2024 09:52:46 +0100 Subject: [PATCH] NTR: PISHPS-395: ship even without tracking data (#909) Co-authored-by: Vitalij Mik --- .../ShipmentManager/Models/TrackingData.php | 12 +++ .../ShipmentManager/ShipmentManager.php | 80 +++++++++++-------- src/Resources/config/services/facades.xml | 1 + src/Service/TrackingInfoStructFactory.php | 11 +-- .../ShipmentManager/ShipmentManagerTest.php | 7 +- 5 files changed, 70 insertions(+), 41 deletions(-) diff --git a/src/Components/ShipmentManager/Models/TrackingData.php b/src/Components/ShipmentManager/Models/TrackingData.php index b330e8782..5c8e73317 100644 --- a/src/Components/ShipmentManager/Models/TrackingData.php +++ b/src/Components/ShipmentManager/Models/TrackingData.php @@ -55,4 +55,16 @@ public function getTrackingUrl(): string { return $this->trackingUrl; } + + /** + * @return array + */ + public function toArray(): array + { + return [ + 'carrier' => $this->carrier, + 'code' => $this->code, + 'tracking_url' => $this->trackingUrl, + ]; + } } diff --git a/src/Components/ShipmentManager/ShipmentManager.php b/src/Components/ShipmentManager/ShipmentManager.php index d3f017cac..0abceb07a 100644 --- a/src/Components/ShipmentManager/ShipmentManager.php +++ b/src/Components/ShipmentManager/ShipmentManager.php @@ -17,8 +17,10 @@ use Kiener\MolliePayments\Service\OrderService; use Kiener\MolliePayments\Service\TrackingInfoStructFactory; use Kiener\MolliePayments\Service\Transition\DeliveryTransitionServiceInterface; +use Kiener\MolliePayments\Struct\MollieApi\ShipmentTrackingInfoStruct; use Kiener\MolliePayments\Struct\Order\OrderAttributes; use Kiener\MolliePayments\Struct\OrderLineItemEntity\OrderLineItemEntityAttributes; +use Psr\Log\LoggerInterface; use Shopware\Core\Checkout\Cart\LineItem\LineItem; use Shopware\Core\Checkout\Order\Aggregate\OrderLineItem\OrderLineItemCollection; use Shopware\Core\Checkout\Order\Aggregate\OrderLineItem\OrderLineItemEntity; @@ -66,6 +68,11 @@ class ShipmentManager implements ShipmentManagerInterface */ private $trackingFactory; + /** + * @var LoggerInterface + */ + private $logger; + /** * @param DeliveryTransitionServiceInterface $deliveryTransitionService * @param Order $mollieApiOrderService @@ -75,7 +82,7 @@ class ShipmentManager implements ShipmentManagerInterface * @param OrderItemsExtractor $orderItemsExtractor * @param TrackingInfoStructFactory $trackingFactory */ - public function __construct(DeliveryTransitionServiceInterface $deliveryTransitionService, Order $mollieApiOrderService, ShipmentInterface $shipmentService, OrderService $orderService, OrderDeliveryExtractor $orderDataExtractor, OrderItemsExtractor $orderItemsExtractor, TrackingInfoStructFactory $trackingFactory) + public function __construct(DeliveryTransitionServiceInterface $deliveryTransitionService, Order $mollieApiOrderService, ShipmentInterface $shipmentService, OrderService $orderService, OrderDeliveryExtractor $orderDataExtractor, OrderItemsExtractor $orderItemsExtractor, TrackingInfoStructFactory $trackingFactory, LoggerInterface $logger) { $this->deliveryTransitionService = $deliveryTransitionService; $this->mollieApiOrderService = $mollieApiOrderService; @@ -84,6 +91,7 @@ public function __construct(DeliveryTransitionServiceInterface $deliveryTransiti $this->orderDataExtractor = $orderDataExtractor; $this->orderItemsExtractor = $orderItemsExtractor; $this->trackingFactory = $trackingFactory; + $this->logger = $logger; } @@ -130,8 +138,8 @@ public function getTotals(string $orderId, Context $context): array * @param null|TrackingData $tracking * @param array $shippingItems * @param Context $context - * @throws NoDeliveriesFoundException * @throws NoLineItemsProvidedException + * @throws NoDeliveriesFoundException * @return \Mollie\Api\Resources\Shipment */ public function shipOrder(OrderEntity $order, ?TrackingData $tracking, array $shippingItems, Context $context): \Mollie\Api\Resources\Shipment @@ -141,15 +149,7 @@ public function shipOrder(OrderEntity $order, ?TrackingData $tracking, array $sh } - if ($tracking instanceof TrackingData) { - $trackingData = $this->trackingFactory->create( - $tracking->getCarrier(), - $tracking->getCode(), - $tracking->getTrackingUrl() - ); - } else { - $trackingData = $this->trackingFactory->trackingFromOrder($order); - } + $trackingData = $this->getTrackingInfoStruct($tracking, $order); $orderAttr = new OrderAttributes($order); @@ -208,15 +208,7 @@ public function shipOrder(OrderEntity $order, ?TrackingData $tracking, array $sh */ public function shipOrderRest(OrderEntity $order, ?TrackingData $tracking, Context $context): \Mollie\Api\Resources\Shipment { - if ($tracking instanceof TrackingData) { - $trackingData = $this->trackingFactory->create( - $tracking->getCarrier(), - $tracking->getCode(), - $tracking->getTrackingUrl() - ); - } else { - $trackingData = $this->trackingFactory->trackingFromOrder($order); - } + $trackingData = $this->getTrackingInfoStruct($tracking, $order); $orderAttr = new OrderAttributes($order); @@ -262,20 +254,12 @@ public function shipItem(OrderEntity $order, string $itemIdentifier, int $quanti $lineItem = $lineItems->first(); unset($lineItems); - if (!$lineItem instanceof OrderLineItemEntity) { + if (! $lineItem instanceof OrderLineItemEntity) { throw new OrderLineItemNotFoundException($itemIdentifier); } - if ($tracking instanceof TrackingData) { - $mollieTracking = $this->trackingFactory->create( - $tracking->getCarrier(), - $tracking->getCode(), - $tracking->getTrackingUrl() - ); - } else { - $mollieTracking = $this->trackingFactory->trackingFromOrder($order); - } + $mollieTracking = $this->getTrackingInfoStruct($tracking, $order); $mollieOrderLineId = $this->orderService->getMollieOrderLineId($lineItem); @@ -352,7 +336,7 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie // If it's not a "product" type lineItem, for example if it's a completely custom lineItem type, // check if the payload has a productNumber in it that matches the itemIdentifier. - if (!empty($lineItem->getPayload()) && + if (! empty($lineItem->getPayload()) && array_key_exists('productNumber', $lineItem->getPayload()) && $lineItem->getPayload()['productNumber'] === $itemIdentifier) { return true; @@ -361,12 +345,12 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie // Check itemIdentifier against the mollie order_line_id custom field $customFields = $lineItem->getCustomFields() ?? []; $mollieOrderLineId = $customFields[CustomFieldsInterface::MOLLIE_KEY][CustomFieldsInterface::ORDER_LINE_KEY] ?? null; - if (!is_null($mollieOrderLineId) && $mollieOrderLineId === $itemIdentifier) { + if (! is_null($mollieOrderLineId) && $mollieOrderLineId === $itemIdentifier) { return true; } // If it hasn't passed any of the above tests, check if the itemIdentifier is a valid Uuid... - if (!Uuid::isValid($itemIdentifier)) { + if (! Uuid::isValid($itemIdentifier)) { return false; } @@ -380,4 +364,34 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie return false; }); } + + /** + * @param null|TrackingData $tracking + * @param OrderEntity $order + * @throws NoDeliveriesFoundException + * @return null|ShipmentTrackingInfoStruct + */ + private function getTrackingInfoStruct(?TrackingData $tracking, OrderEntity $order): ?ShipmentTrackingInfoStruct + { + try { + if ($tracking instanceof TrackingData) { + $trackingData = $this->trackingFactory->create( + $tracking->getCarrier(), + $tracking->getCode(), + $tracking->getTrackingUrl() + ); + } else { + $trackingData = $this->trackingFactory->trackingFromOrder($order); + } + return $trackingData; + } catch (\InvalidArgumentException $exception) { + $loggerData = ['exception' => $exception->getMessage(), 'order' => $order->getOrderNumber()]; + if ($tracking instanceof TrackingData) { + $loggerData['tracking'] = $tracking->toArray(); + } + + $this->logger->warning('Failed to get tracking information', $loggerData); + return null; + } + } } diff --git a/src/Resources/config/services/facades.xml b/src/Resources/config/services/facades.xml index a80790453..8c38d5f02 100644 --- a/src/Resources/config/services/facades.xml +++ b/src/Resources/config/services/facades.xml @@ -14,6 +14,7 @@ + diff --git a/src/Service/TrackingInfoStructFactory.php b/src/Service/TrackingInfoStructFactory.php index 5d4b75dcc..d85fe20b3 100644 --- a/src/Service/TrackingInfoStructFactory.php +++ b/src/Service/TrackingInfoStructFactory.php @@ -100,6 +100,12 @@ public function create(string $trackingCarrier, string $trackingCode, string $tr */ private function createInfoStruct(string $trackingCarrier, string $trackingCode, string $trackingUrl): ?ShipmentTrackingInfoStruct { + $this->logger->debug('Creating tracking information for shipment.', [ + 'trackingCarrier' => $trackingCarrier, + 'trackingCode' => $trackingCode, + 'trackingUrl' => $trackingUrl + ]); + if (empty($trackingCarrier) && empty($trackingCode)) { $this->logger->debug('No tracking information provided for shipment.'); return null; @@ -113,11 +119,6 @@ private function createInfoStruct(string $trackingCarrier, string $trackingCode, throw new \InvalidArgumentException('Missing Argument for Tracking Code!'); } - $this->logger->debug('Creating tracking information for shipment.', [ - 'trackingCarrier' => $trackingCarrier, - 'trackingCode' => $trackingCode, - 'trackingUrl' => $trackingUrl - ]); // determine if the provided tracking code is actually a tracking URL if (empty($trackingUrl) === true && $this->urlParsingService->isUrl($trackingCode)) { diff --git a/tests/PHPUnit/Components/ShipmentManager/ShipmentManagerTest.php b/tests/PHPUnit/Components/ShipmentManager/ShipmentManagerTest.php index c937fba96..911abc2e2 100644 --- a/tests/PHPUnit/Components/ShipmentManager/ShipmentManagerTest.php +++ b/tests/PHPUnit/Components/ShipmentManager/ShipmentManagerTest.php @@ -51,11 +51,11 @@ class ShipmentManagerTest extends TestCase public function setUp(): void { $this->fakeShipmentService = new FakeShipment(); - + $logger = new NullLogger(); $deliveryTransitionService = $this->createMock(DeliveryTransitionService::class); $mollieApiOrderService = $this->getMockBuilder(Order::class)->disableOriginalConstructor()->getMock(); $orderService = $this->getMockBuilder(OrderService::class)->disableOriginalConstructor()->getMock(); - $deliveryExtractor = new OrderDeliveryExtractor(new NullLogger()); + $deliveryExtractor = new OrderDeliveryExtractor($logger); $this->shipmentManager = new ShipmentManager( $deliveryTransitionService, @@ -64,7 +64,8 @@ public function setUp(): void $orderService, $deliveryExtractor, new OrderItemsExtractor(), - new TrackingInfoStructFactory(new UrlParsingService(), new NullLogger()) + new TrackingInfoStructFactory(new UrlParsingService(), $logger), + $logger ); $this->context = $this->getMockBuilder(Context::class)->disableOriginalConstructor()->getMock();