Merge pull request #23220 from marination/account-add-rename

feat: Validate and Update Child Company Accounts on Rename/Adding Account

erpnext/accounts/doctype/account/account.py

@@ -117,7 +117,9 @@ class Account(NestedSet):
 
 			for d in frappe.db.get_values('Account', filters=filters, fieldname=["company", "name"], as_dict=True):
 				parent_acc_name_map[d["company"]] = d["name"]
+
 			if not parent_acc_name_map: return
+
 			self.create_account_for_child_company(parent_acc_name_map, descendants, parent_acc_name)
 
 	def validate_group_or_ledger(self):
@@ -289,10 +291,30 @@ def validate_account_number(name, account_number, company):
 				.format(account_number, account_with_same_number))
 
 @frappe.whitelist()
-def update_account_number(name, account_name, account_number=None):
+def update_account_number(name, account_name, account_number=None, from_descendant=False):
-
 	account = frappe.db.get_value("Account", name, "company", as_dict=True)
 	if not account: return
+
+	old_acc_name, old_acc_number = frappe.db.get_value('Account', name, \
+				["account_name", "account_number"])
+
+	# check if account exists in parent company
+	ancestors = get_ancestors_of("Company", account.company)
+	allow_independent_account_creation = frappe.get_value("Company", account.company, "allow_account_creation_against_child_company")
+
+	if ancestors and not allow_independent_account_creation:
+		for ancestor in ancestors:
+			if frappe.db.get_value("Account", {'account_name': old_acc_name, 'company': ancestor}, 'name'):
+				# same account in parent company exists
+				allow_child_account_creation = _("Allow Account Creation Against Child Company")
+
+				message = _("Account {0} exists in parent company {1}.").format(frappe.bold(old_acc_name), frappe.bold(ancestor))
+				message += "<br>" + _("Renaming it is only allowed via parent company {0}, \
+					to avoid mismatch.").format(frappe.bold(ancestor)) + "<br><br>"
+				message += _("To overrule this, enable '{0}' in company {1}").format(allow_child_account_creation, frappe.bold(account.company))
+
+				frappe.throw(message, title=_("Rename Not Allowed"))
+
 	validate_account_number(name, account_number, account.company)
 	if account_number:
 		frappe.db.set_value("Account", name, "account_number", account_number.strip())
@@ -300,6 +322,12 @@ def update_account_number(name, account_name, account_number=None):
 		frappe.db.set_value("Account", name, "account_number", "")
 	frappe.db.set_value("Account", name, "account_name", account_name.strip())
 
+	if not from_descendant:
+		# Update and rename in child company accounts as well
+		descendants = get_descendants_of('Company', account.company)
+		if descendants:
+			sync_update_account_number_in_child(descendants, old_acc_name, account_name, account_number, old_acc_number)
+
 	new_name = get_account_autoname(account_number, account_name, account.company)
 	if name != new_name:
 		frappe.rename_doc("Account", name, new_name, force=1)
@@ -330,3 +358,14 @@ def get_root_company(company):
 	# return the topmost company in the hierarchy
 	ancestors = get_ancestors_of('Company', company, "lft asc")
 	return [ancestors[0]] if ancestors else []
+
+def sync_update_account_number_in_child(descendants, old_acc_name, account_name, account_number=None, old_acc_number=None):
+	filters = {
+		"company": ["in", descendants],
+		"account_name": old_acc_name,
+	}
+	if old_acc_number:
+		filters["account_number"] = old_acc_number
+
+	for d in frappe.db.get_values('Account', filters=filters, fieldname=["company", "name"], as_dict=True):
+			update_account_number(d["name"], account_name, account_number, from_descendant=True)

erpnext/accounts/doctype/account/test_account.py

@@ -5,8 +5,7 @@ from __future__ import unicode_literals
 import unittest
 import frappe
 from erpnext.stock import get_warehouse_account, get_company_default_inventory_account
-from erpnext.accounts.doctype.account.account import update_account_number
+from erpnext.accounts.doctype.account.account import update_account_number, merge_account
-from erpnext.accounts.doctype.account.account import merge_account
 
 class TestAccount(unittest.TestCase):
 	def test_rename_account(self):
@@ -99,7 +98,8 @@ class TestAccount(unittest.TestCase):
 			"Softwares - _TC", doc.is_group, doc.root_type, doc.company)
 
 	def test_account_sync(self):
-		del frappe.local.flags["ignore_root_company_validation"]
+		frappe.local.flags.pop("ignore_root_company_validation", None)
+
 		acc = frappe.new_doc("Account")
 		acc.account_name = "Test Sync Account"
 		acc.parent_account = "Temporary Accounts - _TC3"
@@ -111,6 +111,55 @@ class TestAccount(unittest.TestCase):
 		self.assertEqual(acc_tc_4, "Test Sync Account - _TC4")
 		self.assertEqual(acc_tc_5, "Test Sync Account - _TC5")
 
+	def test_account_rename_sync(self):
+		frappe.local.flags.pop("ignore_root_company_validation", None)
+
+		acc = frappe.new_doc("Account")
+		acc.account_name = "Test Rename Account"
+		acc.parent_account = "Temporary Accounts - _TC3"
+		acc.company = "_Test Company 3"
+		acc.insert()
+
+		# Rename account in parent company
+		update_account_number(acc.name, "Test Rename Sync Account", "1234")
+
+		# Check if renamed in children
+		self.assertTrue(frappe.db.exists("Account", {'account_name': "Test Rename Sync Account", "company": "_Test Company 4", "account_number": "1234"}))
+		self.assertTrue(frappe.db.exists("Account", {'account_name': "Test Rename Sync Account", "company": "_Test Company 5", "account_number": "1234"}))
+
+		frappe.delete_doc("Account", "1234 - Test Rename Sync Account - _TC3")
+		frappe.delete_doc("Account", "1234 - Test Rename Sync Account - _TC4")
+		frappe.delete_doc("Account", "1234 - Test Rename Sync Account - _TC5")
+
+	def test_child_company_account_rename_sync(self):
+		frappe.local.flags.pop("ignore_root_company_validation", None)
+
+		acc = frappe.new_doc("Account")
+		acc.account_name = "Test Group Account"
+		acc.parent_account = "Temporary Accounts - _TC3"
+		acc.is_group = 1
+		acc.company = "_Test Company 3"
+		acc.insert()
+
+		self.assertTrue(frappe.db.exists("Account", {'account_name': "Test Group Account", "company": "_Test Company 4"}))
+		self.assertTrue(frappe.db.exists("Account", {'account_name': "Test Group Account", "company": "_Test Company 5"}))
+
+		# Try renaming child company account
+		acc_tc_5 = frappe.db.get_value('Account', {'account_name': "Test Group Account", "company": "_Test Company 5"})
+		self.assertRaises(frappe.ValidationError, update_account_number, acc_tc_5, "Test Modified Account")
+
+		# Rename child company account with allow_account_creation_against_child_company enabled
+		frappe.db.set_value("Company", "_Test Company 5", "allow_account_creation_against_child_company", 1)
+
+		update_account_number(acc_tc_5, "Test Modified Account")
+		self.assertTrue(frappe.db.exists("Account", {'name': "Test Modified Account - _TC5", "company": "_Test Company 5"}))
+
+		frappe.db.set_value("Company", "_Test Company 5", "allow_account_creation_against_child_company", 0)
+
+		to_delete = ["Test Group Account - _TC3", "Test Group Account - _TC4", "Test Modified Account - _TC5"]
+		for doc in to_delete:
+			frappe.delete_doc("Account", doc)
+
 def _make_test_records(verbose):
 	from frappe.test_runner import make_test_objects