diff --git a/erpnext/accounts/doctype/budget/budget.py b/erpnext/accounts/doctype/budget/budget.py index bceffd3627d..15e3ad06860 100644 --- a/erpnext/accounts/doctype/budget/budget.py +++ b/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")) ) diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.js b/erpnext/buying/doctype/purchase_order/purchase_order.js index 7d0c56d0aff..e8dcea1883e 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.js +++ b/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() { diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.py b/erpnext/buying/doctype/purchase_order/purchase_order.py index bbc35fe355f..d6edace0687 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.py +++ b/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() diff --git a/erpnext/buying/doctype/purchase_order/services/status.py b/erpnext/buying/doctype/purchase_order/services/status.py index cf799a71076..7f7ab402eb3 100644 --- a/erpnext/buying/doctype/purchase_order/services/status.py +++ b/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) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 0a02ab188e0..cf4c953e412 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/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) diff --git a/erpnext/controllers/item_close.py b/erpnext/controllers/item_close.py index f89b3c72a52..a777abc105d 100644 --- a/erpnext/controllers/item_close.py +++ b/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. diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index a108d507390..e0f1cd3bd94 100644 --- a/erpnext/controllers/status_updater.py +++ b/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 diff --git a/erpnext/controllers/tests/test_item_close.py b/erpnext/controllers/tests/test_item_close.py index 678ed6dfccf..821b3b74112 100644 --- a/erpnext/controllers/tests/test_item_close.py +++ b/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): diff --git a/erpnext/controllers/tests/test_item_close_billing.py b/erpnext/controllers/tests/test_item_close_billing.py index e9faa25dd72..a677b3d48fa 100644 --- a/erpnext/controllers/tests/test_item_close_billing.py +++ b/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): diff --git a/erpnext/public/js/utils/item_close.js b/erpnext/public/js/utils/item_close.js index cf08a76bc6a..f3fbfee25a3 100644 --- a/erpnext/public/js/utils/item_close.js +++ b/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, diff --git a/erpnext/selling/doctype/sales_order/mapper.py b/erpnext/selling/doctype/sales_order/mapper.py index d918686d0ab..8ca0fcfaae0 100644 --- a/erpnext/selling/doctype/sales_order/mapper.py +++ b/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) diff --git a/erpnext/selling/doctype/sales_order/sales_order.js b/erpnext/selling/doctype/sales_order/sales_order.js index 46600df4d63..03d95f4d155 100644 --- a/erpnext/selling/doctype/sales_order/sales_order.js +++ b/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; diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index 2911403c496..e92a248afb9 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/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.""" diff --git a/erpnext/selling/doctype/sales_order/services/status.py b/erpnext/selling/doctype/sales_order/services/status.py index bb6921cc232..6b7a9eae2d2 100644 --- a/erpnext/selling/doctype/sales_order/services/status.py +++ b/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): diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.js b/erpnext/stock/doctype/delivery_note/delivery_note.js index d3dec6d4c42..58608d804dd 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.js +++ b/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) { diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 2bb6ac70629..15ab95b0595 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/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) diff --git a/erpnext/stock/doctype/delivery_note/mapper.py b/erpnext/stock/doctype/delivery_note/mapper.py index 1138f0b7079..4ee9ccc13d4 100644 --- a/erpnext/stock/doctype/delivery_note/mapper.py +++ b/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 diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js index ae7fe9c5bee..a1a358ad5ac 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js +++ b/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() { diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 4bc6cdba155..c85f53ff1a8 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/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) diff --git a/erpnext/stock/doctype/purchase_receipt/services/billing_status.py b/erpnext/stock/doctype/purchase_receipt/services/billing_status.py index fe3f0825424..526be234ce5 100644 --- a/erpnext/stock/doctype/purchase_receipt/services/billing_status.py +++ b/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: