refactor: share the row close plumbing across doctypes

Removes the duplication left over from adding the doctypes one at a time.

`is_bundle_of_closed_row` was copied between the Sales Order and Delivery Note
mappers, differing only in a doctype name; it now derives that from the packed
item's parenttype. `is_item_closable` was identical on Delivery Note and
Purchase Receipt and repeated the billing clause on the order doctypes; billing
is now the default on AccountsController and the orders add their own
fulfilment axis. The close dialog config was written out per doctype, differing
by a qty field, a column label and a sentence, and is now two builders in
erpnext/public/js/utils/item_close.js.

Also stops counting closed rows as committed spend in the budget's ordered
amount, which sums per row but only guarded the parent status.

When every row is closed there is nothing left to measure against, so the
percentage falls back to the whole table and reports what actually happened.
Writing off two unbilled rows leaves per_billed at 0; writing off two rows that
were fully received leaves per_received at 100 and per_billed at 0. A constant
would have been wrong in one direction or the other.

erpnext/accounts/doctype/budget/budget.py

@@ -729,6 +729,7 @@ def get_ordered_amount(params):
 			(child.item_code == item_code)
 			& (parent.docstatus == 1)
 			& (child.amount > child.billed_amt)
+			& (child.closed == 0)
 			& (parent.status != "Closed")
 			& Criterion.all(get_other_condition(params, child, parent, "Purchase Order"))
 		)

erpnext/buying/doctype/purchase_order/purchase_order.js

@@ -706,30 +706,16 @@ erpnext.buying.PurchaseOrderController = class PurchaseOrderController extends (
 	}
 
 	set_item_close_buttons() {
-		erpnext.item_close.add_buttons(this.frm, {
-			is_closable: (item) =>
-				!item.closed &&
-				(flt(item.received_qty) < flt(item.qty) || flt(item.billed_amt) < flt(item.amount)),
-			help: __(
-				"Closed rows stop being expected. Their pending quantity is written off and they are skipped when creating a Purchase Receipt or Purchase Invoice."
-			),
-			summarise: (item) => ({
-				item_code: item.item_code,
-				item_name: item.item_name,
-				qty: item.qty,
-				received_qty: item.received_qty || 0,
-				pending_qty: Math.max(flt(item.qty) - flt(item.received_qty), 0),
-				pending_amount: Math.max(flt(item.amount) - flt(item.billed_amt), 0),
-			}),
-			columns: [
-				erpnext.item_close.column("item_code", __("Item Code"), "Data", 3),
-				erpnext.item_close.column("item_name", __("Item Name"), "Data", 2),
-				erpnext.item_close.column("qty", __("Qty")),
-				erpnext.item_close.column("received_qty", __("Received Qty")),
-				erpnext.item_close.column("pending_qty", __("Pending Qty")),
-				erpnext.item_close.column("pending_amount", __("Pending Amount"), "Currency", 2),
-			],
-		});
+		erpnext.item_close.add_buttons(
+			this.frm,
+			erpnext.item_close.fulfilment_config({
+				qty_field: "received_qty",
+				qty_label: __("Received Qty"),
+				help: __(
+					"Closed rows stop being expected. Their pending quantity is written off and they are skipped when creating a Purchase Receipt or Purchase Invoice."
+				),
+			})
+		);
 	}
 
 	update_dropship_delivered_qty() {

erpnext/buying/doctype/purchase_order/purchase_order.py

@@ -373,7 +373,7 @@ class PurchaseOrder(BuyingController):
 		StatusService(self).recalculate_after_item_close()
 
 	def is_item_closable(self, item):
-		return flt(item.received_qty) < flt(item.qty) or flt(item.billed_amt) < flt(item.amount)
+		return flt(item.received_qty) < flt(item.qty) or super().is_item_closable(item)
 
 	def on_submit(self):
 		super().on_submit()

erpnext/buying/doctype/purchase_order/services/status.py

@@ -55,17 +55,9 @@ class StatusService:
 	def update_receiving_percentage(self) -> None:
 		doc = self.doc
 		total_qty, received_qty = 0.0, 0.0
-		for item in doc.items:
-			if item.closed:
-				continue
+		for item in [item for item in doc.items if not item.closed] or doc.items:
 			received_qty += min(item.received_qty, item.qty)
 			total_qty += item.qty
 
-		if total_qty and received_qty:
-			per_received = flt(received_qty / total_qty) * 100
-		elif doc.items and not total_qty:
-			per_received = 100
-		else:
-			per_received = 0
-
+		per_received = flt(received_qty / total_qty) * 100 if total_qty else 0
 		doc.db_set("per_received", per_received, update_modified=False)

erpnext/controllers/accounts_controller.py

@@ -211,6 +211,14 @@ class AccountsController(TransactionBase):
 				)
 				frappe.msgprint(msg)
 
