fix: update items respect workflow "Only Allow Edit For" role (#55662)

erpnext/accounts/services/child_item_update.py

@@ -5,7 +5,7 @@
 
 import frappe
 from frappe import _
-from frappe.model.workflow import get_workflow_name, is_transition_condition_satisfied
+from frappe.model.workflow import get_workflow_name
 from frappe.utils import flt, get_link_to_form, getdate
 
 from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import get_accounting_dimensions
@@ -222,15 +222,12 @@ class ChildItemUpdater:
 		current_state = self.parent.get(workflow_doc.workflow_state_field)
 		roles = frappe.get_roles()
 
-		transitions = [
-			t.as_dict()
-			for t in workflow_doc.transitions
-			if t.next_state == current_state
-			and t.allowed in roles
-			and is_transition_condition_satisfied(t, self.parent)
-		]
+		allowed = any(
+			state.state == current_state and (not state.allow_edit or state.allow_edit in roles)
+			for state in workflow_doc.states
+		)
 
-		if not transitions:
+		if not allowed:
 			frappe.throw(
 				_("You are not allowed to update as per the conditions set in {} Workflow.").format(
 					get_link_to_form("Workflow", workflow)

erpnext/selling/doctype/sales_order/test_sales_order.py

@@ -724,6 +724,41 @@ class TestSalesOrder(ERPNextTestSuite):
 		workflow.is_active = 0
 		workflow.save()
 
+	def test_update_child_qty_rate_follows_allow_edit(self):
+		from frappe.model.workflow import apply_workflow
+
+		workflow = make_sales_order_edit_perm_workflow()
+		so = make_sales_order(item_code="_Test Item", qty=1, rate=150, do_not_submit=1)
+		apply_workflow(so, "Approve")
+
+		trans_item = json.dumps(
+			[{"item_code": "_Test Item", "rate": 150, "qty": 2, "docname": so.items[0].name}]
+		)
+
+		# Test Junior Approver performed the transition into Approved, but the state's
+		# Only Allow Edit For is Test Approver — the mover must not be able to edit.
+		mover = "test@example.com"
+		mover_user = frappe.get_doc("User", mover)
+		mover_user.add_roles("Sales User", "Test Junior Approver")
+		with self.set_user(mover):
+			self.assertRaises(
+				frappe.ValidationError, update_child_qty_rate, "Sales Order", trans_item, so.name
+			)
+
+		# The configured allow_edit role can edit even without any transition rights.
+		editor = "test2@example.com"
+		editor_user = frappe.get_doc("User", editor)
+		editor_user.add_roles("Sales User", "Test Approver")
+		with self.set_user(editor):
+			update_child_qty_rate("Sales Order", trans_item, so.name)
+			so.reload()
+			self.assertEqual(so.items[0].qty, 2)
+
+		mover_user.remove_roles("Sales User", "Test Junior Approver", "Test Approver")
+		editor_user.remove_roles("Sales User", "Test Junior Approver", "Test Approver")
+		workflow.is_active = 0
+		workflow.save()
+
 	def test_material_request_for_product_bundle(self):
 		# Create the Material Request from the sales order for the Packing Items
 		# Check whether the material request has the correct packing item or not.
@@ -3041,3 +3076,43 @@ def make_sales_order_workflow():
 	workflow.insert(ignore_permissions=True)
 
 	return workflow
+
+
+def make_sales_order_edit_perm_workflow():
+	if frappe.db.exists("Workflow", "SO Edit Perm Workflow"):
+		doc = frappe.get_doc("Workflow", "SO Edit Perm Workflow")
+		doc.set("is_active", 1)
+		doc.save()
+		return doc
+
+	frappe.get_doc(doctype="Role", role_name="Test Junior Approver").insert(ignore_if_duplicate=True)
+	frappe.get_doc(doctype="Role", role_name="Test Approver").insert(ignore_if_duplicate=True)
+	frappe.cache().hdel("roles", frappe.session.user)
+
+	workflow = frappe.get_doc(
+		{
+			"doctype": "Workflow",
+			"workflow_name": "SO Edit Perm Workflow",
+			"document_type": "Sales Order",
+			"workflow_state_field": "workflow_state",
+			"is_active": 1,
+			"send_email_alert": 0,
+		}
+	)
+	workflow.append("states", dict(state="Pending", allow_edit="All"))
+	# Only Allow Edit For is Test Approver, while the transition into Approved is
+	# performed by Test Junior Approver — the two roles are deliberately different.
+	workflow.append("states", dict(state="Approved", allow_edit="Test Approver", doc_status=1))
+	workflow.append(
+		"transitions",
+		dict(
+			state="Pending",
+			action="Approve",
+			next_state="Approved",
+			allowed="Test Junior Approver",
+			allow_self_approval=1,
+		),
+	)
+	workflow.insert(ignore_permissions=True)
+
+	return workflow