fix: Don't allow merging accounts with different currency (#37074)

* fix: Don't allow merging accounts with different currency

* test: Update conflicting values

* test: Update conflicting values

erpnext/accounts/doctype/account/account.js

@@ -137,9 +137,6 @@ frappe.ui.form.on("Account", {
 					args: {
 						old: frm.doc.name,
 						new: data.name,
-						is_group: frm.doc.is_group,
-						root_type: frm.doc.root_type,
-						company: frm.doc.company,
 					},
 					callback: function (r) {
 						if (!r.exc) {

erpnext/accounts/doctype/account/account.py

@@ -18,6 +18,10 @@ class BalanceMismatchError(frappe.ValidationError):
 	pass
 
 
+class InvalidAccountMergeError(frappe.ValidationError):
+	pass
+
+
 class Account(NestedSet):
 	nsm_parent_field = "parent_account"
 
@@ -460,25 +464,34 @@ def update_account_number(name, account_name, account_number=None, from_descenda
 
 
 @frappe.whitelist()
-def merge_account(old, new, is_group, root_type, company):
+def merge_account(old, new):
 	# Validate properties before merging
 	new_account = frappe.get_cached_doc("Account", new)
+	old_account = frappe.get_cached_doc("Account", old)
 
 	if not new_account:
 		throw(_("Account {0} does not exist").format(new))
 
-	if (new_account.is_group, new_account.root_type, new_account.company) != (
+	if (
-		cint(is_group),
+		cint(new_account.is_group),
-		root_type,
+		new_account.root_type,
-		company,
+		new_account.company,
+		cstr(new_account.account_currency),
+	) != (
+		cint(old_account.is_group),
+		old_account.root_type,
+		old_account.company,
+		cstr(old_account.account_currency),
 	):
 		throw(
-			_(
+			msg=_(
-				"""Merging is only possible if following properties are same in both records. Is Group, Root Type, Company"""
+				"""Merging is only possible if following properties are same in both records. Is Group, Root Type, Company and Account Currency"""
-			)
+			),
+			title=("Invalid Accounts"),
+			exc=InvalidAccountMergeError,
 		)
 
-	if is_group and new_account.parent_account == old:
+	if old_account.is_group and new_account.parent_account == old:
 		new_account.db_set("parent_account", frappe.get_cached_value("Account", old, "parent_account"))
 
 	frappe.rename_doc("Account", old, new, merge=1, force=1)

erpnext/accounts/doctype/account/test_account.py

@@ -7,7 +7,11 @@ import unittest
 import frappe
 from frappe.test_runner import make_test_records
 
-from erpnext.accounts.doctype.account.account import merge_account, update_account_number
+from erpnext.accounts.doctype.account.account import (
+	InvalidAccountMergeError,
+	merge_account,
+	update_account_number,
+)
 from erpnext.stock import get_company_default_inventory_account, get_warehouse_account
 
 test_dependencies = ["Company"]
@@ -47,49 +51,53 @@ class TestAccount(unittest.TestCase):
 		frappe.delete_doc("Account", "1211-11-4 - 6 - Debtors 1 - Test - - _TC")
 
 	def test_merge_account(self):
-		if not frappe.db.exists("Account", "Current Assets - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Current Assets",
-			acc.account_name = "Current Assets"
+			is_group=1,
-			acc.is_group = 1
+			parent_account="Application of Funds (Assets) - _TC",
-			acc.parent_account = "Application of Funds (Assets) - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Securities and Deposits - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Securities and Deposits",
-			acc.account_name = "Securities and Deposits"
+			is_group=1,
-			acc.parent_account = "Current Assets - _TC"
+			parent_account="Current Assets - _TC",
-			acc.is_group = 1
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Earnest Money - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Earnest Money",
-			acc.account_name = "Earnest Money"
+			parent_account="Securities and Deposits - _TC",
-			acc.parent_account = "Securities and Deposits - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Cash In Hand - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Cash In Hand",
-			acc.account_name = "Cash In Hand"
+			is_group=1,
-			acc.is_group = 1
+			parent_account="Current Assets - _TC",
-			acc.parent_account = "Current Assets - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+		)
-			acc.insert()
+
-		if not frappe.db.exists("Account", "Accumulated Depreciation - _TC"):
+		create_account(
-			acc = frappe.new_doc("Account")
+			account_name="Receivable INR",
-			acc.account_name = "Accumulated Depreciation"
+			parent_account="Current Assets - _TC",
-			acc.parent_account = "Fixed Assets - _TC"
+			company="_Test Company",
-			acc.company = "_Test Company"
+			account_currency="INR",
-			acc.account_type = "Accumulated Depreciation"
+		)
-			acc.insert()
+
+		create_account(
+			account_name="Receivable USD",
+			parent_account="Current Assets - _TC",
+			company="_Test Company",
+			account_currency="USD",
+		)
 
-		doc = frappe.get_doc("Account", "Securities and Deposits - _TC")
 		parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
 
 		self.assertEqual(parent, "Securities and Deposits - _TC")
 
-		merge_account(
+		merge_account("Securities and Deposits - _TC", "Cash In Hand - _TC")
-			"Securities and Deposits - _TC", "Cash In Hand - _TC", doc.is_group, doc.root_type, doc.company
+
-		)
 		parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
 
 		# Parent account of the child account changes after merging
@@ -98,30 +106,28 @@ class TestAccount(unittest.TestCase):
 		# Old account doesn't exist after merging
 		self.assertFalse(frappe.db.exists("Account", "Securities and Deposits - _TC"))
 
-		doc = frappe.get_doc("Account", "Current Assets - _TC")
-
 		# Raise error as is_group property doesn't match
 		self.assertRaises(
-			frappe.ValidationError,
+			InvalidAccountMergeError,
 			merge_account,
 			"Current Assets - _TC",
 			"Accumulated Depreciation - _TC",
-			doc.is_group,
-			doc.root_type,
-			doc.company,
 		)
 
-		doc = frappe.get_doc("Account", "Capital Stock - _TC")
-
 		# Raise error as root_type property doesn't match
 		self.assertRaises(
-			frappe.ValidationError,
+			InvalidAccountMergeError,
 			merge_account,
 			"Capital Stock - _TC",
 			"Softwares - _TC",
-			doc.is_group,
+		)
-			doc.root_type,
+
-			doc.company,
+		# Raise error as currency doesn't match
+		self.assertRaises(
+			InvalidAccountMergeError,
+			merge_account,
+			"Receivable INR - _TC",
+			"Receivable USD - _TC",
 		)
 
 	def test_account_sync(self):
@@ -400,11 +406,20 @@ def create_account(**kwargs):
 		"Account", filters={"account_name": kwargs.get("account_name"), "company": kwargs.get("company")}
 	)
 	if account:
-		return account
+		account = frappe.get_doc("Account", account)
+		account.update(
+			dict(
+				is_group=kwargs.get("is_group", 0),
+				parent_account=kwargs.get("parent_account"),
+			)
+		)
+		account.save()
+		return account.name
 	else:
 		account = frappe.get_doc(
 			dict(
 				doctype="Account",
+				is_group=kwargs.get("is_group", 0),
 				account_name=kwargs.get("account_name"),
 				account_type=kwargs.get("account_type"),
 				parent_account=kwargs.get("parent_account"),

erpnext/accounts/doctype/ledger_merge/ledger_merge.py

@@ -48,9 +48,6 @@ def start_merge(docname):
 				merge_account(
 					row.account,
 					ledger_merge.account,
-					ledger_merge.is_group,
-					ledger_merge.root_type,
-					ledger_merge.company,
 				)
 				row.db_set("merged", 1)
 				frappe.db.commit()