From c4c031ef4368a84fb35412d8729e78db4ede23cd Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Thu, 5 Dec 2019 11:22:46 +0100 Subject: [PATCH] Explore: Cleanup redundant state variables and unused actions (#20837) --- public/app/features/explore/Explore.tsx | 18 ++++------ .../app/features/explore/ExploreToolbar.tsx | 31 ++++++++-------- .../app/features/explore/state/actionTypes.ts | 21 ----------- .../features/explore/state/actions.test.ts | 2 +- public/app/features/explore/state/actions.ts | 35 +++++++++++-------- .../features/explore/state/reducers.test.ts | 4 --- public/app/features/explore/state/reducers.ts | 35 ------------------- .../app/features/explore/state/selectors.ts | 15 ++++++++ public/app/types/explore.ts | 32 +++++++---------- 9 files changed, 69 insertions(+), 124 deletions(-) diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 31903c458a0..6a6a2f81b4e 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -1,5 +1,5 @@ // Libraries -import React, { ComponentType } from 'react'; +import React from 'react'; import { hot } from 'react-hot-loader'; import { css } from 'emotion'; import { connect } from 'react-redux'; @@ -27,13 +27,13 @@ import { // Types import { DataQuery, - ExploreStartPageProps, DataSourceApi, PanelData, RawTimeRange, GraphSeriesXY, TimeZone, AbsoluteTimeRange, + LoadingState, } from '@grafana/data'; import { @@ -71,7 +71,6 @@ const getStyles = memoizeOne(() => { }); interface ExploreProps { - StartPage?: ComponentType; changeSize: typeof changeSize; datasourceInstance: DataSourceApi; datasourceMissing: boolean; @@ -87,7 +86,6 @@ interface ExploreProps { scanStopAction: typeof scanStopAction; setQueries: typeof setQueries; split: boolean; - showingStartPage?: boolean; queryKeys: string[]; initialDatasource: string; initialQueries: DataQuery[]; @@ -251,11 +249,9 @@ export class Explore extends React.PureComponent { render() { const { - StartPage, datasourceInstance, datasourceMissing, exploreId, - showingStartPage, split, queryKeys, mode, @@ -270,6 +266,8 @@ export class Explore extends React.PureComponent { } = this.props; const exploreClass = split ? 'explore explore-split' : 'explore'; const styles = getStyles(); + const StartPage = datasourceInstance?.components?.ExploreStartPage; + const showStartPage = !queryResponse || queryResponse.state === LoadingState.NotStarted; return (
@@ -288,7 +286,7 @@ export class Explore extends React.PureComponent { return (
- {showingStartPage && StartPage && ( + {showStartPage && StartPage && (
{ />
)} - {!showingStartPage && ( + {!showStartPage && ( <> {mode === ExploreMode.Metrics && ( { return { @@ -50,11 +51,9 @@ interface OwnProps { interface StateProps { datasourceMissing: boolean; - exploreDatasources: DataSourceSelectItem[]; loading: boolean; range: TimeRange; timeZone: TimeZone; - selectedDatasource?: DataSourceSelectItem; splitted: boolean; syncedTimes: boolean; refreshInterval?: string; @@ -67,6 +66,7 @@ interface StateProps { queries: DataQuery[]; datasourceLoading?: boolean; containerWidth: number; + datasourceName?: string; } interface DispatchProps { @@ -137,16 +137,20 @@ export class UnConnectedExploreToolbar extends PureComponent { }); }; + getSelectedDatasource = () => { + const { datasourceName } = this.props; + const exploreDatasources = getExploreDatasources(); + return datasourceName ? exploreDatasources.find(datasource => datasource.name === datasourceName) : undefined; + }; + render() { const { datasourceMissing, - exploreDatasources, closeSplit, exploreId, loading, range, timeZone, - selectedDatasource, splitted, syncedTimes, refreshInterval, @@ -203,8 +207,8 @@ export class UnConnectedExploreToolbar extends PureComponent { > @@ -331,7 +335,6 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps const { datasourceInstance, datasourceMissing, - exploreDatasources, range, refreshInterval, loading, @@ -344,21 +347,15 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps datasourceLoading, containerWidth, } = exploreItem; - const selectedDatasource = datasourceInstance - ? exploreDatasources.find(datasource => datasource.name === datasourceInstance.name) - : undefined; - const hasLiveOption = - datasourceInstance && datasourceInstance.meta && datasourceInstance.meta.streaming && mode === ExploreMode.Logs - ? true - : false; + + const hasLiveOption = datasourceInstance?.meta?.streaming && mode === ExploreMode.Logs; return { datasourceMissing, - exploreDatasources, + datasourceName: datasourceInstance?.name, loading, range, timeZone: getTimeZone(state.user), - selectedDatasource, splitted, refreshInterval, supportedModes, diff --git a/public/app/features/explore/state/actionTypes.ts b/public/app/features/explore/state/actionTypes.ts index c361262f793..ac48b619f88 100644 --- a/public/app/features/explore/state/actionTypes.ts +++ b/public/app/features/explore/state/actionTypes.ts @@ -4,7 +4,6 @@ import { Emitter } from 'app/core/core'; import { DataQuery, - DataSourceSelectItem, DataSourceApi, QueryFixAction, PanelData, @@ -81,10 +80,6 @@ export interface ClearOriginPayload { exploreId: ExploreId; } -export interface ClearRefreshIntervalPayload { - exploreId: ExploreId; -} - export interface HighlightLogsExpressionPayload { exploreId: ExploreId; expressions: string[]; @@ -122,10 +117,6 @@ export interface ModifyQueriesPayload { modifier: (query: DataQuery, modification: QueryFixAction) => DataQuery; } -export interface QueryStartPayload { - exploreId: ExploreId; -} - export interface QueryEndedPayload { exploreId: ExploreId; response: PanelData; @@ -199,11 +190,6 @@ export interface QueriesImportedPayload { queries: DataQuery[]; } -export interface LoadExploreDataSourcesPayload { - exploreId: ExploreId; - exploreDatasources: DataSourceSelectItem[]; -} - export interface SetUrlReplacedPayload { exploreId: ExploreId; } @@ -312,10 +298,6 @@ export const loadDatasourceReadyAction = actionCreatorFactory('explore/MODIFY_QUERIES').create(); -export const queryStartAction = actionCreatorFactory('explore/QUERY_START').create(); - -export const queryEndedAction = actionCreatorFactory('explore/QUERY_ENDED').create(); - export const queryStreamUpdatedAction = actionCreatorFactory( 'explore/QUERY_STREAM_UPDATED' ).create(); @@ -389,9 +371,6 @@ export const toggleLogLevelAction = actionCreatorFactory( */ export const resetExploreAction = actionCreatorFactory('explore/RESET_EXPLORE').create(); export const queriesImportedAction = actionCreatorFactory('explore/QueriesImported').create(); -export const loadExploreDatasources = actionCreatorFactory( - 'explore/LOAD_EXPLORE_DATASOURCES' -).create(); export const historyUpdatedAction = actionCreatorFactory('explore/HISTORY_UPDATED').create(); diff --git a/public/app/features/explore/state/actions.test.ts b/public/app/features/explore/state/actions.test.ts index ce431ec3e15..b310f0a9e17 100644 --- a/public/app/features/explore/state/actions.test.ts +++ b/public/app/features/explore/state/actions.test.ts @@ -107,7 +107,7 @@ describe('refreshExplore', () => { .givenThunk(refreshExplore) .whenThunkIsDispatched(exploreId); - const initializeExplore = dispatchedActions[2] as ActionOf; + const initializeExplore = dispatchedActions[1] as ActionOf; const { type, payload } = initializeExplore; expect(type).toEqual(initializeExploreAction.type); diff --git a/public/app/features/explore/state/actions.ts b/public/app/features/explore/state/actions.ts index 0d760b7175c..a42a65a29ca 100644 --- a/public/app/features/explore/state/actions.ts +++ b/public/app/features/explore/state/actions.ts @@ -29,7 +29,6 @@ import { AbsoluteTimeRange, DataQuery, DataSourceApi, - DataSourceSelectItem, dateTimeForTimeZone, isDateTime, LoadingState, @@ -57,7 +56,6 @@ import { loadDatasourcePendingAction, loadDatasourceReadyAction, LoadDatasourceReadyPayload, - loadExploreDatasources, modifyQueriesAction, queriesImportedAction, queryStoreSubscriptionAction, @@ -84,6 +82,7 @@ import { getTimeSrv, TimeSrv } from '../../dashboard/services/TimeSrv'; import { preProcessPanelData, runRequest } from '../../dashboard/state/runRequest'; import { PanelModel } from 'app/features/dashboard/state'; import { DataSourceSrv } from '@grafana/runtime'; +import { getExploreDatasources } from './selectors'; /** * Updates UI state and save it to the URL @@ -244,18 +243,7 @@ export function loadExploreDatasourcesAndSetDatasource( datasourceName: string ): ThunkResult { return dispatch => { - const exploreDatasources: DataSourceSelectItem[] = getDatasourceSrv() - .getExternal() - .map( - (ds: any) => - ({ - value: ds.name, - name: ds.name, - meta: ds.meta, - } as DataSourceSelectItem) - ); - - dispatch(loadExploreDatasources({ exploreId, exploreDatasources })); + const exploreDatasources = getExploreDatasources(); if (exploreDatasources.length >= 1) { dispatch(changeDatasource(exploreId, datasourceName)); @@ -320,6 +308,14 @@ export const loadDatasourceReady = ( }); }; +/** + * Import queries from previous datasource if possible eg Loki and Prometheus have similar query language so the + * labels part can be reused to get similar data. + * @param exploreId + * @param queries + * @param sourceDataSource + * @param targetDataSource + */ export function importQueries( exploreId: ExploreId, queries: DataQuery[], @@ -464,6 +460,8 @@ export function runQueries(exploreId: ExploreId): ThunkResult { // Side-effect: Saving history in localstorage const nextHistory = updateHistory(history, datasourceId, queries); dispatch(historyUpdatedAction({ exploreId, history: nextHistory })); + + // We save queries to the URL here so that only successfully run queries change the URL. dispatch(stateSave()); } @@ -505,6 +503,10 @@ const toRawTimeRange = (range: TimeRange): RawTimeRange => { }; }; +/** + * Save local redux state back to the URL. Should be called when there is some change that should affect the URL. + * Not all of the redux state is reflected in URL though. + */ export const stateSave = (): ThunkResult => { return (dispatch, getState) => { const { left, right, split } = getState().explore; @@ -714,6 +716,11 @@ export const changeDedupStrategy = (exploreId: ExploreId, dedupStrategy: LogsDed }; }; +/** + * Reacts to changes in URL state that we need to sync back to our redux state. Checks the internal update variable + * to see which parts change and need to be synced. + * @param exploreId + */ export function refreshExplore(exploreId: ExploreId): ThunkResult { return (dispatch, getState) => { const itemState = getState().explore[exploreId]; diff --git a/public/app/features/explore/state/reducers.test.ts b/public/app/features/explore/state/reducers.test.ts index 0f8f9e58f46..6e5d33767e7 100644 --- a/public/app/features/explore/state/reducers.test.ts +++ b/public/app/features/explore/state/reducers.test.ts @@ -90,15 +90,11 @@ describe('Explore item reducer', () => { const queryKeys: string[] = []; const initalState: Partial = { datasourceInstance: null, - StartPage: null, - showingStartPage: false, queries, queryKeys, }; const expectedState: any = { datasourceInstance, - StartPage, - showingStartPage: true, queries, queryKeys, graphResult: null, diff --git a/public/app/features/explore/state/reducers.ts b/public/app/features/explore/state/reducers.ts index 11f3eaa4e77..f478f368062 100644 --- a/public/app/features/explore/state/reducers.ts +++ b/public/app/features/explore/state/reducers.ts @@ -27,12 +27,10 @@ import { ActionTypes, splitCloseAction, SplitCloseActionPayload, - loadExploreDatasources, historyUpdatedAction, changeModeAction, setUrlReplacedAction, scanStopAction, - queryStartAction, changeRangeAction, clearOriginAction, addQueryRowAction, @@ -84,13 +82,11 @@ export const makeInitialUpdateState = (): ExploreUpdateState => ({ * Returns a fresh Explore area state */ export const makeExploreItemState = (): ExploreItemState => ({ - StartPage: undefined, containerWidth: 0, datasourceInstance: null, requestedDatasourceName: null, datasourceLoading: null, datasourceMissing: false, - exploreDatasources: [], history: [], queries: [], initialized: false, @@ -229,7 +225,6 @@ export const itemReducer = reducerFactory({} as ExploreItemSta graphResult: null, tableResult: null, logsResult: null, - showingStartPage: Boolean(state.StartPage), queryKeys: getQueryKeys(queries, state.datasourceInstance), queryResponse: createEmptyQueryResponse(), loading: false, @@ -277,7 +272,6 @@ export const itemReducer = reducerFactory({} as ExploreItemSta const { datasourceInstance, version } = action.payload; // Custom components - const StartPage = datasourceInstance.components.ExploreStartPage; stopQueryState(state.querySubscription); let newMetadata = datasourceInstance.meta; @@ -308,8 +302,6 @@ export const itemReducer = reducerFactory({} as ExploreItemSta latency: 0, queryResponse: createEmptyQueryResponse(), loading: false, - StartPage: datasourceInstance.components.ExploreStartPage, - showingStartPage: Boolean(StartPage), queryKeys: [], supportedModes, mode, @@ -382,22 +374,6 @@ export const itemReducer = reducerFactory({} as ExploreItemSta }; }, }) - .addMapper({ - filter: queryStartAction, - mapper: (state): ExploreItemState => { - return { - ...state, - latency: 0, - queryResponse: { - ...state.queryResponse, - state: LoadingState.Loading, - error: null, - }, - loading: true, - update: makeInitialUpdateState(), - }; - }, - }) .addMapper({ filter: removeQueryRowAction, mapper: (state, action): ExploreItemState => { @@ -496,15 +472,6 @@ export const itemReducer = reducerFactory({} as ExploreItemSta }; }, }) - .addMapper({ - filter: loadExploreDatasources, - mapper: (state, action): ExploreItemState => { - return { - ...state, - exploreDatasources: action.payload.exploreDatasources, - }; - }, - }) .addMapper({ filter: historyUpdatedAction, mapper: (state, action): ExploreItemState => { @@ -599,7 +566,6 @@ export const processQueryResponse = ( graphResult: null, tableResult: null, logsResult: null, - showingStartPage: false, update: makeInitialUpdateState(), }; } @@ -625,7 +591,6 @@ export const processQueryResponse = ( tableResult, logsResult, loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming, - showingStartPage: false, update: makeInitialUpdateState(), }; }; diff --git a/public/app/features/explore/state/selectors.ts b/public/app/features/explore/state/selectors.ts index 32e097b4f62..f14c6ad7e7d 100644 --- a/public/app/features/explore/state/selectors.ts +++ b/public/app/features/explore/state/selectors.ts @@ -1,6 +1,8 @@ import { createSelector } from 'reselect'; import { ExploreItemState } from 'app/types'; import { filterLogLevels, dedupLogRows } from 'app/core/logs_model'; +import { getDatasourceSrv } from '../../plugins/datasource_srv'; +import { DataSourceSelectItem } from '@grafana/data'; const logsRowsSelector = (state: ExploreItemState) => state.logsResult && state.logsResult.rows; const hiddenLogLevelsSelector = (state: ExploreItemState) => state.hiddenLogLevels; @@ -17,3 +19,16 @@ export const deduplicatedRowsSelector = createSelector( return dedupLogRows(filteredRows, dedupStrategy); } ); + +export const getExploreDatasources = (): DataSourceSelectItem[] => { + return getDatasourceSrv() + .getExternal() + .map( + (ds: any) => + ({ + value: ds.name, + name: ds.name, + meta: ds.meta, + } as DataSourceSelectItem) + ); +}; diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index df67ab83b53..4bd117a8014 100644 --- a/public/app/types/explore.ts +++ b/public/app/types/explore.ts @@ -1,12 +1,9 @@ import { Unsubscribable } from 'rxjs'; -import { ComponentType } from 'react'; import { HistoryItem, DataQuery, - DataSourceSelectItem, DataSourceApi, QueryHint, - ExploreStartPageProps, PanelData, DataQueryRequest, RawTimeRange, @@ -54,10 +51,6 @@ export interface ExploreState { } export interface ExploreItemState { - /** - * React component to be shown when no queries have been run yet, e.g., for a query language cheat sheet. - */ - StartPage?: ComponentType; /** * Width used for calculating the graph interval (can't have more datapoints than pixels) */ @@ -82,10 +75,6 @@ export interface ExploreItemState { * Emitter to send events to the rest of Grafana. */ eventBridge?: Emitter; - /** - * List of datasources to be shown in the datasource selector. - */ - exploreDatasources: DataSourceSelectItem[]; /** * List of timeseries to be shown in the Explore graph result viewer. */ @@ -132,10 +121,6 @@ export interface ExploreItemState { * True if graph result viewer is expanded. Query runs will contain graph queries. */ showingGraph: boolean; - /** - * True StartPage needs to be shown. Typically set to `false` once queries have been run. - */ - showingStartPage?: boolean; /** * True if table result viewer is expanded. Query runs will contain table queries. */ @@ -167,8 +152,15 @@ export interface ExploreItemState { */ refreshInterval?: string; + /** + * Copy of the state of the URL which is in store.location.query. This is duplicated here so we can diff the two + * after a change to see if we need to sync url state back to redux store (like on clicking Back in browser). + */ urlState: ExploreUrlState; + /** + * Map of what changed between real url and local urlState so we can partially update just the things that are needed. + */ update: ExploreUpdateState; latency: number; @@ -189,6 +181,11 @@ export interface ExploreItemState { querySubscription?: Unsubscribable; queryResponse: PanelData; + + /** + * Panel Id that is set if we come to explore from a penel. Used so we can get back to it and optionally modify the + * query of that panel. + */ originPanelId?: number; } @@ -217,11 +214,6 @@ export interface ExploreUrlState { context?: string; } -export interface QueryIntervals { - interval: string; - intervalMs: number; -} - export interface QueryOptions { minInterval: string; maxDataPoints?: number;