fix: keep progress percentages honest when a row is closed

A closed row was counted as fully settled, so closing one of two unbilled
Delivery Note rows pushed per_billed to 50 and the document read "Partially
Billed" with nothing invoiced. The same inflation applied to per_received,
per_delivered and per_picked.

Closed rows now leave the denominator instead of counting as done, so a
percentage stays a true measure of what was received, delivered or billed
against what is still expected. Writing off one of two unbilled rows leaves
per_billed at 0; billing the other takes it to 100. When every row is closed
nothing is outstanding, so the percentage is 100 by definition.

This mirrors how Delivery Note already excludes returned value from the
billing denominator.

Amending a cancelled document no longer inherits its write-offs. Frappe copies
no_copy fields when amending so a document can be corrected and resubmitted,
and unlike billed_amt or received_qty nothing recomputes `closed`, so the flag
would silently keep a row out of billing on the new document.
This commit is contained in:
Mihir Kandoi
2026-07-29 15:36:22 +05:30
parent 354708a54e
commit 992530c706
9 changed files with 88 additions and 28 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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