From d620720445dd7e1cddd4cf8c6467de3f56a4d51f Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 16:33:53 +0530 Subject: [PATCH 1/4] fix(stock): validate warehouse accounts when used --- erpnext/setup/doctype/company/company.py | 1 + erpnext/stock/__init__.py | 24 ++++- .../purchase_receipt/purchase_receipt.py | 18 ++-- .../stock/doctype/warehouse/test_warehouse.py | 91 +++++++++++++++++++ erpnext/stock/doctype/warehouse/warehouse.py | 10 +- .../subcontracting_receipt.py | 10 +- 6 files changed, 135 insertions(+), 19 deletions(-) diff --git a/erpnext/setup/doctype/company/company.py b/erpnext/setup/doctype/company/company.py index c53dfa5fb40..05b3ce7aa7b 100644 --- a/erpnext/setup/doctype/company/company.py +++ b/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"]: diff --git a/erpnext/stock/__init__.py b/erpnext/stock/__init__.py index aa556c62434..19d8bcd21e3 100644 --- a/erpnext/stock/__init__.py +++ b/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 diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 08fe7feff56..25c6fd987f5 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/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 diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 5b6f8f727fb..688ba94ef78 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -125,6 +125,70 @@ 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.assertRaises(frappe.ValidationError, 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_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 +206,33 @@ 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) + + 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" diff --git a/erpnext/stock/doctype/warehouse/warehouse.py b/erpnext/stock/doctype/warehouse/warehouse.py index b9600fddc9b..9451228716e 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -52,10 +52,18 @@ class Warehouse(NestedSet): self.name = self.warehouse_name + def before_insert(self): + if ( + self.company + and not self.flags.ignore_inventory_account_validation + and frappe.get_cached_value("Company", self.company, "enable_perpetual_inventory") + ): + get_warehouse_account(self, get_warehouse_account_map(self.company)) + 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) diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py index 8be5f453632..501fde5b3dd 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py +++ b/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) From 5e3d0947c8a3b27844242d557f8a1989fd3ea6e2 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 18:26:08 +0530 Subject: [PATCH 2/4] fix(stock): validate new warehouse inventory account after naming Move the insert-time check from before_insert to validate. before_insert runs before set_new_name, so the validation message rendered the warehouse name as None. validate runs after naming and only applies to new documents via is_new(). Resolve inheritance through the parent's lft/rgt bounds instead of the request-cached warehouse account map. The cached map can be stale within a request (a parent created moments earlier is missing from it), which made get_warehouse_account trigger a full nested-set rebuild_tree and could falsely reject a child whose parent carries a valid account. rebuild_tree enables auto_commit_on_many_writes, which must not run inside a document insert. --- erpnext/stock/doctype/warehouse/warehouse.py | 28 ++++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/erpnext/stock/doctype/warehouse/warehouse.py b/erpnext/stock/doctype/warehouse/warehouse.py index 9451228716e..e92b617600e 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -52,14 +52,6 @@ class Warehouse(NestedSet): self.name = self.warehouse_name - def before_insert(self): - if ( - self.company - and not self.flags.ignore_inventory_account_validation - and frappe.get_cached_value("Company", self.company, "enable_perpetual_inventory") - ): - get_warehouse_account(self, get_warehouse_account_map(self.company)) - 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")): @@ -70,8 +62,28 @@ class Warehouse(NestedSet): 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() From da2c422bf67d0ba2d4c3618b8e93db9c30493367 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 18:26:10 +0530 Subject: [PATCH 3/4] test(stock): cover named validation error and same-transaction parent inheritance --- .../stock/doctype/warehouse/test_warehouse.py | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 688ba94ef78..051fb14e49d 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -157,7 +157,7 @@ class TestWarehouse(FrappeTestCase): } ) - self.assertRaises(frappe.ValidationError, warehouse.insert) + 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 @@ -181,6 +181,37 @@ class TestWarehouse(FrappeTestCase): 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) From 5a61ea64961c860cda80c77f7439dea737e6d6cf Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 18:58:49 +0530 Subject: [PATCH 4/4] test(stock): isolate warehouse account fixtures from class-level state FrappeTestCase on this branch rolls back per class, not per test, so sibling tests leak state. test_new_warehouse_can_inherit_inventory_account left an explicit account on the root group, which made later ambiguous fixtures resolve through the root: the insert validation stopped raising and the unresolved warehouse stayed in the map. The fixture helper now clears group warehouse accounts so every call re-establishes ambiguity. The fallback test also clears the account of the warehouse it picks, since a leftover explicit account skips the single-account fallback it asserts. --- erpnext/stock/doctype/warehouse/test_warehouse.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 051fb14e49d..4417e89a882 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/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" ) @@ -250,6 +251,11 @@ def create_ambiguous_inventory_account_warehouse(): 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})