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.

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

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

Open
plagasul opened this issue Dec 12, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@plagasul
Copy link

plagasul commented Dec 12, 2020

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
Copy link
Author

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
Copy link
Member

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

  • they contain very important information such as payment information and/or shipping/contact details.
  • as a side effect order numbers should not change (otherwise it would certainly end up in a total mess)

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
Copy link
Author

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 tobiasfabian added the bug Something isn't working label Jan 12, 2021
@tobiasfabian
Copy link
Member

@tobiasfabian tobiasfabian added enhancement New feature or request and removed bug Something isn't working labels Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants