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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] sale_coupon_limit > sale_loyalty_limit: Migration to version 16.0 #156

Merged
merged 13 commits into from
Nov 20, 2023
Merged

[16.0][MIG] sale_coupon_limit > sale_loyalty_limit: Migration to version 16.0 #156

merged 13 commits into from
Nov 20, 2023

Conversation

pilarvargas-tecnativa
Copy link
Contributor

@pilarvargas-tecnativa pilarvargas-tecnativa commented Oct 2, 2023 •
edited
Loading

@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-sale_coupon_limit branch 5 times, most recently from eacc42e to 3082f88 Compare October 6, 2023 14:38
@pilarvargas-tecnativa pilarvargas-tecnativa changed the title [16.0][MIG] sale_coupon_limit: Migration to version 16.0 [16.0][MIG] sale_coupon_limit > sale_loyalty_limit: Migration to version 16.0 Oct 6, 2023
@pilarvargas-tecnativa pilarvargas-tecnativa marked this pull request as ready for review October 6, 2023 14:41
@pilarvargas-tecnativa pilarvargas-tecnativa mentioned this pull request Oct 6, 2023
Merged
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-sale_coupon_limit branch 2 times, most recently from fb0a886 to 4871fb4 Compare October 6, 2023 15:58
chienandalu
chienandalu suggested changes Oct 9, 2023
View reviewed changes
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/coupon_rule.py Outdated Show resolved Hide resolved
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-sale_coupon_limit branch 2 times, most recently from 96cd8c4 to d2b811e Compare October 9, 2023 21:49
chienandalu
chienandalu suggested changes Oct 10, 2023
View reviewed changes
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
sale_loyalty_limit/models/sale_order.py Outdated Show resolved Hide resolved
@pilarvargas-tecnativa
Copy link
Contributor Author

@chienandalu I have gone round the whole code reducing what was not necessary:

  • When calculating how many times it has been used it is not necessary to take into account the coupons, you simply have to take into account the order lines where the program that complies with the rules has been applied.

  • It is only necessary to make the check in _program_check_compute_points.

Keeping in mind that "if one rule passes, they all pass":

  • For the limit of uses for a client, if all rules have that limit set, it will be taken into account that if in a rule has not yet reached the limit the promotion will still apply, i.e. the highest limit set in the rules. If it is not stated in all the rules then there will be no limit to apply.

  • For the limit of uses set for a seller, it will only apply in case all rules have it set. It will be taken into account whether or not the limit of the rule with the highest limit has been reached, because if one passes, they all pass, the minimum limits will be exceeded by the maximum limits.

  • The strict limit rule will also be applied only if it has been established in all the rules defined in the promotion.

I hope everything is well explained and understood :)

@pilarvargas-tecnativa
Copy link
Contributor Author

What I am left wondering is that I cannot use the computed field to get the number of times a promotion has been used due to the execution of the _update_programs_and_rewards method when confirming a sales order because it does not exclude the current order.

@chienandalu
Copy link
Member

What I am left wondering is that I cannot use the computed field to get the number of times a promotion has been used due to the execution of the _update_programs_and_rewards method when confirming a sales order because it does not exclude the current order.

Why would you exclude the current order?

chienandalu
chienandalu reviewed Nov 14, 2023
View reviewed changes
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.

When calculating how many times it has been used it is not necessary to take into account the coupons, you simply have to take into account the order lines where the program that complies with the rules has been applied.

What about generated coupons?

Keeping in mind that "if one rule passes, they all pass"...

I'm wondering if it makes much sense to have the limits at that level and not at loyalty.program globaly

sale_loyalty_limit/tests/test_sale_loyalty_limit.py Outdated Show resolved Hide resolved
sale_loyalty_limit/tests/test_sale_loyalty_limit.py Outdated Show resolved Hide resolved
@pilarvargas-tecnativa
Copy link
Contributor Author

Why would you exclude the current order?

Because when confirming the sale order (action_confirm) it calls again _update_programs_and_rewards. If we set the limit to 1 use, when the promotion is applied it calls _update_programs_and_rewards and if it has not been used it will calculate 0 uses, when confirming the sales order it calls _update_programs_and_rewards again, it recalculates the uses and will take into account the use in the current sales order and eliminates the reward line, so if the limit is set to 1 it will never be applied after confirming the order. You would have to set the limit to 2 in order for it to be applied 1 time.

@pilarvargas-tecnativa
Copy link
Contributor Author

When calculating how many times it has been used it is not necessary to take into account the coupons, you simply have to take into account the order lines where the program that complies with the rules has been applied.

What about generated coupons?

