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] hr_expense: multi-currency #30959

Closed
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
3 participants
@nim-odoo
Copy link
Contributor

nim-odoo commented Feb 8, 2019

  • Set your company to USD
  • Create an expense in EUR:
    Amount: 100
    Tax: 15% Excluded
  • Validate, post the journal entries

Not a single AML is correct.

opw-1938570

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

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

@robodoo robodoo added the seen 🙂 label Feb 8, 2019

@nim-odoo nim-odoo force-pushed the odoo-dev:12.0-opw-1938570-hr_expense-nim branch to 60fe62f Feb 8, 2019

@nim-odoo nim-odoo changed the title [FIX] hr_expense: multi-currency [WIP][FIX] hr_expense: multi-currency Feb 8, 2019

@nim-odoo

This comment has been minimized.

Copy link
Contributor Author

nim-odoo commented Feb 8, 2019

Test necessary

@nim-odoo nim-odoo added the OE label Feb 8, 2019

@nim-odoo nim-odoo requested a review from kebeclibre Feb 8, 2019

@nim-odoo

This comment has been minimized.

Copy link
Contributor Author

nim-odoo commented Feb 8, 2019

@kebeclibre

This comment has been minimized.

Copy link
Contributor

kebeclibre commented Feb 8, 2019

Ouch, indeed before the fix doesn't look good.
Fix seems okay, and screenshot after the fix too, provided that:

  • the expense report has two entries for the Air Flight
  • one of 100 taxed 15% ; in Euros
  • the other of 700 taxed 15%; in Euros

I'd also recommend to write some tests

@nim-odoo nim-odoo force-pushed the odoo-dev:12.0-opw-1938570-hr_expense-nim branch 2 times, most recently from 7c0ad69 to 2087a1f Feb 11, 2019

@nim-odoo nim-odoo changed the title [WIP][FIX] hr_expense: multi-currency [FIX] hr_expense: multi-currency Feb 11, 2019

@nim-odoo nim-odoo self-assigned this Feb 11, 2019

@nim-odoo nim-odoo force-pushed the odoo-dev:12.0-opw-1938570-hr_expense-nim branch from 2087a1f to 33f959e Feb 11, 2019

@robodoo robodoo added the CI 🤖 label Feb 11, 2019

@nim-odoo

This comment has been minimized.

Copy link
Contributor Author

nim-odoo commented Feb 11, 2019

robodoo r+

robodoo pushed a commit that referenced this pull request Feb 11, 2019

[FIX] hr_expense: multi-currency
- Set your company to USD
- Create an expense in EUR:
  Amount: 100
  Tax: 15% Excluded
- Validate, post the journal entries

It crashes because of a missing `.id`, but on top of that... not a
single AML is correct.

opw-1938570

closes #30959
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 11, 2019

Staging failed: ci/runbot (view more at http://runbot.odoo.com/runbot/build/445803)

@robodoo robodoo added the error 🙅 label Feb 11, 2019

[FIX] hr_expense: multi-currency
- Set your company to USD
- Create an expense in EUR:
  Amount: 100
  Tax: 15% Excluded
- Validate, post the journal entries

It crashes because of a missing `.id`, but on top of that... not a
single AML is correct.

opw-1938570

@nim-odoo nim-odoo force-pushed the odoo-dev:12.0-opw-1938570-hr_expense-nim branch from 33f959e to ab06e9c Feb 12, 2019

@nim-odoo

This comment has been minimized.

Copy link
Contributor Author

nim-odoo commented Feb 12, 2019

robodoo r+

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 12, 2019

I'm sorry, @nim-odoo. This PR is already reviewed, reviewing it again is useless.

@nim-odoo

This comment has been minimized.

Copy link
Contributor Author

nim-odoo commented Feb 12, 2019

robodoo r-

@robodoo robodoo added CI 🤖 and removed error 🙅 labels Feb 12, 2019

@nim-odoo

This comment has been minimized.

Copy link
Contributor Author

nim-odoo commented Feb 12, 2019

robodoo r+

@robodoo robodoo added the r+ 👌 label Feb 12, 2019

robodoo pushed a commit that referenced this pull request Feb 12, 2019

[FIX] hr_expense: multi-currency
- Set your company to USD
- Create an expense in EUR:
  Amount: 100
  Tax: 15% Excluded
- Validate, post the journal entries

It crashes because of a missing `.id`, but on top of that... not a
single AML is correct.

opw-1938570

closes #30959
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 12, 2019

Merged, thanks!

@robodoo robodoo closed this Feb 12, 2019

@nim-odoo nim-odoo deleted the odoo-dev:12.0-opw-1938570-hr_expense-nim branch Feb 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment