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 1/4] 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 147e38622ceb..733652a214be 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 2/4] 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 733652a214be..9acf326bdf40 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: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 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 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 3/4] 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 27e478cea34b..0930569fb50f 100644 --- a/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue +++ b/client/src/components/History/CurrentHistory/HistoryOperations/SelectionOperations.vue @@ -13,10 +13,13 @@ With {{ numSelected }} selected... - + Unhide - + Hide Undelete Delete @@ -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 4/4] 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 0930569fb50f..c3165c5a5d96 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) {