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][FIX] pos_order_to_sale_order: recompute taxes #1318

Conversation

danielduqma
Copy link
Contributor

@danielduqma danielduqma commented Feb 21, 2025

Recompute sale.order taxes, as received ones are the ones from product, without fiscal position mapping applied. Follows #1291

How to reproduce in branch 16.0:

  1. Configure a fiscal position that maps an included tax to an excluded one
    image
  2. Configure a product with included taxes
    image
  3. Add this product to cart in PoS
    image
  4. Created sale.order has different taxes and totals
    image

With this change, taxes are recomputed right after sale order creation, so taxes are the same as in PoS:

image

cc @legalsylvain @Jordi-Buitrago @navarrorico

FL-556-5334

@OCA-git-bot
Copy link
Contributor

Hi @legalsylvain,
some modules you are maintaining are being modified, check this out!

@legalsylvain
Copy link
Contributor

hi @danielduqma you can rebase to have green CI.

@danielduqma danielduqma force-pushed the 16.0-fix-pos_order_to_sale_order-taxes-mapping branch from 5f3da08 to 934f79d Compare February 24, 2025 07:49
Recompute `sale.order` taxes, as received ones are the ones from
product, without fiscal position mapping applied.
@danielduqma danielduqma force-pushed the 16.0-fix-pos_order_to_sale_order-taxes-mapping branch from 934f79d to 5f81300 Compare February 24, 2025 08:51
@danielduqma danielduqma changed the title [16.0][FIX] pos_order_to_sale_order: mapped taxes [16.0][FIX] pos_order_to_sale_order: recompute taxes Feb 24, 2025
@danielduqma danielduqma marked this pull request as ready for review February 24, 2025 09:33
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link

@alejandro-vallejoFL alejandro-vallejoFL left a comment

Choose a reason for hiding this comment

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

LGTM

@legalsylvain
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1318-by-legalsylvain-bump-patch, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 925e776 into OCA:16.0 Feb 25, 2025
5 of 7 checks passed
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.

4 participants