From 173c87beb5b846f06cf629b691038d9050d43f23 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= <pm.jamroz@gmail.com>
Date: Wed, 19 Jul 2023 08:39:52 +0200
Subject: [PATCH] DatasourcePicker: Call onChange only if data source changes
 (#71630)

Call onChange only when data source changes
---
 .../components/picker/DataSourceDropdown.test.tsx    | 12 ++++++++++--
 .../components/picker/DataSourceDropdown.tsx         |  6 ++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/public/app/features/datasources/components/picker/DataSourceDropdown.test.tsx b/public/app/features/datasources/components/picker/DataSourceDropdown.test.tsx
index 3b688a93a2f..aac37c4968c 100644
--- a/public/app/features/datasources/components/picker/DataSourceDropdown.test.tsx
+++ b/public/app/features/datasources/components/picker/DataSourceDropdown.test.tsx
@@ -142,7 +142,7 @@ describe('DataSourceDropdown', () => {
       );
     });
 
-    it('should dispaly the current selected DS in the selector', async () => {
+    it('should display the current selected DS in the selector', async () => {
       getInstanceSettingsMock.mockReturnValue(mockDS2);
       render(<DataSourceDropdown onChange={jest.fn()} current={mockDS2}></DataSourceDropdown>);
       expect(screen.getByTestId('Select a data source')).toHaveAttribute('placeholder', mockDS2.name);
@@ -163,7 +163,7 @@ describe('DataSourceDropdown', () => {
       expect(await findByText(cards[0], mockDS2.name, { selector: 'span' })).toBeInTheDocument();
     });
 
-    it('should dispaly the default DS as selected when `current` is not set', async () => {
+    it('should display the default DS as selected when `current` is not set', async () => {
       getInstanceSettingsMock.mockReturnValue(mockDS2);
       render(<DataSourceDropdown onChange={jest.fn()} current={undefined}></DataSourceDropdown>);
       expect(screen.getByTestId('Select a data source')).toHaveAttribute('placeholder', mockDS2.name);
@@ -216,6 +216,14 @@ describe('DataSourceDropdown', () => {
       expect(screen.queryByText(mockDS1.name, { selector: 'span' })).toBeNull();
     });
 
+    it('should not call onChange when the currently selected data source is clicked', async () => {
+      const onChange = jest.fn();
+      await setupOpenDropdown(user, { onChange });
+
+      await user.click(await screen.findByText(mockDS1.name, { selector: 'span' }));
+      expect(onChange).not.toBeCalled();
+    });
+
     it('should push recently used datasources when a data source is clicked', async () => {
       const onChange = jest.fn();
       await setupOpenDropdown(user, { onChange });
diff --git a/public/app/features/datasources/components/picker/DataSourceDropdown.tsx b/public/app/features/datasources/components/picker/DataSourceDropdown.tsx
index 2604592c9b2..78d7fe9c16e 100644
--- a/public/app/features/datasources/components/picker/DataSourceDropdown.tsx
+++ b/public/app/features/datasources/components/picker/DataSourceDropdown.tsx
@@ -202,7 +202,10 @@ export function DataSourceDropdown(props: DataSourceDropdownProps) {
               filterTerm={filterTerm}
               onChange={(ds: DataSourceInstanceSettings, defaultQueries?: DataQuery[] | GrafanaQuery[]) => {
                 onClose();
-                onChange(ds, defaultQueries);
+                if (ds.uid !== currentValue?.uid) {
+                  onChange(ds, defaultQueries);
+                  reportInteraction(INTERACTION_EVENT_NAME, { item: INTERACTION_ITEM.SELECT_DS, ds_type: ds.type });
+                }
               }}
               onClose={onClose}
               current={currentValue}
@@ -253,7 +256,6 @@ const PickerContent = React.forwardRef<HTMLDivElement, PickerContentProps>((prop
   const changeCallback = useCallback(
     (ds: DataSourceInstanceSettings) => {
       onChange(ds);
-      reportInteraction(INTERACTION_EVENT_NAME, { item: INTERACTION_ITEM.SELECT_DS, ds_type: ds.type });
     },
     [onChange]
   );