From fdcda85f8759a169f6e1fcb0e0e9d4913b61a99b Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 31 Aug 2020 16:10:52 +0530 Subject: [PATCH 1/5] fix: Validate Missing Accounts in Child Companies - If parent account exists in Parent and now child, throw error --- erpnext/accounts/doctype/account/account.py | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index 164f120067..b383ba4e30 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -117,7 +117,29 @@ 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 + + if not parent_acc_name_map: + # map can be empty if only one descendant or all descendants without parent account exist(s) + # or if no descendants exist + if descendants: + frappe.throw(_("Parent Account {0} does not exist in any Child Company").format(frappe.bold(parent_acc_name)), + title=_("Account Missing")) + else: + # no descendants and empty map, nothing to sync + return + + companies_missing_account = [] + for company in descendants: + if not company in parent_acc_name_map: + companies_missing_account.append(company) + + # If atleast any one of the descendants does not have the parent account, block transaction + if companies_missing_account: + message = _("Parent Account {0} does not exist in the following companies:").format(frappe.bold(parent_acc_name)) + message += "

" + message += _("Please make sure the account exists in the child companies as well") + frappe.throw(message, title=_("Account Missing")) + self.create_account_for_child_company(parent_acc_name_map, descendants, parent_acc_name) def validate_group_or_ledger(self): From 8938d1040db8602ff54ff14745804629fa656f5f Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 31 Aug 2020 18:18:10 +0530 Subject: [PATCH 2/5] fix: Rename/Update Child company Accounts as well --- erpnext/accounts/doctype/account/account.py | 24 +++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index b383ba4e30..0a888935ba 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -311,10 +311,13 @@ 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"]) + validate_account_number(name, account_number, account.company) if account_number: frappe.db.set_value("Account", name, "account_number", account_number.strip()) @@ -322,6 +325,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) @@ -352,3 +361,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) From 95333188323b4941ba72f46b49406991a92bf1b4 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 11 Sep 2020 16:09:27 +0530 Subject: [PATCH 3/5] chore: Tests for Rename and Add child Account --- .../accounts/doctype/account/test_account.py | 55 ++++++++++++++++++- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index 89bb0184af..3c2fb942ac 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -6,7 +6,7 @@ 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 merge_account +from erpnext.accounts.doctype.account.account import merge_account, update_account_number class TestAccount(unittest.TestCase): def test_rename_account(self): @@ -99,7 +99,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 +112,56 @@ 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 renmamed 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_account_sync_with_missing_parent_account_in_child_company(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"})) + + acc_tc_5 = frappe.db.get_value('Account', {'account_name': "Test Group Account", "company": "_Test Company 5"}) + # Rename group account in one child company + update_account_number(acc_tc_5, "Test Modified Account") + + # Add child account to test group account in parent company + # which will try to do the same in child company + acc = frappe.new_doc("Account") + acc.account_name = "Test Child Account" + acc.parent_account = "Test Group Account - _TC3" + acc.company = "_Test Company 3" + + self.assertRaises(frappe.ValidationError, acc.insert) + + to_delete = ["Test Group Account - _TC3", "Test Group Account - _TC5", "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 From 02c550c07df84fbb9ea65b207e4df9760cc9876d Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 11 Sep 2020 16:22:54 +0530 Subject: [PATCH 4/5] fix: Redundant imports --- erpnext/accounts/doctype/account/test_account.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index 3c2fb942ac..6819081648 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/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 merge_account, update_account_number +from erpnext.accounts.doctype.account.account import update_account_number, merge_account class TestAccount(unittest.TestCase): def test_rename_account(self): From fd155408e5455341656268c74cfce72a5da5b5ec Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Sep 2020 14:42:10 +0530 Subject: [PATCH 5/5] fix: Check if account exists in parent company on rename - Check if child company is dependent on parent company - Check if account-to-be-renamed exists in parent, if yes, block - Revised Test basd on the same --- erpnext/accounts/doctype/account/account.py | 39 +++++++++---------- .../accounts/doctype/account/test_account.py | 23 ++++++----- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index 0a888935ba..2c151443d8 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -118,27 +118,7 @@ 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: - # map can be empty if only one descendant or all descendants without parent account exist(s) - # or if no descendants exist - if descendants: - frappe.throw(_("Parent Account {0} does not exist in any Child Company").format(frappe.bold(parent_acc_name)), - title=_("Account Missing")) - else: - # no descendants and empty map, nothing to sync - return - - companies_missing_account = [] - for company in descendants: - if not company in parent_acc_name_map: - companies_missing_account.append(company) - - # If atleast any one of the descendants does not have the parent account, block transaction - if companies_missing_account: - message = _("Parent Account {0} does not exist in the following companies:").format(frappe.bold(parent_acc_name)) - message += "

  • " + "
  • ".join(companies_missing_account) + "
" - message += _("Please make sure the account exists in the child companies as well") - frappe.throw(message, title=_("Account Missing")) + if not parent_acc_name_map: return self.create_account_for_child_company(parent_acc_name_map, descendants, parent_acc_name) @@ -318,6 +298,23 @@ def update_account_number(name, account_name, account_number=None, from_descenda 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 += "
" + _("Renaming it is only allowed via parent company {0}, \ + to avoid mismatch.").format(frappe.bold(ancestor)) + "

" + 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()) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index 6819081648..b6a950b1b5 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -123,7 +123,7 @@ class TestAccount(unittest.TestCase): # Rename account in parent company update_account_number(acc.name, "Test Rename Sync Account", "1234") - # Check if renmamed in children + # 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"})) @@ -131,7 +131,7 @@ class TestAccount(unittest.TestCase): frappe.delete_doc("Account", "1234 - Test Rename Sync Account - _TC4") frappe.delete_doc("Account", "1234 - Test Rename Sync Account - _TC5") - def test_account_sync_with_missing_parent_account_in_child_company(self): + def test_child_company_account_rename_sync(self): frappe.local.flags.pop("ignore_root_company_validation", None) acc = frappe.new_doc("Account") @@ -144,20 +144,19 @@ class TestAccount(unittest.TestCase): 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"}) - # Rename group account in one child company + 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"})) - # Add child account to test group account in parent company - # which will try to do the same in child company - acc = frappe.new_doc("Account") - acc.account_name = "Test Child Account" - acc.parent_account = "Test Group Account - _TC3" - acc.company = "_Test Company 3" + frappe.db.set_value("Company", "_Test Company 5", "allow_account_creation_against_child_company", 0) - self.assertRaises(frappe.ValidationError, acc.insert) - - to_delete = ["Test Group Account - _TC3", "Test Group Account - _TC5", "Test Modified Account - _TC5"] + to_delete = ["Test Group Account - _TC3", "Test Group Account - _TC4", "Test Modified Account - _TC5"] for doc in to_delete: frappe.delete_doc("Account", doc)