From f8192bf428708d0344b140f9df623d744ff0ae9d Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Thu, 18 Nov 2021 15:13:56 +0100 Subject: [PATCH] Azure Monitor: Clean up fields when editing Metrics (#41762) --- .../MetricsQueryEditor.test.tsx | 42 +--------- .../MetricsQueryEditor/dataHooks.test.ts | 77 ++++++++++++++++++- .../MetricsQueryEditor/dataHooks.ts | 45 ++++++++--- .../MetricsQueryEditor/setQueryValue.ts | 27 +++++++ 4 files changed, 138 insertions(+), 53 deletions(-) diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx index 42cf69d505e..0d013ea943c 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.test.tsx @@ -109,46 +109,8 @@ describe('Azure Monitor QueryEditor', () => { azureMonitor: { ...mockQuery.azureMonitor, metricName: 'metric-b', - }, - }); - }); - - it('should auto select a default aggregation if none exists once a metric is selected', async () => { - const mockDatasource = createMockDatasource(); - const onChange = jest.fn(); - const mockQuery = createMockQuery(); - (mockQuery.azureMonitor ?? {}).aggregation = undefined; - mockDatasource.getMetricNames = jest.fn().mockResolvedValue([ - { - value: 'metric-a', - text: 'Metric A', - }, - { - value: 'metric-b', - text: 'Metric B', - }, - ]); - render( - {}} - /> - ); - await waitFor(() => expect(screen.getByTestId('azure-monitor-metrics-query-editor')).toBeInTheDocument()); - - const metrics = await screen.findByLabelText('Metric'); - await selectOptionInTest(metrics, 'Metric B'); - - expect(onChange).toHaveBeenLastCalledWith({ - ...mockQuery, - azureMonitor: { - ...mockQuery.azureMonitor, - metricName: 'metric-b', - aggregation: 'Average', + aggregation: undefined, + timeGrain: '', }, }); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.test.ts index 06c5ab692a7..22a9af6cabb 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.test.ts @@ -2,6 +2,7 @@ import { renderHook } from '@testing-library/react-hooks'; import { DataHook, + updateSubscriptions, useAsyncState, useMetricNames, useResourceGroups, @@ -340,8 +341,82 @@ describe('AzureMonitor: metrics dataHooks', () => { expect(onChange).toHaveBeenCalledWith({ ...query, - azureMonitor: scenario.expectedClearedQueryPartial, + azureMonitor: { + ...scenario.expectedClearedQueryPartial, + dimensionFilters: [], + timeGrain: '', + }, }); }); }); }); + +describe('AzureMonitor: updateSubscriptions', () => { + const bareQuery = { + refId: 'A', + queryType: AzureQueryType.AzureMonitor, + }; + + [ + { + description: 'should not update with no subscriptions', + query: bareQuery, + subscriptionOptions: [], + }, + { + description: 'should not update with the subscription as an option', + query: { ...bareQuery, subscription: 'foo' }, + subscriptionOptions: [{ label: 'foo', value: 'foo' }], + }, + { + description: 'should update with the first subscription', + query: { ...bareQuery }, + subscriptionOptions: [{ label: 'foo', value: 'foo' }], + onChangeArgs: { + ...bareQuery, + subscription: 'foo', + azureMonitor: { + dimensionFilters: [], + timeGrain: '', + }, + }, + }, + { + description: 'should update with the default subscription if the current subsription does not exists', + query: { ...bareQuery, subscription: 'bar' }, + subscriptionOptions: [{ label: 'foo', value: 'foo' }], + onChangeArgs: { + ...bareQuery, + subscription: 'foo', + azureMonitor: { + dimensionFilters: [], + timeGrain: '', + }, + }, + }, + { + description: 'should clean up if neither the default sub nor the current sub exists', + query: { ...bareQuery, subscription: 'bar' }, + subscriptionOptions: [{ label: 'foo', value: 'foo' }], + defaultSubscription: 'foobar', + onChangeArgs: { + ...bareQuery, + subscription: '', + azureMonitor: { + dimensionFilters: [], + timeGrain: '', + }, + }, + }, + ].forEach((test) => { + it(test.description, () => { + const onChange = jest.fn(); + updateSubscriptions(test.query, test.subscriptionOptions, onChange, test.defaultSubscription); + if (test.onChangeArgs) { + expect(onChange).toHaveBeenCalledWith(test.onChangeArgs); + } else { + expect(onChange).not.toHaveBeenCalled(); + } + }); + }); +}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts index 1b8cea47280..0977360cec1 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts @@ -55,8 +55,38 @@ export function useAsyncState(asyncFn: () => Promise, setError: Function, return finalValue; } -export const useSubscriptions: DataHook = (query, datasource, onChange, setError) => { +export const updateSubscriptions = ( + query: AzureMonitorQuery, + subscriptionOptions: AzureMonitorOption[], + onChange: OnChangeFn, + defaultSubscription?: string +) => { const { subscription } = query; + + // Return early if subscriptions havent loaded, or if the query already has a subscription + if (!subscriptionOptions.length || (subscription && hasOption(subscriptionOptions, subscription))) { + return; + } + + const defaultSub = defaultSubscription || subscriptionOptions[0].value; + + if (!subscription && defaultSub && hasOption(subscriptionOptions, defaultSub)) { + onChange(setSubscriptionID(query, defaultSub)); + } + + // Check if the current subscription is in the list of subscriptions + if (subscription && !hasOption(subscriptionOptions, subscription)) { + if (hasOption(subscriptionOptions, defaultSub)) { + // Use the default sub if is on theh list + onChange(setSubscriptionID(query, defaultSub)); + } else { + // Neither the current subscription nor the defaultSub is on the list, remove it + onChange(setSubscriptionID(query, '')); + } + } +}; + +export const useSubscriptions: DataHook = (query, datasource, onChange, setError) => { const defaultSubscription = datasource.azureMonitorDatasource.defaultSubscriptionId; const subscriptionOptions = useAsyncState( @@ -69,17 +99,8 @@ export const useSubscriptions: DataHook = (query, datasource, onChange, setError ); useEffect(() => { - // Return early if subscriptions havent loaded, or if the query already has a subscription - if (!subscriptionOptions.length || (subscription && hasOption(subscriptionOptions, subscription))) { - return; - } - - const defaultSub = defaultSubscription || subscriptionOptions[0].value; - - if (!subscription && defaultSub && hasOption(subscriptionOptions, defaultSub)) { - onChange(setSubscriptionID(query, defaultSub)); - } - }, [subscriptionOptions, query, subscription, defaultSubscription, onChange]); + updateSubscriptions(query, subscriptionOptions, onChange, defaultSubscription); + }, [subscriptionOptions, query, defaultSubscription, onChange]); return subscriptionOptions; }; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts index 0c9506a8805..108fe67cb1b 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts @@ -11,6 +11,13 @@ export function setSubscriptionID(query: AzureMonitorQuery, subscriptionID: stri azureMonitor: { ...query.azureMonitor, resourceGroup: undefined, + metricDefinition: undefined, + metricNamespace: undefined, + resourceName: undefined, + metricName: undefined, + aggregation: undefined, + timeGrain: '', + dimensionFilters: [], }, }; } @@ -25,7 +32,13 @@ export function setResourceGroup(query: AzureMonitorQuery, resourceGroup: string azureMonitor: { ...query.azureMonitor, resourceGroup: resourceGroup, + metricDefinition: undefined, + metricNamespace: undefined, resourceName: undefined, + metricName: undefined, + aggregation: undefined, + timeGrain: '', + dimensionFilters: [], }, }; } @@ -44,6 +57,9 @@ export function setResourceType(query: AzureMonitorQuery, resourceType: string | resourceName: undefined, metricNamespace: undefined, metricName: undefined, + aggregation: undefined, + timeGrain: '', + dimensionFilters: [], }, }; @@ -60,6 +76,11 @@ export function setResourceName(query: AzureMonitorQuery, resourceName: string | azureMonitor: { ...query.azureMonitor, resourceName: resourceName, + metricNamespace: undefined, + metricName: undefined, + aggregation: undefined, + timeGrain: '', + dimensionFilters: [], }, }; } @@ -75,6 +96,9 @@ export function setMetricNamespace(query: AzureMonitorQuery, metricNamespace: st ...query.azureMonitor, metricNamespace: metricNamespace, metricName: undefined, + aggregation: undefined, + timeGrain: '', + dimensionFilters: [], }, }; } @@ -89,6 +113,9 @@ export function setMetricName(query: AzureMonitorQuery, metricName: string | und azureMonitor: { ...query.azureMonitor, metricName: metricName, + aggregation: undefined, + timeGrain: '', + dimensionFilters: [], }, }; }