Create new 'abandoned' order status.

   if (!$order || ...) {
+    uc_order_update_status($order->order_id, 'abandoned');

If $order is FALSE for some reason, trying to update its status will result in an error.

I also don't think we should touch the order status if it's not in_checkout for some reason, or if the uid changes. Perhaps the customer called in to complete the order, and the admin manually changed the status while it's still in the customer's session.

I am not really sure what that uid check is meant to achieve in the first place, or what would trigger it to fail.

Patch failure is "Cannot redeclare uc_order_update_7004()"

This is because there's already a uc_order_update_7004() defined in uc_order.install. The hook_update_N() added by the patch needs to be renamed uc_order_update_7005().

@wodenx: Can you post an updated patch?

+    if ($order && $order->status == 'in_checkout' && $order->modified < REQUEST_TIME - 600) {

Shouldn't that be $order->order_status? Let's also get that "600" moved to a constant rather than using magic numbers.

Would be nice to get a test added for this, it would have to update $order->modified manually to avoid the test having to wait ten minutes, but otherwise should be fairly easy to write.

This looks good and works well in testing.

I wonder if we should have a hook_update_N() to do the initial update here, setting the order_status directly; on sites with many "in checkout" orders (perhaps upgrades from D6) the first cron run might take a long time.

@wodenx: Thanks for adding the tests - that more than anything helps me see how this is supposed to work and convinces me that it works properly.

I agree with @longwave in #12 that the initial update of the status should be done as a batch in a hook_update_N() rather than leaving it to the first run of hook_cron(). Otherwise, I think this is ready to go in.

As this is a new status, nothing should be expecting to react on it, so I think we can safely abandon old orders in a single query.

Tested and works for me on a site which had ~500 In Checkout orders. Update successfully moved all In Checkout to Abandoned. New In Checkout orders were moved to Abandoned via cron after 10 minutes.

.module files are not guaranteed to be available during hook_update_N() unless loaded explicitly, which is why I hardcoded the constant. I think the timeout should belong to uc_cart rather than uc_order; the "in checkout" and "abandoned" statuses should probably technically be owned by uc_cart as well, but it's not worth fixing that now.

Back to RTBC for #14, will commit later.

Committed #14.

Hey not sure if I should re-open or open my own issue, but it looks like something was missed in all of this:

#10:

Let's also get that "600" moved to a constant rather than using magic numbers.

It looks like 600 is still hard coded into uc_cart.pages.inc in the latest branch.

Patch attached

See #18 and #19. Test your patch with the relevant modules disabled and see if it still works.

I disabled uc_cart -- ran update.php, no problems
I disabled uc_order -- ran update.php, no problems

although i didn't have any updates to perform.. does that matter?

Committed, thanks. This is valid; the only place the constant should not be used is in the update hook in uc_order.install.