This issues was copied from discounts_not_applying_conditions

Original post (by Lyle):

On my test site, I had set up a quantity discount for a certain product. It worked for those products, but it also gets applied for carts without that product in it. In addition, there weren't enough products in the cart to trigger the rule, and the discount is inactive to begin with. About the only condition that should allow the discount to be applied is the fact that the end date is 10 years in the future. That shouldn't be allowed to override all of the other limiting conditions.

Comment (by zmove):

Yes, I found a some bugs into discount module too...

For example, I created a discount on a certain product that is applied if you purchase at least 5 products.

I have 2 of this product in my cart, and the discount is applied.

Update:

I have tested this and can't even get quantity discounts to work at all. I am successfully using taxonomy discounts that "discount product type price from order" on a live site so I know the module at least partially works.

We need to run a full audit of the module to ascertain which aspects are not working. This should happen within the next couple of weeks.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Bartezz’s picture

Seems like I bumped the wrong post... (http://drupal.org/node/324296)

Any news on the conditions? I have looked at the code but that is beyond me :(
Hope this bug will get squashed soon!

Cheers

DianeOfTheMoon’s picture

Are people still having this issue?

I'm getting unexpected behavior where if I set the discounts to be applied when the product is placed in the cart (for a buy 1 get 1 free), I don't see any discount...but setting it to apply on checkout shows me the expected result.

I'm not seeing an issue with creating a discount of qty 5 product, apply 100% discount on 2 items. Could someone give me some more details about how to duplicate it, and I'll try to take a look?

Bartezz’s picture

Yeah am still having issues with the conditions. I don't think it's been addressed yet... and I can't find what's causing it to fail myself... :(

DianeOfTheMoon’s picture

Well, can you give me the setup you have for uc_discounts? I'll try to recreate it. I'll need both the discount and all the params, plus the module level parameters.

Bartezz’s picture

Hi Diane,

I have the following modules installed;

Ubercart - core (optional)
- Discounts 5.x-1.x-dev

Ubercart - discounts
- Order total 5.x-1.x-dev
- Product 5.x-1.x-dev

No I have a 'product' discount for a certain product set up as followed;

Name: T-shirt 5-7
Description: T-shirt  5-10 pieces 6% bulk discount
Max Discounts: 0
End Processing: no
Exclusive: no
Weight: 0
Start Time: Thu, 02/10/2008 - 00:00
End Time: Wed, 02/10/2030 - 23:59
Active: yes

Conditions
Group	Weight	Property	Op	Item	Value	Actions
1	-10	Product	>=	T-shirt 	5	edit	delete
1	10	Product	<	T-shirt 	10	edit	delete
Add Condition

Actions
Property	Weight	Item	Qty	Amount	Actions
Discount product price from order	0	T-shirt		6%	edit	delete
Add Action

This doesn't work, at all!
Yet if i remove the second condition (1;10;Product;<;T-shirt;10;edit;delete) the module does apply discount when ordering 5 or more products.

So my conclusion, adding more than 1 condition breaks the discount.
I've tried with different variables on different products but to no avail.

Hope you can help!

Thanx....

adam_b’s picture

I'm having huge problems with this also. I thought for a while that the problem was related to multiple conditions, but just trying to add a simple percentage discount for people buying 10 or more copies of an product isn't working correctly:

Name: 10 users = 10% off
Description: 10% discount for each 10 users
Max Discounts: 0
End Processing: yes
Exclusive: yes
Weight: 0
Start Time: Mon, 01/12/2008 - 00:00
End Time: Wed, 31/12/2008 - 23:59
Active: yes

Conditions
Group	Weight	Property	Op	Item	Value	Actions
1	0	Order Total	>=		10	edit	delete
Add Condition

Actions
Property	Weight	Item	Qty	Amount	Actions
Discount amount from order total	0	0		10%	edit	delete
Add Action

The discount is applied even with fewer than 10 items:

Checkout
Cart contents
Qty	Products	Price
7x	Welfare Benefits and Tax Credits - online
	£252.00
Subtotal: £252.00

Discounts
The following discounts will be applied to your order:
Discount	Amount
10% discount for each 10 users	£25.20
	Subtotal: £25.20

Any info/updates gratefully received.

Bartezz’s picture

Well in my case it's not the precentage but really the fact that I have an amount bigger than AND and amount smaller than conditions together for one action like described above #5....

I'm starting to give up hope on this as I haven't seen any response by the maintainers on this matter, neither here or on ubercart.org...

DianeOfTheMoon’s picture

Bartezz,

It looks like the maintainers have removed the >, >=, <, and <= conditions from the CVS code...don't know if it was intentional or not, but even if I go for a fix it's not future compatible at the moment.

And, since the maintainers aren't around, I'm not sure of the best way to go about this. I don't want to step on toes, but we actually need this module working for our company, so I'll have to start fixing things soon.

Does anyone have an email address for Zmove or the other maintainers?

EDIT:

Actually, it's more like the compare operator is only for the product type. There's no option here to specify a "less than" quantity, only the quantity to trigger the discount and an overall "use x number of times".

I guess in that situation (for the 5-10 shirts) you would set the product = T-shirt, quantity 5 and only have the discount apply once. Does this sound correct?

Bartezz’s picture

Hi Diane,

Not sure if I understand what you are saying? The comparison oprator is used for product type?

Then why does it apply a discount for > 5 items (if I don't also set a second operator < 10)? I think it does work for quantity, but just very buggy.

I could set 5 discounts, ==5, ==6, == 7, == 8, ==9, ==10 items and then apply the discount. But with a whole catalog of products to which these discount conditions should apply that's heaps of work, well 5 times more than doing it with a single dicount with 2 working comparisson operators :)

I hope to delve into this next week but time is scarce and am also working on improvements to the uc_ogone module.

Will post back any feedback, conclusions, code etc!

Cheers

DianeOfTheMoon’s picture

Basically, the way CVS is working is that you can specify a condition of ::product:: = t-shirt with a ::value:: of 5.

From there, the uc_discounts_product.module was checking to see if the quantity was >= ::value:: and if so, setting the discount. It also appeared (this is in CVS, mind you) that the uc_discounts framework was doing the same check but checking quantity == ::value::, causing anything but the exact quantity to fail on the discount.

I've just finished rewriting the module so that:
a) uc_discounts doesn't double check to see if the discount should be applied, only taking what the submodule provides.
b) the uc_discounts_product module will now take the max discounts from the discount and will try and figure out the total number of discount iterations to provide. Previously, it would only apply a discount once for products, making cases of buy one get one free only provide one free item if four were bought.

I still have a bit more to do on it, but this should bring the CVS modules up to where I can use them. With that said, can you explain what exactly you are looking for on the discount application? For what I can see, using the CVS (which I would assume would become a new 5.1 version) trunk for 5.1, you would create a discount, give it a max discounts of 2 and specifiy that they need to buy 5 tshirts That would mean that if they bought between 5 and 10 tshirts, the discount would be applied, but not over that amount.

From there, I need to know how you would expect the discount to work. Are you looking for a 6% reduction in the cost of all the tshirts (up to 10) or just for the ones between 5 and 10?

And, I'm sorry that I'm working off of CVS and not the release from a while ago...but it seems like most of the 5.1 modules were rewritten since the last release.

adam_b’s picture

What I'd expect is to be able to give, for example a 10% reduction if people buy 5 or more T-shirts, and a 20% reduction if they buy 10 or more T-shirts. Therefore:

  • quantity = 1-4 = no discount
  • quantity = 5-9 = 10% discount on all T-shirts
  • quantity = 10 or more = 20% discount on all T-shirts

Hope this helps?

Bartezz’s picture

This is exactly how I'd expected it to work as well and the way most bulk discounts are applied.
The option to only apply that 10% discount for the 5-9 T-shirts and full price for the first 4 would be nice but I think it would be a rare form of bulk discounting, at least in most retail cases.

What I've also found, is that I needed a submodule (posted on the ubercart forum http://www.ubercart.org/contrib/143#comment-24926) to create bulk discounts based on taxonomy and quantity. The problem was, that as soon as a product was customized, let's say a custom print, it became an unique product and thus not adding up to reach that 5-9 quantity eventhough it basically still was that t-shirt.

So what I've done is install that taxonomy discount module, assigned a taxonomy term to the t-shirt and then created a discount based on the quantity of products of a certain taxonomy term. Get's complicated I know but I need it to work this way.

Di, thank you for working on this module. I really do hope you're getting it to work properly. Cause honestly, if I'd knew that setting up discounts was so hard on ubercart I'd have looked for an alternative. What I find so strange is that there are hundreds of ubercart webshops around... do none of these guys ever offer other than very simple discounts? Maybe if you get this module up and running we start seeing discounts all over ubercart webshops and we can go on a shopping franzy :)

Cheers,
Bartezz

DianeOfTheMoon’s picture

Status: Active » Needs review
FileSize
8.92 KB

Bartezz,

Would you happen to have a development instance you could test with? Here's a patch that should correct the issue that I had with the discounts and should allow you set the discounts what I would call an "expected" manner.

Now, this is only the second patch that I've done and I haven't done much more than local testing, so please don't try this on production and bear with me if I've messed something up.

You'll need to grab the 5.1 branch from CVS and apply the patch, then try creating two discounts. Discount one should be with your t-shirts and a value of 5 set as the condition, where the action is 10% off and no quantity set. Discount two should be with your t-shirts and a value of 10, where the action is 10% and no quantity.

This should cause the processing to find the 5 item discount and apply the 10% value to all the tshirts in your cart, then, if you have 10 or more, it'll match on the other and give an additional 10% off on all the tshirts (for a total of 20% on all items).

To me, this would be the expected behavior for the actions. If you want to limit things, you can either set the Max Discounts for the discount itself (which limits the number of iterations) or if you have a quantity set in the action, it'll apply the discount to that number of items * the matching groups of items.

So, if you wanted to do a buy 4, get 1 free, you would do quantity 5 condition, quantity 1 100% action. To limit it to say 3 per customer, just set the Max Discounts to 3 and it'll stop there. Otherwise, when you add five in the cart, the fifth is free, 10 in the cart gets you two free, etc.

Does the logic for the fix and flow sound good, and can you let me know if you see any issues?

Bartezz’s picture

Hi Di,

Thanx for this!!!! Big time!

I will try your patch next week on a testing site and do some testing!
When you say; Discount one should be with your t-shirts and a value of 5 set as the condition, does that mean a discount set up with an >= operator like such;

// Condition
Group	Weight	Property	Op	Item	Value	Actions
1	-10	Product	>=	T-shirt	5	edit	delete

//Action
Property	Weight	Item	Qty	Amount	Actions
Discount product price from order	0	T-shirt		10%	edit	delete

I was actaully thinking of applying discounts the way you said

discount 1: > 5 = 10%
PLUS
discount 2: > 10 += 10%
total discount being 20%.

But I still find it weird that one can not limit the discount by setting a second condition < 10 in that same discount.

// Condition
Group	Weight	Property	Op	Item	Value	Actions
1	-10	Product	>=	T-shirt	5	edit	delete
1	-9	Product	<=	T-shirt	10	edit	delete

//Action
Property	Weight	Item	Qty	Amount	Actions
Discount product price from order	0	T-shirt		10%	edit	delete

Let me explain why. Sometimes shop owners get their products pre packet in bundels. Let's say we're talking coffee cups here. When a shop orders them from the factory they can only buy them packed by 6 in a box. So when the shops sells them in their online shop the might only want to give a discount if you buy 1 - 6 cups. That would make their life easier, if you order 5, they only have to remove one cup from the factory packaging. But if you order 7 they need to package a single cup for your order -> more work -> no discount. That's why it would be nice to have the second condition working as well. I bet there are better examples then this one but I think it explains well enough :) Do you think this would be workable with your patch?

Not that I currently need this for my shop but I bet with all them ubercarters out there that some shopowners would like this feature.

Anyway, in stead of asking for more features I should help you and test your patch first ey :) My imagination was running wild again!

Have a great weekend Di!
Cheers,
Bartezz

DianeOfTheMoon’s picture

Bartezz,

There might be a lot of stuff I might want to do with this in regards to the company I work for, but I also need to respect the maintainers and the direction that they have moved with the module. Now, if I end up taking this over (which it's starting to look like I might) then I may start trying to do something more drastic.

When you try out the CVS code, you'll see that there's really no >, <, <=, or >= for the item quantity...it just assumes >=. I think that's where some of our confusion talking has come from. So, you have to look at all conditions (currently) as being >=.

But, I think you have the idea on how to layer the discounts. :)

I think the second item is more of an edge case, but you should be able to create a discount with a condition of one item and an action on one item (so, 1-1), but use the Max Discounts in the discount set to 6 (max of six discount iterations applied).

Bartezz’s picture

Hi Di,

With all due respect I think the maintainers have abandoned this module like a bad date. There is no replies to forum posts, emails, no updates in response to forum posts (as far as I can see), nothing at all! So your 'take over' is more like a loving adoption in my eyes!

You are saying; "you'll see that there's really no >, <, <=, or >= for the item quantity"
Was there never? Not even in one of the first versions? (http://www.ubercart.org/contrib/143)

Cause to me it's very strange that the discount module allows you to set up multiple conditions for a discount with operators like > < >= etc but not handling more than one of those conditions in the backend let alone completely disregarding the operators. Why would any sane programmer go thru all the trouble to give users an interface in which they can set operators only to disregard them and hardcode a >= as operator? Sounds like dressing up for some big gala when you're staying at home anyway :)

Anyhow, I'll be testing your patch monday or tuesday and will let you know how that turns out!

Thanx a million again!

Cheers,
Bartezz

psynaptic’s picture

This module was created by someone else, posted in the contrib section on ubercart.org and deemed important and good enough to host on drupal.org. There are some major problems with this module. Some parts just don't work at all.

The module has 4 maintainers but development has stalled somewhat due to other commitments. I personally have been working on my biggest project to date recently so have been very quite on drupal.org.

If anyone shows they have the right skills to become another co-maintainer of this module then I would be more than happy to grant access to make it easier to contribute and actually get things committed. A successful candidate would show his/her ability to follow Drupal's coding standards, keep commits separate (e.g. don't lump new feature with code style improvements) and post issues about major changes before committing to the main branches.

