Skip to content

[FIX] sale_order_currency_rate : Change curency_rate calculation#3260

Merged
OCA-git-bot merged 1 commit intoOCA:16.0from
factorlibre:16.0-sale_order_currency_rate
Feb 2, 2026
Merged

[FIX] sale_order_currency_rate : Change curency_rate calculation#3260
OCA-git-bot merged 1 commit intoOCA:16.0from
factorlibre:16.0-sale_order_currency_rate

Conversation

@dalonsofl
Copy link
Copy Markdown
Contributor

@dalonsofl dalonsofl commented Aug 2, 2024

Module

sale_order_currency_rate

Describe the bug

The currency rate applied is not correct and the value in report is incorrect too

To Reproduce

  1. Create a sale order and confirm
  2. Check the value in sale report

(image from runboat OCA)

image

Captura desde 2024-08-02 16-01-00

Instead of divining by the unit per USD is using the USD per unit and the value is incorect. Because this reason, the best option is using the base odoo calculation of currency_rate.

image

Affected versions:

16.0

@dalonsofl dalonsofl force-pushed the 16.0-sale_order_currency_rate branch 2 times, most recently from 27bf36b to 247e5c5 Compare August 2, 2024 14:14
@dalonsofl dalonsofl changed the title [FIX] sale_order_currency_rate : Change [FIX] sale_order_currency_rate : Change curency_rate calculation Aug 2, 2024
Copy link
Copy Markdown
Contributor

@aliciagaarzo aliciagaarzo left a comment

Choose a reason for hiding this comment

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

LGTM!

@rousseldenis
Copy link
Copy Markdown
Contributor

@dalonsofl Could you add a test case that highlight the bug and avoid regression ?

@dalonsofl dalonsofl force-pushed the 16.0-sale_order_currency_rate branch from 247e5c5 to 2c6b4e0 Compare September 24, 2024 16:44
@dalonsofl
Copy link
Copy Markdown
Contributor Author

Hi @rousseldenis !

I have added a simple test recently (I hope it will be enough), if anything else is needed, text me again without problem.

Regards

Copy link
Copy Markdown
Contributor

@amarcosg amarcosg left a comment

Choose a reason for hiding this comment

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

LGTM!

@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link
Copy Markdown
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

LGTM. Of course using core computation is the best

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot merge major

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-3260-by-rousseldenis-bump-major, awaiting test results.

@OCA-git-bot
Copy link
Copy Markdown
Contributor

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

@OCA-git-bot OCA-git-bot merged commit d073051 into OCA:16.0 Feb 2, 2026
2 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.

5 participants