Merge pull request #58039 from mihir-kandoi/backport-58036-version-15-hotfix

fix(stock): validate warehouse accounts when used (backport #58036)

erpnext/setup/doctype/company/company.py

@@ -346,6 +346,7 @@ class Company(NestedSet):
 			)
 			warehouse.flags.ignore_permissions = True
 			warehouse.flags.ignore_mandatory = True
+			warehouse.flags.ignore_inventory_account_validation = True
 			warehouse.insert()
 
 			if wh_detail["is_group"]:

erpnext/stock/__init__.py

@@ -16,6 +16,17 @@ install_docs = [
 ]
 
 
+class WarehouseAccountMap(frappe._dict):
+	def __missing__(self, warehouse):
+		account = get_warehouse_account(frappe.get_cached_doc("Warehouse", warehouse))
+		account_details = frappe._dict(
+			account=account,
+			account_currency=frappe.get_cached_value("Account", account, "account_currency"),
+		)
+		self[warehouse] = account_details
+		return account_details
+
+
 def get_warehouse_account_map(company=None):
 	company_warehouse_account_map = company and frappe.flags.setdefault("warehouse_account_map", {}).get(
 		company
@@ -23,7 +34,7 @@ def get_warehouse_account_map(company=None):
 	warehouse_account_map = frappe.flags.warehouse_account_map
 
 	if not warehouse_account_map or not company_warehouse_account_map or frappe.flags.in_test:
-		warehouse_account = frappe._dict()
+		warehouse_account = WarehouseAccountMap()
 
 		filters = {}
 		if company:
@@ -37,7 +48,7 @@ def get_warehouse_account_map(company=None):
 			order_by="lft, rgt",
 		):
 			if not d.account:
-				d.account = get_warehouse_account(d, warehouse_account)
+				d.account = get_warehouse_account(d, warehouse_account, raise_error=False)
 
 			if d.account:
 				d.account_currency = frappe.db.get_value("Account", d.account, "account_currency", cache=True)
@@ -47,10 +58,13 @@ def get_warehouse_account_map(company=None):
 		else:
 			frappe.flags.warehouse_account_map = warehouse_account
 
-	return frappe.flags.warehouse_account_map.get(company) or frappe.flags.warehouse_account_map
+	if company:
+		return frappe.flags.warehouse_account_map.get(company, WarehouseAccountMap())
+
+	return frappe.flags.warehouse_account_map
 
 
-def get_warehouse_account(warehouse, warehouse_account=None):
+def get_warehouse_account(warehouse, warehouse_account=None, *, raise_error=True):
 	account = warehouse.account
 	if not account and warehouse.parent_warehouse:
 		if warehouse_account:
@@ -86,7 +100,7 @@ def get_warehouse_account(warehouse, warehouse_account=None):
 		if len(inventory_accounts) == 1:
 			account = inventory_accounts[0]
 
