Relocate Rule Trigger Condition to Entry Action Config (Release N)

flexirule/patches/v1_0/migrate_trigger_condition_to_entry_action.py

@@ -0,0 +1,128 @@
+import json
+
+import frappe
+
+
+def execute():
+	"""
+	Safe migration patch to relocate Rule.trigger_condition to Entry Action config.
+	Strictly idempotent and conservative.
+	"""
+	rules = frappe.get_all("Rule", fields=["name", "trigger_condition"])
+
+	for r in rules:
+		try:
+			doc = frappe.get_doc("Rule", r.name)
+
+			# 1. Identify Entry Action
+			entry_actions = [
+				a
+				for a in doc.actions
+				if (a.get("action_type") == "Entry Action" or a.get("action_id") == "root")
+			]
+
+			if not entry_actions:
+				# Case C: No entry action - should be handled by ensure_start_node but let's be safe
+				doc.ensure_start_node()
+				entry_action = doc.get_entry_action()
+			elif len(entry_actions) > 1:
+				# Case C: Multiple entry actions
+				frappe.log_error(
+					f"Rule {doc.name} has multiple entry actions. Skipping migration.",
+					"FlexiRule Trigger Condition Migration",
+				)
+				continue
+			else:
+				entry_action = entry_actions[0]
+
+			if not entry_action:
+				continue
+
+			# 2. Parse conditions for comparison
+			legacy_cond = _parse_json(doc.trigger_condition)
+			action_cond = _parse_json(entry_action.config)
+
+			# Case D: Malformed JSON check (if they were strings and failed to parse)
+			if doc.trigger_condition and legacy_cond is None:
+				frappe.log_error(
+					f"Rule {doc.name} has malformed legacy trigger_condition. Skipping migration.",
+					"FlexiRule Trigger Condition Migration",
+				)
+				continue
+
+			if entry_action.config and action_cond is None:
+				frappe.log_error(
+					f"Rule {doc.name} has malformed Entry Action config. Skipping migration.",
+					"FlexiRule Trigger Condition Migration",
+				)
+				continue
+
+			# 3. Decision Logic
+			if not legacy_cond or _is_effectively_empty(legacy_cond):
+				# Nothing to migrate
+				continue
+
+			if not action_cond or _is_effectively_empty(action_cond):
+				# Case A: Clean Move
+				_update_action_config(entry_action, doc.trigger_condition)
+				doc.compile_conditions()
+				frappe.db.set_value(
+					"Rule", doc.name, "compiled_expression", doc.compiled_expression, update_modified=False
+				)
+			else:
+				# Both have data - check for ambiguity
+				if _is_equal(legacy_cond, action_cond):
+					# Already migrated (Idempotent)
+					continue
+				else:
+					# Case B: Ambiguous Data
+					frappe.log_error(
+						f"Rule {doc.name} has differing conditions in Rule.trigger_condition and EntryAction.config. Skipping migration.",
+						"FlexiRule Trigger Condition Migration",
+					)
+					continue
+
+		except Exception as e:
+			import traceback
+
+			frappe.log_error(
+				f"Migration failed for Rule {r.name}: {e!s}\n\n{traceback.format_exc()}",
+				"FlexiRule Trigger Condition Migration",
+			)
+
+
+def _parse_json(value):
+	if not value:
+		return {}
+	if isinstance(value, dict | list):
+		return value
+	try:
+		return json.loads(value)
+	except (ValueError, TypeError):
+		return None
+
+
+def _is_effectively_empty(cond):
+	if not cond:
+		return True
+	if isinstance(cond, list) and not cond:
+		return True
+	if isinstance(cond, dict):
+		if not cond.get("conditions") and not cond.get("collection"):
+			return True
+	return False
+
+
+def _is_equal(cond1, cond2):
+	# Simple stable comparison
+	return json.dumps(cond1, sort_keys=True) == json.dumps(cond2, sort_keys=True)
+
+
+def _update_action_config(action_doc, config_value):
+	# Update via frappe.db.set_value to bypass hooks as requested
+	if isinstance(config_value, dict | list):
+		config_value = json.dumps(config_value)
+
+	frappe.db.set_value("Rule Action", action_doc.name, "config", config_value, update_modified=False)
+	# Update in-memory for compile_conditions
+	action_doc.config = config_value

flexirule/public/js/flexirule/rule_builder/stores/useRuleStore.js

