From a0c3dcb8c6a0e32973c082e673ef9b4255bd8307 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Tue, 31 Jan 2023 12:54:16 +0100 Subject: [PATCH] Azure Monitor: Fix selection when using a search term (#62562) --- .../MetricsQueryEditor.test.tsx | 11 +++++-- .../ResourcePicker/NestedRow.test.tsx | 16 ++++++++++ .../components/ResourcePicker/NestedRow.tsx | 2 +- .../ResourcePicker/ResourcePicker.test.tsx | 31 +++++++++++++++++- .../ResourcePicker/ResourcePicker.tsx | 7 +++- .../resourcePicker/resourcePickerData.test.ts | 31 ++++++++++++++++++ .../resourcePicker/resourcePickerData.ts | 32 +++++++++++++++++++ 7 files changed, 125 insertions(+), 5 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 debb4c68082..b5ddf15c837 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 @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; @@ -108,6 +108,10 @@ describe('MetricsQueryEditor', () => { expect(resourcePickerButton).toBeInTheDocument(); resourcePickerButton.click(); + await waitFor(() => { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + }); + const selection = await screen.findAllByLabelText('web-server'); expect(selection).toHaveLength(2); }); @@ -412,8 +416,11 @@ describe('MetricsQueryEditor', () => { await userEvent.type(advancedInput, 'def-123'); const updatedCheckboxes = await screen.findAllByLabelText('web-server'); - expect(updatedCheckboxes.length).toBe(1); + expect(updatedCheckboxes.length).toBe(2); + // Unselect the one listed in the rows expect(updatedCheckboxes[0]).not.toBeChecked(); + // But the one in the advanced section should still be selected + expect(updatedCheckboxes[1]).toBeChecked(); }); it('should call onApply with a new subscription when a user types it in the selection box', async () => { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.test.tsx index 47ae0943a4c..683979a438b 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.test.tsx @@ -69,4 +69,20 @@ describe('NestedRow', () => { const box = screen.queryByRole('checkbox'); expect(box).toBeDisabled(); }); + + it('should check a checkbox if the uri matches regardless of the case', () => { + render( + + + + +
+ ); + const box = screen.queryByRole('checkbox'); + expect(box).toBeChecked(); + }); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.tsx index 246e1f193ea..160d77d300e 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRow.tsx @@ -32,7 +32,7 @@ const NestedRow: React.FC = ({ const styles = useStyles2(getStyles); const [rowStatus, setRowStatus] = useState<'open' | 'closed' | 'loading'>('closed'); - const isSelected = !!selectedRows.find((v) => v.uri === row.uri); + const isSelected = !!selectedRows.find((v) => v.uri.toLowerCase() === row.uri.toLowerCase()); const isDisabled = !isSelected && disableRow(row, selectedRows); const isOpen = rowStatus === 'open'; 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 6b4b330882e..4cc55ca2f07 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 @@ -1,4 +1,4 @@ -import { act, render, screen } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { omit } from 'lodash'; import React from 'react'; @@ -105,6 +105,9 @@ describe('AzureMonitor ResourcePicker', () => { it('should show scroll down to a resource and mark it as selected if there is one saved', async () => { render(); + await waitFor(() => { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + }); const resourceCheckboxes = await screen.findAllByLabelText('db-server'); expect(resourceCheckboxes.length).toBe(2); expect(resourceCheckboxes[0]).toBeChecked(); @@ -222,6 +225,9 @@ describe('AzureMonitor ResourcePicker', () => { ]} /> ); + await waitFor(() => { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + }); const checkbox = await screen.findAllByLabelText('web-server'); expect(checkbox).toHaveLength(2); expect(checkbox.at(0)).toBeChecked(); @@ -332,6 +338,29 @@ describe('AzureMonitor ResourcePicker', () => { ); }); + it('display a row for a selected resource even if it is not part of the current rows', async () => { + const resourcePickerData = createMockResourcePickerData([]); + resourcePickerData.fetchInitialRows = jest.fn().mockResolvedValue([]); + render( + + ); + const checkbox = await screen.findAllByLabelText('web-server'); + expect(checkbox).toHaveLength(1); + expect(checkbox.at(0)).toBeChecked(); + }); + describe('when rendering resource picker without any selectable entry types', () => { it('renders no checkboxes', async () => { await act(async () => { 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 475215e0383..863e3bc15c0 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 @@ -88,11 +88,16 @@ const ResourcePicker = ({ const sanitized = internalSelected.filter((r) => isValid(r)); const found = internalSelected && findRows(rows, resourcesToStrings(sanitized)); + if (sanitized?.length > found.length) { + // Not all the selected items are in the current rows, so we need to generate the row + // information for those. + return setSelectedRows(resourcePickerData.parseRows(sanitized)); + } if (found && found.length) { return setSelectedRows(found); } return setSelectedRows([]); - }, [internalSelected, rows]); + }, [internalSelected, rows, resourcePickerData]); // Request resources for an expanded resource group const requestNestedRows = useCallback( diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts index 747486c3cca..634394593e8 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts @@ -6,6 +6,7 @@ import { import createMockDatasource from '../__mocks__/datasource'; import { createMockInstanceSetttings } from '../__mocks__/instanceSettings'; import { mockGetValidLocations } from '../__mocks__/resourcePickerRows'; +import { ResourceRowType } from '../components/ResourcePicker/types'; import { AzureGraphResponse } from '../types'; import ResourcePickerData from './resourcePickerData'; @@ -469,4 +470,34 @@ describe('AzureMonitor resourcePickerData', () => { expect(resourcePickerData.getResourcesForResourceGroup).toBeCalledTimes(1); }); }); + + describe('parseRows', () => { + [ + { + input: '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/web-server', + expected: { + id: 'web-server', + name: 'web-server', + type: ResourceRowType.Resource, + uri: '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/web-server', + typeLabel: 'Virtual machines', + }, + }, + { + input: { + subscription: 'def-456', + }, + expected: { + id: 'def-456', + name: 'def-456', + type: ResourceRowType.Subscription, + uri: '/subscriptions/def-456', + typeLabel: '', + }, + }, + ].forEach(({ input, expected }) => { + const { resourcePickerData } = createResourcePickerData([]); + expect(resourcePickerData.parseRows([input])[0]).toMatchObject(expected); + }); + }); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts index bf05f220b0c..148dbb62fdc 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts @@ -9,6 +9,7 @@ import { ResourceRow, ResourceRowGroup, ResourceRowType } from '../components/Re import { addResources, findRow, + parseMultipleResourceDetails, parseResourceDetails, parseResourceURI, resourceToString, @@ -419,4 +420,35 @@ export default class ResourcePickerData extends DataSourceWithBackend): ResourceRow[] { + const resourceObjs = parseMultipleResourceDetails(resources); + const newSelectedRows: ResourceRow[] = []; + resourceObjs.forEach((resource, i) => { + let id = resource.resourceName; + let name = resource.resourceName; + let rtype = ResourceRowType.Resource; + if (!id) { + id = resource.resourceGroup; + name = resource.resourceGroup; + rtype = ResourceRowType.ResourceGroup; + if (!id) { + id = resource.subscription; + name = resource.subscription; + rtype = ResourceRowType.Subscription; + } + } + newSelectedRows.push({ + id: id ?? '', + name: name ?? '', + type: rtype, + uri: resourceToString(resource), + typeLabel: + resourceTypeDisplayNames[resource.metricNamespace?.toLowerCase() ?? ''] ?? resource.metricNamespace ?? '', + locationDisplayName: this.logLocationsMap.get(resource.region ?? '')?.displayName || resource.region, + location: resource.region, + }); + }); + return newSelectedRows; + } }