From 3e73ba54602fb42a18328f1985b1e920fa2e2770 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Tue, 24 Jan 2023 15:44:48 +0100 Subject: [PATCH] Azure Monitor: Adapt Advanced component to multiple resources (#61981) --- .../AdvancedResourcePicker.test.tsx | 50 ++++++ .../AdvancedResourcePicker.tsx | 97 +++++++++++ .../LogsQueryEditor/LogsQueryEditor.tsx | 7 + .../AdvancedResourcePicker.test.tsx | 104 +++++++++++ .../AdvancedResourcePicker.tsx | 163 ++++++++++++++++++ .../MetricsQueryEditor/MetricsQueryEditor.tsx | 9 +- .../ResourceField/ResourceField.tsx | 3 + .../ResourcePicker/AdvancedMulti.test.tsx | 16 ++ .../ResourcePicker/AdvancedMulti.tsx | 33 ++++ .../ResourcePicker/ResourcePicker.test.tsx | 97 ++++++++--- .../ResourcePicker/ResourcePicker.tsx | 38 ++-- .../components/ResourcePicker/utils.test.ts | 29 ++++ .../components/ResourcePicker/utils.ts | 10 +- .../e2e/selectors.ts | 3 + 14 files changed, 622 insertions(+), 37 deletions(-) create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.test.tsx create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.tsx create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.test.tsx create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.tsx create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.test.tsx create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.tsx diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.test.tsx new file mode 100644 index 00000000000..91eaa57842d --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.test.tsx @@ -0,0 +1,50 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; + +import AdvancedResourcePicker from './AdvancedResourcePicker'; + +describe('AdvancedResourcePicker', () => { + it('should set a parameter as an object', async () => { + const onChange = jest.fn(); + const { rerender } = render(); + + const subsInput = await screen.findByTestId('input-advanced-resource-picker-1'); + await userEvent.type(subsInput, 'd'); + expect(onChange).toHaveBeenCalledWith(['d']); + + rerender(); + expect(screen.getByDisplayValue('/subscriptions/def-123')).toBeInTheDocument(); + }); + + it('should initialize with an empty resource', () => { + const onChange = jest.fn(); + render(); + expect(onChange).toHaveBeenCalledWith(['']); + }); + + it('should add a resource', async () => { + const onChange = jest.fn(); + render(); + const addButton = await screen.findByText('Add resource URI'); + addButton.click(); + expect(onChange).toHaveBeenCalledWith(['/subscriptions/def-123', '']); + }); + + it('should remove a resource', async () => { + const onChange = jest.fn(); + render(); + const removeButton = await screen.findByTestId('remove-resource'); + removeButton.click(); + expect(onChange).toHaveBeenCalledWith([]); + }); + + it('should render multiple resources', async () => { + render( + + ); + + expect(screen.getByDisplayValue('/subscriptions/def-123')).toBeInTheDocument(); + expect(screen.getByDisplayValue('/subscriptions/def-456')).toBeInTheDocument(); + }); +}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.tsx new file mode 100644 index 00000000000..fc5d7347507 --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/AdvancedResourcePicker.tsx @@ -0,0 +1,97 @@ +import { css } from '@emotion/css'; +import React, { useEffect } from 'react'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { AccessoryButton } from '@grafana/experimental'; +import { Icon, Input, Tooltip, Label, Button, useStyles2 } from '@grafana/ui'; + +export interface ResourcePickerProps { + resources: T[]; + onChange: (resources: T[]) => void; +} + +const getStyles = (theme: GrafanaTheme2) => ({ + resourceList: css({ width: '100%', display: 'flex', marginBlock: theme.spacing(1) }), +}); + +const AdvancedResourcePicker = ({ resources, onChange }: ResourcePickerProps) => { + const styles = useStyles2(getStyles); + + useEffect(() => { + // Ensure there is at least one resource + if (resources.length === 0) { + onChange(['']); + } + }, [resources, onChange]); + + const onResourceChange = (index: number, resource: string) => { + const newResources = [...resources]; + newResources[index] = resource; + onChange(newResources); + }; + + const removeResource = (index: number) => { + const newResources = [...resources]; + newResources.splice(index, 1); + onChange(newResources); + }; + + const addResource = () => { + onChange(resources.concat('')); + }; + + return ( + <> + + {resources.map((resource, index) => ( +
+
+ onResourceChange(index, event.currentTarget.value)} + placeholder="ex: /subscriptions/$subId" + data-testid={`input-advanced-resource-picker-${index + 1}`} + /> + removeResource(index)} + data-testid={`remove-resource`} + hidden={resources.length === 1} + /> +
+
+ ))} + + + ); +}; + +export default AdvancedResourcePicker; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx index 9513ee1f6d3..b1c7d95d64c 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx @@ -10,6 +10,7 @@ import ResourceField from '../ResourceField'; import { ResourceRow, ResourceRowGroup, ResourceRowType } from '../ResourcePicker/types'; import { parseResourceDetails } from '../ResourcePicker/utils'; +import AdvancedResourcePicker from './AdvancedResourcePicker'; import FormatAsField from './FormatAsField'; import QueryField from './QueryField'; import useMigrations from './useMigrations'; @@ -75,6 +76,12 @@ const LogsQueryEditor: React.FC = ({ resources={query.azureLogAnalytics?.resources ?? []} queryType="logs" disableRow={disableRow} + renderAdvanced={(resources, onChange) => ( + // It's required to cast resources because the resource picker + // specifies the type to string | AzureMetricResource. + // eslint-disable-next-line + + )} /> diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.test.tsx new file mode 100644 index 00000000000..57ff4eaa53a --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.test.tsx @@ -0,0 +1,104 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; + +import AdvancedResourcePicker from './AdvancedResourcePicker'; + +describe('AdvancedResourcePicker', () => { + it('should set a parameter as an object', async () => { + const onChange = jest.fn(); + const { rerender } = render(); + + const subsInput = await screen.findByLabelText('Subscription'); + await userEvent.type(subsInput, 'd'); + expect(onChange).toHaveBeenCalledWith([{ subscription: 'd' }]); + + rerender(); + expect(screen.getByLabelText('Subscription').outerHTML).toMatch('value="def-123"'); + }); + + it('should initialize with an empty resource', () => { + const onChange = jest.fn(); + render(); + expect(onChange).toHaveBeenCalledWith([{}]); + }); + + it('should add a resource', async () => { + const onChange = jest.fn(); + render(); + const addButton = await screen.findByText('Add resource'); + addButton.click(); + expect(onChange).toHaveBeenCalledWith([ + { subscription: 'def-123' }, + { subscription: 'def-123', resourceGroup: '', resourceName: '' }, + ]); + }); + + it('should remove a resource', async () => { + const onChange = jest.fn(); + render(); + const removeButton = await screen.findByTestId('remove-resource'); + removeButton.click(); + expect(onChange).toHaveBeenCalledWith([]); + }); + + it('should update all resources when editing the subscription', async () => { + const onChange = jest.fn(); + render( + + ); + const subsInput = await screen.findByLabelText('Subscription'); + await userEvent.type(subsInput, 'd'); + expect(onChange).toHaveBeenCalledWith([{ subscription: 'def-123d' }, { subscription: 'def-123d' }]); + }); + + it('should update all resources when editing the namespace', async () => { + const onChange = jest.fn(); + render( + + ); + const subsInput = await screen.findByLabelText('Namespace'); + await userEvent.type(subsInput, 'b'); + expect(onChange).toHaveBeenCalledWith([{ metricNamespace: 'aab' }, { metricNamespace: 'aab' }]); + }); + + it('should update all resources when editing the region', async () => { + const onChange = jest.fn(); + render(); + const subsInput = await screen.findByLabelText('Region'); + await userEvent.type(subsInput, 'b'); + expect(onChange).toHaveBeenCalledWith([{ region: 'aab' }, { region: 'aab' }]); + }); + + it('should render multiple resources', async () => { + render( + + ); + + expect(screen.getByDisplayValue('sub1')).toBeInTheDocument(); + expect(screen.getByDisplayValue('ns1')).toBeInTheDocument(); + expect(screen.getByDisplayValue('rg1')).toBeInTheDocument(); + expect(screen.getByDisplayValue('res1')).toBeInTheDocument(); + expect(screen.getByDisplayValue('rg2')).toBeInTheDocument(); + expect(screen.getByDisplayValue('res2')).toBeInTheDocument(); + }); +}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.tsx new file mode 100644 index 00000000000..94bc7c88823 --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AdvancedResourcePicker.tsx @@ -0,0 +1,163 @@ +import { css } from '@emotion/css'; +import React, { useEffect } from 'react'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { AccessoryButton } from '@grafana/experimental'; +import { Input, Label, InlineField, Button, useStyles2 } from '@grafana/ui'; + +import { selectors } from '../../e2e/selectors'; +import { AzureMetricResource } from '../../types'; + +export interface ResourcePickerProps { + resources: T[]; + onChange: (resources: T[]) => void; +} + +const getStyles = (theme: GrafanaTheme2) => ({ + resourceList: css({ display: 'flex', columnGap: theme.spacing(1), flexWrap: 'wrap', marginBottom: theme.spacing(1) }), + resource: css({ flex: '0 0 auto' }), + resourceLabel: css({ padding: theme.spacing(1) }), + resourceGroupAndName: css({ display: 'flex', columnGap: theme.spacing(0.5) }), +}); + +const AdvancedResourcePicker = ({ resources, onChange }: ResourcePickerProps) => { + const styles = useStyles2(getStyles); + + useEffect(() => { + // Ensure there is at least one resource + if (resources.length === 0) { + onChange([{}]); + } + }, [resources, onChange]); + + const onResourceChange = (index: number, resource: AzureMetricResource) => { + const newResources = [...resources]; + newResources[index] = resource; + onChange(newResources); + }; + + const removeResource = (index: number) => { + const newResources = [...resources]; + newResources.splice(index, 1); + onChange(newResources); + }; + + const addResource = () => { + onChange( + resources.concat({ + subscription: resources[0]?.subscription, + metricNamespace: resources[0]?.metricNamespace, + resourceGroup: '', + resourceName: '', + }) + ); + }; + + const onCommonPropChange = (r: Partial) => { + onChange(resources.map((resource) => ({ ...resource, ...r }))); + }; + + return ( + <> + + onCommonPropChange({ subscription: event.currentTarget.value })} + placeholder="aaaaaaaa-bbbb-cccc-dddd-eeeeeeee" + /> + + + onCommonPropChange({ metricNamespace: event.currentTarget.value })} + placeholder="Microsoft.Insights/metricNamespaces" + /> + + + onCommonPropChange({ region: event.currentTarget.value })} + placeholder="northeurope" + /> + +
+ {resources.map((resource, index) => ( +
+ {resources.length !== 1 && } + +
+ + onResourceChange(index, { ...resource, resourceGroup: event.currentTarget.value }) + } + placeholder="resource-group" + /> + removeResource(index)} + hidden={resources.length === 1} + data-testid={'remove-resource'} + /> +
+ + + + onResourceChange(index, { ...resource, resourceName: event.currentTarget.value })} + placeholder="name" + /> + +
+ ))} +
+ + + ); +}; + +export default AdvancedResourcePicker; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx index 262840b382d..ec2e079fbb5 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx @@ -6,11 +6,12 @@ import { config } from '@grafana/runtime'; import { multiResourceCompatibleTypes } from '../../azureMetadata'; import type Datasource from '../../datasource'; -import type { AzureMonitorQuery, AzureMonitorOption, AzureMonitorErrorish } from '../../types'; +import type { AzureMonitorQuery, AzureMonitorOption, AzureMonitorErrorish, AzureMetricResource } from '../../types'; import ResourceField from '../ResourceField'; import { ResourceRow, ResourceRowGroup, ResourceRowType } from '../ResourcePicker/types'; import { parseResourceDetails } from '../ResourcePicker/utils'; +import AdvancedResourcePicker from './AdvancedResourcePicker'; import AggregationField from './AggregationField'; import DimensionFields from './DimensionFields'; import LegendFormatField from './LegendFormatField'; @@ -88,6 +89,12 @@ const MetricsQueryEditor: React.FC = ({ resources={resources ?? []} queryType={'metrics'} disableRow={disableRow} + renderAdvanced={(resources, onChange) => ( + // It's required to cast resources because the resource picker + // specifies the type to string | AzureMetricResource. + // eslint-disable-next-line + + )} /> extends AzureQueryEditorFieldProps { inlineField?: boolean; labelWidth?: number; disableRow: (row: ResourceRow, selectedRows: ResourceRowGroup) => boolean; + renderAdvanced: (resources: T[], onChange: (resources: T[]) => void) => React.ReactNode; } const ResourceField: React.FC> = ({ @@ -32,6 +33,7 @@ const ResourceField: React.FC> inlineField, labelWidth, disableRow, + renderAdvanced, }) => { const styles = useStyles2(getStyles); const [pickerIsOpen, setPickerIsOpen] = useState(false); @@ -71,6 +73,7 @@ const ResourceField: React.FC> selectableEntryTypes={selectableEntryTypes} queryType={queryType} disableRow={disableRow} + renderAdvanced={renderAdvanced} /> diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.test.tsx new file mode 100644 index 00000000000..d6a5d9e2cce --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.test.tsx @@ -0,0 +1,16 @@ +import { render, screen } from '@testing-library/react'; +import React from 'react'; + +import AdvancedMulti from './AdvancedMulti'; + +describe('AdvancedMulti', () => { + it('should expand and render a section', async () => { + const onChange = jest.fn(); + const renderAdvanced = jest.fn().mockReturnValue(
details!
); + render(); + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); + + expect(await screen.findByText('details!')).toBeInTheDocument(); + }); +}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.tsx new file mode 100644 index 00000000000..9c7b9b35e51 --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/AdvancedMulti.tsx @@ -0,0 +1,33 @@ +import React, { useState } from 'react'; + +import { Collapse } from '@grafana/ui'; + +import { selectors } from '../../e2e/selectors'; +import { AzureMetricResource } from '../../types'; +import { Space } from '../Space'; + +export interface ResourcePickerProps { + resources: T[]; + onChange: (resources: T[]) => void; + renderAdvanced: (resources: T[], onChange: (resources: T[]) => void) => React.ReactNode; +} + +const AdvancedMulti = ({ resources, onChange, renderAdvanced }: ResourcePickerProps) => { + const [isAdvancedOpen, setIsAdvancedOpen] = useState(!!resources.length && JSON.stringify(resources).includes('$')); + + return ( +
+ setIsAdvancedOpen(!isAdvancedOpen)} + > + {renderAdvanced(resources, onChange)} + + +
+ ); +}; + +export default AdvancedMulti; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx index 0c201f583ab..3bad6927b27 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx @@ -59,7 +59,7 @@ const queryType: ResourcePickerQueryType = 'logs'; const defaultProps = { templateVariables: [], - resources: [noResourceURI], + resources: [], resourcePickerData: createMockResourcePickerData(), onCancel: noop, onApply: noop, @@ -71,6 +71,7 @@ const defaultProps = { ], queryType, disableRow: jest.fn(), + renderAdvanced: jest.fn(), }; describe('AzureMonitor ResourcePicker', () => { @@ -141,6 +142,7 @@ describe('AzureMonitor ResourcePicker', () => { expect(subscriptionCheckbox).not.toBeChecked(); subscriptionCheckbox.click(); const applyButton = screen.getByRole('button', { name: 'Apply' }); + expect(applyButton).toBeEnabled(); applyButton.click(); expect(onApply).toBeCalledTimes(1); expect(onApply).toBeCalledWith(['/subscriptions/def-123']); @@ -174,26 +176,56 @@ describe('AzureMonitor ResourcePicker', () => { expect(onApply).toBeCalledWith([]); }); - it('should call onApply with a new subscription when a user clicks on the checkbox in the row', async () => { + it('should call onApply with a new resource when a user clicks on the checkbox in the row', async () => { const onApply = jest.fn(); - render(); - const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); - expect(subscriptionCheckbox).toBeInTheDocument(); - expect(subscriptionCheckbox).not.toBeChecked(); - subscriptionCheckbox.click(); + render(); + + const subscriptionButton = await screen.findByRole('button', { name: 'Expand Primary Subscription' }); + expect(subscriptionButton).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Expand A Great Resource Group' })).not.toBeInTheDocument(); + subscriptionButton.click(); + + const resourceGroupButton = await screen.findByRole('button', { name: 'Expand A Great Resource Group' }); + resourceGroupButton.click(); + const checkbox = await screen.findByLabelText('web-server'); + await userEvent.click(checkbox); + expect(checkbox).toBeChecked(); const applyButton = screen.getByRole('button', { name: 'Apply' }); applyButton.click(); + expect(onApply).toBeCalledTimes(1); - expect(onApply).toBeCalledWith([{ subscription: 'def-123' }]); + expect(onApply).toBeCalledWith([ + { + metricNamespace: 'Microsoft.Compute/virtualMachines', + region: 'northeurope', + resourceGroup: 'dev-3', + resourceName: 'web-server', + subscription: 'def-456', + }, + ]); }); it('should call onApply removing a resource element', async () => { const onApply = jest.fn(); - render(); - const subscriptionCheckbox = await screen.findAllByLabelText('Primary Subscription'); - expect(subscriptionCheckbox).toHaveLength(2); - expect(subscriptionCheckbox.at(0)).toBeChecked(); - subscriptionCheckbox.at(0)?.click(); + render( + + ); + const checkbox = await screen.findAllByLabelText('web-server'); + expect(checkbox).toHaveLength(2); + expect(checkbox.at(0)).toBeChecked(); + checkbox.at(0)?.click(); const applyButton = screen.getByRole('button', { name: 'Apply' }); applyButton.click(); expect(onApply).toBeCalledTimes(1); @@ -202,7 +234,7 @@ describe('AzureMonitor ResourcePicker', () => { it('should call onApply with a new subscription uri when a user types it in the selection box', async () => { const onApply = jest.fn(); - render(); + render(); const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckbox).toBeInTheDocument(); expect(subscriptionCheckbox).not.toBeChecked(); @@ -222,7 +254,7 @@ describe('AzureMonitor ResourcePicker', () => { it('should call onApply with a new subscription when a user types it in the selection box', async () => { const onApply = jest.fn(); - render(); + render(); const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckbox).toBeInTheDocument(); expect(subscriptionCheckbox).not.toBeChecked(); @@ -232,20 +264,41 @@ describe('AzureMonitor ResourcePicker', () => { const advancedInput = await screen.findByLabelText('Subscription'); await userEvent.type(advancedInput, 'def-123'); + const nsInput = await screen.findByLabelText('Namespace'); + await userEvent.type(nsInput, 'ns'); + const rgInput = await screen.findByLabelText('Resource Group'); + await userEvent.type(rgInput, 'rg'); + const rnInput = await screen.findByLabelText('Resource Name'); + await userEvent.type(rnInput, 'rn'); const applyButton = screen.getByRole('button', { name: 'Apply' }); applyButton.click(); expect(onApply).toBeCalledTimes(1); - expect(onApply).toBeCalledWith([{ subscription: 'def-123' }]); + expect(onApply).toBeCalledWith([ + { subscription: 'def-123', metricNamespace: 'ns', resourceGroup: 'rg', resourceName: 'rn' }, + ]); }); it('should show unselect a subscription if the value is manually edited', async () => { - render(); - const subscriptionCheckboxes = await screen.findAllByLabelText('Dev Subscription'); - expect(subscriptionCheckboxes.length).toBe(2); - expect(subscriptionCheckboxes[0]).toBeChecked(); - expect(subscriptionCheckboxes[1]).toBeChecked(); + render( + + ); + const checkboxes = await screen.findAllByLabelText('web-server'); + expect(checkboxes.length).toBe(2); + expect(checkboxes[0]).toBeChecked(); + expect(checkboxes[1]).toBeChecked(); const advancedSection = screen.getByText('Advanced'); advancedSection.click(); @@ -253,7 +306,7 @@ describe('AzureMonitor ResourcePicker', () => { const advancedInput = await screen.findByLabelText('Subscription'); await userEvent.type(advancedInput, 'def-123'); - const updatedCheckboxes = await screen.findAllByLabelText('Dev Subscription'); + const updatedCheckboxes = await screen.findAllByLabelText('web-server'); expect(updatedCheckboxes.length).toBe(1); expect(updatedCheckboxes[0]).not.toBeChecked(); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx index 6eb36f5776f..c99550162c3 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx @@ -2,6 +2,7 @@ import { cx } from '@emotion/css'; import React, { useCallback, useEffect, useState } from 'react'; import { useEffectOnce } from 'react-use'; +import { config } from '@grafana/runtime'; import { Alert, Button, LoadingPlaceholder, useStyles2 } from '@grafana/ui'; import { selectors } from '../../e2e/selectors'; @@ -11,6 +12,7 @@ import messageFromError from '../../utils/messageFromError'; import { Space } from '../Space'; import Advanced from './Advanced'; +import AdvancedMulti from './AdvancedMulti'; import NestedRow from './NestedRow'; import Search from './Search'; import getStyles from './styles'; @@ -26,6 +28,7 @@ interface ResourcePickerProps { onApply: (resources: T[]) => void; onCancel: () => void; disableRow: (row: ResourceRow, selectedRows: ResourceRowGroup) => boolean; + renderAdvanced: (resources: T[], onChange: (resources: T[]) => void) => React.ReactNode; } const ResourcePicker = ({ @@ -36,6 +39,7 @@ const ResourcePicker = ({ selectableEntryTypes, queryType, disableRow, + renderAdvanced, }: ResourcePickerProps) => { const styles = useStyles2(getStyles); @@ -71,13 +75,18 @@ const ResourcePicker = ({ loadInitialData(); }); + // Avoid using empty resources + const isValid = (r: string | AzureMetricResource) => + typeof r === 'string' ? r !== '' : r.subscription && r.resourceGroup && r.resourceName && r.metricNamespace; + // set selected row data whenever row or selection changes useEffect(() => { if (!internalSelected) { setSelectedRows([]); } - const found = internalSelected && findRows(rows, resourcesToStrings(internalSelected)); + const sanitized = internalSelected.filter((r) => isValid(r)); + const found = internalSelected && findRows(rows, resourcesToStrings(sanitized)); if (found && found.length) { return setSelectedRows(found); } @@ -106,15 +115,11 @@ const ResourcePicker = ({ [resourcePickerData, rows, queryType] ); - const resourceIsString = resources?.length && typeof resources[0] === 'string'; const handleSelectionChanged = useCallback( (row: ResourceRow, isSelected: boolean) => { if (isSelected) { - const newRes = resourceIsString ? row.uri : parseMultipleResourceDetails([row.uri], row.location)[0]; - const newSelected = (internalSelected ? internalSelected.concat(newRes) : [newRes]).filter((r) => { - // avoid setting empty resources - return typeof r === 'string' ? r !== '' : r.subscription; - }); + const newRes = queryType === 'logs' ? row.uri : parseMultipleResourceDetails([row.uri], row.location)[0]; + const newSelected = internalSelected ? internalSelected.concat(newRes) : [newRes]; setInternalSelected(newSelected); } else { const newInternalSelected = internalSelected?.filter((r) => { @@ -123,14 +128,14 @@ const ResourcePicker = ({ setInternalSelected(newInternalSelected); } }, - [resourceIsString, internalSelected, setInternalSelected] + [queryType, internalSelected, setInternalSelected] ); const handleApply = useCallback(() => { if (internalSelected) { - onApply(resourceIsString ? internalSelected : parseMultipleResourceDetails(internalSelected)); + onApply(queryType === 'logs' ? internalSelected : parseMultipleResourceDetails(internalSelected)); } - }, [resourceIsString, internalSelected, onApply]); + }, [queryType, internalSelected, onApply]); const handleSearch = useCallback( async (searchWord: string) => { @@ -239,11 +244,20 @@ const ResourcePicker = ({ )} - setInternalSelected(r)} /> + {config.featureToggles.azureMultipleResourcePicker ? ( + setInternalSelected(r)} + renderAdvanced={renderAdvanced} + /> + ) : ( + setInternalSelected(r)} /> + )} +