mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-13 14:41:53 +00:00
fix(stock): validate warehouse accounts when used
This commit is contained in:
@@ -32,7 +32,7 @@ from erpnext.exceptions import (
|
|||||||
)
|
)
|
||||||
from erpnext.setup.doctype.brand.brand import get_brand_defaults
|
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.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.batch.batch import get_batch_qty
|
from erpnext.stock.doctype.batch.batch import get_batch_qty
|
||||||
from erpnext.stock.doctype.inventory_dimension.inventory_dimension import (
|
from erpnext.stock.doctype.inventory_dimension.inventory_dimension import (
|
||||||
get_evaluated_inventory_dimension,
|
get_evaluated_inventory_dimension,
|
||||||
@@ -268,7 +268,9 @@ class StockController(AccountsController):
|
|||||||
def use_item_inventory_account(self):
|
def use_item_inventory_account(self):
|
||||||
return frappe.get_cached_value("Company", self.company, "enable_item_wise_inventory_account")
|
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()
|
account_dict = frappe._dict()
|
||||||
|
|
||||||
if isinstance(row, dict):
|
if isinstance(row, dict):
|
||||||
@@ -297,8 +299,15 @@ class StockController(AccountsController):
|
|||||||
if not warehouse:
|
if not warehouse:
|
||||||
warehouse = self.get(warehouse_field)
|
warehouse = self.get(warehouse_field)
|
||||||
|
|
||||||
if warehouse and warehouse in inventory_account_map:
|
if warehouse:
|
||||||
account_dict = inventory_account_map[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
|
return account_dict
|
||||||
|
|
||||||
|
|||||||
@@ -410,6 +410,7 @@ class Company(NestedSet):
|
|||||||
)
|
)
|
||||||
warehouse.flags.ignore_permissions = True
|
warehouse.flags.ignore_permissions = True
|
||||||
warehouse.flags.ignore_mandatory = True
|
warehouse.flags.ignore_mandatory = True
|
||||||
|
warehouse.flags.ignore_inventory_account_validation = True
|
||||||
warehouse.insert()
|
warehouse.insert()
|
||||||
|
|
||||||
if wh_detail["is_group"]:
|
if wh_detail["is_group"]:
|
||||||
|
|||||||
@@ -37,7 +37,7 @@ def get_warehouse_account_map(company=None):
|
|||||||
order_by="lft, rgt",
|
order_by="lft, rgt",
|
||||||
):
|
):
|
||||||
if not d.account:
|
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:
|
if d.account:
|
||||||
d.account_currency = frappe.db.get_value("Account", d.account, "account_currency", cache=True)
|
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:
|
else:
|
||||||
frappe.flags.warehouse_account_map = warehouse_account
|
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
|
account = warehouse.account
|
||||||
if not account and warehouse.parent_warehouse:
|
if not account and warehouse.parent_warehouse:
|
||||||
if warehouse_account:
|
if warehouse_account:
|
||||||
@@ -86,7 +89,7 @@ def get_warehouse_account(warehouse, warehouse_account=None):
|
|||||||
if len(inventory_accounts) == 1:
|
if len(inventory_accounts) == 1:
|
||||||
account = inventory_accounts[0]
|
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(
|
frappe.throw(
|
||||||
_("Please set Account in Warehouse {0} or Default Inventory Account in Company {1}").format(
|
_("Please set Account in Warehouse {0} or Default Inventory Account in Company {1}").format(
|
||||||
warehouse.name, warehouse.company
|
warehouse.name, warehouse.company
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ from erpnext.accounts.utils import get_account_currency
|
|||||||
from erpnext.assets.doctype.asset.asset import get_asset_account, is_cwip_accounting_enabled
|
from erpnext.assets.doctype.asset.asset import get_asset_account, is_cwip_accounting_enabled
|
||||||
from erpnext.controllers.accounts_controller import merge_taxes
|
from erpnext.controllers.accounts_controller import merge_taxes
|
||||||
from erpnext.controllers.buying_controller import BuyingController
|
from erpnext.controllers.buying_controller import BuyingController
|
||||||
|
from erpnext.stock import get_warehouse_account
|
||||||
from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_transaction
|
from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_transaction
|
||||||
from erpnext.stock.doctype.stock_reservation_entry.stock_reservation_entry import StockReservation
|
from erpnext.stock.doctype.stock_reservation_entry.stock_reservation_entry import StockReservation
|
||||||
from erpnext.stock.serial_batch_bundle import (
|
from erpnext.stock.serial_batch_bundle import (
|
||||||
@@ -765,11 +766,15 @@ class PurchaseReceipt(BuyingController):
|
|||||||
supplier_warehouse_account = None
|
supplier_warehouse_account = None
|
||||||
supplier_warehouse_account_currency = None
|
supplier_warehouse_account_currency = None
|
||||||
if self.supplier_warehouse:
|
if self.supplier_warehouse:
|
||||||
if _inv_dict := self.get_inventory_account_dict(
|
# The account is optional only when this lookup can skip a duplicate entry.
|
||||||
d, inventory_account_map, "supplier_warehouse"
|
supplier_warehouse_account = get_warehouse_account(
|
||||||
):
|
frappe.get_cached_doc("Warehouse", self.supplier_warehouse),
|
||||||
supplier_warehouse_account = _inv_dict["account"]
|
raise_error=bool(flt(d.rm_supp_cost)),
|
||||||
supplier_warehouse_account_currency = _inv_dict["account_currency"]
|
)
|
||||||
|
if supplier_warehouse_account:
|
||||||
|
supplier_warehouse_account_currency = get_account_currency(
|
||||||
|
supplier_warehouse_account
|
||||||
|
)
|
||||||
|
|
||||||
# If PR is sub-contracted and fg item rate is zero
|
# If PR is sub-contracted and fg item rate is zero
|
||||||
# in that case if account for source and target warehouse are same,
|
# in that case if account for source and target warehouse are same,
|
||||||
|
|||||||
@@ -117,6 +117,86 @@ class TestWarehouse(ERPNextTestSuite):
|
|||||||
)
|
)
|
||||||
self.assertRaises(frappe.ValidationError, get_warehouse_account, warehouse)
|
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():
|
def create_inventory_fallback_company():
|
||||||
company = "_Test Company Inventory Fallback"
|
company = "_Test Company Inventory Fallback"
|
||||||
@@ -134,6 +214,33 @@ def create_inventory_fallback_company():
|
|||||||
return 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):
|
def create_warehouse(warehouse_name, properties=None, company=None):
|
||||||
if not company:
|
if not company:
|
||||||
company = "_Test Company"
|
company = "_Test Company"
|
||||||
|
|||||||
@@ -60,9 +60,17 @@ class Warehouse(NestedSet):
|
|||||||
|
|
||||||
self.name = self.warehouse_name
|
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):
|
def onload(self):
|
||||||
if self.company and cint(frappe.db.get_value("Company", self.company, "enable_perpetual_inventory")):
|
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:
|
if account:
|
||||||
self.set_onload("account", account)
|
self.set_onload("account", account)
|
||||||
|
|||||||
Reference in New Issue
Block a user