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

[17.0][MIG] sale_financial_risk #377

Merged
merged 62 commits into from
Jun 18, 2024

Conversation

sbiosca-s73
Copy link

@sbiosca-s73 sbiosca-s73 commented May 2, 2024

Migrate sale_financial_risk module to version 17.0

@sbiosca-s73 sbiosca-s73 changed the title 17.0 mig sale financial risk [17.0][MIG] sale_financial_risk May 2, 2024
@sbiosca-s73 sbiosca-s73 force-pushed the 17.0-mig_sale_financial_risk branch from 3c094a8 to 82f088f Compare May 2, 2024 10:17
@pedrobaeza
Copy link
Member

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone May 2, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request May 2, 2024
8 tasks
sale_financial_risk/models/res_partner.py Outdated Show resolved Hide resolved
sale_financial_risk/models/sale.py Outdated Show resolved Hide resolved
@sbiosca-s73 sbiosca-s73 force-pushed the 17.0-mig_sale_financial_risk branch from 82f088f to cb77e01 Compare May 2, 2024 11:03
@sbiosca-s73
Copy link
Author

@carlosdauden changes done

@sbiosca-s73 sbiosca-s73 force-pushed the 17.0-mig_sale_financial_risk branch from bb179a5 to 2ad78b5 Compare May 2, 2024 13:50
@sbiosca-s73
Copy link
Author

@carlosdauden changes done

@sbiosca-s73
Copy link
Author

Not working test-requirements because the version of module account_financial_risk isn't correctly version

Copy link
Contributor

@carlosdauden carlosdauden left a comment

Choose a reason for hiding this comment

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

Code LGTM

carlosdauden and others added 13 commits May 27, 2024 16:24
Currently translated at 7.7% (1 of 13 strings)

Translation: credit-control-12.0/credit-control-12.0-sale_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-sale_financial_risk/fr/
Currently translated at 7.7% (1 of 13 strings)

Translation: credit-control-12.0/credit-control-12.0-sale_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-sale_financial_risk/pt_BR/
Currently translated at 46.2% (6 of 13 strings)

Translation: credit-control-12.0/credit-control-12.0-sale_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-sale_financial_risk/it/
josep-tecnativa and others added 7 commits May 27, 2024 16:24
- Include context keys for avoiding mail operations overhead.
Currently translated at 100.0% (21 of 21 strings)

Translation: credit-control-16.0/credit-control-16.0-sale_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-sale_financial_risk/es/
This issue appeared for multi-company, multi-currency configuration. Whenever risk was calculated for the partner, Sale Order amount was converted to company currency, instead of currency set on the partner.

This commit fixes the issue by converting to correct currency (set on the partner).
The test_manual_currency_risk_not_exceeded test that assesses that the
manual currency risk has not been passed fails when another module changes
the company's currency. The reason for the failure is that if the company
currency matches the currency of the sell order it does not convert
correctly and in that case it interprets the limit within the same currency.
For example, if the company currency is set as EUR and the sales order
is placed in EUR, the conversion of the 100€ is not done and therefore
exceeds the limit set to 99. To solve this, the company currency is
first checked to set a different currency in the order. On the other
hand, the tests should be frozen at a certain date to prevent the
exchange rate of the exchange currency from going up or down thus
avoiding errors when running the test, which should check that it has
not been exceeded.
Currently translated at 100.0% (21 of 21 strings)

Translation: credit-control-16.0/credit-control-16.0-sale_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-16-0/credit-control-16-0-sale_financial_risk/nl/
@sbiosca-s73 sbiosca-s73 force-pushed the 17.0-mig_sale_financial_risk branch 6 times, most recently from 1bd780a to 3e15923 Compare May 27, 2024 14:59
@OCA-git-bot
Copy link
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). 🤖

@Danisan
Copy link
Sponsor

Danisan commented Jun 17, 2024

@pedrobaeza good work!

@pedrobaeza
Copy link
Member

You have to give thanks to the contributors doing the migration and the review.

@sbiosca-s73 please split the pre-commit changes from the migration itself as stated in the migration guide: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0

@sbiosca-s73
Copy link
Author

@pedrobaeza changes done. Thanks!

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 954f57c into OCA:17.0 Jun 18, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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