From b27985ef3824c8df8f0710745a7fa63378e2471f Mon Sep 17 00:00:00 2001 From: Shavonn Brown Date: Wed, 4 Mar 2020 09:11:11 -0500 Subject: [PATCH] Azure Monitor: config editor updates, update sameas switch, fix test snaps (#22554) * azure monitor config editor updates, do not show helper for sameas unless just switched, not create * update snapshots --- packages/grafana-data/src/utils/datasource.ts | 7 +- .../components/AnalyticsConfig.test.tsx | 6 +- .../components/AnalyticsConfig.tsx | 47 ++++++++---- .../components/ConfigEditor.tsx | 72 ++++++++----------- .../components/InsightsConfig.test.tsx | 17 ++--- .../components/InsightsConfig.tsx | 26 +++---- .../components/MonitorConfig.tsx | 14 ++-- .../__snapshots__/ConfigEditor.test.tsx.snap | 52 ++++++++++++-- .../InsightsConfig.test.tsx.snap | 39 +++++----- 9 files changed, 160 insertions(+), 120 deletions(-) diff --git a/packages/grafana-data/src/utils/datasource.ts b/packages/grafana-data/src/utils/datasource.ts index 58c55410744..9b5e7f5fdc2 100644 --- a/packages/grafana-data/src/utils/datasource.ts +++ b/packages/grafana-data/src/utils/datasource.ts @@ -85,7 +85,10 @@ export const updateDatasourcePluginSecureJsonDataOption = ( + props: DataSourcePluginOptionsEditorProps, + key: string +) => { const config = props.options; props.onOptionsChange({ @@ -99,4 +102,4 @@ export function updateDatasourcePluginResetOption(props: DataSourcePluginOptions [key]: false, }, }); -} +}; 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 f9649abe664..0a737cc8512 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 @@ -39,9 +39,9 @@ const setup = (propOverrides?: object) => { subscriptions: [], workspaces: [], makeSameAs: jest.fn(), - onUpdateOptions: jest.fn(), - onUpdateOption: jest.fn(), - onUpdateSecureOption: jest.fn(), + onUpdateDatasourceOptions: jest.fn(), + onUpdateJsonDataOption: jest.fn(), + onUpdateSecureJsonDataOption: 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 3ef6188578a..9e2b97fd524 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 @@ -4,47 +4,59 @@ import { AzureCredentialsForm } from './AzureCredentialsForm'; import { Switch, FormLabel, Select, Button } from '@grafana/ui'; import { AzureDataSourceSettings } from '../types'; +export interface State { + sameAsSwitched: boolean; +} + export interface Props { options: AzureDataSourceSettings; subscriptions: SelectableValue[]; workspaces: SelectableValue[]; makeSameAs: () => void; - onUpdateOptions: (options: AzureDataSourceSettings) => void; - onUpdateOption: (key: string, val: any) => void; - onUpdateSecureOption: (key: string, val: any) => void; + onUpdateDatasourceOptions: (options: AzureDataSourceSettings) => void; + onUpdateJsonDataOption: (key: string, val: any) => void; + onUpdateSecureJsonDataOption: (key: string, val: any) => void; onResetOptionKey: (key: string) => void; onLoadSubscriptions: (type?: string) => void; onLoadWorkspaces: (type?: string) => void; } -export class AnalyticsConfig extends PureComponent { +export class AnalyticsConfig extends PureComponent { + constructor(props: Props) { + super(props); + + this.state = { + sameAsSwitched: false, + }; + } + onLogAnalyticsTenantIdChange = (event: ChangeEvent) => { - this.props.onUpdateOption('logAnalyticsTenantId', event.target.value); + this.props.onUpdateJsonDataOption('logAnalyticsTenantId', event.target.value); }; onLogAnalyticsClientIdChange = (event: ChangeEvent) => { - this.props.onUpdateOption('logAnalyticsClientId', event.target.value); + this.props.onUpdateJsonDataOption('logAnalyticsClientId', event.target.value); }; onLogAnalyticsClientSecretChange = (event: ChangeEvent) => { - this.props.onUpdateSecureOption('logAnalyticsClientSecret', event.target.value); + this.props.onUpdateSecureJsonDataOption('logAnalyticsClientSecret', event.target.value); }; onLogAnalyticsSubscriptionSelect = (logAnalyticsSubscription: SelectableValue) => { - this.props.onUpdateOption('logAnalyticsSubscriptionId', logAnalyticsSubscription.value); + this.props.onUpdateJsonDataOption('logAnalyticsSubscriptionId', logAnalyticsSubscription.value); }; onWorkspaceSelectChange = (logAnalyticsDefaultWorkspace: SelectableValue) => { - this.props.onUpdateOption('logAnalyticsDefaultWorkspace', logAnalyticsDefaultWorkspace.value); + this.props.onUpdateJsonDataOption('logAnalyticsDefaultWorkspace', logAnalyticsDefaultWorkspace.value); }; onAzureLogAnalyticsSameAsChange = () => { - const { options, onUpdateOptions, makeSameAs } = this.props; + const { options, onUpdateDatasourceOptions, makeSameAs } = this.props; if (!options.jsonData.azureLogAnalyticsSameAs && options.secureJsonData.clientSecret) { makeSameAs(); } else if (!options.jsonData.azureLogAnalyticsSameAs) { // if currently off, clear monitor secret - onUpdateOptions({ + onUpdateDatasourceOptions({ ...options, jsonData: { ...options.jsonData, @@ -58,11 +70,13 @@ export class AnalyticsConfig extends PureComponent { clientSecret: false, }, }); - } else { - this.props.onUpdateOption('azureLogAnalyticsSameAs', !options.jsonData.azureLogAnalyticsSameAs); - } - // init popover to warn secret needs to be re-entered + this.setState({ + sameAsSwitched: true, + }); + } else { + this.props.onUpdateJsonDataOption('azureLogAnalyticsSameAs', !options.jsonData.azureLogAnalyticsSameAs); + } }; onLogAnalyticsResetClientSecret = () => { @@ -100,6 +114,8 @@ export class AnalyticsConfig extends PureComponent { workspaces, } = this.props; + const { sameAsSwitched } = this.state; + if (!jsonData.hasOwnProperty('azureLogAnalyticsSameAs')) { jsonData.azureLogAnalyticsSameAs = true; } @@ -111,6 +127,7 @@ export class AnalyticsConfig extends PureComponent { }; const showSameAsHelpMsg = + sameAsSwitched && jsonData.azureLogAnalyticsSameAs && secureJsonFields && !secureJsonFields.clientSecret && diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ConfigEditor.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ConfigEditor.tsx index e97897b7eed..0570f9d9145 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ConfigEditor.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ConfigEditor.tsx @@ -68,23 +68,35 @@ export class ConfigEditor extends PureComponent { } }; - updateOption = (key: keyof AzureDataSourceJsonData, val: any) => { + updateJsonDataOption = (key: keyof AzureDataSourceJsonData, val: any) => { updateDatasourcePluginJsonDataOption(this.props, key, val); }; - updateSecureOption = (key: keyof AzureDataSourceSecureJsonData, val: any) => { + updateSecureJsonDataOption = (key: keyof AzureDataSourceSecureJsonData, val: any) => { updateDatasourcePluginSecureJsonDataOption(this.props, key, val); }; - resetKey = (key: string) => { + resetSecureKey = (key: keyof AzureDataSourceSecureJsonData) => { updateDatasourcePluginResetOption(this.props, key); }; + onUpdateJsonDataOption = (key: keyof AzureDataSourceJsonData) => ( + event: React.SyntheticEvent + ) => { + this.updateJsonDataOption(key, event.currentTarget.value); + }; + + onUpdateSecureJsonDataOption = (key: keyof AzureDataSourceSecureJsonData) => ( + event: React.SyntheticEvent + ) => { + this.updateSecureJsonDataOption(key, event.currentTarget.value); + }; + makeSameAs = (updatedClientSecret?: string) => { const { options } = this.props; const clientSecret = updatedClientSecret || options.secureJsonData.clientSecret; - this.updateOptions({ + this.props.onOptionsChange({ ...options, jsonData: { ...options.jsonData, @@ -101,32 +113,6 @@ export class ConfigEditor extends PureComponent { }); }; - updateOptions = (options: AzureDataSourceSettings) => { - if (options.hasOwnProperty('secureJsonData')) { - if (options.secureJsonData.hasOwnProperty('clientSecret') && options.secureJsonData.clientSecret.length === 0) { - delete options.secureJsonData.clientSecret; - } - - if ( - options.secureJsonData.hasOwnProperty('logAnalyticsClientSecret') && - options.secureJsonData.logAnalyticsClientSecret.length === 0 - ) { - delete options.secureJsonData.logAnalyticsClientSecret; - } - - if ( - options.secureJsonData.hasOwnProperty('appInsightsApiKey') && - options.secureJsonData.appInsightsApiKey.length === 0 - ) { - delete options.secureJsonData.appInsightsApiKey; - } - } - - this.props.onOptionsChange({ - ...options, - }); - }; - hasNecessaryCredentials = () => { if (!this.props.options.secureJsonFields.clientSecret && !this.props.options.secureJsonData.clientSecret) { return false; @@ -214,7 +200,7 @@ export class ConfigEditor extends PureComponent { if (subscriptions && subscriptions.length > 0) { this.setState({ subscriptions }); - this.updateOption('subscriptionId', this.props.options.jsonData.subscriptionId || subscriptions[0].value); + this.updateJsonDataOption('subscriptionId', this.props.options.jsonData.subscriptionId || subscriptions[0].value); } if (this.props.options.jsonData.subscriptionId && this.props.options.jsonData.azureLogAnalyticsSameAs) { @@ -233,7 +219,7 @@ export class ConfigEditor extends PureComponent { if (logAnalyticsSubscriptions && logAnalyticsSubscriptions.length > 0) { this.setState({ logAnalyticsSubscriptions }); - this.updateOption( + this.updateJsonDataOption( 'logAnalyticsSubscriptionId', this.props.options.jsonData.logAnalyticsSubscriptionId || logAnalyticsSubscriptions[0].value ); @@ -257,7 +243,7 @@ export class ConfigEditor extends PureComponent { if (logAnalyticsWorkspaces.length > 0) { this.setState({ logAnalyticsWorkspaces }); - this.updateOption( + this.updateJsonDataOption( 'logAnalyticsDefaultWorkspace', this.props.options.jsonData.logAnalyticsDefaultWorkspace || logAnalyticsWorkspaces[0].value ); @@ -278,9 +264,9 @@ export class ConfigEditor extends PureComponent { subscriptions={subscriptions} makeSameAs={this.makeSameAs} onLoadSubscriptions={this.onLoadSubscriptions} - onUpdateOption={this.updateOption} - onUpdateSecureOption={this.updateSecureOption} - onResetOptionKey={this.resetKey} + onUpdateJsonDataOption={this.updateJsonDataOption} + onUpdateSecureJsonDataOption={this.updateSecureJsonDataOption} + onResetOptionKey={this.resetSecureKey} /> { workspaces={logAnalyticsWorkspaces} subscriptions={logAnalyticsSubscriptions} makeSameAs={this.makeSameAs} - onUpdateOptions={this.updateOptions} - onUpdateOption={this.updateOption} - onUpdateSecureOption={this.updateSecureOption} - onResetOptionKey={this.resetKey} + onUpdateDatasourceOptions={this.props.onOptionsChange} + onUpdateJsonDataOption={this.updateJsonDataOption} + onUpdateSecureJsonDataOption={this.updateSecureJsonDataOption} + onResetOptionKey={this.resetSecureKey} onLoadSubscriptions={this.onLoadSubscriptions} onLoadWorkspaces={this.getWorkspaces} /> ); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.test.tsx index fbfce5c210e..2446edb3005 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.test.tsx @@ -20,22 +20,17 @@ const setup = (propOverrides?: object) => { basicAuthPassword: '', withCredentials: false, isDefault: false, - secureJsonFields: { - appInsightsApiKey: false, - }, + secureJsonFields: {}, jsonData: { cloudName: '', subscriptionId: '', - appInsightsAppId: 'cvvcc020-2cpo-123a-a3d0-df6547fki792', - }, - secureJsonData: { - appInsightsApiKey: 'e7f3f775-a987-4b3f-3835-51c4f982kl48', }, + secureJsonData: {}, version: 1, readOnly: false, }, - onUpdateOption: jest.fn(), - onUpdateSecureOption: jest.fn(), + onUpdateJsonDataOption: jest.fn(), + onUpdateSecureJsonDataOption: jest.fn(), onResetOptionKey: jest.fn(), }; @@ -53,7 +48,7 @@ describe('Render', () => { it('should disable insights api key input', () => { const wrapper = setup({ - datasourceConfig: { + options: { secureJsonFields: { appInsightsApiKey: true, }, @@ -70,7 +65,7 @@ describe('Render', () => { it('should enable insights api key input', () => { const wrapper = setup({ - datasourceConfig: { + options: { secureJsonFields: { appInsightsApiKey: false, }, diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.tsx index eb1e9c9ea23..38b339bb648 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/InsightsConfig.tsx @@ -1,28 +1,24 @@ -import React, { PureComponent, ChangeEvent } from 'react'; +import React, { PureComponent } from 'react'; import { FormLabel, Button, Input } from '@grafana/ui'; -import { AzureDataSourceSettings } from '../types'; +import { AzureDataSourceSettings, AzureDataSourceJsonData, AzureDataSourceSecureJsonData } from '../types'; export interface Props { options: AzureDataSourceSettings; - onUpdateOption: (key: string, val: any) => void; - onUpdateSecureOption: (key: string, val: any) => void; + onUpdateJsonDataOption: ( + key: keyof AzureDataSourceJsonData + ) => (event: React.SyntheticEvent) => void; + onUpdateSecureJsonDataOption: ( + key: keyof AzureDataSourceSecureJsonData + ) => (event: React.SyntheticEvent) => void; onResetOptionKey: (key: string) => void; } export class InsightsConfig extends PureComponent { - onAppInsightsAppIdChange = (event: ChangeEvent) => { - this.props.onUpdateOption('appInsightsAppId', event.target.value); - }; - - onAppInsightsApiKeyChange = (event: ChangeEvent) => { - this.props.onUpdateSecureOption('appInsightsApiKey', event.target.value); - }; - onAppInsightsResetApiKey = () => { this.props.onResetOptionKey('appInsightsApiKey'); }; render() { - const { options } = this.props; + const { options, onUpdateJsonDataOption, onUpdateSecureJsonDataOption } = this.props; return ( <>

Application Insights Details

@@ -50,7 +46,7 @@ export class InsightsConfig extends PureComponent { className="width-30" placeholder="XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX" value={options.secureJsonData.appInsightsApiKey || ''} - onChange={this.onAppInsightsApiKeyChange} + onChange={onUpdateSecureJsonDataOption('appInsightsApiKey')} /> @@ -63,7 +59,7 @@ export class InsightsConfig extends PureComponent { 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 714bf5fddb2..cc7d7670335 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 @@ -14,23 +14,23 @@ export interface Props { options: AzureDataSourceSettings; subscriptions: SelectableValue[]; makeSameAs: (updatedClientSecret?: string) => void; - onUpdateOption: (key: string, val: any) => void; - onUpdateSecureOption: (key: string, val: any) => void; + onUpdateJsonDataOption: (key: string, val: any) => void; + onUpdateSecureJsonDataOption: (key: string, val: any) => void; onResetOptionKey: (key: string) => void; onLoadSubscriptions: () => void; } export class MonitorConfig extends PureComponent { onAzureCloudSelect = (cloudName: SelectableValue) => { - this.props.onUpdateOption('cloudName', cloudName.value); + this.props.onUpdateJsonDataOption('cloudName', cloudName.value); }; onTenantIdChange = (event: ChangeEvent) => { - this.props.onUpdateOption('tenantId', event.target.value); + this.props.onUpdateJsonDataOption('tenantId', event.target.value); }; onClientIdChange = (event: ChangeEvent) => { - this.props.onUpdateOption('clientId', event.target.value); + this.props.onUpdateJsonDataOption('clientId', event.target.value); }; onClientSecretChange = (event: ChangeEvent) => { @@ -39,7 +39,7 @@ export class MonitorConfig extends PureComponent { if (options.jsonData.azureLogAnalyticsSameAs && event.target.value) { makeSameAs(event.target.value); } else { - this.props.onUpdateSecureOption('clientSecret', event.target.value); + this.props.onUpdateSecureJsonDataOption('clientSecret', event.target.value); } }; @@ -48,7 +48,7 @@ export class MonitorConfig extends PureComponent { }; onSubscriptionSelect = (subscription: SelectableValue) => { - this.props.onUpdateOption('subscriptionId', subscription.value); + this.props.onUpdateJsonDataOption('subscriptionId', subscription.value); }; render() { 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 6a2dd524ec1..f7ee799e71d 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 @@ -6,8 +6,8 @@ exports[`Render should render component 1`] = ` makeSameAs={[Function]} onLoadSubscriptions={[Function]} onResetOptionKey={[Function]} - onUpdateOption={[Function]} - onUpdateSecureOption={[Function]} + onUpdateJsonDataOption={[Function]} + onUpdateSecureJsonDataOption={[Function]} options={ Object { "access": "proxy", @@ -43,9 +43,47 @@ exports[`Render should render component 1`] = ` onLoadSubscriptions={[Function]} onLoadWorkspaces={[Function]} onResetOptionKey={[Function]} - onUpdateOption={[Function]} - onUpdateOptions={[Function]} - onUpdateSecureOption={[Function]} + onUpdateDatasourceOptions={ + [MockFunction] { + "calls": Array [ + Array [ + Object { + "access": "proxy", + "basicAuth": false, + "basicAuthPassword": "", + "basicAuthUser": "", + "database": "", + "id": 21, + "isDefault": false, + "jsonData": Object { + "azureLogAnalyticsSameAs": true, + "cloudName": "azuremonitor", + "subscriptionId": "44987801-6nn6-49he-9b2d-9106972f9789", + }, + "name": "Azure Monitor-10-10", + "orgId": 1, + "password": "", + "readOnly": false, + "secureJsonFields": Object {}, + "type": "grafana-azure-monitor-datasource", + "typeLogoUrl": "", + "url": "/api/datasources/proxy/21", + "user": "", + "version": 1, + "withCredentials": false, + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], + } + } + onUpdateJsonDataOption={[Function]} + onUpdateSecureJsonDataOption={[Function]} options={ Object { "access": "proxy", @@ -79,8 +117,8 @@ exports[`Render should render component 1`] = ` /> API Key + + +
- +
@@ -49,8 +59,7 @@ exports[`Render should disable insights api key input 1`] = ` > @@ -85,9 +94,8 @@ exports[`Render should enable insights api key input 1`] = ` > @@ -108,8 +116,7 @@ exports[`Render should enable insights api key input 1`] = ` > @@ -144,9 +151,8 @@ exports[`Render should render component 1`] = ` > @@ -167,8 +173,7 @@ exports[`Render should render component 1`] = ` >