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

Many unsupported exchange rates: Switch from frankfurter.app to exchangerate.host or allow custom API #25603

Closed
aschl opened this issue May 6, 2021 · 14 comments · Fixed by #26237

Comments

@aschl
Copy link

aschl commented May 6, 2021

Problem:
Exchange rates are currently fetched from https://www.frankfurter.app/. Unfortunately, this API only supports about 30-40 currencies with many world currencies still being not supported. This makes using a multi-currency setup very cumbersome to impossible to maintain if exchange rates have to be added manually.

Proposed solution:
There are other free APIs available that support more exchange rates. One promising API seems to be this one: https://exchangerate.host/
It is also free and open source (https://github.com/Formicka/exchangerate.host) and apparently supports 170 currencies (+ 6000 cryptocurrencies - not sure but might be also interesting for some ERPnext users?!).

It doesn't seem to be too complicated to adjust the current code to support. Unfortunately, I am hosting my instance on frappe.cloud, so I don't have the option to work on the code.
In erpnext/setup/utils.py (L101) the frankfurter.app API is currently called. It seems one would only need to adjust the API call and link to exchangerate.host and this would directly add >100 additional currencies supported by ERPnext (+many cryptocurrencies).
It would be amazing if someone from the core team could test this. It doesn't seem to be a hard fix.

Potential alternatives
Potentially, ERPnext could allow the user to specify the API (and its settings) similar to how it is currently available for SMS APIs (https://docs.erpnext.com/docs/user/manual/en/setting-up/sms-setting). This might be a little bit more work, but would make multi-currency support even more flexible. If the core team wishes to have frankfurter.app still as default, this could be left as a default value. However this way, the user could change to the API they wish to use and could also use a commercial API (such as fixer.io) if they need/want.

Additional context
This issue has been raised already quite some time ago in this thread in the Forum:
https://discuss.erpnext.com/t/is-there-any-plan-for-the-missing-exchange-rates-on-www-frankfurter-app/48782
I believe many companies especially in developing countries with lesser known currencies, that are currently unsupported by the frankfurter.app, would benefit from this.

@aschl
Copy link
Author

aschl commented May 10, 2021

The code in erpnext/setup/utils.py (L101) needs to be changed from

response = requests.get(api_url, params={
    "base": from_currency,
    "symbols": to_currency
})
# expire in 6 hours
response.raise_for_status()
value = response.json()["rates"][to_currency]

to

response = requests.get(api_url, params={
    "symbols": from_currency+","+to_currency,
    # for additional crypto support:
    "source": "crypto"
})
# expire in 6 hours
response.raise_for_status()
value = response.json()["rates"][to_currency]/response.json()["rates"][from_currency]

I tested several currencies(incl. crypto-currencies), which are currently unsupported by frankfurter.app, and it works now with the new API (exchangerate.host).
(As said, I don't have access to a self-hosted version, so I would be glad if someone could implement it, test it and push this feature-request. Thanks a lot!)

@moe01324
Copy link

@aschl i have a v13 test instance running, if you detail me a bit more what I shall test I can give it a try.

rtdany10 added a commit to rtdany10/erpnext that referenced this issue May 15, 2021
Switch from frankfurter.app to exchangerate.host to accomodate more currency usage.
Closes frappe#25603
@aakvatech
Copy link
Contributor

This is a neccesary change to be done

@aschl
Copy link
Author

aschl commented May 15, 2021

I think a few transactions in different currencies using the new API on ERPnext would be sufficient to check whether the API is called correctly. As long as the API call works and the numbers are retrieved correctly, everything should work properly IMHO. (In fact the suggested code is mostly a change in the link that is called and small adjustments how the rate is extracted, so no magic...)

@aakvatech
Copy link
Contributor

Let me try to do a PR, since it's a change in the URL only. Even if we bring in crypto as a 2nd stage change, it's fine. One change at a time.

@rtdany10
Copy link
Contributor

I have already send a PR regarding the same. PR #25722.
Have passed base currency as a parameter to get exact rate.

@dawoodjee
Copy link
Contributor

dawoodjee commented May 16, 2021

Weird, its not working for me

image

I think I've pasted my code right

@frappe.whitelist()
def get_exchange_rate(from_currency, to_currency, transaction_date=None, args=None):
	if not (from_currency and to_currency):
		# manqala 19/09/2016: Should this be an empty return or should it throw and exception?
		return
	if from_currency == to_currency:
		return 1

	if not transaction_date:
		transaction_date = nowdate()
	currency_settings = frappe.get_doc("Accounts Settings").as_dict()
	allow_stale_rates = currency_settings.get("allow_stale")

	filters = [
		["date", "<=", get_datetime_str(transaction_date)],
		["from_currency", "=", from_currency],
		["to_currency", "=", to_currency]
	]

	if args == "for_buying":
		filters.append(["for_buying", "=", "1"])
	elif args == "for_selling":
		filters.append(["for_selling", "=", "1"])

	if not allow_stale_rates:
		stale_days = currency_settings.get("stale_days")
		checkpoint_date = add_days(transaction_date, -stale_days)
		filters.append(["date", ">", get_datetime_str(checkpoint_date)])

	# cksgb 19/09/2016: get last entry in Currency Exchange with from_currency and to_currency.
	entries = frappe.get_all(
		"Currency Exchange", fields=["exchange_rate"], filters=filters, order_by="date desc",
		limit=1)
	if entries:
		return flt(entries[0].exchange_rate)

	try:
		cache = frappe.cache()
		key = "currency_exchange_rate_{0}:{1}:{2}".format(transaction_date, from_currency, to_currency)
		value = cache.get(key)

		if not value:
			import requests
			api_url = "https://api.exchangerate.host/convert"
			response = requests.get(api_url, params={
				"date": transaction_date,
				"from": from_currency,
				"to": to_currency
			})
			# expire in 6 hours
			response.raise_for_status()
			value = response.json()["result"]
			cache.setex(key, value, 6 * 60 * 60)
		return flt(value)
	except:
		frappe.log_error(title="Get Exchange Rate")
		frappe.msgprint(_("Unable to find exchange rate for {0} to {1} for key date {2}. Please create a Currency Exchange record manually").format(from_currency, to_currency, transaction_date))
		return 0.0

def enable_all_roles_and_domains():
	""" enable all roles and domain for testing """
	# add all roles to users
	domains = frappe.get_all("Domain")
	if not domains:
		return

@rtdany10
Copy link
Contributor

@dawoodjee Hope you restarted the bench after you made changes. Also, try digging into the exact error raised by the try-catch.

@dawoodjee
Copy link
Contributor

dawoodjee commented May 17, 2021

I did restart, I'm using v13, isnt it just suppose to work on mine the way its working on your server?

I'll do some troubleshooting, thanks for the pointers.

@aschl
Copy link
Author

aschl commented Jul 17, 2021

Let me try to do a PR, since it's a change in the URL only. Even if we bring in crypto as a 2nd stage change, it's fine. One change at a time.

Hi @aakvatech, can you have a look at the PRs #25722 and #26237 respectively? I am confused where the PR is stuck and what needs to be done. This PR is open since more than two months and I'm not sure why it's not getting merged. Happy to help out somewhere if you or someone else gives me guidance what needs to be fixed to get this finally merged. Thank you very much!

@rtdany10
Copy link
Contributor

We got approved #26237

@Craftint
Copy link

How do we get a access key

image

@Craftint
Copy link

@rtdany10

@rtdany10
Copy link
Contributor

You can use frankfurter.app for free else get a token from https://exchangerate.host/

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants