From dbae23765eb15cbbdf30dd20f4aa0c4ed1a4dfd9 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 31 Aug 2026 17:33:00 +0530 Subject: [PATCH] fix: keep closed rows out of Update Items (#58609) --- .../accounts/services/child_item_update.py | 13 ++- .../tests/test_item_close_update_items.py | 96 +++++++++++++++++++ erpnext/public/js/utils.js | 38 ++++---- 3 files changed, 128 insertions(+), 19 deletions(-) create mode 100644 erpnext/controllers/tests/test_item_close_update_items.py diff --git a/erpnext/accounts/services/child_item_update.py b/erpnext/accounts/services/child_item_update.py index 9d02168fe1f..7d0f7286e83 100644 --- a/erpnext/accounts/services/child_item_update.py +++ b/erpnext/accounts/services/child_item_update.py @@ -82,6 +82,13 @@ class ChildItemUpdater: if is_child_item_unchanged(change_state): continue + if child_item.get("closed"): + frappe.throw( + _( + "Row #{0}: Cannot change item {1} because it is closed. Reopen the row first." + ).format(child_item.idx, child_item.item_code) + ) + self._validate_quantity_and_rate(child_item, d, rate_unchanged) if flt(child_item.get("qty")) != flt(d.get("qty")): @@ -478,7 +485,11 @@ def update_bin_on_delete(row, doctype: str) -> None: def validate_and_delete_children(parent, data, ordered_item=None) -> bool: """Delete child rows not present in data; return True if any were removed.""" updated_item_names = [d.get("docname") for d in data] - deleted_children = [item for item in parent.items if item.name not in updated_item_names] + # A closed row is left out of the payload rather than deleted, so its absence + # must not be read as a removal. + deleted_children = [ + item for item in parent.items if item.name not in updated_item_names and not item.get("closed") + ] for d in deleted_children: validate_child_on_delete(d, parent, ordered_item) diff --git a/erpnext/controllers/tests/test_item_close_update_items.py b/erpnext/controllers/tests/test_item_close_update_items.py new file mode 100644 index 00000000000..6eb156b0d8c --- /dev/null +++ b/erpnext/controllers/tests/test_item_close_update_items.py @@ -0,0 +1,96 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import json + +import frappe +from frappe.utils import add_days, nowdate + +from erpnext.accounts.services.child_item_update import update_child_qty_rate +from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order +from erpnext.controllers.item_close import update_closed_status +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.tests.utils import ERPNextTestSuite + +WAREHOUSE = "_Test Warehouse - _TC" + + +class TestUpdateItemsWithClosedRows(ERPNextTestSuite): + def setUp(self): + self.first_item = make_item(properties={"is_stock_item": 1}).name + self.second_item = make_item(properties={"is_stock_item": 1}).name + + def make_purchase_order(self): + po = create_purchase_order(item_code=self.first_item, qty=10, rate=100, do_not_save=True) + po.append( + "items", + { + "item_code": self.second_item, + "warehouse": WAREHOUSE, + "qty": 10, + "rate": 100, + "schedule_date": add_days(nowdate(), 1), + }, + ) + po.set_missing_values() + po.insert() + po.submit() + update_closed_status("Purchase Order", po.name, [po.items[1].name], 1) + po.reload() + return po + + def as_payload(self, rows, **overrides): + return json.dumps( + [ + { + "docname": row.name, + "item_code": row.item_code, + "qty": overrides.get(row.name, row.qty), + "rate": row.rate, + "uom": row.uom, + "conversion_factor": row.conversion_factor, + "description": row.description, + "schedule_date": str(row.schedule_date), + } + for row in rows + ] + ) + + def test_payload_without_the_closed_row_does_not_delete_it(self): + """The dialog omits closed rows, and absence must not read as removal.""" + po = self.make_purchase_order() + open_row, closed_row = po.items[0], po.items[1] + + update_child_qty_rate("Purchase Order", self.as_payload([open_row], **{open_row.name: 15}), po.name) + + po.reload() + self.assertEqual(len(po.items), 2) + self.assertEqual(po.items[0].qty, 15) + self.assertTrue(po.items[1].closed) + self.assertEqual(po.items[1].name, closed_row.name) + + def test_closed_row_cannot_be_changed_through_the_api(self): + """The dialog hides closed rows, but the whitelisted call is the real gate.""" + po = self.make_purchase_order() + closed_row = po.items[1] + + self.assertRaises( + frappe.ValidationError, + update_child_qty_rate, + "Purchase Order", + self.as_payload(po.items, **{closed_row.name: 99}), + po.name, + ) + + po.reload() + self.assertEqual(po.items[1].qty, 10) + + def test_unchanged_closed_row_in_the_payload_is_tolerated(self): + """A caller sending the whole table untouched should not be rejected.""" + po = self.make_purchase_order() + + update_child_qty_rate("Purchase Order", self.as_payload(po.items), po.name) + + po.reload() + self.assertEqual(len(po.items), 2) + self.assertTrue(po.items[1].closed) diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js index 852dac78ba3..ecd3aa8b4e3 100755 --- a/erpnext/public/js/utils.js +++ b/erpnext/public/js/utils.js @@ -730,24 +730,26 @@ erpnext.utils.update_child_items = function (opts) { const has_reserved_stock = opts.has_reserved_stock ? true : false; const get_precision = (fieldname) => child_meta.fields.find((f) => f.fieldname == fieldname).precision; - this.data = frm.doc[opts.child_docname].map((d) => { - return { - docname: d.name, - name: d.name, - item_code: d.item_code, - item_name: d.item_name, - delivery_date: d.delivery_date, - schedule_date: d.schedule_date, - conversion_factor: d.conversion_factor, - 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, - }; - }); + this.data = frm.doc[opts.child_docname] + .filter((d) => !d.closed) + .map((d) => { + return { + docname: d.name, + name: d.name, + item_code: d.item_code, + item_name: d.item_name, + delivery_date: d.delivery_date, + schedule_date: d.schedule_date, + conversion_factor: d.conversion_factor, + 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, + }; + }); const fields = [ {