commerce icon indicating copy to clipboard operation
commerce copied to clipboard

Discount doesn't match items unless "All categories" toggled on

Open jamiepittock opened this issue 2 years ago • 3 comments

Description

This discount doesn't match any items - 'All categories' toggled off but no categories selected.

Screenshot 2022-03-01 at 14 44 53

This discount does match items - 'All categories' toggled on.

Screenshot 2022-03-01 at 14 45 00

Maybe this is expected behaviour but it tripped us up the other day, and someone on Discord ran into it too, so I thought I'd mention it and check.

Additional info

  • Craft CMS version: 3.7.33
  • Craft Commerce version: 3.4.11

jamiepittock avatar Mar 01 '22 14:03 jamiepittock

If you set All categories to false, then the product in the cart must match the selected categories. If you don't select any, then it cant match and the match fails.

If you want to apply the discount to any product regardless of the categories they are related to, then you need to turn on All Categories.

How do you think we could make it more clear in the labels?

lukeholder avatar Mar 01 '22 14:03 lukeholder

I also ran into this today. I am not sure if it is the functionality that is wrong or just the messaging/clarity. So the use case for me, was that we needed to offer a discount if a user purchased 2 or more of some specific product variants. So naturally, selected the variants this applied to and setup the rules. My expectation is was that if it matched those products the discount would be applied. However, I then realised we had to also turn on All Categories.

The way the screen is presented feels more like an option to either choose specific products or for a more broader match choose certain categories. It really doesn't feel like both are required. So perhaps it needs some more messaging and clarity around this. Even your note above @lukeholder gives it more clarity.

The other issue, and perhaps a bigger discussion above and beyond this specific thread is the use of categories full stop. Not all of the products on our sites sit inside categories. So how does this affect them? All categories I assume matches products that have no cats at all? Some users might not use categories at all and opt for structures/channels instead -as these have more control and additional benefits over categories. I understood that Craft was also moving away from categories entirely, and this had potentially been marked for version 4. Is this still the case and how will this affect this setup?

Finally, another useful option might be productType as an option to match against. So you can match all products in a specific type, quicker and easier than having to pick every single variant. As categories might not reflect the product types directly, so cannot be used this way.

terryupton avatar Mar 02 '22 13:03 terryupton

How do you think we could make it more clear in the labels?

Maybe “All” becomes “Any” and field instructions could help reinforce what’s going on?

Any purchasable Disable to select specific purchasables that should be available to match.

Any category Disable to select specific categories that should be available to match.

Could be too subtle, but it seems like switching off an “Any” control feels like “not just any ____, but these ...”. Implies that further selection limits the criteria, and that you should otherwise leave that setting enabled if you don’t want to limit further.

I’d almost say there could be a soft warning if you’ve flipped a switch off and not designated any items, but that could be excessive.

mattstein avatar Mar 03 '22 17:03 mattstein

Yeh this just tripped me up today and after I spent a little while debugging after a client emailed me asking about it. Came here to find others taking about it as well.

I think the root of the problem is Commerce is treating the lightswitches as an ALL condition instead of OR. Under the hood that's what's happening but it's not especially clear to the users (who aren't programmers).

By default, both light switches are on but let's say you want to limit to just a couple products. Naturally you turn off one light switch and choose your purchasable (which is already kind of counterintuitive).

However since you're limiting a purchasable, users think that they may need to limit categories as well to make sure their logic sticks, so they ALSO flip that light switch box on. But, since there's nothing selected, you end up with Commerce doing the follow logic:

LIMIT THIS DISCOUNT to ONLY (PRODUCTS SELECTED) AND (NO CATEGORIES).

... which matches nothing.

Most users are aren't thinking like: LIMIT THIS DISCOUNT to ONLY (PRODUCTS SELECTED) AND (ALL CATEGORIES SELECTED).

The light switches should really be reversed and the default set to OFF which means all purchasables would be included by default:

Screenshot 2023-03-28 at 1 18 29 PM

Commerce could then figure out how to do the right logic under the hood. Unchecking to limit something is counteruintutive. It's a bit like asking someone to "check this box to opt-out".

One other option would be to just get rid of the lightswitches all together. If a user puts in a limiter, Commerce could use that, otherwise assume every purchasable and/or category is included. We could also consider changing the "Matching items" tab near the top to "Limit items" might help.

Screenshot 2023-03-28 at 1 54 58 PM

Now with Commerce 4, maybe the Matching Items tab needs to go away and use the UI from conditional builder to create something else entirely?

Screenshot 2023-03-28 at 2 10 52 PM

RitterKnightCreative avatar Mar 28 '23 19:03 RitterKnightCreative

I have also been caught out by this before. I think a simple UI and wording change could resolve. But also like @RitterKnightCreative suggestion to "Limit to" instead of "Match".

samueldraper avatar Apr 05 '23 14:04 samueldraper

The issue here is that if you only have a selector for purchasables without the 'all purchasables' guard switch, if you had a single purchasable selected to apply the discount to, and then you deleted that purchasable in the system elsewhere the relationship to that purchasable will be dropped from the discount and then it would start applying the discount to all purchasables. This would not be good obviously.

We are still thinking about this and will see if we can improve the wording in the future.

lukeholder avatar Jul 11 '23 13:07 lukeholder

@lukeholder yeh - that's an interesting wrinkle. I wonder how other systems are handing it?

I also wonder if some defensive coding could be helpful?

For example, if you delete a purchasable that's the only purchasable tied to a discount, maybe remind the user that it has "dependencies" and not let them remove the purchasable? And/or proactively disable the discount since the whole expression basically falls apart anyway in that case?

I would assume a purchasable that still exists in the system but is disabled wouldn't be affected by this (only because it still exists)? From a user's perspective, I think most authors would think removing a one-off punchable that's no longer needed is effectively disabling it.

RitterKnightCreative avatar Jul 11 '23 15:07 RitterKnightCreative

This has been improved in 4.3 due out soon...

CleanShot 2023-08-01 at 18 48 07@2x CleanShot 2023-08-01 at 18 48 39@2x

lukeholder avatar Aug 01 '23 10:08 lukeholder