-	if not account and warehouse.company and not warehouse.is_group:
+	if raise_error and not account and warehouse.company and not warehouse.is_group:
 		frappe.throw(
 			_("Please set Account in Warehouse {0} or Default Inventory Account in Company {1}").format(
 				warehouse.name, warehouse.company

erpnext/stock/doctype/purchase_receipt/purchase_receipt.py

@@ -609,7 +609,7 @@ class PurchaseReceipt(BuyingController):
 
 		def make_sub_contracting_gl_entries(item):
 			# sub-contracting warehouse
-			if flt(item.rm_supp_cost) and warehouse_account.get(self.supplier_warehouse):
+			if flt(item.rm_supp_cost):
 				self.add_gl_entry(
 					gl_entries=gl_entries,
 					account=supplier_warehouse_account,
@@ -718,22 +718,22 @@ class PurchaseReceipt(BuyingController):
 					stock_value_diff = (
 						flt(d.base_net_amount) + flt(d.item_tax_amount) + flt(d.landed_cost_voucher_amount)
 					)
-				elif warehouse_account.get(d.warehouse):
+				elif d.warehouse:
 					stock_value_diff = get_stock_value_difference(self.name, d.name, d.warehouse)
 					stock_asset_account_name = warehouse_account[d.warehouse]["account"]
-					supplier_warehouse_account = warehouse_account.get(self.supplier_warehouse, {}).get(
-						"account"
-					)
-					supplier_warehouse_account_currency = warehouse_account.get(
-						self.supplier_warehouse, {}
-					).get("account_currency")
+					supplier_warehouse_details = warehouse_account.get(self.supplier_warehouse, {})
+					if flt(d.rm_supp_cost):
+						supplier_warehouse_details = warehouse_account[self.supplier_warehouse]
+
+					supplier_warehouse_account = supplier_warehouse_details.get("account")
+					supplier_warehouse_account_currency = supplier_warehouse_details.get("account_currency")
 
 					# If PR is sub-contracted and fg item rate is zero
 					# in that case if account for source and target warehouse are same,
 					# then GL entries should not be posted
 					if (
 						flt(stock_value_diff) == flt(d.rm_supp_cost)
-						and warehouse_account.get(self.supplier_warehouse)
+						and supplier_warehouse_account
 						and stock_asset_account_name == supplier_warehouse_account
 					):
 						continue

erpnext/stock/doctype/warehouse/test_warehouse.py

@@ -112,6 +112,7 @@ class TestWarehouse(FrappeTestCase):
 			frappe.delete_doc("Account", "Extra Inventory Account - _TCIF")
 
 		warehouse = frappe.get_doc("Warehouse", {"company": company, "is_group": 0})
+		warehouse.db_set("account", None)
 		single_account = frappe.db.get_value(
 			"Account", {"account_type": "Stock", "is_group": 0, "company": company}, "name"
 		)
@@ -125,6 +126,101 @@ class TestWarehouse(FrappeTestCase):
 		)
 		self.assertRaises(frappe.ValidationError, get_warehouse_account, warehouse)
 
+	def test_unrelated_warehouse_without_inventory_account_is_ignored(self):
+		from erpnext.stock import get_warehouse_account_map
+
+		company, warehouse = create_ambiguous_inventory_account_warehouse()
+		warehouse_account_map = get_warehouse_account_map(company)
+		resolved_warehouse = next(iter(warehouse_account_map))
+
+		self.assertNotIn(warehouse.name, warehouse_account_map)
+		self.assertTrue(warehouse_account_map[resolved_warehouse].account)
+
+	def test_direct_warehouse_account_map_lookup_remains_strict(self):
+		from erpnext.stock import get_warehouse_account_map
+
+		company, warehouse = create_ambiguous_inventory_account_warehouse()
+
+		with self.assertRaises(frappe.ValidationError):
+			get_warehouse_account_map(company)[warehouse.name]
+
+	def test_new_warehouse_requires_inventory_account(self):
+		company, _warehouse = create_ambiguous_inventory_account_warehouse()
+		frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1)
+		parent_warehouse = frappe.db.get_value("Warehouse", {"company": company, "is_group": 1}, "name")
+		frappe.db.set_value("Warehouse", parent_warehouse, "account", None)
+		warehouse = frappe.get_doc(
+			{
+				"doctype": "Warehouse",
+				"warehouse_name": "Missing Inventory Account",
+				"parent_warehouse": parent_warehouse,
+				"company": company,
+			}
+		)
+
+		self.assertRaisesRegex(frappe.ValidationError, "Missing Inventory Account - _TCIF", warehouse.insert)
+
+	def test_new_warehouse_can_inherit_inventory_account(self):
+		from erpnext.stock import get_warehouse_account
+
+		company, _warehouse = create_ambiguous_inventory_account_warehouse()
+		frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1)
+		parent_warehouse = frappe.db.get_value("Warehouse", {"company": company, "is_group": 1}, "name")
+		inventory_account = frappe.db.get_value(
+			"Account", {"company": company, "account_type": "Stock", "is_group": 0}, "name"
+		)
+		frappe.db.set_value("Warehouse", parent_warehouse, "account", inventory_account)
+
+		warehouse = frappe.get_doc(
+			{
+				"doctype": "Warehouse",
+				"warehouse_name": "Inherited Inventory Account",
+				"parent_warehouse": parent_warehouse,
+				"company": company,
+			}
+		).insert()
+
+		self.assertEqual(get_warehouse_account(warehouse), inventory_account)
+
+	def test_new_warehouse_inherits_from_parent_created_in_same_transaction(self):
+		from erpnext.stock import get_warehouse_account
+
+		company, _warehouse = create_ambiguous_inventory_account_warehouse()
+		frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1)
+		root_warehouse = frappe.db.get_value("Warehouse", {"company": company, "is_group": 1}, "name")
+		inventory_account = frappe.db.get_value(
+			"Account", {"company": company, "account_type": "Stock", "is_group": 0}, "name"
+		)
+
+		parent_warehouse = frappe.get_doc(
+			{
+				"doctype": "Warehouse",
+				"warehouse_name": "New Parent Warehouse",
+				"parent_warehouse": root_warehouse,
+				"company": company,
+				"is_group": 1,
+				"account": inventory_account,
+			}
+		).insert()
+		child_warehouse = frappe.get_doc(
+			{
+				"doctype": "Warehouse",
+				"warehouse_name": "New Child Warehouse",
+				"parent_warehouse": parent_warehouse.name,
+				"company": company,
+			}
+		).insert()
+
+		self.assertEqual(get_warehouse_account(child_warehouse), inventory_account)
+
+	def test_warehouse_onload_allows_missing_inventory_account(self):
+		company, warehouse = create_ambiguous_inventory_account_warehouse()
+		frappe.db.set_value("Company", company, "enable_perpetual_inventory", 1)
+
+		warehouse.run_method("onload")
+
+		self.assertNotIn("account", warehouse.get_onload())
+
 
 def create_inventory_fallback_company():
 	company = "_Test Company Inventory Fallback"
@@ -142,6 +238,38 @@ def create_inventory_fallback_company():
 	return company
 
 
+def create_ambiguous_inventory_account_warehouse():
+	company = create_inventory_fallback_company()
+	frappe.db.set_value("Company", company, "default_inventory_account", None)
+
+	single_account = frappe.db.get_value(
+		"Account", {"account_type": "Stock", "is_group": 0, "company": company}, "name"
+	)
+	warehouses = frappe.get_all(
+		"Warehouse", filters={"company": company, "is_group": 0}, pluck="name", order_by="name"
+	)
+	for warehouse_name in warehouses:
+		frappe.db.set_value("Warehouse", warehouse_name, "account", single_account)
+
+	for group_warehouse in frappe.get_all(
+		"Warehouse", filters={"company": company, "is_group": 1}, pluck="name"
+	):
+		frappe.db.set_value("Warehouse", group_warehouse, "account", None)
+
+	warehouse = frappe.get_doc("Warehouse", warehouses[0])
+	warehouse.db_set({"account": None, "disabled": 0})
+
+	if not frappe.db.exists("Account", "Extra Inventory Account - _TCIF"):
+		create_account(
+			account_name="Extra Inventory Account",
+			parent_account=frappe.db.get_value("Account", single_account, "parent_account"),
+			account_type="Stock",
+			company=company,
+		)
+
+	return company, warehouse
+
+
 def create_warehouse(warehouse_name, properties=None, company=None):
 	if not company:
 		company = "_Test Company"

erpnext/stock/doctype/warehouse/warehouse.py

@@ -55,15 +55,35 @@ class Warehouse(NestedSet):
 	def onload(self):
 		"""load account name for General Ledger Report"""
 		if self.company and cint(frappe.db.get_value("Company", self.company, "enable_perpetual_inventory")):
-			account = self.account or get_warehouse_account(self)
+			account = self.account or get_warehouse_account(self, raise_error=False)
 
 			if account:
 				self.set_onload("account", account)
 		load_address_and_contact(self)
 
 	def validate(self):
+		self.validate_inventory_account()
 		self.warn_about_multiple_warehouse_account()
 
+	def validate_inventory_account(self):
+		if (
+			not self.is_new()
+			or not self.company
+			or self.flags.ignore_inventory_account_validation
+			or not frappe.get_cached_value("Company", self.company, "enable_perpetual_inventory")
+		):
+			return
+
+		warehouse = frappe._dict(self.as_dict())
+		if not self.account and self.parent_warehouse:
+			parent_bounds = frappe.db.get_value(
+				"Warehouse", self.parent_warehouse, ["lft", "rgt"], as_dict=True
+			)
+			if parent_bounds:
+				warehouse.update(parent_bounds)
+
+		get_warehouse_account(warehouse)
+
 	def on_update(self):
 		self.update_nsm_model()
 

erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py

@@ -633,7 +633,7 @@ class SubcontractingReceipt(SubcontractingController):
 
 		for item in self.items:
 			if flt(item.rate) and flt(item.qty):
-				if warehouse_account and warehouse_account.get(item.warehouse):
+				if warehouse_account is not None:
 					stock_value_diff = frappe.db.get_value(
 						"Stock Ledger Entry",
 						{
@@ -647,9 +647,11 @@ class SubcontractingReceipt(SubcontractingController):
 					)
 
 					accepted_warehouse_account = warehouse_account[item.warehouse]["account"]
-					supplier_warehouse_account = warehouse_account.get(self.supplier_warehouse, {}).get(
-						"account"
-					)
+					supplier_warehouse_details = warehouse_account.get(self.supplier_warehouse, {})
+					if flt(item.rm_supp_cost):
+						supplier_warehouse_details = warehouse_account[self.supplier_warehouse]
+
+					supplier_warehouse_account = supplier_warehouse_details.get("account")
 					remarks = self.get("remarks") or _("Accounting Entry for Stock")
 
 					# Accepted Warehouse Account (Debit)