Example of a promotion that generates coupons with a limit of 1 use for admin, 1 coupon generated by this promotion has already been used in a sales order and in the images that I show I am about to apply another generated coupon, in this case I should not allow it since the limit has been set to 1 use and it has already been used.
generatedcoupons

@pilarvargas-tecnativa
Copy link
Contributor Author

I'm wondering if it makes much sense to have the limits at that level and not at loyalty.program globaly

It doesn't have to, the logical thing to do is to configure all the application parameters of a promotion in the same rule, as it was done in v15, although the logic adapts to what odoo proposes.

@chienandalu
Copy link
Member

It doesn't have to, the logical thing to do is to configure all the application parameters of a promotion in the same rule, as it was done in v15, although the logic adapts to what odoo proposes.

But I can't figure out a case where you could take advantage of multiple rules with different limits. What the purpose would be?

It could fit in a multiple rewards design, as you could stablish different limits to different rewards. But in any case I wouldn't complicate this more than necessary and simply put the limits at program level where they were

@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-sale_coupon_limit branch 3 times, most recently from d2164e6 to b79a932 Compare November 15, 2023 15:34
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-sale_coupon_limit branch 3 times, most recently from 8e347f1 to 7125bd8 Compare November 17, 2023 09:54
@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Nov 18, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Nov 17, 2023
Open
15 tasks
chienandalu and others added 12 commits November 20, 2023 07:53
@chienandalu @pilarvargas-tecnativa
[ADD] sale_coupon_limit: New module
34b0edd 
TT30847
@chienandalu @pilarvargas-tecnativa
[IMP] sale_coupon_limit: usage info
bcec8ed 
We add some compute fields to check the current promotion usage by the
limited users. That allows us to refactor the check method a little bit
as well unifying the logic.

TT30847
@chienandalu @pilarvargas-tecnativa
[FIX] sale_coupon_limit: compatibility with sale_couopon_selection_wi…
7087676 
…zard

TT32558
@chienandalu @pilarvargas-tecnativa
[FIX] sale_coupon_limit: cancelled orders
5daac23 
Coupons are reset when a sale order is cancelled, but applied programs
remain linked to the cancelled order. As they're cancelled, we should
not count them in the limits.

TT34264
@chienandalu @pilarvargas-tecnativa
[FIX] sale_coupon_limit: improve rule times used count
d5db372 
Sometimes Odoo doesn't remove the rules properly until the promotions
are refreshed. This could lead to count mismatches. We wan't to ensure
the count looking at the lines instead.

TT34266
@pedrobaeza @pilarvargas-tecnativa
[FIX] sale_coupon_limit: Lower development status
727c5c8 
For complying with rules.
@chienandalu @pilarvargas-tecnativa
[FIX] sale_couopn_limit: ensure compatibility
564af6c 
Simplify rule and ensure compatibility with
sale_coupon_multiple_code_program wich allows to use multiple code
programs putting them in the no code promotions many2many field.
@paradoxxxzero @pilarvargas-tecnativa
[MIG] sale_coupon_limit: Migration to 14.0
05dc4c5
@chienandalu @pilarvargas-tecnativa
[MIG] sale_coupon_limit: Migration to 15.0
0cbd8c9 
TT38310
@chienandalu @pilarvargas-tecnativa
[IMP] coupon_limit, sale_coupon_limit split
ef80e61 
Allow coupon_limit settings to be used by other loyalty modules
@francesco-ooops @pilarvargas-tecnativa
Added translation using Weblate (Italian)
1a640b4
@francesco-ooops @pilarvargas-tecnativa
Translated using Weblate (Italian)
d174851 
Currently translated at 100.0% (9 of 9 strings)

Translation: sale-promotion-15.0/sale-promotion-15.0-sale_coupon_limit
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-15-0/sale-promotion-15-0-sale_coupon_limit/it/
chienandalu
chienandalu approved these changes Nov 20, 2023
View reviewed changes
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.

Thanks :)

@pilarvargas-tecnativa
Copy link
Contributor Author

@CarlosRoca13 take a look please :)

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-156-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a8d6112 into OCA:16.0 Nov 20, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at e2e702a. Thanks a lot for contributing to OCA. ❤️

@pedrobaeza pedrobaeza deleted the 16.0-mig-sale_coupon_limit branch November 20, 2023 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlosRoca13 CarlosRoca13 CarlosRoca13 approved these changes

@chienandalu chienandalu chienandalu approved these changes

Assignees
No one assigned
Labels
approved merged 🎉
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
7 participants
@pilarvargas-tecnativa @chienandalu @pedrobaeza @OCA-git-bot @CarlosRoca13 @paradoxxxzero @francesco-ooops