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.We evaluate Order item conditions order item by order item. So if you create a "Product category: Hats" and "Product quantity: 2" condition, it will only match order items that have a hat product and a quantity over 2. If you have two Hat order items with quantity 1 each, that won't trigger the promotion.
We need to invent a way for the order_item_quantity condition to get all matched order items, to be able to add up and compare their quantities. Not sure what's cleanest yet. We want conditions to be independent of the parent entity, so we can't just pass the promotion. We could invent a fake condition entity_type, but that's kinda dirty. We could invent a way for conditions to request a second param to evaluate(). Remains to be seen.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | after.png | 56.11 KB | MegaChriz |
| #13 | before.png | 70.18 KB | MegaChriz |
| #10 | 2893367-10-quantity.patch | 9.19 KB | bojanz |
| |||
| #7 | 2893367-7-total-quantity.patch | 5.56 KB | bojanz |
Comments
Comment #2
bojanz CreditAttribution: bojanz at Centarro commentedExpanding with possible solutions.
Comment #3
bojanz CreditAttribution: bojanz at Centarro commentedShould be easy once we solve:
#2980700: Introduce offer conditions
#2980702: Allow conditions to optionally be aware of the parent entity
Comment #5
bojanz CreditAttribution: bojanz at Centarro commentedThe offer conditions issue needs order_item_quantity to become a commerce_order condition.
So I've pushed the first part of the fix. It makes the condition operate on an order, and updates the tests.
The second half will land with the two issues in #3.
This means that we're temporarily introducing a regression, but only for about 12h while I commit the other pieces.
Comment #6
bojanz CreditAttribution: bojanz at Centarro commentedBoth issues from #3 have landed, time to wrap this up.
Comment #7
bojanz CreditAttribution: bojanz at Centarro commentedMoved the condition from commerce_order to commerce_promotion, since it's now promotion specific.
Added the new logic with tests.
Still waiting on feedback for the UI (whether we need help text to indicate what's going on).
Comment #8
MegaChriz CreditAttribution: MegaChriz as a volunteer commented@bojanz
Thanks for all your work on discounts lately, I hope I can give some feedback on the UI in the next few days.
Comment #10
bojanz CreditAttribution: bojanz at Centarro commentedFixed test fails, renamed the condition to "Total discounted product quantity", sent it back to the "Products" group. It's not ideal, but we're out of time.
Comment #12
bojanz CreditAttribution: bojanz at Centarro commentedCommitted.
Comment #13
MegaChriz CreditAttribution: MegaChriz at WebCoo commentedUI feedback
Here is some feedback on the UI. I haven't looked at the promotions UI lately, so forgive me if the particular UI I'm commenting on does not belong to this issue.
Tested with custom conditions
I've had two custom order item conditions and after the update to the latest dev I noticed that these moved from "Product Restricted" to "Offer" when using the offer type "Fixed amount off each matching product" (I saw later that on the Commerce Slack channel that there was a change record, so these condition plugins probably need to be updated).
This is how it looked before:
This is how it looks now:
(both conditions are available as patches in #2971381: Add a product SKU condition and #2938731: Order total range condition)
Condition in "Offer" section are additive
It took me a while to notice that the conditions in the "Offer" section are additive instead of restrictive. With the conditions set in the image above, the discount applies to a product whose "SKU" is "e1556", but also to any other product with a quantity of 3. That was a thing that wasn't immediately obvious to me. I realize that part of that confusion is probably because the "Limit by quantity range" doesn't belong there and should instead appear in "Product Restrictive" where it was before.
Nonetheless I think it would be a good idea to emphasize that when using multiple conditions in the "Offer" section, things add up.