mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-15 23:48:38 +00:00
fix(stock): validate warehouse accounts when used
This commit is contained in:
@@ -346,6 +346,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"]:
|
||||||
|
|||||||
@@ -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):
|
def get_warehouse_account_map(company=None):
|
||||||
company_warehouse_account_map = company and frappe.flags.setdefault("warehouse_account_map", {}).get(
|
company_warehouse_account_map = company and frappe.flags.setdefault("warehouse_account_map", {}).get(
|
||||||
company
|
company
|
||||||
@@ -23,7 +34,7 @@ def get_warehouse_account_map(company=None):
|
|||||||
warehouse_account_map = frappe.flags.warehouse_account_map
|
warehouse_account_map = frappe.flags.warehouse_account_map
|
||||||
|
|
||||||
if not warehouse_account_map or not company_warehouse_account_map or frappe.flags.in_test:
|
if not warehouse_account_map or not company_warehouse_account_map or frappe.flags.in_test:
|
||||||
warehouse_account = frappe._dict()
|
warehouse_account = WarehouseAccountMap()
|
||||||
|
|
||||||
filters = {}
|
filters = {}
|
||||||
if company:
|
if company:
|
||||||
@@ -37,7 +48,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 +58,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, 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
|
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 +100,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
|
||||||
|
|||||||
@@ -609,7 +609,7 @@ class PurchaseReceipt(BuyingController):
|
|||||||
|
|
||||||
def make_sub_contracting_gl_entries(item):
|
def make_sub_contracting_gl_entries(item):
|
||||||
# sub-contracting warehouse
|
# 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(
|
self.add_gl_entry(
|
||||||
gl_entries=gl_entries,
|
gl_entries=gl_entries,
|
||||||
account=supplier_warehouse_account,
|
account=supplier_warehouse_account,
|
||||||
@@ -718,22 +718,22 @@ class PurchaseReceipt(BuyingController):
|
|||||||
stock_value_diff = (
|
stock_value_diff = (
|
||||||
flt(d.base_net_amount) + flt(d.item_tax_amount) + flt(d.landed_cost_voucher_amount)
|
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_value_diff = get_stock_value_difference(self.name, d.name, d.warehouse)
|
||||||
stock_asset_account_name = warehouse_account[d.warehouse]["account"]
|
stock_asset_account_name = warehouse_account[d.warehouse]["account"]
|
||||||
supplier_warehouse_account = warehouse_account.get(self.supplier_warehouse, {}).get(
|
supplier_warehouse_details = warehouse_account.get(self.supplier_warehouse, {})
|
||||||
"account"
|
if flt(d.rm_supp_cost):
|
||||||
)
|
supplier_warehouse_details = warehouse_account[self.supplier_warehouse]
|
||||||
supplier_warehouse_account_currency = warehouse_account.get(
|
|
||||||
self.supplier_warehouse, {}
|
supplier_warehouse_account = supplier_warehouse_details.get("account")
|
||||||
).get("account_currency")
|
supplier_warehouse_account_currency = supplier_warehouse_details.get("account_currency")
|
||||||
|
|
||||||
# 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,
|
||||||
# then GL entries should not be posted
|
# then GL entries should not be posted
|
||||||
if (
|
if (
|
||||||
flt(stock_value_diff) == flt(d.rm_supp_cost)
|
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
|
and stock_asset_account_name == supplier_warehouse_account
|
||||||
):
|
):
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -125,6 +125,70 @@ class TestWarehouse(FrappeTestCase):
|
|||||||
)
|
)
|
||||||
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))
|
||||||
|
|
||||||
|
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():
|
def create_inventory_fallback_company():
|
||||||
company = "_Test Company Inventory Fallback"
|
company = "_Test Company Inventory Fallback"
|
||||||
@@ -142,6 +206,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"
|
||||||
|
|||||||
@@ -52,10 +52,18 @@ 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):
|
||||||
"""load account name for General Ledger Report"""
|
"""load account name for General Ledger Report"""
|
||||||
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)
|
||||||
|
|||||||
@@ -633,7 +633,7 @@ class SubcontractingReceipt(SubcontractingController):
|
|||||||
|
|
||||||
for item in self.items:
|
for item in self.items:
|
||||||
if flt(item.rate) and flt(item.qty):
|
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_value_diff = frappe.db.get_value(
|
||||||
"Stock Ledger Entry",
|
"Stock Ledger Entry",
|
||||||
{
|
{
|
||||||
@@ -647,9 +647,11 @@ class SubcontractingReceipt(SubcontractingController):
|
|||||||
)
|
)
|
||||||
|
|
||||||
accepted_warehouse_account = warehouse_account[item.warehouse]["account"]
|
accepted_warehouse_account = warehouse_account[item.warehouse]["account"]
|
||||||
supplier_warehouse_account = warehouse_account.get(self.supplier_warehouse, {}).get(
|
supplier_warehouse_details = warehouse_account.get(self.supplier_warehouse, {})
|
||||||
"account"
|
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")
|
remarks = self.get("remarks") or _("Accounting Entry for Stock")
|
||||||
|
|
||||||
# Accepted Warehouse Account (Debit)
|
# Accepted Warehouse Account (Debit)
|
||||||
|
|||||||
Reference in New Issue
Block a user