mollie / Shopware6

MIT License
51 stars 54 forks source link

Changing the delivery status of an order throws an exception #21

Closed mbegerau closed 4 years ago

mbegerau commented 4 years ago

Environment

OS: Ubuntu 18.04 Apache: 2.4.29 PHP: 7.3.17 MySQL: 5.7.30 Shopware: 6.1.5 (fresh installation from production repository) Mollie: 1.0.10 Other Plugins: Shopware 6 Demo data (1.0.5)

Problem

In the administration if we change the delivery status of an order that used a mollie payment method an error occurs.

Image: Shipping status change error in administration shipping_status_change_error

The error trace says: Expected input to be non empty non associative array. It traces back to: /custom/plugins/MolliePayments/src/Service/DeliveryService.php line 129 and /custom/plugins/MolliePayments/src/Subscriber/OrderDeliverySubscriber.php line 138

Image: Trace in dev tools network tab error_tracing

Detected causes

custom/plugins/MolliePayments/src/Service/DeliveryService.php, lines 127-130

127        return $this->getRepository()->update(
128            $data,
129            $context ?? Context::createDefaultContext()
130        );

The update function expects an array of dataset. Here the data is an associative array of one dataset.

Solution suggestion

custom/plugins/MolliePayments/src/Service/DeliveryService.php Change "$data" in line 128 to: "[$data]"

Follow-up error

After that fix there appears another error. The now working update function itself triggers the ORDER_DELIVERY_WRITTEN_EVENT again before "shipped" => true is saved in the custom fields. Then the $mollieOrder->shipAll() (line 132) throws an exception because this information already has been sent to Mollie. Even if this error wouldn't be thrown this would end in an infinite loop because the update function would always trigger the ORDER_DELIVERY_WRITTEN_EVENT which would call the update function.

Solution suggestion

a) Use the StateMachineTransitionEvent instead of the ORDER_DELIVERY_WRITTEN_EVENT. b) Use some other condition to make sure that on the second run the shipAll function (and the update function) is not called again.

iamreinder commented 4 years ago

We used your solution in the current release, see: https://github.com/mollie/Shopware6/blob/master/src/Service/DeliveryService.php#L128

mbegerau commented 4 years ago

Hey, thanks for your work. Unfortunately only the first part was fixed, the follow-up error still remains and a new error was added.

I'm kind of new to Github and can't find a way to reopen this issue so I created a new issue: #26