Hi,

This thread is the following of a discussions about module improvement and port to 6.x.

the uc_discounts module allow to show a discounted price on a product level, however, this feature is not made in a "clean" way and need some improvements I think :

  • First, the nodeapi function don't have $op switch. Don't know if it brings performances issue (maybe, without $op, the view one is the default one, but it would be better to add it for a better understanding, at least
  • Secondly, the discount calculation is made by passing a fake cart. It works but it's dirty, I think this feature would need a dedicated rewrited function, not a copy of the discount calculation on checkout, quickly adapted to make it work with products

These 2 things will be included in the 6.x package I will release soon.

Comments

zmove’s picture

zmove commented

Hi again,

I would bring more precisions about the discount calculation.

I only worked on uc_discounts_products and uc_discounts_users modules because I only need these 2 ones for my shop at the moment and I forgot to take care of the uc_discounts_order_total module. I understand better now why the product function seems to be a modified copy of the order function. I think uc_discounts was originally written to allow discounts on order total, and then modified to work on products.

But, even if understand better the origin and the logic of that 2 functions, I continue to think that it's a "dirty" way to implement it. Single products are not cart and we need dedicated functions to deal in a product level and dedicated functions to deal in order level.

For me, the best way to do that is to differentiate 2 callback apply functions, to not be obliged to pass a fake cart and fake variables to deal with discounts in a node level. we could have hook_apply_order for submodules that need to deal in an order level only and hook_apply_node for submodules that need to deal in a node level.

Providing these 2 hooks in submodules themselves would allow to pass right parameters to each callback, without the need to create fake variables, and it would allow for submodules creators to have a better control of what their submodules do. If somebody want to deal only in a order level, he could create only a hook_apply_order function that will even not be loaded on node_load.

zmove’s picture

zmove commented

I would like to add a last thing, a question :

Is is useful to let users to choose an option like "when the discount apply" ?

For me it is a useless option and it bring limitations, why ?

  • Because the type of discount you create define the way you want to display it. Example, if I create 10% discount on a product for a certain user, I would probably display the discount on the product level. It will be not logical to display the full product price in the cart, and display the discount only on checkout. Cause the user will see a display price and, when he add a product to the cart he will not see the discount apply...
  • But, if I create a discount based on order total, I would like to display it on the checkout page, in fact, I don't have the choice, I cannot display it before because the discount amount depend on informations on that page. Same if I want to create a shipping discount

This is a reason why I think the behavior to apply a discount should be defined in a submodule level, and not in the "master" level, because it's highly dependent on the "things" you want to discount. That's why the idea of 2 different callback...

joachim’s picture

joachim commented

> Is is useful to let users to choose an option like "when the discount apply" ?

Yes, I've been wondering this too. We should open another issue to discuss this.

Regarding the two fixes this issue is about: $op switch and fake cart -- can you do a patch or a snippet of your code so we can get this into 5?
Rather than your D6 version being a huge lump of code we have to assimilate, I'd like to make a smooth transition between 5 and 6: get as many things as we can into both the 5 and 6 branches.

zmove’s picture

zmove commented

Hi,

As this change is not related to menu, form or other, I think it will can adapt quite easily to 5.x.

I didn't finished this part of the code, but it's possible that submodules needs some changes too, I will post code for submodules I modified too.

For the option like "when the discount apply", are you really happy with it like that ?

For me, the order apply discount option is useful only for discount on order total. If you choose this option and apply a discount on a product, the display price of the product is the discounted product price, when you add it to the cart, you have the full price. It's a non-sense that can easily confuse your visitors.

But, if you choose to apply a discount when a product is added to the cart, when you create a discount on order total, you don't want to see product discounted into the cart, you only want your discount appear on the checkout page. That's why I think this option should be managed in a submodule level, because the behavior you want highly depend on the discount you create.