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

[FIX] purchase_advance_payment: tests currency #5

Merged

Conversation

AurelijaNorvaisaite
Copy link

@AurelijaNorvaisaite AurelijaNorvaisaite commented Mar 20, 2024

Add check to make sure that order and company have different currencies.

https://hive.versada.eu/projects/devops/work_packages/19186

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (16.0-versada@86a9193). Click here to learn what that means.

Additional details and impacted files
@@               Coverage Diff               @@
##             16.0-versada       #5   +/-   ##
===============================================
  Coverage                ?   96.76%           
===============================================
  Files                   ?      313           
  Lines                   ?     6054           
  Branches                ?      691           
===============================================
  Hits                    ?     5858           
  Misses                  ?       95           
  Partials                ?      101           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -146,6 +146,8 @@ def setUpClass(cls):
"currency_id": cls.currency_usd.id,
}
)
if cls.purchase_order_1.currency_id == cls.purchase_order_1.company_id.currency_id:
cls.purchase_order_1.currency_id = cls.currency_usd

Choose a reason for hiding this comment

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

Shouldn't we then always force USD currency? Or it will not work correctly with EUR then? Also would be nice to have a comment explaining why this is needed.

Either way, check seems strange. Maybe just check what kind of currency company currency is and decide. Why order currency must match company currency to switch to usd?..

Choose a reason for hiding this comment

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

By the way, can you enable pre-commit on this submodule and run it, so it would fix code according to OCA guidelines.

Choose a reason for hiding this comment

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

Before our change, company and order already had different currencies. And these tests only works when currencies are different.

Add cehck to make sure that order and company have different currencies.

https://hive.versada.eu/projects/devops/work_packages/19186
@AurelijaNorvaisaite AurelijaNorvaisaite force-pushed the fix-tests-wkf-flexible-test-scopes-avi branch from 590870b to 2b2b75e Compare March 20, 2024 12:50
@AurelijaNorvaisaite AurelijaNorvaisaite merged commit 37cc8d8 into 16.0-versada Mar 20, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants