From 2098b9eb449afbbe6a3a1bfaca5e1c9ffbeaa76d Mon Sep 17 00:00:00 2001 From: Shavonn Brown Date: Wed, 4 Dec 2019 14:35:53 -0500 Subject: [PATCH] Azure Monitor: Standardize Config Editor Implementation (#20455) * initial changes - removal from state, remove anon functions, reset secrets empty * post testing cleanup * init promise cancellation, other cleanup * workspaces response parser, remove version incrementing * update datasource funcs - DRYer * remove prop mutation * func to modify root config opt * fix version issue * update snapshot --- packages/grafana-data/src/types/datasource.ts | 58 +++++ .../azure_monitor/response_parser.ts | 21 ++ .../components/AnalyticsConfig.test.tsx | 12 +- .../components/AnalyticsConfig.tsx | 137 +++------- .../components/AzureCredentialsForm.test.tsx | 6 +- .../components/AzureCredentialsForm.tsx | 71 ++---- .../components/ConfigEditor.test.tsx | 2 +- .../components/ConfigEditor.tsx | 236 +++++++++--------- .../components/InsightsConfig.test.tsx | 11 +- .../components/InsightsConfig.tsx | 73 ++---- .../components/MonitorConfig.tsx | 117 +++------ .../AzureCredentialsForm.test.tsx.snap | 34 +-- .../__snapshots__/ConfigEditor.test.tsx.snap | 84 ++++--- .../InsightsConfig.test.tsx.snap | 34 +-- .../grafana-azure-monitor-datasource/types.ts | 9 +- 15 files changed, 391 insertions(+), 514 deletions(-) diff --git a/packages/grafana-data/src/types/datasource.ts b/packages/grafana-data/src/types/datasource.ts index b37f7b443f8..a239b84ed73 100644 --- a/packages/grafana-data/src/types/datasource.ts +++ b/packages/grafana-data/src/types/datasource.ts @@ -272,6 +272,64 @@ export abstract class DataSourceApi< interpolateVariablesInQueries?(queries: TQuery[]): TQuery[]; } +export function updateDatasourcePluginOption(props: DataSourcePluginOptionsEditorProps, key: string, val: any) { + let config = props.options; + + config = { + ...config, + [key]: val, + }; + + props.onOptionsChange(config); +} + +export function updateDatasourcePluginJsonDataOption( + props: DataSourcePluginOptionsEditorProps, + key: string, + val: any, + secure: boolean +) { + let config = props.options; + + if (secure) { + config = { + ...config, + secureJsonData: { + ...config.secureJsonData, + [key]: val, + }, + }; + } else { + config = { + ...config, + jsonData: { + ...config.jsonData, + [key]: val, + }, + }; + } + + props.onOptionsChange(config); +} + +export function updateDatasourcePluginResetKeyOption(props: DataSourcePluginOptionsEditorProps, key: string) { + let config = props.options; + + config = { + ...config, + secureJsonData: { + ...config.secureJsonData, + [key]: '', + }, + secureJsonFields: { + ...config.secureJsonFields, + [key]: false, + }, + }; + + props.onOptionsChange(config); +} + export interface QueryEditorProps< DSType extends DataSourceApi, TQuery extends DataQuery = DataQuery, diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/response_parser.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/response_parser.ts index 4a14e7067f5..2fb45d85327 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/response_parser.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/response_parser.ts @@ -149,4 +149,25 @@ export default class ResponseParser { return list; } + + static parseWorkspacesForSelect(result: any): Array<{ label: string; value: string }> { + const list: Array<{ label: string; value: string }> = []; + + if (!result) { + return list; + } + + const valueFieldName = 'customerId'; + const textFieldName = 'name'; + for (let i = 0; i < result.data.value.length; i++) { + if (!_.find(list, ['value', _.get(result.data.value[i].properties, valueFieldName)])) { + list.push({ + label: _.get(result.data.value[i], textFieldName), + value: _.get(result.data.value[i].properties, valueFieldName), + }); + } + } + + return list; + } } diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.test.tsx index 928a9b43698..9fa2a5c27ae 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.test.tsx @@ -4,7 +4,7 @@ import AnalyticsConfig, { Props } from './AnalyticsConfig'; const setup = (propOverrides?: object) => { const props: Props = { - datasourceConfig: { + options: { id: 21, orgId: 1, name: 'Azure Monitor-10-10', @@ -24,9 +24,10 @@ const setup = (propOverrides?: object) => { logAnalyticsClientSecret: false, }, jsonData: { + cloudName: '', + subscriptionId: '', azureLogAnalyticsSameAs: false, logAnalyticsDefaultWorkspace: '', - logAnalyticsClientSecret: '', logAnalyticsTenantId: '', }, secureJsonData: { @@ -35,9 +36,10 @@ const setup = (propOverrides?: object) => { version: 1, readOnly: false, }, - logAnalyticsSubscriptions: [], - logAnalyticsWorkspaces: [], - onDatasourceUpdate: jest.fn(), + subscriptions: [], + workspaces: [], + onUpdateOption: jest.fn(), + onResetOptionKey: jest.fn(), onLoadSubscriptions: jest.fn(), onLoadWorkspaces: jest.fn(), }; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.tsx index 61cdcb877bf..08c47d00a47 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.tsx @@ -1,117 +1,52 @@ -import React, { PureComponent } from 'react'; +import React, { PureComponent, ChangeEvent } from 'react'; import { SelectableValue } from '@grafana/data'; import { AzureCredentialsForm } from './AzureCredentialsForm'; import { Switch, FormLabel, Select, Button } from '@grafana/ui'; +import { AzureDataSourceSettings } from '../types'; export interface Props { - datasourceConfig: any; - logAnalyticsSubscriptions: SelectableValue[]; - logAnalyticsWorkspaces: SelectableValue[]; - onDatasourceUpdate: (config: any) => void; + options: AzureDataSourceSettings; + subscriptions: SelectableValue[]; + workspaces: SelectableValue[]; + onUpdateOption: (key: string, val: any, secure: boolean) => void; + onResetOptionKey: (key: string) => void; onLoadSubscriptions: (type?: string) => void; onLoadWorkspaces: (type?: string) => void; } - -export interface State { - config: any; - logAnalyticsSubscriptions: SelectableValue[]; - logAnalyticsWorkspaces: SelectableValue[]; -} - -export class AnalyticsConfig extends PureComponent { - constructor(props: Props) { - super(props); - - const { datasourceConfig } = this.props; - - this.state = { - config: datasourceConfig, - logAnalyticsSubscriptions: [], - logAnalyticsWorkspaces: [], - }; - } - - static getDerivedStateFromProps(props: Props, state: State) { - return { - ...state, - config: props.datasourceConfig, - logAnalyticsSubscriptions: props.logAnalyticsSubscriptions, - logAnalyticsWorkspaces: props.logAnalyticsWorkspaces, - }; - } - - onLogAnalyticsTenantIdChange = (logAnalyticsTenantId: string) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - logAnalyticsTenantId, - }, - }); +export class AnalyticsConfig extends PureComponent { + onLogAnalyticsTenantIdChange = (event: ChangeEvent) => { + this.props.onUpdateOption('logAnalyticsTenantId', event.target.value, false); }; - onLogAnalyticsClientIdChange = (logAnalyticsClientId: string) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - logAnalyticsClientId, - }, - }); + onLogAnalyticsClientIdChange = (event: ChangeEvent) => { + this.props.onUpdateOption('logAnalyticsClientId', event.target.value, false); }; - onLogAnalyticsClientSecretChange = (logAnalyticsClientSecret: string) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - secureJsonData: { - ...this.state.config.secureJsonData, - logAnalyticsClientSecret, - }, - }); - }; - - onLogAnalyticsResetClientSecret = () => { - this.props.onDatasourceUpdate({ - ...this.state.config, - version: this.state.config.version + 1, - secureJsonFields: { ...this.state.config.secureJsonFields, logAnalyticsClientSecret: false }, - }); + onLogAnalyticsClientSecretChange = (event: ChangeEvent) => { + this.props.onUpdateOption('logAnalyticsClientSecret', event.target.value, true); }; onLogAnalyticsSubscriptionSelect = (logAnalyticsSubscription: SelectableValue) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - logAnalyticsSubscriptionId: logAnalyticsSubscription.value, - }, - }); + this.props.onUpdateOption('logAnalyticsSubscriptionId', logAnalyticsSubscription.value, false); }; onWorkspaceSelectChange = (logAnalyticsDefaultWorkspace: SelectableValue) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - logAnalyticsDefaultWorkspace: logAnalyticsDefaultWorkspace.value, - }, - }); + this.props.onUpdateOption('logAnalyticsDefaultWorkspace', logAnalyticsDefaultWorkspace.value, false); }; - onAzureLogAnalyticsSameAsChange = (azureLogAnalyticsSameAs: boolean) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - azureLogAnalyticsSameAs, - }, - }); + onAzureLogAnalyticsSameAsChange = () => { + const { options } = this.props; + this.props.onUpdateOption('azureLogAnalyticsSameAs', !options.jsonData.azureLogAnalyticsSameAs, false); + }; + + onLogAnalyticsResetClientSecret = () => { + this.props.onResetOptionKey('logAnalyticsClientSecret'); }; hasWorkspaceRequiredFields = () => { const { - config: { jsonData, secureJsonData, secureJsonFields }, - } = this.state; + options: { jsonData, secureJsonData, secureJsonFields }, + } = this.props; if (jsonData.azureLogAnalyticsSameAs) { return ( @@ -134,10 +69,14 @@ export class AnalyticsConfig extends PureComponent { render() { const { - config: { jsonData, secureJsonData, secureJsonFields }, - logAnalyticsSubscriptions, - logAnalyticsWorkspaces, - } = this.state; + options: { jsonData, secureJsonData, secureJsonFields }, + subscriptions, + workspaces, + } = this.props; + + if (!jsonData.hasOwnProperty('azureLogAnalyticsSameAs')) { + jsonData.azureLogAnalyticsSameAs = true; + } const addtlAttrs = { ...(jsonData.azureLogAnalyticsSameAs && { @@ -150,12 +89,12 @@ export class AnalyticsConfig extends PureComponent { this.onAzureLogAnalyticsSameAsChange(!jsonData.azureLogAnalyticsSameAs)} + onChange={this.onAzureLogAnalyticsSameAsChange} {...addtlAttrs} /> {!jsonData.azureLogAnalyticsSameAs && ( {
this.onAppInsightsApiKeyChange(event.target.value)} + value={options.secureJsonData.appInsightsApiKey || ''} + onChange={this.onAppInsightsApiKeyChange} />
@@ -100,8 +61,8 @@ export class InsightsConfig extends PureComponent {
this.onAppInsightsAppIdChange(event.target.value)} + value={options.jsonData.appInsightsAppId || ''} + onChange={this.onAppInsightsAppIdChange} />
diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MonitorConfig.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MonitorConfig.tsx index c8d9bfb03bc..c4a3f3b0c43 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MonitorConfig.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MonitorConfig.tsx @@ -1,121 +1,62 @@ -import React, { PureComponent } from 'react'; +import React, { PureComponent, ChangeEvent } from 'react'; import { SelectableValue } from '@grafana/data'; import { AzureCredentialsForm } from './AzureCredentialsForm'; +import { AzureDataSourceSettings } from '../types'; + +const azureClouds = [ + { value: 'azuremonitor', label: 'Azure' }, + { value: 'govazuremonitor', label: 'Azure US Government' }, + { value: 'germanyazuremonitor', label: 'Azure Germany' }, + { value: 'chinaazuremonitor', label: 'Azure China' }, +] as SelectableValue[]; export interface Props { - datasourceConfig: any; + options: AzureDataSourceSettings; subscriptions: SelectableValue[]; - onDatasourceUpdate: (config: any) => void; + onUpdateOption: (key: string, val: any, secure: boolean) => void; + onResetOptionKey: (key: string) => void; onLoadSubscriptions: () => void; } -export interface State { - config: any; - azureClouds: SelectableValue[]; - subscriptions: SelectableValue[]; -} - -export class MonitorConfig extends PureComponent { - constructor(props: Props) { - super(props); - - const { datasourceConfig } = this.props; - - this.state = { - config: datasourceConfig, - azureClouds: [ - { value: 'azuremonitor', label: 'Azure' }, - { value: 'govazuremonitor', label: 'Azure US Government' }, - { value: 'germanyazuremonitor', label: 'Azure Germany' }, - { value: 'chinaazuremonitor', label: 'Azure China' }, - ], - subscriptions: [], - }; - } - - static getDerivedStateFromProps(props: Props, state: State) { - return { - ...state, - config: props.datasourceConfig, - subscriptions: props.subscriptions, - }; - } - +export class MonitorConfig extends PureComponent { onAzureCloudSelect = (cloudName: SelectableValue) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - cloudName: cloudName.value, - }, - }); + this.props.onUpdateOption('cloudName', cloudName.value, false); }; - onTenantIdChange = (tenantId: string) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - tenantId, - }, - }); + onTenantIdChange = (event: ChangeEvent) => { + this.props.onUpdateOption('tenantId', event.target.value, false); }; - onClientIdChange = (clientId: string) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - clientId, - }, - }); + onClientIdChange = (event: ChangeEvent) => { + this.props.onUpdateOption('clientId', event.target.value, false); }; - onClientSecretChange = (clientSecret: string) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - secureJsonData: { - ...this.state.config.secureJsonData, - clientSecret, - }, - }); + onClientSecretChange = (event: ChangeEvent) => { + this.props.onUpdateOption('clientSecret', event.target.value, true); }; onResetClientSecret = () => { - this.props.onDatasourceUpdate({ - ...this.state.config, - version: this.state.config.version + 1, - secureJsonFields: { - ...this.state.config.secureJsonFields, - clientSecret: false, - }, - }); + this.props.onResetOptionKey('clientSecret'); }; onSubscriptionSelect = (subscription: SelectableValue) => { - this.props.onDatasourceUpdate({ - ...this.state.config, - jsonData: { - ...this.state.config.jsonData, - subscriptionId: subscription.value, - }, - }); + this.props.onUpdateOption('subscriptionId', subscription.value, false); }; render() { - const { azureClouds, config, subscriptions } = this.state; + const { options, subscriptions } = this.props; return ( <>

Azure Monitor Details

@@ -109,9 +109,9 @@ exports[`Render should disable azure monitor secret input 1`] = ` > @@ -177,7 +177,7 @@ exports[`Render should disable azure monitor secret input 1`] = ` "SingleValue": [Function], } } - defaultValue="44693801-6ee6-49de-9b2d-9106972f9572" + defaultValue="44987801-6nn6-49he-9b2d-9106972f9789" isClearable={false} isDisabled={false} isLoading={false} @@ -303,9 +303,9 @@ exports[`Render should enable azure monitor load subscriptions button 1`] = ` > @@ -326,9 +326,9 @@ exports[`Render should enable azure monitor load subscriptions button 1`] = ` > @@ -349,7 +349,7 @@ exports[`Render should enable azure monitor load subscriptions button 1`] = ` > @@ -384,7 +384,7 @@ exports[`Render should enable azure monitor load subscriptions button 1`] = ` "SingleValue": [Function], } } - defaultValue="44693801-6ee6-49de-9b2d-9106972f9572" + defaultValue="44987801-6nn6-49he-9b2d-9106972f9789" isClearable={false} isDisabled={false} isLoading={false} @@ -510,9 +510,9 @@ exports[`Render should render component 1`] = ` > @@ -533,9 +533,9 @@ exports[`Render should render component 1`] = ` > @@ -556,7 +556,7 @@ exports[`Render should render component 1`] = ` > @@ -591,7 +591,7 @@ exports[`Render should render component 1`] = ` "SingleValue": [Function], } } - defaultValue="44693801-6ee6-49de-9b2d-9106972f9572" + defaultValue="44987801-6nn6-49he-9b2d-9106972f9789" isClearable={false} isDisabled={false} isLoading={false} diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/ConfigEditor.test.tsx.snap b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/ConfigEditor.test.tsx.snap index 19e85c690aa..da445103cde 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/ConfigEditor.test.tsx.snap +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/ConfigEditor.test.tsx.snap @@ -3,7 +3,10 @@ exports[`Render should render component 1`] = ` - + `; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/InsightsConfig.test.tsx.snap b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/InsightsConfig.test.tsx.snap index 7cb986c8319..f99f74c6a41 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/InsightsConfig.test.tsx.snap +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/__snapshots__/InsightsConfig.test.tsx.snap @@ -21,25 +21,15 @@ exports[`Render should disable insights api key input 1`] = ` > API Key - - -
- +
@@ -60,7 +50,7 @@ exports[`Render should disable insights api key input 1`] = ` @@ -97,7 +87,7 @@ exports[`Render should enable insights api key input 1`] = ` className="width-30" onChange={[Function]} placeholder="XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX" - value="e7f3f661-a933-4b3f-8176-51c4f982ec48" + value="e7f3f775-a987-4b3f-3835-51c4f982kl48" /> @@ -119,7 +109,7 @@ exports[`Render should enable insights api key input 1`] = ` @@ -156,7 +146,7 @@ exports[`Render should render component 1`] = ` className="width-30" onChange={[Function]} placeholder="XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX" - value="e7f3f661-a933-4b3f-8176-51c4f982ec48" + value="e7f3f775-a987-4b3f-3835-51c4f982kl48" /> @@ -178,7 +168,7 @@ exports[`Render should render component 1`] = ` diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/types.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/types.ts index 6f605361069..88ac403127c 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/types.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/types.ts @@ -1,4 +1,6 @@ -import { DataQuery, DataSourceJsonData } from '@grafana/data'; +import { DataQuery, DataSourceJsonData, DataSourceSettings } from '@grafana/data'; + +export type AzureDataSourceSettings = DataSourceSettings; export interface AzureMonitorQuery extends DataQuery { refId: string; @@ -29,8 +31,9 @@ export interface AzureDataSourceJsonData extends DataSourceJsonData { } export interface AzureDataSourceSecureJsonData { - clientSecret: string; - logAnalyticsClientSecret: string; + clientSecret?: string; + logAnalyticsClientSecret?: string; + appInsightsApiKey?: string; } export interface AzureMetricQuery {