Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][MIG] coupon_criteria_multi_product: Migration to version 16.0 #160

Conversation

ernesto-garcia-tecnativa
Copy link
Contributor

@ernesto-garcia-tecnativa ernesto-garcia-tecnativa commented Oct 4, 2023

@Tecnativa TT44317

@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-coupon_criteria_multi_product branch 3 times, most recently from 9bd8382 to 6645cee Compare October 4, 2023 21:32
@ernesto-garcia-tecnativa
Copy link
Contributor Author

@chienandalu @pilarvargas-tecnativa please review

@pilarvargas-tecnativa
Copy link
Contributor

pilarvargas-tecnativa commented Oct 5, 2023

In v16 the condition of having X products in the order lines to apply the promotion is incorporated. You can require X quantity of a specific product or X quantity in total of several products. What is not contemplated in v16 is that if we add several rules, for example:

  • 10% discount if purchased:
    · 5 units of any type of table (rule 1).
    · 1 unit of wastepaper basket (rule 2)
    As soon as one of the rules is fulfilled the discount will be applied, I think that so far there is nothing that forces both rules to be fulfilled and this is the functionality that would not be covered by odoo and why I don't know if it would be necessary to keep the module adding it.

@chienandalu what do you think?

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chienandalu what do you think?

Yes, we discussed that internally. And if you take a look in the migration commit that's where we're heading to.

@ernesto-garcia-tecnativa

  • We'll need to make a migration script to turn the old criteria lines in the multiple rules needed.
  • In the PR for sale_loyalty_criteria_multi_product include this one in a temporary commit so we can test the whole thing already

@pilarvargas-tecnativa
Copy link
Contributor

Yes, we discussed that internally. And if you take a look in the migration commit that's where we're heading to.

So that is what the "validate_all_rules" option is intended to do. Thanks for the clarification

@chienandalu
Copy link
Member

Checking another migration from @pilarvargas-tecnativa I devised that for the sale_loyalty_criteria_multi_product, this will be the method to hook onto: https://github.com/odoo/odoo/blob/a277faa2ffab7559fcbad95fcc1e8fd6a26d756b/addons/sale_loyalty/models/sale_order.py#L789-L794

It won't be easy though as the rule check is quite encapsulated inside that method...

@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-coupon_criteria_multi_product branch 4 times, most recently from e9c7748 to c9fe76b Compare October 12, 2023 01:30
@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-coupon_criteria_multi_product branch 11 times, most recently from db340be to e927c27 Compare October 13, 2023 22:25
@ernesto-garcia-tecnativa
Copy link
Contributor Author

@pilarvargas-tecnativa @chienandalu please review

@pilarvargas-tecnativa
Copy link
Contributor

When the "Repeat" box is checked to apply the promotion to the sales order there must be the minimum quantity of any product specified in the rule, but that is the current behaviour of odoo. I think what should be checked is that if the box is not checked, the promotion is only applied if all the products specified in the rule are in the order lines and meeting the quantity.

@ernesto-garcia-tecnativa
Copy link
Contributor Author

When the "Repeat" box is checked to apply the promotion to the sales order there must be the minimum quantity of any product specified in the rule, but that is the current behaviour of odoo. I think what should be checked is that if the box is not checked, the promotion is only applied if all the products specified in the rule are in the order lines and meeting the quantity.

Ummm I hadn't thought about this, it's fine to me, what do you think of @chienandalu?

@chienandalu
Copy link
Member

Ummm I hadn't thought about this, it's fine to me, what do you think of @chienandalu?

As we decided in our meet last week we were going back to the original architecture as we couldn't hook into Odoo's methods to use the multi-rules in our favor so that's the way to go

@pilarvargas-tecnativa
Copy link
Contributor

As we decided in our meet last week we were going back to the original architecture as we couldn't hook into Odoo's methods to use the multi-rules in our favor so that's the way to go

But you can check rule by rule if it's true, right now the behaviour of odoo is that if the promotion is applicable by buying 3 units of products a, b or c, the moment you add 3 units of any of them the promotion is applied. By checking the "Repeat" option we get the same behaviour, so we are not contemplating the functionality that those 3 products have to be present at the same time in the order lines. All this talking about a single rule, that when there are several if one passes, the promotion is applied.

@pilarvargas-tecnativa
Copy link
Contributor

This is an odoo test, a discount applied when buying 5 units of A or B, but it doesn't take into account A and B at the same time, I think that can be set at rule level and not multi-rule. Or maybe I'm getting confused

Captura desde 2023-10-16 10-54-15

buyproduct

@chienandalu
Copy link
Member

But you can check rule by rule if it's true, right now the behaviour of odoo is that if the promotion is applicable by buying 3 units of products a, b or c, the moment you add 3 units of any of them the promotion is applied. By checking the "Repeat" option we get the same behaviour, so we are not contemplating the functionality that those 3 products have to be present at the same time in the order lines. All this talking about a single rule, that when there are several if one passes, the promotion is applied.

From outside of the method that checks the validity of the promos you only can tell if at least one rule passed the checks but you can't tell if one of them is invalid. Here's the relevant part of the code: https://github.com/odoo/odoo/blob/16.0/addons/sale_loyalty/models/sale_order.py#L844-L845

You can see that there are some flags that need to be activated in order to let the promo go, and if they don't the promo fails. But at soon as one is valid, the promo passes.

You can't hook inside that logic to check them all (or others that might come on top). We would need a hook method at that level to do so

@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-coupon_criteria_multi_product branch 3 times, most recently from fa941d5 to 51e636b Compare October 16, 2023 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants