Merge pull request #19238 from ahmedhamidawan/fix_cancelling_invocation

[24.2] Fix cancellation of workflow scheduling

client/src/components/ProgressBar.vue

@@ -4,9 +4,9 @@
 import { BProgress, BProgressBar } from "bootstrap-vue";
 
 interface Props {
-    total: number;
+    total?: number;
     note: string;
-    loading: boolean;
+    loading?: boolean;
     okCount?: number;
     runningCount?: number;
     newCount?: number;

client/src/components/Tool/ToolCard.vue

@@ -118,7 +118,7 @@ const showHelpForum = computed(() => isConfigLoaded.value && config.value.enable
 
 <template>
     <div class="position-relative">
-        <div class="underlay sticky-top" />
+        <div class="ui-form-header-underlay sticky-top" />
         <div class="tool-header sticky-top bg-secondary px-2 py-1 rounded">
             <div class="d-flex justify-content-between">
                 <div class="py-1 d-flex flex-wrap flex-gapx-1">
@@ -209,19 +209,6 @@ const showHelpForum = computed(() => isConfigLoaded.value && config.value.enable
 </template>
 
 <style lang="scss" scoped>
-@import "scss/theme/blue.scss";
-
-.underlay::after {
-    content: "";
-    display: block;
-    position: absolute;
-    top: -$margin-h;
-    left: -0.5rem;
-    right: -0.5rem;
-    height: 50px;
-    background: linear-gradient($white 75%, change-color($white, $alpha: 0));
-}
-
 .fa-wrench {
     cursor: unset;
 }

client/src/components/Workflow/Invocation/Graph/InvocationGraph.vue

@@ -34,8 +34,6 @@ interface Props {
     workflow: Workflow;
     /** Whether the invocation is terminal */
     isTerminal: boolean;
-    /** Whether the invocation is scheduled */
-    isScheduled: boolean;
     /** The zoom level for the graph */
     zoom?: number;
     /** Whether to show the minimap */

client/src/components/Workflow/Run/WorkflowRunFormSimple.vue

@@ -1,6 +1,7 @@
 <template>
     <div>
-        <div v-if="isConfigLoaded">
+        <div class="ui-form-header-underlay sticky-top" />
+        <div v-if="isConfigLoaded" class="sticky-top">
             <BAlert v-if="!canRunOnHistory" variant="warning" show>
                 <span v-localize>
                     The workflow cannot run because the current history is immutable. Please select a different history

client/src/components/Workflow/WorkflowNavigationTitle.test.ts

@@ -66,17 +66,13 @@ const localVue = getLocalVue();
  * @param version The version of the component to mount (`run_form` or `invocation` view)
  * @param ownsWorkflow Whether the user owns the workflow associated with the invocation
  * @param unimportableWorkflow Whether the workflow import should fail
- * @returns The wrapper object, and the mockRouter object
+ * @returns The wrapper object
  */
 async function mountWorkflowNavigationTitle(
     version: "run_form" | "invocation",
     ownsWorkflow = true,
     unimportableWorkflow = false
 ) {
-    const mockRouter = {
-        push: jest.fn(),
-    };
-
     let workflowId: string;
     let invocation;
     if (version === "invocation") {
@@ -96,9 +92,6 @@ async function mountWorkflowNavigationTitle(
             workflowId,
         },
         localVue,
-        mocks: {
-            $router: mockRouter,
-        },
         pinia: createTestingPinia(),
     });
 
@@ -107,7 +100,7 @@ async function mountWorkflowNavigationTitle(
         username: ownsWorkflow ? WORKFLOW_OWNER : OTHER_USER,
     });
 
-    return { wrapper, mockRouter };
+    return { wrapper };
 }
 
 describe("WorkflowNavigationTitle renders", () => {
@@ -116,10 +109,9 @@ describe("WorkflowNavigationTitle renders", () => {
 
         const heading = wrapper.find(SELECTORS.WORKFLOW_HEADING);
         expect(heading.text()).toContain(`Invoked Workflow: ${SAMPLE_WORKFLOW.name}`);
-        expect(heading.text()).toContain(`(version: ${SAMPLE_WORKFLOW.version + 1})`);
+        expect(heading.text()).toContain(`(Version: ${SAMPLE_WORKFLOW.version + 1})`);
 
-        const actionsGroup = wrapper.find(SELECTORS.ACTIONS_BUTTON_GROUP);
-        const runButton = actionsGroup.find(SELECTORS.ROUTE_TO_RUN_BUTTON);
+        const runButton = wrapper.find(SELECTORS.ROUTE_TO_RUN_BUTTON);
         expect(runButton.attributes("title")).toContain("Rerun");
         expect(runButton.attributes("title")).toContain(SAMPLE_WORKFLOW.name);
     });
@@ -129,24 +121,19 @@ describe("WorkflowNavigationTitle renders", () => {
 
         const heading = wrapper.find(SELECTORS.WORKFLOW_HEADING);
         expect(heading.text()).toContain(`Workflow: ${SAMPLE_WORKFLOW.name}`);
-        expect(heading.text()).toContain(`(version: ${SAMPLE_WORKFLOW.version + 1})`);
+        expect(heading.text()).toContain(`(Version: ${SAMPLE_WORKFLOW.version + 1})`);
 
-        const actionsGroup = wrapper.find(SELECTORS.ACTIONS_BUTTON_GROUP);
-        const runButton = actionsGroup.find(SELECTORS.EXECUTE_WORKFLOW_BUTTON);
+        const runButton = wrapper.find(SELECTORS.EXECUTE_WORKFLOW_BUTTON);
         expect(runButton.attributes("title")).toContain("Run");
     });
 
     it("edit button if user owns the workflow", async () => {
         async function findAndClickEditButton(version: "invocation" | "run_form") {
-            const { wrapper, mockRouter } = await mountWorkflowNavigationTitle(version);
+            const { wrapper } = await mountWorkflowNavigationTitle(version);
             const actionsGroup = wrapper.find(SELECTORS.ACTIONS_BUTTON_GROUP);
 
             const editButton = actionsGroup.find(SELECTORS.EDIT_WORKFLOW_BUTTON);
-            await editButton.trigger("click");
-            await flushPromises();
-
-            expect(mockRouter.push).toHaveBeenCalledTimes(1);
-            expect(mockRouter.push).toHaveBeenCalledWith(
+            expect(editButton.attributes("to")).toBe(
                 `/workflows/edit?id=${SAMPLE_WORKFLOW.id}&version=${SAMPLE_WORKFLOW.version}`
             );
         }

client/src/components/Workflow/WorkflowNavigationTitle.vue

@@ -5,7 +5,6 @@ import { BAlert, BButton, BButtonGroup } from "bootstrap-vue";
 import { storeToRefs } from "pinia";
 import { computed, ref } from "vue";
 import { RouterLink } from "vue-router";
-import { useRouter } from "vue-router/composables";
 
 import { isRegisteredUser } from "@/api";
 import type { WorkflowInvocationElementView } from "@/api/invocations";
@@ -21,8 +20,6 @@ import AsyncButton from "../Common/AsyncButton.vue";
 import ButtonSpinner from "../Common/ButtonSpinner.vue";
 import WorkflowRunButton from "./WorkflowRunButton.vue";
 
-const router = useRouter();
-
 interface Props {
     invocation?: WorkflowInvocationElementView;
     workflowId: string;
@@ -98,16 +95,14 @@ const workflowImportTitle = computed(() => {
 
         <BAlert v-if="error" variant="danger" show>{{ error }}</BAlert>
 
-        <div class="position-relative">
-            <div v-if="workflow" class="ui-portlet-section">
-                <div class="d-flex portlet-header align-items-center">
+        <div class="position-relative mb-2">
+            <div v-if="workflow" class="bg-secondary px-2 py-1 rounded">
+                <div class="d-flex align-items-center flex-gapx-1">
                     <div class="flex-grow-1" data-description="workflow heading">
-                        <div class="px-1">
-                            <FontAwesomeIcon :icon="faSitemap" />
-                            <b class="mx-1">
-                                {{ props.invocation ? "Invoked " : "" }}Workflow: {{ getWorkflowName() }}
-                            </b>
-                            <i>(version: {{ workflow.version + 1 }})</i>
+                        <div>
+                            <FontAwesomeIcon :icon="faSitemap" fixed-width />
+                            <b> {{ props.invocation ? "Invoked " : "" }}Workflow: {{ getWorkflowName() }} </b>
+                            <span>(Version: {{ workflow.version + 1 }})</span>
                         </div>
                     </div>
                     <BButtonGroup>
@@ -122,7 +117,7 @@ const workflowImportTitle = computed(() => {
                             "
                             variant="link"
                             :disabled="workflow.deleted"
-                            @click="router.push(`/workflows/edit?id=${workflow.id}&version=${workflow.version}`)">
+                            :to="`/workflows/edit?id=${workflow.id}&version=${workflow.version}`">
                             <FontAwesomeIcon :icon="faEdit" fixed-width />
                         </BButton>
                         <AsyncButton
@@ -138,30 +133,30 @@ const workflowImportTitle = computed(() => {
                         </AsyncButton>
 
                         <slot name="workflow-title-actions" />
-
-                        <ButtonSpinner
-                            v-if="!props.invocation"
-                            id="run-workflow"
-                            data-description="execute workflow button"
-                            :wait="runWaiting"
-                            :disabled="runDisabled"
-                            size="sm"
-                            title="Run"
-                            @onClick="emit('on-execute')" />
-                        <WorkflowRunButton
-                            v-else
-                            :id="workflow.id"
-                            data-description="route to workflow run button"
-                            variant="link"
-                            :title="
-                                !workflow.deleted
-                                    ? `<b>Rerun</b><br>${getWorkflowName()}`
-                                    : 'This workflow has been deleted.'
-                            "
-                            :disabled="workflow.deleted"
-                            full
-                            :version="workflow.version" />
                     </BButtonGroup>
+                    <ButtonSpinner
+                        v-if="!props.invocation"
+                        id="run-workflow"
+                        data-description="execute workflow button"
+                        :wait="runWaiting"
+                        :disabled="runDisabled"
+                        size="sm"
+                        title="Run Workflow"
+                        @onClick="emit('on-execute')" />
+                    <WorkflowRunButton
+                        v-else
+                        :id="workflow.id"
+                        data-description="route to workflow run button"
+                        variant="link"
+                        :title="
+                            !workflow.deleted
+                                ? `<b>Rerun</b><br>${getWorkflowName()}`
+                                : 'This workflow has been deleted.'
+                        "
+                        :disabled="workflow.deleted"
+                        force
+                        full
+                        :version="workflow.version" />
                 </div>
             </div>
             <div v-if="props.success" class="donemessagelarge">

client/src/components/Workflow/WorkflowRunButton.test.ts

@@ -1,4 +1,5 @@
-import { mount } from "@vue/test-utils";
+import { mount, type Wrapper } from "@vue/test-utils";
+import flushPromises from "flush-promises";
 import { getLocalVue } from "tests/jest/helpers";
 
 import { generateRandomString } from "./testUtils";
@@ -11,22 +12,69 @@ const WORKFLOW_ID = generateRandomString();
 const WORKFLOW_VERSION = 1;
 const WORKFLOW_RUN_BUTTON_SELECTOR = `[data-workflow-run="${WORKFLOW_ID}"]`;
 
-async function mountWorkflowRunButton(props?: { id: string; version: number; full?: boolean }) {
+function getPath() {
+    return `/workflows/run?id=${WORKFLOW_ID}&version=${WORKFLOW_VERSION}`;
+}
+
+async function mountWorkflowRunButton(
+    props?: { id: string; version: number; full?: boolean; force?: boolean },
+    currentPath?: string
+) {
+    const mockRouter = {
+        push: jest.fn(),
+        afterEach: jest.fn(),
+        currentRoute: {
+            fullPath: currentPath || getPath(),
+        },
+    };
+
     const wrapper = mount(WorkflowRunButton as object, {
         propsData: { ...props },
         localVue,
+        mocks: {
+            $router: mockRouter,
+        },
     });
 
-    return { wrapper };
+    return { wrapper, mockRouter };
+}
+
+async function clickButton(button: Wrapper<Vue>) {
+    // Remove the href and target attributes to prevent navigation error
+    // This is done because the `BButton` has a `:to` prop as well as an `@click` event
+    button.element.setAttribute("href", "javascript:void(0)");
+    button.element.setAttribute("target", "");
+
+    await button.trigger("click");
+    await flushPromises();
 }
 
 describe("WorkflowRunButton.vue", () => {
-    it("should render button with icon and route", async () => {
-        const { wrapper } = await mountWorkflowRunButton({ id: WORKFLOW_ID, version: WORKFLOW_VERSION });
+    it("should render button with icon and route to it", async () => {
+        const { wrapper, mockRouter } = await mountWorkflowRunButton({ id: WORKFLOW_ID, version: WORKFLOW_VERSION });
 
         const button = wrapper.find(WORKFLOW_RUN_BUTTON_SELECTOR);
         expect(button.attributes("title")).toBe("Run workflow");
         expect(button.text()).toBe("");
-        expect(button.attributes("href")).toBe(`/workflows/run?id=${WORKFLOW_ID}&version=${WORKFLOW_VERSION}`);
+        expect(button.attributes("href")).toBe(getPath());
+
+        await clickButton(button);
+
+        // Check that router.push was called with the correct arguments
+        expect(mockRouter.push).toHaveBeenCalledWith(getPath());
+    });
+
+    it("should force route if on the same path", async () => {
+        const { wrapper, mockRouter } = await mountWorkflowRunButton(
+            { id: WORKFLOW_ID, version: WORKFLOW_VERSION, force: true },
+            getPath()
+        );
+
+        const button = wrapper.find(WORKFLOW_RUN_BUTTON_SELECTOR);
+
+        await clickButton(button);
+
+        // Check that router.push was called with the correct arguments
+        expect(mockRouter.push).toHaveBeenCalledWith(getPath(), { force: true });
     });
 });

client/src/components/Workflow/WorkflowRunButton.vue

@@ -3,6 +3,7 @@ import { faPlay } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
 import { BButton } from "bootstrap-vue";
 import { computed } from "vue";
+import { useRoute, useRouter } from "vue-router/composables";
 
 interface Props {
     id: string;
@@ -20,13 +21,22 @@ const props = withDefaults(defineProps<Props>(), {
     variant: "primary",
 });
 
+const route = useRoute();
+const router = useRouter();
+
 const runPath = computed(
     () => `/workflows/run?id=${props.id}${props.version !== undefined ? `&version=${props.version}` : ""}`
 );
 
-function forceRunPath() {
-    if (props.force) {
-        window.open(runPath.value);
+function routeToPath() {
+    if (props.force && route.fullPath === runPath.value) {
+        // vue-router 4 supports a native force push with clean URLs,
+        // but we're using a __vkey__ bit as a workaround
+        // Only conditionally force to keep urls clean most of the time.
+        // @ts-ignore - monkeypatched router, drop with migration.
+        router.push(runPath.value, { force: true });
+    } else {
+        router.push(runPath.value);
     }
 }
 </script>
@@ -42,7 +52,7 @@ function forceRunPath() {
         class="text-decoration-none"
         :disabled="disabled"
         :to="runPath"
-        @click="forceRunPath">
+        @click="routeToPath">
         <FontAwesomeIcon :icon="faPlay" fixed-width />
 
         <span v-if="full" v-localize>Run</span>

client/src/components/WorkflowInvocationState/WorkflowInvocationOverview.vue

@@ -15,7 +15,6 @@ import InvocationMessage from "@/components/WorkflowInvocationState/InvocationMe
 interface Props {
     invocation: WorkflowInvocationElementView;
     invocationAndJobTerminal: boolean;
-    invocationSchedulingTerminal: boolean;
     isFullPage?: boolean;
     isSubworkflow?: boolean;
 }
@@ -62,7 +61,6 @@ const uniqueMessages = computed(() => {
                 :invocation="invocation"
                 :workflow="workflow"
                 :is-terminal="invocationAndJobTerminal"
-                :is-scheduled="invocationSchedulingTerminal"
                 :is-full-page="isFullPage"
                 :show-minimap="isFullPage" />
         </div>