refactor: move Inventory Dimension mandatory check from field-level to server-side (#56451)

* refactor: move Inventory Dimension mandatory check from field-level to server-side

* refactor: split large function

* fix: greptile issue
This commit is contained in:
rohitwaghchaure
2026-06-25 20:10:09 +05:30
committed by GitHub
parent a00874fa95
commit 6dad1b1ae6
13 changed files with 279 additions and 52 deletions

View File

@@ -100,6 +100,9 @@ class AssetCapitalization(StockController):
self.set_asset_values()
self.calculate_totals()
self.set_title()
# Asset Capitalization overrides validate() without calling super(), so the shared
# mandatory inventory dimension check must be invoked explicitly here.
self.validate_inventory_dimension_mandatory()
def on_update(self):
if self.stock_items:

View File

@@ -36,6 +36,8 @@ from erpnext.stock import get_warehouse_account_map
from erpnext.stock.doctype.batch.batch import get_batch_qty
from erpnext.stock.doctype.inventory_dimension.inventory_dimension import (
get_evaluated_inventory_dimension,
get_mandatory_dimension_fields,
get_mandatory_inventory_dimensions,
)
from erpnext.stock.doctype.item.item import get_item_defaults
from erpnext.stock.doctype.serial_and_batch_bundle.serial_and_batch_bundle import (
@@ -64,6 +66,7 @@ class StockController(AccountsController):
self.validate_internal_transfer()
self.validate_putaway_capacity()
self.reset_conversion_factor()
self.validate_inventory_dimension_mandatory()
def on_update(self):
super().on_update()
@@ -1140,6 +1143,50 @@ class StockController(AccountsController):
return item_account_wise_cost
def validate_inventory_dimension_mandatory(self):
# Mandatory inventory dimensions are enforced here (instead of via field-level `reqd`)
# so we can skip service rows and never block a document that is being cancelled.
if self.docstatus >= 2:
return
for table_field in ["items", "packed_items", "supplied_items"]:
rows = self.get(table_field)
if rows:
self.validate_mandatory_dimensions_in_table(rows)
def validate_mandatory_dimensions_in_table(self, rows):
child_doctype = rows[0].doctype
dimensions = get_mandatory_inventory_dimensions(child_doctype)
if not dimensions:
return
child_meta = frappe.get_meta(child_doctype)
for dimension in dimensions:
mandatory_fields = get_mandatory_dimension_fields(child_doctype, dimension)
for row in rows:
if mandatory_fields and not self.is_service_item_row(row):
self.validate_mandatory_dimension_row(row, dimension, mandatory_fields, child_meta)
def is_service_item_row(self, row) -> bool:
item_code = row.get("item_code")
return bool(item_code) and not frappe.get_cached_value("Item", item_code, "is_stock_item")
def validate_mandatory_dimension_row(self, row, dimension, mandatory_fields, child_meta):
for fieldname, condition in mandatory_fields:
if not child_meta.has_field(fieldname) or row.get(fieldname):
continue
if condition and not frappe.safe_eval(condition, {"doc": row, "parent": self}):
continue
frappe.throw(
_("Row #{0}: {1} is mandatory for the Inventory Dimension {2}.").format(
row.idx,
bold(_(child_meta.get_label(fieldname))),
bold(dimension.name),
)
)
def update_inventory_dimensions(self, row, sl_dict) -> None:
# To handle delivery note and sales invoice
if row.get("item_row"):

View File

@@ -485,3 +485,4 @@ erpnext.patches.v16_0.clear_procedures_from_receivable_report
erpnext.patches.v16_0.migrate_address_contact_custom_fields
erpnext.patches.v16_0.drop_redundant_serial_no_index_from_sabb
execute:frappe.db.set_single_value("Accounts Settings", "pcv_job_timeout", 3600)
erpnext.patches.v16_0.remove_mandatory_from_inv_dimension_fields

View File

@@ -9,7 +9,6 @@ def get_inventory_dimensions():
"source_fieldname",
"reference_document as doctype",
"reqd",
"mandatory_depends_on",
],
order_by="creation",
distinct=True,
@@ -85,5 +84,5 @@ def execute():
"Custom Field",
{"fieldname": fieldname, "dt": dimension.doctype},
"mandatory_depends_on",
display_depends_on if dimension.reqd else dimension.mandatory_depends_on,
display_depends_on if dimension.reqd else "",
)