+	def is_item_closable(self, item):
+		"""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.
+		"""
+		return flt(item.billed_amt) < flt(item.amount)
+
 	def validate(self):
 		clear_closed_rows_on_amend(self)
 

erpnext/controllers/item_close.py

@@ -106,6 +106,16 @@ def reopen_parent_if_closed(doc) -> None:
 		doc.update_status(REOPEN_STATUS[doc.doctype])
 
 
+def is_bundle_of_closed_row(packed_item) -> bool:
+	"""A packed item follows the row of its parent document that bundles it."""
+	if not packed_item.parent_detail_docname or not packed_item.parenttype:
+		return False
+
+	item_doctype = f"{packed_item.parenttype} Item"
+
+	return bool(frappe.db.get_value(item_doctype, packed_item.parent_detail_docname, "closed"))
+
+
 def clear_closed_rows_on_amend(doc) -> None:
 	"""An amended document starts with nothing written off.
 

erpnext/controllers/status_updater.py

@@ -677,20 +677,18 @@ class StatusUpdater(Document):
 		# 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.
+		# Once every row is written off there is nothing left to measure against,
+		# so fall back to the whole table and report what actually happened.
 		open_records = [r for r in child_records if not (tracks_closed_rows and r["closed"])]
+		basis = open_records or child_records
 
-		sum_ref = sum(abs(record[ref_key]) for record in open_records)
+		sum_ref = sum(abs(record[ref_key]) for record in basis)
 
 		if sum_ref > 0:
 			percentage = round(
-				sum(min(abs(record[target_field]), abs(record[ref_key])) for record in open_records)
-				/ sum_ref
-				* 100,
+				sum(min(abs(record[target_field]), abs(record[ref_key])) for record in basis) / sum_ref * 100,
 				6,
 			)
-		elif child_records and not open_records:
-			# every row written off, so nothing is outstanding
-			percentage = 100
 		else:
 			percentage = 0
 

erpnext/controllers/tests/test_item_close.py

@@ -143,7 +143,9 @@ class TestPurchaseOrderItemClose(ERPNextTestSuite):
 
 		self.close_items(po, po.items)
 
-		self.assertEqual(po.per_billed, 100)
+		# billing written off, but the goods really did arrive
+		self.assertEqual(po.per_billed, 0)
+		self.assertEqual(po.per_received, 100)
 		self.assertEqual(po.status, "Closed")
 
 	def test_receipt_is_not_offered_when_the_rest_is_closed(self):

erpnext/controllers/tests/test_item_close_billing.py

@@ -56,7 +56,8 @@ class TestPurchaseReceiptItemClose(ERPNextTestSuite):
 
 		self.close_items(receipt, receipt.items)
 
-		self.assertEqual(receipt.per_billed, 100)
+		# nothing was billed, and writing every row off must not claim otherwise
+		self.assertEqual(receipt.per_billed, 0)
 		self.assertEqual(receipt.status, "Closed")
 
 	def test_closed_row_is_not_mapped_to_purchase_invoice(self):
@@ -135,7 +136,8 @@ class TestDeliveryNoteItemClose(ERPNextTestSuite):
 
 		self.close_items(note, note.items)
 
-		self.assertEqual(note.per_billed, 100)
+		# nothing was billed, and writing every row off must not claim otherwise
+		self.assertEqual(note.per_billed, 0)
 		self.assertEqual(note.status, "Closed")
 
 	def test_closed_row_is_not_mapped_to_sales_invoice(self):

erpnext/public/js/utils/item_close.js

@@ -76,6 +76,56 @@ erpnext.item_close = {
 		dialog.show();
 	},
 
+	fulfilment_config({ qty_field, qty_label, help }) {
+		return {
+			is_closable: (item) =>
+				!item.closed &&
+				(flt(item[qty_field]) < flt(item.qty) || flt(item.billed_amt) < flt(item.amount)),
+			help: help,
+			summarise: (item) => ({
+				item_code: item.item_code,
+				item_name: item.item_name,
+				qty: item.qty,
+				fulfilled_qty: item[qty_field] || 0,
+				pending_qty: Math.max(flt(item.qty) - flt(item[qty_field]), 0),
+				pending_amount: Math.max(flt(item.amount) - flt(item.billed_amt), 0),
+			}),
+			columns: [
+				erpnext.item_close.column("item_code", __("Item Code"), "Data", 3),
+				erpnext.item_close.column("item_name", __("Item Name"), "Data", 2),
+				erpnext.item_close.column("qty", __("Qty")),
+				erpnext.item_close.column("fulfilled_qty", qty_label),
+				erpnext.item_close.column("pending_qty", __("Pending Qty")),
+				erpnext.item_close.column("pending_amount", __("Pending Amount"), "Currency", 2),
+			],
+		};
+	},
+
+	billing_config(invoice_label) {
+		return {
+			is_closable: (item) => !item.closed && flt(item.billed_amt) < flt(item.amount),
+			help: __(
+				"Closed rows stop being expected. Their unbilled amount is written off and they are skipped when creating a {0}.",
+				[invoice_label]
+			),
+			summarise: (item) => ({
+				item_code: item.item_code,
+				item_name: item.item_name,
+				qty: item.qty,
+				amount: item.amount,
+				billed_amt: item.billed_amt || 0,
+				pending_amount: Math.max(flt(item.amount) - flt(item.billed_amt), 0),
+			}),
+			columns: [
+				erpnext.item_close.column("item_code", __("Item Code"), "Data", 3),
+				erpnext.item_close.column("item_name", __("Item Name"), "Data", 2),
+				erpnext.item_close.column("qty", __("Qty")),
+				erpnext.item_close.column("amount", __("Amount"), "Currency", 2),
+				erpnext.item_close.column("pending_amount", __("Pending Amount"), "Currency", 2),
+			],
+		};
+	},
+
 	column(fieldname, label, fieldtype = "Float", columns = 1) {
 		return {
 			fieldname: fieldname,

erpnext/selling/doctype/sales_order/mapper.py

@@ -13,6 +13,7 @@ from frappe.query_builder.functions import Sum
 from frappe.utils import add_days, cint, flt, nowdate, strip_html
 
 from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_party_account
+from erpnext.controllers.item_close import is_bundle_of_closed_row
 from erpnext.manufacturing.doctype.production_plan.production_plan import (
 	get_items_for_material_requests,
 	get_sales_orders,
@@ -48,14 +49,6 @@ def get_requested_item_qty(sales_order: str) -> dict:
 	return result
 
 
-def is_bundle_of_closed_row(packed_item) -> bool:
-	"""A packed item follows the Sales Order Item row that bundles it."""
-	return bool(
-		packed_item.parent_detail_docname
-		and frappe.db.get_value("Sales Order Item", packed_item.parent_detail_docname, "closed")
-	)
-
-
 @frappe.whitelist()
 def make_material_request(source_name: str, target_doc: str | dict | Document | None = None):
 	requested_item_qty = get_requested_item_qty(source_name)

erpnext/selling/doctype/sales_order/sales_order.js

@@ -1886,30 +1886,16 @@ erpnext.selling.SalesOrderController = class SalesOrderController extends erpnex
 	}
 
 	set_item_close_buttons() {
-		erpnext.item_close.add_buttons(this.frm, {
-			is_closable: (item) =>
-				!item.closed &&
-				(flt(item.delivered_qty) < flt(item.qty) || flt(item.billed_amt) < flt(item.amount)),
-			help: __(
-				"Closed rows stop being expected. Their pending quantity is written off, stock is no longer reserved for them, and they are skipped when creating a Delivery Note or Sales Invoice."
-			),
-			summarise: (item) => ({
-				item_code: item.item_code,
-				item_name: item.item_name,
-				qty: item.qty,
-				delivered_qty: item.delivered_qty || 0,
-				pending_qty: Math.max(flt(item.qty) - flt(item.delivered_qty), 0),
-				pending_amount: Math.max(flt(item.amount) - flt(item.billed_amt), 0),
-			}),
-			columns: [
-				erpnext.item_close.column("item_code", __("Item Code"), "Data", 3),
-				erpnext.item_close.column("item_name", __("Item Name"), "Data", 2),
-				erpnext.item_close.column("qty", __("Qty")),
-				erpnext.item_close.column("delivered_qty", __("Delivered Qty")),
-				erpnext.item_close.column("pending_qty", __("Pending Qty")),
-				erpnext.item_close.column("pending_amount", __("Pending Amount"), "Currency", 2),
-			],
-		});
+		erpnext.item_close.add_buttons(
+			this.frm,
+			erpnext.item_close.fulfilment_config({
+				qty_field: "delivered_qty",
+				qty_label: __("Delivered Qty"),
+				help: __(
+					"Closed rows stop being expected. Their pending quantity is written off, stock is no longer reserved for them, and they are skipped when creating a Delivery Note or Sales Invoice."
+				),
+			})
+		);
 	}
 	update_status(label, status) {
 		var doc = this.frm.doc;

erpnext/selling/doctype/sales_order/sales_order.py

@@ -545,7 +545,7 @@ class SalesOrder(SellingController):
 		StatusService(self).recalculate_after_item_close()
 
 	def is_item_closable(self, item):
-		return flt(item.delivered_qty) < flt(item.qty) or flt(item.billed_amt) < flt(item.amount)
+		return flt(item.delivered_qty) < flt(item.qty) or super().is_item_closable(item)
 
 	def validate_item_close(self, items):
 		"""Reserved stock has to be released deliberately before a row is closed."""

erpnext/selling/doctype/sales_order/services/status.py

@@ -118,9 +118,7 @@ class StatusService:
 		total_qty = 0.0
 		per_picked = 0.0
 
-		for so_item in doc.items:
-			if so_item.closed:
-				continue
+		for so_item in [item for item in doc.items if not item.closed] or doc.items:
 			if cint(
 				frappe.get_cached_value("Item", so_item.item_code, "is_stock_item")
 			) or doc.has_product_bundle(so_item.item_code):

erpnext/stock/doctype/delivery_note/delivery_note.js

@@ -439,27 +439,7 @@ erpnext.stock.DeliveryNoteController = class DeliveryNoteController extends (
 	}
 
 	set_item_close_buttons() {
-		erpnext.item_close.add_buttons(this.frm, {
-			is_closable: (item) => !item.closed && flt(item.billed_amt) < flt(item.amount),
-			help: __(
-				"Closed rows stop being expected. Their unbilled amount is written off and they are skipped when creating a Sales Invoice."
-			),
-			summarise: (item) => ({
-				item_code: item.item_code,
-				item_name: item.item_name,
-				qty: item.qty,
-				amount: item.amount,
-				billed_amt: item.billed_amt || 0,
-				pending_amount: Math.max(flt(item.amount) - flt(item.billed_amt), 0),
-			}),
-			columns: [
-				erpnext.item_close.column("item_code", __("Item Code"), "Data", 3),
-				erpnext.item_close.column("item_name", __("Item Name"), "Data", 2),
-				erpnext.item_close.column("qty", __("Qty")),
-				erpnext.item_close.column("amount", __("Amount"), "Currency", 2),
-				erpnext.item_close.column("pending_amount", __("Pending Amount"), "Currency", 2),
-			],
-		});
+		erpnext.item_close.add_buttons(this.frm, erpnext.item_close.billing_config(__("Sales Invoice")));
 	}
 
 	update_status(status) {

erpnext/stock/doctype/delivery_note/delivery_note.py

@@ -645,9 +645,6 @@ class DeliveryNote(SellingController):
 	def on_item_close_status_change(self):
 		self.update_billing_percentage()
 
-	def is_item_closable(self, item):
-		return flt(item.billed_amt) < flt(item.amount)
-
 	def update_billing_status(self, update_modified=True):
 		BillingStatusService(self).update_billing_status(update_modified)
 

erpnext/stock/doctype/delivery_note/mapper.py

@@ -15,6 +15,7 @@ from frappe.utils import flt
 
 from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_due_date
 from erpnext.controllers.accounts_controller import get_taxes_and_charges, merge_taxes
+from erpnext.controllers.item_close import is_bundle_of_closed_row
 from erpnext.stock.doctype.packed_item.packed_item import is_product_bundle
 
 
@@ -58,14 +59,6 @@ def get_returned_qty_map(delivery_note: str) -> dict:
 	return returned_qty_map
 
 
-def is_bundle_of_closed_row(packed_item) -> bool:
-	"""A packed item follows the Delivery Note Item row that bundles it."""
-	return bool(
-		packed_item.parent_detail_docname
-		and frappe.db.get_value("Delivery Note Item", packed_item.parent_detail_docname, "closed")
-	)
-
-
 @frappe.whitelist()
 def make_sales_invoice(
 	source_name: str, target_doc: str | dict | Document | None = None, args: dict | str | None = None

erpnext/stock/doctype/purchase_receipt/purchase_receipt.js

@@ -292,27 +292,7 @@ erpnext.stock.PurchaseReceiptController = class PurchaseReceiptController extend
 	}
 
 	set_item_close_buttons() {
-		erpnext.item_close.add_buttons(this.frm, {
-			is_closable: (item) => !item.closed && flt(item.billed_amt) < flt(item.amount),
-			help: __(
-				"Closed rows stop being expected. Their unbilled amount is written off and they are skipped when creating a Purchase Invoice."
-			),
-			summarise: (item) => ({
-				item_code: item.item_code,
-				item_name: item.item_name,
-				qty: item.qty,
-				amount: item.amount,
-				billed_amt: item.billed_amt || 0,
-				pending_amount: Math.max(flt(item.amount) - flt(item.billed_amt), 0),
-			}),
-			columns: [
-				erpnext.item_close.column("item_code", __("Item Code"), "Data", 3),
-				erpnext.item_close.column("item_name", __("Item Name"), "Data", 2),
-				erpnext.item_close.column("qty", __("Qty")),
-				erpnext.item_close.column("amount", __("Amount"), "Currency", 2),
-				erpnext.item_close.column("pending_amount", __("Pending Amount"), "Currency", 2),
-			],
-		});
+		erpnext.item_close.add_buttons(this.frm, erpnext.item_close.billing_config(__("Purchase Invoice")));
 	}
 
 	make_purchase_invoice() {

erpnext/stock/doctype/purchase_receipt/purchase_receipt.py

@@ -530,9 +530,6 @@ class PurchaseReceipt(BuyingController):
 	def on_item_close_status_change(self):
 		self.update_billing_status()
 
-	def is_item_closable(self, item):
-		return flt(item.billed_amt) < flt(item.amount)
-
 	def update_billing_status(self, update_modified=True):
 		BillingStatusService(self).update_billing_status(update_modified)
 

erpnext/stock/doctype/purchase_receipt/services/billing_status.py

@@ -186,10 +186,7 @@ def update_billing_percentage(
 		billed_qty_amt = get_billed_qty_amount_against_purchase_receipt(pr_doc)
 		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
-
+	for item in [item for item in pr_doc.items if not item.closed] or pr_doc.items:
 		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
@@ -274,10 +271,7 @@ def update_billing_percentage(
 	if pi_landed_cost_amount < 0:
 		total_billed_amount += abs(pi_landed_cost_amount)
 
-	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)
+	percent_billed = round(100 * (total_billed_amount / (total_amount or 1)), 6)
 	pr_doc.db_set("per_billed", percent_billed)
 
 	if update_modified: