hook_order can_update operation in uc_order_update_status never returns false

Had these same issues. Your patch seemed to fix the issue. Thanks!

The "Order updated" message is still valid if you add a comment, which should still be possible even if you can't change the order status.

Because hook_order() takes the $order object by reference, module_invoke_all() causes notices or E_STRICT errors in PHP 5.3. Since that part can't change, we need to change the rest of the code to expect a single value rather than an array.

@mradcliffe: Did you ever try out the patch in #3 to see if it fixes your problem?

"Should" or "does" work the same? The reason I ask is I don't want to commit the patch and close the issue unless I know it's really fixed. I would like this exact patch to be applied then tested in the same environment where the original problem was showing up.

The patch in #3 looks wrong to me, that's why I'm questioning whether it really fixes the problem. In particular, the patch results in the following code:

       if (function_exists($function) && ($return = $function('can_update', $order, $status))){
        if ($return === FALSE) {
          return FALSE;
        }
      }

and it looks to me like this will *never* return FALSE because if the result from $function() is FALSE the outer conditional will fail and the inner return statement will never be reached. If the patch had come from anyone but Island Usurper I would have rejected it outright, but he generally gets things right ...

I think it should look like this:

       if (function_exists($function)) {
        $return = $function('can_update', $order, $status);
        if ($return === FALSE) {
          return FALSE;
        }
      }

@TR, you're quite right. The patch from #3 doesn't solve the problem at all (other than to prevent array errors for trying to loop over a bool).

I've attached a patch along the lines of #6 that fixes the problem for me.

I have the same problem and #7 fixes it.

I don't know what I was thinking in #3. Obviously not paying attention then.

I need someone to actually test this, though, because I don't have anything that prevents orders from being updated. Probably means I ought to write a test. :P

@Island Usurper
Why don't you try patch in #7?

Oh, you did say you tested it, didn't you? OK, that works for me.

Great. Committed.

Yes #7 works also for me.

Have just upgraded from 2.3 to 2.4

We had applied patch 624670-uc_order.module-can_update-7.patch - refer #7

I notice that I need to apply the patch again - there was no issue doing so - and the capability is now working correctly!

Wanted to have this written up for any others who have applied the patch, and are planning to upgrade to 2.4

We make considerable use of this can_update case to validate changes to order status with the state of the orders sales tax calculations in an external system being used.