From c994951a7164e86909117745162dfec286ee31ab Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 8 May 2024 19:33:42 +0200 Subject: [PATCH 01/10] Fix activity bar touch events on iphone I'm not sure what the issue is here, the minimal fix is this: ``` diff --git a/client/src/components/Popper/usePopper.ts b/client/src/components/Popper/usePopper.ts index ad18847d8d..183d2e49be 100644 --- a/client/src/components/Popper/usePopper.ts +++ b/client/src/components/Popper/usePopper.ts @@ -190,7 +190,7 @@ export function usePopperjs( on(referenceRef.value!, "mouseout", doMouseout); on(popperRef.value!, "mouseout", doMouseout); on(referenceRef.value!, "mousedown", doMouseout); - on(popperRef.value!, "mousedown", doMouseout); + // on(popperRef.value!, "mousedown", doMouseout); break; } ``` but I think we can probably just rely on events happening on referenceRef. --- client/src/components/Popper/usePopper.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/client/src/components/Popper/usePopper.ts b/client/src/components/Popper/usePopper.ts index ad18847d8da..8a2155e6a62 100644 --- a/client/src/components/Popper/usePopper.ts +++ b/client/src/components/Popper/usePopper.ts @@ -186,19 +186,14 @@ export function usePopperjs( case "hover": { on(referenceRef.value!, "mouseover", doMouseover); - on(popperRef.value!, "mouseover", doMouseover); on(referenceRef.value!, "mouseout", doMouseout); - on(popperRef.value!, "mouseout", doMouseout); on(referenceRef.value!, "mousedown", doMouseout); - on(popperRef.value!, "mousedown", doMouseout); break; } case "focus": { on(referenceRef.value!, "focus", doOpen); - on(popperRef.value!, "focus", doOpen); on(referenceRef.value!, "blur", doClose); - on(popperRef.value!, "blur", doClose); break; } @@ -218,16 +213,11 @@ export function usePopperjs( off(referenceRef.value!, "click", doToggle); off(referenceRef.value!, "mouseover", doMouseover); - off(popperRef.value!, "mouseover", doMouseover); off(referenceRef.value!, "mouseout", doMouseout); - off(popperRef.value!, "mouseout", doMouseout); off(referenceRef.value!, "mousedown", doMouseout); - off(popperRef.value!, "mousedown", doMouseout); off(referenceRef.value!, "focus", doOpen); - off(popperRef.value!, "focus", doOpen); off(referenceRef.value!, "blur", doClose); - off(popperRef.value!, "blur", doClose); }; const doCloseForDocument = (e: Event) => { if (referenceRef.value?.contains(e.target as Element)) { From bec08c38e95c07076f866a0d10961e34749e7811 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 9 May 2024 12:00:07 +0200 Subject: [PATCH 02/10] Fix and unify axios error handling Fixes https://sentry.galaxyproject.org/organizations/galaxy/issues/151896/?project=3&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=24h&stream_index=8: ``` TypeError: undefined is not an object (evaluating 'e.response.data') at (webpack://@galaxyproject/galaxy-client/src/components/Login/RegisterForm.vue:201:43) ``` --- client/src/components/Login/ChangePassword.vue | 4 ++-- client/src/components/Login/LoginForm.vue | 9 ++++----- client/src/components/Login/NewUserConfirmation.vue | 4 ++-- client/src/components/Login/RegisterForm.vue | 4 ++-- .../components/User/ExternalIdentities/ExternalLogin.vue | 8 +++----- 5 files changed, 13 insertions(+), 16 deletions(-) diff --git a/client/src/components/Login/ChangePassword.vue b/client/src/components/Login/ChangePassword.vue index 9f47887b027..51cb8076085 100644 --- a/client/src/components/Login/ChangePassword.vue +++ b/client/src/components/Login/ChangePassword.vue @@ -17,6 +17,7 @@ import axios from "axios"; import BootstrapVue from "bootstrap-vue"; import { withPrefix } from "utils/redirect"; +import { errorMessageAsString } from "utils/simple-error"; import Vue from "vue"; Vue.use(BootstrapVue); @@ -64,8 +65,7 @@ export default { }) .catch((error) => { this.variant = "danger"; - const message = error.response && error.response.data && error.response.data.err_msg; - this.message = message || "Password change failed for an unknown reason."; + this.message = errorMessageAsString(error, "Password change failed for an unknown reason."); }); }, }, diff --git a/client/src/components/Login/LoginForm.vue b/client/src/components/Login/LoginForm.vue index 626265bf1c9..95fce8e9116 100644 --- a/client/src/components/Login/LoginForm.vue +++ b/client/src/components/Login/LoginForm.vue @@ -106,12 +106,12 @@ import BootstrapVue from "bootstrap-vue"; import ExternalLogin from "components/User/ExternalIdentities/ExternalLogin"; import _l from "utils/localization"; import { withPrefix } from "utils/redirect"; +import { errorMessageAsString } from "utils/simple-error"; import Vue from "vue"; import NewUserConfirmation from "./NewUserConfirmation"; Vue.use(BootstrapVue); - export default { components: { ExternalLogin, @@ -212,12 +212,12 @@ export default { }) .catch((error) => { this.messageVariant = "danger"; - const message = error.response && error.response.data && error.response.data.err_msg; + const message = errorMessageAsString(error, "Login failed for an unknown reason."); if (this.connectExternalProvider && message && message.toLowerCase().includes("invalid")) { this.messageText = message + " Try logging in to the existing account through an external provider below."; } else { - this.messageText = message || "Login failed for an unknown reason."; + this.messageText = message; } }); }, @@ -233,8 +233,7 @@ export default { }) .catch((error) => { this.messageVariant = "danger"; - const message = error.response.data && error.response.data.err_msg; - this.messageText = message || "Password reset failed for an unknown reason."; + this.messageText = errorMessageAsString(error, "Password reset failed for an unknown reason."); }); }, returnToLogin() { diff --git a/client/src/components/Login/NewUserConfirmation.vue b/client/src/components/Login/NewUserConfirmation.vue index 48c0d4104f4..f0a1821bcde 100644 --- a/client/src/components/Login/NewUserConfirmation.vue +++ b/client/src/components/Login/NewUserConfirmation.vue @@ -60,6 +60,7 @@ import axios from "axios"; import BootstrapVue from "bootstrap-vue"; import { withPrefix } from "utils/redirect"; +import { errorMessageAsString } from "utils/simple-error"; import Vue from "vue"; Vue.use(BootstrapVue); @@ -112,8 +113,7 @@ export default { }) .catch((error) => { this.messageVariant = "danger"; - const message = error.response.data && error.response.data.err_msg; - this.messageText = message || "Login failed for an unknown reason."; + this.messageText = errorMessageAsString(error, "Login failed for an unknown reason."); }); } }, diff --git a/client/src/components/Login/RegisterForm.vue b/client/src/components/Login/RegisterForm.vue index 0a8e9d31a3f..b0fcd5e3fbb 100644 --- a/client/src/components/Login/RegisterForm.vue +++ b/client/src/components/Login/RegisterForm.vue @@ -106,6 +106,7 @@ import BootstrapVue from "bootstrap-vue"; import ExternalLogin from "components/User/ExternalIdentities/ExternalLogin"; import _l from "utils/localization"; import { withPrefix } from "utils/redirect"; +import { errorMessageAsString } from "utils/simple-error"; import Vue from "vue"; Vue.use(BootstrapVue); @@ -198,8 +199,7 @@ export default { .catch((error) => { this.disableCreate = false; this.messageVariant = "danger"; - const message = error.response.data && error.response.data.err_msg; - this.messageText = message || "Registration failed for an unknown reason."; + this.messageText = errorMessageAsString(error, "Registration failed for an unknown reason."); }); }, }, diff --git a/client/src/components/User/ExternalIdentities/ExternalLogin.vue b/client/src/components/User/ExternalIdentities/ExternalLogin.vue index 12f4a28243d..950b2da8e3e 100644 --- a/client/src/components/User/ExternalIdentities/ExternalLogin.vue +++ b/client/src/components/User/ExternalIdentities/ExternalLogin.vue @@ -101,6 +101,7 @@ import axios from "axios"; import BootstrapVue from "bootstrap-vue"; import LoadingSpan from "components/LoadingSpan"; import { getAppRoot } from "onload"; +import { errorMessageAsString } from "utils/simple-error"; import Vue from "vue"; import Multiselect from "vue-multiselect"; @@ -186,8 +187,7 @@ export default { }) .catch((error) => { this.messageVariant = "danger"; - const message = error.response.data && error.response.data.err_msg; - this.messageText = message || "Login failed for an unknown reason."; + this.messageText = errorMessageAsString(error, "Login failed for an unknown reason."); this.loading = false; }); }, @@ -208,9 +208,7 @@ export default { }) .catch((error) => { this.messageVariant = "danger"; - const message = error.response.data && error.response.data.err_msg; - - this.messageText = message || "Login failed for an unknown reason."; + this.messageText = errorMessageAsString(error, "Login failed for an unknown reason."); this.loading = false; }); }, From f70d556fc68f8fa49f56394e4c301e4b4a25bc3d Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 3 May 2024 15:18:50 -0400 Subject: [PATCH 03/10] Raise exception if collection elements missing during download --- lib/galaxy/managers/hdcas.py | 3 +++ lib/galaxy/webapps/galaxy/api/history_contents.py | 10 ++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py index ad5bcda58e8..59b1fc4a5d4 100644 --- a/lib/galaxy/managers/hdcas.py +++ b/lib/galaxy/managers/hdcas.py @@ -42,6 +42,9 @@ def stream_dataset_collection(dataset_collection_instance, upstream_mod_zip=Fals def write_dataset_collection(dataset_collection_instance, archive): names, hdas = get_hda_and_element_identifiers(dataset_collection_instance) for name, hda in zip(names, hdas): + if not hda: + # TODO should we raise galaxy.exceptions.InternalServerError or create a new exception type? + raise Exception("Attempt to write dataset collection with missing elements") if hda.state != hda.states.OK: continue for file_path, relpath in hda.datatype.to_archive(dataset=hda, name=name): diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index bfab8f0ef44..1bb1e21ade1 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -626,8 +626,7 @@ class FastAPIHistoryContents: """Download the content of a history dataset collection as a `zip` archive while maintaining approximate collection structure. """ - archive = self.service.get_dataset_collection_archive_for_download(trans, id) - return StreamingResponse(archive.response(), headers=archive.get_headers()) + return self._download_collection(trans, id) @router.get( "/api/dataset_collections/{id}/download", @@ -644,8 +643,7 @@ class FastAPIHistoryContents: """Download the content of a history dataset collection as a `zip` archive while maintaining approximate collection structure. """ - archive = self.service.get_dataset_collection_archive_for_download(trans, id) - return StreamingResponse(archive.response(), headers=archive.get_headers()) + return self._download_collection(trans, id) @router.post( "/api/histories/{history_id}/contents/dataset_collections/{id}/prepare_download", @@ -1033,3 +1031,7 @@ class FastAPIHistoryContents: ) rval = self.service.materialize(trans, materialize_request) return rval + + def _download_collection(self, trans, id): + archive = self.service.get_dataset_collection_archive_for_download(trans, id) + return StreamingResponse(archive.response(), headers=archive.get_headers()) From 48771a9f6addf065ef308bb1ca1f76fa2f415569 Mon Sep 17 00:00:00 2001 From: John Davis Date: Mon, 6 May 2024 13:43:40 -0400 Subject: [PATCH 04/10] Raise appropriate error; check collection populated state --- lib/galaxy/managers/hdcas.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py index 59b1fc4a5d4..39756b300f4 100644 --- a/lib/galaxy/managers/hdcas.py +++ b/lib/galaxy/managers/hdcas.py @@ -9,6 +9,7 @@ import logging from typing import Dict from galaxy import model +from galaxy.exceptions import RequestParameterInvalidException from galaxy.managers import ( annotatable, base, @@ -40,11 +41,10 @@ def stream_dataset_collection(dataset_collection_instance, upstream_mod_zip=Fals def write_dataset_collection(dataset_collection_instance, archive): + if not dataset_collection_instance.populated_optimized(): + raise RequestParameterInvalidException("Attempt to write dataset collection that has not been populated yet") names, hdas = get_hda_and_element_identifiers(dataset_collection_instance) for name, hda in zip(names, hdas): - if not hda: - # TODO should we raise galaxy.exceptions.InternalServerError or create a new exception type? - raise Exception("Attempt to write dataset collection with missing elements") if hda.state != hda.states.OK: continue for file_path, relpath in hda.datatype.to_archive(dataset=hda, name=name): From 0045692760fc93a05e852e0e9a771f13ec1b66a6 Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 9 May 2024 12:08:20 -0400 Subject: [PATCH 05/10] Access `populated_optimized` on DatasetCollection, not instance. Co-authored-by: Marius van den Beek --- lib/galaxy/managers/hdcas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py index 39756b300f4..69e0d3dfdcd 100644 --- a/lib/galaxy/managers/hdcas.py +++ b/lib/galaxy/managers/hdcas.py @@ -41,7 +41,7 @@ def stream_dataset_collection(dataset_collection_instance, upstream_mod_zip=Fals def write_dataset_collection(dataset_collection_instance, archive): - if not dataset_collection_instance.populated_optimized(): + if not dataset_collection_instance.collection.populated_optimized(): raise RequestParameterInvalidException("Attempt to write dataset collection that has not been populated yet") names, hdas = get_hda_and_element_identifiers(dataset_collection_instance) for name, hda in zip(names, hdas): From 880f3efdca1540876bb58ea5847307e393f01917 Mon Sep 17 00:00:00 2001 From: John Davis Date: Thu, 9 May 2024 13:16:23 -0400 Subject: [PATCH 06/10] Fix bug (populated_optimized is property, not method) --- lib/galaxy/managers/hdcas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py index 69e0d3dfdcd..42d44824515 100644 --- a/lib/galaxy/managers/hdcas.py +++ b/lib/galaxy/managers/hdcas.py @@ -41,7 +41,7 @@ def stream_dataset_collection(dataset_collection_instance, upstream_mod_zip=Fals def write_dataset_collection(dataset_collection_instance, archive): - if not dataset_collection_instance.collection.populated_optimized(): + if not dataset_collection_instance.collection.populated_optimized: raise RequestParameterInvalidException("Attempt to write dataset collection that has not been populated yet") names, hdas = get_hda_and_element_identifiers(dataset_collection_instance) for name, hda in zip(names, hdas): From f218a396a0b7f21778315e1d6f5c1aa5c78aae74 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 10 May 2024 16:08:44 -0400 Subject: [PATCH 07/10] Disable download link if collection not populated --- .../History/CurrentCollection/CollectionOperations.vue | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/src/components/History/CurrentCollection/CollectionOperations.vue b/client/src/components/History/CurrentCollection/CollectionOperations.vue index 2843caddf80..c8c20b4bef5 100644 --- a/client/src/components/History/CurrentCollection/CollectionOperations.vue +++ b/client/src/components/History/CurrentCollection/CollectionOperations.vue @@ -18,6 +18,8 @@ const rerunUrl = computed(() => const showCollectionDetailsUrl = computed(() => props.dsc.job_source_type == "Job" ? `/jobs/${props.dsc.job_source_id}/view` : null ); +const disableDownload = props.dsc.populated_state !== "ok"; + function onDownload() { window.location.href = downloadUrl.value; } @@ -28,6 +30,7 @@ function onDownload() { Date: Wed, 8 May 2024 16:39:18 +0200 Subject: [PATCH 08/10] wait for store reset to propagate --- client/src/components/Workflow/Editor/Index.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index 5149c039f8e..c8bf482560b 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -450,6 +450,7 @@ export default { }, async onRefactor(response) { this.resetStores(); + await nextTick(); await fromSimple(this.id, response.workflow); this._loadEditorData(response.workflow); }, From 9a938002df60fdae358a894d43449ad49cafc283 Mon Sep 17 00:00:00 2001 From: Laila Los <44241786+ElectronicBlueberry@users.noreply.github.com> Date: Mon, 13 May 2024 15:20:03 +0200 Subject: [PATCH 09/10] move next tick to reset stores function --- client/src/components/Workflow/Editor/Index.vue | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index c8bf482560b..d3cd633cfd4 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -278,16 +278,17 @@ export default { } }); - function resetStores() { + async function resetStores() { hasChanges.value = false; connectionStore.$reset(); stepStore.$reset(); stateStore.$reset(); commentStore.$reset(); + await nextTick(); } - onUnmounted(() => { - resetStores(); + onUnmounted(async () => { + await resetStores(); emit("update:confirmation", false); }); @@ -449,8 +450,7 @@ export default { hide_modal(); // hide other modals created in utilities also... }, async onRefactor(response) { - this.resetStores(); - await nextTick(); + await this.resetStores(); await fromSimple(this.id, response.workflow); this._loadEditorData(response.workflow); }, @@ -789,7 +789,7 @@ export default { }, async _loadCurrent(id, version) { if (!this.isNewTempWorkflow) { - this.resetStores(); + await this.resetStores(); this.onWorkflowMessage("Loading workflow...", "progress"); try { From 299b6079538c5902d3919bdd07a7a1ac015b1b66 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 13 May 2024 17:36:12 +0200 Subject: [PATCH 10/10] Fix listing possibly untitled records in Invenio Plugin Ensure the record title is always available and provide a default `No title` otherwise. Even if the title is a required attribute, users can save draft records without title until they are published. --- lib/galaxy/files/sources/invenio.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/files/sources/invenio.py b/lib/galaxy/files/sources/invenio.py index 6edc46dfe78..99b64cdf5a4 100644 --- a/lib/galaxy/files/sources/invenio.py +++ b/lib/galaxy/files/sources/invenio.py @@ -139,7 +139,7 @@ class InvenioRDMFilesSource(RDMFilesSource): record = self.repository.create_draft_record(entry_data["name"], public_name, user_context=user_context) return { "uri": self.repository.to_plugin_uri(record["id"]), - "name": record["metadata"]["title"], + "name": record["title"], "external_link": record["links"]["self_html"], } @@ -225,6 +225,7 @@ class InvenioRepositoryInteractor(RDMRepositoryInteractor): response = requests.post(self.records_url, json=create_record_request, headers=headers) self._ensure_response_has_expected_status_code(response, 201) record = response.json() + record["title"] = self._get_record_title(record) return record def upload_file_to_draft_record( @@ -331,16 +332,23 @@ class InvenioRepositoryInteractor(RDMRepositoryInteractor): for record in records: uri = self.to_plugin_uri(record_id=record["id"]) path = self.plugin.to_relative_path(uri) + name = self._get_record_title(record) rval.append( { "class": "Directory", - "name": record["metadata"]["title"], + "name": name, "uri": uri, "path": path, } ) return rval + def _get_record_title(self, record: InvenioRecord) -> str: + title = record.get("title") + if not title and "metadata" in record: + title = record["metadata"].get("title") + return title or "No title" + def _get_record_files_from_response(self, record_id: str, response: dict) -> List[RemoteFile]: files_enabled = response.get("enabled", False) if not files_enabled: