From 5f14cd25a6ecd5f71fdcfc620320f11863bf6fa2 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 7 Feb 2024 14:50:28 +0100 Subject: [PATCH 1/3] Add test coverage for "any" state in menu options --- .../SelectionOperations.test.js | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js index c66ecb6d197d..147e38622ceb 100644 --- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js +++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.test.js @@ -82,20 +82,33 @@ describe("History Selection Operations", () => { expect(wrapper.find(option).exists()).toBe(false); }); - it("should display 'unhide' option only on hidden items", async () => { + it("should display 'unhide' option on hidden items", async () => { const option = '[data-description="unhide option"]'; expect(wrapper.find(option).exists()).toBe(false); await wrapper.setProps({ filterText: "visible:false" }); expect(wrapper.find(option).exists()).toBe(true); }); - it("should display 'delete' option only on non-deleted items", async () => { + it("should display 'unhide' option when hidden and visible items are mixed", async () => { + const option = '[data-description="unhide option"]'; + 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"]'; 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"]'; + 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"]'; expect(wrapper.find(option).exists()).toBe(true); @@ -103,7 +116,7 @@ describe("History Selection Operations", () => { expect(wrapper.find(option).exists()).toBe(true); }); - it("should display 'undelete' option only on deleted and non-purged items", async () => { + it("should display 'undelete' option on deleted and non-purged items", async () => { const option = '[data-description="undelete option"]'; expect(wrapper.find(option).exists()).toBe(false); await wrapper.setProps({ @@ -113,6 +126,15 @@ describe("History Selection Operations", () => { expect(wrapper.find(option).exists()).toBe(true); }); + it("should display 'undelete' option when non-purged items (deleted or not) are mixed", async () => { + const option = '[data-description="undelete option"]'; + await wrapper.setProps({ + filterText: "deleted:any", + contentSelection: getNonPurgedContentSelection(), + }); + expect(wrapper.find(option).exists()).toBe(true); + }); + it("should not display 'undelete' when is manual selection mode and all selected items are purged", async () => { const option = '[data-description="undelete option"]'; await wrapper.setProps({ @@ -134,6 +156,17 @@ describe("History Selection Operations", () => { expect(wrapper.find(option).exists()).toBe(true); }); + it("should display 'undelete' option when is query selection mode and filtering by any deleted state", async () => { + const option = '[data-description="undelete option"]'; + // 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, + }); + expect(wrapper.find(option).exists()).toBe(true); + }); + it("should display collection building options only on visible and non-deleted items", async () => { const buildListOption = '[data-description="build list"]'; const buildPairOption = '[data-description="build pair"]'; @@ -149,6 +182,14 @@ describe("History Selection Operations", () => { 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: "deleted:any" }); + expect(wrapper.find(buildListOption).exists()).toBe(false); + expect(wrapper.find(buildPairOption).exists()).toBe(false); + expect(wrapper.find(buildListOfPairsOption).exists()).toBe(false); }); it("should display list building option when all are selected", async () => { From bc0b3eb98d0d7426cc425ce84a64a24c83efd364 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 7 Feb 2024 14:52:53 +0100 Subject: [PATCH 2/3] Fix menu display conditions to adapt to "any" filter --- .../HistoryOperations/SelectionOperations.vue | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue index 4954da962ccd..27e478cea34b 100644 --- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue +++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue @@ -25,7 +25,10 @@ data-description="undelete option"> Undelete - + Delete @@ -193,10 +196,14 @@ export default { computed: { /** @returns {Boolean} */ showHidden() { - return HistoryFilters.checkFilter(this.filterText, "visible", false); + return !HistoryFilters.checkFilter(this.filterText, "visible", true); }, /** @returns {Boolean} */ showDeleted() { + return !HistoryFilters.checkFilter(this.filterText, "deleted", false); + }, + /** @returns {Boolean} */ + showStrictDeleted() { return HistoryFilters.checkFilter(this.filterText, "deleted", true); }, /** @returns {Boolean} */ From f60f0ae6e3710a85fca7110f55bd722e8d0fe43c Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 7 Feb 2024 14:58:52 +0100 Subject: [PATCH 3/3] Fix new edge case where all items might be already deleted This case was previously prevented at the UI level but should be considered here. --- lib/galaxy/webapps/galaxy/services/history_contents.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py index 669361389068..68f5c789ea22 100644 --- a/lib/galaxy/webapps/galaxy/services/history_contents.py +++ b/lib/galaxy/webapps/galaxy/services/history_contents.py @@ -1417,8 +1417,12 @@ def _unhide(self, item: HistoryItemModel): def _delete(self, item: HistoryItemModel, trans: ProvidesHistoryContext): if isinstance(item, HistoryDatasetCollectionAssociation): - return self.dataset_collection_manager.delete(trans, "history", item.id, recursive=True, purge=False) - return self.hda_manager.delete(item, flush=self.flush) + self.dataset_collection_manager.delete(trans, "history", item.id, recursive=True, purge=False) + else: + self.hda_manager.delete(item, flush=self.flush) + # In the edge case where all selected items are already deleted we need to force an update + # otherwise the history will wait indefinitely for the items to be deleted + item.update() def _undelete(self, item: HistoryItemModel): if getattr(item, "purged", False):