View File

@@ -0,0 +1,54 @@
import frappe
from erpnext.stock.doctype.inventory_dimension.inventory_dimension import get_inventory_documents
def execute():
"""Mandatory inventory dimensions are now enforced on the server side
(StockController.validate_inventory_dimension_mandatory) instead of via field-level
`reqd`/`mandatory_depends_on`. Clear those properties from the related custom fields."""
dimensions = frappe.get_all(
"Inventory Dimension",
fields=[
"source_fieldname",
"reference_document",
"document_type",
"apply_to_all_doctypes",
],
)
for dimension in dimensions:
if not dimension.source_fieldname or not dimension.reference_document:
continue
# Scope to the exact doctypes where this dimension generated fields so unrelated
# mandatory custom fields (same name/target on a different doctype) are never touched.
if dimension.apply_to_all_doctypes:
doctypes = [d[0] for d in get_inventory_documents()]
elif dimension.document_type:
doctypes = [dimension.document_type]
else:
continue
fieldname = dimension.source_fieldname
fieldnames = [fieldname, f"to_{fieldname}", f"from_{fieldname}", f"rejected_{fieldname}"]
custom_fields = frappe.get_all(
"Custom Field",
filters={
"dt": ("in", doctypes),
"fieldname": ("in", fieldnames),
"fieldtype": "Link",
"options": dimension.reference_document,
},
or_filters={"reqd": 1, "mandatory_depends_on": ("is", "set")},
pluck="name",
)
for name in custom_fields:
frappe.db.set_value(
"Custom Field",
name,
{"reqd": 0, "mandatory_depends_on": ""},
update_modified=False,
)

View File

@@ -51,7 +51,6 @@ frappe.ui.form.on("Inventory Dimension", {
"fetch_from_parent",
"type_of_transaction",
"condition",
"mandatory_depends_on",
"validate_negative_stock",
];

View File

@@ -27,7 +27,7 @@
"condition",
"conditional_mandatory_section",
"reqd",
"mandatory_depends_on",
"mandatory_depends_on_backend",
"conditional_rule_examples_section",
"html_19"
],
@@ -151,13 +151,6 @@
"fieldtype": "Section Break",
"label": "Conditional Rule Examples"
},
{
"depends_on": "eval:!doc.apply_to_all_doctypes",
"description": "To apply condition on parent field use parent.field_name and to apply condition on child table use doc.field_name. Here field_name could be based on the actual column name of the respective field.",
"fieldname": "mandatory_depends_on",
"fieldtype": "Small Text",
"label": "Mandatory Depends On"
},
{
"fieldname": "conditional_mandatory_section",
"fieldtype": "Section Break",
@@ -169,6 +162,13 @@
"fieldtype": "Check",
"label": "Mandatory"
},
{
"depends_on": "eval:!doc.reqd",
"description": "Python expression evaluated on the server. Use doc.fieldname for the row and parent.fieldname for the parent document. When it evaluates to true the dimension becomes mandatory. Example: doc.t_warehouse and doc.qty > 0",
"fieldname": "mandatory_depends_on_backend",
"fieldtype": "Small Text",
"label": "Mandatory Depends On (Backend)"
},
{
"fieldname": "column_break_niy2u",
"fieldtype": "Column Break"
@@ -182,7 +182,7 @@
],
"index_web_pages_for_search": 1,
"links": [],
"modified": "2026-04-08 10:10:16.884388",
"modified": "2026-06-25 11:30:00.000000",
"modified_by": "Administrator",
"module": "Stock",
"name": "Inventory Dimension",

View File

@@ -35,7 +35,7 @@ class InventoryDimension(Document):
document_type: DF.Link | None
fetch_from_parent: DF.Literal[None]
istable: DF.Check
mandatory_depends_on: DF.SmallText | None
mandatory_depends_on_backend: DF.SmallText | None
reference_document: DF.Link
reqd: DF.Check
source_fieldname: DF.Data | None
@@ -118,7 +118,6 @@ class InventoryDimension(Document):
def reset_value(self):
if self.apply_to_all_doctypes:
self.type_of_transaction = ""
self.mandatory_depends_on = ""
self.istable = 0
for field in ["document_type", "condition"]:
@@ -167,15 +166,10 @@ class InventoryDimension(Document):
if label_start_with:
label = f"{label_start_with} {self.dimension_name}"
mandatory_depends_on = self.mandatory_depends_on
if self.reqd:
if doctype == "Stock Entry Detail":
mandatory_depends_on = "eval:doc.s_warehouse"
elif doctype == "Subcontracting Receipt Supplied Item":
mandatory_depends_on = "eval:doc.reference_name"
elif doctype == "Packed Item":
mandatory_depends_on = "eval:doc.parent_detail_docname && ['Delivery Note', 'Sales Invoice', 'POS Invoice'].includes(parent.doctype)"
# Note: `reqd` is intentionally NOT set on the custom fields. Mandatory enforcement
# happens on the server side via StockController.validate_inventory_dimension_mandatory()
# so that it can be gated (e.g. skip service rows) and never blocks documents that are
# being cancelled.
dimension_fields = [
dict(
fieldname="inventory_dimension",
@@ -192,13 +186,6 @@ class InventoryDimension(Document):
label=_(label),
depends_on="eval:doc.s_warehouse" if doctype == "Stock Entry Detail" else "",
search_index=1,
reqd=1
if self.reqd
and not self.mandatory_depends_on
and doctype
not in ["Stock Entry Detail", "Subcontracting Receipt Supplied Item", "Packed Item"]
else 0,
mandatory_depends_on=mandatory_depends_on,
),
]
@@ -211,7 +198,6 @@ class InventoryDimension(Document):
options=self.reference_document,
label=_("Rejected " + self.dimension_name),
search_index=1,
mandatory_depends_on="eval:doc.rejected_qty > 0",
)
)
@@ -238,9 +224,7 @@ class InventoryDimension(Document):
and not frappe.db.get_value("Custom Field", {"dt": dt, "fieldname": self.target_fieldname})
and not field_exists(dt, self.target_fieldname)
):
dimension_field = dimension_fields[1]
dimension_field["mandatory_depends_on"] = ""
dimension_field["reqd"] = 0
dimension_field = dimension_fields[1].copy()
dimension_field["fieldname"] = self.target_fieldname
custom_fields[dt] = dimension_field
@@ -309,7 +293,6 @@ class InventoryDimension(Document):
options=self.reference_document,
label=label,
depends_on=display_depends_on,
mandatory_depends_on=display_depends_on if self.reqd else self.mandatory_depends_on,
),
]
)
@@ -390,6 +373,101 @@ def get_document_wise_inventory_dimensions(doctype) -> dict:
)
@request_cache
def get_mandatory_inventory_dimensions(doctype) -> list:
"""Return the inventory dimensions applicable to `doctype` (a child doctype such as
`Stock Entry Detail`) that need server-side mandatory enforcement.
A dimension qualifies only if it is configured as mandatory (`reqd`) or has a server-side
mandatory condition (`mandatory_depends_on_backend`). Non-mandatory dimensions are never
enforced, including the rejected dimension field on purchase rows."""
dimensions = frappe.get_all(
"Inventory Dimension",
fields=[
"name",
"dimension_name",
"source_fieldname",
"reqd",
"mandatory_depends_on_backend",
],
or_filters={"document_type": doctype, "apply_to_all_doctypes": 1},
)
return [d for d in dimensions if d.reqd or d.mandatory_depends_on_backend]
def get_mandatory_dimension_fields(doctype, dimension) -> list:
"""For a mandatory `dimension` return the list of (fieldname, condition) tuples that must be
filled on a row of `doctype`. `condition` is a python expression evaluated with `doc` (the row)
and `parent`; a `None` condition means the field is unconditionally mandatory.
Mirrors the mandatory logic that used to live on the custom fields in `get_dimension_fields`."""
fields = []
source_fieldname = dimension.source_fieldname
# `mandatory_depends_on_backend` is a raw python expression evaluated server-side
# (with `doc` and `parent`), so it can be used as a condition directly.
backend_condition = (dimension.mandatory_depends_on_backend or "").strip() or None
# Primary source dimension field
if dimension.reqd and doctype == "Stock Entry Detail":
fields.append((source_fieldname, "doc.s_warehouse"))
elif dimension.reqd and doctype == "Subcontracting Receipt Supplied Item":
fields.append((source_fieldname, "doc.reference_name"))
elif dimension.reqd and doctype == "Packed Item":
fields.append(
(
source_fieldname,
"doc.parent_detail_docname and parent.doctype in ['Delivery Note', 'Sales Invoice', 'POS Invoice']",
)
)
elif dimension.reqd:
fields.append((source_fieldname, None))
elif backend_condition:
fields.append((source_fieldname, backend_condition))
# Rejected dimension field (only present on purchase rows). Enforced only when the dimension
# is mandatory for the row AND there is a rejected quantity.
if doctype in ["Purchase Invoice Item", "Purchase Receipt Item"]:
if dimension.reqd:
fields.append((f"rejected_{source_fieldname}", "doc.rejected_qty > 0"))
elif backend_condition:
fields.append((f"rejected_{source_fieldname}", f"({backend_condition}) and doc.rejected_qty > 0"))
# Target/transfer dimension field used for internal transfers (mirrors the old
# `add_transfer_field` behaviour). When the dimension is `reqd` the field inherits the
# transfer display condition, otherwise it inherits the server-side mandatory condition.
if (dimension.reqd or backend_condition) and doctype in [
"Stock Entry Detail",
"Sales Invoice Item",
"Delivery Note Item",
"Purchase Invoice Item",
"Purchase Receipt Item",
]:
if doctype in ["Purchase Invoice Item", "Purchase Receipt Item"]:
transfer_fieldname, display_condition = (
f"from_{source_fieldname}",
"parent.is_internal_supplier == 1",
)
elif doctype == "Stock Entry Detail":
transfer_fieldname, display_condition = f"to_{source_fieldname}", "doc.t_warehouse"
else:
transfer_fieldname, display_condition = (
f"to_{source_fieldname}",
"parent.is_internal_customer == 1",
)
# The transfer field only applies to internal transfers, so its mandatory check is always
# gated on the display condition; the backend condition narrows it further.
if dimension.reqd:
transfer_condition = display_condition
else:
transfer_condition = f"({display_condition}) and ({backend_condition})"
fields.append((transfer_fieldname, transfer_condition))
return fields
@frappe.whitelist()
@request_cache
def get_inventory_dimensions():

View File

@@ -219,35 +219,75 @@ class TestInventoryDimension(ERPNextTestSuite):
doc.reqd = 1
doc.save()
self.assertTrue(
# Mandatory enforcement is now done server-side, so the custom field must NOT be `reqd`.
self.assertFalse(
frappe.db.get_value(
"Custom Field", {"fieldname": "pallet_75", "dt": "Delivery Note Item", "reqd": 1}, "name"
"Custom Field", {"fieldname": "pallet_75", "dt": "Delivery Note Item"}, "reqd"
)
)
item_code = "Test Mandatory Dimension Item"
create_item(item_code)
warehouse = create_warehouse("Mandatory Dimension Warehouse")
dn_doc = create_delivery_note(item_code=item_code, qty=5, warehouse=warehouse, do_not_save=True)
# Dimension value missing -> server-side validation should block the document.
self.assertRaises(frappe.ValidationError, dn_doc.save)
if not frappe.db.exists("Pallet", "Pallet 75 Value"):
frappe.get_doc({"doctype": "Pallet", "pallet_name": "Pallet 75 Value"}).insert(
ignore_permissions=True
)
dn_doc.items[0].pallet_75 = "Pallet 75 Value"
dn_doc.save()
doc.reqd = 0
doc.save()
def test_check_mandatory_depends_on_dimensions(self):
def test_check_mandatory_depends_on_backend(self):
doc = create_inventory_dimension(
reference_document="Pallet",
type_of_transaction="Outward",
dimension_name="Pallet",
dimension_name="Pallet Backend",
apply_to_all_doctypes=0,
document_type="Stock Entry Detail",
document_type="Delivery Note Item",
)
doc.mandatory_depends_on = "t_warehouse"
doc.reqd = 0
doc.mandatory_depends_on_backend = "doc.qty > 0"
doc.save()
self.assertTrue(
# The condition is enforced server-side, the custom field must not carry field-level `reqd`.
self.assertFalse(
frappe.db.get_value(
"Custom Field",
{"fieldname": "pallet", "dt": "Stock Entry Detail", "mandatory_depends_on": "t_warehouse"},
"name",
"Custom Field", {"fieldname": "pallet_backend", "dt": "Delivery Note Item"}, "reqd"
)
)
item_code = "Test Backend Dimension Item"
create_item(item_code)
warehouse = create_warehouse("Backend Dimension Warehouse")
dn_doc = create_delivery_note(item_code=item_code, qty=5, warehouse=warehouse, do_not_save=True)
# qty > 0 -> backend condition is met, so the dimension is mandatory and blocks the save.
self.assertRaises(frappe.ValidationError, dn_doc.save)
if not frappe.db.exists("Pallet", "Pallet Backend Value"):
frappe.get_doc({"doctype": "Pallet", "pallet_name": "Pallet Backend Value"}).insert(
ignore_permissions=True
)
dn_doc.items[0].pallet_backend = "Pallet Backend Value"
dn_doc.save()
# Reset so the always-true condition does not make the dimension mandatory for
# subsequent Delivery Note tests sharing the same test database.
doc.mandatory_depends_on_backend = ""
doc.save()
def test_for_purchase_sales_and_stock_transaction(self):
from erpnext.controllers.sales_and_purchase_return import make_return_doc

View File

@@ -292,6 +292,9 @@ class StockEntry(StockController, SubcontractingInwardController):
self.validate_putaway_capacity()
self.validate_component_and_quantities()
self.validate_finished_good_serial_batch_for_work_order()
# Stock Entry overrides validate() without calling super(), so the shared mandatory
# inventory dimension check must be invoked explicitly here.
self.validate_inventory_dimension_mandatory()
if self.get("purpose") != "Manufacture":
# ignore other item wh difference and empty source/target wh

View File

@@ -83,6 +83,9 @@ class StockReconciliation(StockController):
self.set_total_qty_and_amount()
self.validate_putaway_capacity()
self.validate_inventory_dimension()
# Stock Reconciliation overrides validate() without calling super(), so the shared
# mandatory inventory dimension check must be invoked explicitly here.
self.validate_inventory_dimension_mandatory()
self.validate_uom_is_integer("stock_uom", "qty")
if self._action == "submit":

View File

@@ -162,6 +162,12 @@ class SubcontractingReceipt(SubcontractingController):
self.set_supplied_items_cost_center()
self.set_supplied_items_inventory_dimensions()
# SubcontractingController.validate() does not call super() for Subcontracting Receipt, so
# the shared mandatory inventory dimension check must be invoked explicitly here. It runs
# last so auto-populated supplied-item dimensions (set_supplied_items_inventory_dimensions)
# are already in place.
self.validate_inventory_dimension_mandatory()
def on_submit(self):
self.validate_closed_subcontracting_order()
self.validate_bom_required_qty()

View File

@@ -2045,16 +2045,13 @@ class TestSubcontractingReceipt(ERPNextTestSuite):
create_inventory_dimension,
)
inventory_dimension = create_inventory_dimension(
create_inventory_dimension(
apply_to_all_doctypes=1,
dimension_name="Inv Site",
reference_document="Inv Site",
document_type="Inv Site",
)
inventory_dimension.reqd = 1
inventory_dimension.save()
set_backflush_based_on("BOM")
sco = get_subcontracting_order()
@@ -2074,9 +2071,6 @@ class TestSubcontractingReceipt(ERPNextTestSuite):
self.assertEqual(scr.supplied_items[0].inv_site, "Site 1")
inventory_dimension.reqd = 0
inventory_dimension.save()
def make_return_subcontracting_receipt(**args):
args = frappe._dict(args)