I was trying to implement the hook_order to disable the possibility to delete orders.
In the UC API docs it said to return false on case 'can_delete' but I couldn't get it to work.
I also found an example of this implemented in uc_payment so that if a payment is received an order couldn't be deleted so I tested this and I could delete orders that had payments.

so I dug into the uc_order.module and I found an error in the uc_invoice_order function on line 1839:

1836: foreach (module_list() as $module) {
1837: $function = $module .'_order';
1838: // $order must be passed by reference.
1839: if (function_exists($function) && ($response = $function('can_delete', $order, NULL))) {
1840: // Break out early if possible.
1841: if ($response === FALSE) {
1842: $can_delete = FALSE;
1843: break;
1845: }
1846: }

if I return FALSE in my implementation of hook_order this is also FALSE:
($response = $function('can_delete', $order, NULL))

solved it by adding a NOT in front and then all worked fine:
1839: if (function_exists($function) && !($response = $function('can_delete', $order, NULL))) {

CommentFileSizeAuthor
#10 1112550-hook_order-can_delete.patch898 byteslongwave
#9 1112550-hook_order-can_delete.patch951 byteslongwave

Comments

jan_ver’s picture

jan_ver commented
Title: ok_order can_delete error » hook_order can_delete error
jan_ver’s picture

jan_ver commented

This piece of code works too and looks better (to me):

foreach (module_list() as $module) {
$function = $module .'_order';
// $order must be passed by reference.
if (function_exists($function)) {
if (($function('can_delete', $order, NULL)) === FALSE) {
// Break out early if possible.
$can_delete = FALSE;
break;
}
}
}

I would create a patch but I'm not yet familiar with git and don't have time to get into that right now

tr’s picture

tr commented

@jan_ver: You don't need git to make a patch, you can use diff -up <original-file> <modified-file>.

I'll have to search, but I think this has been pointed out before - there is bad logic surrounding 'can_ship', 'can_update', and 'can_delete' that seems to have been copied into many different places in Ubercart. If I can find those issues I'll consolidate them and we can take care of all of them at once.

tr’s picture

tr commented

The problem and fix is outlined in @sammys patch at http://drupal.org/node/613498#comment-2895096

Other issues that mention this same problem are:
#574066: Object-valued by-reference parameters in Ubercart - Trouble with PHP 5.3 call_user_func_array
#613498: uc_product_add_to_cart_data does not respect non-shippable attributes
#624670: hook_order can_update operation in uc_order_update_status never returns false
#977682: uc_order_is_shippable doesn't let see if any other modules have a say in the matter of shippability
#713054: some orders incorrectly identified as shippable

jan_ver’s picture

jan_ver commented

@TR Thanks for your reply, I did search for existing issues but not for issues with shipping. The bad logic surrounding 'can_ship' was already fixed in UC 2.4 (don't know about 'can_update')

adTumbler’s picture

adTumbler commented

There is a patch for can_update that is required in 2.4 for it to work.

624670-uc_order.module-can_update-7.patch

I am not an expert, but despite implementing both the suggested changes above - I am still not able to get it to respond and do the work I am asking it to do prior to deleting an order.

I would appreciate direction on where can-delete is broken - am deleting an order from the view of all orders - case can_delete is not being called.

Alex

adTumbler’s picture

adTumbler commented
Status: Active » Closed (won't fix)

Closed - more than a year without comment

Issue no longer relevant - will address advanced requirements for AvaTax integration in current versions.

tr’s picture

tr commented
Version: 6.x-2.4 » 6.x-2.x-dev
Status: Closed (won't fix) » Active

It's still broken isn't it?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 1112550-hook_order-can_delete.patch951 bytes

I guess nobody really uses this 'can_delete' hook, but this patch should fix the issue. Also needs porting to 7.x.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 1112550-hook_order-can_delete.patch898 bytes

This is closer to the existing code in 6.x.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 6.x-2.x-dev » 7.x-3.x-dev
Status: Needs review » Patch (to be ported)

Confirmed that #10 works, committed to 6.x.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Patch (to be ported) » Fixed

Ported and committed to 7.x in http://drupalcode.org/project/ubercart.git/commitdiff/5297f4a

tr’s picture

tr commented

@longwave - can you take a quick glance at the use of can_ship, can_update, and can_delete throughout the Ubercart codebase? This faulty if() statement was copied for all of these cases, in a number of different places. It would be nice to fix all of them in an identical manner all at once, rather than just address can_delete in just this one place.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

can_update was fixed previously; can_delete is now handled in the same manner by the changes above.

The can_ship code also looks like it should work in 6.x, and has been replaced in 7.x with a separate hook, however #613498: uc_product_add_to_cart_data does not respect non-shippable attributes is still valid - related to can_ship, but rather an unfortunate design flaw that is more complicated to fix.

tr’s picture

tr commented

Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.