diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index 0ce3d5dea93..42cd44aeabe 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -286,6 +286,99 @@ "search_index": 0, "set_only_once": 0, "unique": 0 + }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "fieldname": "currency_exchange_section", + "fieldtype": "Section Break", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Currency Exchange Settings", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 + }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "1", + "fieldname": "allow_stale", + "fieldtype": "Check", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 1, + "in_standard_filter": 0, + "label": "Allow Stale Exchange Rates", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 + }, + { + "allow_bulk_edit": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "1", + "depends_on": "eval:doc.allow_stale==0", + "fieldname": "stale_days", + "fieldtype": "Int", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Stale Days", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "unique": 0 } ], "has_web_view": 0, @@ -299,7 +392,7 @@ "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2017-06-16 17:39:50.614522", + "modified": "2017-09-05 10:10:03.117505", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.py b/erpnext/accounts/doctype/accounts_settings/accounts_settings.py index dd33ff1ab20..8431173a9ea 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.py +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.py @@ -5,10 +5,20 @@ from __future__ import unicode_literals import frappe -from frappe import _ -from frappe.utils import cint, comma_and +from frappe.utils import cint from frappe.model.document import Document + class AccountsSettings(Document): def on_update(self): - pass \ No newline at end of file + pass + + def validate(self): + self.validate_stale_days() + + def validate_stale_days(self): + if not self.allow_stale and cint(self.stale_days) <= 0: + frappe.msgprint( + "Stale Days should start from 1.", title='Error', indicator='red', + raise_exception=1) + diff --git a/erpnext/accounts/doctype/accounts_settings/test_accounts_settings.js b/erpnext/accounts/doctype/accounts_settings/test_accounts_settings.js new file mode 100644 index 00000000000..f9aa1669643 --- /dev/null +++ b/erpnext/accounts/doctype/accounts_settings/test_accounts_settings.js @@ -0,0 +1,35 @@ +QUnit.module('accounts'); + +QUnit.test("test: Accounts Settings doesn't allow negatives", function (assert) { + let done = assert.async(); + + assert.expect(2); + + frappe.run_serially([ + () => frappe.set_route('Form', 'Accounts Settings', 'Accounts Settings'), + () => frappe.timeout(2), + () => unchecked_if_checked(cur_frm, 'Allow Stale Exchange Rates', frappe.click_check), + () => cur_frm.set_value('stale_days', 0), + () => frappe.click_button('Save'), + () => frappe.timeout(2), + () => { + assert.ok(cur_dialog); + }, + () => frappe.click_button('Close'), + () => cur_frm.set_value('stale_days', -1), + () => frappe.click_button('Save'), + () => frappe.timeout(2), + () => { + assert.ok(cur_dialog); + }, + () => frappe.click_button('Close'), + () => done() + ]); + +}); + +const unchecked_if_checked = function(frm, field_name, fn){ + if (frm.doc.allow_stale) { + return fn(field_name); + } +}; diff --git a/erpnext/accounts/doctype/accounts_settings/test_accounts_settings.py b/erpnext/accounts/doctype/accounts_settings/test_accounts_settings.py new file mode 100644 index 00000000000..bf1e967bdbc --- /dev/null +++ b/erpnext/accounts/doctype/accounts_settings/test_accounts_settings.py @@ -0,0 +1,22 @@ +import unittest + +import frappe + + +class TestAccountsSettings(unittest.TestCase): + def tearDown(self): + # Just in case `save` method succeeds, we need to take things back to default so that other tests + # don't break + cur_settings = frappe.get_doc('Accounts Settings', 'Accounts Settings') + cur_settings.allow_stale = 1 + cur_settings.save() + + def test_stale_days(self): + cur_settings = frappe.get_doc('Accounts Settings', 'Accounts Settings') + cur_settings.allow_stale = 0 + cur_settings.stale_days = 0 + + self.assertRaises(frappe.ValidationError, cur_settings.save) + + cur_settings.stale_days = -1 + self.assertRaises(frappe.ValidationError, cur_settings.save) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js index 61ede97122c..04db9e28ae6 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -403,6 +403,13 @@ frappe.ui.form.on('Payment Entry', { frm.events.set_difference_amount(frm); } + + // Make read only if Accounts Settings doesn't allow stale rates + frappe.model.get_value("Accounts Settings", null, "allow_stale", + function(d){ + frm.set_df_property("source_exchange_rate", "read_only", cint(d.allow_stale) ? 0 : 1); + } + ); }, target_exchange_rate: function(frm) { @@ -421,6 +428,13 @@ frappe.ui.form.on('Payment Entry', { frm.events.set_difference_amount(frm); } frm.set_paid_amount_based_on_received_amount = false; + + // Make read only if Accounts Settings doesn't allow stale rates + frappe.model.get_value("Accounts Settings", null, "allow_stale", + function(d){ + frm.set_df_property("target_exchange_rate", "read_only", cint(d.allow_stale) ? 0 : 1); + } + ); }, paid_amount: function(frm) { diff --git a/erpnext/docs/user/manual/en/accounts/setup/accounts-settings.md b/erpnext/docs/user/manual/en/accounts/setup/accounts-settings.md index 3bada56ffd8..f47f6e66100 100644 --- a/erpnext/docs/user/manual/en/accounts/setup/accounts-settings.md +++ b/erpnext/docs/user/manual/en/accounts/setup/accounts-settings.md @@ -13,4 +13,8 @@ * Unlink Payment on Cancellation of Invoice: If checked, system will unlink the payment against the invoice. Otherwise, it will show the link error. +* Allow Stale Exchange Rate: This should be unchecked if you want ERPNext to check the age of records fetched from Currency Exchange in foreign currency transactions. If it is unchecked, the exchange rate field will be read-only in documents. + +* Stale Days: The number of days to use when deciding if a Currency Exchange record is stale. E.g If Currency Exchange records are to be updated every day, the Stale Days should be set as 1. + {next} diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 4b0c538f896..f961a5b81e9 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -439,8 +439,9 @@ erpnext.patches.v8_7.set_offline_in_pos_settings #11-09-17 erpnext.patches.v8_9.add_setup_progress_actions #08-09-2017 erpnext.patches.v8_9.rename_company_sales_target_field erpnext.patches.v8_8.set_bom_rate_as_per_uom -erpnext.patches.v9_0.remove_subscription_module erpnext.patches.v8_7.make_subscription_from_recurring_data +erpnext.patches.v8_8.add_new_fields_in_accounts_settings +erpnext.patches.v9_0.remove_subscription_module erpnext.patches.v8_9.set_print_zero_amount_taxes erpnext.patches.v8_9.set_default_customer_group erpnext.patches.v8_9.remove_employee_from_salary_structure_parent diff --git a/erpnext/patches/v8_8/add_new_fields_in_accounts_settings.py b/erpnext/patches/v8_8/add_new_fields_in_accounts_settings.py new file mode 100644 index 00000000000..bd25f15d789 --- /dev/null +++ b/erpnext/patches/v8_8/add_new_fields_in_accounts_settings.py @@ -0,0 +1,9 @@ +from __future__ import unicode_literals +import frappe + + +def execute(): + frappe.db.sql( + "INSERT INTO `tabSingles` (`doctype`, `field`, `value`) VALUES ('Accounts Settings', 'allow_stale', '1'), " + "('Accounts Settings', 'stale_days', '1')" + ) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index a5ef15e60bf..5f35ea44de8 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -519,6 +519,7 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ }, conversion_rate: function() { + const me = this.frm; if(this.frm.doc.currency === this.get_company_currency()) { this.frm.set_value("conversion_rate", 1.0); } @@ -536,6 +537,12 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ } } + // Make read only if Accounts Settings doesn't allow stale rates + frappe.model.get_value("Accounts Settings", null, "allow_stale", + function(d){ + me.set_df_property("conversion_rate", "read_only", cint(d.allow_stale) ? 0 : 1); + } + ); }, set_actual_charges_based_on_currency: function() { diff --git a/erpnext/setup/doctype/currency_exchange/test_currency_exchange.py b/erpnext/setup/doctype/currency_exchange/test_currency_exchange.py index a477379deda..6d5848ad78f 100644 --- a/erpnext/setup/doctype/currency_exchange/test_currency_exchange.py +++ b/erpnext/setup/doctype/currency_exchange/test_currency_exchange.py @@ -1,9 +1,9 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt from __future__ import unicode_literals - - import frappe, unittest +from erpnext.setup.utils import get_exchange_rate + test_records = frappe.get_test_records('Currency Exchange') @@ -28,11 +28,21 @@ def save_new_records(test_records): class TestCurrencyExchange(unittest.TestCase): - def test_exchnage_rate(self): - from erpnext.setup.utils import get_exchange_rate + def clear_cache(self): + cache = frappe.cache() + key = "currency_exchange_rate:{0}:{1}".format("USD", "INR") + cache.delete(key) + def tearDown(self): + frappe.db.set_value("Accounts Settings", None, "allow_stale", 1) + self.clear_cache() + + def test_exchange_rate(self): save_new_records(test_records) + frappe.db.set_value("Accounts Settings", None, "allow_stale", 1) + + # Start with allow_stale is True exchange_rate = get_exchange_rate("USD", "INR", "2016-01-01") self.assertEqual(exchange_rate, 60.0) @@ -43,6 +53,51 @@ class TestCurrencyExchange(unittest.TestCase): self.assertEqual(exchange_rate, 62.9) # Exchange rate as on 15th Dec, 2015, should be fetched from fixer.io + self.clear_cache() exchange_rate = get_exchange_rate("USD", "INR", "2015-12-15") self.assertFalse(exchange_rate == 60) - self.assertEqual(exchange_rate, 66.894) \ No newline at end of file + self.assertEqual(exchange_rate, 66.894) + + def test_exchange_rate_strict(self): + # strict currency settings + frappe.db.set_value("Accounts Settings", None, "allow_stale", 0) + frappe.db.set_value("Accounts Settings", None, "stale_days", 1) + + exchange_rate = get_exchange_rate("USD", "INR", "2016-01-01") + self.assertEqual(exchange_rate, 60.0) + + # Will fetch from fixer.io + self.clear_cache() + exchange_rate = get_exchange_rate("USD", "INR", "2016-01-15") + self.assertEqual(exchange_rate, 67.79) + + exchange_rate = get_exchange_rate("USD", "INR", "2016-01-30") + self.assertEqual(exchange_rate, 62.9) + + # Exchange rate as on 15th Dec, 2015, should be fetched from fixer.io + self.clear_cache() + exchange_rate = get_exchange_rate("USD", "INR", "2015-12-15") + self.assertEqual(exchange_rate, 66.894) + + exchange_rate = get_exchange_rate("INR", "NGN", "2016-01-10") + self.assertEqual(exchange_rate, 65.1) + + # NGN is not available on fixer.io so these should return 0 + exchange_rate = get_exchange_rate("INR", "NGN", "2016-01-09") + self.assertEqual(exchange_rate, 0) + + exchange_rate = get_exchange_rate("INR", "NGN", "2016-01-11") + self.assertEqual(exchange_rate, 0) + + def test_exchange_rate_strict_switched(self): + # Start with allow_stale is True + exchange_rate = get_exchange_rate("USD", "INR", "2016-01-15") + self.assertEqual(exchange_rate, 65.1) + + frappe.db.set_value("Accounts Settings", None, "allow_stale", 0) + frappe.db.set_value("Accounts Settings", None, "stale_days", 1) + + # Will fetch from fixer.io + self.clear_cache() + exchange_rate = get_exchange_rate("USD", "INR", "2016-01-15") + self.assertEqual(exchange_rate, 67.79) \ No newline at end of file diff --git a/erpnext/setup/doctype/currency_exchange/test_records.json b/erpnext/setup/doctype/currency_exchange/test_records.json index d2f658b443c..0c9cfbb67c0 100644 --- a/erpnext/setup/doctype/currency_exchange/test_records.json +++ b/erpnext/setup/doctype/currency_exchange/test_records.json @@ -33,5 +33,12 @@ "exchange_rate": 62.9, "from_currency": "USD", "to_currency": "INR" + }, + { + "doctype": "Currency Exchange", + "date": "2016-01-10", + "exchange_rate": 65.1, + "from_currency": "INR", + "to_currency": "NGN" } ] \ No newline at end of file diff --git a/erpnext/setup/utils.py b/erpnext/setup/utils.py index bdbf3f4ec2b..f003ce4b1c9 100644 --- a/erpnext/setup/utils.py +++ b/erpnext/setup/utils.py @@ -4,7 +4,7 @@ from __future__ import unicode_literals import frappe from frappe import _ -from frappe.utils import flt +from frappe.utils import flt, add_days from frappe.utils import get_datetime_str, nowdate def get_root_of(doctype): @@ -56,8 +56,6 @@ def before_tests(): @frappe.whitelist() def get_exchange_rate(from_currency, to_currency, transaction_date=None): - if not transaction_date: - transaction_date = nowdate() if not (from_currency and to_currency): # manqala 19/09/2016: Should this be an empty return or should it throw and exception? return @@ -65,13 +63,27 @@ def get_exchange_rate(from_currency, to_currency, transaction_date=None): 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 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=[ - ["date", "<=", get_datetime_str(transaction_date)], - ["from_currency", "=", from_currency], - ["to_currency", "=", to_currency] - ], order_by="date desc", limit=1) + 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)