diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index 8431d5f1184..4da21e3942b 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -888,25 +888,26 @@ class TestAsset(AssetSetup): }, ) asset_category.insert() - asset = create_asset(asset_category=asset_category_name, calculate_depreciation=1, do_not_save=1) - with self.assertRaises(frappe.ValidationError) as err: - asset.save() + try: + asset = create_asset(asset_category=asset_category_name, calculate_depreciation=1, do_not_save=1) + with self.assertRaises(frappe.ValidationError) as err: + asset.save() - self.assertTrue( - "Please set Depreciation related Accounts in Asset Category Computers or Company" - in str(err.exception) - ) - - frappe.db.set_value("Company", "_Test Company", company_depreciation_accounts) - if asset_category_account: - frappe.db.set_value( - "Asset Category Account", - asset_category_account.name, - { - "accumulated_depreciation_account": asset_category_account.accumulated_depreciation_account, - "depreciation_expense_account": asset_category_account.depreciation_expense_account, - }, + self.assertTrue( + "Please set Depreciation related Accounts in Asset Category Computers or Company" + in str(err.exception) ) + finally: + frappe.db.set_value("Company", "_Test Company", company_depreciation_accounts) + if asset_category_account: + frappe.db.set_value( + "Asset Category Account", + asset_category_account.name, + { + "accumulated_depreciation_account": asset_category_account.accumulated_depreciation_account, + "depreciation_expense_account": asset_category_account.depreciation_expense_account, + }, + ) class TestDepreciationMethods(AssetSetup): diff --git a/erpnext/assets/doctype/asset_category/asset_category.py b/erpnext/assets/doctype/asset_category/asset_category.py index 2edf71099af..7c0e0a50dad 100644 --- a/erpnext/assets/doctype/asset_category/asset_category.py +++ b/erpnext/assets/doctype/asset_category/asset_category.py @@ -130,33 +130,64 @@ class AssetCategory(Document): "accumulated_depreciation_account": "Accumulated Depreciation Account", "depreciation_expense_account": "Depreciation Expense Account", } - missing_depreciation_account_msg = [] - for row in self.accounts: - if not has_depreciable_asset(self.name, row.company_name): - continue + + error_msg = [] + companies_with_accounts = set() + + def validate_company_accounts(company, acc_row=None): default_accounts = frappe.get_cached_value( "Company", - row.company_name, + company, ["accumulated_depreciation_account", "depreciation_expense_account"], as_dict=True, ) for fieldname, label in depreciation_account_map.items(): - if not row.get(fieldname) and not default_accounts.get(fieldname): - missing_depreciation_account_msg.append( - _("Row #{0}: Missing {1} for company {2}.").format( - row.idx, - label, - get_link_to_form("Company", row.company_name), + row_value = acc_row.get(fieldname) if acc_row else None + if not row_value and not default_accounts.get(fieldname): + if acc_row: + error_msg.append( + _("Row #{0}: Missing {1} for company {2}.").format( + acc_row.idx, + label, + get_link_to_form("Company", company), + ) ) - ) - if missing_depreciation_account_msg: + else: + msg = _("Missing account configuration for company {0}.").format( + get_link_to_form("Company", company), + ) + if msg not in error_msg: + error_msg.append(msg) + + companies_with_assets = frappe.db.get_all( + "Asset", + { + "calculate_depreciation": 1, + "asset_category": self.name, + "status": ["in", ("Submitted", "Partially Depreciated")], + }, + pluck="company", + distinct=True, + ) + + for acc_row in self.accounts: + companies_with_accounts.add(acc_row.company_name) + if acc_row.company_name in companies_with_assets: + validate_company_accounts(acc_row.company_name, acc_row) + + for company in companies_with_assets: + if company not in companies_with_accounts: + validate_company_accounts(company) + + if error_msg: msg = _( "Since there are active depreciable assets under this category, the following accounts are required.

" ) msg += _( "You can either configure default depreciation accounts in the Company or set the required accounts in the following rows:

" ) - msg += "
".join(missing_depreciation_account_msg) + msg += "
".join(error_msg) + frappe.throw(msg, title=_("Missing Accounts")) @@ -182,17 +213,3 @@ def get_asset_category_account( ) return account - - -def has_depreciable_asset(asset_category, company): - return bool( - frappe.db.count( - "Asset", - { - "calculate_depreciation": 1, - "asset_category": asset_category, - "company": company, - "status": ["in", ("Submitted", "Partially Depreciated")], - }, - ) - ) diff --git a/erpnext/assets/doctype/asset_category/test_asset_category.py b/erpnext/assets/doctype/asset_category/test_asset_category.py index ae7aafe8716..995185fbf95 100644 --- a/erpnext/assets/doctype/asset_category/test_asset_category.py +++ b/erpnext/assets/doctype/asset_category/test_asset_category.py @@ -98,19 +98,21 @@ class TestAssetCategory(IntegrationTestCase): "depreciation_expense_account": "", }, ) - asset_category = frappe.get_doc("Asset Category", asset.asset_category) - asset_category.enable_cwip_accounting = 0 - for row in asset_category.accounts: - if row.company_name == asset.company and ( - row.accumulated_depreciation_account or row.depreciation_expense_account - ): - row.accumulated_depreciation_account = None - row.depreciation_expense_account = None - with self.assertRaises(frappe.ValidationError) as err: - asset_category.save() + try: + asset_category = frappe.get_doc("Asset Category", asset.asset_category) + asset_category.enable_cwip_accounting = 0 + for row in asset_category.accounts: + if row.company_name == asset.company and ( + row.accumulated_depreciation_account or row.depreciation_expense_account + ): + row.accumulated_depreciation_account = None + row.depreciation_expense_account = None + with self.assertRaises(frappe.ValidationError) as err: + asset_category.save() - self.assertTrue( - "Since there are active depreciable assets under this category, the following accounts are required." - in str(err.exception) - ) - frappe.db.set_value("Company", asset.company, company_acccount_depreciation) + self.assertTrue( + "Since there are active depreciable assets under this category, the following accounts are required." + in str(err.exception) + ) + finally: + frappe.db.set_value("Company", asset.company, company_acccount_depreciation)