From 11d7f02a51578fa9a5c899d5b2cbbefc175436e7 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 29 Jul 2026 16:35:56 +0530 Subject: [PATCH] test: make return row closure a stated rule Whether a return row could be closed was decided by the sign of its amount rather than by anyone's intent: the signed comparison happened to reject it, and switching to magnitudes happened to allow it. Closing a whole return document was already allowed, so allowing the row is the consistent choice, but it should be written down. `is_item_closable` now says so, and the behaviour is covered on both Delivery Note and Purchase Receipt returns rather than only the one that was reported. Also asserts the property worth protecting: closing a row on a return leaves the original document's returned qty and per_returned untouched. --- erpnext/controllers/accounts_controller.py | 9 ++++-- .../tests/test_item_close_billing.py | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 24b54776017..d7dc8f63bbd 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -215,8 +215,13 @@ 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. Amounts are compared as - magnitudes so return rows, which carry negative amounts, stay closable. + extend this with their own fulfilment axis. + + Amounts are compared as magnitudes so that return rows stay closable. + That is deliberate: writing off a credit note that will never be issued + is a real decision, and closing a whole return document is already + allowed. Leaving it to the sign of the amount would decide it by + accident. """ return abs(flt(item.billed_amt)) < abs(flt(item.amount)) diff --git a/erpnext/controllers/tests/test_item_close_billing.py b/erpnext/controllers/tests/test_item_close_billing.py index 517dc697984..3acaa2b4699 100644 --- a/erpnext/controllers/tests/test_item_close_billing.py +++ b/erpnext/controllers/tests/test_item_close_billing.py @@ -94,6 +94,20 @@ class TestPurchaseReceiptItemClose(ERPNextTestSuite): self.assertNotEqual(receipt.status, "Closed") self.assertEqual(receipt.per_billed, 0) + def test_unbilled_return_row_can_be_closed(self): + """Return rows are closable by design, not by an accident of sign.""" + receipt = self.make_purchase_receipt() + return_receipt = make_return_doc("Purchase Receipt", receipt.name) + return_receipt.insert() + return_receipt.submit() + + row = return_receipt.items[0] + self.assertLess(row.amount, 0) + self.assertTrue(return_receipt.is_item_closable(row)) + + self.close_items(return_receipt, [row]) + self.assertTrue(return_receipt.items[0].closed) + class TestDeliveryNoteItemClose(ERPNextTestSuite): def setUp(self): @@ -215,3 +229,20 @@ class TestDeliveryNoteItemClose(ERPNextTestSuite): pending = abs(flt(row.amount)) - abs(flt(row.billed_amt)) self.assertEqual(pending, abs(flt(note.items[0].amount))) self.assertGreater(pending, 0) + + def test_closing_a_return_row_leaves_the_original_untouched(self): + """Writing off a credit note must not disturb what was returned.""" + note = self.make_delivery_note() + return_note = make_return_doc("Delivery Note", note.name) + return_note.insert() + return_note.submit() + + note.reload() + before = [(row.returned_qty, row.closed) for row in note.items] + per_returned_before = note.per_returned + + self.close_items(return_note, [return_note.items[0]]) + + note.reload() + self.assertEqual([(row.returned_qty, row.closed) for row in note.items], before) + self.assertEqual(note.per_returned, per_returned_before)