From 5074597d00fbb27779c5eb477ad266241b5b7ea6 Mon Sep 17 00:00:00 2001 From: Shubh Doshi <124681920+shubhdoshi21@users.noreply.github.com> Date: Wed, 3 Jun 2026 11:50:49 +0530 Subject: [PATCH] perf: batch status check for on-hold/closed documents, remove N+1 queries (#54798) --- .../purchase_invoice/purchase_invoice.py | 17 +++------ .../doctype/purchase_order/purchase_order.py | 18 ++------- erpnext/buying/utils.py | 9 ++++- erpnext/controllers/buying_controller.py | 13 ------- erpnext/controllers/selling_controller.py | 8 ++-- erpnext/controllers/stock_controller.py | 37 +++++++++++++++++++ .../doctype/work_order/work_order.py | 8 +--- .../purchase_receipt/purchase_receipt.py | 13 +------ .../subcontracting_receipt.py | 5 +-- 9 files changed, 62 insertions(+), 66 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 8b417584e35..5a7e6bdef5a 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -36,7 +36,6 @@ from erpnext.accounts.party import get_due_date, get_party_account from erpnext.accounts.utils import get_account_currency, get_fiscal_year, update_voucher_outstanding from erpnext.assets.doctype.asset.asset import is_cwip_accounting_enabled from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account -from erpnext.buying.utils import check_on_hold_or_closed_status from erpnext.controllers.accounts_controller import merge_taxes, validate_account_head from erpnext.controllers.buying_controller import BuyingController from erpnext.stock.doctype.purchase_receipt.purchase_receipt import ( @@ -282,7 +281,9 @@ class PurchaseInvoice(BuyingController): self.check_conversion_rate() self.validate_credit_to_acc() self.clear_unallocated_advances("Purchase Invoice Advance", "advances") - self.check_on_hold_or_closed_status() + self.check_for_on_hold_or_closed_status( + "Purchase Order", "purchase_order", exclude_if_field="purchase_receipt" + ) self.validate_with_previous_doc() self.validate_uom_is_integer("uom", "qty") self.validate_uom_is_integer("stock_uom", "stock_qty") @@ -387,14 +388,6 @@ class PurchaseInvoice(BuyingController): self.party_account_currency = account.account_currency - def check_on_hold_or_closed_status(self): - check_list = [] - - for d in self.get("items"): - if d.purchase_order and d.purchase_order not in check_list and not d.purchase_receipt: - check_list.append(d.purchase_order) - check_on_hold_or_closed_status("Purchase Order", d.purchase_order) - def validate_with_previous_doc(self): super().validate_with_previous_doc( { @@ -1681,7 +1674,9 @@ class PurchaseInvoice(BuyingController): super().on_cancel() PurchaseTaxWithholding(self).on_cancel() - self.check_on_hold_or_closed_status() + self.check_for_on_hold_or_closed_status( + "Purchase Order", "purchase_order", exclude_if_field="purchase_receipt" + ) if self.is_return and not self.update_billed_amount_in_purchase_order: # NOTE status updating bypassed for is_return diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.py b/erpnext/buying/doctype/purchase_order/purchase_order.py index 26458f6275c..165a434754d 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/purchase_order.py @@ -17,7 +17,7 @@ from erpnext.accounts.doctype.sales_invoice.sales_invoice import ( validate_inter_company_party, ) from erpnext.accounts.party import get_party_account, get_party_account_currency -from erpnext.buying.utils import check_on_hold_or_closed_status, validate_for_items +from erpnext.buying.utils import validate_for_items from erpnext.controllers.buying_controller import BuyingController from erpnext.manufacturing.doctype.blanket_order.blanket_order import ( validate_against_blanket_order, @@ -201,7 +201,7 @@ class PurchaseOrder(BuyingController): self.validate_supplier() self.validate_schedule_date() validate_for_items(self) - self.check_on_hold_or_closed_status() + self.check_for_on_hold_or_closed_status("Material Request", "material_request") self.validate_uom_is_integer("uom", "qty") self.validate_uom_is_integer("stock_uom", "stock_qty") @@ -380,18 +380,6 @@ class PurchaseOrder(BuyingController): d.base_rate ) = d.price_list_rate = d.rate = d.last_purchase_rate = item_last_purchase_rate - # Check for Closed status - def check_on_hold_or_closed_status(self): - check_list = [] - for d in self.get("items"): - if ( - d.meta.get_field("material_request") - and d.material_request - and d.material_request not in check_list - ): - check_list.append(d.material_request) - check_on_hold_or_closed_status("Material Request", d.material_request) - def update_ordered_qty(self, po_item_rows=None): """update requested qty (before ordered_qty is updated)""" item_wh_list = [] @@ -473,7 +461,7 @@ class PurchaseOrder(BuyingController): self.set_received_qty_to_zero_for_drop_ship_items() self.update_receiving_percentage() - self.check_on_hold_or_closed_status() + self.check_for_on_hold_or_closed_status("Material Request", "material_request") self.db_set("status", "Cancelled") diff --git a/erpnext/buying/utils.py b/erpnext/buying/utils.py index 7b80bf08290..f661ecb5d3d 100644 --- a/erpnext/buying/utils.py +++ b/erpnext/buying/utils.py @@ -113,7 +113,14 @@ def check_on_hold_or_closed_status(doctype, docname) -> None: status = frappe.db.get_value(doctype, docname, "status") if status in ("Closed", "On Hold"): - frappe.throw(_("{0} {1} status is {2}").format(doctype, docname, status), frappe.InvalidStatusError) + frappe.throw( + _("{0} {1} status is {2}.").format( + frappe.bold(_(doctype)), + frappe.bold(docname), + frappe.bold(_(status)), + ), + frappe.InvalidStatusError, + ) @frappe.whitelist() diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index 1ee6789f8d0..1fac4f8b216 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -683,19 +683,6 @@ class BuyingController(SubcontractingController): ) ) - def check_for_on_hold_or_closed_status(self, ref_doctype, ref_fieldname): - for d in self.get("items"): - if d.get(ref_fieldname): - status = frappe.db.get_value(ref_doctype, d.get(ref_fieldname), "status") - if status in ("Closed", "On Hold"): - frappe.throw( - _("{ref_doctype} {ref_name} is {status}.").format( - ref_doctype=frappe.bold(_(ref_doctype)), - ref_name=frappe.bold(d.get(ref_fieldname)), - status=frappe.bold(_(status)), - ) - ) - def update_stock_ledger(self, allow_negative_stock=False, via_landed_cost_voucher=False): self.update_ordered_and_reserved_qty() diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 4a7cae8fcfd..9ac3f3b6977 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -469,11 +469,9 @@ class SellingController(StockController): return so_qty, so_warehouse def check_sales_order_on_hold_or_close(self, ref_fieldname): - for d in self.get("items"): - if d.get(ref_fieldname): - status = frappe.db.get_value("Sales Order", d.get(ref_fieldname), "status") - if status in ("Closed", "On Hold") and not self.is_return: - frappe.throw(_("Sales Order {0} is {1}").format(d.get(ref_fieldname), status)) + if self.is_return: + return + self.check_for_on_hold_or_closed_status("Sales Order", ref_fieldname) def update_reserved_qty(self): so_map = {} diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 96f6cf16707..bdfa5dc1ee4 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -1937,6 +1937,43 @@ class StockController(AccountsController): qty -= working_qty + def check_for_on_hold_or_closed_status( + self, ref_doctype: str, ref_fieldname: str, exclude_if_field: str | None = None + ) -> None: + def _include(d): + return d.get(ref_fieldname) and not (exclude_if_field and d.get(exclude_if_field)) + + included = [(d, d.get(ref_fieldname)) for d in self.get("items") if _include(d)] + if not included: + return + + status_map = { + r.name: r.status + for r in frappe.get_all( + ref_doctype, + filters={"name": ["in", {name for _, name in included}]}, + fields=["name", "status"], + ) + } + + errors = [] + seen = set() + for _d, ref_name in included: + if ref_name in seen: + continue + seen.add(ref_name) + if (status := status_map.get(ref_name)) in ("Closed", "On Hold"): + errors.append( + _("{ref_doctype} {ref_name} status is {status}.").format( + ref_doctype=frappe.bold(_(ref_doctype)), + ref_name=frappe.bold(ref_name), + status=frappe.bold(_(status)), + ) + ) + + if errors: + frappe.throw("
".join(errors), frappe.InvalidStatusError) + @frappe.whitelist() def show_accounting_ledger_preview(company: str, doctype: str, docname: str): diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 49765f3a79c..9c2ed1a2255 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -25,6 +25,7 @@ from frappe.utils import ( ) from pypika import functions as fn +from erpnext.buying.utils import check_on_hold_or_closed_status from erpnext.manufacturing.doctype.bom.bom import ( get_bom_item_rate, get_bom_items_as_dict, @@ -439,7 +440,7 @@ class WorkOrder(Document): production_item = main_item_code if self.sales_order: - self.check_sales_order_on_hold_or_close() + check_on_hold_or_closed_status("Sales Order", self.sales_order) SalesOrder = frappe.qb.DocType("Sales Order") SalesOrderItem = frappe.qb.DocType("Sales Order Item") @@ -495,11 +496,6 @@ class WorkOrder(Document): else: frappe.throw(_("Sales Order {0} is not valid").format(self.sales_order)) - def check_sales_order_on_hold_or_close(self): - status = frappe.db.get_value("Sales Order", self.sales_order, "status") - if status in ("Closed", "On Hold"): - frappe.throw(_("Sales Order {0} is {1}").format(self.sales_order, status)) - def set_default_warehouse(self): if not self.wip_warehouse and not self.skip_transfer: self.wip_warehouse = frappe.get_cached_value("Company", self.company, "default_wip_warehouse") diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 30afd561482..8f3df98bd7d 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -16,7 +16,6 @@ from pypika import functions as fn import erpnext from erpnext.accounts.utils import get_account_currency from erpnext.assets.doctype.asset.asset import get_asset_account, is_cwip_accounting_enabled -from erpnext.buying.utils import check_on_hold_or_closed_status from erpnext.controllers.accounts_controller import merge_taxes from erpnext.controllers.buying_controller import BuyingController from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_transaction @@ -265,7 +264,7 @@ class PurchaseReceipt(BuyingController): self.validate_cwip_accounts() self.validate_provisional_expense_account() - self.check_on_hold_or_closed_status() + self.check_for_on_hold_or_closed_status("Purchase Order", "purchase_order") if getdate(self.posting_date) > getdate(nowdate()): throw(_("Posting Date cannot be future date")) @@ -373,14 +372,6 @@ class PurchaseReceipt(BuyingController): po_qty, po_warehouse = frappe.db.get_value("Purchase Order Item", po_detail, ["qty", "warehouse"]) return po_qty, po_warehouse - # Check for Closed status - def check_on_hold_or_closed_status(self): - check_list = [] - for d in self.get("items"): - if d.meta.get_field("purchase_order") and d.purchase_order and d.purchase_order not in check_list: - check_list.append(d.purchase_order) - check_on_hold_or_closed_status("Purchase Order", d.purchase_order) - # on submit def on_submit(self): super().on_submit() @@ -456,7 +447,7 @@ class PurchaseReceipt(BuyingController): def on_cancel(self): super().on_cancel() - self.check_on_hold_or_closed_status() + self.check_for_on_hold_or_closed_status("Purchase Order", "purchase_order") # Check if Purchase Invoice has been submitted against current Purchase Order submitted = frappe.db.sql( """select t1.name diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py index ff974ff8340..f9646699ca2 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py +++ b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py @@ -12,7 +12,6 @@ from frappe.utils import cint, flt, get_link_to_form, getdate, nowdate import erpnext from erpnext.accounts.utils import get_account_currency -from erpnext.buying.utils import check_on_hold_or_closed_status from erpnext.controllers.subcontracting_controller import SubcontractingController from erpnext.setup.doctype.brand.brand import get_brand_defaults from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults @@ -216,9 +215,7 @@ class SubcontractingReceipt(SubcontractingController): self.create_raw_materials_supplied_or_received() def validate_closed_subcontracting_order(self): - for item in self.items: - if item.subcontracting_order: - check_on_hold_or_closed_status("Subcontracting Order", item.subcontracting_order) + self.check_for_on_hold_or_closed_status("Subcontracting Order", "subcontracting_order") def update_job_card(self): for row in self.get("items"):