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.
This commit is contained in:
Mihir Kandoi
2026-07-29 16:17:50 +05:30
parent 1e5cc08b1d
commit 27dffd916d
3 changed files with 28 additions and 3 deletions

View File

@@ -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)

View File

@@ -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"))

View File

@@ -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)