From 164ce63e280565cc960fb0e0d9f074ec003d5a2a Mon Sep 17 00:00:00 2001
From: Yaelle Chaudy <42030685+yaelleC@users.noreply.github.com>
Date: Fri, 21 Jan 2022 12:19:09 +0100
Subject: [PATCH] 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
---
 .../MetricStatEditor.test.tsx                 | 90 ++++++++++++++++++-
 .../MetricStatEditor/MetricStatEditor.tsx     | 23 ++++-
 2 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx
index e2b70b4dbd1..ddb479e9cf9 100644
--- a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx
+++ b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.test.tsx
@@ -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' }]]);
+    });
+  });
 });
diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx
index 39742b921d5..18260abdf0a 100644
--- a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx
+++ b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx
@@ -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 }) => {