refactored how testing state was handled, using redux for this felt way to require way to much code

public/app/features/datasources/settings/DataSourceSettings.test.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 import { DataSourceSettings, Props } from './DataSourceSettings';
-import { DataSource, DataSourceTest, NavModel } from '../../../types';
+import { DataSource, NavModel } from '../../../types';
 import { getMockDataSource } from '../__mocks__/dataSourcesMocks';
 import { getMockPlugin } from '../../plugins/__mocks__/pluginMocks';
 
@@ -15,8 +15,6 @@ const setup = (propOverrides?: object) => {
     loadDataSource: jest.fn(),
     setDataSourceName: jest.fn(),
     updateDataSource: jest.fn(),
-    testing: {} as DataSourceTest,
-    clearTesting: jest.fn(),
   };
 
   Object.assign(props, propOverrides);

public/app/features/datasources/settings/DataSourceSettings.tsx

@@ -1,33 +1,40 @@
 import React, { PureComponent } from 'react';
 import { hot } from 'react-hot-loader';
 import { connect } from 'react-redux';
-import { DataSource, DataSourceTest, NavModel, Plugin } from 'app/types/';
-import PageHeader from '../../../core/components/PageHeader/PageHeader';
-import PageLoader from '../../../core/components/PageLoader/PageLoader';
+
+import PageHeader from 'app/core/components/PageHeader/PageHeader';
+import PageLoader from 'app/core/components/PageLoader/PageLoader';
 import PluginSettings from './PluginSettings';
 import BasicSettings from './BasicSettings';
 import ButtonRow from './ButtonRow';
-import appEvents from '../../../core/app_events';
-import { clearTesting, deleteDataSource, loadDataSource, setDataSourceName, updateDataSource } from '../state/actions';
-import { getNavModel } from '../../../core/selectors/navModel';
-import { getRouteParamsId } from '../../../core/selectors/location';
+
+import appEvents from 'app/core/app_events';
+import { getBackendSrv } from 'app/core/services/backend_srv';
+import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
+
 import { getDataSource, getDataSourceMeta } from '../state/selectors';
+import { deleteDataSource, loadDataSource, setDataSourceName, updateDataSource } from '../state/actions';
+import { getNavModel } from 'app/core/selectors/navModel';
+import { getRouteParamsId } from 'app/core/selectors/location';
+
+import { DataSource, NavModel, Plugin } from 'app/types/';
 
 export interface Props {
   navModel: NavModel;
   dataSource: DataSource;
   dataSourceMeta: Plugin;
   pageId: number;
-  testing: DataSourceTest;
   deleteDataSource: typeof deleteDataSource;
   loadDataSource: typeof loadDataSource;
   setDataSourceName: typeof setDataSourceName;
   updateDataSource: typeof updateDataSource;
-  clearTesting: typeof clearTesting;
 }
