wagnerwagner / merx

Merx is a plugin to create online shops with Kirby.
https://merx.wagnerwagner.de
102 stars 10 forks source link

Severe errors after deleting orders (Sorting number of a complete order cannot be changed) #28

Open plagasul opened 3 years ago

plagasul commented 3 years ago

Trying to delete an order (such as a test sandbox order) results in the error message "Sorting number of a complete order cannot be changed." on top of the delete confirmation popup.

image

Clicking Delete again results in 'the page cannot be found' error:

image

...but deletes the order page.

Then errors start to happen. ->

  1. On successive orders , order numbers are being repeated.

image

  1. At least on localhost, successive orders, after confirming paypal (in my tests), are redirected to success page BUT returning the same exception "Sorting number of a complete order cannot be changed."

So, as this is within a try / catch clause, apparently merx()->completePayment($_GET); fails.

BUT the order appears in the panel orders page, and its payment show as complete.

It fails, but it completes.

Thank you

plagasul commented 3 years ago

A workaround to allow orders to be placed again.

Attention, this WILL change orders sorting number, and so most possibly the order number shown to the customer.

  1. Under plugins/merx/index.php comment lines 103 to 112, inclusive (two exceptions thrown when attempting to modify num or status of an order page)

  2. Run the folowing script or equivalent, to change the sorting number of pages , so it starts at 1 and grows consecutively, according to invoiceDate:

<?php
$ordersByDate = page('orders')->children()->listed()->sortBy('invoiceDate', 'asc');
for ($i = 0; $i < $ordersByDate->count(); $i++ ) {
    $newSortNumber = $i+1;
    try {
        $ordersByDate->nth($i)->changeNum($newSortNumber);
    } catch (Exception $e) {
        echo 'Exception: ' . $e->getMessage() . '<br />';
    }
}
  1. Under plugins/merx/index.php UNcomment lines 103 to 112, inclusive

That fixes merx()->completePayment() returning the exception. Orders can be placed again.

tobiasfabian commented 3 years ago

Hey @plagasul,

thank you for your detailed comments.

What you experienced is somehow intended behavior. It’s intended that orders may not be deleted because

That’s why I added the delete: false option to the order page blueprint and additionally added these hooks you’ve mentioned (merx/index.php#L103-L112). I should have added an additional page.delete:before hook to prevent delete order pages. This would resulted in one error (e.g. Order pages must not be deleted) instead of several errors.

If you really want to delete test orders you should delete the order pages via ftp or ssh.


Another way to test orders could be a separate domain (test.domain.tld) wich points to the same kirby installation. You can set a custom configuration with a custom orders page.

// site/config.test.domain.tld.php

return [
  'ww.merx.ordersPage' => 'test-orders',
];

What do you think, I’m open to your opinions. Should order pages be deletable?

plagasul commented 3 years ago

Thanks for answering @tobiasfabian

I believe it would be enough that orders can't really be deleted at all, as in this case the user was able to delete them by merely pressing the button once again.

Perhaps a warning somewhere in the docs so we can prepare for this with an approach such as the one you are proposing. Thanks for that.

tobiasfabian commented 3 years ago

Similar discussion on Kirby forum.
https://forum.getkirby.com/t/merx-plugin-to-create-online-shops-with-kirby-3/13735/206