From dd49f9d182ec6de9e34c6d7e60fc384ad1c91a05 Mon Sep 17 00:00:00 2001 From: Sarah Zinger Date: Wed, 23 Mar 2022 09:38:46 -0400 Subject: [PATCH] Azure Monitor: Small bug fixes for Resource Picker (#46665) - fixes issue with checkbox styling - fixes issue with selecting subscriptions --- .../__mocks__/datasource.ts | 1 - .../__mocks__/resourcePickerData.ts | 1 - .../__mocks__/resourcePickerRows.ts | 149 ++------- .../NestedResourceTable.test.tsx | 108 ------- .../components/ResourcePicker/NestedRows.tsx | 13 +- .../ResourcePicker/ResourcePicker.test.tsx | 222 +++++++------ .../ResourcePicker/ResourcePicker.tsx | 49 ++- .../components/ResourcePicker/styles.ts | 4 + .../components/ResourcePicker/types.ts | 3 +- .../components/ResourcePicker/utils.ts | 10 +- .../resourcePicker/resourcePickerData.test.ts | 297 ++++++++++++------ .../resourcePicker/resourcePickerData.ts | 81 +++-- 12 files changed, 448 insertions(+), 490 deletions(-) delete mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedResourceTable.test.tsx diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/datasource.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/datasource.ts index ebeeb707e62..23492d50d3f 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/datasource.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/datasource.ts @@ -41,7 +41,6 @@ export default function createMockDatasource(overrides?: DeepPartial getResourceGroupsBySubscriptionId: jest.fn().mockResolvedValue([]), getResourcesForResourceGroup: jest.fn().mockResolvedValue([]), getResourceURIFromWorkspace: jest.fn().mockReturnValue(''), - transformVariablesToRow: jest.fn().mockReturnValue({}), }, ...overrides, }; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/resourcePickerData.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/resourcePickerData.ts index f3a59ecf533..9d963262bd7 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/resourcePickerData.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/resourcePickerData.ts @@ -10,7 +10,6 @@ export default function createMockResourcePickerData(overrides?: DeepPartial [ - { - id: '/subscriptions/abc-123', - name: 'Primary Subscription', - type: ResourceRowType.Subscription, - typeLabel: 'Subscription', - children: [ - { - id: '/subscriptions/abc-123/resourceGroups/prod', - name: 'Production', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [], - }, - { - id: '/subscriptions/abc-123/resourceGroups/pre-prod', - name: 'Pre-production', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [], - }, - ], - }, - - { - id: '/subscriptions/def-456', - name: 'Dev Subscription', - type: ResourceRowType.Subscription, - typeLabel: 'Subscription', - children: [ - { - id: '/subscriptions/def-456/resourceGroups/dev', - name: 'Development', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [ - { - id: '/subscription/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/web-server', - name: 'web-server', - typeLabel: 'Microsoft.Compute/virtualMachines', - type: ResourceRowType.Resource, - location: 'northeurope', - }, - { - id: '/subscription/def-456/resourceGroups/dev/providers/Microsoft.Compute/disks/web-server_DataDisk', - name: 'web-server_DataDisk', - typeLabel: 'Microsoft.Compute/disks', - type: ResourceRowType.Resource, - location: 'northeurope', - }, - - { - id: '/subscription/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/db-server', - name: 'db-server', - typeLabel: 'Microsoft.Compute/virtualMachines', - type: ResourceRowType.Resource, - location: 'northeurope', - }, - - { - id: '/subscription/def-456/resourceGroups/dev/providers/Microsoft.Compute/disks/db-server_DataDisk', - name: 'db-server_DataDisk', - typeLabel: 'Microsoft.Compute/disks', - type: ResourceRowType.Resource, - location: 'northeurope', - }, - ], - }, - { - id: '/subscriptions/def-456/resourceGroups/test', - name: 'Test', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [], - }, - { - id: '/subscriptions/def-456/resourceGroups/qa', - name: 'QA', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [], - }, - ], - }, - - { - id: '$$grafana-templateVariables$$', - name: 'Template variables', - type: ResourceRowType.VariableGroup, - typeLabel: 'Variables', - children: [ - { - id: '$machine', - name: '$machine', - type: ResourceRowType.Variable, - typeLabel: 'Variable', - }, - { - id: '$workspace', - name: '$workspace', - type: ResourceRowType.Variable, - typeLabel: 'Variable', - }, - ], - }, -]; - export const createMockSubscriptions = (): ResourceRowGroup => [ { id: 'def-123', + uri: '/subscriptions/def-123', name: 'Primary Subscription', type: ResourceRowType.Subscription, typeLabel: 'Subscription', @@ -117,6 +11,7 @@ export const createMockSubscriptions = (): ResourceRowGroup => [ }, { id: 'def-456', + uri: '/subscriptions/def-456', name: 'Dev Subscription', type: ResourceRowType.Subscription, typeLabel: 'Subscription', @@ -124,6 +19,7 @@ export const createMockSubscriptions = (): ResourceRowGroup => [ }, { id: 'def-789', + uri: '/subscriptions/def-789', name: 'Test Subscription', type: ResourceRowType.Subscription, typeLabel: 'Subscription', @@ -133,35 +29,40 @@ export const createMockSubscriptions = (): ResourceRowGroup => [ export const createMockResourceGroupsBySubscription = (): ResourceRowGroup => [ { - id: '/subscriptions/def-456/resourceGroups/dev-1', + id: 'dev-1', + uri: '/subscriptions/def-456/resourceGroups/dev-1', name: 'Development', type: ResourceRowType.ResourceGroup, typeLabel: 'Resource Group', children: [], }, { - id: '/subscriptions/def-456/resourceGroups/dev-2', + id: 'dev-2', + uri: '/subscriptions/def-456/resourceGroups/dev-2', name: 'Development', type: ResourceRowType.ResourceGroup, typeLabel: 'Resource Group', children: [], }, { - id: '/subscriptions/def-456/resourceGroups/dev-3', + id: 'dev-3', + uri: '/subscriptions/def-456/resourceGroups/dev-3', + name: 'A Great Resource Group', + type: ResourceRowType.ResourceGroup, + typeLabel: 'Resource Group', + children: [], + }, + { + id: 'dev-4', + uri: '/subscriptions/def-456/resourceGroups/dev-4', name: 'Development', type: ResourceRowType.ResourceGroup, typeLabel: 'Resource Group', children: [], }, { - id: '/subscriptions/def-456/resourceGroups/dev-4', - name: 'Development', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [], - }, - { - id: '/subscriptions/def-456/resourceGroups/dev-5', + id: 'dev-5', + uri: '/subscriptions/def-456/resourceGroups/dev-5', name: 'Development', type: ResourceRowType.ResourceGroup, typeLabel: 'Resource Group', @@ -171,14 +72,16 @@ export const createMockResourceGroupsBySubscription = (): ResourceRowGroup => [ export const mockResourcesByResourceGroup = (): ResourceRowGroup => [ { - id: 'Microsoft.Compute/virtualMachines/web-server', + id: 'web-server', + uri: '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/virtualMachines/web-server', name: 'web-server', typeLabel: 'Microsoft.Compute/virtualMachines', type: ResourceRowType.Resource, location: 'northeurope', }, { - id: 'Microsoft.Compute/disks/web-server_DataDisk', + id: 'web-server_DataDisk', + uri: '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/disks/web-server_DataDisk', name: 'web-server_DataDisk', typeLabel: 'Microsoft.Compute/disks', type: ResourceRowType.Resource, @@ -186,7 +89,8 @@ export const mockResourcesByResourceGroup = (): ResourceRowGroup => [ }, { - id: 'Microsoft.Compute/virtualMachines/db-server', + id: 'db-server', + uri: '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/virtualMachines/db-server', name: 'db-server', typeLabel: 'Microsoft.Compute/virtualMachines', type: ResourceRowType.Resource, @@ -194,7 +98,8 @@ export const mockResourcesByResourceGroup = (): ResourceRowGroup => [ }, { - id: 'Microsoft.Compute/disks/db-server_DataDisk', + id: 'db-server_DataDisk', + uri: '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/disks/db-server_DataDisk', name: 'db-server_DataDisk', typeLabel: 'Microsoft.Compute/disks', type: ResourceRowType.Resource, diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedResourceTable.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedResourceTable.test.tsx deleted file mode 100644 index 276c030ee87..00000000000 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedResourceTable.test.tsx +++ /dev/null @@ -1,108 +0,0 @@ -import { act, render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import React from 'react'; - -import { createMockResourcePickerRows } from '../../__mocks__/resourcePickerRows'; -import NestedResourceTable from './NestedResourceTable'; -import { findRow } from './utils'; - -describe('AzureMonitor NestedResourceTable', () => { - const noop: any = () => {}; - - const getElementById = document.getElementById; - beforeEach(() => { - document.getElementById = jest.fn().mockReturnValue({ - scrollIntoView: jest.fn(), - }); - }); - afterEach(() => { - document.getElementById = getElementById; - }); - - it('renders subscriptions', () => { - const rows = createMockResourcePickerRows(); - - render(); - - expect(screen.getByText('Primary Subscription')).toBeInTheDocument(); - expect(screen.getByText('Dev Subscription')).toBeInTheDocument(); - }); - - it('opens to the selected resource', () => { - const rows = createMockResourcePickerRows(); - const selected = findRow( - rows, - '/subscription/def-456/resourceGroups/dev/providers/Microsoft.Compute/disks/web-server_DataDisk' - ); - - if (!selected) { - throw new Error("couldn't find row, test data stale"); - } - - render( - - ); - - expect(screen.getByText('web-server_DataDisk')).toBeInTheDocument(); - }); - - it("expands subscriptions when they're clicked", async () => { - const rows = createMockResourcePickerRows(); - const promise = Promise.resolve(); - const requestNestedRows = jest.fn().mockReturnValue(promise); - render( - - ); - - const expandButton = screen.getAllByLabelText('Expand')[1]; - userEvent.click(expandButton); - - expect(requestNestedRows).toBeCalledWith( - expect.objectContaining({ - id: '/subscriptions/def-456', - name: 'Dev Subscription', - typeLabel: 'Subscription', - }) - ); - - expect(screen.queryByText('Development')).not.toBeInTheDocument(); - await act(() => promise); - expect(screen.getByText('Development')).toBeInTheDocument(); - }); - - it('supports selecting variables', async () => { - const rows = createMockResourcePickerRows(); - const promise = Promise.resolve(); - const requestNestedRows = jest.fn().mockReturnValue(promise); - const onRowSelectedChange = jest.fn(); - render( - - ); - - const expandButton = screen.getAllByLabelText('Expand')[2]; - userEvent.click(expandButton); - - await act(() => promise); - - const checkbox = screen.getByLabelText('$workspace'); - userEvent.click(checkbox); - - expect(onRowSelectedChange).toHaveBeenCalledWith( - expect.objectContaining({ - id: '$workspace', - name: '$workspace', - }), - true - ); - }); -}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRows.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRows.tsx index 08f91640b07..dd674a8c541 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRows.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/NestedRows.tsx @@ -67,8 +67,7 @@ const NestedRow: React.FC = ({ row, selectedRows, level, request useEffect(() => { // Assuming we don't have multi-select yet const selectedRow = selectedRows[0]; - - const containsChild = selectedRow && !!findRow(row.children ?? [], selectedRow.id); + const containsChild = selectedRow && !!findRow(row.children ?? [], selectedRow.uri); if (containsChild) { setRowStatus('open'); @@ -203,7 +202,7 @@ const NestedEntry: React.FC = ({ @@ -215,7 +214,13 @@ const NestedEntry: React.FC = ({ {isSelectable && ( <> - + )} 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 e78c2ef1834..2829e4cdf31 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 @@ -10,119 +10,143 @@ import { } from '../../__mocks__/resourcePickerRows'; const noResourceURI = ''; -const singleSubscriptionSelectionURI = 'def-456'; +const singleSubscriptionSelectionURI = '/subscriptions/def-456'; const singleResourceGroupSelectionURI = '/subscriptions/def-456/resourceGroups/dev-3'; const singleResourceSelectionURI = - '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/virtualMachines/db-serverproviders/Microsoft.Compute/virtualMachines/db-server'; + '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/virtualMachines/db-server'; +const createResourcePickerDataMock = () => { + return createMockResourcePickerData({ + getSubscriptions: jest.fn().mockResolvedValue(createMockSubscriptions()), + getResourceGroupsBySubscriptionId: jest.fn().mockResolvedValue(createMockResourceGroupsBySubscription()), + getResourcesForResourceGroup: jest.fn().mockResolvedValue(mockResourcesByResourceGroup()), + }); +}; describe('AzureMonitor ResourcePicker', () => { const noop: any = () => {}; - beforeEach(() => { window.HTMLElement.prototype.scrollIntoView = function () {}; }); - describe('when rendering the resource picker without a selection', () => { - it('should load subscriptions', async () => { - const resourePickerDataMock = createMockResourcePickerData({ - getSubscriptions: jest.fn().mockResolvedValue(createMockSubscriptions()), - getResourceGroupsBySubscriptionId: jest.fn(), - getResourcesForResourceGroup: jest.fn(), - }); - render( - - ); - - expect(await screen.findByText('Primary Subscription')).toBeInTheDocument(); - expect(resourePickerDataMock.getSubscriptions).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourceGroupsBySubscriptionId).not.toHaveBeenCalled(); - expect(resourePickerDataMock.getResourcesForResourceGroup).not.toHaveBeenCalled(); - }); + it('should pre-load subscriptions when there is no existing selection', async () => { + render( + + ); + const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); + expect(subscriptionCheckbox).toBeInTheDocument(); + expect(subscriptionCheckbox).not.toBeChecked(); + const uncheckedCheckboxes = await screen.findAllByRole('checkbox', { checked: false }); + expect(uncheckedCheckboxes.length).toBe(3); }); - describe('when rendering the resource picker with a subscription selected', () => { - it('should load subscriptions once', async () => { - const resourePickerDataMock = createMockResourcePickerData({ - getSubscriptions: jest.fn().mockResolvedValue(createMockSubscriptions()), - getResourceGroupsBySubscriptionId: jest.fn(), - getResourcesForResourceGroup: jest.fn(), - }); - render( - - ); - - expect(await screen.findByText('Primary Subscription')).toBeInTheDocument(); - expect(resourePickerDataMock.getSubscriptions).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourceGroupsBySubscriptionId).not.toHaveBeenCalled(); - expect(resourePickerDataMock.getResourcesForResourceGroup).not.toHaveBeenCalled(); - }); + it('should show a subscription as selected if there is one saved', async () => { + render( + + ); + const subscriptionCheckbox = await screen.findByLabelText('Dev Subscription'); + expect(subscriptionCheckbox).toBeChecked(); }); - describe('when rendering the resource picker with a resource group selected', () => { - it('should load subscriptions and resource groups for its parent subscription once', async () => { - const resourePickerDataMock = createMockResourcePickerData({ - getSubscriptions: jest.fn().mockResolvedValue(createMockSubscriptions()), - getResourceGroupsBySubscriptionId: jest.fn().mockResolvedValue(createMockResourceGroupsBySubscription()), - getResourcesForResourceGroup: jest.fn(), - }); - render( - - ); - - expect(await screen.findByText('Primary Subscription')).toBeInTheDocument(); - expect(resourePickerDataMock.getSubscriptions).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourceGroupsBySubscriptionId).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourceGroupsBySubscriptionId).toHaveBeenLastCalledWith( - singleSubscriptionSelectionURI - ); - expect(resourePickerDataMock.getResourcesForResourceGroup).not.toHaveBeenCalled(); - }); + it('should show a resource group as selected if there is one saved', async () => { + render( + + ); + const resourceGroupCheckbox = await screen.findByLabelText('A Great Resource Group'); + expect(resourceGroupCheckbox).toBeChecked(); }); - describe('when rendering the resource picker with a resource selected', () => { - it('should load subscriptions, resource groups and resources once', async () => { - const resourePickerDataMock = createMockResourcePickerData({ - getSubscriptions: jest.fn().mockResolvedValue(createMockSubscriptions()), - getResourceGroupsBySubscriptionId: jest.fn().mockResolvedValue(createMockResourceGroupsBySubscription()), - getResourcesForResourceGroup: jest.fn().mockResolvedValue(mockResourcesByResourceGroup()), - }); - render( - - ); + it('should show a resource as selected if there is one saved', async () => { + render( + + ); - expect(await screen.findByText('Primary Subscription')).toBeInTheDocument(); - expect(resourePickerDataMock.getSubscriptions).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourceGroupsBySubscriptionId).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourceGroupsBySubscriptionId).toHaveBeenLastCalledWith( - singleSubscriptionSelectionURI - ); - expect(resourePickerDataMock.getResourcesForResourceGroup).toHaveBeenCalledTimes(1); - expect(resourePickerDataMock.getResourcesForResourceGroup).toHaveBeenLastCalledWith( - singleResourceGroupSelectionURI - ); - }); + const resourceCheckbox = await screen.findByLabelText('db-server'); + expect(resourceCheckbox).toBeChecked(); + }); + + it('should be able to expand a subscription when clicked and reveal resource groups', async () => { + render( + + ); + const expandSubscriptionButton = await screen.findByLabelText('Expand Primary Subscription'); + expect(expandSubscriptionButton).toBeInTheDocument(); + expect(screen.queryByLabelText('A Great Resource Group')).not.toBeInTheDocument(); + expandSubscriptionButton.click(); + expect(await screen.findByLabelText('A Great Resource Group')).toBeInTheDocument(); + }); + + it('should call onApply with a new subscription uri when a user selects it', async () => { + const onApply = jest.fn(); + render( + + ); + const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); + expect(subscriptionCheckbox).toBeInTheDocument(); + expect(subscriptionCheckbox).not.toBeChecked(); + subscriptionCheckbox.click(); + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith('/subscriptions/def-123'); + }); + + it('should call onApply with a template variable when a user selects it', async () => { + const onApply = jest.fn(); + render( + + ); + + const expandButton = await screen.findByLabelText('Expand Template variables'); + expandButton.click(); + + const workSpaceCheckbox = await screen.findByLabelText('$workspace'); + workSpaceCheckbox.click(); + + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith('$workspace'); }); }); 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 d81f643f7e9..ec5637bbc3e 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 @@ -10,6 +10,7 @@ import NestedResourceTable from './NestedResourceTable'; import { ResourceRow, ResourceRowGroup, ResourceRowType } from './types'; import { addResources, findRow, parseResourceURI } from './utils'; +const TEMPLATE_VARIABLE_GROUP_ID = '$$grafana-templateVariables$$'; interface ResourcePickerProps { resourcePickerData: ResourcePickerData; resourceURI: string | undefined; @@ -31,12 +32,12 @@ const ResourcePicker = ({ type LoadingStatus = 'NotStarted' | 'Started' | 'Done'; const [loadingStatus, setLoadingStatus] = useState('NotStarted'); const [azureRows, setAzureRows] = useState([]); - const [internalSelected, setInternalSelected] = useState(resourceURI); + const [internalSelectedURI, setInternalSelectedURI] = useState(resourceURI); const [errorMessage, setErrorMessage] = useState(undefined); // Sync the resourceURI prop to internal state useEffect(() => { - setInternalSelected(resourceURI); + setInternalSelectedURI(resourceURI); }, [resourceURI]); // Request initial data on first mount @@ -46,13 +47,13 @@ const ResourcePicker = ({ try { setLoadingStatus('Started'); let resources = await resourcePickerData.getSubscriptions(); - if (!internalSelected) { + if (!internalSelectedURI) { setAzureRows(resources); setLoadingStatus('Done'); return; } - const parsedURI = parseResourceURI(internalSelected ?? ''); + const parsedURI = parseResourceURI(internalSelectedURI ?? ''); if (parsedURI) { const resourceGroupURI = `/subscriptions/${parsedURI.subscriptionID}/resourceGroups/${parsedURI.resourceGroup}`; @@ -61,7 +62,7 @@ const ResourcePicker = ({ const resourceGroups = await resourcePickerData.getResourceGroupsBySubscriptionId( parsedURI.subscriptionID ); - resources = addResources(resources, parsedURI.subscriptionID, resourceGroups); + resources = addResources(resources, `/subscriptions/${parsedURI.subscriptionID}`, resourceGroups); } // if a resource was previously selected, but the resources under the parent resource group have not been loaded yet @@ -80,16 +81,16 @@ const ResourcePicker = ({ loadInitialData(); } - }, [resourcePickerData, internalSelected, azureRows, loadingStatus]); + }, [resourcePickerData, internalSelectedURI, azureRows, loadingStatus]); const rows = useMemo(() => { - const templateVariableRow = resourcePickerData.transformVariablesToRow(templateVariables); + const templateVariableRow = transformVariablesToRow(templateVariables); return templateVariables.length ? [...azureRows, templateVariableRow] : azureRows; - }, [resourcePickerData, azureRows, templateVariables]); + }, [azureRows, templateVariables]); // Map the selected item into an array of rows const selectedResourceRows = useMemo(() => { - const found = internalSelected && findRow(rows, internalSelected); + const found = internalSelectedURI && findRow(rows, internalSelectedURI); return found ? [ { @@ -98,7 +99,7 @@ const ResourcePicker = ({ }, ] : []; - }, [internalSelected, rows]); + }, [internalSelectedURI, rows]); // Request resources for a expanded resource group const requestNestedRows = useCallback( @@ -110,7 +111,7 @@ const ResourcePicker = ({ // template variable group, though that shouldn't happen in practice if ( resourceGroupOrSubscription.children?.length || - resourceGroupOrSubscription.id === ResourcePickerData.templateVariableGroupID + resourceGroupOrSubscription.uri === TEMPLATE_VARIABLE_GROUP_ID ) { return; } @@ -121,7 +122,7 @@ const ResourcePicker = ({ ? await resourcePickerData.getResourceGroupsBySubscriptionId(resourceGroupOrSubscription.id) : await resourcePickerData.getResourcesForResourceGroup(resourceGroupOrSubscription.id); - const newRows = addResources(azureRows, resourceGroupOrSubscription.id, rows); + const newRows = addResources(azureRows, resourceGroupOrSubscription.uri, rows); setAzureRows(newRows); } catch (error) { @@ -132,14 +133,13 @@ const ResourcePicker = ({ [resourcePickerData, azureRows] ); - // Select const handleSelectionChanged = useCallback((row: ResourceRow, isSelected: boolean) => { - isSelected ? setInternalSelected(row.id) : setInternalSelected(undefined); + isSelected ? setInternalSelectedURI(row.uri) : setInternalSelectedURI(undefined); }, []); const handleApply = useCallback(() => { - onApply(internalSelected); - }, [internalSelected, onApply]); + onApply(internalSelectedURI); + }, [internalSelectedURI, onApply]); return (
@@ -211,3 +211,20 @@ const getStyles = (theme: GrafanaTheme2) => ({ color: theme.colors.text.secondary, }), }); + +function transformVariablesToRow(templateVariables: string[]): ResourceRow { + return { + id: TEMPLATE_VARIABLE_GROUP_ID, + uri: TEMPLATE_VARIABLE_GROUP_ID, + name: 'Template variables', + type: ResourceRowType.VariableGroup, + typeLabel: 'Variables', + children: templateVariables.map((v) => ({ + id: v, + uri: v, + name: v, + type: ResourceRowType.Variable, + typeLabel: 'Variable', + })), + }; +} diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/styles.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/styles.ts index 9f72e94a365..957fec90642 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/styles.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/styles.ts @@ -64,6 +64,10 @@ const getStyles = (theme: GrafanaTheme2) => ({ textOverflow: 'ellipsis', whiteSpace: 'nowrap', }), + + nestedRowCheckbox: css({ + zIndex: 0, + }), }); export default getStyles; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/types.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/types.ts index 2f02f61837f..7183f427bf9 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/types.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/types.ts @@ -7,7 +7,8 @@ export enum ResourceRowType { } export interface ResourceRow { - id: string; + id: string; // azure's raw data id usually passes along a uri (except in the case of subscriptions), to make things less confusing for ourselves we parse the id string out of the uri or vice versa + uri: string; // ex: /subscriptions/subid123 name: string; type: ResourceRowType; typeLabel: string; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts index cbfa79ffef9..3c3511d75e2 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts @@ -26,14 +26,14 @@ export function isGUIDish(input: string) { return !!input.match(/^[A-Z0-9]+/i); } -export function findRow(rows: ResourceRowGroup, id: string): ResourceRow | undefined { +export function findRow(rows: ResourceRowGroup, uri: string): ResourceRow | undefined { for (const row of rows) { - if (row.id.toLowerCase() === id.toLowerCase()) { + if (row.uri.toLowerCase() === uri.toLowerCase()) { return row; } if (row.children) { - const result = findRow(row.children, id); + const result = findRow(row.children, uri); if (result) { return result; @@ -44,9 +44,9 @@ export function findRow(rows: ResourceRowGroup, id: string): ResourceRow | undef return undefined; } -export function addResources(rows: ResourceRowGroup, targetResourceGroupID: string, newResources: ResourceRowGroup) { +export function addResources(rows: ResourceRowGroup, targetParentId: string, newResources: ResourceRowGroup) { return produce(rows, (draftState) => { - const draftRow = findRow(draftState, targetResourceGroupID); + const draftRow = findRow(draftState, targetParentId); if (!draftRow) { // This case shouldn't happen often because we're usually coming here from a resource we already have 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 7064d1a9bff..887db6140c1 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 @@ -4,129 +4,246 @@ import { createMockARGSubscriptionResponse, } from '../__mocks__/argResourcePickerResponse'; import { createMockInstanceSetttings } from '../__mocks__/instanceSettings'; -import { ResourceRowType } from '../components/ResourcePicker/types'; import ResourcePickerData from './resourcePickerData'; +import { AzureGraphResponse } from '../types'; -const instanceSettings = createMockInstanceSetttings(); -const resourcePickerData = new ResourcePickerData(instanceSettings); -let postResource: jest.Mock; +const createResourcePickerData = (responses: AzureGraphResponse[]) => { + const instanceSettings = createMockInstanceSetttings(); + const resourcePickerData = new ResourcePickerData(instanceSettings); + const postResource = jest.fn(); + responses.forEach((res) => { + postResource.mockResolvedValueOnce(res); + }); + resourcePickerData.postResource = postResource; + + return { resourcePickerData, postResource }; +}; describe('AzureMonitor resourcePickerData', () => { describe('getSubscriptions', () => { - beforeEach(() => { - postResource = jest.fn().mockResolvedValue(createMockARGSubscriptionResponse()); - resourcePickerData.postResource = postResource; - }); - - it('calls ARG API', async () => { + it('makes 1 call to ARG with the correct path and query arguments', async () => { + const mockResponse = createMockARGSubscriptionResponse(); + const { resourcePickerData, postResource } = createResourcePickerData([mockResponse]); await resourcePickerData.getSubscriptions(); - expect(postResource).toHaveBeenCalled(); - const argQuery = postResource.mock.calls[0][1].query; + expect(postResource).toBeCalledTimes(1); + const firstCall = postResource.mock.calls[0]; + const [path, postBody] = firstCall; + expect(path).toEqual('resourcegraph/providers/Microsoft.ResourceGraph/resources?api-version=2021-03-01'); + expect(postBody.query).toContain("where type == 'microsoft.resources/subscriptions'"); + }); + it('returns formatted subscriptions', async () => { + const mockResponse = createMockARGSubscriptionResponse(); + const { resourcePickerData } = createResourcePickerData([mockResponse]); - expect(argQuery).toContain(`where type == 'microsoft.resources/subscriptions'`); + const subscriptions = await resourcePickerData.getSubscriptions(); + expect(subscriptions.length).toEqual(6); + expect(subscriptions[0]).toEqual({ + id: '1', + name: 'Primary Subscription', + type: 'Subscription', + typeLabel: 'Subscription', + uri: '/subscriptions/1', + children: [], + }); }); - describe('when there is more than one page', () => { - beforeEach(() => { - const response1 = { - ...createMockARGSubscriptionResponse(), - $skipToken: 'aaa', - }; - const response2 = createMockARGSubscriptionResponse(); - postResource = jest.fn(); - postResource.mockResolvedValueOnce(response1); - postResource.mockResolvedValueOnce(response2); - resourcePickerData.postResource = postResource; - }); + it('makes multiple requests when arg returns a skipToken and passes the right skipToken to each subsequent call', async () => { + const response1 = { + ...createMockARGSubscriptionResponse(), + $skipToken: 'skipfirst100', + }; + const response2 = createMockARGSubscriptionResponse(); + const { resourcePickerData, postResource } = createResourcePickerData([response1, response2]); - it('should requests additional pages', async () => { - await resourcePickerData.getSubscriptions(); - expect(postResource).toHaveBeenCalledTimes(2); - }); + await resourcePickerData.getSubscriptions(); - it('should use the skipToken of the previous page', async () => { - await resourcePickerData.getSubscriptions(); - const secondCall = postResource.mock.calls[1]; - expect(secondCall[1]).toMatchObject({ options: { $skipToken: 'aaa', resultFormat: 'objectArray' } }); + expect(postResource).toHaveBeenCalledTimes(2); + const secondCall = postResource.mock.calls[1]; + const [_, postBody] = secondCall; + expect(postBody.options.$skipToken).toEqual('skipfirst100'); + }); + + it('returns a concatenates a formatted array of subscriptions when there are multiple pages from arg', async () => { + const response1 = { + ...createMockARGSubscriptionResponse(), + $skipToken: 'skipfirst100', + }; + const response2 = createMockARGSubscriptionResponse(); + const { resourcePickerData } = createResourcePickerData([response1, response2]); + + const subscriptions = await resourcePickerData.getSubscriptions(); + + expect(subscriptions.length).toEqual(12); + expect(subscriptions[0]).toEqual({ + id: '1', + name: 'Primary Subscription', + type: 'Subscription', + typeLabel: 'Subscription', + uri: '/subscriptions/1', + children: [], }); }); + + it('throws an error if it does not recieve data from arg', async () => { + const mockResponse = { data: [] }; + const { resourcePickerData } = createResourcePickerData([mockResponse]); + try { + await resourcePickerData.getSubscriptions(); + throw Error('expected getSubscriptions to fail but it succeeded'); + } catch (err) { + expect(err.message).toEqual('unable to fetch subscriptions'); + } + }); }); - describe('getResourcesForResourceGroup', () => { - beforeEach(() => { - postResource = jest.fn().mockResolvedValue(createMockARGResourceGroupsResponse()); - resourcePickerData.postResource = postResource; - }); - - it('calls ARG API', async () => { + describe('getResourceGroupsBySubscriptionId', () => { + it('makes 1 call to ARG with the correct path and query arguments', async () => { + const mockResponse = createMockARGResourceGroupsResponse(); + const { resourcePickerData, postResource } = createResourcePickerData([mockResponse]); await resourcePickerData.getResourceGroupsBySubscriptionId('123'); - expect(postResource).toHaveBeenCalled(); - const argQuery = postResource.mock.calls[0][1].query; + expect(postResource).toBeCalledTimes(1); + const firstCall = postResource.mock.calls[0]; + const [path, postBody] = firstCall; + expect(path).toEqual('resourcegraph/providers/Microsoft.ResourceGraph/resources?api-version=2021-03-01'); + expect(postBody.query).toContain("type == 'microsoft.resources/subscriptions/resourcegroups'"); + expect(postBody.query).toContain("where subscriptionId == '123'"); + }); + it('returns formatted resourceGroups', async () => { + const mockResponse = createMockARGResourceGroupsResponse(); + const { resourcePickerData } = createResourcePickerData([mockResponse]); - expect(argQuery).toContain(`| where subscriptionId == '123'`); + const resourceGroups = await resourcePickerData.getResourceGroupsBySubscriptionId('123'); + expect(resourceGroups.length).toEqual(6); + expect(resourceGroups[0]).toEqual({ + id: 'prod', + name: 'Production', + type: 'ResourceGroup', + typeLabel: 'Resource Group', + uri: '/subscriptions/abc-123/resourceGroups/prod', + children: [], + }); }); - describe('when there is more than one page', () => { - beforeEach(() => { - const response1 = { - ...createMockARGResourceGroupsResponse(), - $skipToken: 'aaa', - }; - const response2 = createMockARGResourceGroupsResponse(); - postResource = jest.fn(); - postResource.mockResolvedValueOnce(response1); - postResource.mockResolvedValueOnce(response2); - resourcePickerData.postResource = postResource; - }); + it('makes multiple requests when it is returned a skip token', async () => { + const response1 = { + ...createMockARGResourceGroupsResponse(), + $skipToken: 'skipfirst100', + }; + const response2 = createMockARGResourceGroupsResponse(); + const { resourcePickerData, postResource } = createResourcePickerData([response1, response2]); - it('should requests additional pages', async () => { - await resourcePickerData.getResourceGroupsBySubscriptionId('123'); - expect(postResource).toHaveBeenCalledTimes(2); - }); + await resourcePickerData.getResourceGroupsBySubscriptionId('123'); - it('should use the skipToken of the previous page', async () => { - await resourcePickerData.getResourceGroupsBySubscriptionId('123'); - const secondCall = postResource.mock.calls[1]; - expect(secondCall[1]).toMatchObject({ options: { $skipToken: 'aaa', resultFormat: 'objectArray' } }); + expect(postResource).toHaveBeenCalledTimes(2); + const secondCall = postResource.mock.calls[1]; + const [_, postBody] = secondCall; + expect(postBody.options.$skipToken).toEqual('skipfirst100'); + }); + + it('returns a concatonized and formatted array of resourceGroups when there are multiple pages', async () => { + const response1 = { + ...createMockARGResourceGroupsResponse(), + $skipToken: 'skipfirst100', + }; + const response2 = createMockARGResourceGroupsResponse(); + const { resourcePickerData } = createResourcePickerData([response1, response2]); + + const resourceGroups = await resourcePickerData.getResourceGroupsBySubscriptionId('123'); + + expect(resourceGroups.length).toEqual(12); + expect(resourceGroups[0]).toEqual({ + id: 'prod', + name: 'Production', + type: 'ResourceGroup', + typeLabel: 'Resource Group', + uri: '/subscriptions/abc-123/resourceGroups/prod', + children: [], }); }); + + it('throws an error if it does not receive data', async () => { + const mockResponse = { data: [] }; + const { resourcePickerData } = createResourcePickerData([mockResponse]); + try { + await resourcePickerData.getResourceGroupsBySubscriptionId('123'); + throw Error('expected getSubscriptions to fail but it succeeded'); + } catch (err) { + expect(err.message).toEqual('unable to fetch resource groups'); + } + }); + + it('throws an error if it recieves data with a malformed uri', async () => { + const mockResponse = { + data: [ + { + resourceGroupURI: '/a-differently-formatted/uri/than/the/type/we/planned/to/parse', + resourceGroupName: 'Production', + }, + ], + }; + const { resourcePickerData } = createResourcePickerData([mockResponse]); + try { + await resourcePickerData.getResourceGroupsBySubscriptionId('123'); + throw Error('expected getResourceGroupsBySubscriptionId to fail but it succeeded'); + } catch (err) { + expect(err.message).toEqual('unable to fetch resource groups'); + } + }); }); describe('getResourcesForResourceGroup', () => { - const resourceRow = { - id: '/subscriptions/def-456/resourceGroups/dev', - name: 'Dev', - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource group', - }; + it('makes 1 call to ARG with the correct path and query arguments', async () => { + const mockResponse = createARGResourcesResponse(); + const { resourcePickerData, postResource } = createResourcePickerData([mockResponse]); + await resourcePickerData.getResourcesForResourceGroup('dev'); - beforeEach(() => { - postResource = jest.fn().mockResolvedValue(createARGResourcesResponse()); - resourcePickerData.postResource = postResource; + expect(postResource).toBeCalledTimes(1); + const firstCall = postResource.mock.calls[0]; + const [path, postBody] = firstCall; + expect(path).toEqual('resourcegraph/providers/Microsoft.ResourceGraph/resources?api-version=2021-03-01'); + expect(postBody.query).toContain('resources'); + expect(postBody.query).toContain('where id hasprefix "dev"'); }); - - it('requests resources for the specified resource row', async () => { - await resourcePickerData.getResourcesForResourceGroup(resourceRow.id); - - expect(postResource).toHaveBeenCalled(); - const argQuery = postResource.mock.calls[0][1].query; - - expect(argQuery).toContain(resourceRow.id); - }); - it('returns formatted resources', async () => { - const results = await resourcePickerData.getResourcesForResourceGroup(resourceRow.id); + const mockResponse = createARGResourcesResponse(); + const { resourcePickerData } = createResourcePickerData([mockResponse]); - expect(results.map((v) => v.id)).toEqual([ - '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/web-server', - '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/disks/web-server_DataDisk', - '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/db-server', - '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/disks/db-server_DataDisk', - ]); + const resources = await resourcePickerData.getResourcesForResourceGroup('dev'); - results.forEach((v) => expect(v.type).toEqual(ResourceRowType.Resource)); + expect(resources.length).toEqual(4); + expect(resources[0]).toEqual({ + id: 'web-server', + name: 'web-server', + type: 'Resource', + location: 'North Europe', + resourceGroupName: 'dev', + typeLabel: 'Microsoft.Compute/virtualMachines', + uri: '/subscriptions/def-456/resourceGroups/dev/providers/Microsoft.Compute/virtualMachines/web-server', + }); + }); + + it('throws an error if it recieves data with a malformed uri', async () => { + const mockResponse = { + data: [ + { + id: '/a-differently-formatted/uri/than/the/type/we/planned/to/parse', + name: 'web-server', + type: 'Microsoft.Compute/virtualMachines', + resourceGroup: 'dev', + subscriptionId: 'def-456', + location: 'northeurope', + }, + ], + }; + const { resourcePickerData } = createResourcePickerData([mockResponse]); + try { + await resourcePickerData.getResourcesForResourceGroup('dev'); + throw Error('expected getResourcesForResourceGroup to fail but it succeeded'); + } catch (err) { + expect(err.message).toEqual('unable to fetch resource details'); + } }); }); }); 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 54ac2692360..c2dee619835 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 @@ -7,7 +7,7 @@ import { logsSupportedResourceTypesKusto, resourceTypeDisplayNames, } from '../azureMetadata'; -import { ResourceRow, ResourceRowGroup, ResourceRowType } from '../components/ResourcePicker/types'; +import { ResourceRowGroup, ResourceRowType } from '../components/ResourcePicker/types'; import { parseResourceURI } from '../components/ResourcePicker/utils'; import { AzureDataSourceJsonData, @@ -31,8 +31,6 @@ export default class ResourcePickerData extends DataSourceWithBackend { const query = ` resources @@ -59,7 +57,7 @@ export default class ResourcePickerData extends DataSourceWithBackend(query, 1, options); if (!resourceResponse.data.length) { - throw new Error('unable to fetch resource details'); + throw new Error('unable to fetch subscriptions'); } resources = resources.concat(resourceResponse.data); $skipToken = resourceResponse.$skipToken; @@ -69,13 +67,14 @@ export default class ResourcePickerData extends DataSourceWithBackend ({ name: subscription.subscriptionName, id: subscription.subscriptionId, + uri: `/subscriptions/${subscription.subscriptionId}`, typeLabel: 'Subscription', type: ResourceRowType.Subscription, children: [], })); } - async getResourceGroupsBySubscriptionId(subscriptionId: string) { + async getResourceGroupsBySubscriptionId(subscriptionId: string): Promise { const query = ` resources | join kind=inner ( @@ -89,7 +88,7 @@ export default class ResourcePickerData extends DataSourceWithBackend(query, 1, options); if (!resourceResponse.data.length) { - throw new Error('unable to fetch resource details'); + throw new Error('unable to fetch resource groups'); } - resources = resources.concat(resourceResponse.data); + resourceGroups = resourceGroups.concat(resourceResponse.data); $skipToken = resourceResponse.$skipToken; allFetched = !$skipToken; } - return resources.map((r) => ({ - name: r.resourceGroupName, - id: r.resourceGroupURI, - type: ResourceRowType.ResourceGroup, - typeLabel: 'Resource Group', - children: [], - })); + return resourceGroups.map((r) => { + const parsedUri = parseResourceURI(r.resourceGroupURI); + if (!parsedUri || !parsedUri.resourceGroup) { + throw new Error('unable to fetch resource groups'); + } + return { + name: r.resourceGroupName, + uri: r.resourceGroupURI, + id: parsedUri.resourceGroup, + type: ResourceRowType.ResourceGroup, + typeLabel: 'Resource Group', + children: [], + }; + }); } - async getResourcesForResourceGroup(resourceGroupId: string) { + async getResourcesForResourceGroup(resourceGroupId: string): Promise { const { data: response } = await this.makeResourceGraphRequest(` resources | where id hasprefix "${resourceGroupId}" | where type in (${logsSupportedResourceTypesKusto}) and location in (${logsSupportedLocationsKusto}) `); - return formatResourceGroupChildren(response); + return response.map((item) => { + const parsedUri = parseResourceURI(item.id); + if (!parsedUri || !parsedUri.resource) { + throw new Error('unable to fetch resource details'); + } + return { + name: item.name, + id: parsedUri.resource, + uri: item.id, + resourceGroupName: item.resourceGroup, + type: ResourceRowType.Resource, + typeLabel: resourceTypeDisplayNames[item.type] || item.type, + location: locationDisplayNames[item.location] || item.location, + }; + }); } + // used to make the select resource button that launches the resource picker show a nicer file path to users async getResourceURIDisplayProperties(resourceURI: string): Promise { const { subscriptionID, resourceGroup } = parseResourceURI(resourceURI) ?? {}; @@ -206,30 +227,4 @@ export default class ResourcePickerData extends DataSourceWithBackend ({ - id: v, - name: v, - type: ResourceRowType.Variable, - typeLabel: 'Variable', - })), - }; - } -} - -function formatResourceGroupChildren(rawData: RawAzureResourceItem[]): ResourceRowGroup { - return rawData.map((item) => ({ - name: item.name, - id: item.id, - resourceGroupName: item.resourceGroup, - type: ResourceRowType.Resource, - typeLabel: resourceTypeDisplayNames[item.type] || item.type, - location: locationDisplayNames[item.location] || item.location, - })); }