Repository: kestra-io/kestra (java)

Issue:
Show warning when manually executing a flow containing a `{{ trigger.uri }}` variable
### Feature description

When a user attempts to execute a flow that has a trigger variable such as `{{ trigger.uri }}`, show a warning in the UI explaining that such a variable cannot be resolved in a manual execution:

"The trigger variable used in this flow cannot be parsed in a manual execution."

Ideally (if possible), point to the trigger variable used:

"The trigger variable XXX used in this flow cannot be parsed in manually triggered executions." e.g.:
"The trigger variable `{{ trigger.uri }}` used in this flow cannot be parsed in manually triggered executions."


We can consider adding this as a question in the logs:

"Are you trying to manually execute a flow with trigger variables?"
- risk of FP and FN

Pull request:
feat(ui): warn usage of trigger variable in flow when executing
close #2126 

Patch:
diff --git a/core/src/main/java/io/kestra/core/services/PluginDefaultService.java b/core/src/main/java/io/kestra/core/services/PluginDefaultService.java
index cfd2657de..4bbe85afc 100644
--- a/core/src/main/java/io/kestra/core/services/PluginDefaultService.java
+++ b/core/src/main/java/io/kestra/core/services/PluginDefaultService.java
@@ -68,6 +68,10 @@ public class PluginDefaultService {
     protected List<PluginDefault> mergeAllDefaults(Flow flow) {
         List<PluginDefault> list = new ArrayList<>();
 
+        if (flow.getPluginDefaults() != null) {
+            list.addAll(flow.getPluginDefaults());
+        }
+
         if (taskGlobalDefault != null && taskGlobalDefault.getDefaults() != null) {
             if (warnOnce.compareAndSet(false, true)) {
                 log.warn("Global Task Defaults are deprecated, please use Global Plugin Defaults instead via the 'kestra.plugins.defaults' property.");
@@ -79,10 +83,6 @@ public class PluginDefaultService {
             list.addAll(pluginGlobalDefault.getDefaults());
         }
 
-        if (flow.getPluginDefaults() != null) {
-            list.addAll(flow.getPluginDefaults());
-        }
-
         return list;
     }
 
diff --git a/ui/src/components/flows/FlowRootTopBar.vue b/ui/src/components/flows/FlowRootTopBar.vue
index 872f17aff..c72b6dceb 100644
--- a/ui/src/components/flows/FlowRootTopBar.vue
+++ b/ui/src/components/flows/FlowRootTopBar.vue
@@ -29,6 +29,7 @@
                         :disabled="flow.disabled"
                         :flow-id="flow.id"
                         :namespace="flow.namespace"
+                        :flow-source="flow.source"
                     />
                 </li>
             </ul>
diff --git a/ui/src/components/flows/TriggerFlow.vue b/ui/src/components/flows/TriggerFlow.vue
index 79096f8de..ccbbee856 100644
--- a/ui/src/components/flows/TriggerFlow.vue
+++ b/ui/src/components/flows/TriggerFlow.vue
@@ -76,6 +76,10 @@
             type: {
                 type: String,
                 default: "primary"
+            },
+            flowSource: {
+                type: String,
+                default: null
             }
         },
         data() {
@@ -104,9 +108,19 @@
                     });
                     this.isOpen = !this.isOpen;
                     return;
-                } else if (this.computedNamespace !== undefined && this.computedFlowId !== undefined) {
+                }
+                else if (this.checkForTrigger) {
+                    this.$toast().confirm(
+                        this.$t("trigger_check_warning"),
+                        () => {
+                            this.isOpen = !this.isOpen;
+                        },
+                        () => {});
+                }
+                else if (this.computedNamespace !== undefined && this.computedFlowId !== undefined) {
                     this.isOpen = !this.isOpen;
-                } else {
+                }
+                else {
                     this.$store.dispatch("execution/loadNamespaces");
                     this.isSelectFlowOpen = !this.isSelectFlowOpen;
                 }
@@ -146,6 +160,13 @@
             },
             computedNamespace() {
                 return this.namespace || this.localNamespace;
+            },
+            checkForTrigger() {
+                if (this.flowSource) {
+                    const triggerRegex = /\{\{\s*\(?\s*(\|\||&&)?\s*trigger\s*(\.\w+|\|\s*\w+)?\s*\}\}/;
+                    return triggerRegex.test(this.flowSource);
+                }
+                return false;
             }
         },
         watch: {
diff --git a/ui/src/translations/en.json b/ui/src/translations/en.json
index 22f591dc5..cd2532fa7 100644
--- a/ui/src/translations/en.json
+++ b/ui/src/translations/en.json
@@ -842,6 +842,7 @@
     "data": "Data",
     "webhook link copied": "Webhook link copied.",
     "copy url": "Copy URL",
-    "scheduleDate": "Schedule date"
+    "scheduleDate": "Schedule date",
+    "trigger_check_warning": "Warning: Usage of the `trigger` variable detected, executing your flow manually won't fullfill the trigger variable."
   }
 }
