From b2e2879b07dc99a41e9d42cc5956472f64f0f2a4 Mon Sep 17 00:00:00 2001 From: Sarah Zinger Date: Tue, 18 Oct 2022 14:34:27 -0400 Subject: [PATCH] Cloudwatch: Fix issue where selected log groups clear from dashboards if there are more than 50 results (#57196) --- .../components/LogGroupSelector.test.tsx | 30 ++----------------- .../components/LogGroupSelector.tsx | 16 ++++------ 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx b/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx index 4f2acd8f0be..22d5b36464b 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import lodash from 'lodash'; // eslint-disable-line lodash/import-scope import React from 'react'; @@ -26,7 +26,7 @@ describe('LogGroupSelector', () => { jest.resetAllMocks(); }); - it('updates upstream query log groups on region change', async () => { + it('does not clear previously selected log groups after region change', async () => { ds.datasource.api.describeLogGroups = jest.fn().mockImplementation(async (params: DescribeLogGroupsRequest) => { if (params.region === 'region1') { return Promise.resolve(['log_group_1'].map(toOption)); @@ -40,36 +40,10 @@ describe('LogGroupSelector', () => { }; const { rerender } = render(); - await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1)); - expect(onChange).toHaveBeenLastCalledWith(['log_group_1']); expect(await screen.findByText('log_group_1')).toBeInTheDocument(); act(() => rerender()); - await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1)); - expect(onChange).toHaveBeenLastCalledWith([]); - }); - - it('does not update upstream query log groups if saved is false', async () => { - ds.datasource.api.describeLogGroups = jest.fn().mockImplementation(async (params: DescribeLogGroupsRequest) => { - if (params.region === 'region1') { - return Promise.resolve(['log_group_1'].map(toOption)); - } else { - return Promise.resolve(['log_group_2'].map(toOption)); - } - }); - const props = { - ...defaultProps, - selectedLogGroups: ['log_group_1'], - }; - - const { rerender } = render(); - await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1)); - expect(onChange).toHaveBeenLastCalledWith(['log_group_1']); expect(await screen.findByText('log_group_1')).toBeInTheDocument(); - - act(() => rerender()); - await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1)); - expect(onChange).toHaveBeenLastCalledWith(['log_group_1']); }); it('should merge results of remote log groups search with existing results', async () => { diff --git a/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.tsx b/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.tsx index 5d3a5df1d2e..7e5a3e09c67 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.tsx @@ -1,4 +1,4 @@ -import { debounce, intersection, unionBy } from 'lodash'; +import { debounce, unionBy } from 'lodash'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { SelectableValue, toOption } from '@grafana/data'; @@ -25,7 +25,7 @@ export interface LogGroupSelectorProps { onOpenMenu?: () => Promise; refId?: string; width?: number | 'auto'; - saved?: boolean; + saved?: boolean; // is only used in the config editor } export const LogGroupSelector: React.FC = ({ @@ -90,7 +90,7 @@ export const LogGroupSelector: React.FC = ({ // Reset the log group options if the datasource or region change and are saved useEffect(() => { - async function resetLogGroups() { + async function getAvailableLogGroupOptions() { // Don't call describeLogGroups if datasource or region is undefined if (!datasource || !datasource.getActualRegion(region)) { setAvailableLogGroups([]); @@ -100,19 +100,15 @@ export const LogGroupSelector: React.FC = ({ setLoadingLogGroups(true); return fetchLogGroupOptions(datasource.getActualRegion(region)) .then((logGroups) => { - const newSelectedLogGroups = intersection( - selectedLogGroups, - logGroups.map((l) => l.value || '') - ); - onChange(newSelectedLogGroups); setAvailableLogGroups(logGroups); }) .finally(() => { setLoadingLogGroups(false); }); } - // Only reset if the current datasource is saved - saved && resetLogGroups(); + + // Config editor does not fetch new log group options unless changes have been saved + saved && getAvailableLogGroupOptions(); // this hook shouldn't get called every time selectedLogGroups or onChange updates // eslint-disable-next-line react-hooks/exhaustive-deps }, [datasource, region, saved]);