From 3a2c844a8abe363de2fb9889b771caf1ad529f00 Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Wed, 6 May 2020 12:53:28 +0300 Subject: [PATCH] Search: folder view improvements (#24324) * Search: Do not set items if no results returned * Search: Simplify canSave logic * Search: Add initialLoading state * Search: Add itemsFetching state to folder * Search: Fix props and tests * Search: Fix strict null check --- public/app/core/services/search_srv.ts | 2 +- .../search/components/DashboardActions.tsx | 2 +- .../search/components/DashboardListPage.tsx | 4 +- .../search/components/ManageDashboards.tsx | 32 ++++++++---- .../search/components/SearchResults.tsx | 4 +- .../search/components/SectionHeader.tsx | 4 +- .../search/hooks/useManageDashboards.ts | 35 +++++-------- public/app/features/search/hooks/useSearch.ts | 14 ++--- .../features/search/reducers/actionTypes.ts | 3 +- .../search/reducers/dashboardSearch.test.ts | 52 +++++++------------ .../search/reducers/dashboardSearch.ts | 23 ++++++-- .../search/reducers/manageDashboards.test.ts | 19 +------ .../search/reducers/manageDashboards.ts | 17 +----- public/app/features/search/types.ts | 1 + 14 files changed, 90 insertions(+), 122 deletions(-) diff --git a/public/app/core/services/search_srv.ts b/public/app/core/services/search_srv.ts index 25e7ff91511..ff9d1f8c5f4 100644 --- a/public/app/core/services/search_srv.ts +++ b/public/app/core/services/search_srv.ts @@ -78,7 +78,7 @@ export class SearchSrv { if (query.layout === SearchLayout.List) { return backendSrv .search({ ...query, type: DashboardSearchItemType.DashDB }) - .then(results => [{ items: results }]); + .then(results => (results.length ? [{ items: results }] : [])); } if (!filters) { diff --git a/public/app/features/search/components/DashboardActions.tsx b/public/app/features/search/components/DashboardActions.tsx index 6ae7632b662..5360b3579e0 100644 --- a/public/app/features/search/components/DashboardActions.tsx +++ b/public/app/features/search/components/DashboardActions.tsx @@ -4,7 +4,7 @@ import { HorizontalGroup, LinkButton } from '@grafana/ui'; export interface Props { folderId?: number; isEditor: boolean; - canEdit: boolean; + canEdit?: boolean; } export const DashboardActions: FC = ({ folderId, isEditor, canEdit }) => { diff --git a/public/app/features/search/components/DashboardListPage.tsx b/public/app/features/search/components/DashboardListPage.tsx index 5aa01ec6012..f502ed13acc 100644 --- a/public/app/features/search/components/DashboardListPage.tsx +++ b/public/app/features/search/components/DashboardListPage.tsx @@ -28,14 +28,14 @@ export const DashboardListPage: FC = memo(({ navModel, uid, url }) => { getLocationSrv().update({ path }); } - return { id: folder.id, pageNavModel: { ...navModel, ...model } }; + return { folder, pageNavModel: { ...navModel, ...model } }; }); }, [uid]); return ( - + ); diff --git a/public/app/features/search/components/ManageDashboards.tsx b/public/app/features/search/components/ManageDashboards.tsx index f933d62a4e7..28e13db3b33 100644 --- a/public/app/features/search/components/ManageDashboards.tsx +++ b/public/app/features/search/components/ManageDashboards.tsx @@ -1,27 +1,29 @@ import React, { FC, memo, useState } from 'react'; import { css } from 'emotion'; -import { HorizontalGroup, stylesFactory, useTheme } from '@grafana/ui'; +import { HorizontalGroup, stylesFactory, useTheme, Spinner } from '@grafana/ui'; import { GrafanaTheme } from '@grafana/data'; import { contextSrv } from 'app/core/services/context_srv'; import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA'; +import { FilterInput } from 'app/core/components/FilterInput/FilterInput'; +import { FolderDTO } from 'app/types'; +import { useManageDashboards } from '../hooks/useManageDashboards'; +import { SearchLayout } from '../types'; import { ConfirmDeleteModal } from './ConfirmDeleteModal'; import { MoveToFolderModal } from './MoveToFolderModal'; import { useSearchQuery } from '../hooks/useSearchQuery'; -import { useManageDashboards } from '../hooks/useManageDashboards'; import { SearchResultsFilter } from './SearchResultsFilter'; import { SearchResults } from './SearchResults'; import { DashboardActions } from './DashboardActions'; -import { SearchLayout } from '../types'; -import { FilterInput } from 'app/core/components/FilterInput/FilterInput'; export interface Props { - folderId?: number; - folderUid?: string; + folder?: FolderDTO; } const { isEditor } = contextSrv; -export const ManageDashboards: FC = memo(({ folderId, folderUid }) => { +export const ManageDashboards: FC = memo(({ folder }) => { + const folderId = folder?.id; + const folderUid = folder?.uid; const theme = useTheme(); const styles = getStyles(theme); const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); @@ -47,6 +49,7 @@ export const ManageDashboards: FC = memo(({ folderId, folderUid }) => { const { results, loading, + initialLoading, canSave, allChecked, hasEditPermissionInFolders, @@ -57,7 +60,8 @@ export const ManageDashboards: FC = memo(({ folderId, folderUid }) => { onToggleAllChecked, onDeleteItems, onMoveItems, - } = useManageDashboards(query, { hasEditPermissionInFolders: contextSrv.hasEditPermissionInFolders }, folderUid); + noFolders, + } = useManageDashboards(query, {}, folder); const onMoveTo = () => { setIsMoveModalOpen(true); @@ -67,7 +71,11 @@ export const ManageDashboards: FC = memo(({ folderId, folderUid }) => { setIsDeleteModalOpen(true); }; - if (canSave && folderId && !hasFilters && results.length === 0 && !loading) { + if (initialLoading) { + return ; + } + + if (noFolders && !hasFilters) { return ( { height: 100%; margin-top: ${theme.spacing.xl}; `, + spinner: css` + display: flex; + justify-content: center; + align-items: center; + min-height: 200px; + `, }; }); diff --git a/public/app/features/search/components/SearchResults.tsx b/public/app/features/search/components/SearchResults.tsx index 7d4522bd508..ef444ddea8c 100644 --- a/public/app/features/search/components/SearchResults.tsx +++ b/public/app/features/search/components/SearchResults.tsx @@ -4,7 +4,7 @@ import { FixedSizeList } from 'react-window'; import AutoSizer from 'react-virtualized-auto-sizer'; import { GrafanaTheme } from '@grafana/data'; import { stylesFactory, useTheme, Spinner } from '@grafana/ui'; -import { DashboardSection, OnToggleChecked, SearchLayout, DashboardSearchHit } from '../types'; +import { DashboardSection, OnToggleChecked, SearchLayout } from '../types'; import { SEARCH_ITEM_HEIGHT, SEARCH_ITEM_MARGIN } from '../constants'; import { SearchItem } from './SearchItem'; import { SectionHeader } from './SectionHeader'; @@ -15,7 +15,7 @@ export interface Props { onTagSelected: (name: string) => any; onToggleChecked?: OnToggleChecked; onToggleSection: (section: DashboardSection) => void; - results: DashboardSearchHit[]; + results: DashboardSection[]; layout?: string; } diff --git a/public/app/features/search/components/SectionHeader.tsx b/public/app/features/search/components/SectionHeader.tsx index 689128f27c2..a2c4e597005 100644 --- a/public/app/features/search/components/SectionHeader.tsx +++ b/public/app/features/search/components/SectionHeader.tsx @@ -1,7 +1,7 @@ import React, { FC, useCallback } from 'react'; import { css, cx } from 'emotion'; import { GrafanaTheme } from '@grafana/data'; -import { Icon, IconButton, stylesFactory, useTheme } from '@grafana/ui'; +import { Icon, IconButton, Spinner, stylesFactory, useTheme } from '@grafana/ui'; import { DashboardSection, OnToggleChecked } from '../types'; import { SearchCheckbox } from './SearchCheckbox'; import { getSectionIcon } from '../utils'; @@ -51,7 +51,7 @@ export const SectionHeader: FC = ({ )} - + {section.itemsFetching ? : } ); }; diff --git a/public/app/features/search/hooks/useManageDashboards.ts b/public/app/features/search/hooks/useManageDashboards.ts index a48b89cb0bf..5cdcdb94c90 100644 --- a/public/app/features/search/hooks/useManageDashboards.ts +++ b/public/app/features/search/hooks/useManageDashboards.ts @@ -1,43 +1,26 @@ import { useMemo, useReducer } from 'react'; -import { backendSrv } from 'app/core/services/backend_srv'; +import { FolderDTO } from 'app/types'; +import { contextSrv } from 'app/core/services/context_srv'; import { DashboardQuery, DashboardSection, OnDeleteItems, OnMoveItems, OnToggleChecked } from '../types'; -import { - DELETE_ITEMS, - MOVE_ITEMS, - TOGGLE_ALL_CHECKED, - TOGGLE_CHECKED, - TOGGLE_CAN_SAVE, - TOGGLE_EDIT_PERMISSIONS, -} from '../reducers/actionTypes'; +import { DELETE_ITEMS, MOVE_ITEMS, TOGGLE_ALL_CHECKED, TOGGLE_CHECKED } from '../reducers/actionTypes'; import { manageDashboardsReducer, manageDashboardsState, ManageDashboardsState } from '../reducers/manageDashboards'; import { useSearch } from './useSearch'; export const useManageDashboards = ( query: DashboardQuery, state: Partial = {}, - folderUid?: string + folder?: FolderDTO ) => { const reducer = useReducer(manageDashboardsReducer, { ...manageDashboardsState, ...state, }); - const searchCallback = (folderUid: string | undefined) => { - if (folderUid) { - backendSrv.getFolderByUid(folderUid).then(folder => { - dispatch({ type: TOGGLE_CAN_SAVE, payload: folder.canSave }); - if (!folder.canSave) { - dispatch({ type: TOGGLE_EDIT_PERMISSIONS, payload: false }); - } - }); - } - }; - const { - state: { results, loading, canSave, allChecked, hasEditPermissionInFolders }, + state: { results, loading, initialLoading, allChecked }, onToggleSection, dispatch, - } = useSearch(query, reducer, { folderUid, searchCallback }); + } = useSearch(query, reducer, {}); const onToggleChecked: OnToggleChecked = item => { dispatch({ type: TOGGLE_CHECKED, payload: item }); @@ -64,9 +47,14 @@ export const useManageDashboards = ( results, ]); + const canSave = folder?.canSave; + const hasEditPermissionInFolders = canSave ? contextSrv.hasEditPermissionInFolders : false; + const noFolders = canSave && folder?.id && results.length === 0 && !loading && !initialLoading; + return { results, loading, + initialLoading, canSave, allChecked, hasEditPermissionInFolders, @@ -77,5 +65,6 @@ export const useManageDashboards = ( onToggleAllChecked, onDeleteItems, onMoveItems, + noFolders, }; }; diff --git a/public/app/features/search/hooks/useSearch.ts b/public/app/features/search/hooks/useSearch.ts index 7be66337856..58b90e46714 100644 --- a/public/app/features/search/hooks/useSearch.ts +++ b/public/app/features/search/hooks/useSearch.ts @@ -2,7 +2,7 @@ import { useEffect } from 'react'; import { useDebounce } from 'react-use'; import { SearchSrv } from 'app/core/services/search_srv'; import { backendSrv } from 'app/core/services/backend_srv'; -import { FETCH_RESULTS, FETCH_ITEMS, TOGGLE_SECTION, SEARCH_START } from '../reducers/actionTypes'; +import { FETCH_RESULTS, FETCH_ITEMS, TOGGLE_SECTION, SEARCH_START, FETCH_ITEMS_START } from '../reducers/actionTypes'; import { DashboardSection, UseSearch } from '../types'; import { hasId, getParsedQuery } from '../utils'; @@ -16,8 +16,8 @@ const searchSrv = new SearchSrv(); * @param reducer - return result of useReducer * @param params - custom params */ -export const useSearch: UseSearch = (query, reducer, params) => { - const { queryParsing, folderUid, searchCallback } = params; +export const useSearch: UseSearch = (query, reducer, params = {}) => { + const { queryParsing } = params; const [state, dispatch] = reducer; const search = () => { @@ -25,10 +25,6 @@ export const useSearch: UseSearch = (query, reducer, params) => { const parsedQuery = getParsedQuery(query, queryParsing); searchSrv.search(parsedQuery).then(results => { dispatch({ type: FETCH_RESULTS, payload: results }); - - if (searchCallback) { - searchCallback(folderUid); - } }); }; @@ -37,11 +33,11 @@ export const useSearch: UseSearch = (query, reducer, params) => { dispatch({ type: SEARCH_START }); }, [query.tag, query.sort, query.starred, query.layout]); - useDebounce(search, 300, [query, folderUid, queryParsing]); + useDebounce(search, 300, [query, queryParsing]); - // TODO as possible improvement, show spinner after expanding section while items are fetching const onToggleSection = (section: DashboardSection) => { if (hasId(section.title) && !section.items.length) { + dispatch({ type: FETCH_ITEMS_START, payload: section.id }); backendSrv.search({ folderIds: [section.id] }).then(items => { dispatch({ type: FETCH_ITEMS, payload: { section, items } }); dispatch({ type: TOGGLE_SECTION, payload: section }); diff --git a/public/app/features/search/reducers/actionTypes.ts b/public/app/features/search/reducers/actionTypes.ts index e45368cce31..3e2d92fa35a 100644 --- a/public/app/features/search/reducers/actionTypes.ts +++ b/public/app/features/search/reducers/actionTypes.ts @@ -1,13 +1,12 @@ export const FETCH_RESULTS = 'FETCH_RESULTS'; export const TOGGLE_SECTION = 'TOGGLE_SECTION'; export const FETCH_ITEMS = 'FETCH_ITEMS'; +export const FETCH_ITEMS_START = 'FETCH_ITEMS_START'; export const MOVE_SELECTION_UP = 'MOVE_SELECTION_UP'; export const MOVE_SELECTION_DOWN = 'MOVE_SELECTION_DOWN'; export const SEARCH_START = 'SEARCH_START'; // Manage dashboards -export const TOGGLE_CAN_SAVE = 'TOGGLE_CAN_SAVE'; -export const TOGGLE_EDIT_PERMISSIONS = 'TOGGLE_EDIT_PERMISSIONS'; export const TOGGLE_ALL_CHECKED = 'TOGGLE_ALL_CHECKED'; export const TOGGLE_CHECKED = 'TOGGLE_SECTION_CHECKED'; export const MOVE_ITEMS = 'MOVE_ITEMS'; diff --git a/public/app/features/search/reducers/dashboardSearch.test.ts b/public/app/features/search/reducers/dashboardSearch.test.ts index 85d3cb32bd8..fc47dfd5435 100644 --- a/public/app/features/search/reducers/dashboardSearch.test.ts +++ b/public/app/features/search/reducers/dashboardSearch.test.ts @@ -2,6 +2,7 @@ import { FETCH_ITEMS, FETCH_RESULTS, TOGGLE_SECTION, MOVE_SELECTION_DOWN, MOVE_S import { searchReducer as reducer, dashboardsSearchState } from './dashboardSearch'; import { searchResults, sections } from '../testData'; +const defaultState = { selectedIndex: 0, loading: false, results: sections as any[], initialLoading: false }; describe('Dashboard Search reducer', () => { it('should return the initial state', () => { expect(reducer(dashboardsSearchState, {} as any)).toEqual(dashboardsSearchState); @@ -9,20 +10,14 @@ describe('Dashboard Search reducer', () => { it('should set the results and mark first item as selected', () => { const newState = reducer(dashboardsSearchState, { type: FETCH_RESULTS, payload: searchResults }); - expect(newState).toEqual({ loading: false, selectedIndex: 0, results: searchResults }); + expect(newState).toEqual({ loading: false, selectedIndex: 0, results: searchResults, initialLoading: false }); expect(newState.results[0].selected).toBeTruthy(); }); it('should toggle selected section', () => { - const newState = reducer( - { selectedIndex: 0, loading: false, results: sections as any[] }, - { type: TOGGLE_SECTION, payload: sections[5] } - ); + const newState = reducer(defaultState, { type: TOGGLE_SECTION, payload: sections[5] }); expect(newState.results[5].expanded).toBeFalsy(); - const newState2 = reducer( - { selectedIndex: 0, loading: false, results: sections as any[] }, - { type: TOGGLE_SECTION, payload: sections[1] } - ); + const newState2 = reducer(defaultState, { type: TOGGLE_SECTION, payload: sections[1] }); expect(newState2.results[1].expanded).toBeTruthy(); }); @@ -43,26 +38,20 @@ describe('Dashboard Search reducer', () => { isStarred: false, }, ]; - const newState = reducer( - { selectedIndex: 0, loading: false, results: sections as any[] }, - { - type: FETCH_ITEMS, - payload: { - section: sections[2], - items, - }, - } - ); + const newState = reducer(defaultState, { + type: FETCH_ITEMS, + payload: { + section: sections[2], + items, + }, + }); expect(newState.results[2].items).toEqual(items); }); it('should handle MOVE_SELECTION_DOWN', () => { - const newState = reducer( - { loading: false, selectedIndex: 0, results: sections as any[] }, - { - type: MOVE_SELECTION_DOWN, - } - ); + const newState = reducer(defaultState, { + type: MOVE_SELECTION_DOWN, + }); expect(newState.selectedIndex).toEqual(1); expect(newState.results[0].items[0].selected).toBeTruthy(); @@ -76,7 +65,7 @@ describe('Dashboard Search reducer', () => { // Shouldn't go over the visible results length - 1 (9) const newState3 = reducer( - { loading: false, selectedIndex: 9, results: sections as any[] }, + { ...defaultState, selectedIndex: 9 }, { type: MOVE_SELECTION_DOWN, } @@ -86,17 +75,14 @@ describe('Dashboard Search reducer', () => { it('should handle MOVE_SELECTION_UP', () => { // shouldn't move beyond 0 - const newState = reducer( - { loading: false, selectedIndex: 0, results: sections as any[] }, - { - type: MOVE_SELECTION_UP, - } - ); + const newState = reducer(defaultState, { + type: MOVE_SELECTION_UP, + }); expect(newState.selectedIndex).toEqual(0); const newState2 = reducer( - { loading: false, selectedIndex: 3, results: sections as any[] }, + { ...defaultState, selectedIndex: 3 }, { type: MOVE_SELECTION_UP, } diff --git a/public/app/features/search/reducers/dashboardSearch.ts b/public/app/features/search/reducers/dashboardSearch.ts index 967aca821f0..2bc42c7b5a6 100644 --- a/public/app/features/search/reducers/dashboardSearch.ts +++ b/public/app/features/search/reducers/dashboardSearch.ts @@ -1,4 +1,4 @@ -import { DashboardSearchHit, DashboardSection, SearchAction } from '../types'; +import { DashboardSection, SearchAction } from '../types'; import { getFlattenedSections, getLookupField, markSelected } from '../utils'; import { FETCH_ITEMS, @@ -7,17 +7,21 @@ import { MOVE_SELECTION_DOWN, MOVE_SELECTION_UP, SEARCH_START, + FETCH_ITEMS_START, } from './actionTypes'; export interface DashboardsSearchState { - results: DashboardSearchHit[]; + results: DashboardSection[]; loading: boolean; selectedIndex: number; + /** Used for first time page load */ + initialLoading: boolean; } export const dashboardsSearchState: DashboardsSearchState = { results: [], loading: true, + initialLoading: true, selectedIndex: 0, }; @@ -34,7 +38,7 @@ export const searchReducer = (state: DashboardsSearchState, action: SearchAction if (results.length > 0) { results[0].selected = true; } - return { ...state, results, loading: false }; + return { ...state, results, loading: false, initialLoading: false }; } case TOGGLE_SECTION: { const section = action.payload; @@ -53,14 +57,25 @@ export const searchReducer = (state: DashboardsSearchState, action: SearchAction const { section, items } = action.payload; return { ...state, + itemsFetching: false, results: state.results.map((result: DashboardSection) => { if (section.id === result.id) { - return { ...result, items }; + return { ...result, items, itemsFetching: false }; } return result; }), }; } + case FETCH_ITEMS_START: { + const id = action.payload; + if (id) { + return { + ...state, + results: state.results.map(result => (result.id === id ? { ...result, itemsFetching: true } : result)), + }; + } + return state; + } case MOVE_SELECTION_DOWN: { const flatIds = getFlattenedSections(state.results); if (state.selectedIndex < flatIds.length - 1) { diff --git a/public/app/features/search/reducers/manageDashboards.test.ts b/public/app/features/search/reducers/manageDashboards.test.ts index 5e16060e52e..3f4cff7fd1b 100644 --- a/public/app/features/search/reducers/manageDashboards.test.ts +++ b/public/app/features/search/reducers/manageDashboards.test.ts @@ -1,11 +1,4 @@ -import { - TOGGLE_CAN_SAVE, - TOGGLE_EDIT_PERMISSIONS, - TOGGLE_ALL_CHECKED, - TOGGLE_CHECKED, - DELETE_ITEMS, - MOVE_ITEMS, -} from './actionTypes'; +import { TOGGLE_ALL_CHECKED, TOGGLE_CHECKED, DELETE_ITEMS, MOVE_ITEMS } from './actionTypes'; import { manageDashboardsReducer as reducer, manageDashboardsState as state } from './manageDashboards'; import { sections } from '../testData'; import { UidsToDelete } from '../types'; @@ -34,16 +27,6 @@ describe('Manage dashboards reducer', () => { expect(newState2.allChecked).toBe(false); }); - it('should handle TOGGLE_CAN_SAVE', () => { - const newState = reducer(state, { type: TOGGLE_CAN_SAVE, payload: true }); - expect(newState.canSave).toBe(true); - }); - - it('should handle TOGGLE_EDIT_PERMISSIONS', () => { - const newState = reducer(state, { type: TOGGLE_EDIT_PERMISSIONS, payload: true }); - expect(newState.hasEditPermissionInFolders).toBe(true); - }); - it('should handle TOGGLE_CHECKED sections', () => { const newState = reducer({ ...state, results }, { type: TOGGLE_CHECKED, payload: results[0] }); expect(newState.results[0].checked).toBe(true); diff --git a/public/app/features/search/reducers/manageDashboards.ts b/public/app/features/search/reducers/manageDashboards.ts index 86189917443..2ef8a9b4935 100644 --- a/public/app/features/search/reducers/manageDashboards.ts +++ b/public/app/features/search/reducers/manageDashboards.ts @@ -1,34 +1,19 @@ import { DashboardSectionItem, SearchAction } from '../types'; -import { - TOGGLE_CAN_SAVE, - TOGGLE_EDIT_PERMISSIONS, - TOGGLE_ALL_CHECKED, - TOGGLE_CHECKED, - MOVE_ITEMS, - DELETE_ITEMS, -} from './actionTypes'; +import { TOGGLE_ALL_CHECKED, TOGGLE_CHECKED, MOVE_ITEMS, DELETE_ITEMS } from './actionTypes'; import { dashboardsSearchState, DashboardsSearchState, searchReducer } from './dashboardSearch'; import { mergeReducers } from '../utils'; export interface ManageDashboardsState extends DashboardsSearchState { - canSave: boolean; allChecked: boolean; - hasEditPermissionInFolders: boolean; } export const manageDashboardsState: ManageDashboardsState = { ...dashboardsSearchState, - canSave: false, allChecked: false, - hasEditPermissionInFolders: false, }; const reducer = (state: ManageDashboardsState, action: SearchAction) => { switch (action.type) { - case TOGGLE_CAN_SAVE: - return { ...state, canSave: action.payload }; - case TOGGLE_EDIT_PERMISSIONS: - return { ...state, hasEditPermissionInFolders: action.payload }; case TOGGLE_ALL_CHECKED: const newAllChecked = !state.allChecked; return { diff --git a/public/app/features/search/types.ts b/public/app/features/search/types.ts index cb14285d79b..1c827c03e0c 100644 --- a/public/app/features/search/types.ts +++ b/public/app/features/search/types.ts @@ -23,6 +23,7 @@ export interface DashboardSection { selected?: boolean; type: DashboardSearchItemType; slug?: string; + itemsFetching?: boolean; } export interface DashboardSectionItem {