+
 interface State {
   dataSource: DataSource;
-  hasClosedTest: boolean;
+  isTesting?: boolean;
+  testingMessage?: string;
+  testingStatus?: string;
 }
 
 enum DataSourceStates {
@@ -36,10 +43,13 @@ enum DataSourceStates {
 }
 
 export class DataSourceSettings extends PureComponent<Props, State> {
-  state = {
-    dataSource: {} as DataSource,
-    hasClosedTest: false,
-  };
+  constructor(props) {
+    super(props);
+
+    this.state = {
+      dataSource: {} as DataSource,
+    };
+  }
 
   async componentDidMount() {
     const { loadDataSource, pageId } = this.props;
@@ -47,27 +57,12 @@ export class DataSourceSettings extends PureComponent<Props, State> {
     await loadDataSource(pageId);
   }
 
-  componentDidUpdate(prevProps) {
-    const { clearTesting } = this.props;
-
-    if (!this.state.hasClosedTest && prevProps.testing.status === 'success') {
-      this.setState({ hasClosedTest: true });
-
-      setTimeout(() => {
-        clearTesting();
-        this.setState({ hasClosedTest: false });
-      }, 3000);
-    }
-  }
-
-  componentWillUnmount() {
-    this.props.clearTesting();
-  }
-
-  onSubmit = event => {
+  onSubmit = async event => {
     event.preventDefault();
 
-    this.props.updateDataSource({ ...this.state.dataSource, name: this.props.dataSource.name });
+    await this.props.updateDataSource({ ...this.state.dataSource, name: this.props.dataSource.name });
+
+    this.testDataSource();
   };
 
   onDelete = () => {
@@ -131,8 +126,45 @@ export class DataSourceSettings extends PureComponent<Props, State> {
     );
   }
 
+  async testDataSource() {
+    const dsApi = await getDatasourceSrv().get(this.state.dataSource.name);
+
+    if (!dsApi.testDatasource) {
+      return;
+    }
+
+    this.setState({ isTesting: true, testingMessage: 'Testing...', testingStatus: 'info' });
+
+    getBackendSrv().withNoBackendCache(async () => {
+      try {
+        const result = await dsApi.testDatasource();
+
+        this.setState({
+          isTesting: false,
+          testingStatus: result.status,
+          testingMessage: result.message,
+        });
+      } catch (err) {
+        let message = '';
+
+        if (err.statusText) {
+          message = 'HTTP Error ' + err.statusText;
+        } else {
+          message = err.message;
+        }
+
+        this.setState({
+          isTesting: false,
+          testingStatus: 'error',
+          testingMessage: message,
+        });
+      }
+    });
+  }
+
   render() {
-    const { dataSource, dataSourceMeta, navModel, testing } = this.props;
+    const { dataSource, dataSourceMeta, navModel } = this.props;
+    const { testingMessage, testingStatus } = this.state;
 
     return (
       <div>
@@ -160,26 +192,20 @@ export class DataSourceSettings extends PureComponent<Props, State> {
                 )}
 
                 <div className="gf-form-group section">
-                  {testing.inProgress && (
-                    <h5>
-                      Testing.... <i className="fa fa-spiner fa-spin" />
-                    </h5>
-                  )}
-                  {!testing.inProgress &&
-                    testing.status && (
-                      <div className={`alert-${testing.status} alert`}>
-                        <div className="alert-icon">
-                          {testing.status === 'error' ? (
-                            <i className="fa fa-exclamation-triangle" />
-                          ) : (
-                            <i className="fa fa-check" />
-                          )}
-                        </div>
-                        <div className="alert-body">
-                          <div className="alert-title">{testing.message}</div>
-                        </div>
+                  {testingMessage && (
+                    <div className={`alert-${testingStatus} alert`}>
+                      <div className="alert-icon">
+                        {testingStatus === 'error' ? (
+                          <i className="fa fa-exclamation-triangle" />
+                        ) : (
+                          <i className="fa fa-check" />
+                        )}
+                      </div>
+                      <div className="alert-body">
+                        <div className="alert-title">{testingMessage}</div>
                       </div>
-                    )}
+                    </div>
+                  )}
                 </div>
 
                 <ButtonRow
@@ -205,7 +231,6 @@ function mapStateToProps(state) {
     dataSource: getDataSource(state.dataSources, pageId),
     dataSourceMeta: getDataSourceMeta(state.dataSources, dataSource.type),
     pageId: pageId,
-    testing: state.dataSources.testing,
   };
 }
 
@@ -214,7 +239,6 @@ const mapDispatchToProps = {
   loadDataSource,
   setDataSourceName,
   updateDataSource,
-  clearTesting,
 };
 
 export default hot(module)(connect(mapStateToProps, mapDispatchToProps)(DataSourceSettings));

public/app/features/datasources/state/actions.ts

@@ -1,13 +1,12 @@
 import { ThunkAction } from 'redux-thunk';
-import { DataSource, Plugin, StoreState } from 'app/types';
-import { getBackendSrv } from '../../../core/services/backend_srv';
-import { getDatasourceSrv } from '../../plugins/datasource_srv';
-import { LayoutMode } from '../../../core/components/LayoutSelector/LayoutSelector';
-import { updateLocation, updateNavIndex, UpdateNavIndexAction } from '../../../core/actions';
-import { UpdateLocationAction } from '../../../core/actions/location';
-import { buildNavModel } from './navModel';
-
 import config from '../../../core/config';
+import { getBackendSrv } from 'app/core/services/backend_srv';
+import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
+import { LayoutMode } from 'app/core/components/LayoutSelector/LayoutSelector';
+import { updateLocation, updateNavIndex, UpdateNavIndexAction } from 'app/core/actions';
+import { UpdateLocationAction } from 'app/core/actions/location';
+import { buildNavModel } from './navModel';
+import { DataSource, Plugin, StoreState } from 'app/types';
 
 export enum ActionTypes {
   LoadDataSources = 'LOAD_DATA_SOURCES',
@@ -18,10 +17,6 @@ export enum ActionTypes {
   SetDataSourcesLayoutMode = 'SET_DATA_SOURCES_LAYOUT_MODE',
   SetDataSourceTypeSearchQuery = 'SET_DATA_SOURCE_TYPE_SEARCH_QUERY',
   SetDataSourceName = 'SET_DATA_SOURCE_NAME',
-  SetDataSourceTestingProgess = 'SET_TESTING_PROGRESS',
-  SetDataSourceTestingSuccess = 'SET_DATA_SOURCE_TESTING_SUCCESS',
-  SetDataSourceTestingFail = 'SET_DATA_SOURCE_TESTING_FAIL',
-  ClearTesting = 'CLEAR_TEST',
 }
 
 interface LoadDataSourcesAction {
@@ -64,25 +59,6 @@ interface SetDataSourceNameAction {
   payload: string;
 }
 
-interface SetDataSourceTestingProgessAction {
-  type: ActionTypes.SetDataSourceTestingProgess;
-  payload: boolean;
-}
-
-interface SetDataSourceTestingSuccessAction {
-  type: ActionTypes.SetDataSourceTestingSuccess;
-  payload: { status: string; message: string };
-}
-
-interface SetDataSourceTestingFailAction {
-  type: ActionTypes.SetDataSourceTestingFail;
-  payload: string;
-}
-
-interface ClearTestingAction {
-  type: ActionTypes.ClearTesting;
-}
-
 const dataSourcesLoaded = (dataSources: DataSource[]): LoadDataSourcesAction => ({
   type: ActionTypes.LoadDataSources,
   payload: dataSources,
@@ -123,28 +99,6 @@ export const setDataSourceName = (name: string) => ({
   payload: name,
 });
 
-export const clearTesting = (): ClearTestingAction => ({
-  type: ActionTypes.ClearTesting,
-});
-
-const setDataSourceTestingProgress = (state: boolean): SetDataSourceTestingProgessAction => ({
-  type: ActionTypes.SetDataSourceTestingProgess,
-  payload: state,
-});
-
-const setDataSourceTestingSuccess = (status: string, message: string): SetDataSourceTestingSuccessAction => ({
-  type: ActionTypes.SetDataSourceTestingSuccess,
-  payload: {
-    status: status,
-    message: message,
-  },
-});
-
-const setDataSourceTestingFail = (message: string): SetDataSourceTestingFailAction => ({
-  type: ActionTypes.SetDataSourceTestingFail,
-  payload: message,
-});
-
 export type Action =
   | LoadDataSourcesAction
   | SetDataSourcesSearchQueryAction
@@ -155,11 +109,7 @@ export type Action =
   | LoadDataSourceAction
   | UpdateNavIndexAction
   | LoadDataSourceMetaAction
-  | SetDataSourceNameAction
-  | SetDataSourceTestingProgessAction
-  | SetDataSourceTestingSuccessAction
-  | SetDataSourceTestingFailAction
-  | ClearTestingAction;
+  | SetDataSourceNameAction;
 
 type ThunkResult<R> = ThunkAction<R, StoreState, undefined, Action>;
 
@@ -211,15 +161,9 @@ export function loadDataSourceTypes(): ThunkResult<void> {
 
 export function updateDataSource(dataSource: DataSource): ThunkResult<void> {
   return async dispatch => {
-    await getBackendSrv()
-      .put(`/api/datasources/${dataSource.id}`, dataSource)
-      .then(response => {
-        updateFrontendSettings().then(() => {
-          testDataSource(dispatch, response.name);
-        });
-      });
-
-    dispatch(loadDataSource(dataSource.id));
+    await getBackendSrv().put(`/api/datasources/${dataSource.id}`, dataSource);
+    await updateFrontendSettings();
+    return dispatch(loadDataSource(dataSource.id));
   };
 }
 
@@ -270,40 +214,6 @@ function updateFrontendSettings() {
     });
 }
 
-function testDataSource(dispatch, name) {
-  dispatch(setDataSourceTestingProgress(true));
-  getDatasourceSrv()
-    .get(name)
-    .then(dataSource => {
-      if (!dataSource.testDatasource) {
-        return;
-      }
-
-      // make test call in no backend cache context
-      getBackendSrv()
-        .withNoBackendCache(() => {
-          return dataSource
-            .testDatasource()
-            .then(result => {
-              dispatch(setDataSourceTestingSuccess(result.status, result.message));
-            })
-            .catch(err => {
-              let message = '';
-
-              if (err.statusText) {
-                message = 'HTTP Error ' + err.statusText;
-              } else {
-                message = err.message;
-              }
-              dispatch(setDataSourceTestingFail(message));
-            });
-        })
-        .finally(() => {
-          dispatch(setDataSourceTestingProgress(false));
-        });
-    });
-}
-
 function nameHasSuffix(name) {
   return name.endsWith('-', name.length - 1);
 }

public/app/features/datasources/state/reducers.ts

@@ -12,7 +12,6 @@ const initialState: DataSourcesState = {
   dataSourceTypeSearchQuery: '',
   hasFetched: false,
   dataSourceMeta: {} as Plugin,
-  testing: { inProgress: false, status: '', message: '' },
 };
 
 export const dataSourcesReducer = (state = initialState, action: Action): DataSourcesState => {
@@ -40,28 +39,6 @@ export const dataSourcesReducer = (state = initialState, action: Action): DataSo
 
     case ActionTypes.SetDataSourceName:
       return { ...state, dataSource: { ...state.dataSource, name: action.payload } };
-
-    case ActionTypes.SetDataSourceTestingProgess:
-      return { ...state, testing: { ...state.testing, inProgress: action.payload } };
-
-    case ActionTypes.SetDataSourceTestingSuccess:
-      return {
-        ...state,
-        testing: {
-          status: action.payload.status,
-          message: action.payload.message,
-          inProgress: false,
-        },
-      };
-
-    case ActionTypes.SetDataSourceTestingFail:
-      return {
-        ...state,
-        testing: { status: 'error', message: action.payload, inProgress: false },
-      };
-
-    case ActionTypes.ClearTesting:
-      return { ...state, testing: { inProgress: false, status: '', message: '' } };
   }
 
   return state;

public/app/plugins/datasource/cloudwatch/datasource.ts

@@ -362,14 +362,9 @@ export default class CloudWatchDatasource {
     const metricName = 'EstimatedCharges';
     const dimensions = {};
 
-    return this.getDimensionValues(region, namespace, metricName, 'ServiceName', dimensions).then(
-      () => {
-        return { status: 'success', message: 'Data source is working' };
-      },
-      err => {
-        return { status: 'error', message: err.message };
-      }
-    );
+    return this.getDimensionValues(region, namespace, metricName, 'ServiceName', dimensions).then(() => {
+      return { status: 'success', message: 'Data source is working' };
+    });
   }
 
   awsRequest(url, data) {

public/app/types/datasources.ts

@@ -25,12 +25,6 @@ export interface DataSource {
   testDatasource?: () => Promise<any>;
 }
 
-export interface DataSourceTest {
-  inProgress: boolean;
-  message: string;
-  status: string;
-}
-
 export interface DataSourcesState {
   dataSources: DataSource[];
   searchQuery: string;
@@ -41,5 +35,4 @@ export interface DataSourcesState {
   dataSource: DataSource;
   dataSourceMeta: Plugin;
   hasFetched: boolean;
-  testing: DataSourceTest;
 }

public/app/types/index.ts

@@ -7,7 +7,7 @@ import { DashboardState } from './dashboard';
 import { DashboardAcl, OrgRole, PermissionLevel } from './acl';
 import { ApiKey, ApiKeysState, NewApiKey } from './apiKeys';
 import { Invitee, OrgUser, User, UsersState, UserState } from './user';
-import { DataSource, DataSourceTest, DataSourcesState } from './datasources';
+import { DataSource, DataSourcesState } from './datasources';
 import {
   TimeRange,
   LoadingState,
@@ -88,7 +88,6 @@ export {
   AppNotificationTimeout,
   DashboardSearchHit,
   UserState,
-  DataSourceTest,
 };
 
 export interface StoreState {