@@ -289,7 +289,10 @@ export const useRuleStore = defineStore("rule-builder-rule", () => {
 			doc.is_active = currentIsActive;
 
 			const startNode = graphStore.nodes.find((el) => el.type === "start");
-			doc.trigger_condition = serializeField(startNode?.data?.trigger_condition);
+			// Dual-write: serialize Start Node config back to root trigger_condition
+			doc.trigger_condition = serializeField(
+				startNode?.data?.trigger_condition || startNode?.data?.config
+			);
 			doc.compiled_expression = null;
 
 			if (startNode?.data) {
@@ -378,10 +381,11 @@ export const useRuleStore = defineStore("rule-builder-rule", () => {
 						operation: node.data?.operation,
 					}) || {};
 				const conditionPayload =
-					action_type === "Condition"
+					action_type === "Condition" || action_type === "Entry Action"
 						? getConditionPayload({
 								config: node.data?.config,
 								condition_json: node.data?.condition_json,
+								trigger_condition: node.data?.trigger_condition,
 						  })
 						: null;
 
@@ -406,7 +410,7 @@ export const useRuleStore = defineStore("rule-builder-rule", () => {
 				}
 
 				const finalConfig =
-					action_type === "Condition"
+					action_type === "Condition" || action_type === "Entry Action"
 						? conditionPayload || normalizedConfig
 						: normalizedConfig;
 

flexirule/public/js/flexirule/rule_builder/utils/condition_payload.js

@@ -46,7 +46,10 @@ export function getConditionPayload(actionLike = {}) {
 	if (fromConfig !== null) return fromConfig;
 
 	const legacy = safeJsonParse(actionLike.condition_json, null);
-	return isConditionPayloadShape(legacy) ? legacy : null;
+	if (isConditionPayloadShape(legacy)) return legacy;
+
+	const trigger = safeJsonParse(actionLike.trigger_condition, null);
+	return isConditionPayloadShape(trigger) ? trigger : null;
 }
 
 export function hasConditionPayload(actionLike = {}) {

flexirule/ruleflow/core/action_handlers/sub_rule.py

@@ -236,13 +236,12 @@ def execute(self, action, context, engine):
 			)
 
 			# Evaluate trigger condition if not skipped
-			if not skip_conditions and sub_rule_doc.trigger_condition:
+			sub_rule_condition = sub_rule_doc.get_entry_condition()
+			if not skip_conditions and sub_rule_condition:
 				if not sub_rule_doc.compiled_expression:
 					from flexirule.ruleflow.core.compiler import ConditionCompiler
 
-					sub_rule_doc.compiled_expression = ConditionCompiler().compile(
-						sub_rule_doc.trigger_condition
-					)
+					sub_rule_doc.compiled_expression = ConditionCompiler().compile(sub_rule_condition)
 
 				caller_rule_meta = {
 					"name": engine.rule.name,

flexirule/ruleflow/core/coordinator.py

@@ -243,7 +243,12 @@ def _build_runtime_registry() -> dict:
 				"execution_mode": row.get("execution_mode") or "Synchronous",
 				"debug_mode": bool(row.get("debug_mode")),
 				"compiled_expression": row.get("compiled_expression") or "",
-				"has_trigger_condition": bool(row.get("trigger_condition")),
+				"has_trigger_condition": bool(row.get("trigger_condition"))
+				or bool(
+					frappe.db.get_value(
+						"Rule Action", {"parent": rule_name, "action_type": "Entry Action"}, "config"
+					)
+				),
 				# Only use explicitly configured watched fields for dispatch-time pruning.
 				# Trigger-condition dependencies are evaluated in eligibility and are not safe skip keys.
 				"watched_fields": sorted(set(explicit_watched)),
@@ -617,11 +622,11 @@ def _has_field(doctype, fieldname):
 				return False, _("Trigger Evaluation Error: {0}").format(str(e))
 
 		# Conditions MUST be pre-compiled - no runtime JSON parsing
-		elif rule_doc.get("trigger_condition"):
+		elif rule_doc.get_entry_condition():
 			try:
 				import json
 
-				parsed = json.loads(rule_doc.get("trigger_condition"))
+				parsed = rule_doc.get_entry_condition()
 				if isinstance(parsed, dict) and not parsed.get("conditions"):
 					return True, _("Eligible")
 				if isinstance(parsed, list) and not parsed:

flexirule/ruleflow/doctype/rule/rule.py

@@ -514,9 +514,17 @@ def resolve_entry_condition(self):
 					if isinstance(self.trigger_condition, str)
 					else self.trigger_condition
 				)
-				return legacy_config
+				# Check if legacy config is non-empty
+				if legacy_config and (
+					isinstance(legacy_config, list)
+					or (
+						isinstance(legacy_config, dict)
+						and (legacy_config.get("conditions") or legacy_config.get("collection"))
+					)
+				):
+					return legacy_config
 			except (ValueError, TypeError):
-				return None
+				pass
 
 		return config
 

flexirule/ruleflow/doctype/rule/test_migration.py

@@ -0,0 +1,109 @@
+import json
+
+import frappe
+from frappe.tests.utils import FrappeTestCase
+
+
+class TestTriggerConditionMigration(FrappeTestCase):
+	def setUp(self):
+		self.rule_name = "Migration Test Rule"
+		if frappe.db.exists("Rule", self.rule_name):
+			frappe.delete_doc("Rule", self.rule_name)
+
+		# Create a rule with legacy trigger_condition
+		self.condition = {
+			"op": "and",
+			"conditions": [{"left": {"ref": "doc.status"}, "op": "==", "right": {"value": "Open"}}],
+		}
+
+		self.rule = frappe.get_doc(
+			{
+				"doctype": "Rule",
+				"rule_name": self.rule_name,
+				"document_type": "User",
+				"trigger_type": "DocType Event",
+				"trigger_event": "Before Save",
+				"trigger_condition": json.dumps(self.condition),
+				"actions": [
+					{
+						"action_id": "root",
+						"action_type": "Entry Action",
+						"action_label": "Start",
+						"next_step_if_true": "stop_node",
+					},
+					{
+						"action_id": "stop_node",
+						"action_type": "Stop",
+						"action_label": "Stop",
+						"operation": "Success",
+					},
+				],
+			}
+		)
+		self.rule.insert(ignore_permissions=True)
+		# Clear config of entry action to simulate Case A
+		entry_action = self.rule.get_entry_action()
+		frappe.db.set_value("Rule Action", entry_action.name, "config", None)
+
+	def test_migration_case_a_clean_move(self):
+		from flexirule.patches.v1_0.migrate_trigger_condition_to_entry_action import execute
+
+		execute()
+
+		# Reload
+		self.rule.reload()
+		entry_action = self.rule.get_entry_action()
+
+		# Verify migrated
+		self.assertEqual(json.loads(entry_action.config), self.condition)
+		# Verify legacy field is NOT cleared
+		self.assertEqual(json.loads(self.rule.trigger_condition), self.condition)
+		# Verify compiled_expression is updated
+		self.assertTrue(self.rule.compiled_expression)
+
+	def test_migration_idempotency(self):
+		from flexirule.patches.v1_0.migrate_trigger_condition_to_entry_action import execute
+
+		# Run once
+		execute()
+		self.rule.reload()
+		first_config = self.rule.get_entry_action().config
+
+		# Run again
+		execute()
+		self.rule.reload()
+		second_config = self.rule.get_entry_action().config
+
+		self.assertEqual(first_config, second_config)
+
+	def test_migration_case_b_ambiguity(self):
+		# Set different config in Entry Action
+		other_condition = {
+			"op": "and",
+			"conditions": [{"left": {"ref": "doc.status"}, "op": "==", "right": {"value": "Closed"}}],
+		}
+		entry_action = self.rule.get_entry_action()
+		frappe.db.set_value("Rule Action", entry_action.name, "config", json.dumps(other_condition))
+
+		# Mock log_error to verify it was called since Error Log is proving tricky in test transaction
+		import flexirule.patches.v1_0.migrate_trigger_condition_to_entry_action as patch_mod
+		from flexirule.patches.v1_0.migrate_trigger_condition_to_entry_action import execute
+
+		original_log = patch_mod.frappe.log_error
+		log_called = [False]
+
+		def mock_log(msg, title):
+			log_called[0] = True
+
+		patch_mod.frappe.log_error = mock_log
+		try:
+			execute()
+		finally:
+			patch_mod.frappe.log_error = original_log
+
+		self.assertTrue(log_called[0])
+
+		# Verify NOT migrated
+		self.rule.reload()
+		entry_action = self.rule.get_entry_action()
+		self.assertEqual(json.loads(entry_action.config), other_condition)