From 27dffd916dad6306a69ad9234560a12442c655c2 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 29 Jul 2026 16:17:50 +0530 Subject: [PATCH] fix: normalise the closed flag and keep return rows closable Two issues from review. The endpoint stored whatever integer the caller passed, so `closed=2` was treated as closed by every truthy check while `validate_closed_source_items` looks for exactly 1. A submit-authorized caller could suppress a row from mapping and still submit an invoice against it. Verified: the value persisted as 2, truthy checks saw it as closed, the guard query did not find it. `is_item_closable` compared signed amounts, so an unbilled return row with a negative amount looked already completed and could not be closed. Verified on a real return Delivery Note: amount -500, billed_amt 0, predicate false. Magnitudes are compared instead, matching how the percentage funnel already treats these fields. --- erpnext/controllers/accounts_controller.py | 5 ++-- erpnext/controllers/item_close.py | 2 +- .../tests/test_item_close_billing.py | 24 +++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index cf4c953e412..24b54776017 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -215,9 +215,10 @@ class AccountsController(TransactionBase): """A row can be closed while anything is still pending on it. Billing is the axis every closable document shares; the order doctypes - extend this with their own fulfilment axis. + extend this with their own fulfilment axis. Amounts are compared as + magnitudes so return rows, which carry negative amounts, stay closable. """ - return flt(item.billed_amt) < flt(item.amount) + return abs(flt(item.billed_amt)) < abs(flt(item.amount)) def validate(self): clear_closed_rows_on_amend(self) diff --git a/erpnext/controllers/item_close.py b/erpnext/controllers/item_close.py index a777abc105d..13a9a40623e 100644 --- a/erpnext/controllers/item_close.py +++ b/erpnext/controllers/item_close.py @@ -47,7 +47,7 @@ def update_closed_status(doctype: str, name: str, item_names: str | list[str], c if not has_closable_items(doctype): frappe.throw(_("Rows of {0} cannot be closed individually").format(_(doctype))) - closed = cint(closed) + closed = 1 if cint(closed) else 0 item_names = set(frappe.parse_json(item_names) or []) if not item_names: frappe.throw(_("Select at least one row")) diff --git a/erpnext/controllers/tests/test_item_close_billing.py b/erpnext/controllers/tests/test_item_close_billing.py index a677b3d48fa..c10e10b02fb 100644 --- a/erpnext/controllers/tests/test_item_close_billing.py +++ b/erpnext/controllers/tests/test_item_close_billing.py @@ -4,6 +4,7 @@ import frappe from erpnext.controllers.item_close import update_closed_status +from erpnext.controllers.sales_and_purchase_return import make_return_doc from erpnext.stock.doctype.delivery_note.mapper import make_sales_invoice from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note from erpnext.stock.doctype.item.test_item import make_item @@ -177,3 +178,26 @@ class TestDeliveryNoteItemClose(ERPNextTestSuite): amended.insert() self.assertFalse(any(row.closed for row in amended.items)) + + def test_noncanonical_closed_value_is_normalised(self): + """A truthy non-1 value must not slip past the exact-match submission guard.""" + note = self.make_delivery_note() + + update_closed_status("Delivery Note", note.name, [note.items[1].name], 2) + + note.reload() + self.assertEqual(note.items[1].closed, 1) + + def test_unbilled_return_row_can_be_closed(self): + """Return rows carry negative amounts and must still be closable.""" + note = self.make_delivery_note() + return_note = make_return_doc("Delivery Note", note.name) + return_note.insert() + return_note.submit() + + row = return_note.items[0] + self.assertLess(row.amount, 0) + self.assertTrue(return_note.is_item_closable(row)) + + self.close_items(return_note, [row]) + self.assertTrue(return_note.items[0].closed)