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

* 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

(cherry picked from commit 5e21e7cd1d)

# Conflicts:
#	erpnext/accounts/doctype/account/account.js
#	erpnext/accounts/doctype/account/account.py

* chore: resolve conflicts

---------

Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com>

erpnext/accounts/doctype/account/account.js

@@ -117,9 +117,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"
 
@@ -444,24 +448,35 @@ 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
-	if not frappe.db.exists("Account", new):
+	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))
 
-	val = list(frappe.db.get_value("Account", new, ["is_group", "root_type", "company"]))
+	if (
-
+		cint(new_account.is_group),
-	if val != [cint(is_group), root_type, company]:
+		new_account.root_type,
+		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 frappe.db.get_value("Account", new, "parent_account") == old:
+	if old_account.is_group and new_account.parent_account == old:
-		frappe.db.set_value(
+		new_account.db_set("parent_account", frappe.get_cached_value("Account", old, "parent_account"))
-			"Account", new, "parent_account", frappe.db.get_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

@@ -49,9 +49,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()