Currently, commerce_entity_reference_delete() isn't rekeying $items properly. So, for example, if we have 6 items keyed 0 through 5 and we try to remove the first one, we end up with an array with keys 1 through 5.

The specific problem that I'm observing because of this is that when I delete the line item on an order and then view the order, the commerce_line_items field appears to be empty. Something somewhere must be checking whether $items[0] is empty, but I haven't quite tracked that down yet.

CommentFileSizeAuthor
#3 1323074-commerce-entity-reference-delete-rekey.patch1.45 KBdamien tournoud
#2 reset-keys-on-reference-deletion-1323074-2.patch514 bytesserialjaywalker
#1 reset-keys-on-reference-deletion-1323074-1.patch2 KBserialjaywalker

Comments

serialjaywalker’s picture

serialjaywalker commented
StatusFileSize
new reset-keys-on-reference-deletion-1323074-1.patch2 KB

The attached patch adds a call to array_values().

Updated: Oops, this patch did more than I thought. I'll post a new one shortly.

serialjaywalker’s picture

serialjaywalker commented
StatusFileSize
new reset-keys-on-reference-deletion-1323074-2.patch514 bytes

Try this patch.

damien tournoud’s picture

damien tournoud commented
Status: Active » Fixed
StatusFileSize
new 1323074-commerce-entity-reference-delete-rekey.patch1.45 KB

Thanks for the patch.

I merged the attached variation of it into 7.x-1.x.

rszrama’s picture

rszrama commented

Thanks for the fix. After thinking about it, I'm pretty sure the fact that I wasn't rekeying was just a relic of this function's prior dependence on entity_metadata_wrapper(). When I got rid of that, we should've added the code to rekey (and obviously we should've had a solution before - can't remember what it was about the wrapper that kept me from doing so).

And kudos to Damien on the assist. : )

rszrama’s picture

rszrama commented
Status: Fixed » Reviewed & tested by the community

Oh, actually, Damien, I don't see this when I pull. Perhaps you forgot to push? I won't recommit this unless I hear from you that you actually didn't get it in. Setting to RTBC so we don't forget to get it in.

Also, for posterity's sake - rekeying is exactly what happens when you remove an item from a multi-value field. Just confirmed locally w/ a little text field.

rszrama’s picture

rszrama commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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