Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MOL-1196: Tracking #607

Merged
merged 6 commits into from
Dec 4, 2023
Merged

MOL-1196: Tracking #607

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d548cb8
MOL-1196: Tracking
Jul 31, 2023
cfd494e
NTR: Merge branch 'master' into users/vm/MOL-1196-tracking
Nov 27, 2023
d5d81db
MOL-1196: fix pipeline
Nov 27, 2023
8f63450
MOL-1196: extract to factory
Nov 27, 2023
2dbf5af
MOL-1196: check tracking code lenght
Nov 28, 2023
c947525
MOL-1196: update comment
Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 0 additions & 128 deletions src/Event/MollieOrderShipmentTrackingEvent.php
View file
Open in desktop

This file was deleted.

36 changes: 15 additions & 21 deletions src/Facade/MollieShipment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,6 +21,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;
Expand Down Expand Up @@ -61,11 +63,17 @@ class MollieShipment implements MollieShipmentInterface
*/
private $orderDataExtractor;

/**
* @var TrackingInfoStructFactory
*/
private $trackingInfoStructFactory;

/**
* @var LoggerInterface
*/
private $logger;


/**
* @param MolliePaymentExtractor $extractor
* @param DeliveryTransitionServiceInterface $deliveryTransitionService
Expand All @@ -76,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;
Expand All @@ -86,6 +94,7 @@ public function __construct(MolliePaymentExtractor $extractor, DeliveryTransitio
$this->orderService = $orderService;
$this->orderDataExtractor = $orderDataExtractor;
$this->logger = $logger;
$this->trackingInfoStructFactory = $trackingInfoStructFactory;
}

/**
Expand Down Expand Up @@ -170,7 +179,9 @@ public function setShipment(string $orderDeliveryId, Context $context): bool
return false;
}

$addedMollieShipment = $this->mollieApiOrderService->setShipment($mollieOrderId, $order->getSalesChannelId());
$trackingInfoStruct = $this->trackingInfoStructFactory->createFromDelivery($delivery);

$addedMollieShipment = $this->mollieApiOrderService->setShipment($mollieOrderId, $trackingInfoStruct, $order->getSalesChannelId());

if ($addedMollieShipment) {
$values = [CustomFieldsInterface::DELIVERY_SHIPPED => true];
Expand Down Expand Up @@ -250,7 +261,7 @@ public function shipOrder(
$shipment = $this->mollieApiShipmentService->shipOrder(
$mollieOrderId,
$order->getSalesChannelId(),
$this->createTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl)
$this->trackingInfoStructFactory->create($trackingCarrier, $trackingCode, $trackingUrl)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really wondering, this PR is about automating tracking in whatever shipping is being done.
so if we extract tracking from delivery in the constructor....then i would assume its used somewhere, but i only see function arguments being used here, and the calling instance doesnt seem to have changed which means its the old logic? so where is the auto-extracted data then used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the shipment facade is too big and too much things, the tests are too complicated, i wanted to extract some private methods into seperate services in order to write smaller tests. the factory is not used anywhere else but it makes the tests easier

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesnt really give an answer to the question
i think we should talk about it :)

);

$delivery = $this->orderDataExtractor->extractDelivery($order, $context);
Expand Down Expand Up @@ -371,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);
Expand Down Expand Up @@ -463,21 +474,4 @@ private function findMatchingLineItems(OrderEntity $order, string $itemIdentifie
return false;
});
}

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!');
}

return new ShipmentTrackingInfoStruct($trackingCarrier, $trackingCode, $trackingUrl);
}
}
3 changes: 3 additions & 0 deletions src/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@
<argument key="$envAppUrl">%env(default::APP_URL)%</argument>
</service>

<service id="Kiener\MolliePayments\Service\TrackingInfoStructFactory"/>


</services>

</container>
2 changes: 2 additions & 0 deletions src/Resources/config/services/facades.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@
<argument type="service" id="Kiener\MolliePayments\Service\OrderDeliveryService"/>
<argument type="service" id="Kiener\MolliePayments\Service\OrderService"/>
<argument type="service" id="Kiener\MolliePayments\Service\MollieApi\OrderDataExtractor"/>
<argument type="service" id="Kiener\MolliePayments\Service\TrackingInfoStructFactory"/>
<argument type="service" id="mollie_payments.logger"/>
</service>


<service id="Kiener\MolliePayments\Facade\MolliePaymentDoPay" public="true">
<argument type="service" id="Kiener\MolliePayments\Service\MollieApi\OrderDataExtractor"/>
<argument type="service" id="Kiener\MolliePayments\Service\MollieApi\Builder\MollieOrderBuilder"/>
Expand Down
5 changes: 0 additions & 5 deletions src/Resources/config/services/subscriber.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@
<tag name="kernel.event_subscriber"/>
</service>

<service id="Kiener\MolliePayments\Subscriber\ShippingSubscriber">
<argument type="service" id="Kiener\MolliePayments\Facade\MollieShipment"/>
<argument type="service" id="mollie_payments.logger"/>
<tag name="kernel.event_subscriber"/>
</service>

<service id="Kiener\MolliePayments\Subscriber\SystemConfigSubscriber">
<argument type="service" id="Kiener\MolliePayments\Service\SettingsService"/>
Expand Down
12 changes: 9 additions & 3 deletions src/Service/MollieApi/Order.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -391,14 +394,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);
boxblinkracer marked this conversation as resolved.
Show resolved Hide resolved

return true;
}
Expand Down
1 change: 1 addition & 0 deletions src/Service/OrderDeliveryService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
54 changes: 54 additions & 0 deletions src/Service/TrackingInfoStructFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php
declare(strict_types=1);

namespace Kiener\MolliePayments\Service;

use Kiener\MolliePayments\Struct\MollieApi\ShipmentTrackingInfoStruct;
use Shopware\Core\Checkout\Order\Aggregate\OrderDelivery\OrderDeliveryEntity;

class TrackingInfoStructFactory
boxblinkracer marked this conversation as resolved.
Show resolved Hide resolved
{
public function createFromDelivery(OrderDeliveryEntity $orderDeliveryEntity):?ShipmentTrackingInfoStruct
{
$trackingCodes = $orderDeliveryEntity->getTrackingCodes();
$shippingMethod = $orderDeliveryEntity->getShippingMethod();
if ($shippingMethod === null) {
return null;
}
if (count($trackingCodes) !== 1) {
return null;
boxblinkracer marked this conversation as resolved.
Show resolved Hide resolved
}

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not work, thats just a string concat
in sw5 there was a placeholder being allowed for specific values
so we need to figure out if its the same, or come up with something like {trackingCode}, and then this would need to replaced, a basic concat leads to an invalid url very likely

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in SW6 there is trackingCode variable just in email, the URL itself does not have a tracking Code

https://docs.shopware.com/en/shopware-6-en/tutorials-and-faq/track-and-trace it seems like sw6 expects that the tracking code is at the end of the URL

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, the info icon says clearly its the placeholder %s


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);
}
}
Loading
Loading