CloudWatch: Fix MetricName resetting on Namespace change (#44165)

* Added tests for wanted behavious in MetricsStatEditor

* Removed wrong import

* Updated MetricStatEditor to add new behaviour

* Updated tests to use react-select-event

* removed unused export

* Adding check for empty string in select value and force null

* clean up

* added check to avoid call to getMetric if there is no metricName selected

* removed todo

* removed empty lines

* Moved external import to top

* Refactored tests to copy props rather than overriding them
This commit is contained in:
Yaelle Chaudy 2022-01-21 12:19:09 +01:00 committed by GitHub
parent 5b7e05cde0
commit 164ce63e28
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 110 additions and 3 deletions

View File

@ -1,5 +1,6 @@
import React from 'react';
import { cleanup, fireEvent, render, screen } from '@testing-library/react';
import { cleanup, fireEvent, render, screen, act } from '@testing-library/react';
import selectEvent from 'react-select-event';
import { setupMockedDataSource } from '../../__mocks__/CloudWatchDataSource';
import '@testing-library/jest-dom';
import { CloudWatchMetricsQuery } from '../../types';
@ -99,4 +100,91 @@ describe('MetricStatEditor', () => {
expect(screen.getByLabelText('Match exact - optional')).not.toBeChecked();
});
});
describe('validating Query namespace / metricName', () => {
const namespaces = [
{ value: 'n1', label: 'n1', text: 'n1' },
{ value: 'n2', label: 'n2', text: 'n2' },
];
const metrics = [
{ value: 'm1', label: 'm1', text: 'm1' },
{ value: 'm2', label: 'm2', text: 'm2' },
];
const onChange = jest.fn();
const onRunQuery = jest.fn();
const propsNamespaceMetrics = {
...props,
onChange,
onRunQuery,
};
beforeEach(() => {
propsNamespaceMetrics.datasource.getNamespaces = jest.fn().mockResolvedValue(namespaces);
propsNamespaceMetrics.datasource.getMetrics = jest.fn().mockResolvedValue(metrics);
onChange.mockClear();
onRunQuery.mockClear();
});
it('should select namespace and metric name correctly', async () => {
await act(async () => {
render(<MetricStatEditor {...propsNamespaceMetrics} />);
});
const namespaceSelect = screen.getByLabelText('Namespace');
const metricsSelect = screen.getByLabelText('Metric name');
expect(namespaceSelect).toBeInTheDocument();
expect(metricsSelect).toBeInTheDocument();
await selectEvent.select(namespaceSelect, 'n1');
await selectEvent.select(metricsSelect, 'm1');
expect(onChange.mock.calls).toEqual([
[{ ...propsNamespaceMetrics.query, namespace: 'n1' }], // First call, namespace select
[{ ...propsNamespaceMetrics.query, metricName: 'm1' }], // Second call, metric select
]);
expect(onRunQuery).toHaveBeenCalledTimes(2);
});
it('should remove metricName from query if it does not exist in new namespace', async () => {
propsNamespaceMetrics.datasource.getMetrics = jest
.fn()
.mockImplementation((namespace: string, region: string) => {
let mockMetrics =
namespace === 'n1' && region === props.query.region
? metrics
: [{ value: 'oldNamespaceMetric', label: 'oldNamespaceMetric', text: 'oldNamespaceMetric' }];
return Promise.resolve(mockMetrics);
});
propsNamespaceMetrics.query.metricName = 'oldNamespaceMetric';
propsNamespaceMetrics.query.namespace = 'n2';
await act(async () => {
render(<MetricStatEditor {...propsNamespaceMetrics} />);
});
const namespaceSelect = screen.getByLabelText('Namespace');
expect(screen.getByText('n2')).toBeInTheDocument();
expect(screen.getByText('oldNamespaceMetric')).toBeInTheDocument();
await selectEvent.select(namespaceSelect, 'n1');
expect(onChange.mock.calls).toEqual([[{ ...propsNamespaceMetrics.query, metricName: '', namespace: 'n1' }]]);
});
it('should not remove metricName from query if it does exist in new namespace', async () => {
propsNamespaceMetrics.query.namespace = 'n1';
propsNamespaceMetrics.query.metricName = 'm1';
await act(async () => {
render(<MetricStatEditor {...propsNamespaceMetrics} />);
});
const namespaceSelect = screen.getByLabelText('Namespace');
expect(screen.getByText('n1')).toBeInTheDocument();
expect(screen.getByText('m1')).toBeInTheDocument();
await selectEvent.select(namespaceSelect, 'n2');
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange.mock.calls).toEqual([[{ ...propsNamespaceMetrics.query, metricName: 'm1', namespace: 'n2' }]]);
});
});
});

View File

@ -6,6 +6,7 @@ import { CloudWatchDatasource } from '../../datasource';
import { useDimensionKeys, useMetrics, useNamespaces } from '../../hooks';
import { CloudWatchMetricsQuery } from '../../types';
import { appendTemplateVariables, toOption } from '../../utils/utils';
import { SelectableValue } from '@grafana/data';
export type Props = {
query: CloudWatchMetricsQuery;
@ -32,6 +33,24 @@ export function MetricStatEditor({
onRunQuery();
};
const onNamespaceChange = async (query: CloudWatchMetricsQuery) => {
const validatedQuery = await validateMetricName(query);
onQueryChange(validatedQuery);
};
const validateMetricName = async (query: CloudWatchMetricsQuery) => {
let { metricName, namespace, region } = query;
if (!metricName) {
return query;
}
await datasource.getMetrics(namespace, region).then((result: Array<SelectableValue<string>>) => {
if (!result.find((metric) => metric.value === metricName)) {
metricName = '';
}
});
return { ...query, metricName };
};
return (
<EditorRows>
<EditorRow>
@ -44,7 +63,7 @@ export function MetricStatEditor({
options={namespaces}
onChange={({ value: namespace }) => {
if (namespace) {
onQueryChange({ ...query, namespace });
onNamespaceChange({ ...query, namespace });
}
}}
/>
@ -52,7 +71,7 @@ export function MetricStatEditor({
<EditorField label="Metric name" width={16}>
<Select
aria-label="Metric name"
value={query.metricName}
value={query.metricName || null}
allowCustomValue
options={metrics}
onChange={({ value: metricName }) => {