fix: validate depreciation accounts exist for all companies with depreciable assets

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)
+		try:
-		with self.assertRaises(frappe.ValidationError) as err:
+			asset = create_asset(asset_category=asset_category_name, calculate_depreciation=1, do_not_save=1)
-			asset.save()
+			with self.assertRaises(frappe.ValidationError) as err:
+				asset.save()
 
-		self.assertTrue(
+			self.assertTrue(
-			"Please set Depreciation related Accounts in Asset Category Computers or Company"
+				"Please set Depreciation related Accounts in Asset Category Computers or Company"
-			in str(err.exception)
+				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,
-				},
 			)
+		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):

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:
+		error_msg = []
-			if not has_depreciable_asset(self.name, row.company_name):
+		companies_with_accounts = set()
-				continue
+
+		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):
+				row_value = acc_row.get(fieldname) if acc_row else None
-					missing_depreciation_account_msg.append(
+				if not row_value and not default_accounts.get(fieldname):
-						_("Row #{0}: Missing <b>{1}</b> for company <b>{2}</b>.").format(
+					if acc_row:
-							row.idx,
+						error_msg.append(
-							label,
+							_("Row #{0}: Missing <b>{1}</b> for company <b>{2}</b>.").format(
-							get_link_to_form("Company", row.company_name),
+								acc_row.idx,
+								label,
+								get_link_to_form("Company", company),
+							)
 						)
-					)
+					else:
-		if missing_depreciation_account_msg:
+						msg = _("Missing account configuration for company <b>{0}</b>.").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. <br><br>"
 			)
 			msg += _(
 				"You can either configure default depreciation accounts in the Company or set the required accounts in the following rows: <br><br>"
 			)
-			msg += "<br>".join(missing_depreciation_account_msg)
+			msg += "<br>".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")],
-			},
-		)
-	)

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)
+		try:
-		asset_category.enable_cwip_accounting = 0
+			asset_category = frappe.get_doc("Asset Category", asset.asset_category)
-		for row in asset_category.accounts:
+			asset_category.enable_cwip_accounting = 0
-			if row.company_name == asset.company and (
+			for row in asset_category.accounts:
-				row.accumulated_depreciation_account or row.depreciation_expense_account
+				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
+					row.accumulated_depreciation_account = None
-		with self.assertRaises(frappe.ValidationError) as err:
+					row.depreciation_expense_account = None
-			asset_category.save()
+			with self.assertRaises(frappe.ValidationError) as err:
+				asset_category.save()
 
-		self.assertTrue(
+			self.assertTrue(
-			"Since there are active depreciable assets under this category, the following accounts are required."
+				"Since there are active depreciable assets under this category, the following accounts are required."
-			in str(err.exception)
+				in str(err.exception)
-		)
+			)
-		frappe.db.set_value("Company", asset.company, company_acccount_depreciation)
+		finally:
+			frappe.db.set_value("Company", asset.company, company_acccount_depreciation)