From 258f11f08d8b9379ca816501922e27d690b04b85 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Wed, 26 Apr 2023 11:20:10 +0100 Subject: [PATCH] Nested folders: deduplicate selection of children (#67229) * scaffold out dedupe logic, use mock api to get descendant info * rename methods * use for..of * some renaming for clarity --- .../api/browseDashboardsAPI.ts | 57 ++++++++++++++++++- .../BrowseActions/BrowseActions.tsx | 4 +- .../BrowseActions/DeleteModal.test.tsx | 7 ++- .../components/BrowseActions/DeleteModal.tsx | 25 ++++---- .../BrowseActions/MoveModal.test.tsx | 7 ++- .../components/BrowseActions/MoveModal.tsx | 28 ++++----- .../components/BrowseView.tsx | 4 +- .../features/browse-dashboards/state/hooks.ts | 56 +++++++++++++++++- 8 files changed, 152 insertions(+), 36 deletions(-) diff --git a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts index 3fe6e34364a..a3e0ca101a7 100644 --- a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts +++ b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts @@ -1,9 +1,12 @@ import { BaseQueryFn, createApi } from '@reduxjs/toolkit/query/react'; import { lastValueFrom } from 'rxjs'; +import { isTruthy } from '@grafana/data'; import { BackendSrvRequest, getBackendSrv } from '@grafana/runtime'; import { FolderDTO } from 'app/types'; +import { DashboardTreeSelection } from '../types'; + interface RequestOptions extends BackendSrvRequest { manageError?: (err: unknown) => { error: unknown }; showErrorAlert?: boolean; @@ -35,8 +38,60 @@ export const browseDashboardsAPI = createApi({ getFolder: builder.query({ query: (folderUID) => ({ url: `/folders/${folderUID}` }), }), + getAffectedItems: builder.query< + // TODO move to folder types file once structure is finalised + { + folder: number; + dashboard: number; + libraryPanel: number; + alertRule: number; + }, + DashboardTreeSelection + >({ + queryFn: async (selectedItems) => { + const folderUIDs = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); + // Mock descendant count + // TODO convert to real implementation + const mockDescendantCount = { + folder: 1, + dashboard: 1, + libraryPanel: 1, + alertRule: 1, + }; + const promises = folderUIDs.map((id) => { + return new Promise((resolve, reject) => { + // Artificial delay to simulate network request + setTimeout(() => { + resolve(mockDescendantCount); + // reject(new Error('Uh oh!')); + }, 1000); + }); + }); + + const results = await Promise.all(promises); + const aggregatedResults = results.reduce( + (acc, val) => ({ + folder: acc.folder + val.folder, + dashboard: acc.dashboard + val.dashboard, + libraryPanel: acc.libraryPanel + val.libraryPanel, + alertRule: acc.alertRule + val.alertRule, + }), + { + folder: 0, + dashboard: 0, + libraryPanel: 0, + alertRule: 0, + } + ); + + // Add in the top level selected items + aggregatedResults.folder += Object.values(selectedItems.folder).filter(isTruthy).length; + aggregatedResults.dashboard += Object.values(selectedItems.dashboard).filter(isTruthy).length; + return { data: aggregatedResults }; + }, + }), }), }); -export const { useGetFolderQuery } = browseDashboardsAPI; +export const { useGetFolderQuery, useGetAffectedItemsQuery } = browseDashboardsAPI; export { skipToken } from '@reduxjs/toolkit/query/react'; diff --git a/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx b/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx index 9d5aca6f4d4..235cddea5b4 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/BrowseActions.tsx @@ -6,7 +6,7 @@ import { Button, useStyles2 } from '@grafana/ui'; import appEvents from 'app/core/app_events'; import { ShowModalReactEvent } from 'app/types/events'; -import { useSelectedItemsState } from '../../state'; +import { useActionSelectionState } from '../../state'; import { DeleteModal } from './DeleteModal'; import { MoveModal } from './MoveModal'; @@ -15,7 +15,7 @@ export interface Props {} export function BrowseActions() { const styles = useStyles2(getStyles); - const selectedItems = useSelectedItemsState(); + const selectedItems = useActionSelectionState(); const onMove = () => { appEvents.publish( diff --git a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx index 95d1f53993f..aec7aef75b3 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx @@ -1,9 +1,14 @@ -import { render, screen } from '@testing-library/react'; +import { render as rtlRender, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { TestProvider } from 'test/helpers/TestProvider'; import { DeleteModal, Props } from './DeleteModal'; +function render(...[ui, options]: Parameters) { + rtlRender({ui}, options); +} + describe('browse-dashboards DeleteModal', () => { const mockOnDismiss = jest.fn(); const mockOnConfirm = jest.fn(); diff --git a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx index 6bba8ea1839..332688bb22e 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx @@ -1,9 +1,10 @@ import { css } from '@emotion/css'; import React from 'react'; -import { GrafanaTheme2, isTruthy } from '@grafana/data'; -import { ConfirmModal, useStyles2 } from '@grafana/ui'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Alert, ConfirmModal, Spinner, useStyles2 } from '@grafana/ui'; +import { useGetAffectedItemsQuery } from '../../api/browseDashboardsAPI'; import { DashboardTreeSelection } from '../../types'; import { buildBreakdownString } from './utils'; @@ -17,14 +18,7 @@ export interface Props { export const DeleteModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Props) => { const styles = useStyles2(getStyles); - - // TODO abstract all this counting logic out - const folderCount = Object.values(selectedItems.folder).filter(isTruthy).length; - const dashboardCount = Object.values(selectedItems.dashboard).filter(isTruthy).length; - // hardcoded values for now - // TODO replace with dummy API - const libraryPanelCount = 1; - const alertRuleCount = 1; + const { data, isFetching, isLoading, error } = useGetAffectedItemsQuery(selectedItems); const onDelete = () => { onConfirm(); @@ -36,9 +30,13 @@ export const DeleteModal = ({ onConfirm, onDismiss, selectedItems, ...props }: P body={
This action will delete the following content: -

- {buildBreakdownString(folderCount, dashboardCount, libraryPanelCount, alertRuleCount)} -

+
+ <> + {data && buildBreakdownString(data.folder, data.dashboard, data.libraryPanel, data.alertRule)} + {(isFetching || isLoading) && } + {error && } + +
} confirmationText="Delete" @@ -55,6 +53,7 @@ const getStyles = (theme: GrafanaTheme2) => ({ breakdown: css({ ...theme.typography.bodySmall, color: theme.colors.text.secondary, + marginBottom: theme.spacing(2), }), modalBody: css({ ...theme.typography.body, diff --git a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx index 21f37c6aad9..a5ae8362f95 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx @@ -1,6 +1,7 @@ -import { render, screen } from '@testing-library/react'; +import { render as rtlRender, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { TestProvider } from 'test/helpers/TestProvider'; import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; import * as api from 'app/features/manage-dashboards/state/actions'; @@ -8,6 +9,10 @@ import { DashboardSearchHit } from 'app/features/search/types'; import { MoveModal, Props } from './MoveModal'; +function render(...[ui, options]: Parameters) { + rtlRender({ui}, options); +} + describe('browse-dashboards MoveModal', () => { const mockOnDismiss = jest.fn(); const mockOnConfirm = jest.fn(); diff --git a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx index 8c8ac827dd2..54dcefb868d 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx @@ -1,10 +1,11 @@ import { css } from '@emotion/css'; import React, { useState } from 'react'; -import { GrafanaTheme2, isTruthy } from '@grafana/data'; -import { Alert, Button, Field, Modal, useStyles2 } from '@grafana/ui'; +import { GrafanaTheme2 } from '@grafana/data'; +import { Alert, Button, Field, Modal, Spinner, useStyles2 } from '@grafana/ui'; import { FolderPicker } from 'app/core/components/Select/FolderPicker'; +import { useGetAffectedItemsQuery } from '../../api/browseDashboardsAPI'; import { DashboardTreeSelection } from '../../types'; import { buildBreakdownString } from './utils'; @@ -19,14 +20,8 @@ export interface Props { export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Props) => { const [moveTarget, setMoveTarget] = useState(); const styles = useStyles2(getStyles); - - // TODO abstract all this counting logic out - const folderCount = Object.values(selectedItems.folder).filter(isTruthy).length; - const dashboardCount = Object.values(selectedItems.dashboard).filter(isTruthy).length; - // hardcoded values for now - // TODO replace with dummy API - const libraryPanelCount = 1; - const alertRuleCount = 1; + const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); + const { data, isFetching, isLoading, error } = useGetAffectedItemsQuery(selectedItems); const onMove = () => { if (moveTarget !== undefined) { @@ -37,11 +32,15 @@ export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Pro return ( - {folderCount > 0 && } + {selectedFolders.length > 0 && } This action will move the following content: -

- {buildBreakdownString(folderCount, dashboardCount, libraryPanelCount, alertRuleCount)} -

+
+ <> + {data && buildBreakdownString(data.folder, data.dashboard, data.libraryPanel, data.alertRule)} + {(isFetching || isLoading) && } + {error && } + +
setMoveTarget(uid)} /> @@ -61,5 +60,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ breakdown: css({ ...theme.typography.bodySmall, color: theme.colors.text.secondary, + marginBottom: theme.spacing(2), }), }); diff --git a/public/app/features/browse-dashboards/components/BrowseView.tsx b/public/app/features/browse-dashboards/components/BrowseView.tsx index 233dd22998a..8245f789213 100644 --- a/public/app/features/browse-dashboards/components/BrowseView.tsx +++ b/public/app/features/browse-dashboards/components/BrowseView.tsx @@ -5,7 +5,7 @@ import { useDispatch } from 'app/types'; import { useFlatTreeState, - useSelectedItemsState, + useCheckboxSelectionState, fetchChildren, setFolderOpenState, setItemSelectionState, @@ -22,7 +22,7 @@ interface BrowseViewProps { export function BrowseView({ folderUID, width, height }: BrowseViewProps) { const dispatch = useDispatch(); const flatTree = useFlatTreeState(folderUID); - const selectedItems = useSelectedItemsState(); + const selectedItems = useCheckboxSelectionState(); useEffect(() => { dispatch(fetchChildren(folderUID)); diff --git a/public/app/features/browse-dashboards/state/hooks.ts b/public/app/features/browse-dashboards/state/hooks.ts index e09bdb5e38d..dd6a78319bc 100644 --- a/public/app/features/browse-dashboards/state/hooks.ts +++ b/public/app/features/browse-dashboards/state/hooks.ts @@ -3,7 +3,7 @@ import { createSelector } from 'reselect'; import { DashboardViewItem } from 'app/features/search/types'; import { useSelector, StoreState } from 'app/types'; -import { DashboardsTreeItem } from '../types'; +import { DashboardsTreeItem, DashboardTreeSelection } from '../types'; const flatTreeSelector = createSelector( (wholeState: StoreState) => wholeState.browseDashboards.rootItems, @@ -24,6 +24,14 @@ const hasSelectionSelector = createSelector( } ); +const selectedItemsForActionsSelector = createSelector( + (wholeState: StoreState) => wholeState.browseDashboards.selectedItems, + (wholeState: StoreState) => wholeState.browseDashboards.childrenByParentUID, + (selectedItems, childrenByParentUID) => { + return getSelectedItemsForActions(selectedItems, childrenByParentUID); + } +); + export function useFlatTreeState(folderUID: string | undefined) { return useSelector((state) => flatTreeSelector(state, folderUID)); } @@ -32,10 +40,14 @@ export function useHasSelection() { return useSelector((state) => hasSelectionSelector(state)); } -export function useSelectedItemsState() { +export function useCheckboxSelectionState() { return useSelector((wholeState: StoreState) => wholeState.browseDashboards.selectedItems); } +export function useActionSelectionState() { + return useSelector((state) => selectedItemsForActionsSelector(state)); +} + /** * Creates a list of items, with level indicating it's 'nested' in the tree structure * @@ -83,3 +95,43 @@ function createFlatTree( return items.flatMap((item) => mapItem(item, folderUID, level)); } + +/** + * Returns a DashboardTreeSelection but unselects any selected folder's children. + * This is useful when making backend requests to move or delete items. + * In this case, we only need to move/delete the parent folder and it will cascade to the children. + * @param selectedItemsState Overall selection state + * @param childrenByParentUID Arrays of children keyed by their parent UID + */ +function getSelectedItemsForActions( + selectedItemsState: DashboardTreeSelection, + childrenByParentUID: Record +): Omit { + // Take a copy of the selected items to work with + // We don't care about panels here, only dashboards and folders can be moved or deleted + const result: Omit = { + dashboard: { ...selectedItemsState.dashboard }, + folder: { ...selectedItemsState.folder }, + }; + + // Loop over selected folders in the input + for (const folderUID of Object.keys(selectedItemsState.folder)) { + const isSelected = selectedItemsState.folder[folderUID]; + if (isSelected) { + // Unselect any children in the output + const children = childrenByParentUID[folderUID]; + if (children) { + for (const child of children) { + if (child.kind === 'dashboard') { + result.dashboard[child.uid] = false; + } + if (child.kind === 'folder') { + result.folder[child.uid] = false; + } + } + } + } + } + + return result; +}