I'm going a little crazy trying to track this down, so I guess it's at the point where I need professional insight. ; )
Basically, I have an API function that deletes a product line item from a shopping cart order like so:
/**
* Deletes a product line item from a shopping cart order.
*
* @param $order
* The shopping cart order to delete from.
* @param $line_item_id
* The ID of the product line item to delete from the order.
*/
function commerce_cart_order_product_line_item_delete($order, $line_item_id) {
$order_wrapper = entity_metadata_wrapper('commerce_order', $order);
// Remove the line item from the line item reference field.
foreach ($order_wrapper->line_items as $delta => $line_item_wrapper) {
if ($line_item_wrapper->type->value() == 'product' &&
$line_item_wrapper->line_item_id->value() == $line_item_id) {
// Store the line item and referenced product in variables so we can pass
// them to Rules even after removing the line item from the order.
$product = $line_item_wrapper->product->value();
$line_item = $line_item_wrapper->value();
// Remove the line item from the order.
$order_wrapper->line_items->offsetUnset($delta);
// Invoke the product removal event with the line item about to be deleted.
rules_invoke_all('commerce_cart_product_remove', $order_wrapper->value(), $product, $line_item->quantity, $line_item);
// Delete the actual line item.
commerce_line_item_delete($line_item->line_item_id);
return $order_wrapper->value();
}
}
}
When passed a line item ID, it wraps the order and iterates over the list looking for a matching product line item with that ID. I then stash some variables for use in the rule invocation and unset the offset at which the line item was found. This appears to work great, resulting in the line item being deleted, the order's line item reference field being updated, and the event being invoked.
What gets me into trouble is the additional helper function that intends to delete every product line item from an order. I tried at first to use the same function and just allow the $line_item_id to be set to NULL to remove every product line item. Honestly, that wasn't ideal just for the API confusion it created, but also it wouldn't work.
I changed my approach and added the following API function to empty a cart:
/**
* Deletes every product line item from a shopping cart order.
*
* @param $order
* The shopping cart order to empty.
*
* @return
* The order with the product line items all removed.
*/
function commerce_cart_order_empty($order) {
$order_wrapper = entity_metadata_wrapper('commerce_order', $order);
// Build an array of product line item IDs.
$line_item_ids = array();
foreach ($order_wrapper->line_items as $delta => $line_item_wrapper) {
if ($line_item_wrapper->type->value() == 'product') {
$line_item_ids[] = $line_item_wrapper->line_item_id->value();
}
}
// Delete each line item one by one from the order. This is done this way
// instead of unsetting each as we find it to ensure that changing delta
// values don't prevent an item from being removed from the order.
foreach ($line_item_ids as $line_item_id) {
$order = commerce_cart_order_product_line_item_delete($order, $line_item_id);
}
return $order;
}
What you see here is as far as I could get. I first iterate over the line items array and stash the line item IDs in a local array. I then iterate over these IDs and remove them one by one. Each time I receive back the updated order object and can pass it back to the line item deletion function on the next iteration. This deletes the first line item no problem, but I run into problems on the second item. Basically, it fails with this error message:
EntityMetadataWrapperException: Unable to get the data property type as the parent data structure is not set. in EntityStructureWrapper->getPropertyValue() (line 396 of /Users/rszrama/src/cvs/d7/entity/1/includes/entity.wrapper.inc).
By dumping my data every step of the way, I can see that the problem comes when commerce_cart_order_product_line_item_delete() tries to iterate over $order_wrapper->line_items the second time through. If I dump $order_wrapper->line_items->value(), it shows a line items array with delta values that have been adjusted since the last removal (i.e. even though I removed delta 0 the first time through, the array has been re-indexed the second time to begin at 0). The problem is that in the database, the delta value is still 1. I'm not sure if that conflict is what's causing my problem or not...
That's about the only thing I can drill this down to, though... it appears as though offsetUnset reindexes items to the wrapper but doesn't reindex items in the field data table. Or perhaps the list wrapper on load ignores the delta values in the database. The problem of course is that the dumped value is showing the line item just fine that should be getting wrapped in the iteration.
Baa. I have no clue where to look next. : P
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | entity.list_.patch | 1.36 KB | fago |
Comments
Comment #1
rszrama commentedShot in the dark, but I wonder if this could be a problem caused by the field cache?
Comment #2
fagohm, yes the list wrapper completely ignores the delta values. A "list" is supposed to be a numerically indexed bunch of items, with the index having no meaning thus always starting with 0, i.e. you can rely on $list[0] to get the first item and so on.
The field API should be fine with that, as it won't care much about deltas either. But when you remove a value, an index is missing afterwards and I guess this is what triggers a bug.. It looks like the iterator gets a fresh-numerically indexed array via $this->value().
If that's the problem, does the attached patch help you?
Comment #3
rszrama commentedI don't quite understand the fix, but it does appear to be working. Can you also confirm for me if I should be saving entities after performing the offsetUnset? I couldn't tell if it was saving itself or not.
Comment #4
fagoGreat it's working :) You should save it yourself as it never saves automatically. Only Rules does so.
-> Committed + 1 nasty bug less!
Comment #5
fagoComment #6
rszrama commentedGreat, thanks for the extra info. : )
I ended up pulling a page out of your own book with a $skip_save parameter. ; )