diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index f9d9301d873..93ced013539 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -14,6 +14,7 @@ export const Pages = { DataSource: { name: 'Data source settings page name input field', delete: 'Data source settings page Delete button', + readOnly: 'Data source settings page read only message', saveAndTest: 'Data source settings page Save and Test button', alert: 'Data source settings page Alert', }, diff --git a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts index afd5c12ee2f..60a8d6fe27e 100644 --- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts +++ b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts @@ -15,6 +15,16 @@ import { BackendDataSourceResponse, toDataQueryResponse } from './queryResponse' const ExpressionDatasourceID = '__expr__'; +class HealthCheckError extends Error { + details: HealthCheckResultDetails; + + constructor(message: string, details: HealthCheckResultDetails) { + super(message); + this.details = details; + this.name = 'HealthCheckError'; + } +} + /** * Describes the current health status of a data source plugin. * @@ -26,6 +36,16 @@ export enum HealthStatus { Error = 'ERROR', } +/** + * Describes the details in the payload returned when checking the health of a data source + * plugin. + * + * If the 'message' key exists, this will be displayed in the error message in DataSourceSettingsPage + * + * @public + */ +export type HealthCheckResultDetails = Record | undefined; + /** * Describes the payload returned when checking the health of a data source * plugin. @@ -35,7 +55,7 @@ export enum HealthStatus { export interface HealthCheckResult { status: HealthStatus; message: string; - details?: Record; + details: HealthCheckResultDetails; } /** @@ -183,7 +203,8 @@ class DataSourceWithBackend< message: res.message, }; } - throw new Error(res.message); + + throw new HealthCheckError(res.message, res.details); }); } } diff --git a/public/app/features/datasources/settings/DataSourceSettingsPage.test.tsx b/public/app/features/datasources/settings/DataSourceSettingsPage.test.tsx index bec5e1aa716..b46fadf7883 100644 --- a/public/app/features/datasources/settings/DataSourceSettingsPage.test.tsx +++ b/public/app/features/datasources/settings/DataSourceSettingsPage.test.tsx @@ -1,85 +1,142 @@ import React from 'react'; -import { shallow } from 'enzyme'; import { DataSourceSettingsPage, Props } from './DataSourceSettingsPage'; -import { DataSourceConstructor, DataSourcePlugin, DataSourceSettings, NavModel } from '@grafana/data'; import { getMockDataSource } from '../__mocks__/dataSourcesMocks'; import { getMockPlugin } from '../../plugins/__mocks__/pluginMocks'; import { dataSourceLoaded, setDataSourceName, setIsDefault } from '../state/reducers'; import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps'; import { cleanUpAction } from 'app/core/actions/cleanUp'; +import { screen, render } from '@testing-library/react'; +import { selectors } from '@grafana/e2e-selectors'; +import { PluginState } from '@grafana/data'; -const pluginMock = new DataSourcePlugin({} as DataSourceConstructor); - -jest.mock('app/features/plugins/plugin_loader', () => { - return { - importDataSourcePlugin: () => Promise.resolve(pluginMock), - }; +const getMockNode = () => ({ + text: 'text', + subTitle: 'subtitle', + icon: 'icon', }); -const setup = (propOverrides?: object) => { - const props: Props = { - ...getRouteComponentProps(), - navModel: {} as NavModel, - dataSource: getMockDataSource(), - dataSourceMeta: getMockPlugin(), - dataSourceId: 1, - deleteDataSource: jest.fn(), - loadDataSource: jest.fn(), - setDataSourceName, - updateDataSource: jest.fn(), - initDataSourceSettings: jest.fn(), - testDataSource: jest.fn(), - setIsDefault, - dataSourceLoaded, - cleanUpAction, - page: null, - plugin: null, - loadError: null, - testingStatus: {}, - ...propOverrides, - }; - - return shallow(); -}; +const getProps = (): Props => ({ + ...getRouteComponentProps(), + navModel: { + node: getMockNode(), + main: getMockNode(), + }, + dataSource: getMockDataSource(), + dataSourceMeta: getMockPlugin(), + dataSourceId: 1, + deleteDataSource: jest.fn(), + loadDataSource: jest.fn(), + setDataSourceName, + updateDataSource: jest.fn(), + initDataSourceSettings: jest.fn(), + testDataSource: jest.fn(), + setIsDefault, + dataSourceLoaded, + cleanUpAction, + page: null, + plugin: null, + loadError: null, + testingStatus: {}, +}); describe('Render', () => { - it('should render component', () => { - const wrapper = setup(); - expect(wrapper).toMatchSnapshot(); + it('should not render loading when props are ready', () => { + render(); + + expect(screen.queryByText('Loading ...')).not.toBeInTheDocument(); }); - it('should render loader', () => { - const wrapper = setup({ - dataSource: {} as DataSourceSettings, - plugin: pluginMock, - }); + it('should render loading if datasource is not ready', () => { + const mockProps = getProps(); + mockProps.dataSource.id = 0; - expect(wrapper).toMatchSnapshot(); + render(); + + expect(screen.getByText('Loading ...')).toBeInTheDocument(); }); - it('should render beta info text', () => { - const wrapper = setup({ - dataSourceMeta: { ...getMockPlugin(), state: 'beta' }, - }); + it('should render beta info text if plugin state is beta', () => { + const mockProps = getProps(); + mockProps.dataSourceMeta.state = PluginState.beta; - expect(wrapper).toMatchSnapshot(); + render(); + + expect( + screen.getByTitle('Beta Plugin: There could be bugs and minor breaking changes to this plugin') + ).toBeInTheDocument(); }); - it('should render alpha info text', () => { - const wrapper = setup({ - dataSourceMeta: { ...getMockPlugin(), state: 'alpha' }, - plugin: pluginMock, - }); + it('should render alpha info text if plugin state is alpha', () => { + const mockProps = getProps(); + mockProps.dataSourceMeta.state = PluginState.alpha; - expect(wrapper).toMatchSnapshot(); + render(); + + expect( + screen.getByTitle('Alpha Plugin: This plugin is a work in progress and updates may include breaking changes') + ).toBeInTheDocument(); }); - it('should render is ready only message', () => { - const wrapper = setup({ - dataSource: { ...getMockDataSource(), readOnly: true }, - plugin: pluginMock, - }); + it('should not render is ready only message is readOnly is false', () => { + const mockProps = getProps(); + mockProps.dataSource.readOnly = false; - expect(wrapper).toMatchSnapshot(); + render(); + + expect(screen.queryByLabelText(selectors.pages.DataSource.readOnly)).not.toBeInTheDocument(); + }); + + it('should render is ready only message is readOnly is true', () => { + const mockProps = getProps(); + mockProps.dataSource.readOnly = true; + + render(); + + expect(screen.getByLabelText(selectors.pages.DataSource.readOnly)).toBeInTheDocument(); + }); + + it('should render error message with detailed message', () => { + const mockProps = { + ...getProps(), + testingStatus: { + message: 'message', + status: 'error', + details: { message: 'detailed message' }, + }, + }; + + render(); + + expect(screen.getByText(mockProps.testingStatus.message)).toBeInTheDocument(); + expect(screen.getByText(mockProps.testingStatus.details.message)).toBeInTheDocument(); + }); + + it('should render error message with empty details', () => { + const mockProps = { + ...getProps(), + testingStatus: { + message: 'message', + status: 'error', + details: {}, + }, + }; + + render(); + + expect(screen.getByText(mockProps.testingStatus.message)).toBeInTheDocument(); + }); + + it('should render error message without details', () => { + const mockProps = { + ...getProps(), + testingStatus: { + message: 'message', + status: 'error', + }, + }; + + render(); + + expect(screen.getByText(mockProps.testingStatus.message)).toBeInTheDocument(); }); }); diff --git a/public/app/features/datasources/settings/DataSourceSettingsPage.tsx b/public/app/features/datasources/settings/DataSourceSettingsPage.tsx index 7fc561c7ec1..46beaf30ff7 100644 --- a/public/app/features/datasources/settings/DataSourceSettingsPage.tsx +++ b/public/app/features/datasources/settings/DataSourceSettingsPage.tsx @@ -127,7 +127,7 @@ export class DataSourceSettingsPage extends PureComponent { renderIsReadOnlyMessage() { return ( - + This data source was added by config and cannot be modified using the UI. Please contact your server admin to update this data source. @@ -234,12 +234,14 @@ export class DataSourceSettingsPage extends PureComponent { )}
- {testingStatus && testingStatus.message && ( + {testingStatus?.message && ( + > + {testingStatus.details?.message ?? null} + )}
diff --git a/public/app/features/datasources/settings/__snapshots__/DataSourceSettingsPage.test.tsx.snap b/public/app/features/datasources/settings/__snapshots__/DataSourceSettingsPage.test.tsx.snap deleted file mode 100644 index 82db4395d93..00000000000 --- a/public/app/features/datasources/settings/__snapshots__/DataSourceSettingsPage.test.tsx.snap +++ /dev/null @@ -1,439 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Render should render alpha info text 1`] = ` - - -
-
-
- - -
- - - -
- - -
- - -`; - -exports[`Render should render beta info text 1`] = ` - - -
-
-
- - -
- - -
- - -
- - -`; - -exports[`Render should render component 1`] = ` - - -
-
- - -
- - -
- - -`; - -exports[`Render should render is ready only message 1`] = ` - - -
-
- - This data source was added by config and cannot be modified using the UI. Please contact your server admin to update this data source. - - - - -
- - -
- - -`; - -exports[`Render should render loader 1`] = ` - - - -`; diff --git a/public/app/features/datasources/state/actions.ts b/public/app/features/datasources/state/actions.ts index f08d6a1331d..161a7f921e5 100644 --- a/public/app/features/datasources/state/actions.ts +++ b/public/app/features/datasources/state/actions.ts @@ -95,15 +95,10 @@ export const testDataSource = ( dispatch(testDataSourceSucceeded(result)); } catch (err) { - let message = ''; + const { statusText, message: errMessage, details } = err; + const message = statusText ? 'HTTP error ' + statusText : errMessage; - if (err.statusText) { - message = 'HTTP error ' + err.statusText; - } else { - message = err.message; - } - - dispatch(testDataSourceFailed({ message })); + dispatch(testDataSourceFailed({ message, details })); } }); }; diff --git a/public/app/features/datasources/state/reducers.ts b/public/app/features/datasources/state/reducers.ts index 191f6f84d2b..0a7c6277cb4 100644 --- a/public/app/features/datasources/state/reducers.ts +++ b/public/app/features/datasources/state/reducers.ts @@ -1,7 +1,7 @@ import { AnyAction, createAction } from '@reduxjs/toolkit'; import { DataSourcePluginMeta, DataSourceSettings } from '@grafana/data'; -import { DataSourcesState, DataSourceSettingsState } from 'app/types'; +import { DataSourcesState, DataSourceSettingsState, TestingStatus } from 'app/types'; import { LayoutMode, LayoutModes } from 'app/core/components/LayoutSelector/LayoutSelector'; import { DataSourceTypesLoadedPayload } from './actions'; import { GenericDataSourcePlugin } from '../settings/PluginSettings'; @@ -96,10 +96,7 @@ export const dataSourcesReducer = (state: DataSourcesState = initialState, actio }; export const initialDataSourceSettingsState: DataSourceSettingsState = { - testingStatus: { - status: null, - message: null, - }, + testingStatus: {}, loadError: null, plugin: null, }; @@ -112,12 +109,9 @@ export const initDataSourceSettingsFailed = createAction('dataSourceSetti export const testDataSourceStarting = createAction('dataSourceSettings/testDataSourceStarting'); -export const testDataSourceSucceeded = createAction<{ - status: string; - message: string; -}>('dataSourceSettings/testDataSourceSucceeded'); +export const testDataSourceSucceeded = createAction('dataSourceSettings/testDataSourceSucceeded'); -export const testDataSourceFailed = createAction<{ message: string }>('dataSourceSettings/testDataSourceFailed'); +export const testDataSourceFailed = createAction('dataSourceSettings/testDataSourceFailed'); export const dataSourceSettingsReducer = ( state: DataSourceSettingsState = initialDataSourceSettingsState, @@ -145,8 +139,9 @@ export const dataSourceSettingsReducer = ( return { ...state, testingStatus: { - status: action.payload.status, - message: action.payload.message, + status: action.payload?.status, + message: action.payload?.message, + details: action.payload?.details, }, }; } @@ -156,7 +151,8 @@ export const dataSourceSettingsReducer = ( ...state, testingStatus: { status: 'error', - message: action.payload.message, + message: action.payload?.message, + details: action.payload?.details, }, }; } diff --git a/public/app/types/datasources.ts b/public/app/types/datasources.ts index a7eaf5c86b8..a434cf657b1 100644 --- a/public/app/types/datasources.ts +++ b/public/app/types/datasources.ts @@ -1,6 +1,7 @@ import { LayoutMode } from '../core/components/LayoutSelector/LayoutSelector'; import { DataSourcePluginMeta, DataSourceSettings } from '@grafana/data'; import { GenericDataSourcePlugin } from 'app/features/datasources/settings/PluginSettings'; +import { HealthCheckResultDetails } from '@grafana/runtime/src/utils/DataSourceWithBackend'; export interface DataSourcesState { dataSources: DataSourceSettings[]; @@ -16,12 +17,15 @@ export interface DataSourcesState { categories: DataSourcePluginCategory[]; } +export interface TestingStatus { + message?: string | null; + status?: string | null; + details?: HealthCheckResultDetails; +} + export interface DataSourceSettingsState { plugin?: GenericDataSourcePlugin | null; - testingStatus?: { - message?: string | null; - status?: string | null; - }; + testingStatus?: TestingStatus; loadError?: string | null; }