refactor(stock): address layering/robustness review findings (#8, #9, #10)

#8 ledger_preview: wrap the submit-in-memory dry run in a savepoint inside
get_accounting_ledger_preview / get_stock_ledger_preview and roll back to it in a
finally, so the preview never persists entries regardless of caller (previously
only the whitelisted show_*_preview wrappers' full rollback made it safe).

#9 exceptions: move BatchExpiredError and the QualityInspection* errors into a new
erpnext/stock/exceptions.py and re-export them from stock_controller for backward
compatibility (job_card and tests still import from the controller; identity is
preserved). Services now import from the neutral module instead of back from the
controller they were extracted out of.

#10 quality inspection: extract the duplicated doctype->inspection-field map into a
single INSPECTION_FIELDNAME_MAP constant in the service, consumed by both
validate_inspection and check_item_quality_inspection.

Verified: ledger snapshots, quality inspection suite, stock_entry batch-expiry test
stay green; preview smoke-tested to persist nothing and not roll back the caller.
This commit is contained in:
Nabin Hait
2026-06-05 12:29:55 +05:30
parent 3dba21f814
commit 78d5fbaca4
5 changed files with 83 additions and 55 deletions

View File

@@ -23,25 +23,17 @@ from erpnext.setup.doctype.brand.brand import get_brand_defaults
from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults
from erpnext.stock import get_warehouse_account_map from erpnext.stock import get_warehouse_account_map
from erpnext.stock.doctype.item.item import get_item_defaults from erpnext.stock.doctype.item.item import get_item_defaults
# Re-exported for backward compatibility; canonical home is erpnext.stock.exceptions.
from erpnext.stock.exceptions import (
BatchExpiredError,
QualityInspectionNotSubmittedError,
QualityInspectionRejectedError,
QualityInspectionRequiredError,
)
from erpnext.stock.stock_ledger import get_items_to_be_repost from erpnext.stock.stock_ledger import get_items_to_be_repost
class QualityInspectionRequiredError(frappe.ValidationError):
pass
class QualityInspectionRejectedError(frappe.ValidationError):
pass
class QualityInspectionNotSubmittedError(frappe.ValidationError):
pass
class BatchExpiredError(frappe.ValidationError):
pass
class StockController(AccountsController): class StockController(AccountsController):
def validate(self): def validate(self):
super().validate() super().validate()
@@ -674,18 +666,12 @@ def repost_required_for_queue(doc: StockController) -> bool:
@frappe.whitelist() @frappe.whitelist()
def check_item_quality_inspection(doctype: str, docstatus: str | int, items: str | list[dict]): def check_item_quality_inspection(doctype: str, docstatus: str | int, items: str | list[dict]):
from erpnext.stock.services.quality_inspection import INSPECTION_FIELDNAME_MAP
if isinstance(items, str): if isinstance(items, str):
items = json.loads(items) items = json.loads(items)
inspection_fieldname_map = { inspection_fieldname = INSPECTION_FIELDNAME_MAP.get(doctype)
"Purchase Receipt": "inspection_required_before_purchase",
"Purchase Invoice": "inspection_required_before_purchase",
"Subcontracting Receipt": "inspection_required_before_purchase",
"Sales Invoice": "inspection_required_before_delivery",
"Delivery Note": "inspection_required_before_delivery",
}
inspection_fieldname = inspection_fieldname_map.get(doctype)
if inspection_fieldname is None: if inspection_fieldname is None:
return [] return []

View File

@@ -0,0 +1,27 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: GNU General Public License v3. See license.txt
"""Shared exceptions for stock transactions.
Raised by the stock services (serial/batch bundle, quality inspection) and
re-exported from ``stock_controller`` for backward compatibility, so the services
do not have to import back from the controller they were extracted out of.
"""
import frappe
class QualityInspectionRequiredError(frappe.ValidationError):
pass
class QualityInspectionRejectedError(frappe.ValidationError):
pass
class QualityInspectionNotSubmittedError(frappe.ValidationError):
pass
class BatchExpiredError(frappe.ValidationError):
pass

View File

@@ -30,17 +30,23 @@ def get_accounting_ledger_preview(doc, filters):
"against_voucher", "against_voucher",
] ]
doc.docstatus = 1 # Dry run: submit in memory to materialise GL entries, read them, then roll back
# to the savepoint so the preview never persists anything, regardless of caller.
frappe.db.savepoint("ledger_preview")
try:
doc.docstatus = 1
if doc.get("update_stock") or doc.doctype in ("Purchase Receipt", "Delivery Note", "Stock Entry"): if doc.get("update_stock") or doc.doctype in ("Purchase Receipt", "Delivery Note", "Stock Entry"):
doc.update_stock_ledger() doc.update_stock_ledger()
doc.make_gl_entries() doc.make_gl_entries()
columns = get_gl_columns(filters) columns = get_gl_columns(filters)
gl_entries = get_gl_entries_for_preview(doc.doctype, doc.name, fields) gl_entries = get_gl_entries_for_preview(doc.doctype, doc.name, fields)
gl_columns = get_columns(columns, fields) gl_columns = get_columns(columns, fields)
gl_data = get_data(fields, gl_entries) gl_data = get_data(fields, gl_entries)
finally:
frappe.db.rollback(save_point="ledger_preview")
return gl_columns, gl_data return gl_columns, gl_data
@@ -74,15 +80,21 @@ def get_stock_ledger_preview(doc, filters):
] ]
if doc.get("update_stock") or doc.doctype in ("Purchase Receipt", "Delivery Note", "Stock Entry"): if doc.get("update_stock") or doc.doctype in ("Purchase Receipt", "Delivery Note", "Stock Entry"):
doc.docstatus = 1 # Dry run: submit in memory to materialise SLEs, read them, then roll back to
doc.make_bundle_using_old_serial_batch_fields() # the savepoint so the preview never persists anything, regardless of caller.
doc.update_stock_ledger() frappe.db.savepoint("ledger_preview")
try:
doc.docstatus = 1
doc.make_bundle_using_old_serial_batch_fields()
doc.update_stock_ledger()
columns = get_sl_columns(filters) columns = get_sl_columns(filters)
sl_entries = get_sl_entries_for_preview(doc.doctype, doc.name, fields) sl_entries = get_sl_entries_for_preview(doc.doctype, doc.name, fields)
sl_columns = get_columns(columns, columns_fields) sl_columns = get_columns(columns, columns_fields)
sl_data = get_data(columns_fields, sl_entries) sl_data = get_data(columns_fields, sl_entries)
finally:
frappe.db.rollback(save_point="ledger_preview")
return sl_columns, sl_data return sl_columns, sl_data

