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
diff --git a/erpnext/accounts/doctype/account/account.js b/erpnext/accounts/doctype/account/account.js
index 3c0eb85..bcf7efc 100644
--- a/erpnext/accounts/doctype/account/account.js
+++ b/erpnext/accounts/doctype/account/account.js
@@ -137,9 +137,6 @@
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) {
diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py
index c1eca72..02e6c20 100644
--- a/erpnext/accounts/doctype/account/account.py
+++ b/erpnext/accounts/doctype/account/account.py
@@ -18,6 +18,10 @@
pass
+class InvalidAccountMergeError(frappe.ValidationError):
+ pass
+
+
class Account(NestedSet):
nsm_parent_field = "parent_account"
@@ -460,25 +464,34 @@
@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) != (
- cint(is_group),
- root_type,
- company,
+ if (
+ cint(new_account.is_group),
+ 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(
- _(
- """Merging is only possible if following properties are same in both records. Is Group, Root Type, Company"""
- )
+ msg=_(
+ """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)
diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py
index 62303bd..30eebef 100644
--- a/erpnext/accounts/doctype/account/test_account.py
+++ b/erpnext/accounts/doctype/account/test_account.py
@@ -7,7 +7,11 @@
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 @@
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"):
- acc = frappe.new_doc("Account")
- acc.account_name = "Current Assets"
- acc.is_group = 1
- acc.parent_account = "Application of Funds (Assets) - _TC"
- acc.company = "_Test Company"
- acc.insert()
- if not frappe.db.exists("Account", "Securities and Deposits - _TC"):
- acc = frappe.new_doc("Account")
- acc.account_name = "Securities and Deposits"
- acc.parent_account = "Current Assets - _TC"
- acc.is_group = 1
- acc.company = "_Test Company"
- acc.insert()
- if not frappe.db.exists("Account", "Earnest Money - _TC"):
- acc = frappe.new_doc("Account")
- acc.account_name = "Earnest Money"
- acc.parent_account = "Securities and Deposits - _TC"
- acc.company = "_Test Company"
- acc.insert()
- if not frappe.db.exists("Account", "Cash In Hand - _TC"):
- acc = frappe.new_doc("Account")
- acc.account_name = "Cash In Hand"
- acc.is_group = 1
- acc.parent_account = "Current Assets - _TC"
- acc.company = "_Test Company"
- acc.insert()
- if not frappe.db.exists("Account", "Accumulated Depreciation - _TC"):
- acc = frappe.new_doc("Account")
- acc.account_name = "Accumulated Depreciation"
- acc.parent_account = "Fixed Assets - _TC"
- acc.company = "_Test Company"
- acc.account_type = "Accumulated Depreciation"
- acc.insert()
+ create_account(
+ account_name="Current Assets",
+ is_group=1,
+ parent_account="Application of Funds (Assets) - _TC",
+ company="_Test Company",
+ )
- doc = frappe.get_doc("Account", "Securities and Deposits - _TC")
+ create_account(
+ account_name="Securities and Deposits",
+ is_group=1,
+ parent_account="Current Assets - _TC",
+ company="_Test Company",
+ )
+
+ create_account(
+ account_name="Earnest Money",
+ parent_account="Securities and Deposits - _TC",
+ company="_Test Company",
+ )
+
+ create_account(
+ account_name="Cash In Hand",
+ is_group=1,
+ parent_account="Current Assets - _TC",
+ company="_Test Company",
+ )
+
+ create_account(
+ account_name="Receivable INR",
+ parent_account="Current Assets - _TC",
+ company="_Test Company",
+ account_currency="INR",
+ )
+
+ create_account(
+ account_name="Receivable USD",
+ parent_account="Current Assets - _TC",
+ company="_Test Company",
+ account_currency="USD",
+ )
+
parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
self.assertEqual(parent, "Securities and Deposits - _TC")
- merge_account(
- "Securities and Deposits - _TC", "Cash In Hand - _TC", doc.is_group, doc.root_type, doc.company
- )
+ merge_account("Securities and Deposits - _TC", "Cash In Hand - _TC")
+
parent = frappe.db.get_value("Account", "Earnest Money - _TC", "parent_account")
# Parent account of the child account changes after merging
@@ -98,30 +106,28 @@
# 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 @@
"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"),
diff --git a/erpnext/accounts/doctype/ledger_merge/ledger_merge.py b/erpnext/accounts/doctype/ledger_merge/ledger_merge.py
index 381083b..362d273 100644
--- a/erpnext/accounts/doctype/ledger_merge/ledger_merge.py
+++ b/erpnext/accounts/doctype/ledger_merge/ledger_merge.py
@@ -48,9 +48,6 @@
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()