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

[15.0][MIG] sale_coupon_auto_refresh: Migration to version 15.0 #102

Merged
merged 26 commits into from
Feb 1, 2023

Conversation

pilarvargas-tecnativa
Copy link
Contributor

@pilarvargas-tecnativa pilarvargas-tecnativa commented Jan 27, 2023

cc @Tecnativa TT37307

@chienandalu @victoralmau please review

chienandalu and others added 24 commits January 26, 2023 16:06
When applying a promo code via wizard, Odoo creates a sale.order.line *before* filling the sale.order 'code_promo_program_id' field.
After the creation, though, the auto refresh override kicks in, calling the 'recompute_coupon_lines' method.
That method finds a sale.order.line that was created from a promo program, but such program is not linked to the sale.order, and so the line gets deleted.
However, since the line has just been created, Odoo will try to run the '_check_company' method on it, which will try to read the field 'company_id' of the deleted line.
This operation will result in a MissingError, raised by the '__get__' method of Field class.

This error is fixed by adding the 'skip_auto_refresh_coupons' flag to the wizard context when executing the 'process_coupon' method.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-promotion-14.0/sale-promotion-14.0-sale_coupon_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-14-0/sale-promotion-14-0-sale_coupon_auto_refresh/
Currently translated at 100.0% (14 of 14 strings)

Translation: sale-promotion-14.0/sale-promotion-14.0-sale_coupon_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-14-0/sale-promotion-14-0-sale_coupon_auto_refresh/zh_CN/
Currently translated at 100.0% (14 of 14 strings)

Translation: sale-promotion-14.0/sale-promotion-14.0-sale_coupon_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-14-0/sale-promotion-14-0-sale_coupon_auto_refresh/fr/
we need to pass skip_auto_refresh_coupons=True in the context when
adding / editing / removing lines otherwise we can end up with the
promotion line being added twice or removed twice (the latter causing a
crash)
This commit, alongside with odoo/odoo#77989, fixes MissingError being raised when:
- the line(s) that allow coupons to be applied are deleted
- the line(s) that allow coupons to be applied are updated making coupons applicable no more

Moreover:
- a check on which fields are updated is introduced in order to boost performances
- the module has been slightly refactored for better readability

Closes OCA#20
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-promotion-14.0/sale-promotion-14.0-sale_coupon_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-14-0/sale-promotion-14-0-sale_coupon_auto_refresh/
Currently translated at 6.2% (1 of 16 strings)

Translation: sale-promotion-14.0/sale-promotion-14.0-sale_coupon_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-14-0/sale-promotion-14-0-sale_coupon_auto_refresh/it/
Currently translated at 100.0% (16 of 16 strings)

Translation: sale-promotion-14.0/sale-promotion-14.0-sale_coupon_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-14-0/sale-promotion-14-0-sale_coupon_auto_refresh/it/
This can allow us to work altogether with flexible triggers like the
ones we could need with sale_coupon_criteria_order_based

TT37479
We should not refresh the lines before the proccess of applying the
coupon is finished.

TT38806
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 :) (Remove WIP from the PR title)

@pilarvargas-tecnativa pilarvargas-tecnativa changed the title [15.0][WIP] sale_coupon_auto_refresh: Migration to version 15.0 [15.0][MIG] sale_coupon_auto_refresh: Migration to version 15.0 Feb 1, 2023
@pedrobaeza
Copy link
Member

Please rename the migration commit from [WIP] sale_coupon_auto_refresh: Migration to version 15.0 to [MIG] sale_coupon_auto_refresh: Migration to version 15.0

Use the new `get_depends()` method as the former `depends` attribute
(now in `_depends`) is no longer reliable.
@pilarvargas-tecnativa
Copy link
Contributor Author

Please rename the migration commit from [WIP] sale_coupon_auto_refresh: Migration to version 15.0 to [MIG] sale_coupon_auto_refresh: Migration to version 15.0

Sorry I didn't realise, it's changed, thanks!

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot migration sale_coupon_auto_refresh
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Feb 1, 2023
@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 15.0-ocabot-merge-pr-102-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot mentioned this pull request Feb 1, 2023
21 tasks
OCA-git-bot added a commit that referenced this pull request Feb 1, 2023
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-102-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-102-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-102-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 4bbaa26 into OCA:15.0 Feb 1, 2023
@OCA-git-bot
Copy link
Contributor

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

pilarvargas-tecnativa pushed a commit to Tecnativa/sale-promotion that referenced this pull request May 30, 2023
Signed-off-by pedrobaeza
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants