From 13883a00b07d919cb21dd6924ed3c7d7f5a7c5a5 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/controllers/stock_controller.py | 17 ++- erpnext/setup/doctype/company/company.py | 1 + erpnext/stock/__init__.py | 11 +- .../purchase_receipt/services/gl_composer.py | 15 ++- .../stock/doctype/warehouse/test_warehouse.py | 107 ++++++++++++++++++ erpnext/stock/doctype/warehouse/warehouse.py | 10 +- 6 files changed, 147 insertions(+), 14 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 8a9cbbd8de3..11bf64ffd53 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -31,7 +31,7 @@ from erpnext.exceptions import ( ) from erpnext.setup.doctype.brand.brand import get_brand_defaults from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults -from erpnext.stock import get_warehouse_account_map +from erpnext.stock import get_warehouse_account, get_warehouse_account_map from erpnext.stock.doctype.item.item import get_item_defaults from erpnext.stock.services.internal_transfer import StockInternalTransferService from erpnext.stock.stock_ledger import get_items_to_be_repost @@ -135,7 +135,9 @@ class StockController(AccountsController): def use_item_inventory_account(self): return frappe.get_cached_value("Company", self.company, "enable_item_wise_inventory_account") - def get_inventory_account_dict(self, row, inventory_account_map, warehouse_field=None): + def get_inventory_account_dict( + self, row, inventory_account_map, warehouse_field=None, *, raise_error=True + ): account_dict = frappe._dict() if isinstance(row, dict): @@ -164,8 +166,15 @@ class StockController(AccountsController): if not warehouse: warehouse = self.get(warehouse_field) - if warehouse and warehouse in inventory_account_map: - account_dict = inventory_account_map[warehouse] + if warehouse: + account_dict = inventory_account_map.get(warehouse) + if not account_dict and raise_error: + account = get_warehouse_account(frappe.get_cached_doc("Warehouse", warehouse)) + account_dict = frappe._dict( + account=account, + account_currency=frappe.get_cached_value("Account", account, "account_currency"), + ) + inventory_account_map[warehouse] = account_dict return account_dict diff --git a/erpnext/setup/doctype/company/company.py b/erpnext/setup/doctype/company/company.py index 7fac5de4ac1..f4c43106d54 100644 --- a/erpnext/setup/doctype/company/company.py +++ b/erpnext/setup/doctype/company/company.py @@ -522,6 +522,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 4587fc857f5..12a3a98a78c 100644 --- a/erpnext/stock/__init__.py +++ b/erpnext/stock/__init__.py @@ -37,7 +37,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 +47,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, frappe._dict()) + + 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: @@ -87,7 +90,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/services/gl_composer.py b/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py index cab4337f4bf..6639223dc56 100644 --- a/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py +++ b/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py @@ -8,6 +8,7 @@ from frappe.utils import cint, flt import erpnext from erpnext.accounts.general_ledger import process_gl_map from erpnext.accounts.utils import get_account_currency +from erpnext.stock import get_warehouse_account from erpnext.stock.services.base_stock_gl_composer import BaseStockGLComposer @@ -312,11 +313,15 @@ class PurchaseReceiptGLComposer(BaseStockGLComposer): supplier_warehouse_account = None supplier_warehouse_account_currency = None if doc.supplier_warehouse: - if _inv_dict := doc.get_inventory_account_dict( - d, inventory_account_map, "supplier_warehouse" - ): - supplier_warehouse_account = _inv_dict["account"] - supplier_warehouse_account_currency = _inv_dict["account_currency"] + # The account is optional only when this lookup can skip a duplicate entry. + supplier_warehouse_account = get_warehouse_account( + frappe.get_cached_doc("Warehouse", doc.supplier_warehouse), + raise_error=bool(flt(d.rm_supp_cost)), + ) + if supplier_warehouse_account: + supplier_warehouse_account_currency = get_account_currency( + supplier_warehouse_account + ) if ( flt(stock_value_diff) == flt(d.rm_supp_cost) diff --git a/erpnext/stock/doctype/warehouse/test_warehouse.py b/erpnext/stock/doctype/warehouse/test_warehouse.py index 3d67905741c..cd010b13e42 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -116,6 +116,86 @@ class TestWarehouse(ERPNextTestSuite): ) 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)) + stock_entry = frappe.get_doc({"doctype": "Stock Entry", "company": company}) + + self.assertNotIn(warehouse.name, warehouse_account_map) + self.assertTrue( + stock_entry.get_inventory_account_dict( + frappe._dict(warehouse=resolved_warehouse), warehouse_account_map + ).account + ) + self.assertFalse( + stock_entry.get_inventory_account_dict( + frappe._dict(supplier_warehouse=warehouse.name), + warehouse_account_map, + "supplier_warehouse", + raise_error=False, + ) + ) + + def test_warehouse_without_inventory_account_is_validated_when_used(self): + from erpnext.stock import get_warehouse_account_map + + company, warehouse = create_ambiguous_inventory_account_warehouse() + stock_entry = frappe.get_doc({"doctype": "Stock Entry", "company": company}) + + with self.assertRaises(frappe.ValidationError): + stock_entry.get_inventory_account_dict( + frappe._dict(warehouse=warehouse.name), get_warehouse_account_map(company) + ) + + 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" @@ -133,6 +213,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 cac2319196a..5a313b6cc02 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -61,9 +61,17 @@ 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): 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) From d2b5c6ad408ee272658cb1ffb1d3b1f3e077c2c6 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 5a313b6cc02..a3d73173659 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.py +++ b/erpnext/stock/doctype/warehouse/warehouse.py @@ -61,14 +61,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): if self.company and cint(frappe.db.get_value("Company", self.company, "enable_perpetual_inventory")): account = self.account or get_warehouse_account(self, raise_error=False) @@ -79,8 +71,28 @@ class Warehouse(NestedSet): self.set_onload("stock_exists", self.check_if_sle_exists(non_cancelled_only=True)) 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 624e97f6c14221480cccab560af0db36534e8a31 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 18:26:09 +0530 Subject: [PATCH 3/4] chore(stock): drop redundant supplier warehouse comment --- erpnext/stock/doctype/purchase_receipt/services/gl_composer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py b/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py index 6639223dc56..61350d78200 100644 --- a/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py +++ b/erpnext/stock/doctype/purchase_receipt/services/gl_composer.py @@ -313,7 +313,6 @@ class PurchaseReceiptGLComposer(BaseStockGLComposer): supplier_warehouse_account = None supplier_warehouse_account_currency = None if doc.supplier_warehouse: - # The account is optional only when this lookup can skip a duplicate entry. supplier_warehouse_account = get_warehouse_account( frappe.get_cached_doc("Warehouse", doc.supplier_warehouse), raise_error=bool(flt(d.rm_supp_cost)), From b54a4f62853dae4f5a9293fd8293296ab42c54db Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 11 Aug 2026 18:26:10 +0530 Subject: [PATCH 4/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 cd010b13e42..6d17b21984d 100644 --- a/erpnext/stock/doctype/warehouse/test_warehouse.py +++ b/erpnext/stock/doctype/warehouse/test_warehouse.py @@ -164,7 +164,7 @@ class TestWarehouse(ERPNextTestSuite): } ) - 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 @@ -188,6 +188,37 @@ class TestWarehouse(ERPNextTestSuite): 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)