\ No newline at end of file


Test patch:
diff --git a/core/src/test/java/io/kestra/core/services/PluginDefaultServiceTest.java b/core/src/test/java/io/kestra/core/services/PluginDefaultServiceTest.java
index 995b85ffb..3060f4ec3 100644
--- a/core/src/test/java/io/kestra/core/services/PluginDefaultServiceTest.java
+++ b/core/src/test/java/io/kestra/core/services/PluginDefaultServiceTest.java
@@ -25,12 +25,16 @@ import lombok.ToString;
 import lombok.experimental.SuperBuilder;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.time.Duration;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Stream;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
@@ -99,6 +103,51 @@ class PluginDefaultServiceTest {
         ), result);
     }
 
+    @ParameterizedTest
+    @MethodSource
+    void flowDefaultsOverrideGlobalDefaults(boolean flowDefaultForced, boolean globalDefaultForced, String fooValue, String barValue, String bazValue) {
+        final DefaultPrecedenceTester task = DefaultPrecedenceTester.builder()
+            .id("test")
+            .type(DefaultPrecedenceTester.class.getName())
+            .propBaz("taskValue")
+            .build();
+
+        final PluginDefault flowDefault = new PluginDefault(DefaultPrecedenceTester.class.getName(), flowDefaultForced, ImmutableMap.of(
+            "propBar", "flowValue",
+            "propBaz", "flowValue"
+        ));
+        final PluginDefault globalDefault = new PluginDefault(DefaultPrecedenceTester.class.getName(), globalDefaultForced, ImmutableMap.of(
+            "propFoo", "globalValue",
+            "propBar", "globalValue",
+            "propBaz", "globalValue"
+        ));
+
+        final Flow flowWithPluginDefault = Flow.builder()
+            .tasks(Collections.singletonList(task))
+            .pluginDefaults(List.of(flowDefault))
+            .build();
+
+        final PluginGlobalDefaultConfiguration pluginGlobalDefaultConfiguration = new PluginGlobalDefaultConfiguration();
+        pluginGlobalDefaultConfiguration.defaults = List.of(globalDefault);
+
+        pluginDefaultService.pluginGlobalDefault = pluginGlobalDefaultConfiguration;
+
+        final Flow injected = pluginDefaultService.injectDefaults(flowWithPluginDefault);
+
+        assertThat(((DefaultPrecedenceTester) injected.getTasks().getFirst()).getPropFoo(), is(fooValue));
+        assertThat(((DefaultPrecedenceTester) injected.getTasks().getFirst()).getPropBar(), is(barValue));
+        assertThat(((DefaultPrecedenceTester) injected.getTasks().getFirst()).getPropBaz(), is(bazValue));
+    }
+
+    private static Stream<Arguments> flowDefaultsOverrideGlobalDefaults() {
+        return Stream.of(
+            Arguments.of(false, false, "globalValue", "flowValue", "taskValue"),
+            Arguments.of(false, true, "globalValue", "globalValue", "globalValue"),
+            Arguments.of(true, false, "globalValue", "flowValue", "flowValue"),
+            Arguments.of(true, true, "globalValue", "flowValue", "flowValue")
+        );
+    }
+
     @Test
     public void injectFlowAndGlobals() {
         DefaultTester task = DefaultTester.builder()
@@ -297,4 +346,23 @@ class PluginDefaultServiceTest {
             private Map<String, String> val;
         }
     }
+
+    @SuperBuilder
+    @ToString
+    @EqualsAndHashCode
+    @Getter
+    @NoArgsConstructor
+    @Plugin(aliases = "io.kestra.core.services.DefaultPrecedenceTesterAlias")
+    public static class DefaultPrecedenceTester extends Task implements RunnableTask<VoidOutput> {
+        private String propFoo;
+
+        private String propBar;
+
+        private String propBaz;
+
+        @Override
+        public VoidOutput run(RunContext runContext) throws Exception {
+            return null;
+        }
+    }
 }
\ No newline at end of file