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

[FW][FIX] website_sale: harmonize tax computation #159764

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Mar 28, 2024

Standard sale tax flows rely on _get_tax_included_price_unit,
whereas part of website_sale flows do, while another part relies
on _fix_tax_included_price_company, which doesn't handle some
advanced cases (fiscal position mapping of price_included taxes).

This commit drops the use of _fix_tax_included_price_company in
website_sale, to only use the newest API of _get_tax_included_price_unit,
supposed to handle more cases.

Also makes all taxes computation go through a single entry point,
_apply_taxes_to_price, already used for combination_info logic (/shop/product),
but not in _get_sales_prices (/shop page).

opw-3700803

Fixes #155162


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

Forward-Port-Of: #159122

@robodoo
Copy link
Contributor

robodoo commented Mar 28, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented Mar 28, 2024

@Feyensv @smetl cherrypicking of pull request #159122 failed.

stdout:

Auto-merging addons/account/models/product.py
Auto-merging addons/sale/models/sale_order_line.py
CONFLICT (content): Merge conflict in addons/sale/models/sale_order_line.py

stderr:

19:02:11.316330 git.c:463               trace: built-in: git cherry-pick f53495c9bc5250f815d92bd222b2d0131fb37ba9
error: could not apply f53495c9bc52... [IMP] account: split _get_tax_included_unit_price_from_price util
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Mar 28, 2024
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Mar 28, 2024
@Feyensv Feyensv self-assigned this Mar 29, 2024
@Feyensv Feyensv force-pushed the saas-16.3-16.0-wsale-tax-broll-vfe-u9UY-fw branch 3 times, most recently from 3a9737d to f4bdbf2 Compare March 29, 2024 11:42
separate the true computation of the tax value from the computation of the base unit price (uom & curr conversion, ...).

That way, the risks of unwanted side-effects is reduced when the base unit price is already computed (sale, website_sale).

Also allows preprocessing the taxes mapping, which doesn't have to be done multiple times when we compute different amounts for the same product/template.

original commit: 9ba0bf8
@Feyensv Feyensv force-pushed the saas-16.3-16.0-wsale-tax-broll-vfe-u9UY-fw branch 5 times, most recently from 4bbce71 to 8400616 Compare March 29, 2024 15:12
Standard `sale` tax flows rely on `_get_tax_included_price_unit`,
whereas part of `website_sale` flows do, while another part relies
on `_fix_tax_included_price_company`, which doesn't handle some
advanced cases (fiscal position mapping of price_included taxes).

This commit drops the use of `_fix_tax_included_price_company` in
website_sale, to only use the newest API of `_get_tax_included_price_unit`,
supposed to handle more cases.

Also makes all taxes computation go through a single entry point,
`_apply_taxes_to_price`, already used for `combination_info` logic (/shop/product),
but not in `_get_sales_prices` (/shop page).

opw-3700803

original commit : 5e522d4
@Feyensv Feyensv force-pushed the saas-16.3-16.0-wsale-tax-broll-vfe-u9UY-fw branch from 8400616 to cc9c014 Compare March 29, 2024 17:01
@Feyensv
Copy link
Contributor

Feyensv commented Apr 2, 2024

@robodoo r+ rebase-ff

@robodoo
Copy link
Contributor

robodoo commented Apr 2, 2024

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Apr 2, 2024
separate the true computation of the tax value from the computation of the base unit price (uom & curr conversion, ...).

That way, the risks of unwanted side-effects is reduced when the base unit price is already computed (sale, website_sale).

Also allows preprocessing the taxes mapping, which doesn't have to be done multiple times when we compute different amounts for the same product/template.

original commit: 9ba0bf8

Part-of: #159764
robodoo pushed a commit that referenced this pull request Apr 2, 2024
Standard `sale` tax flows rely on `_get_tax_included_price_unit`,
whereas part of `website_sale` flows do, while another part relies
on `_fix_tax_included_price_company`, which doesn't handle some
advanced cases (fiscal position mapping of price_included taxes).

This commit drops the use of `_fix_tax_included_price_company` in
website_sale, to only use the newest API of `_get_tax_included_price_unit`,
supposed to handle more cases.

Also makes all taxes computation go through a single entry point,
`_apply_taxes_to_price`, already used for `combination_info` logic (/shop/product),
but not in `_get_sales_prices` (/shop page).

opw-3700803

original commit : 5e522d4

closes #159764

Signed-off-by: Laurent Smet (las) <las@odoo.com>
Signed-off-by: Victor Feyens (vfe) <vfe@odoo.com>
@robodoo robodoo closed this Apr 2, 2024
@fw-bot fw-bot deleted the saas-16.3-16.0-wsale-tax-broll-vfe-u9UY-fw branch April 16, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants