From 1b86a496227ee18ae312e715c3040f8b6ae1e3a9 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Tue, 17 Jan 2023 14:11:11 +0100 Subject: [PATCH] AzureMonitor: Adapt ResourcePicker and Advanced components to multiple resources (#61605) --- .../ResourceField/ResourceField.tsx | 5 +- .../ResourcePicker/Advanced.test.tsx | 21 +- .../components/ResourcePicker/Advanced.tsx | 219 ++++++++++-------- .../ResourcePicker/ResourcePicker.test.tsx | 82 +++++-- .../ResourcePicker/ResourcePicker.tsx | 51 ++-- .../components/ResourcePicker/utils.test.ts | 94 +++++++- .../components/ResourcePicker/utils.ts | 23 +- .../resourcePicker/resourcePickerData.test.ts | 57 +++++ .../resourcePicker/resourcePickerData.ts | 40 +++- 9 files changed, 430 insertions(+), 162 deletions(-) diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx index 18e5c3f800d..4098d27b9ac 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx @@ -63,8 +63,9 @@ const ResourceField: React.FC> > resources && handleApply(resources[0])} onCancel={closePicker} selectableEntryTypes={selectableEntryTypes} queryType={queryType} diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx index e9efb20c08d..f4f55d70b0d 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx @@ -7,29 +7,38 @@ import Advanced from './Advanced'; describe('AzureMonitor ResourcePicker', () => { it('should set a parameter as an object', async () => { const onChange = jest.fn(); - const { rerender } = render(); + const { rerender } = render(); const advancedSection = screen.getByText('Advanced'); advancedSection.click(); const subsInput = await screen.findByLabelText('Subscription'); await userEvent.type(subsInput, 'd'); - expect(onChange).toHaveBeenCalledWith({ subscription: 'd' }); + expect(onChange).toHaveBeenCalledWith([{ subscription: 'd' }]); - rerender(); + rerender(); expect(screen.getByLabelText('Subscription').outerHTML).toMatch('value="def-123"'); }); it('should set a parameter as uri', async () => { const onChange = jest.fn(); - const { rerender } = render(); + const { rerender } = render(); const advancedSection = screen.getByText('Advanced'); advancedSection.click(); const subsInput = await screen.findByLabelText('Resource URI'); await userEvent.type(subsInput, '/'); - expect(onChange).toHaveBeenCalledWith('/'); + expect(onChange).toHaveBeenCalledWith(['/']); - rerender(); + rerender(); expect(screen.getByLabelText('Resource URI').outerHTML).toMatch('value="/subscriptions/sub"'); }); + + it('should render multiple resources', async () => { + render(); + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); + + expect(screen.getByDisplayValue('/subscriptions/sub1')).toBeInTheDocument(); + expect(screen.getByDisplayValue('/subscriptions/sub2')).toBeInTheDocument(); + }); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx index 2a6958a50b7..15c20289dd7 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx @@ -7,12 +7,18 @@ import { AzureMetricResource } from '../../types'; import { Space } from '../Space'; interface ResourcePickerProps { - resource: T; - onChange: (resource: T) => void; + resources: T[]; + onChange: (resources: T[]) => void; } -const Advanced = ({ resource, onChange }: ResourcePickerProps) => { - const [isAdvancedOpen, setIsAdvancedOpen] = useState(!!resource && JSON.stringify(resource).includes('$')); +const Advanced = ({ resources, onChange }: ResourcePickerProps) => { + const [isAdvancedOpen, setIsAdvancedOpen] = useState(!!resources.length && JSON.stringify(resources).includes('$')); + + const onResourceChange = (resource: string | AzureMetricResource, index: number) => { + const newResources = [...resources]; + newResources[index] = resource; + onChange(newResources); + }; return (
@@ -22,104 +28,115 @@ const Advanced = ({ resource, onChange }: ResourcePickerProps setIsAdvancedOpen(!isAdvancedOpen)} > - {typeof resource === 'string' ? ( - <> - {' '} - - onChange(event.currentTarget.value)} - placeholder="ex: /subscriptions/$subId" - /> - - ) : ( - <> - - onChange({ ...resource, subscription: event.currentTarget.value })} - placeholder="aaaaaaaa-bbbb-cccc-dddd-eeeeeeee" - /> - - - onChange({ ...resource, resourceGroup: event.currentTarget.value })} - placeholder="resource-group" - /> - - - onChange({ ...resource, metricNamespace: event.currentTarget.value })} - placeholder="Microsoft.Insights/metricNamespaces" - /> - - - onChange({ ...resource, resourceName: event.currentTarget.value })} - placeholder="name" - /> - - - )} + + onResourceChange({ ...resource, resourceName: event.currentTarget.value }, index) + } + placeholder="name" + /> + + + )} +
+ ))} 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 4e2922b4568..6537802765d 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 @@ -17,6 +17,15 @@ import { ResourceRowType } from './types'; import ResourcePicker from '.'; +jest.mock('@grafana/runtime', () => ({ + ...(jest.requireActual('@grafana/runtime') as unknown as object), + getTemplateSrv: () => ({ + replace: (val: string) => { + return val; + }, + }), +})); + const noResourceURI = ''; const singleSubscriptionSelectionURI = '/subscriptions/def-456'; const singleResourceGroupSelectionURI = '/subscriptions/def-456/resourceGroups/dev-3'; @@ -50,7 +59,7 @@ const queryType: ResourcePickerQueryType = 'logs'; const defaultProps = { templateVariables: [], - resource: noResourceURI, + resources: [noResourceURI], resourcePickerData: createMockResourcePickerData(), onCancel: noop, onApply: noop, @@ -68,7 +77,7 @@ describe('AzureMonitor ResourcePicker', () => { window.HTMLElement.prototype.scrollIntoView = jest.fn(); }); it('should pre-load subscriptions when there is no existing selection', async () => { - render(); + render(); const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckbox).toBeInTheDocument(); expect(subscriptionCheckbox).not.toBeChecked(); @@ -77,7 +86,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('should show a subscription as selected if there is one saved', async () => { - render(); + render(); const subscriptionCheckboxes = await screen.findAllByLabelText('Dev Subscription'); expect(subscriptionCheckboxes.length).toBe(2); expect(subscriptionCheckboxes[0]).toBeChecked(); @@ -85,7 +94,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('should show a resourceGroup as selected if there is one saved', async () => { - render(); + render(); const resourceGroupCheckboxes = await screen.findAllByLabelText('A Great Resource Group'); expect(resourceGroupCheckboxes.length).toBe(2); expect(resourceGroupCheckboxes[0]).toBeChecked(); @@ -93,7 +102,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('should show scroll down to a resource and mark it as selected if there is one saved', async () => { - render(); + render(); const resourceCheckboxes = await screen.findAllByLabelText('db-server'); expect(resourceCheckboxes.length).toBe(2); expect(resourceCheckboxes[0]).toBeChecked(); @@ -101,7 +110,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('opens the selected nested resources', async () => { - render(); + render(); const collapseSubscriptionBtn = await screen.findByLabelText('Collapse Dev Subscription'); expect(collapseSubscriptionBtn).toBeInTheDocument(); const collapseResourceGroupBtn = await screen.findByLabelText('Collapse A Great Resource Group'); @@ -109,7 +118,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('scrolls down to the selected resource', async () => { - render(); + render(); await screen.findByLabelText('Collapse A Great Resource Group'); expect(window.HTMLElement.prototype.scrollIntoView).toBeCalledTimes(1); }); @@ -133,12 +142,40 @@ describe('AzureMonitor ResourcePicker', () => { const applyButton = screen.getByRole('button', { name: 'Apply' }); applyButton.click(); expect(onApply).toBeCalledTimes(1); - expect(onApply).toBeCalledWith('/subscriptions/def-123'); + expect(onApply).toBeCalledWith(['/subscriptions/def-123']); + }); + + it('should call onApply removing an 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(); + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith([]); + }); + + it('should call onApply removing an element ignoring the case', async () => { + const onApply = jest.fn(); + render( + + ); + const subscriptionCheckbox = await screen.findAllByLabelText('A Great Resource Group'); + expect(subscriptionCheckbox).toHaveLength(2); + expect(subscriptionCheckbox.at(0)).toBeChecked(); + subscriptionCheckbox.at(0)?.click(); + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith([]); }); it('should call onApply with a new subscription when a user clicks on the checkbox in the row', async () => { const onApply = jest.fn(); - render(); + render(); const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckbox).toBeInTheDocument(); expect(subscriptionCheckbox).not.toBeChecked(); @@ -146,7 +183,20 @@ describe('AzureMonitor ResourcePicker', () => { 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' }]); + }); + + 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(); + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith([]); }); it('should call onApply with a new subscription uri when a user types it in the selection box', async () => { @@ -166,12 +216,12 @@ describe('AzureMonitor ResourcePicker', () => { applyButton.click(); expect(onApply).toBeCalledTimes(1); - expect(onApply).toBeCalledWith('/subscriptions/def-123'); + expect(onApply).toBeCalledWith(['/subscriptions/def-123']); }); 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(); @@ -186,11 +236,11 @@ describe('AzureMonitor ResourcePicker', () => { applyButton.click(); expect(onApply).toBeCalledTimes(1); - expect(onApply).toBeCalledWith({ subscription: 'def-123' }); + expect(onApply).toBeCalledWith([{ subscription: 'def-123' }]); }); it('should show unselect a subscription if the value is manually edited', async () => { - render(); + render(); const subscriptionCheckboxes = await screen.findAllByLabelText('Dev Subscription'); expect(subscriptionCheckboxes.length).toBe(2); expect(subscriptionCheckboxes[0]).toBeChecked(); @@ -264,7 +314,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('resets result when the user clears their search', async () => { - render(); + render(); const subscriptionCheckboxBeforeSearch = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckboxBeforeSearch).toBeInTheDocument(); @@ -295,7 +345,7 @@ describe('AzureMonitor ResourcePicker', () => { {...defaultProps} queryType={'metrics'} resourcePickerData={resourcePickerData} - resource={noResourceURI} + resources={[noResourceURI]} /> ); const subscriptionExpand = await screen.findByLabelText('Expand Primary Subscription'); 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 5a45f394632..aa9a5ffbb61 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 @@ -15,21 +15,21 @@ import NestedRow from './NestedRow'; import Search from './Search'; import getStyles from './styles'; import { ResourceRow, ResourceRowGroup, ResourceRowType } from './types'; -import { findRow, parseResourceDetails, resourceToString } from './utils'; +import { findRows, parseMultipleResourceDetails, resourcesToStrings, matchURI, resourceToString } from './utils'; interface ResourcePickerProps { resourcePickerData: ResourcePickerData; - resource: T; + resources: T[]; selectableEntryTypes: ResourceRowType[]; queryType: ResourcePickerQueryType; - onApply: (resource?: T) => void; + onApply: (resources: T[]) => void; onCancel: () => void; } const ResourcePicker = ({ resourcePickerData, - resource, + resources, onApply, onCancel, selectableEntryTypes, @@ -40,14 +40,14 @@ const ResourcePicker = ({ const [isLoading, setIsLoading] = useState(false); const [rows, setRows] = useState([]); const [selectedRows, setSelectedRows] = useState([]); - const [internalSelected, setInternalSelected] = useState(resource); + const [internalSelected, setInternalSelected] = useState(resources); const [errorMessage, setErrorMessage] = useState(undefined); const [shouldShowLimitFlag, setShouldShowLimitFlag] = useState(false); // Sync the resourceURI prop to internal state useEffect(() => { - setInternalSelected(resource); - }, [resource]); + setInternalSelected(resources); + }, [resources]); const loadInitialData = useCallback(async () => { if (!isLoading) { @@ -55,7 +55,7 @@ const ResourcePicker = ({ setIsLoading(true); const resources = await resourcePickerData.fetchInitialRows( queryType, - parseResourceDetails(internalSelected ?? {}) + parseMultipleResourceDetails(internalSelected ?? {}) ); setRows(resources); } catch (error) { @@ -75,14 +75,9 @@ const ResourcePicker = ({ setSelectedRows([]); } - const found = internalSelected && findRow(rows, resourceToString(internalSelected)); - if (found) { - return setSelectedRows([ - { - ...found, - children: undefined, - }, - ]); + const found = internalSelected && findRows(rows, resourcesToStrings(internalSelected)); + if (found && found.length) { + return setSelectedRows(found); } return setSelectedRows([]); }, [internalSelected, rows]); @@ -109,19 +104,29 @@ const ResourcePicker = ({ [resourcePickerData, rows, queryType] ); - const resourceIsString = typeof resource === 'string'; + const resourceIsString = resources?.length && typeof resources[0] === 'string'; const handleSelectionChanged = useCallback( (row: ResourceRow, isSelected: boolean) => { - isSelected - ? setInternalSelected(resourceIsString ? row.uri : parseResourceDetails(row.uri, row.location)) - : setInternalSelected(resourceIsString ? '' : {}); + 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; + }); + setInternalSelected(newSelected); + } else { + const newInternalSelected = internalSelected?.filter((r) => { + return !matchURI(resourceToString(r), row.uri); + }); + setInternalSelected(newInternalSelected); + } }, - [resourceIsString] + [resourceIsString, internalSelected, setInternalSelected] ); const handleApply = useCallback(() => { if (internalSelected) { - onApply(resourceIsString ? internalSelected : parseResourceDetails(internalSelected)); + onApply(resourceIsString ? internalSelected : parseMultipleResourceDetails(internalSelected)); } }, [resourceIsString, internalSelected, onApply]); @@ -230,7 +235,7 @@ const ResourcePicker = ({ )} - setInternalSelected(r)} /> + setInternalSelected(r)} />