Posted by longwave on March 25, 2011 at 5:14pm
3 followers
Jump to:
| Project: | Ubercart |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Issue Summary
Spawned from #1059804-11: Notice Undefined property stdClass::$type in uc_product_kit_uc_order_product_alter()
This hook is called both when customer carts are converted to orders at checkout, and also when products are added to existing orders from the admin pages, but the error raised in the linked issue was due to the fact that these two calling points are inconsistent.
Comments
#1
In fact, it is called every single time anything is modified on the checkout page. Presumably, it's triggered via the AJAX calls. Since I wanted to make a modification to the product just once, I was required to put in a check to see if it had already been modified.
In any case, the solution to the issue is to add a parameter to the hook invocation, something like:
<?phphook_uc_order_product_alter(&$product, $order, $context = 'checkout')
?>
...where $context can be 'checkout' or 'order_admin'. By adding an argument with a default, existing code won't break, and new code can tell where it's been called from. Wait...can hooks have default values?
In a similar vein, would it be reasonable to have a hook_uc_order_product($op, $product), where $op can be 'load', 'save', and 'delete' (or would that just be overkill)?
#2
Another, probably better approach would be build from the suggestion at #1425978-27: Editing an order does not update tax and have an explicit new order status for orders at the admin screen. Any implementation of the hook gets the order passed in, so it can just check $order->order_status.
In fact, this might already be possible, in which case maybe all that's needed is slightly more detailed documentation for the hook.