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

10.0 mig currency rate update #415

Merged
merged 9 commits into from
Apr 14, 2017

Conversation

nikiwaibel
Copy link

hi, this is my first pull request to OCA - hope it's all okay

@pedrobaeza pedrobaeza mentioned this pull request Nov 7, 2016
39 tasks
Update README
Code cleanup: remove @api.one, better error messages, remove call to MOD_NAME which was not defined any more
Use account.menu_config_multi_currency as parent menu
@alexis-via
Copy link
Contributor

I improved this PR and made a PR to niki's branch nikiwaibel#1

…ll_mig_improvements

10.0 Improve your PR to port currency_rate_update to v10
Copy link
Contributor

@alexis-via alexis-via left a comment

Choose a reason for hiding this comment

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

Code review and test with ECB service

Copy link

@LeartS LeartS left a comment

Choose a reason for hiding this comment

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

👍 Code review and test with Yahoo Finance and ECB.

Just one question: I see that all res.currency.rates records are set as 00:00:00 time (actually 01:00:00 in my case because of the timezone, which may be a little bug), is there a reason why we can't have multiple rates per day, assuming one executes the rate updater manually?

@alexis-via
Copy link
Contributor

I get a crash when I try to add "currencies to update with this service":

ValueError: Invalid domain term (u'id', u'in', {u'rate_ids': [[5], [4, 129]], u'name': u'EUR', u'symbol': u'\u20ac', u'active': True, u'rate': 1, u'date': u'2010-01-01'})

@alexis-via
Copy link
Contributor

I think we need the same kind of fix for the bug I talked about in my previous comment than the one I made for the module account_payment_order in OCA/bank-payment:
OCA/bank-payment#354

@alexis-via
Copy link
Contributor

Here is my PR to fix the bug nikiwaibel#2

[FIX] Crash when you add currencies to "Currencies to update with this service"
@oca-clabot
Copy link

Hey @nikiwaibel,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

@alexis-via
Copy link
Contributor

@nikiwaibel Have you signed OCA's CLA, or it is just an error of the bot ?

@nikiwaibel
Copy link
Author

nikiwaibel commented Apr 5, 2017 via email

@alexis-via
Copy link
Contributor

The crash I reported 7 days ago and fixed a few days ago was caused by this bug in odoo: odoo/odoo#16072

@alexis-via
Copy link
Contributor

Fix for multi-company nikiwaibel#3

…ticompany

[FIX] currency_rate_update should now work in multi-company
@alexis-via alexis-via merged commit b084b53 into OCA:10.0 Apr 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants