mollie / Shopware6

47 stars 52 forks source link

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

Closed mbegerau closed 4 years ago

mbegerau commented 4 years ago

This is a follow up to #21

Environment

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

Problem

In the administration I open an order which was paid by a Mollie payment method (credit card in this case). When I change the delivery state to "shipped" I get an 500 error.

image: DevTools: Error when changing delivery state to "shipped" non-existent-query-parameter

The error message is: "Error executing API call (422: Unprocessable Entity): Non-existent query parameter "mode" for this API call."

The API documentation states that "testmode" (if OAuth) and "embed" are valid query string parameters: https://docs.mollie.com/reference/v2/orders-api/get-order

Solution suggestion

The error can be removed by changing OrderDeliverySubscriber.php

117                    $parameters = [
118                        'mode' => 'live',
119                    ];

and

136                    $parameters = [
137                        'mode' => 'test',
138                    ];

to $parameters = [];

The "testmode" parameter should be valid and can stay there, I think.

Follow-up error

The follow-up error I described in #21 is still in place. When I apply the solution to the problem above I still get an exception because the updateDelivery() in line 167 will trigger the OrderDeliveryWritten event again. This would end up in an infinite loop but the shipAll() in line 164 already throws an exception because it can't run twice for the same order.

Solution

As I wrote in #21 there are two ways: a) Use the StateMachineTransitionEvent instead of the ORDER_DELIVERY_WRITTEN_EVENT. (would require lots of changes) b) Use some other condition to make sure that on the second run the shipAll function (and the update function) is not called again. (see example below)

For example right now we hotfix this problem by adding this condition:

85            if (empty($payload['stateId'])) {
86                continue;
87            }

This ensures that the loop only runs when a state change is going to be saved.

image: Suggested solution: Adding a condition at the beginning of the foreach loop payload-condition

GerDner commented 4 years ago

i can confirm the bug and the hotfix in OrderDeliverySubscriber.php

GerDner commented 4 years ago

this pretty much breaks Mollie for all Shopware 6 Shops. We use Mollie and DHL Shipping Plugin. And we encounter this error on every action in the shopware backend which changes the Order (Packet Labels, Invoice generation, etc.)

iamreinder commented 4 years ago

Thanks for your hard work. I've released a fix in version 1.0.15 of the Mollie Payments plugin for Shopware 6. Please let me know if this works out for you.

mbegerau commented 4 years ago

Hey, I just tested it with Shopware 6.2.2 and Mollie 1.0.15: And everything works! Thanks for applying my suggestion. And also thanks for the Apple Pay fix!

iamreinder commented 4 years ago

Great to hear this worked out. I'm also pretty pleased with the Apple Pay fix 😊