From 9e4e81cec5b280135dc937fde0c6f322da520cf1 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Thu, 1 Feb 2024 10:24:23 -0600
Subject: [PATCH 01/17] fix request handling on client for WorkflowEditor
 create/save as

---
 .../src/components/Workflow/Editor/Index.vue  | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue
index 122285db6b0..fd94a48c053 100644
--- a/client/src/components/Workflow/Editor/Index.vue
+++ b/client/src/components/Workflow/Editor/Index.vue
@@ -178,6 +178,7 @@ import { hide_modal } from "@/layout/modal";
 import { getAppRoot } from "@/onload/loadConfig";
 import { useScopePointerStore } from "@/stores/scopePointerStore";
 import { LastQueue } from "@/utils/promise-queue";
+import { errorMessageAsString } from "@/utils/simple-error";
 
 import { defaultPosition } from "./composables/useDefaultStepPosition";
 import { fromSimple, toSimple } from "./modules/model";
@@ -555,7 +556,15 @@ export default {
                 this.hasChanges = false;
                 await this.routeToWorkflow(newId);
             } catch (e) {
-                this.onWorkflowError("Saving workflow failed, please contact an administrator.");
+                if (create) {
+                    throw e;
+                }
+                const errorHeading = `Saving workflow as '${rename_name}' failed`;
+                this.onWorkflowError(errorHeading, errorMessageAsString(e) || "Please contact an administrator.", {
+                    Ok: () => {
+                        this.hideModal();
+                    },
+                });
             }
         },
         onSaveAs() {
@@ -617,13 +626,15 @@ export default {
                     );
                 }
             } catch (e) {
-                this.onWorkflowError("Creating workflow failed"),
-                    e || "Please contact an administrator.",
+                this.onWorkflowError(
+                    "Creating workflow failed",
+                    errorMessageAsString(e) || "Please contact an administrator.",
                     {
                         Ok: () => {
                             this.hideModal();
                         },
-                    };
+                    }
+                );
             }
         },
         nameValidate() {

From 194f6bdecbdc8b291bc24a6eec69f1d34a820660 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Thu, 1 Feb 2024 10:15:42 -0600
Subject: [PATCH 02/17] use `api/workflows` endpoint instead of
 `save_workflow_as` controller

---
 .../src/components/Workflow/Editor/Index.vue  | 44 +++++--------------
 client/src/components/Workflow/services.js    | 12 +++++
 2 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue
index fd94a48c053..4772e9554c5 100644
--- a/client/src/components/Workflow/Editor/Index.vue
+++ b/client/src/components/Workflow/Editor/Index.vue
@@ -180,6 +180,7 @@ import { useScopePointerStore } from "@/stores/scopePointerStore";
 import { LastQueue } from "@/utils/promise-queue";
 import { errorMessageAsString } from "@/utils/simple-error";
 
+import { Services } from "../services";
 import { defaultPosition } from "./composables/useDefaultStepPosition";
 import { fromSimple, toSimple } from "./modules/model";
 import { getModule, getVersions, loadWorkflow, saveWorkflow } from "./modules/services";
@@ -275,6 +276,7 @@ export default {
         });
 
         function resetStores() {
+            hasChanges.value = false;
             connectionStore.$reset();
             stepStore.$reset();
             stateStore.$reset();
@@ -319,6 +321,7 @@ export default {
             annotation: null,
             name: "Unnamed Workflow",
             tags: this.workflowTags,
+            services: null,
             stateMessages: [],
             insertedStateMessages: [],
             refactorActions: [],
@@ -382,6 +385,7 @@ export default {
         },
     },
     async created() {
+        this.services = new Services();
         this.lastQueue = new LastQueue();
         await this._loadCurrent(this.id, this.version);
         hide_modal();
@@ -533,9 +537,9 @@ export default {
         onDownload() {
             window.location = `${getAppRoot()}api/workflows/${this.id}/download?format=json-download`;
         },
-        async doSaveAs(create = false) {
-            const rename_name = create ? this.name : this.saveAsName ?? `SavedAs_${this.name}`;
-            const rename_annotation = create ? this.annotation || "" : this.saveAsAnnotation ?? "";
+        async doSaveAs() {
+            const rename_name = this.saveAsName ?? `SavedAs_${this.name}`;
+            const rename_annotation = this.saveAsAnnotation ?? "";
 
             // This is an old web controller endpoint that wants form data posted...
             const formData = new FormData();
@@ -547,18 +551,9 @@ export default {
             try {
                 const response = await axios.post(`${getAppRoot()}workflow/save_workflow_as`, formData);
                 const newId = response.data;
-
-                if (!create) {
-                    this.name = rename_name;
-                    this.annotation = rename_annotation;
-                }
-
                 this.hasChanges = false;
                 await this.routeToWorkflow(newId);
             } catch (e) {
-                if (create) {
-                    throw e;
-                }
                 const errorHeading = `Saving workflow as '${rename_name}' failed`;
                 this.onWorkflowError(errorHeading, errorMessageAsString(e) || "Please contact an administrator.", {
                     Ok: () => {
@@ -605,26 +600,11 @@ export default {
                 return;
             }
             try {
-                // if nothing other than payload vars changed, just use `create` endpoint
-                if (!this.hasChanges) {
-                    const payload = {
-                        workflow_name: this.name,
-                        workflow_annotation: this.annotation || "",
-                        workflow_tags: this.tags,
-                    };
-                    const { data } = await axios.put(`${getAppRoot()}workflow/create`, payload);
-                    const { id, message } = data;
-
-                    await this.routeToWorkflow(id);
-                    Toast.success(message);
-                } else {
-                    // otherwise, use `save_as` endpoint to include steps, etc.
-                    await this.doSaveAs(true);
-                    const stepCount = Object.keys(this.steps).length;
-                    Toast.success(
-                        `Created workflow ${this.name} with ${stepCount} ${stepCount === 1 ? "step" : "steps"}.`
-                    );
-                }
+                const { id, name, number_of_steps } = await this.services.createWorkflow(this);
+                const message = `Created new workflow '${name}' with ${number_of_steps} steps.`;
+                this.hasChanges = false;
+                await this.routeToWorkflow(id);
+                Toast.success(message);
             } catch (e) {
                 this.onWorkflowError(
                     "Creating workflow failed",
diff --git a/client/src/components/Workflow/services.js b/client/src/components/Workflow/services.js
index e7a0f0fcaa3..d354fda8182 100644
--- a/client/src/components/Workflow/services.js
+++ b/client/src/components/Workflow/services.js
@@ -3,6 +3,8 @@ import axios from "axios";
 import { withPrefix } from "utils/redirect";
 import { rethrowSimple } from "utils/simple-error";
 
+import { toSimple } from "./Editor/modules/model";
+
 /** Workflow data request helper **/
 export class Services {
     async copyWorkflow(workflow) {
@@ -27,6 +29,16 @@ export class Services {
         }
     }
 
+    async createWorkflow(workflow) {
+        const url = withPrefix("/api/workflows");
+        try {
+            const { data } = await axios.post(url, { workflow: toSimple(workflow.id, workflow) });
+            return data;
+        } catch (e) {
+            rethrowSimple(e);
+        }
+    }
+
     async deleteWorkflow(id) {
         const url = withPrefix(`/api/workflows/${id}`);
         try {

From 5f6090291cb6acf68004f61059d5d21845797e5c Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Thu, 1 Feb 2024 10:16:58 -0600
Subject: [PATCH 03/17] fix workflow editor creater bugs

Prevent the "expected Object got Array" error message.
Ensure `CreatorEditor` is loaded with existing creator(s).
---
 client/src/components/SchemaOrg/CreatorEditor.vue    | 8 ++++++--
 client/src/components/Workflow/Editor/Attributes.vue | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/client/src/components/SchemaOrg/CreatorEditor.vue b/client/src/components/SchemaOrg/CreatorEditor.vue
index 0a0e1d0066d..1310a1a2dfa 100644
--- a/client/src/components/SchemaOrg/CreatorEditor.vue
+++ b/client/src/components/SchemaOrg/CreatorEditor.vue
@@ -52,6 +52,7 @@ export default {
     props: {
         creators: {
             type: Array,
+            default: () => [],
         },
     },
     data() {
@@ -61,8 +62,11 @@ export default {
         };
     },
     watch: {
-        creators() {
-            this.creatorsCurrent = this.creators;
+        creators: {
+            handler(newCreators) {
+                this.creatorsCurrent = newCreators;
+            },
+            immediate: true,
         },
     },
     methods: {
diff --git a/client/src/components/Workflow/Editor/Attributes.vue b/client/src/components/Workflow/Editor/Attributes.vue
index 2247b4dcad4..062635e6567 100644
--- a/client/src/components/Workflow/Editor/Attributes.vue
+++ b/client/src/components/Workflow/Editor/Attributes.vue
@@ -97,7 +97,7 @@ export default {
             default: "",
         },
         creator: {
-            type: Object,
+            type: Array,
             default: null,
         },
         version: {

From 93e0725f0ee96a405c8b9d677da6e9326c7de340 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Thu, 1 Feb 2024 10:57:51 -0600
Subject: [PATCH 04/17] use POST api/workflows instead of `save_workflow_as`
 controller

---
 .../src/components/Workflow/Editor/Index.vue  | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue
index 4772e9554c5..84b079f6f53 100644
--- a/client/src/components/Workflow/Editor/Index.vue
+++ b/client/src/components/Workflow/Editor/Index.vue
@@ -164,7 +164,6 @@
 </template>
 
 <script>
-import axios from "axios";
 import { Toast } from "composables/toast";
 import { storeToRefs } from "pinia";
 import Vue, { computed, onUnmounted, ref, unref } from "vue";
@@ -182,7 +181,7 @@ import { errorMessageAsString } from "@/utils/simple-error";
 
 import { Services } from "../services";
 import { defaultPosition } from "./composables/useDefaultStepPosition";
-import { fromSimple, toSimple } from "./modules/model";
+import { fromSimple } from "./modules/model";
 import { getModule, getVersions, loadWorkflow, saveWorkflow } from "./modules/services";
 import { getStateUpgradeMessages } from "./modules/utilities";
 import reportDefault from "./reportDefault";
@@ -538,21 +537,19 @@ export default {
             window.location = `${getAppRoot()}api/workflows/${this.id}/download?format=json-download`;
         },
         async doSaveAs() {
+            if (!this.saveAsName && !this.nameValidate()) {
+                return;
+            }
             const rename_name = this.saveAsName ?? `SavedAs_${this.name}`;
             const rename_annotation = this.saveAsAnnotation ?? "";
 
-            // This is an old web controller endpoint that wants form data posted...
-            const formData = new FormData();
-            formData.append("workflow_name", rename_name);
-            formData.append("workflow_annotation", rename_annotation);
-            formData.append("from_tool_form", true);
-            formData.append("workflow_data", JSON.stringify(toSimple(this.id, this)));
-
             try {
-                const response = await axios.post(`${getAppRoot()}workflow/save_workflow_as`, formData);
-                const newId = response.data;
+                const newSaveAsWf = { ...this, name: rename_name, annotation: rename_annotation };
+                const { id, name, number_of_steps } = await this.services.createWorkflow(newSaveAsWf);
+                const message = `Created new workflow '${name}' with ${number_of_steps} steps.`;
                 this.hasChanges = false;
-                await this.routeToWorkflow(newId);
+                await this.routeToWorkflow(id);
+                Toast.success(message);
             } catch (e) {
                 const errorHeading = `Saving workflow as '${rename_name}' failed`;
                 this.onWorkflowError(errorHeading, errorMessageAsString(e) || "Please contact an administrator.", {

From ae982a2afbb199ed5b20e59d233b3a2a49d4e6a6 Mon Sep 17 00:00:00 2001
From: nuwang <2070605+nuwang@users.noreply.github.com>
Date: Fri, 9 Feb 2024 12:03:24 +0530
Subject: [PATCH 05/17] Add initializers to filesource options

---
 lib/galaxy/files/sources/__init__.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/files/sources/__init__.py b/lib/galaxy/files/sources/__init__.py
index d369fcfe429..cc6c854a36c 100644
--- a/lib/galaxy/files/sources/__init__.py
+++ b/lib/galaxy/files/sources/__init__.py
@@ -53,13 +53,18 @@ class FilesSourceProperties(TypedDict):
 
 
 class FilesSourceOptions:
-    """Options to control behaviour of filesource operations, such as realize_to and write_from"""
+    """Options to control behavior of file source operations, such as realize_to, write_from and list."""
+
+    # Indicates access to the FS operation with intent to write.
+    # Even if a file source is "writeable" some directories (or elements) may be restricted or read-only
+    # so those should be skipped while browsing with writeable=True.
+    writeable: Optional[bool] = False
 
     # Property overrides for values initially configured through the constructor. For example
     # the HTTPFilesSource passes in additional http_headers through these properties, which
     # are merged with constructor defined http_headers. The interpretation of these properties
     # are filesystem specific.
-    extra_props: Optional[FilesSourceProperties]
+    extra_props: Optional[FilesSourceProperties] = {}
 
 
 class SingleFileSource(metaclass=abc.ABCMeta):

From bffb7a99c50aeaafe8f4314d844f23d602e8f010 Mon Sep 17 00:00:00 2001
From: nuwang <2070605+nuwang@users.noreply.github.com>
Date: Fri, 9 Feb 2024 15:57:29 +0530
Subject: [PATCH 06/17] Use dataclass to initialize default values

---
 lib/galaxy/files/sources/__init__.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/files/sources/__init__.py b/lib/galaxy/files/sources/__init__.py
index cc6c854a36c..24cf91f654e 100644
--- a/lib/galaxy/files/sources/__init__.py
+++ b/lib/galaxy/files/sources/__init__.py
@@ -1,4 +1,5 @@
 import abc
+from dataclasses import dataclass, field
 import os
 import time
 from typing import (
@@ -52,6 +53,7 @@ class FilesSourceProperties(TypedDict):
     browsable: NotRequired[bool]
 
 
+@dataclass
 class FilesSourceOptions:
     """Options to control behavior of file source operations, such as realize_to, write_from and list."""
 
@@ -64,7 +66,7 @@ class FilesSourceOptions:
     # the HTTPFilesSource passes in additional http_headers through these properties, which
     # are merged with constructor defined http_headers. The interpretation of these properties
     # are filesystem specific.
-    extra_props: Optional[FilesSourceProperties] = {}
+    extra_props: Optional[FilesSourceProperties] = field(default_factory=lambda: {})
 
 
 class SingleFileSource(metaclass=abc.ABCMeta):

From c7a5e4ff59cd93b3f336a6676c77524db51f0357 Mon Sep 17 00:00:00 2001
From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
Date: Fri, 9 Feb 2024 16:41:45 +0530
Subject: [PATCH 07/17] Code review suggestion

Co-authored-by: Marius van den Beek <m.vandenbeek@gmail.com>
---
 lib/galaxy/files/sources/__init__.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/files/sources/__init__.py b/lib/galaxy/files/sources/__init__.py
index 24cf91f654e..40c2081d01c 100644
--- a/lib/galaxy/files/sources/__init__.py
+++ b/lib/galaxy/files/sources/__init__.py
@@ -1,7 +1,10 @@
 import abc
-from dataclasses import dataclass, field
 import os
 import time
+from dataclasses import (
+    dataclass,
+    field,
+)
 from typing import (
     Any,
     ClassVar,
@@ -66,7 +69,7 @@ class FilesSourceOptions:
     # the HTTPFilesSource passes in additional http_headers through these properties, which
     # are merged with constructor defined http_headers. The interpretation of these properties
     # are filesystem specific.
-    extra_props: Optional[FilesSourceProperties] = field(default_factory=lambda: {})
+    extra_props: Optional[FilesSourceProperties] = field(default_factory=lambda: FilesSourceProperties())
 
 
 class SingleFileSource(metaclass=abc.ABCMeta):

From 880977943eaea770d36cb0997b7bea062ebf4849 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Sat, 10 Feb 2024 12:14:59 +0100
Subject: [PATCH 08/17] Add templating to test IT

---
 test/functional/tools/interactivetool_simple.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/functional/tools/interactivetool_simple.xml b/test/functional/tools/interactivetool_simple.xml
index 63e4c4b410e..8919ec251d2 100644
--- a/test/functional/tools/interactivetool_simple.xml
+++ b/test/functional/tools/interactivetool_simple.xml
@@ -3,7 +3,7 @@
         <container type="docker">galaxy/test-http-example:0.1</container>
     </requirements>
     <entry_points>
-        <entry_point name="Simple" requires_domain="True">
+        <entry_point name="Simple IT for $__user_email__" requires_domain="True">
             <port>7000</port>
             <url>/</url>
         </entry_point>

From 3aa84429e078ae2530eb848b502821df8d34c77a Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Thu, 8 Feb 2024 11:42:43 +0100
Subject: [PATCH 09/17] Build param dict before creating entrypoint

Fixes https://github.com/galaxyproject/galaxy/issues/17438.
---
 lib/galaxy/jobs/__init__.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py
index 91a0ed792be..fcce9e0b738 100644
--- a/lib/galaxy/jobs/__init__.py
+++ b/lib/galaxy/jobs/__init__.py
@@ -1249,9 +1249,6 @@ class MinimalJobWrapper(HasResourceParameters):
 
         tool_evaluator = self._get_tool_evaluator(job)
         compute_environment = compute_environment or self.default_compute_environment(job)
-        if hasattr(self.app, "interactivetool_manager"):
-            self.interactivetools = tool_evaluator.populate_interactivetools()
-            self.app.interactivetool_manager.create_interactivetool(job, self.tool, self.interactivetools)
         tool_evaluator.set_compute_environment(compute_environment, get_special=get_special)
         (
             self.command_line,
@@ -1260,6 +1257,9 @@ class MinimalJobWrapper(HasResourceParameters):
             self.environment_variables,
         ) = tool_evaluator.build()
         job.command_line = self.command_line
+        if hasattr(self.app, "interactivetool_manager"):
+            self.interactivetools = tool_evaluator.populate_interactivetools()
+            self.app.interactivetool_manager.create_interactivetool(job, self.tool, self.interactivetools)
 
         # Ensure galaxy_lib_dir is set in case there are any later chdirs
         self.galaxy_lib_dir  # noqa: B018

From f130a3edf8ab8d2d8b614401035e42ab3894f717 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 12 Feb 2024 11:17:31 +0100
Subject: [PATCH 10/17] Refactor test to reduce selector duplication

---
 .../SelectionOperations.test.js               | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js
index 147e38622ce..733652a214b 100644
--- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js
+++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js
@@ -24,6 +24,7 @@ const TASKS_CONFIG = {
     enable_celery_tasks: true,
 };
 
+const getMenuSelectorFor = (option) => `[data-description="${option} option"]`;
 const getPurgedContentSelection = () => new Map([["FAKE_ID", { purged: true }]]);
 const getNonPurgedContentSelection = () => new Map([["FAKE_ID", { purged: false }]]);
 
@@ -76,48 +77,48 @@ describe("History Selection Operations", () => {
             });
 
             it("should display 'hide' option only on visible items", async () => {
-                const option = '[data-description="hide option"]';
+                const option = getMenuSelectorFor("hide");
                 expect(wrapper.find(option).exists()).toBe(true);
                 await wrapper.setProps({ filterText: "visible:false" });
                 expect(wrapper.find(option).exists()).toBe(false);
             });
 
             it("should display 'unhide' option on hidden items", async () => {
-                const option = '[data-description="unhide option"]';
+                const option = getMenuSelectorFor("unhide");
                 expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({ filterText: "visible:false" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
             it("should display 'unhide' option when hidden and visible items are mixed", async () => {
-                const option = '[data-description="unhide option"]';
+                const option = getMenuSelectorFor("unhide");
                 expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({ filterText: "visible:any" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
             it("should display 'delete' option on non-deleted items", async () => {
-                const option = '[data-description="delete option"]';
+                const option = getMenuSelectorFor("delete");
                 expect(wrapper.find(option).exists()).toBe(true);
                 await wrapper.setProps({ filterText: "deleted:true" });
                 expect(wrapper.find(option).exists()).toBe(false);
             });
 
             it("should display 'delete' option when non-deleted and deleted items are mixed", async () => {
-                const option = '[data-description="delete option"]';
+                const option = getMenuSelectorFor("delete");
                 await wrapper.setProps({ filterText: "deleted:any" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
             it("should display 'permanently delete' option always", async () => {
-                const option = '[data-description="purge option"]';
+                const option = getMenuSelectorFor("purge");
                 expect(wrapper.find(option).exists()).toBe(true);
                 await wrapper.setProps({ filterText: "deleted:true" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
             it("should display 'undelete' option on deleted and non-purged items", async () => {
-                const option = '[data-description="undelete option"]';
+                const option = getMenuSelectorFor("undelete");
                 expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({
                     filterText: "deleted:true",
@@ -127,7 +128,7 @@ describe("History Selection Operations", () => {
             });
 
             it("should display 'undelete' option when non-purged items (deleted or not) are mixed", async () => {
-                const option = '[data-description="undelete option"]';
+                const option = getMenuSelectorFor("undelete");
                 await wrapper.setProps({
                     filterText: "deleted:any",
                     contentSelection: getNonPurgedContentSelection(),
@@ -136,7 +137,7 @@ describe("History Selection Operations", () => {
             });
 
             it("should not display 'undelete' when is manual selection mode and all selected items are purged", async () => {
-                const option = '[data-description="undelete option"]';
+                const option = getMenuSelectorFor("undelete");
                 await wrapper.setProps({
                     filterText: "deleted:true",
                     contentSelection: getPurgedContentSelection(),
@@ -146,7 +147,7 @@ describe("History Selection Operations", () => {
             });
 
             it("should display 'undelete' option when is query selection mode and filtering by deleted", async () => {
-                const option = '[data-description="undelete option"]';
+                const option = getMenuSelectorFor("undelete");
                 // In query selection mode we don't know if some items may not be purged, so we allow to undelete
                 await wrapper.setProps({
                     filterText: "deleted:true",
@@ -157,7 +158,7 @@ describe("History Selection Operations", () => {
             });
 
             it("should display 'undelete' option when is query selection mode and filtering by any deleted state", async () => {
-                const option = '[data-description="undelete option"]';
+                const option = getMenuSelectorFor("undelete");
                 // In query selection mode we don't know if some items may not be purged, so we allow to undelete
                 await wrapper.setProps({
                     filterText: "deleted:any",

From 79fee2a7d42ab44a184007299fb3888876099e7d Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 12 Feb 2024 17:06:20 +0100
Subject: [PATCH 11/17] Add more test conditions for bulk menu

---
 .../SelectionOperations.test.js               | 99 +++++++++++++------
 1 file changed, 71 insertions(+), 28 deletions(-)

diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js
index 733652a214b..9acf326bdf4 100644
--- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js
+++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js
@@ -25,8 +25,13 @@ const TASKS_CONFIG = {
 };
 
 const getMenuSelectorFor = (option) => `[data-description="${option} option"]`;
-const getPurgedContentSelection = () => new Map([["FAKE_ID", { purged: true }]]);
-const getNonPurgedContentSelection = () => new Map([["FAKE_ID", { purged: false }]]);
+
+const getPurgedSelection = () => new Map([["FAKE_ID", { purged: true }]]);
+const getNonPurgedSelection = () => new Map([["FAKE_ID", { purged: false }]]);
+const getVisibleSelection = () => new Map([["FAKE_ID", { visible: true }]]);
+const getHiddenSelection = () => new Map([["FAKE_ID", { visible: false }]]);
+const getDeletedSelection = () => new Map([["FAKE_ID", { deleted: true }]]);
+const getActiveSelection = () => new Map([["FAKE_ID", { deleted: false }]]);
 
 async function mountSelectionOperationsWrapper(config) {
     const wrapper = shallowMount(
@@ -76,32 +81,62 @@ describe("History Selection Operations", () => {
                 expect(wrapper.find('[data-description="selected count"]').text()).toContain("10");
             });
 
-            it("should display 'hide' option only on visible items", async () => {
+            it("should display 'hide' option on visible items", async () => {
                 const option = getMenuSelectorFor("hide");
                 expect(wrapper.find(option).exists()).toBe(true);
+                await wrapper.setProps({ filterText: "visible:true" });
+                expect(wrapper.find(option).exists()).toBe(true);
+            });
+
+            it("should display 'hide' option when visible and hidden items are mixed", async () => {
+                const option = getMenuSelectorFor("hide");
+                expect(wrapper.find(option).exists()).toBe(true);
+                await wrapper.setProps({ filterText: "visible:any" });
+                expect(wrapper.find(option).exists()).toBe(true);
+            });
+
+            it("should not display 'hide' option when only hidden items are selected", async () => {
+                const option = getMenuSelectorFor("hide");
+                expect(wrapper.find(option).exists()).toBe(true);
+                await wrapper.setProps({ filterText: "visible:any", contentSelection: getHiddenSelection() });
+                expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({ filterText: "visible:false" });
                 expect(wrapper.find(option).exists()).toBe(false);
             });
 
             it("should display 'unhide' option on hidden items", async () => {
                 const option = getMenuSelectorFor("unhide");
-                expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({ filterText: "visible:false" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
             it("should display 'unhide' option when hidden and visible items are mixed", async () => {
                 const option = getMenuSelectorFor("unhide");
-                expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({ filterText: "visible:any" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
+            it("should not display 'unhide' option when only visible items are selected", async () => {
+                const option = getMenuSelectorFor("unhide");
+                await wrapper.setProps({
+                    filterText: "visible:any",
+                    contentSelection: getVisibleSelection(),
+                });
+                expect(wrapper.find(option).exists()).toBe(false);
+            });
+
+            it("should display 'delete' option on non-deleted items", async () => {
+                const option = getMenuSelectorFor("delete");
+                expect(wrapper.find(option).exists()).toBe(true);
+                await wrapper.setProps({ filterText: "deleted:false" });
+                expect(wrapper.find(option).exists()).toBe(true);
+            });
+
             it("should display 'delete' option on non-deleted items", async () => {
                 const option = getMenuSelectorFor("delete");
                 expect(wrapper.find(option).exists()).toBe(true);
-                await wrapper.setProps({ filterText: "deleted:true" });
-                expect(wrapper.find(option).exists()).toBe(false);
+                await wrapper.setProps({ filterText: "deleted:false" });
+                expect(wrapper.find(option).exists()).toBe(true);
             });
 
             it("should display 'delete' option when non-deleted and deleted items are mixed", async () => {
@@ -110,10 +145,17 @@ describe("History Selection Operations", () => {
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
+            it("should not display 'delete' option when only deleted items are selected", async () => {
+                const option = getMenuSelectorFor("delete");
+                expect(wrapper.find(option).exists()).toBe(true);
+                await wrapper.setProps({ filterText: "deleted:any", contentSelection: getDeletedSelection() });
+                expect(wrapper.find(option).exists()).toBe(false);
+            });
+
             it("should display 'permanently delete' option always", async () => {
                 const option = getMenuSelectorFor("purge");
                 expect(wrapper.find(option).exists()).toBe(true);
-                await wrapper.setProps({ filterText: "deleted:true" });
+                await wrapper.setProps({ filterText: "deleted:any visible:any" });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
@@ -122,7 +164,7 @@ describe("History Selection Operations", () => {
                 expect(wrapper.find(option).exists()).toBe(false);
                 await wrapper.setProps({
                     filterText: "deleted:true",
-                    contentSelection: getNonPurgedContentSelection(),
+                    contentSelection: getNonPurgedSelection(),
                 });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
@@ -131,16 +173,24 @@ describe("History Selection Operations", () => {
                 const option = getMenuSelectorFor("undelete");
                 await wrapper.setProps({
                     filterText: "deleted:any",
-                    contentSelection: getNonPurgedContentSelection(),
+                    contentSelection: getNonPurgedSelection(),
                 });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
-            it("should not display 'undelete' when is manual selection mode and all selected items are purged", async () => {
+            it("should not display 'undelete' when only non-deleted items are selected", async () => {
                 const option = getMenuSelectorFor("undelete");
                 await wrapper.setProps({
-                    filterText: "deleted:true",
-                    contentSelection: getPurgedContentSelection(),
+                    filterText: "deleted:any",
+                    contentSelection: getActiveSelection(),
+                });
+                expect(wrapper.find(option).exists()).toBe(false);
+            });
+
+            it("should not display 'undelete' when only purged items are selected", async () => {
+                const option = getMenuSelectorFor("undelete");
+                await wrapper.setProps({
+                    contentSelection: getPurgedSelection(),
                     isQuerySelection: false,
                 });
                 expect(wrapper.find(option).exists()).toBe(false);
@@ -151,7 +201,7 @@ describe("History Selection Operations", () => {
                 // In query selection mode we don't know if some items may not be purged, so we allow to undelete
                 await wrapper.setProps({
                     filterText: "deleted:true",
-                    contentSelection: getPurgedContentSelection(),
+                    contentSelection: getPurgedSelection(),
                     isQuerySelection: true,
                 });
                 expect(wrapper.find(option).exists()).toBe(true);
@@ -160,33 +210,26 @@ describe("History Selection Operations", () => {
             it("should display 'undelete' option when is query selection mode and filtering by any deleted state", async () => {
                 const option = getMenuSelectorFor("undelete");
                 // In query selection mode we don't know if some items may not be purged, so we allow to undelete
-                await wrapper.setProps({
-                    filterText: "deleted:any",
-                    contentSelection: getPurgedContentSelection(),
-                    isQuerySelection: true,
-                });
+                await wrapper.setProps({ filterText: "deleted:any", isQuerySelection: true });
                 expect(wrapper.find(option).exists()).toBe(true);
             });
 
-            it("should display collection building options only on visible and non-deleted items", async () => {
+            it("should display collection building options only on active (non-deleted) items", async () => {
                 const buildListOption = '[data-description="build list"]';
                 const buildPairOption = '[data-description="build pair"]';
                 const buildListOfPairsOption = '[data-description="build list of pairs"]';
+                await wrapper.setProps({ filterText: "visible:true deleted:false" });
                 expect(wrapper.find(buildListOption).exists()).toBe(true);
                 expect(wrapper.find(buildPairOption).exists()).toBe(true);
                 expect(wrapper.find(buildListOfPairsOption).exists()).toBe(true);
-                await wrapper.setProps({ filterText: "visible:false" });
-                expect(wrapper.find(buildListOption).exists()).toBe(false);
-                expect(wrapper.find(buildPairOption).exists()).toBe(false);
-                expect(wrapper.find(buildListOfPairsOption).exists()).toBe(false);
                 await wrapper.setProps({ filterText: "deleted:true" });
                 expect(wrapper.find(buildListOption).exists()).toBe(false);
                 expect(wrapper.find(buildPairOption).exists()).toBe(false);
                 expect(wrapper.find(buildListOfPairsOption).exists()).toBe(false);
-                await wrapper.setProps({ filterText: "visible:any" });
-                expect(wrapper.find(buildListOption).exists()).toBe(false);
-                expect(wrapper.find(buildPairOption).exists()).toBe(false);
-                expect(wrapper.find(buildListOfPairsOption).exists()).toBe(false);
+                await wrapper.setProps({ filterText: "visible:any deleted:false" });
+                expect(wrapper.find(buildListOption).exists()).toBe(true);
+                expect(wrapper.find(buildPairOption).exists()).toBe(true);
+                expect(wrapper.find(buildListOfPairsOption).exists()).toBe(true);
                 await wrapper.setProps({ filterText: "deleted:any" });
                 expect(wrapper.find(buildListOption).exists()).toBe(false);
                 expect(wrapper.find(buildPairOption).exists()).toBe(false);

From 811f8cf8fd73ae613605a59ccf2cddd52e5d0a3a Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 12 Feb 2024 17:06:58 +0100
Subject: [PATCH 12/17] Fix bulk menu conditions

---
 .../HistoryOperations/SelectionOperations.vue | 71 ++++++++++++++++---
 1 file changed, 60 insertions(+), 11 deletions(-)

diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue
index 27e478cea34..0930569fb50 100644
--- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue
+++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue
@@ -13,10 +13,13 @@
                 <b-dropdown-text>
                     <span v-localize data-description="selected count">With {{ numSelected }} selected...</span>
                 </b-dropdown-text>
-                <b-dropdown-item v-if="showHidden" v-b-modal:show-selected-content data-description="unhide option">
+                <b-dropdown-item
+                    v-if="canUnhideSelection"
+                    v-b-modal:show-selected-content
+                    data-description="unhide option">
                     <span v-localize>Unhide</span>
                 </b-dropdown-item>
-                <b-dropdown-item v-else v-b-modal:hide-selected-content data-description="hide option">
+                <b-dropdown-item v-if="canHideSelection" v-b-modal:hide-selected-content data-description="hide option">
                     <span v-localize>Hide</span>
                 </b-dropdown-item>
                 <b-dropdown-item
@@ -26,7 +29,7 @@
                     <span v-localize>Undelete</span>
                 </b-dropdown-item>
                 <b-dropdown-item
-                    v-if="!showStrictDeleted"
+                    v-if="canDeleteSelection"
                     v-b-modal:delete-selected-content
                     data-description="delete option">
                     <span v-localize>Delete</span>
@@ -195,20 +198,37 @@ export default {
     },
     computed: {
         /** @returns {Boolean} */
-        showHidden() {
-            return !HistoryFilters.checkFilter(this.filterText, "visible", true);
+        canUnhideSelection() {
+            return (
+                this.areAllSelectedHidden ||
+                (HistoryFilters.checkFilter(this.filterText, "visible", "any") && !this.areAllSelectedVisible)
+            );
+        },
+        /** @returns {Boolean} */
+        canHideSelection() {
+            return (
+                this.areAllSelectedVisible ||
+                (HistoryFilters.checkFilter(this.filterText, "visible", "any") && !this.areAllSelectedHidden)
+            );
         },
         /** @returns {Boolean} */
         showDeleted() {
             return !HistoryFilters.checkFilter(this.filterText, "deleted", false);
         },
         /** @returns {Boolean} */
-        showStrictDeleted() {
-            return HistoryFilters.checkFilter(this.filterText, "deleted", true);
+        canDeleteSelection() {
+            return (
+                this.areAllSelectedActive ||
+                (HistoryFilters.checkFilter(this.filterText, "deleted", "any") && !this.areAllSelectedDeleted)
+            );
+        },
+        /** @returns {Boolean} */
+        canUndeleteSelection() {
+            return this.showDeleted && (this.isQuerySelection || !this.areAllSelectedPurged);
         },
         /** @returns {Boolean} */
         showBuildOptions() {
-            return !this.isQuerySelection && !this.showHidden && !this.showDeleted;
+            return !this.isQuerySelection && this.areAllSelectedActive && !this.showDeleted;
         },
         /** @returns {Boolean} */
         showBuildOptionForAll() {
@@ -229,9 +249,6 @@ export default {
         noTagsSelected() {
             return this.selectedTags.length === 0;
         },
-        canUndeleteSelection() {
-            return this.showDeleted && (this.isQuerySelection || !this.areAllSelectedPurged);
-        },
         areAllSelectedPurged() {
             for (const item of this.contentSelection.values()) {
                 if (Object.prototype.hasOwnProperty.call(item, "purged") && !item["purged"]) {
@@ -240,6 +257,38 @@ export default {
             }
             return true;
         },
+        areAllSelectedVisible() {
+            for (const item of this.contentSelection.values()) {
+                if (Object.prototype.hasOwnProperty.call(item, "visible") && !item["visible"]) {
+                    return false;
+                }
+            }
+            return true;
+        },
+        areAllSelectedHidden() {
+            for (const item of this.contentSelection.values()) {
+                if (Object.prototype.hasOwnProperty.call(item, "visible") && item["visible"]) {
+                    return false;
+                }
+            }
+            return true;
+        },
+        areAllSelectedActive() {
+            for (const item of this.contentSelection.values()) {
+                if (Object.prototype.hasOwnProperty.call(item, "deleted") && item["deleted"]) {
+                    return false;
+                }
+            }
+            return true;
+        },
+        areAllSelectedDeleted() {
+            for (const item of this.contentSelection.values()) {
+                if (Object.prototype.hasOwnProperty.call(item, "deleted") && !item["deleted"]) {
+                    return false;
+                }
+            }
+            return true;
+        },
     },
     watch: {
         hasSelection(newVal) {

From b7b5c05afd097bff69d007b5970ef0026ad95118 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Mon, 12 Feb 2024 17:20:36 +0100
Subject: [PATCH 13/17] Increase readability

---
 .../HistoryOperations/SelectionOperations.vue | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue
index 0930569fb50..c3165c5a5d9 100644
--- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue
+++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue
@@ -199,17 +199,11 @@ export default {
     computed: {
         /** @returns {Boolean} */
         canUnhideSelection() {
-            return (
-                this.areAllSelectedHidden ||
-                (HistoryFilters.checkFilter(this.filterText, "visible", "any") && !this.areAllSelectedVisible)
-            );
+            return this.areAllSelectedHidden || (this.isAnyVisibilityAllowed && !this.areAllSelectedVisible);
         },
         /** @returns {Boolean} */
         canHideSelection() {
-            return (
-                this.areAllSelectedVisible ||
-                (HistoryFilters.checkFilter(this.filterText, "visible", "any") && !this.areAllSelectedHidden)
-            );
+            return this.areAllSelectedVisible || (this.isAnyVisibilityAllowed && !this.areAllSelectedHidden);
         },
         /** @returns {Boolean} */
         showDeleted() {
@@ -217,10 +211,7 @@ export default {
         },
         /** @returns {Boolean} */
         canDeleteSelection() {
-            return (
-                this.areAllSelectedActive ||
-                (HistoryFilters.checkFilter(this.filterText, "deleted", "any") && !this.areAllSelectedDeleted)
-            );
+            return this.areAllSelectedActive || (this.isAnyDeletedStateAllowed && !this.areAllSelectedDeleted);
         },
         /** @returns {Boolean} */
         canUndeleteSelection() {
@@ -289,6 +280,12 @@ export default {
             }
             return true;
         },
+        isAnyVisibilityAllowed() {
+            return HistoryFilters.checkFilter(this.filterText, "visible", "any");
+        },
+        isAnyDeletedStateAllowed() {
+            return HistoryFilters.checkFilter(this.filterText, "deleted", "any");
+        },
     },
     watch: {
         hasSelection(newVal) {

From 9eebb606477bfc217354a9d3aa947d9e3d427db5 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Sat, 10 Feb 2024 12:10:28 +0100
Subject: [PATCH 14/17] Sandwich IT entry point creating after param_dict
 building and env var creation

---
 lib/galaxy/jobs/__init__.py            |  5 ++--
 lib/galaxy/tools/evaluation.py         | 35 +++++++++++++++++++++++---
 lib/galaxy/tools/remote_tool_eval.py   |  2 +-
 test/unit/app/jobs/test_job_wrapper.py |  5 +---
 test/unit/app/tools/test_evaluation.py |  2 +-
 5 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py
index fcce9e0b738..3329c98253c 100644
--- a/lib/galaxy/jobs/__init__.py
+++ b/lib/galaxy/jobs/__init__.py
@@ -1255,11 +1255,9 @@ class MinimalJobWrapper(HasResourceParameters):
             self.version_command_line,
             self.extra_filenames,
             self.environment_variables,
+            self.interactivetools,
         ) = tool_evaluator.build()
         job.command_line = self.command_line
-        if hasattr(self.app, "interactivetool_manager"):
-            self.interactivetools = tool_evaluator.populate_interactivetools()
-            self.app.interactivetool_manager.create_interactivetool(job, self.tool, self.interactivetools)
 
         # Ensure galaxy_lib_dir is set in case there are any later chdirs
         self.galaxy_lib_dir  # noqa: B018
@@ -2623,6 +2621,7 @@ class TaskWrapper(JobWrapper):
                 self.version_command_line,
                 extra_filenames,
                 self.environment_variables,
+                *_,
             ) = tool_evaluator.build()
             self.extra_filenames.extend(extra_filenames)
 
diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py
index d06af3b01d5..9e4a8964a4c 100644
--- a/lib/galaxy/tools/evaluation.py
+++ b/lib/galaxy/tools/evaluation.py
@@ -121,6 +121,7 @@ class ToolEvaluator:
         self.environment_variables: List[Dict[str, str]] = []
         self.version_command_line: Optional[str] = None
         self.command_line: Optional[str] = None
+        self.interactivetools: List[Dict[str, Any]] = []
 
     def set_compute_environment(self, compute_environment: ComputeEnvironment, get_special: Optional[Callable] = None):
         """
@@ -514,7 +515,12 @@ class ToolEvaluator:
             # the paths rewritten.
             self.__walk_inputs(self.tool.inputs, param_dict, rewrite_unstructured_paths)
 
-    def populate_interactivetools(self):
+    def _create_interactivetools_entry_points(self):
+        if hasattr(self.app, "interactivetool_manager"):
+            self.interactivetools = self._populate_interactivetools_template()
+            self.app.interactivetool_manager.create_interactivetool(self.job, self.tool, self.interactivetools)
+
+    def _populate_interactivetools_template(self):
         """
         Populate InteractiveTools templated values.
         """
@@ -571,12 +577,21 @@ class ToolEvaluator:
         compute environment.
         """
         config_file = self.tool.config_file
+        global_tool_logs(
+            self._create_interactivetools_entry_points, config_file, "Building Interactive Tool Entry Points"
+        )
         global_tool_logs(self._build_config_files, config_file, "Building Config Files")
         global_tool_logs(self._build_param_file, config_file, "Building Param File")
         global_tool_logs(self._build_command_line, config_file, "Building Command Line")
         global_tool_logs(self._build_version_command, config_file, "Building Version Command Line")
         global_tool_logs(self._build_environment_variables, config_file, "Building Environment Variables")
-        return self.command_line, self.version_command_line, self.extra_filenames, self.environment_variables
+        return (
+            self.command_line,
+            self.version_command_line,
+            self.extra_filenames,
+            self.environment_variables,
+            self.interactivetools,
+        )
 
     def _build_command_line(self):
         """
@@ -823,7 +838,13 @@ class PartialToolEvaluator(ToolEvaluator):
     def build(self):
         config_file = self.tool.config_file
         global_tool_logs(self._build_environment_variables, config_file, "Building Environment Variables")
-        return self.command_line, self.version_command_line, self.extra_filenames, self.environment_variables
+        return (
+            self.command_line,
+            self.version_command_line,
+            self.extra_filenames,
+            self.environment_variables,
+            self.interactivetools,
+        )
 
 
 class RemoteToolEvaluator(ToolEvaluator):
@@ -841,4 +862,10 @@ class RemoteToolEvaluator(ToolEvaluator):
         global_tool_logs(self._build_param_file, config_file, "Building Param File")
         global_tool_logs(self._build_command_line, config_file, "Building Command Line")
         global_tool_logs(self._build_version_command, config_file, "Building Version Command Line")
-        return self.command_line, self.version_command_line, self.extra_filenames, self.environment_variables
+        return (
+            self.command_line,
+            self.version_command_line,
+            self.extra_filenames,
+            self.environment_variables,
+            self.interactivetools,
+        )
diff --git a/lib/galaxy/tools/remote_tool_eval.py b/lib/galaxy/tools/remote_tool_eval.py
index e69ce53899a..b07c6cf6a87 100644
--- a/lib/galaxy/tools/remote_tool_eval.py
+++ b/lib/galaxy/tools/remote_tool_eval.py
@@ -114,7 +114,7 @@ def main(TMPDIR, WORKING_DIRECTORY, IMPORT_STORE_DIRECTORY):
     )
     tool_evaluator.set_compute_environment(compute_environment=SharedComputeEnvironment(job_io=job_io, job=job_io.job))
     with open(os.path.join(WORKING_DIRECTORY, "tool_script.sh"), "a") as out:
-        command_line, version_command_line, extra_filenames, environment_variables = tool_evaluator.build()
+        command_line, version_command_line, extra_filenames, environment_variables, *_ = tool_evaluator.build()
         out.write(f'{version_command_line or ""}{command_line}')
 
 
diff --git a/test/unit/app/jobs/test_job_wrapper.py b/test/unit/app/jobs/test_job_wrapper.py
index 40862720e24..6c914c3efaf 100644
--- a/test/unit/app/jobs/test_job_wrapper.py
+++ b/test/unit/app/jobs/test_job_wrapper.py
@@ -113,14 +113,11 @@ class MockEvaluator:
         self.local_working_directory = local_working_directory
         self.param_dict = {}
 
-    def populate_interactivetools(self):
-        return []
-
     def set_compute_environment(self, *args, **kwds):
         pass
 
     def build(self):
-        return TEST_COMMAND, "", [], []
+        return TEST_COMMAND, "", [], [], []
 
 
 class MockJobQueue:
diff --git a/test/unit/app/tools/test_evaluation.py b/test/unit/app/tools/test_evaluation.py
index 24473b5d7fc..06e0e2380e1 100644
--- a/test/unit/app/tools/test_evaluation.py
+++ b/test/unit/app/tools/test_evaluation.py
@@ -139,7 +139,7 @@ class TestToolEvaluator(TestCase, UsesApp):
         self.tool.config_files.append(("conf1", None, "$thresh"))
         self.tool._command_line = "prog1 $conf1"
         self._set_compute_environment()
-        command_line, _, extra_filenames, _ = self.evaluator.build()
+        command_line, _, extra_filenames, *_ = self.evaluator.build()
         assert len(extra_filenames) == 1
         config_filename = extra_filenames[0]
         config_basename = os.path.basename(config_filename)

From 8c3d967ad3bce144f0335c847cb694b17d2a6940 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Thu, 8 Feb 2024 13:14:31 -0600
Subject: [PATCH 15/17] fix published history collections are empty on expand

Only check ownership in `__get_history_collection_instance` if:
`collection_instance.history.published` = False
---
 lib/galaxy/managers/collections.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py
index e941c69d680..88d12d62b09 100644
--- a/lib/galaxy/managers/collections.py
+++ b/lib/galaxy/managers/collections.py
@@ -807,7 +807,7 @@ class DatasetCollectionManager:
             raise RequestParameterInvalidException("History dataset collection association not found")
         # TODO: that sure looks like a bug, we can't check ownership using the history of the object we're checking ownership for ...
         history = getattr(trans, "history", collection_instance.history)
-        if check_ownership:
+        if check_ownership and collection_instance.history.published is False:
             self.history_manager.error_unless_owner(collection_instance.history, trans.user, current_history=history)
         if check_accessible:
             self.history_manager.error_unless_accessible(

From 4eab971489c79f3bcb159c4cb5276d88bdd75759 Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Fri, 9 Feb 2024 09:57:59 -0600
Subject: [PATCH 16/17] remove `check_ownership` from
 `dataset_collections.contents`

... since this api only needs to `check_accessible` as it is not modifying a collection
---
 lib/galaxy/managers/collections.py                        | 2 +-
 lib/galaxy/webapps/galaxy/services/dataset_collections.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py
index 88d12d62b09..e941c69d680 100644
--- a/lib/galaxy/managers/collections.py
+++ b/lib/galaxy/managers/collections.py
@@ -807,7 +807,7 @@ class DatasetCollectionManager:
             raise RequestParameterInvalidException("History dataset collection association not found")
         # TODO: that sure looks like a bug, we can't check ownership using the history of the object we're checking ownership for ...
         history = getattr(trans, "history", collection_instance.history)
-        if check_ownership and collection_instance.history.published is False:
+        if check_ownership:
             self.history_manager.error_unless_owner(collection_instance.history, trans.user, current_history=history)
         if check_accessible:
             self.history_manager.error_unless_accessible(
diff --git a/lib/galaxy/webapps/galaxy/services/dataset_collections.py b/lib/galaxy/webapps/galaxy/services/dataset_collections.py
index d13e2066b3f..c5f7cf4181a 100644
--- a/lib/galaxy/webapps/galaxy/services/dataset_collections.py
+++ b/lib/galaxy/webapps/galaxy/services/dataset_collections.py
@@ -250,7 +250,7 @@ class DatasetCollectionsService(ServiceBase, UsesLibraryMixinItems):
                 "Parameter instance_type not being 'history' is not yet implemented."
             )
         hdca: "HistoryDatasetCollectionAssociation" = self.collection_manager.get_dataset_collection_instance(
-            trans, "history", hdca_id, check_ownership=True
+            trans, "history", hdca_id
         )
 
         # check to make sure the dsc is part of the validated hdca

From d18003e6616c05e2bf7e98d8c0e97a715523c80d Mon Sep 17 00:00:00 2001
From: Ahmed Awan <qe66653@umbc.edu>
Date: Mon, 12 Feb 2024 10:30:13 -0600
Subject: [PATCH 17/17] add api test for hdcas in public histories being
 accessible

---
 lib/galaxy_test/api/test_dataset_collections.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/galaxy_test/api/test_dataset_collections.py b/lib/galaxy_test/api/test_dataset_collections.py
index c7601e2242a..f24f8f70786 100644
--- a/lib/galaxy_test/api/test_dataset_collections.py
+++ b/lib/galaxy_test/api/test_dataset_collections.py
@@ -418,6 +418,18 @@ class TestDatasetCollectionsApi(ApiTestCase):
             contents_response = self._get(contents_url)
             self._assert_status_code_is(contents_response, 403)
 
+    @requires_new_user
+    def test_published_collection_contents_accessible(self, history_id):
+        # request contents on an hdca that is in a published history
+        hdca, contents_url = self._create_collection_contents_pair(history_id)
+        with self._different_user():
+            contents_response = self._get(contents_url)
+            self._assert_status_code_is(contents_response, 403)
+        self.dataset_populator.make_public(history_id)
+        with self._different_user():
+            contents_response = self._get(contents_url)
+            self._assert_status_code_is(contents_response, 200)
+
     def test_collection_contents_invalid_collection(self, history_id):
         # request an invalid collection from a valid hdca, should get 404
         hdca, contents_url = self._create_collection_contents_pair(history_id)