If anyone thinks they can contribute effectively, feel free to get in touch using my contact form.

Bartezz’s picture

Hi Di,

Sorry for not coming back on this for ages! Have been busy finishing other projects before the end of 2008. Best wishes btw ;)

Well I've downloaded 5.x-1.x dev, applied your patch and created two discounts. Works nicely, no bugs!!
(well had to trun on taxes module because 5.x.1.x-dev was nagging, javascript getTax() undefined: http://drupal.org/node/336210)
So thanx a million for that! You should apply for that vacany below ;)

Yet I'm still not all to happy with how things are going so I will try and fix some things to get the operators working...

Will keep ya posted.

Cheers

trimpton’s picture

I am interested in using this module but I dont even have the >= or > < in my uc_discounts module 5x that I obtained here. Where can I get the version the patch in #13 is for? Thanks

Bartezz’s picture

The patch is for the latest version which you probably already downloaded; http://ftp.drupal.org/files/projects/uc_discounts-5.x-1.x-dev.tar.gz

Download and patch, then create a discount, the second screen allows you to set conditions and actions and that's where the useless >= < etc should be visible!

Cheers

trimpton’s picture

Hmm That is what I did and still only have "=" but if this doesn't work at all I guess Ill try to have to work out another solution.

DianeOfTheMoon’s picture

