diff --git a/erpnext/buying/doctype/purchase_order/services/status.py b/erpnext/buying/doctype/purchase_order/services/status.py index b9c43a5652e..cf799a71076 100644 --- a/erpnext/buying/doctype/purchase_order/services/status.py +++ b/erpnext/buying/doctype/purchase_order/services/status.py @@ -56,9 +56,16 @@ class StatusService: doc = self.doc total_qty, received_qty = 0.0, 0.0 for item in doc.items: - received_qty += item.qty if item.closed else min(item.received_qty, item.qty) + if item.closed: + continue + received_qty += min(item.received_qty, item.qty) total_qty += item.qty + if total_qty and received_qty: - doc.db_set("per_received", flt(received_qty / total_qty) * 100, update_modified=False) + per_received = flt(received_qty / total_qty) * 100 + elif doc.items and not total_qty: + per_received = 100 else: - doc.db_set("per_received", 0, update_modified=False) + per_received = 0 + + doc.db_set("per_received", per_received, update_modified=False) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 56e6e381bb5..0a02ab188e0 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -39,6 +39,7 @@ from erpnext.accounts.utils import ( get_advance_payment_doctypes as _get_advance_payment_doctypes, ) from erpnext.accounts.utils import get_fiscal_year, validate_fiscal_year +from erpnext.controllers.item_close import clear_closed_rows_on_amend from erpnext.controllers.print_settings import ( set_print_templates_for_item_table, set_print_templates_for_taxes, @@ -211,6 +212,8 @@ class AccountsController(TransactionBase): frappe.msgprint(msg) def validate(self): + clear_closed_rows_on_amend(self) + if not self.get("is_return") and not self.get("is_debit_note"): self.validate_qty_is_not_zero() diff --git a/erpnext/controllers/item_close.py b/erpnext/controllers/item_close.py index 911f9e907d5..f89b3c72a52 100644 --- a/erpnext/controllers/item_close.py +++ b/erpnext/controllers/item_close.py @@ -106,6 +106,20 @@ def reopen_parent_if_closed(doc) -> None: doc.update_status(REOPEN_STATUS[doc.doctype]) +def clear_closed_rows_on_amend(doc) -> None: + """An amended document starts with nothing written off. + + Frappe copies `no_copy` fields when amending so a cancelled document can be + corrected and resubmitted, which would otherwise carry a write-off decision + that was made against the cancelled document onto the new one. + """ + if not doc.is_new() or not doc.get("amended_from") or not has_closable_items(doc.doctype): + return + + for row in doc.get("items") or []: + row.closed = 0 + + def validate_parent_reopen(doc) -> None: """Block reopening a parent whose rows are all closed. diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index 8cea29bed61..a108d507390 100644 --- a/erpnext/controllers/status_updater.py +++ b/erpnext/controllers/status_updater.py @@ -674,20 +674,23 @@ class StatusUpdater(Document): # For operator dicts, the alias is in the "as" key; for strings, use the field name directly ref_key = target_ref_field.get("as") if isinstance(target_ref_field, dict) else target_ref_field - def settled(record): - """A closed row is settled in full, so it stops holding the parent open.""" - if tracks_closed_rows and record["closed"]: - return abs(record[ref_key]) + # A closed row is written off, so it leaves the denominator rather than + # counting as done. The percentage stays a true measure of what was + # actually received, delivered or billed against what is still expected. + open_records = [r for r in child_records if not (tracks_closed_rows and r["closed"])] - return min(abs(record[target_field]), abs(record[ref_key])) - - sum_ref = sum(abs(record[ref_key]) for record in child_records) + sum_ref = sum(abs(record[ref_key]) for record in open_records) if sum_ref > 0: percentage = round( - sum(settled(record) for record in child_records) / sum_ref * 100, + sum(min(abs(record[target_field]), abs(record[ref_key])) for record in open_records) + / sum_ref + * 100, 6, ) + elif child_records and not open_records: + # every row written off, so nothing is outstanding + percentage = 100 else: percentage = 0 diff --git a/erpnext/controllers/tests/test_item_close.py b/erpnext/controllers/tests/test_item_close.py index 4daa6f8b7ea..678ed6dfccf 100644 --- a/erpnext/controllers/tests/test_item_close.py +++ b/erpnext/controllers/tests/test_item_close.py @@ -17,9 +17,7 @@ WAREHOUSE = "_Test Warehouse - _TC" def get_ordered_qty(item_code): - return flt( - frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": WAREHOUSE}, "ordered_qty") - ) + return flt(frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": WAREHOUSE}, "ordered_qty")) class TestPurchaseOrderItemClose(ERPNextTestSuite): @@ -114,7 +112,8 @@ class TestPurchaseOrderItemClose(ERPNextTestSuite): self.assertEqual(po.status, "To Receive and Bill") self.assertTrue(po.items[0].closed) self.assertFalse(po.items[1].closed) - self.assertEqual(po.per_received, 50) + # nothing received, and the closed row is written off rather than counted + self.assertEqual(po.per_received, 0) self.assertEqual(get_ordered_qty(self.second_item), 10) self.assertEqual(get_ordered_qty(self.first_item), 0) @@ -215,3 +214,16 @@ class TestPurchaseOrderItemClose(ERPNextTestSuite): ["any-row"], 1, ) + + def test_amending_clears_closed_rows(self): + """Frappe keeps no_copy fields when amending, so the flag must be cleared.""" + po = self.make_purchase_order() + self.close_items(po, [po.items[1]]) + po.cancel() + + amended = frappe.copy_doc(po, ignore_no_copy=True) + amended.docstatus = 0 + amended.amended_from = po.name + amended.insert() + + self.assertFalse(any(row.closed for row in amended.items)) diff --git a/erpnext/controllers/tests/test_item_close_billing.py b/erpnext/controllers/tests/test_item_close_billing.py index d298fe89b27..e9faa25dd72 100644 --- a/erpnext/controllers/tests/test_item_close_billing.py +++ b/erpnext/controllers/tests/test_item_close_billing.py @@ -41,13 +41,15 @@ class TestPurchaseReceiptItemClose(ERPNextTestSuite): update_closed_status(doc.doctype, doc.name, [row.name for row in rows], closed) doc.reload() - def test_closing_row_settles_billing_percentage(self): + def test_closing_a_row_does_not_inflate_billing_percentage(self): receipt = self.make_purchase_receipt() self.assertEqual(receipt.per_billed, 0) self.close_items(receipt, [receipt.items[1]]) - self.assertEqual(receipt.per_billed, 50) + # nothing was billed, so the receipt must not read as partly billed + self.assertEqual(receipt.per_billed, 0) + self.assertEqual(receipt.status, "To Bill") def test_closing_every_row_closes_the_receipt(self): receipt = self.make_purchase_receipt() @@ -87,7 +89,7 @@ class TestPurchaseReceiptItemClose(ERPNextTestSuite): self.close_items(receipt, [receipt.items[1]], closed=0) self.assertNotEqual(receipt.status, "Closed") - self.assertEqual(receipt.per_billed, 50) + self.assertEqual(receipt.per_billed, 0) class TestDeliveryNoteItemClose(ERPNextTestSuite): @@ -118,13 +120,15 @@ class TestDeliveryNoteItemClose(ERPNextTestSuite): update_closed_status(doc.doctype, doc.name, [row.name for row in rows], closed) doc.reload() - def test_closing_row_settles_billing_percentage(self): + def test_closing_a_row_does_not_inflate_billing_percentage(self): note = self.make_delivery_note() self.assertEqual(note.per_billed, 0) self.close_items(note, [note.items[1]]) - self.assertEqual(note.per_billed, 50) + # nothing was billed, so the note must not read as partially billed + self.assertEqual(note.per_billed, 0) + self.assertEqual(note.status, "To Bill") def test_closing_every_row_closes_the_note(self): note = self.make_delivery_note() @@ -158,3 +162,16 @@ class TestDeliveryNoteItemClose(ERPNextTestSuite): self.assertEqual(note.per_returned, 0) self.assertEqual(note.status, "Closed") + + def test_amending_clears_closed_rows(self): + """Frappe keeps no_copy fields when amending, so the flag must be cleared.""" + note = self.make_delivery_note() + self.close_items(note, [note.items[1]]) + note.cancel() + + amended = frappe.copy_doc(note, ignore_no_copy=True) + amended.docstatus = 0 + amended.amended_from = note.name + amended.insert() + + self.assertFalse(any(row.closed for row in amended.items)) diff --git a/erpnext/controllers/tests/test_item_close_sales_order.py b/erpnext/controllers/tests/test_item_close_sales_order.py index fe233daff84..ad45d6ead26 100644 --- a/erpnext/controllers/tests/test_item_close_sales_order.py +++ b/erpnext/controllers/tests/test_item_close_sales_order.py @@ -15,9 +15,7 @@ WAREHOUSE = "_Test Warehouse - _TC" def get_reserved_qty(item_code): - return flt( - frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": WAREHOUSE}, "reserved_qty") - ) + return flt(frappe.db.get_value("Bin", {"item_code": item_code, "warehouse": WAREHOUSE}, "reserved_qty")) class TestSalesOrderItemClose(ERPNextTestSuite): diff --git a/erpnext/selling/doctype/sales_order/services/status.py b/erpnext/selling/doctype/sales_order/services/status.py index 142cfe01adb..bb6921cc232 100644 --- a/erpnext/selling/doctype/sales_order/services/status.py +++ b/erpnext/selling/doctype/sales_order/services/status.py @@ -119,12 +119,12 @@ class StatusService: per_picked = 0.0 for so_item in doc.items: + if so_item.closed: + continue if cint( frappe.get_cached_value("Item", so_item.item_code, "is_stock_item") ) or doc.has_product_bundle(so_item.item_code): - total_picked_qty += ( - flt(so_item.stock_qty) if so_item.closed else flt(so_item.picked_qty) - ) + total_picked_qty += flt(so_item.picked_qty) total_qty += flt(so_item.stock_qty) if total_picked_qty and total_qty: diff --git a/erpnext/stock/doctype/purchase_receipt/services/billing_status.py b/erpnext/stock/doctype/purchase_receipt/services/billing_status.py index 160d6085945..fe3f0825424 100644 --- a/erpnext/stock/doctype/purchase_receipt/services/billing_status.py +++ b/erpnext/stock/doctype/purchase_receipt/services/billing_status.py @@ -187,6 +187,9 @@ def update_billing_percentage( billed_qty_amt_based_on_po = get_billed_qty_amount_against_purchase_order(pr_doc) for item in pr_doc.items: + if item.closed: + continue + returned_qty = flt(item_wise_returned_qty.get(item.name)) returned_amount = flt(returned_qty) * flt(item.rate) pending_amount = flt(item.amount) - returned_amount @@ -198,7 +201,7 @@ def update_billing_percentage( total_billable_amount = pending_amount if item.billed_amt <= pending_amount else item.billed_amt total_amount += total_billable_amount - total_billed_amount += total_billable_amount if item.closed else abs(flt(item.billed_amt)) + total_billed_amount += abs(flt(item.billed_amt)) if pr_doc.get("is_return") and not total_amount and total_billed_amount: total_amount = total_billed_amount @@ -271,7 +274,10 @@ def update_billing_percentage( if pi_landed_cost_amount < 0: total_billed_amount += abs(pi_landed_cost_amount) - percent_billed = round(100 * (total_billed_amount / (total_amount or 1)), 6) + if not total_amount and pr_doc.items and all(item.closed for item in pr_doc.items): + percent_billed = 100 + else: + percent_billed = round(100 * (total_billed_amount / (total_amount or 1)), 6) pr_doc.db_set("per_billed", percent_billed) if update_modified: