From 09d86951b12bc0260d0163c801dac31207c25c9e Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 21 Nov 2024 17:13:26 +0000 Subject: [PATCH 01/33] Update title of edited PRs only if base ref changed Also: - Update title of reopened PRs too. - Don't unnecessarily checkout the repo. --- .github/workflows/pr-title-update.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-title-update.yml b/.github/workflows/pr-title-update.yml index 6a4c5021f63d..ee47e16d84e1 100644 --- a/.github/workflows/pr-title-update.yml +++ b/.github/workflows/pr-title-update.yml @@ -2,17 +2,17 @@ name: Update PR title on: pull_request_target: - types: [opened, edited] + types: [opened, edited, reopened] branches: - "release_**" jobs: update-title: + if: github.event.action != 'edited' || github.event.changes.base.ref.from != '' runs-on: ubuntu-latest permissions: pull-requests: write steps: - - uses: actions/checkout@v4 - name: Update PR title env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -23,5 +23,5 @@ jobs: VERSION=$(echo $TARGET_BRANCH | grep -oP '\d+\.\d+') if [[ -n "$VERSION" && ! "$PR_TITLE" =~ ^\[$VERSION\] ]]; then NEW_TITLE="[$VERSION] $PR_TITLE" - gh pr edit $PR_NUMBER --title "$NEW_TITLE" + gh pr edit $PR_NUMBER --repo "${{ github.repository }}" --title "$NEW_TITLE" fi From f3e65fa1b95bca02981283f2259d085fa738138a Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 25 Nov 2024 11:39:58 +0100 Subject: [PATCH 02/33] Refactor PR title update workflow to handle version extraction and title formatting more efficiently --- .github/workflows/pr-title-update.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr-title-update.yml b/.github/workflows/pr-title-update.yml index ee47e16d84e1..6ed945cae0f7 100644 --- a/.github/workflows/pr-title-update.yml +++ b/.github/workflows/pr-title-update.yml @@ -3,8 +3,6 @@ name: Update PR title on: pull_request_target: types: [opened, edited, reopened] - branches: - - "release_**" jobs: update-title: @@ -19,9 +17,14 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} TARGET_BRANCH: "${{ github.base_ref }}" PR_TITLE: "${{ github.event.pull_request.title }}" + REPO: "${{ github.repository }}" run: | - VERSION=$(echo $TARGET_BRANCH | grep -oP '\d+\.\d+') - if [[ -n "$VERSION" && ! "$PR_TITLE" =~ ^\[$VERSION\] ]]; then - NEW_TITLE="[$VERSION] $PR_TITLE" - gh pr edit $PR_NUMBER --repo "${{ github.repository }}" --title "$NEW_TITLE" + VERSION=$(echo $TARGET_BRANCH | grep -oP 'release_\K.*' || true) + if [[ -n "$VERSION" ]]; then + NEW_TITLE="[$VERSION] ${PR_TITLE#*\] }" + else + NEW_TITLE="${PR_TITLE#*\] }" + fi + if [[ "$NEW_TITLE" != "$PR_TITLE" ]]; then + gh pr edit $PR_NUMBER --repo "$REPO" --title "$NEW_TITLE" fi From afdfa789f171963cdcb0509a442a7196be70861c Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 26 Nov 2024 12:20:19 +0100 Subject: [PATCH 03/33] Refactor version extraction in PR title update workflow for improved accuracy Co-authored-by: Nicola Soranzo --- .github/workflows/pr-title-update.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-title-update.yml b/.github/workflows/pr-title-update.yml index 6ed945cae0f7..8ffd54a45da4 100644 --- a/.github/workflows/pr-title-update.yml +++ b/.github/workflows/pr-title-update.yml @@ -19,11 +19,10 @@ jobs: PR_TITLE: "${{ github.event.pull_request.title }}" REPO: "${{ github.repository }}" run: | - VERSION=$(echo $TARGET_BRANCH | grep -oP 'release_\K.*' || true) + VERSION=$(echo $TARGET_BRANCH | grep -oP '^release_\K\d+.\d+$' || true) + NEW_TITLE=$(echo "$PR_TITLE" | sed -E "s/\[[0-9]+\.[0-9]+\] //") if [[ -n "$VERSION" ]]; then - NEW_TITLE="[$VERSION] ${PR_TITLE#*\] }" - else - NEW_TITLE="${PR_TITLE#*\] }" + NEW_TITLE="[$VERSION] $NEW_TITLE" fi if [[ "$NEW_TITLE" != "$PR_TITLE" ]]; then gh pr edit $PR_NUMBER --repo "$REPO" --title "$NEW_TITLE" From 137dc4eea10a866f13c463ef43b42ad5921870a4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 26 Nov 2024 12:50:13 +0100 Subject: [PATCH 04/33] Fix workflow_invocation_step not being added to session if the step is not a job step. This broke in https://github.com/galaxyproject/galaxy/commit/ab2f76c94d89fd9419b1dc0bc5ea606834cdbe2b where ``` # Safeguard: imported_invocation_step was implicitly merged into this Session prior to SQLAlchemy 2.0. self.sa_session.add(imported_invocation_step) ``` was replaced with ``` ensure_object_added_to_session(imported_invocation, session=self.sa_session) ``` which seems like a small typo-like bug. --- lib/galaxy/model/store/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index 5789dfb2f1b8..8099d5d915dc 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -1070,7 +1070,7 @@ def attach_workflow_step(imported_object, attrs): for step_attrs in invocation_attrs["steps"]: imported_invocation_step = model.WorkflowInvocationStep() imported_invocation_step.workflow_invocation = imported_invocation - ensure_object_added_to_session(imported_invocation, session=self.sa_session) + ensure_object_added_to_session(imported_invocation_step, session=self.sa_session) attach_workflow_step(imported_invocation_step, step_attrs) restore_times(imported_invocation_step, step_attrs) imported_invocation_step.action = step_attrs["action"] From e9194d702866acd5820d5d7a9613e3368271d5c7 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 26 Nov 2024 12:58:29 +0100 Subject: [PATCH 05/33] Fix exporting of jobs that set copied_from_history_dataset_association Without this change jobs like `__EXTRACT_DATASET__` would not be included in a history or invocation export`. --- lib/galaxy/model/store/__init__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index 8099d5d915dc..b7d31d6afae4 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -2364,12 +2364,12 @@ def to_json(attributes): # # Get all jobs associated with included HDAs. - jobs_dict: Dict[str, model.Job] = {} + jobs_dict: Dict[int, model.Job] = {} implicit_collection_jobs_dict = {} def record_job(job): - if not job: - # No viable job. + if not job or job.id in jobs_dict: + # No viable job or job already recorded. return jobs_dict[job.id] = job @@ -2395,10 +2395,11 @@ def record_associated_jobs(obj): ) job_hda = hda while job_hda.copied_from_history_dataset_association: # should this check library datasets as well? + # record job (if one exists) even if dataset was copied + # copy could have been created manually through UI/API or using database operation tool, + # in which case we have a relevant job to export. + record_associated_jobs(job_hda) job_hda = job_hda.copied_from_history_dataset_association - if not job_hda.creating_job_associations: - # No viable HDA found. - continue record_associated_jobs(job_hda) From e57201e92d9be78cb57422a3fdf29b839d0f8354 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 26 Nov 2024 15:31:51 +0100 Subject: [PATCH 06/33] Drop unused copied_to_history_dataset_collection_association relationship --- lib/galaxy/model/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 03524f1ab88b..ea43fb6dd1d5 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -6693,11 +6693,6 @@ class HistoryDatasetCollectionAssociation( primaryjoin=copied_from_history_dataset_collection_association_id == id, remote_side=[id], uselist=False, - back_populates="copied_to_history_dataset_collection_association", - ) - copied_to_history_dataset_collection_association = relationship( - "HistoryDatasetCollectionAssociation", - back_populates="copied_from_history_dataset_collection_association", ) implicit_input_collections = relationship( "ImplicitlyCreatedDatasetCollectionInput", From 28ce456292f0da71656505c3d6d8a1cbf67979a3 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 26 Nov 2024 15:47:39 +0100 Subject: [PATCH 07/33] Prevent assigning opied_from_history_dataset_collection_association if hdca points to self --- lib/galaxy/model/store/__init__.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index b7d31d6afae4..fa7b8f79e02a 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -996,14 +996,16 @@ def _import_collection_copied_associations( # sense. hdca_copied_from_sinks = object_import_tracker.hdca_copied_from_sinks if copied_from_object_key in object_import_tracker.hdcas_by_key: - hdca.copied_from_history_dataset_collection_association = object_import_tracker.hdcas_by_key[ - copied_from_object_key - ] + source_hdca = object_import_tracker.hdcas_by_key[copied_from_object_key] + if source_hdca is not hdca: + # We may not have the copied source, in which case the first included HDCA in the chain + # acts as the source, so here we make sure we don't create a cycle. + hdca.copied_from_history_dataset_collection_association = source_hdca else: if copied_from_object_key in hdca_copied_from_sinks: - hdca.copied_from_history_dataset_collection_association = object_import_tracker.hdcas_by_key[ - hdca_copied_from_sinks[copied_from_object_key] - ] + source_hdca = object_import_tracker.hdcas_by_key[hdca_copied_from_sinks[copied_from_object_key]] + if source_hdca is not hdca: + hdca.copied_from_history_dataset_collection_association = source_hdca else: hdca_copied_from_sinks[copied_from_object_key] = dataset_collection_key From b265c2bda9737fa1f27e0f4876d91c0591bfc1a9 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 26 Nov 2024 15:48:19 +0100 Subject: [PATCH 08/33] Don't duplicate exported collections if they're references multiple times --- lib/galaxy/model/store/__init__.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index fa7b8f79e02a..d9aab22b92b0 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -1923,12 +1923,14 @@ def __init__( self.export_files = export_files self.included_datasets: Dict[model.DatasetInstance, Tuple[model.DatasetInstance, bool]] = {} self.dataset_implicit_conversions: Dict[model.DatasetInstance, model.ImplicitlyConvertedDatasetAssociation] = {} - self.included_collections: List[Union[model.DatasetCollection, model.HistoryDatasetCollectionAssociation]] = [] + self.included_collections: Dict[ + Union[model.DatasetCollection, model.HistoryDatasetCollectionAssociation], + Union[model.DatasetCollection, model.HistoryDatasetCollectionAssociation], + ] = {} self.included_libraries: List[model.Library] = [] self.included_library_folders: List[model.LibraryFolder] = [] self.included_invocations: List[model.WorkflowInvocation] = [] self.collection_datasets: Set[int] = set() - self.collections_attrs: List[Union[model.DatasetCollection, model.HistoryDatasetCollectionAssociation]] = [] self.dataset_id_to_path: Dict[int, Tuple[Optional[str], Optional[str]]] = {} self.job_output_dataset_associations: Dict[int, Dict[str, model.DatasetInstance]] = {} @@ -2289,8 +2291,7 @@ def export_collection( def add_dataset_collection( self, collection: Union[model.DatasetCollection, model.HistoryDatasetCollectionAssociation] ) -> None: - self.collections_attrs.append(collection) - self.included_collections.append(collection) + self.included_collections[collection] = collection def add_implicit_conversion_dataset( self, @@ -2345,7 +2346,7 @@ def to_json(attributes): collections_attrs_filename = os.path.join(export_directory, ATTRS_FILENAME_COLLECTIONS) with open(collections_attrs_filename, "w") as collections_attrs_out: - collections_attrs_out.write(to_json(self.collections_attrs)) + collections_attrs_out.write(to_json(self.included_collections.values())) conversions_attrs_filename = os.path.join(export_directory, ATTRS_FILENAME_CONVERSIONS) with open(conversions_attrs_filename, "w") as conversions_attrs_out: From 2d7667d95842fa3f157816c42c04be0163dfe559 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 27 Nov 2024 17:26:58 +0100 Subject: [PATCH 09/33] Add regression test for #18927 --- test/integration/test_workflow_tasks.py | 44 +++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/integration/test_workflow_tasks.py b/test/integration/test_workflow_tasks.py index c250b624439f..3f8869dc9109 100644 --- a/test/integration/test_workflow_tasks.py +++ b/test/integration/test_workflow_tasks.py @@ -17,6 +17,7 @@ from galaxy_test.base import api_asserts from galaxy_test.base.api import UsesCeleryTasks from galaxy_test.base.populators import ( + DatasetCollectionPopulator, DatasetPopulator, RunJobsSummary, WorkflowPopulator, @@ -27,6 +28,7 @@ class TestWorkflowTasksIntegration(PosixFileSourceSetup, IntegrationTestCase, UsesCeleryTasks, RunsWorkflowFixtures): dataset_populator: DatasetPopulator + dataset_collection_populator: DatasetCollectionPopulator framework_tool_and_types = True @classmethod @@ -37,6 +39,7 @@ def handle_galaxy_config_kwds(cls, config): def setUp(self): super().setUp() self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + self.dataset_collection_populator = DatasetCollectionPopulator(self.galaxy_interactor) self.workflow_populator = WorkflowPopulator(self.galaxy_interactor) self._write_file_fixtures() @@ -124,6 +127,47 @@ def test_export_import_invocation_with_step_parameter(self): invocation_details = self._export_and_import_workflow_invocation(summary, use_uris) self._rerun_imported_workflow(summary, invocation_details) + def test_export_import_invocation_with_copied_hdca_and_database_operation_tool(self): + with self.dataset_populator.test_history() as history_id: + self.dataset_collection_populator.create_list_in_history(history_id=history_id, wait=True).json() + new_history = self.dataset_populator.copy_history(history_id=history_id).json() + copied_collection = self.dataset_populator.get_history_collection_details(new_history["id"]) + workflow_id = self.workflow_populator.upload_yaml_workflow( + """class: GalaxyWorkflow +inputs: + input: + type: collection + collection_type: list +steps: + extract_dataset: + tool_id: __EXTRACT_DATASET__ + in: + input: + source: input +""" + ) + inputs = {"input": {"src": "hdca", "id": copied_collection["id"]}} + workflow_request = {"history": f"hist_id={new_history['id']}", "inputs_by": "name", "inputs": inputs} + invocation = self.workflow_populator.invoke_workflow_raw( + workflow_id, workflow_request, assert_ok=True + ).json() + invocation_id = invocation["id"] + self.workflow_populator.wait_for_invocation_and_jobs(history_id, workflow_id, invocation_id) + jobs = self.workflow_populator.get_invocation_jobs(invocation_id) + summary = RunJobsSummary( + history_id=history_id, + workflow_id=workflow_id, + invocation_id=invocation["id"], + inputs=inputs, + jobs=jobs, + invocation=invocation, + workflow_request=workflow_request, + ) + imported_invocation_details = self._export_and_import_workflow_invocation(summary) + original_contents = self.dataset_populator.get_history_contents(new_history["id"]) + contents = self.dataset_populator.get_history_contents(imported_invocation_details["history_id"]) + assert len(contents) == len(original_contents) == 5 + def _export_and_import_workflow_invocation( self, summary: RunJobsSummary, use_uris: bool = True, model_store_format="tgz" ) -> Dict[str, Any]: From f631f626697e786add3c81d0997cdbb0d705bd4d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 27 Nov 2024 18:56:27 +0100 Subject: [PATCH 10/33] Don't store workflow_path when importing invocation It's a temporary path and probably doesn't make much sense in the context of an imported invocation. --- lib/galaxy/managers/workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index b54afd169066..183ffde1f98c 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -592,7 +592,7 @@ def read_workflow_from_path(self, app, user, path, allow_in_directory=None) -> m import_options = ImportOptions() import_options.deduplicate_subworkflows = True as_dict = python_to_workflow(as_dict, galaxy_interface, workflow_directory=None, import_options=import_options) - raw_description = RawWorkflowDescription(as_dict, path) + raw_description = RawWorkflowDescription(as_dict) created_workflow = self.build_workflow_from_raw_description(trans, raw_description, WorkflowCreateOptions()) return created_workflow.workflow From 91fc1d80c7fb6364e87b1df69212cdd5b2e55dc7 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 27 Nov 2024 19:01:20 +0100 Subject: [PATCH 11/33] Only allow admin users to sync workflows to filesystem --- lib/galaxy/managers/workflows.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 183ffde1f98c..622b1b58c2de 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -925,8 +925,9 @@ def to_format_2(wf_dict, **kwds): return wf_dict def _sync_stored_workflow(self, trans, stored_workflow): - workflow_path = stored_workflow.from_path - self.store_workflow_to_path(workflow_path, stored_workflow, stored_workflow.latest_workflow, trans=trans) + if trans.user_is_admin: + workflow_path = stored_workflow.from_path + self.store_workflow_to_path(workflow_path, stored_workflow, stored_workflow.latest_workflow, trans=trans) def store_workflow_artifacts(self, directory, filename_base, workflow, **kwd): modern_workflow_path = os.path.join(directory, f"{filename_base}.gxwf.yml") From e301ec99c7ff09fe1cd4b36eeeadef359cd6ffe8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 28 Nov 2024 13:48:51 +0100 Subject: [PATCH 12/33] Fix up action_arguments for workflows converted by gxformat2 < 0.20.0 Fixes https://github.com/galaxyproject/galaxy/issues/18995 --- lib/galaxy/model/__init__.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index ea43fb6dd1d5..9cd465dbcd24 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -2486,7 +2486,7 @@ class PostJobAction(Base, RepresentById): workflow_step_id = Column(Integer, ForeignKey("workflow_step.id"), index=True, nullable=True) action_type = Column(String(255), nullable=False) output_name = Column(String(255), nullable=True) - action_arguments = Column(MutableJSONType, nullable=True) + _action_arguments = Column("action_arguments", MutableJSONType, nullable=True) workflow_step = relationship( "WorkflowStep", back_populates="post_job_actions", @@ -2500,6 +2500,18 @@ def __init__(self, action_type, workflow_step=None, output_name=None, action_arg self.workflow_step = workflow_step ensure_object_added_to_session(self, object_in_session=workflow_step) + @property + def action_arguments(self): + if self.action_type in ("HideDatasetAction", "DeleteIntermediatesAction") and self._action_arguments is True: + # Fix up broken workflows resulting from imports with gxformat2 <= 0.20.0 + return {} + else: + return self._action_arguments + + @action_arguments.setter + def action_arguments(self, value: Dict[str, Any]): + self._action_arguments = value + class PostJobActionAssociation(Base, RepresentById): __tablename__ = "post_job_action_association" From a42d7f30a74638a2d5baeaccbca17f0e480b67f9 Mon Sep 17 00:00:00 2001 From: Wolfgang Maier Date: Fri, 29 Nov 2024 12:30:30 +0100 Subject: [PATCH 13/33] Create harmonized collections from correct tool outputs --- lib/galaxy/tools/__init__.py | 7 ++--- .../tools/harmonize_two_collections_list.xml | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 42fe49f58f4d..97fd83f8561a 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -3660,14 +3660,13 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history final_sorted_identifiers = [ element.element_identifier for element in elements1 if element.element_identifier in old_elements2_dict ] - # Raise Exception if it is empty if len(final_sorted_identifiers) == 0: # Create empty collections: output_collections.create_collection( - next(iter(self.outputs.values())), "output1", elements={}, propagate_hda_tags=False + self.outputs["output1"], "output1", elements={}, propagate_hda_tags=False ) output_collections.create_collection( - next(iter(self.outputs.values())), "output2", elements={}, propagate_hda_tags=False + self.outputs["output2"], "output2", elements={}, propagate_hda_tags=False ) return @@ -3685,7 +3684,7 @@ def output_with_selected_identifiers(old_elements_dict, output_label): self._add_datasets_to_history(history, new_elements.values()) # Create collections: output_collections.create_collection( - next(iter(self.outputs.values())), output_label, elements=new_elements, propagate_hda_tags=False + self.outputs[output_label], output_label, elements=new_elements, propagate_hda_tags=False ) # Create outputs: diff --git a/lib/galaxy/tools/harmonize_two_collections_list.xml b/lib/galaxy/tools/harmonize_two_collections_list.xml index 7828ec5138e6..f580779835f1 100644 --- a/lib/galaxy/tools/harmonize_two_collections_list.xml +++ b/lib/galaxy/tools/harmonize_two_collections_list.xml @@ -171,6 +171,37 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Date: Fri, 29 Nov 2024 13:50:44 +0100 Subject: [PATCH 14/33] Fix test file type --- lib/galaxy/tools/harmonize_two_collections_list.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/harmonize_two_collections_list.xml b/lib/galaxy/tools/harmonize_two_collections_list.xml index f580779835f1..546351802e77 100644 --- a/lib/galaxy/tools/harmonize_two_collections_list.xml +++ b/lib/galaxy/tools/harmonize_two_collections_list.xml @@ -182,7 +182,7 @@ - + @@ -197,7 +197,7 @@ - + From 891d7aa0bfc8f70d0ece6e731687712b634a9bf5 Mon Sep 17 00:00:00 2001 From: Wolfgang Maier Date: Fri, 29 Nov 2024 15:08:35 +0100 Subject: [PATCH 15/33] Try to fix tool test --- lib/galaxy/tools/harmonize_two_collections_list.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/harmonize_two_collections_list.xml b/lib/galaxy/tools/harmonize_two_collections_list.xml index 546351802e77..3a12173f04e8 100644 --- a/lib/galaxy/tools/harmonize_two_collections_list.xml +++ b/lib/galaxy/tools/harmonize_two_collections_list.xml @@ -182,7 +182,7 @@ - + @@ -197,7 +197,7 @@ - + From 68efe8d1d0f55807997316eddedb9afe85a85310 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 3 Dec 2024 10:49:07 -0600 Subject: [PATCH 16/33] [24.2] Fix cancellation of workflow scheduling Fixes https://github.com/galaxyproject/galaxy/issues/19237 --- client/src/api/invocations.ts | 15 ++++++++++ .../WorkflowInvocationState.vue | 28 +++++++++++++------ .../WorkflowInvocationState/services.js | 14 ---------- 3 files changed, 35 insertions(+), 22 deletions(-) delete mode 100644 client/src/components/WorkflowInvocationState/services.js diff --git a/client/src/api/invocations.ts b/client/src/api/invocations.ts index 935fa7463bff..a6cb5957030f 100644 --- a/client/src/api/invocations.ts +++ b/client/src/api/invocations.ts @@ -1,3 +1,6 @@ +import { rethrowSimple } from "@/utils/simple-error"; + +import { GalaxyApi } from "./client"; import { type components } from "./schema"; export type WorkflowInvocationElementView = components["schemas"]["WorkflowInvocationElementView"]; @@ -12,3 +15,15 @@ export type StepJobSummary = | components["schemas"]["InvocationStepJobsResponseCollectionJobsModel"]; export type WorkflowInvocation = components["schemas"]["WorkflowInvocationResponse"]; + +export async function cancelWorkflowScheduling(invocationId: string) { + const { data, error } = await GalaxyApi().DELETE("/api/invocations/{invocation_id}", { + params: { + path: { invocation_id: invocationId }, + }, + }); + if (error) { + rethrowSimple(error); + } + return data; +} diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue b/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue index 839dda0cf7bb..8eaf178ee656 100644 --- a/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue +++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue @@ -5,12 +5,12 @@ import { BAlert, BBadge, BButton, BTab, BTabs } from "bootstrap-vue"; import { computed, onUnmounted, ref, watch } from "vue"; import { type InvocationJobsSummary, type InvocationStep, type WorkflowInvocationElementView } from "@/api/invocations"; +import { cancelWorkflowScheduling } from "@/api/invocations"; import { useAnimationFrameResizeObserver } from "@/composables/sensors/animationFrameResizeObserver"; import { useInvocationStore } from "@/stores/invocationStore"; import { useWorkflowStore } from "@/stores/workflowStore"; import { errorMessageAsString } from "@/utils/simple-error"; -import { cancelWorkflowScheduling } from "./services"; import { errorCount as jobStatesSummaryErrorCount, isTerminal, @@ -53,6 +53,7 @@ const stepStatesInterval = ref(undefined); const jobStatesInterval = ref(undefined); const invocationLoaded = ref(false); const errorMessage = ref(null); +const cancellingInvocation = ref(false); // after the report tab is first activated, no longer lazy-render it from then on const reportActive = ref(false); @@ -219,11 +220,22 @@ async function pollJobStatesUntilTerminal() { function onError(e: any) { console.error(e); } -function onCancel() { - emit("invocation-cancelled"); -} -function cancelWorkflowSchedulingLocal() { - cancelWorkflowScheduling(props.invocationId).then(onCancel).catch(onError); +async function onCancel() { + try { + cancellingInvocation.value = true; + await cancelWorkflowScheduling(props.invocationId); + } catch (e) { + onError(e); + } finally { + emit("invocation-cancelled"); + + // Update the invocation state to reflect the cancellation + setTimeout(async () => { + await invocationStore.fetchInvocationForId({ id: props.invocationId }); + await invocationStore.fetchInvocationJobsSummaryForId({ id: props.invocationId }); + cancellingInvocation.value = false; + }, 3000); + } } @@ -243,6 +255,7 @@ function cancelWorkflowSchedulingLocal() { size="sm" class="text-decoration-none" variant="link" + :disabled="cancellingInvocation" @click="onCancel"> Cancel @@ -302,8 +315,7 @@ function cancelWorkflowSchedulingLocal() { :is-full-page="props.isFullPage" :invocation-and-job-terminal="invocationAndJobTerminal" :invocation-scheduling-terminal="invocationSchedulingTerminal" - :is-subworkflow="isSubworkflow" - @invocation-cancelled="cancelWorkflowSchedulingLocal" /> + :is-subworkflow="isSubworkflow" /> getRootFromIndexLink() + path; - -export function getInvocationJobsSummary(invocationId) { - const url = getUrl(`api/invocations/${invocationId}/jobs_summary`); - return axios.get(url); -} - -export function cancelWorkflowScheduling(invocationId) { - const url = getUrl(`api/invocations/${invocationId}`); - return axios.delete(url); -} From 274da120234e58777ad4139481557734704a6cc2 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 3 Dec 2024 12:04:04 -0600 Subject: [PATCH 17/33] fix force routing for `WorkflowRunButton` and use force prop in `WorkflowNavigationTitle` --- .../Workflow/WorkflowNavigationTitle.vue | 1 + .../Workflow/WorkflowRunButton.test.ts | 60 +++++++++++++++++-- .../components/Workflow/WorkflowRunButton.vue | 18 ++++-- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/client/src/components/Workflow/WorkflowNavigationTitle.vue b/client/src/components/Workflow/WorkflowNavigationTitle.vue index 5237d61e7e12..a0d6afa6dbf5 100644 --- a/client/src/components/Workflow/WorkflowNavigationTitle.vue +++ b/client/src/components/Workflow/WorkflowNavigationTitle.vue @@ -159,6 +159,7 @@ const workflowImportTitle = computed(() => { : 'This workflow has been deleted.' " :disabled="workflow.deleted" + force full :version="workflow.version" /> diff --git a/client/src/components/Workflow/WorkflowRunButton.test.ts b/client/src/components/Workflow/WorkflowRunButton.test.ts index 0aec407666a4..431e8861a964 100644 --- a/client/src/components/Workflow/WorkflowRunButton.test.ts +++ b/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) { + // 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 }); }); }); diff --git a/client/src/components/Workflow/WorkflowRunButton.vue b/client/src/components/Workflow/WorkflowRunButton.vue index f5e26c981da9..7a3f94b79486 100644 --- a/client/src/components/Workflow/WorkflowRunButton.vue +++ b/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(), { 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); } } @@ -42,7 +52,7 @@ function forceRunPath() { class="text-decoration-none" :disabled="disabled" :to="runPath" - @click="forceRunPath"> + @click="routeToPath"> Run From 14159200effda0998467f77d58296d543cc27987 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 3 Dec 2024 12:07:18 -0600 Subject: [PATCH 18/33] use `to` prop for edit button in `WorkflowNavigationTitle` This shows a href route for the button in the browser, and keeping an empty `@click` event means we keep the button styling. --- .../Workflow/WorkflowNavigationTitle.test.ts | 19 ++++--------------- .../Workflow/WorkflowNavigationTitle.vue | 6 ++---- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/client/src/components/Workflow/WorkflowNavigationTitle.test.ts b/client/src/components/Workflow/WorkflowNavigationTitle.test.ts index 75dfa51431a9..8a6888138523 100644 --- a/client/src/components/Workflow/WorkflowNavigationTitle.test.ts +++ b/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", () => { @@ -138,15 +131,11 @@ describe("WorkflowNavigationTitle renders", () => { 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}` ); } diff --git a/client/src/components/Workflow/WorkflowNavigationTitle.vue b/client/src/components/Workflow/WorkflowNavigationTitle.vue index a0d6afa6dbf5..34b5f7d66fa9 100644 --- a/client/src/components/Workflow/WorkflowNavigationTitle.vue +++ b/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; @@ -122,7 +119,8 @@ 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}`" + @click="() => {}"> Date: Tue, 3 Dec 2024 13:06:14 -0600 Subject: [PATCH 19/33] match `WorkflowNavigationTitle` styling with `ToolCard` header --- client/src/components/Tool/ToolCard.vue | 15 +---- .../Workflow/Run/WorkflowRunFormSimple.vue | 3 +- .../Workflow/WorkflowNavigationTitle.vue | 66 +++++++++---------- client/src/style/scss/ui.scss | 11 ++++ 4 files changed, 45 insertions(+), 50 deletions(-) diff --git a/client/src/components/Tool/ToolCard.vue b/client/src/components/Tool/ToolCard.vue index f09527328008..f76955803d2b 100644 --- a/client/src/components/Tool/ToolCard.vue +++ b/client/src/components/Tool/ToolCard.vue @@ -118,7 +118,7 @@ const showHelpForum = computed(() => isConfigLoaded.value && config.value.enable diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue b/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue index cb32e4f11288..a3e530696926 100644 --- a/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue +++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue @@ -275,18 +275,7 @@ function itemToPieChartSpec(item: piechartData) { type: "nominal", legend: { type: "symbol", - title: "", // should be item.category_title but it doesn't align right so just hide it - direction: "vertical", - titleAlign: "right", - padding: 10, - // rowPadding: 3, - labelOffset: 40, - // symbolOffset: 50, - labelLimit: 120, - // labelAlign: 'center', - columnPadding: 5, - // clipHeight: 20, - titleOrient: "top", + title: item.category_title, }, }, tooltip: [ From e3ac780d64c7483bf9cce5a0b51136bef2d45567 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 9 Dec 2024 19:06:33 +0100 Subject: [PATCH 28/33] Increase PieChart legend font size Slightly increase for improved visibility --- .../WorkflowInvocationState/WorkflowInvocationMetrics.vue | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue b/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue index a3e530696926..2298d1880cd5 100644 --- a/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue +++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue @@ -276,6 +276,8 @@ function itemToPieChartSpec(item: piechartData) { legend: { type: "symbol", title: item.category_title, + titleFontSize: 16, + labelFontSize: 14, }, }, tooltip: [ From 631da6456d42d417d0ee85ecba5030f337bd919a Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 9 Dec 2024 19:08:42 +0100 Subject: [PATCH 29/33] Fix label for 'Group By' dropdown in Workflow Invocation Metrics Make it dynamic like the other one for time. --- .../WorkflowInvocationState/WorkflowInvocationMetrics.vue | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue b/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue index 2298d1880cd5..3a1a1bbf8e19 100644 --- a/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue +++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationMetrics.vue @@ -372,6 +372,10 @@ function getTimingInTitle(timing: string): string { const timingInTitles = computed(() => { return getTimingInTitle(timing.value); }); + +const groupByInTitles = computed(() => { + return attributeToLabel[groupBy.value]; +});