Trimpton,

I was using the version culled directly from CVS to make my changes with, and I only have '=' as an operator, too.

However, it'll actually act like '>=' so you should be able to approach your discounts like that. Depending on what happens with our store I may try and tweak other things (we are launching tomorrow) as I get time.

If you need to do a 'less than' discount, just set your amount to 1 and have the discount limit to your upper amount. So, if you want to do less than 5 items, get a discount, have the amount = 1, with 4 groups max.

That'll keep it all in line.

DianeOfTheMoon’s picture

Great, I'm glad it's working for you, too!

Would somebody mind doing a quick review of the code to make sure everything is in order?

Bartezz’s picture

Hi Di,

Do ya think you'll ever come to fixing the operator part?
It would be great if these would work as expected so that multiple conditions can be set (eg. quantity > 3 && quantity < 5 = action: discount 10%).

I also attached a helper module called taxonomy discounts. (http://www.ubercart.org/contrib/143#comment-24926)
I've changed some stuff in the code to make it actually work and play nice with other helper modules.

As mentioned here http://drupal.org/node/323840#comment-1151060 I had trouble bulk/quantity discounting when a product had attributes which allowed a customer to specify the product. Once specified/customized ubercart saw the product as unique and thus not adding up to reach that bulk/quantity discount. My workaround is to use the helper module attached and create a term for each product then create a bulk/quantity discount for that term (which is basically just one product).

Maybe you could have a glance at it and see if there are possible security holes in it?!

Cheers