Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
If you add some comparison_properties via hook_commerce_cart_product_comparison_properties_alter() which are available on one line item type but not on another you will get an "Undefined property"-error from the commerce_cart_product_add() function.
There should be either checked whether a property exists or not before accessing it. Or you pass not only current $comparison_properties but also the line-item type to the hook.
I prefer the second solution as it would give devs the possibility to declare different comparison properties for different line-item types.
Comment | File | Size | Author |
---|---|---|---|
#4 | 1925646.cart_line_item_comparison.patch | 2.63 KB | rszrama |
Comments
Comment #1
rszrama CreditAttribution: rszrama commentedHmm, honestly, it shouldn't be trying to combine line items of different types. That sounds like the real issue to me.
Comment #2
haggins CreditAttribution: haggins commentedThat's not the point. It does not try to combine different types. But if combination is enabled at different types it will check all properties you added in hook_commerce_cart_product_comparison_properties_alter() independent from whether the property is available at the current line-item type or not.
Example:
1st type: product
no additional fields (=default installation)
2nd type: shirt
additional field/property: field_size
As I would like to combine shirts of the same size I implement above hook as follows:
If you now add shirts everything is fine. But if you add products then commerce_add_to_cart() fails as it trys to compare the field_size property of products which is not available for this line-item type.
Hope it's explained clearer now :)
edit: this does not happen if you add product to an empty order as then there is nothing to compare. There must already be at least one line-item in the order/cart - regardless of its type.
Comment #3
rszrama CreditAttribution: rszrama commentedOk, I improved our combining logic like so:
I'd also add that you don't really need that array_merge(). You could just do
$comparison_properties[] = 'field_size'
.Comment #4
rszrama CreditAttribution: rszrama commentedUgh, patch did not attach. :-/
Comment #5
haggins CreditAttribution: haggins commentedThank you Ryan!
I wasn't able to apply the patch yet. But with your description and by a (quick) code review it looks as this solves the issue perfectly :)
Comment #6
rszrama CreditAttribution: rszrama commentedTest bot liked it, too. Committed.
Comment #7.0
(not verified) CreditAttribution: commentedUpdated issue summary.