View File

@@ -10,6 +10,22 @@ inspection have a present / submitted / non-rejected Quality Inspection.
import frappe import frappe
from frappe import _ from frappe import _
from erpnext.stock.exceptions import (
QualityInspectionNotSubmittedError,
QualityInspectionRejectedError,
QualityInspectionRequiredError,
)
# Doctype -> the document-level "inspection required" flag. Shared with
# check_item_quality_inspection in stock_controller so the two stay in sync.
INSPECTION_FIELDNAME_MAP = {
"Purchase Receipt": "inspection_required_before_purchase",
"Purchase Invoice": "inspection_required_before_purchase",
"Subcontracting Receipt": "inspection_required_before_purchase",
"Sales Invoice": "inspection_required_before_delivery",
"Delivery Note": "inspection_required_before_delivery",
}
class QualityInspectionService: class QualityInspectionService:
def __init__(self, doc) -> None: def __init__(self, doc) -> None:
@@ -17,14 +33,7 @@ class QualityInspectionService:
def validate_inspection(self): def validate_inspection(self):
"""Checks if quality inspection is set/ is valid for Items that require inspection.""" """Checks if quality inspection is set/ is valid for Items that require inspection."""
inspection_fieldname_map = { inspection_required_fieldname = INSPECTION_FIELDNAME_MAP.get(self.doc.doctype)
"Purchase Receipt": "inspection_required_before_purchase",
"Purchase Invoice": "inspection_required_before_purchase",
"Subcontracting Receipt": "inspection_required_before_purchase",
"Sales Invoice": "inspection_required_before_delivery",
"Delivery Note": "inspection_required_before_delivery",
}
inspection_required_fieldname = inspection_fieldname_map.get(self.doc.doctype)
# return if inspection is not required on document level # return if inspection is not required on document level
if ( if (
@@ -64,8 +73,6 @@ class QualityInspectionService:
def validate_qi_presence(self, row): def validate_qi_presence(self, row):
"""Check if QI is present on row level. Warn on save and stop on submit if missing.""" """Check if QI is present on row level. Warn on save and stop on submit if missing."""
from erpnext.controllers.stock_controller import QualityInspectionRequiredError
if not row.quality_inspection: if not row.quality_inspection:
msg = _("Row #{0}: Quality Inspection is required for Item {1}").format( msg = _("Row #{0}: Quality Inspection is required for Item {1}").format(
row.idx, frappe.bold(row.item_code) row.idx, frappe.bold(row.item_code)
@@ -77,8 +84,6 @@ class QualityInspectionService:
def validate_qi_submission(self, row): def validate_qi_submission(self, row):
"""Check if QI is submitted on row level, during submission""" """Check if QI is submitted on row level, during submission"""
from erpnext.controllers.stock_controller import QualityInspectionNotSubmittedError
action = frappe.get_single_value("Stock Settings", "action_if_quality_inspection_is_not_submitted") action = frappe.get_single_value("Stock Settings", "action_if_quality_inspection_is_not_submitted")
qa_docstatus = frappe.db.get_value("Quality Inspection", row.quality_inspection, "docstatus") qa_docstatus = frappe.db.get_value("Quality Inspection", row.quality_inspection, "docstatus")
@@ -94,8 +99,6 @@ class QualityInspectionService:
def validate_qi_rejection(self, row): def validate_qi_rejection(self, row):
"""Check if QI is rejected on row level, during submission""" """Check if QI is rejected on row level, during submission"""
from erpnext.controllers.stock_controller import QualityInspectionRejectedError
action = frappe.get_single_value("Stock Settings", "action_if_quality_inspection_is_rejected") action = frappe.get_single_value("Stock Settings", "action_if_quality_inspection_is_rejected")
qa_status = frappe.db.get_value("Quality Inspection", row.quality_inspection, "status") qa_status = frappe.db.get_value("Quality Inspection", row.quality_inspection, "status")

View File

@@ -102,8 +102,8 @@ class SerialBatchBundleService:
) )
def validate_serialized_batch(self): def validate_serialized_batch(self):
from erpnext.controllers.stock_controller import BatchExpiredError
from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos
from erpnext.stock.exceptions import BatchExpiredError
is_material_issue = False is_material_issue = False
if self.doc.doctype == "Stock Entry" and self.doc.purpose in ["Material Issue", "Material Transfer"]: if self.doc.doctype == "Stock Entry" and self.doc.purpose in ["Material Issue", "Material Transfer"]: