i believe the functionality of the "Require single product to qualify" option has been implemented incorrectly in the 2.x branch. i think this option has been confused by the "Required product" option underneath it (which i believe is new in the 2.x branch).

the description under the "Require single product to qualify" checkbox in the form reads: "Requires a single product to meet the qualifying requirements to receive discount. Otherwise qualifying requirements may be met by the combination of selected products." to me, this means that the "Require single product to qualify" should not care which product meets the criteria, just that it's a single product, rather than the combination of all products in the cart.

in other words, if i have a discount requiring a quantity of 4 products with this option checked, the discount should apply if i add 4 x product A to the cart or 4 x product B to the cart. the discount should not apply if add 2 x product A and 2 x product B.

instead, i believe the way 2.x works is that the "Require single product to qualify" option is paired with the "Required product" option underneath it: it enforces the qualifying type/amount criteria, but only against the one specific "required" product in the cart. (meaning that you'd need to create a separate discount for each relevant product to achieve functionality like i described above.)

the selection of specific applicable products seems to sit squarely in the "Discount Application" side of the discount (to use the fieldsets of the 2.x form for reference), rather than the "Conditions of Qualification" side. it'd be much more flexible that way... you can easily create the module's current behavior by checking the "Require single product to qualify" box (to ensure that *any* one product in the cart meets the type/amount criteria), and then filtering the discount by whichever single product you'd like it apply to.

i know this is a subject that has been discussed before and the interpretation gets messy, but from where i'm sitting it does seem to be a bug.

CommentFileSizeAuthor
#7 614962.patch8.35 KBezra-g
#4 614962.patch8.38 KBezra-g

Comments

ezra-g’s picture

ezra-g commented

Thanks for the clear explanation here and your issue reference-- It can be tricky to understand these details and your example is helpful.

if i have a discount requiring a quantity of 4 products with this option checked, the discount should apply if i add 4 x product A to the cart or 4 x product B to the cart. the discount should not apply if add 2 x product A and 2 x product B.

You're right about that kind of functionality. However, I believe the new "Required Product" selector *does* enable a new kind of functionality that requires selecting a product on the qualification, rather than the application side of the discount.

For example:
Qualification: If the user is purchasing yearly membership (Required product dropdown)
Application: Discount 10% off on the following products: (Products determined by Filter type and products selections).

I think confusion around these details led to the new feature wiping out the old one. I think the solution here is to add a [none] option to the Required Product *qualification* select box. If a product is selected, then it will be required for the discount to be active. Otherwise, the "requires single product to qualify" checkbox acts as you've described. This preserves both use cases without adding UI. Of course, we'll also add the relevant discount processing.

How does that sound?

I've asked Sborsody for comment since she wrote the original patch in the issue you referenced.

arh1’s picture

arh1 commented

thanks, Ezra!

at a glance, both your explanation of the new functionality and your proposed solution to the old, removed functionality sound good.

the '[none]' option in the "Required Product" select list sounds like it will restore the old, removed functionality by effectively saying that any one item in the cart (along with the other qualification criteria) can trigger the discount.

so, we might change the description under the "Require single product to qualify" checkbox in the form to read something like: "Requires a single product to meet the qualifying requirements to receive the discount. Otherwise qualifying requirements may be met by the combination of selected products. Select '[none]' to allow any given single product to meet the qualifying requirements."

Sborsody’s picture

Sborsody commented

I think I've been too far away from this to have anything meaningful to add. I trust ezra-g will address it well.

The bigger picture I see is that there's an almost infinite way to configure a discount and it is difficult to anticipate what kinds of discounts users want to configure. Because of that, I think having some sort of AND, OR conditional arguments (cf. Discount Framework) within the configuration options for a discount makes it flexible enough that it can remove the time spent on support later down the road. My rather lame attempt to implement this without drastic changes was by combining discounts but I recognize that it doesn't work as well as I intended.

ezra-g’s picture

ezra-g commented
Status: Active » Needs review
StatusFileSize
new 614962.patch8.38 KB

Ok, I believe this patch preserves all of the use cases in discussed in this issue, and I have tested each of them separately:

-- Discount conditions can be met by a combination of products
-- Only a Single type of product may qualify
-- Discount only applies with product X in the cart

I based the help text off of your suggestion, Andy, but then I realized that it would be more correct to say that we're requiring a single product SKU to meet the qualification requirements, rather than a single product, in order to describe the use case you described.

See the embedded screenshot for the new description text on both form elements.

Only local images are allowed.

arh1’s picture

arh1 commented

thanks, Ezra.

but at a glance this patch isn't doing what i expected... if i have a discount requiring a quantity of 4 products, and the "Require single product SKU to qualify" option checked, and "No specific product required" selected, the discount is still applying if i have 2 x product A and 2 x product B in my cart.

reading back through the posts above, i may have miscommunicated what i expected (and the way i believe the 1.x version works)... the config combination i'm looking for is a way to say that one product in the cart must meet the qualification criteria, but it can be any one product in the cart.

so, with the qualification conditions i just mentioned above, neither of these combinations of products in the cart will trigger the discount:

* 2 x product A + 2 x product B
* 1 x product A + 1 x product B + 1 x product C + 1 x product D

but both of these combinations of products in the cart will trigger the discount:

* 4 x product A + whatever else...
* 4 x product B + whatever else...

the ability to say that the one product which must meet the qualification criteria must be product A is a more specific case, such that only this combination of products in the cart will trigger the discount:

* 4 (or more) x product A + whatever else...

am i missing something with the patch, or did we miscommunicate above?

thanks again for your work on this.

ezra-g’s picture

ezra-g commented

Just a heads up that I should have something in the queue for this later today.

ezra-g’s picture

ezra-g commented
StatusFileSize
new 614962.patch8.35 KB

Here's a revised patch. This is more like un-revised, as my last minute revisions broke the originally submitted patch.

arh1’s picture

arh1 commented

at first glance, the patch from #7 looks great. thanks, Ezra!

i'm headed out for the holiday, but will test more thoroughly when i'm back next week.

ezra-g’s picture

ezra-g commented
Category: bug » feature
Status: Needs review » Fixed

Thanks for the review, Andy.

I'm fairly confident that this is the right fix so I'm committing this in order to move forward with a few bug fixes. We can always continue to tweak if that turns out to be necessary. Thanks!

Status: Fixed » Closed (fixed)

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

vincent rommelaars’s picture

vincent rommelaars commented
Category: feature » support
Priority: Normal » Major

Hi,

I'm quite new to Drupal and patching modules, so I'm sorry about that.
I've downloaded the 6.x-2.x-dev version of this module and I was loking for a solution described in this issue.
I did download the patch, which normally I would apply manually.
In this patch there is some code to alter that comes from another patch discussed in the issue at http://drupal.org/node/448624.
So I tried to apply this patch first, but this one isn't applied on the 6.x-2.x-dev version as far as I can discover.
Can someone point me in the right direction or provide a complete patch or maybe an updated dev version with this feature.

I hope anyone can help me with this.

Thanks in advance...!