Payroll Entry - Sometimes uses the wrong exchange rate due to a race condition on ajax call

[jonathonsim]

Information about bug

You have multiple companies with different currencies, and you load the payroll entry page : very intermittently, due to a race condition (described below) it will select an incorrect exchange rate.

When you load the page it will load with the last company you ran payroll for. In our case, we have companies with currency of 'NZD" (lets call it 'NZD Company' and another with currency 'PHP' (lets call it 'PHP Company') for which we are running the payroll entry.

When you load the page, it will load the last company used eg 'PHP Company', but the currency will initially default to 'NZD'. This will then note that the currency ('NZD') is different from the default for that company ('PHP') and load the exchange rate for that currency via an ajax call. It will also fire a javascript hook setup on 'company' that sets the currency to the currency for the company ('PHP').

Depending on the timing of the asynchronous calls happening here, sometimes the ajax call to 'api/method/erpnext.setup.utils.get_exchange_rate' will return after the currency has been updated back to the correct 'php' value, thereby giving you an exchange rate conversion of 'PHP' to 'PHP' but with an exchange rate of 35 (rather than 1). When the payroll entry creates journal entries they will all end out incorrectly being multiplied by 35.

This happens in hrms/payroll/doctype/payroll_entry/payroll_entry.js

hrms/hrms/payroll/doctype/payroll_entry/payroll_entry.js

Lines 274 to 305 in f64da73

	currency: function (frm) {
	var company_currency;
	if (!frm.doc.company) {
	company_currency = erpnext.get_currency(frappe.defaults.get_default("Company"));
	} else {
	company_currency = erpnext.get_currency(frm.doc.company);
	}
	if (frm.doc.currency) {
	if (company_currency != frm.doc.currency) {
	frappe.call({
	method: "erpnext.setup.utils.get_exchange_rate",
	args: {
	from_currency: frm.doc.currency,
	to_currency: company_currency,
	},
	callback: function (r) {
	frm.set_value("exchange_rate", flt(r.message));
	frm.set_df_property("exchange_rate", "hidden", 0);
	frm.set_df_property(
	"exchange_rate",
	"description",
	"1 " + frm.doc.currency + " = [?] " + company_currency,
	);
	},
	});
	} else {
	frm.set_value("exchange_rate", 1.0);
	frm.set_df_property("exchange_rate", "hidden", 1);
	frm.set_df_property("exchange_rate", "description", "");
	}
	}
	},

Note that is hard to replicate this, since it is based on flukes of timing, network conditions etc, but one way to force it to happen is to edit https://github.com/frappe/erpnext/blob/5a1a37e915da542d44f1551f1e7b79db4104f1c8/erpnext/setup/utils.py#L50 and add time.sleep(1) to make that call take longer to return.

One fix would be to check the currencies still differed in the callback of the ajax request and ignore it if they've changed

                                check_currency = frm.doc.currency
				frappe.call({
					method: "erpnext.setup.utils.get_exchange_rate",
					args: {
						from_currency: frm.doc.currency,
						to_currency: company_currency,
					},
					callback: function (r) {
                                           if (cur_frm.doc.currency == check_currency) {
						frm.set_value("exchange_rate", flt(r.message));
						frm.set_df_property("exchange_rate", "hidden", 0);
						frm.set_df_property(
							"exchange_rate",
							"description",
							"1 " + frm.doc.currency + " = [?] " + company_currency,
						);
                                            }
					},
				});

Module

Payroll

Version

erpnext 15.31.5
frappe 15.36.1
hrms 15.25.2

Installation method

manual install

Relevant log output / Stack trace / Full Error Message.

Code of Conduct

• I agree to follow this project's Code of Conduct