mirror of
https://github.com/frappe/erpnext.git
synced 2026-09-02 08:03:21 +00:00
fix: keep closed rows out of Update Items (#58609)
This commit is contained in:
@@ -82,6 +82,13 @@ class ChildItemUpdater:
|
|||||||
if is_child_item_unchanged(change_state):
|
if is_child_item_unchanged(change_state):
|
||||||
continue
|
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)
|
self._validate_quantity_and_rate(child_item, d, rate_unchanged)
|
||||||
|
|
||||||
if flt(child_item.get("qty")) != flt(d.get("qty")):
|
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:
|
def validate_and_delete_children(parent, data, ordered_item=None) -> bool:
|
||||||
"""Delete child rows not present in data; return True if any were removed."""
|
"""Delete child rows not present in data; return True if any were removed."""
|
||||||
updated_item_names = [d.get("docname") for d in data]
|
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:
|
for d in deleted_children:
|
||||||
validate_child_on_delete(d, parent, ordered_item)
|
validate_child_on_delete(d, parent, ordered_item)
|
||||||
|
|||||||
96
erpnext/controllers/tests/test_item_close_update_items.py
Normal file
96
erpnext/controllers/tests/test_item_close_update_items.py
Normal file
@@ -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)
|
||||||
@@ -730,7 +730,9 @@ erpnext.utils.update_child_items = function (opts) {
|
|||||||
const has_reserved_stock = opts.has_reserved_stock ? true : false;
|
const has_reserved_stock = opts.has_reserved_stock ? true : false;
|
||||||
const get_precision = (fieldname) => child_meta.fields.find((f) => f.fieldname == fieldname).precision;
|
const get_precision = (fieldname) => child_meta.fields.find((f) => f.fieldname == fieldname).precision;
|
||||||
|
|
||||||
this.data = frm.doc[opts.child_docname].map((d) => {
|
this.data = frm.doc[opts.child_docname]
|
||||||
|
.filter((d) => !d.closed)
|
||||||
|
.map((d) => {
|
||||||
return {
|
return {
|
||||||
docname: d.name,
|
docname: d.name,
|
||||||
name: d.name,
|
name: d.name,
|
||||||
|
|||||||
Reference in New Issue
Block a user