From 2569ab8d715960055de81d871d9f9b3c18447f0a Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Wed, 14 Jun 2023 11:00:52 +0100 Subject: [PATCH] [v10.0.x] NestedFolders: Fix select all in folder view selecting items out of folder (#69783) NestedFolders: Fix select all in folder view selecting items out of folder (#69780) * NestedFolders: Fix select all selecting items outside of folder when viewing a folder * fix lint errors (cherry picked from commit ff89217d66e2bbada472db21dc50368edb648403) --- .../BrowseDashboardsPage.tsx | 1 + .../BrowseActions/BrowseActions.tsx | 7 +-- .../components/BrowseView.tsx | 2 +- .../components/SearchView.tsx | 2 +- .../browse-dashboards/state/reducers.test.ts | 54 ++++++++++++------- .../browse-dashboards/state/reducers.ts | 29 +++++++--- 6 files changed, 62 insertions(+), 33 deletions(-) diff --git a/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx b/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx index 5c30b1c6cda..27f56a5bfff 100644 --- a/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx +++ b/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx @@ -45,6 +45,7 @@ const BrowseDashboardsPage = memo(({ match }: Props) => { dispatch( setAllSelection({ isSelected: false, + folderUID: undefined, }) ); }, [dispatch, folderUID, stateManager]); diff --git a/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx b/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx index 07b607d1c7f..bb02c3dffec 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx @@ -38,11 +38,8 @@ export function BrowseActions() { const isSearching = stateManager.hasSearchFilters(); const onActionComplete = (parentsToRefresh: Set) => { - dispatch( - setAllSelection({ - isSelected: false, - }) - ); + dispatch(setAllSelection({ isSelected: false, folderUID: undefined })); + if (isSearching) { // Redo search query stateManager.doSearchWithDebounce(); diff --git a/public/app/features/browse-dashboards/components/BrowseView.tsx b/public/app/features/browse-dashboards/components/BrowseView.tsx index 8afd4cb9d51..a5a6816b431 100644 --- a/public/app/features/browse-dashboards/components/BrowseView.tsx +++ b/public/app/features/browse-dashboards/components/BrowseView.tsx @@ -128,7 +128,7 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr height={height} isSelected={isSelected} onFolderClick={handleFolderClick} - onAllSelectionChange={(newState) => dispatch(setAllSelection({ isSelected: newState }))} + onAllSelectionChange={(newState) => dispatch(setAllSelection({ isSelected: newState, folderUID }))} onItemSelectionChange={handleItemSelectionChange} /> ); diff --git a/public/app/features/browse-dashboards/components/SearchView.tsx b/public/app/features/browse-dashboards/components/SearchView.tsx index 1c4653de2a9..f2e4b2d19de 100644 --- a/public/app/features/browse-dashboards/components/SearchView.tsx +++ b/public/app/features/browse-dashboards/components/SearchView.tsx @@ -46,7 +46,7 @@ export function SearchView({ width, height, canSelect }: SearchViewProps) { ); const clearSelection = useCallback(() => { - dispatch(setAllSelection({ isSelected: false })); + dispatch(setAllSelection({ isSelected: false, folderUID: undefined })); }, [dispatch]); const handleItemSelectionChange = useCallback( diff --git a/public/app/features/browse-dashboards/state/reducers.test.ts b/public/app/features/browse-dashboards/state/reducers.test.ts index 6d3334347a1..c1eb542c209 100644 --- a/public/app/features/browse-dashboards/state/reducers.test.ts +++ b/public/app/features/browse-dashboards/state/reducers.test.ts @@ -252,15 +252,15 @@ describe('browse-dashboards reducers', () => { }); describe('setAllSelection', () => { - it('selects all loaded items', () => { - const state = createInitialState(); + let seed = 1; + const topLevelDashboard = wellFormedDashboard(seed++).item; + const topLevelFolder = wellFormedFolder(seed++).item; + const childDashboard = wellFormedDashboard(seed++, {}, { parentUID: topLevelFolder.uid }).item; + const childFolder = wellFormedFolder(seed++, {}, { parentUID: topLevelFolder.uid }).item; + const grandchildDashboard = wellFormedDashboard(seed++, {}, { parentUID: childFolder.uid }).item; - let seed = 1; - const topLevelDashboard = wellFormedDashboard(seed++).item; - const topLevelFolder = wellFormedFolder(seed++).item; - const childDashboard = wellFormedDashboard(seed++, {}, { parentUID: topLevelFolder.uid }).item; - const childFolder = wellFormedFolder(seed++, {}, { parentUID: topLevelFolder.uid }).item; - const grandchildDashboard = wellFormedDashboard(seed++, {}, { parentUID: childFolder.uid }).item; + it('selects all items in the root folder', () => { + const state = createInitialState(); state.rootItems = [topLevelFolder, topLevelDashboard]; state.childrenByParentUID[topLevelFolder.uid] = [childDashboard, childFolder]; @@ -269,7 +269,7 @@ describe('browse-dashboards reducers', () => { state.selectedItems.folder[childFolder.uid] = false; state.selectedItems.dashboard[grandchildDashboard.uid] = true; - setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: true } }); + setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: true, folderUID: undefined } }); expect(state.selectedItems).toEqual({ $all: true, @@ -286,16 +286,9 @@ describe('browse-dashboards reducers', () => { }); }); - it('deselects all items', () => { + it('selects all items when viewing a folder', () => { const state = createInitialState(); - let seed = 1; - const topLevelDashboard = wellFormedDashboard(seed++).item; - const topLevelFolder = wellFormedFolder(seed++).item; - const childDashboard = wellFormedDashboard(seed++, {}, { parentUID: topLevelFolder.uid }).item; - const childFolder = wellFormedFolder(seed++, {}, { parentUID: topLevelFolder.uid }).item; - const grandchildDashboard = wellFormedDashboard(seed++, {}, { parentUID: childFolder.uid }).item; - state.rootItems = [topLevelFolder, topLevelDashboard]; state.childrenByParentUID[topLevelFolder.uid] = [childDashboard, childFolder]; state.childrenByParentUID[childFolder.uid] = [grandchildDashboard]; @@ -303,7 +296,32 @@ describe('browse-dashboards reducers', () => { state.selectedItems.folder[childFolder.uid] = false; state.selectedItems.dashboard[grandchildDashboard.uid] = true; - setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: false } }); + setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: true, folderUID: topLevelFolder.uid } }); + + expect(state.selectedItems).toEqual({ + $all: true, + dashboard: { + [childDashboard.uid]: true, + [grandchildDashboard.uid]: true, + }, + folder: { + [childFolder.uid]: true, + }, + panel: {}, + }); + }); + + it('deselects all items', () => { + const state = createInitialState(); + + state.rootItems = [topLevelFolder, topLevelDashboard]; + state.childrenByParentUID[topLevelFolder.uid] = [childDashboard, childFolder]; + state.childrenByParentUID[childFolder.uid] = [grandchildDashboard]; + + state.selectedItems.folder[childFolder.uid] = false; + state.selectedItems.dashboard[grandchildDashboard.uid] = true; + + setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: false, folderUID: undefined } }); // Deselecting only sets selection = false for things already selected expect(state.selectedItems).toEqual({ diff --git a/public/app/features/browse-dashboards/state/reducers.ts b/public/app/features/browse-dashboards/state/reducers.ts index f9dde18434e..733b188a38a 100644 --- a/public/app/features/browse-dashboards/state/reducers.ts +++ b/public/app/features/browse-dashboards/state/reducers.ts @@ -90,8 +90,11 @@ export function setItemSelectionState( state.selectedItems.$all = state.rootItems?.every((v) => state.selectedItems[v.kind][v.uid]) ?? false; } -export function setAllSelection(state: BrowseDashboardsState, action: PayloadAction<{ isSelected: boolean }>) { - const { isSelected } = action.payload; +export function setAllSelection( + state: BrowseDashboardsState, + action: PayloadAction<{ isSelected: boolean; folderUID: string | undefined }> +) { + const { isSelected, folderUID: folderUIDArg } = action.payload; state.selectedItems.$all = isSelected; @@ -102,17 +105,27 @@ export function setAllSelection(state: BrowseDashboardsState, action: PayloadAct // redux, so we just need to iterate over the selected items to flip them to false if (isSelected) { - for (const folderUID in state.childrenByParentUID) { - const children = state.childrenByParentUID[folderUID] ?? []; + // Recursively select the children of the folder in view + function selectChildrenOfFolder(folderUID: string | undefined) { + const collection = folderUID ? state.childrenByParentUID[folderUID] : state.rootItems; - for (const child of children) { + // Bail early if the collection isn't found (not loaded yet) + if (!collection) { + return; + } + + for (const child of collection) { state.selectedItems[child.kind][child.uid] = isSelected; + + if (child.kind !== 'folder') { + continue; + } + + selectChildrenOfFolder(child.uid); } } - for (const child of state.rootItems ?? []) { - state.selectedItems[child.kind][child.uid] = isSelected; - } + selectChildrenOfFolder(folderUIDArg); } else { // if deselecting only need to loop over what we've already selected for (const kind in state.selectedItems) {