From c136ad1f16f58e84b1a51f177a5a3f9023e251fc Mon Sep 17 00:00:00 2001 From: Giordano Ricci Date: Fri, 24 Feb 2023 11:48:30 +0000 Subject: [PATCH] Datasource Onboarding: Prevent flickering of onboarding page after first load (#63360) * Datasource Onboarding: Prevent flickering of onboarding page after first load * add loading state to loadDatasources & refactor * fix test * avoid loading state when loading datasources on add * fix test * add explainer on why fetching datasources is needed --- .../dashboard/containers/NewDashboardPage.tsx | 15 +++++---------- .../datasources/components/DataSourcesList.tsx | 5 ++--- .../datasources/components/NewDataSource.tsx | 4 ++-- .../pages/EditDataSourcePage.test.tsx | 2 +- .../app/features/datasources/state/actions.ts | 15 ++++++++++++--- public/app/features/datasources/state/hooks.ts | 4 ++++ .../features/datasources/state/reducers.test.ts | 8 ++++---- .../app/features/datasources/state/reducers.ts | 13 +++++++++---- .../app/features/explore/EmptyStateWrapper.tsx | 17 +++++------------ public/app/types/datasources.ts | 2 +- 10 files changed, 45 insertions(+), 40 deletions(-) diff --git a/public/app/features/dashboard/containers/NewDashboardPage.tsx b/public/app/features/dashboard/containers/NewDashboardPage.tsx index 368b98abeb3..f33fb8f5076 100644 --- a/public/app/features/dashboard/containers/NewDashboardPage.tsx +++ b/public/app/features/dashboard/containers/NewDashboardPage.tsx @@ -1,24 +1,19 @@ import React, { useState } from 'react'; -import { useEffectOnce } from 'react-use'; import { config } from '@grafana/runtime'; import { t } from 'app/core/internationalization'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; import { EmptyStateNoDatasource } from 'app/features/datasources/components/EmptyStateNoDatasource'; -import { loadDataSources } from 'app/features/datasources/state'; -import { useDispatch, useSelector } from 'app/types'; +import { useLoadDataSources } from 'app/features/datasources/state'; +import { useSelector } from 'app/types'; import DashboardPage from './DashboardPage'; export default function NewDashboardPage(props: GrafanaRouteComponentProps) { - const dispatch = useDispatch(); - useEffectOnce(() => { - dispatch(loadDataSources()); - }); + const { isLoading } = useLoadDataSources(); - const { hasDatasource, loading } = useSelector((state) => ({ + const { hasDatasource } = useSelector((state) => ({ hasDatasource: state.dataSources.dataSourcesCount > 0, - loading: !state.dataSources.hasFetched, })); const [createDashboard, setCreateDashboard] = useState(false); const showDashboardPage = hasDatasource || createDashboard || !config.featureToggles.datasourceOnboarding; @@ -28,7 +23,7 @@ export default function NewDashboardPage(props: GrafanaRouteComponentProps) { ) : ( setCreateDashboard(true)} - loading={loading} + loading={isLoading} title={t('datasource-onboarding.welcome', 'Welcome to Grafana dashboards!')} CTAText={t('datasource-onboarding.sampleData', 'Or set up a new dashboard with sample data')} navId="dashboards/browse" diff --git a/public/app/features/datasources/components/DataSourcesList.tsx b/public/app/features/datasources/components/DataSourcesList.tsx index 1ed271312ff..d18b2fdef53 100644 --- a/public/app/features/datasources/components/DataSourcesList.tsx +++ b/public/app/features/datasources/components/DataSourcesList.tsx @@ -17,11 +17,10 @@ import { constructDataSourceExploreUrl } from '../utils'; import { DataSourcesListHeader } from './DataSourcesListHeader'; export function DataSourcesList() { - useLoadDataSources(); + const { isLoading } = useLoadDataSources(); const dataSources = useSelector((state) => getDataSources(state.dataSources)); const dataSourcesCount = useSelector(({ dataSources }: StoreState) => getDataSourcesCount(dataSources)); - const hasFetched = useSelector(({ dataSources }: StoreState) => dataSources.hasFetched); const hasCreateRights = contextSrv.hasPermission(AccessControlAction.DataSourcesCreate); const hasWriteRights = contextSrv.hasPermission(AccessControlAction.DataSourcesWrite); const hasExploreRights = contextSrv.hasPermission(AccessControlAction.DataSourcesExplore); @@ -30,7 +29,7 @@ export function DataSourcesList() { getFilteredDataSourcePlugins(s.dataSources)); const searchQuery = useSelector((s: StoreState) => s.dataSources.dataSourceTypeSearchQuery); - const isLoading = useSelector((s: StoreState) => s.dataSources.isLoadingDataSources); + const isLoadingDatasourcePlugins = useSelector((s: StoreState) => s.dataSources.isLoadingDataSourcePlugins); const dataSourceCategories = useSelector((s: StoreState) => s.dataSources.categories); const onAddDataSource = useAddDatasource(); const onSetSearchQuery = (q: string) => dispatch(setDataSourceTypeSearchQuery(q)); @@ -33,7 +33,7 @@ export function NewDataSource() { dataSources={filteredDataSources} dataSourceCategories={dataSourceCategories} searchQuery={searchQuery} - isLoading={isLoading} + isLoading={isLoadingDatasourcePlugins} onAddDataSource={onAddDataSource} onSetSearchQuery={onSetSearchQuery} /> diff --git a/public/app/features/datasources/pages/EditDataSourcePage.test.tsx b/public/app/features/datasources/pages/EditDataSourcePage.test.tsx index ad9ac025879..30cbc03d60e 100644 --- a/public/app/features/datasources/pages/EditDataSourcePage.test.tsx +++ b/public/app/features/datasources/pages/EditDataSourcePage.test.tsx @@ -80,7 +80,7 @@ describe('', () => { dataSource: dataSource, dataSourceMeta: dataSourceMeta, layoutMode: LayoutModes.Grid, - hasFetched: true, + isLoadingDataSources: false, }, navIndex: { ...navIndex, diff --git a/public/app/features/datasources/state/actions.ts b/public/app/features/datasources/state/actions.ts index 4f5ac276fdd..972e58f8e38 100644 --- a/public/app/features/datasources/state/actions.ts +++ b/public/app/features/datasources/state/actions.ts @@ -28,6 +28,7 @@ import { dataSourceMetaLoaded, dataSourcePluginsLoad, dataSourcePluginsLoaded, + dataSourcesLoad, dataSourcesLoaded, initDataSourceSettingsFailed, initDataSourceSettingsSucceeded, @@ -144,8 +145,9 @@ export const testDataSource = ( }; }; -export function loadDataSources(): ThunkResult { +export function loadDataSources(): ThunkResult> { return async (dispatch) => { + dispatch(dataSourcesLoad()); const response = await api.getDataSources(); dispatch(dataSourcesLoaded(response)); }; @@ -193,9 +195,16 @@ export function loadDataSourceMeta(dataSource: DataSourceSettings): ThunkResult< }; } -export function addDataSource(plugin: DataSourcePluginMeta, editRoute = DATASOURCES_ROUTES.Edit): ThunkResult { +export function addDataSource( + plugin: DataSourcePluginMeta, + editRoute = DATASOURCES_ROUTES.Edit +): ThunkResult> { return async (dispatch, getStore) => { - await dispatch(loadDataSources()); + // update the list of datasources first. + // We later use this list to check whether the name of the datasource + // being created is unuque or not and assign a new name to it if needed. + const response = await api.getDataSources(); + dispatch(dataSourcesLoaded(response)); const dataSources = getStore().dataSources.dataSources; const isFirstDataSource = dataSources.length === 0; diff --git a/public/app/features/datasources/state/hooks.ts b/public/app/features/datasources/state/hooks.ts index 3687ea740fc..8877233d2d9 100644 --- a/public/app/features/datasources/state/hooks.ts +++ b/public/app/features/datasources/state/hooks.ts @@ -51,10 +51,14 @@ export const useTestDataSource = (uid: string) => { export const useLoadDataSources = () => { const dispatch = useDispatch(); + const isLoading = useSelector((state) => state.dataSources.isLoadingDataSources); + const dataSources = useSelector((state) => state.dataSources.dataSources); useEffect(() => { dispatch(loadDataSources()); }, [dispatch]); + + return { isLoading, dataSources }; }; export const useLoadDataSource = (uid: string) => { diff --git a/public/app/features/datasources/state/reducers.test.ts b/public/app/features/datasources/state/reducers.test.ts index f3adfbd78e2..078b28fbe24 100644 --- a/public/app/features/datasources/state/reducers.test.ts +++ b/public/app/features/datasources/state/reducers.test.ts @@ -47,7 +47,7 @@ describe('dataSourcesReducer', () => { reducerTester() .givenReducer(dataSourcesReducer, initialState) .whenActionIsDispatched(dataSourcesLoaded(dataSources)) - .thenStateShouldEqual({ ...initialState, hasFetched: true, dataSources, dataSourcesCount: 1 }); + .thenStateShouldEqual({ ...initialState, isLoadingDataSources: false, dataSources, dataSourcesCount: 1 }); }); }); @@ -89,19 +89,19 @@ describe('dataSourcesReducer', () => { reducerTester() .givenReducer(dataSourcesReducer, state) .whenActionIsDispatched(dataSourcePluginsLoad()) - .thenStateShouldEqual({ ...initialState, isLoadingDataSources: true }); + .thenStateShouldEqual({ ...initialState, isLoadingDataSourcePlugins: true }); }); }); describe('when dataSourcePluginsLoaded is dispatched', () => { it('then state should be correct', () => { const dataSourceTypes = [mockPlugin()]; - const state: DataSourcesState = { ...initialState, isLoadingDataSources: true }; + const state: DataSourcesState = { ...initialState, isLoadingDataSourcePlugins: true }; reducerTester() .givenReducer(dataSourcesReducer, state) .whenActionIsDispatched(dataSourcePluginsLoaded({ plugins: dataSourceTypes, categories: [] })) - .thenStateShouldEqual({ ...initialState, plugins: dataSourceTypes, isLoadingDataSources: false }); + .thenStateShouldEqual({ ...initialState, plugins: dataSourceTypes, isLoadingDataSourcePlugins: false }); }); }); diff --git a/public/app/features/datasources/state/reducers.ts b/public/app/features/datasources/state/reducers.ts index 3a9921d1af3..0a06664f556 100644 --- a/public/app/features/datasources/state/reducers.ts +++ b/public/app/features/datasources/state/reducers.ts @@ -17,13 +17,14 @@ export const initialState: DataSourcesState = { searchQuery: '', dataSourcesCount: 0, dataSourceTypeSearchQuery: '', - hasFetched: false, isLoadingDataSources: false, + isLoadingDataSourcePlugins: false, dataSourceMeta: {} as DataSourcePluginMeta, isSortAscending: true, }; export const dataSourceLoaded = createAction('dataSources/dataSourceLoaded'); +export const dataSourcesLoad = createAction('dataSources/dataSourcesLoad'); export const dataSourcesLoaded = createAction('dataSources/dataSourcesLoaded'); export const dataSourceMetaLoaded = createAction('dataSources/dataSourceMetaLoaded'); export const dataSourcePluginsLoad = createAction('dataSources/dataSourcePluginsLoad'); @@ -43,10 +44,14 @@ export const setIsSortAscending = createAction('dataSources/setIsSortAs // the frozen state. // https://github.com/reduxjs/redux-toolkit/issues/242 export const dataSourcesReducer = (state: DataSourcesState = initialState, action: AnyAction): DataSourcesState => { + if (dataSourcesLoad.match(action)) { + return { ...state, isLoadingDataSources: true }; + } + if (dataSourcesLoaded.match(action)) { return { ...state, - hasFetched: true, + isLoadingDataSources: false, dataSources: action.payload, dataSourcesCount: action.payload.length, }; @@ -65,7 +70,7 @@ export const dataSourcesReducer = (state: DataSourcesState = initialState, actio } if (dataSourcePluginsLoad.match(action)) { - return { ...state, plugins: [], isLoadingDataSources: true }; + return { ...state, plugins: [], isLoadingDataSourcePlugins: true }; } if (dataSourcePluginsLoaded.match(action)) { @@ -73,7 +78,7 @@ export const dataSourcesReducer = (state: DataSourcesState = initialState, actio ...state, plugins: action.payload.plugins, categories: action.payload.categories, - isLoadingDataSources: false, + isLoadingDataSourcePlugins: false, }; } diff --git a/public/app/features/explore/EmptyStateWrapper.tsx b/public/app/features/explore/EmptyStateWrapper.tsx index e0c10d08e65..0ab42ea2ad8 100644 --- a/public/app/features/explore/EmptyStateWrapper.tsx +++ b/public/app/features/explore/EmptyStateWrapper.tsx @@ -1,26 +1,19 @@ import React, { useState } from 'react'; -import { useEffectOnce } from 'react-use'; import { config } from '@grafana/runtime'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; import { EmptyStateNoDatasource } from 'app/features/datasources/components/EmptyStateNoDatasource'; -import { ExploreQueryParams, useDispatch, useSelector } from 'app/types'; +import { ExploreQueryParams, useSelector } from 'app/types'; -import { loadDataSources } from '../datasources/state'; +import { useLoadDataSources } from '../datasources/state'; import { ExplorePage } from './ExplorePage'; export default function EmptyStateWrapper(props: GrafanaRouteComponentProps<{}, ExploreQueryParams>) { - const dispatch = useDispatch(); - useEffectOnce(() => { - if (config.featureToggles.datasourceOnboarding) { - dispatch(loadDataSources()); - } - }); + const { isLoading } = useLoadDataSources(); - const { hasDatasource, loading } = useSelector((state) => ({ + const { hasDatasource } = useSelector((state) => ({ hasDatasource: state.dataSources.dataSourcesCount > 0, - loading: !state.dataSources.hasFetched, })); const [showOnboarding, setShowOnboarding] = useState(config.featureToggles.datasourceOnboarding); const showExplorePage = hasDatasource || !showOnboarding; @@ -30,7 +23,7 @@ export default function EmptyStateWrapper(props: GrafanaRouteComponentProps<{}, ) : ( setShowOnboarding(false)} - loading={loading} + loading={isLoading} title="Welcome to Grafana Explore!" CTAText="Or explore sample data" navId="explore" diff --git a/public/app/types/datasources.ts b/public/app/types/datasources.ts index 585185c221e..62cac8b34ac 100644 --- a/public/app/types/datasources.ts +++ b/public/app/types/datasources.ts @@ -10,8 +10,8 @@ export interface DataSourcesState { dataSourcesCount: number; dataSource: DataSourceSettings; dataSourceMeta: DataSourcePluginMeta; - hasFetched: boolean; isLoadingDataSources: boolean; + isLoadingDataSourcePlugins: boolean; plugins: DataSourcePluginMeta[]; categories: DataSourcePluginCategory[]; isSortAscending: boolean;