From 55fe26904622273a0089695197e666237bc87c8b Mon Sep 17 00:00:00 2001 From: Pandiyan P Date: Fri, 7 Aug 2026 23:02:29 +0530 Subject: [PATCH] fix: allow selecting a warehouse for new items in the update items dialog (#57876) --- .../accounts/services/child_item_update.py | 49 ++++++-- .../purchase_order/test_purchase_order.py | 53 ++++++++- erpnext/public/js/utils.js | 26 ++++ .../doctype/sales_order/test_sales_order.py | 111 ++++++++++++++++++ 4 files changed, 226 insertions(+), 13 deletions(-) diff --git a/erpnext/accounts/services/child_item_update.py b/erpnext/accounts/services/child_item_update.py index ae7f8109f74..d66c5621f7a 100644 --- a/erpnext/accounts/services/child_item_update.py +++ b/erpnext/accounts/services/child_item_update.py @@ -16,6 +16,11 @@ from erpnext.stock.get_item_details import ( get_conversion_factor, get_item_warehouse_, ) +from erpnext.stock.utils import ( + is_group_warehouse, + validate_disabled_warehouse, + validate_warehouse_company, +) class ChildItemUpdater: @@ -340,7 +345,7 @@ def set_order_defaults( child_item.update({date_fieldname: trans_item.get(date_fieldname) or p_doc.get(date_fieldname)}) child_item.stock_uom = item.stock_uom child_item.uom = trans_item.get("uom") or item.stock_uom - child_item.warehouse = get_item_warehouse_(p_doc, item, overwrite_warehouse=True) + child_item.warehouse = get_new_child_item_warehouse(p_doc, item, trans_item, child_doctype) conversion_factor = flt(get_conversion_factor(item.item_code, child_item.uom).get("conversion_factor")) child_item.conversion_factor = flt(trans_item.get("conversion_factor")) or conversion_factor child_item.update(get_bin_details(child_item.item_code, child_item.warehouse, p_doc.get("company"))) @@ -349,20 +354,44 @@ def set_order_defaults( child_item.base_rate = 1 child_item.base_amount = 1 - if child_doctype == "Sales Order Item": - child_item.warehouse = get_item_warehouse_(p_doc, item, overwrite_warehouse=True) - if not child_item.warehouse: - frappe.throw( - _( - "Cannot find a default warehouse for item {0}. Please set one in the Item Master or in Stock Settings." - ).format(frappe.bold(item.item_code)) - ) - set_child_tax_template_and_map(item, child_item, p_doc) add_taxes_from_tax_template(child_item, p_doc) return child_item +def get_new_child_item_warehouse(p_doc, item, trans_item: dict, child_doctype: str) -> str | None: + """Return the warehouse picked in the Update Items dialog, else the configured default. + + Validates whichever warehouse was resolved, since a submitted parent skips validate(). + """ + warehouse = trans_item.get("warehouse") or get_item_warehouse_(p_doc, item, overwrite_warehouse=True) + + if not warehouse: + if is_warehouse_required_for_new_child_item(child_doctype, item, trans_item): + frappe.throw( + _( + "Cannot find a default warehouse for item {0}. Please select one in the Update Items dialog, or set a default in the Item Master or in the Company." + ).format(frappe.bold(item.item_code)) + ) + return None + + validate_warehouse_company(warehouse, p_doc.company) + validate_disabled_warehouse(warehouse) + is_group_warehouse(warehouse) + return warehouse + + +def is_warehouse_required_for_new_child_item(child_doctype: str, item, trans_item: dict) -> bool: + """Sales Order always needs one; buying documents only for stock rows, as in validate_stock_item_warehouse.""" + if child_doctype == "Sales Order Item": + return True + + if child_doctype in ("Purchase Order Item", "Supplier Quotation Item"): + return bool(item.is_stock_item and flt(trans_item.get("qty")) and not item.delivered_by_supplier) + + return False + + def validate_child_on_delete(row, parent, ordered_item=None) -> None: """Raise if a partially transacted child item is being deleted.""" if parent.doctype == "Sales Order": diff --git a/erpnext/buying/doctype/purchase_order/test_purchase_order.py b/erpnext/buying/doctype/purchase_order/test_purchase_order.py index 43b441ee19e..820e4f6aa2c 100644 --- a/erpnext/buying/doctype/purchase_order/test_purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/test_purchase_order.py @@ -320,6 +320,7 @@ class TestPurchaseOrder(ERPNextTestSuite): po.load_from_db() existing_ordered_qty = get_ordered_qty() + existing_ordered_qty_in_new_warehouse = get_ordered_qty(warehouse="_Test Warehouse 2 - _TC") first_item_of_po = po.get("items")[0] trans_item = json.dumps( @@ -330,16 +331,62 @@ class TestPurchaseOrder(ERPNextTestSuite): "qty": first_item_of_po.qty, "docname": first_item_of_po.name, }, - {"item_code": "_Test Item", "rate": 200, "qty": 7}, + {"item_code": "_Test Item", "rate": 200, "qty": 7, "warehouse": "_Test Warehouse 2 - _TC"}, ] ) update_child_qty_rate("Purchase Order", trans_item, po.name) po.reload() self.assertEqual(len(po.get("items")), 2) + self.assertEqual(po.get("items")[-1].warehouse, "_Test Warehouse 2 - _TC") self.assertEqual(po.status, "To Receive and Bill") - # ordered qty should increase on row addition - self.assertEqual(get_ordered_qty(), existing_ordered_qty + 7) + # ordered qty should increase on row addition, in the warehouse passed for the new row + self.assertEqual(get_ordered_qty(), existing_ordered_qty) + self.assertEqual( + get_ordered_qty(warehouse="_Test Warehouse 2 - _TC"), + existing_ordered_qty_in_new_warehouse + 7, + ) + + def test_update_child_adding_new_item_without_any_default_warehouse(self): + stock_item = make_item("_Test PO Item Without Default Warehouse", {"is_stock_item": 1}).name + service_item = make_item("_Test PO Item Non Stock", {"is_stock_item": 0}).name + + po = create_purchase_order(do_not_save=1) + po.save() + po.submit() + first_item_of_po = po.get("items")[0] + + company_default = frappe.db.get_value("Company", po.company, "default_warehouse") + frappe.db.set_value("Company", po.company, "default_warehouse", None) + self.addCleanup(frappe.db.set_value, "Company", po.company, "default_warehouse", company_default) + + def get_trans_items(item_code): + return json.dumps( + [ + { + "item_code": first_item_of_po.item_code, + "rate": first_item_of_po.rate, + "qty": first_item_of_po.qty, + "docname": first_item_of_po.name, + }, + {"item_code": item_code, "rate": 200, "qty": 7}, + ] + ) + + self.assertRaisesRegex( + frappe.ValidationError, + "Cannot find a default warehouse", + update_child_qty_rate, + "Purchase Order", + get_trans_items(stock_item), + po.name, + ) + + update_child_qty_rate("Purchase Order", get_trans_items(service_item), po.name) + + po.reload() + self.assertEqual(po.get("items")[-1].item_code, service_item) + self.assertFalse(po.get("items")[-1].warehouse) def test_update_child_removing_item(self): po = create_purchase_order(do_not_save=1) diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js index acaf7fb056e..66a634336e8 100755 --- a/erpnext/public/js/utils.js +++ b/erpnext/public/js/utils.js @@ -742,6 +742,7 @@ erpnext.utils.update_child_items = function (opts) { qty: d.qty, rate: d.rate, uom: d.uom, + warehouse: d.warehouse, fg_item: d.fg_item, fg_item_qty: d.fg_item_qty, description: d.description, @@ -829,6 +830,7 @@ erpnext.utils.update_child_items = function (opts) { item_name, bom_no, description, + warehouse, } = r.message; const row = dialog.fields_dict.trans_items.df.data.find( (row) => row.name == me.doc.name @@ -842,6 +844,7 @@ erpnext.utils.update_child_items = function (opts) { item_name: item_name, bom_no: bom_no, description: me.doc.description || description, + warehouse: me.doc.docname ? me.doc.warehouse : warehouse, }); dialog.fields_dict.trans_items.grid.refresh(); } @@ -929,6 +932,29 @@ erpnext.utils.update_child_items = function (opts) { }); } + const warehouse_df = child_meta.fields.find((f) => f.fieldname == "warehouse"); + if (warehouse_df) { + fields.splice(3, 0, { + fieldtype: "Link", + fieldname: "warehouse", + options: "Warehouse", + in_list_view: 1, + label: __(warehouse_df.label), + // only new rows may set it, existing rows would leave their + // reserved qty stranded in the previous warehouse's bin + read_only_depends_on: "eval:doc.docname", + get_query: () => { + return { + filters: { + company: frm.doc.company, + is_group: 0, + disabled: 0, + }, + }; + }, + }); + } + if (["Purchase Order", "Sales Order"].includes(frm.doc.doctype) && frm.doc.is_subcontracted) { fields.push( { diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index cec1114b1f3..0ec0f203c14 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -36,6 +36,7 @@ from erpnext.selling.doctype.sales_order.sales_order import ( from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_bin_details +from erpnext.stock.utils import InvalidWarehouseCompany from erpnext.tests.utils import ERPNextTestSuite @@ -587,6 +588,116 @@ class TestSalesOrder(ERPNextTestSuite): self.assertEqual(updated_total, prev_total + 1400) self.assertNotEqual(updated_total_in_words, prev_total_in_words) + def test_update_child_adding_new_item_with_warehouse(self): + so = make_sales_order(item_code="_Test Item", qty=4) + + first_item_of_so = so.get("items")[0] + self.assertNotEqual(first_item_of_so.warehouse, "_Test Warehouse 2 - _TC") + + def get_trans_item(warehouse): + return json.dumps( + [ + { + "item_code": first_item_of_so.item_code, + "rate": first_item_of_so.rate, + "qty": first_item_of_so.qty, + "docname": first_item_of_so.name, + "warehouse": warehouse, + }, + {"item_code": "_Test Item 2", "rate": 200, "qty": 7, "warehouse": warehouse}, + ] + ) + + self.assertRaises( + InvalidWarehouseCompany, + update_child_qty_rate, + "Sales Order", + get_trans_item("_Test Warehouse 2 - _TC1"), + so.name, + ) + + self.assertRaisesRegex( + frappe.ValidationError, + "Group node warehouse", + update_child_qty_rate, + "Sales Order", + get_trans_item("_Test Warehouse Group - _TC"), + so.name, + ) + + if not frappe.db.exists("Warehouse", "_Test Disabled Warehouse - _TC"): + frappe.get_doc( + { + "doctype": "Warehouse", + "warehouse_name": "_Test Disabled Warehouse", + "company": "_Test Company", + "disabled": 1, + } + ).insert() + + self.assertRaisesRegex( + frappe.ValidationError, + "Disabled Warehouse", + update_child_qty_rate, + "Sales Order", + get_trans_item("_Test Disabled Warehouse - _TC"), + so.name, + ) + + update_child_qty_rate("Sales Order", get_trans_item("_Test Warehouse 2 - _TC"), so.name) + + so.reload() + # the new row picks up the warehouse selected in the dialog + self.assertEqual(so.get("items")[-1].item_code, "_Test Item 2") + self.assertEqual(so.get("items")[-1].warehouse, "_Test Warehouse 2 - _TC") + # existing rows keep theirs, so their reserved qty stays in the same bin + self.assertEqual(so.get("items")[0].warehouse, first_item_of_so.warehouse) + + def test_update_child_adding_new_item_without_any_default_warehouse(self): + item_code = make_item("_Test Item Without Default Warehouse", {"is_stock_item": 1}).name + so = make_sales_order(item_code="_Test Item", qty=4) + existing_item = so.get("items")[0] + + # a company gets a default warehouse when its warehouses are created + company_default = frappe.db.get_value("Company", so.company, "default_warehouse") + frappe.db.set_value("Company", so.company, "default_warehouse", None) + self.addCleanup(frappe.db.set_value, "Company", so.company, "default_warehouse", company_default) + + def get_trans_items(warehouse=None): + new_row = {"item_code": item_code, "rate": 200, "qty": 7} + if warehouse: + new_row["warehouse"] = warehouse + + return json.dumps( + [ + { + "item_code": existing_item.item_code, + "rate": existing_item.rate, + "qty": existing_item.qty, + "docname": existing_item.name, + }, + new_row, + ] + ) + + # no default in the Item Master, Item Group, Brand or Company + self.assertRaisesRegex( + frappe.ValidationError, + "Cannot find a default warehouse", + update_child_qty_rate, + "Sales Order", + get_trans_items(), + so.name, + ) + + update_child_qty_rate("Sales Order", get_trans_items("_Test Warehouse - _TC"), so.name) + + so.reload() + self.assertEqual(len(so.get("items")), 2) + self.assertEqual(so.get("items")[0].warehouse, existing_item.warehouse) + self.assertEqual(so.get("items")[-1].item_code, item_code) + self.assertEqual(so.get("items")[-1].warehouse, "_Test Warehouse - _TC") + def test_update_child_removing_item(self): so = make_sales_order(**{"item_list": [{"item_code": "_Test Item", "qty": 5, "rate": 1000}]}) create_dn_against_so(so.name, 2)