From f17b762596d1fb030d41ef108b47245d1a2a0975 Mon Sep 17 00:00:00 2001
From: Thales Macedo Garitezi <thalesmg@gmail.com>
Date: Mon, 6 Nov 2023 10:09:14 -0300
Subject: [PATCH] chore: don't disable rule that references non-existent bridge

After feedback from QA, we decided to rollback enforcing the rule to be disabled.
---
 ...qx_bridge_v1_compatibility_layer_SUITE.erl | 14 +++++-----
 .../emqx_rule_engine/src/emqx_rule_engine.erl | 26 +++++++++----------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl
index 843b58d8f..571a0a0e1 100644
--- a/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl
+++ b/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl
@@ -732,7 +732,7 @@ t_scenario_2(Config) ->
     %% ===================================================================================
     %% Try to create a rule referencing a non-existent bridge.  It succeeds, but it's
     %% implicitly disabled.  Trying to update it later without creating the bridge should
-    %% keep it disabled.
+    %% allow it to be enabled.
     %% ===================================================================================
     BridgeName = <<"scenario2">>,
     RuleTopic = <<"t/scenario2">>,
@@ -746,9 +746,9 @@ t_scenario_2(Config) ->
             ],
             #{overrides => #{enable => true}}
         ),
-    ?assertNot(is_rule_enabled(RuleId0)),
+    ?assert(is_rule_enabled(RuleId0)),
     ?assertMatch({ok, {{_, 200, _}, _, _}}, enable_rule_http(RuleId0)),
-    ?assertNot(is_rule_enabled(RuleId0)),
+    ?assert(is_rule_enabled(RuleId0)),
 
     %% ===================================================================================
     %% Now we create the bridge, and attempt to create a new enabled rule.  It should
@@ -768,13 +768,13 @@ t_scenario_2(Config) ->
             ],
             #{overrides => #{enable => true}}
         ),
-    ?assertNot(is_rule_enabled(RuleId0)),
+    ?assert(is_rule_enabled(RuleId0)),
     ?assert(is_rule_enabled(RuleId1)),
     ?assertMatch({ok, {{_, 200, _}, _, _}}, enable_rule_http(RuleId0)),
     ?assert(is_rule_enabled(RuleId0)),
 
     %% ===================================================================================
-    %% Creating a rule with mixed existent/non-existent bridges should deny enabling it.
+    %% Creating a rule with mixed existent/non-existent bridges should allow enabling it.
     %% ===================================================================================
     NonExistentBridgeName = <<"scenario2_not_created">>,
     {ok, #{<<"id">> := RuleId2}} =
@@ -801,8 +801,8 @@ t_scenario_2(Config) ->
                 }
             }
         ),
-    ?assertNot(is_rule_enabled(RuleId2)),
+    ?assert(is_rule_enabled(RuleId2)),
     ?assertMatch({ok, {{_, 200, _}, _, _}}, enable_rule_http(RuleId2)),
-    ?assertNot(is_rule_enabled(RuleId2)),
+    ?assert(is_rule_enabled(RuleId2)),
 
     ok.
diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.erl b/apps/emqx_rule_engine/src/emqx_rule_engine.erl
index 612cb5ff3..aadd3d4f5 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_engine.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_engine.erl
@@ -478,20 +478,18 @@ with_parsed_rule(Params = #{id := RuleId, sql := Sql, actions := Actions}, Creat
                 %% -- calculated fields end
             },
             InputEnable = maps:get(enable, Params, true),
-            Enable =
-                case validate_bridge_existence_in_actions(Rule0) of
-                    ok ->
-                        InputEnable;
-                    {error, NonExistentBridgeIDs} ->
-                        ?SLOG(error, #{
-                            msg => "action_references_nonexistent_bridges",
-                            rule_id => RuleId,
-                            nonexistent_bridge_ids => NonExistentBridgeIDs,
-                            hint => "this rule will be disabled"
-                        }),
-                        false
-                end,
-            Rule = Rule0#{enable => Enable},
+            case validate_bridge_existence_in_actions(Rule0) of
+                ok ->
+                    ok;
+                {error, NonExistentBridgeIDs} ->
+                    ?SLOG(error, #{
+                        msg => "action_references_nonexistent_bridges",
+                        rule_id => RuleId,
+                        nonexistent_bridge_ids => NonExistentBridgeIDs,
+                        hint => "this rule will be disabled"
+                    })
+            end,
+            Rule = Rule0#{enable => InputEnable},
             ok = Fun(Rule),
             {ok, Rule};
         {error, Reason} ->