diff --git a/packages/manager/.changeset/pr-11615-upcoming-features-1738754714037.md b/packages/manager/.changeset/pr-11615-upcoming-features-1738754714037.md new file mode 100644 index 00000000000..e0be2fc553b --- /dev/null +++ b/packages/manager/.changeset/pr-11615-upcoming-features-1738754714037.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Update Tags dependency and filtering based on Region in CloudPulse ([#11615](https://github.com/linode/manager/pull/11615)) diff --git a/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.test.ts b/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.test.ts index 37940ce169e..36048768e50 100644 --- a/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.test.ts +++ b/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.test.ts @@ -13,6 +13,7 @@ import { getMetricsCallCustomFilters, getRegionProperties, getResourcesProperties, + getTagsProperties, getTimeDurationProperties, } from './FilterBuilder'; import { deepEqual, getFilters } from './FilterBuilder'; @@ -52,6 +53,38 @@ it('test getRegionProperties method', () => { } }); +it('test getTagsProperties', () => { + const tagsConfig = linodeConfig?.filters.find( + (filterObj) => filterObj.name === 'Tags' + ); + + expect(tagsConfig).toBeDefined(); + + if (tagsConfig) { + const { + disabled, + handleTagsChange, + label, + region, + resourceType, + } = getTagsProperties( + { + config: tagsConfig, + dashboard: mockDashboard, + dependentFilters: { region: 'us-east' }, + isServiceAnalyticsIntegration: true, + }, + vi.fn() + ); + const { name } = tagsConfig.configuration; + expect(handleTagsChange).toBeDefined(); + expect(disabled).toEqual(false); + expect(label).toEqual(name); + expect(region).toEqual('us-east'); + expect(resourceType).toEqual('linode'); + } +}); + it('test getTimeDuratonProperties method', () => { const timeDurationConfig = linodeConfig?.filters.find( ({ name }) => name === 'Time Range' @@ -145,6 +178,46 @@ it('test getResourceSelectionProperties method with disabled true', () => { } }); +describe('checkIfWeNeedToDisableFilterByFilterKey', () => { + // resources filter has region as mandatory and tags as an optional filter, this should reflect in the dependent filters + it('should enable filter when dependent filter region is provided', () => { + const result = checkIfWeNeedToDisableFilterByFilterKey( + 'resource_id', + { region: 'us-east' }, + mockDashboard + ); + expect(result).toEqual(false); + }); + + it('should disable filter when dependent filter region is undefined', () => { + const result = checkIfWeNeedToDisableFilterByFilterKey( + 'resource_id', + { region: undefined }, + mockDashboard + ); + expect(result).toEqual(true); + }); + + it('should disable filter when no dependent filters are provided', () => { + const result = checkIfWeNeedToDisableFilterByFilterKey( + 'resource_id', + {}, + mockDashboard + ); + expect(result).toEqual(true); + }); + + it('should disable filter when required dependent filter is undefined in dependent filters but defined in preferences', () => { + const result = checkIfWeNeedToDisableFilterByFilterKey( + 'resource_id', + { region: 'us-east', tags: undefined }, + mockDashboard, + { region: 'us-east', tags: ['tag-1'] } // tags are defined in preferences which confirms that this optional filter was selected + ); + expect(result).toEqual(true); + }); +}); + it('test checkIfWeNeedToDisableFilterByFilterKey method all cases', () => { let result = checkIfWeNeedToDisableFilterByFilterKey( 'resource_id', @@ -169,6 +242,23 @@ it('test checkIfWeNeedToDisableFilterByFilterKey method all cases', () => { ); expect(result).toEqual(true); + + result = checkIfWeNeedToDisableFilterByFilterKey( + 'resource_id', + { region: 'us-east', tags: undefined }, + mockDashboard, + { ['region']: 'us-east', ['tags']: ['tag-1'] } + ); + + expect(result).toEqual(true); // disabled is true as tags are not updated in dependent filters + + result = checkIfWeNeedToDisableFilterByFilterKey( + 'tags', + { region: undefined }, + mockDashboard + ); + + expect(result).toEqual(true); }); it('test buildXfilter method', () => { diff --git a/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts b/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts index 122135e4102..1506b679bac 100644 --- a/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts +++ b/packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts @@ -54,6 +54,7 @@ interface CloudPulseMandatoryFilterCheckProps { * @param config - accepts a CloudPulseServiceTypeFilters of tag key * @param handleTagsChange - the callback when we select new tag * @param dashboard - the selected dashboard's service type + * @param dependentFilters - tags are dependent on region filter, we need this for disabling the tags selection component * @param isServiceAnalyticsIntegration - only if this is false, we need to save preferences , else no need * @returns CloudPulseTagSelectProps */ @@ -61,14 +62,26 @@ export const getTagsProperties = ( props: CloudPulseFilterProperties, handleTagsChange: (tags: CloudPulseTags[], savePref?: boolean) => void ): CloudPulseTagsSelectProps => { - const { name: label, placeholder } = props.config.configuration; - const { dashboard, isServiceAnalyticsIntegration, preferences } = props; + const { filterKey, name: label, placeholder } = props.config.configuration; + const { + dashboard, + dependentFilters, + isServiceAnalyticsIntegration, + preferences, + } = props; return { defaultValue: preferences?.[TAGS], + disabled: checkIfWeNeedToDisableFilterByFilterKey( + filterKey, + dependentFilters ?? {}, + dashboard, + preferences + ), handleTagsChange, label, optional: props.config.configuration.isOptional, placeholder, + region: dependentFilters?.[REGION], resourceType: dashboard.service_type, savePreferences: !isServiceAnalyticsIntegration, }; @@ -133,7 +146,8 @@ export const getResourcesProperties = ( disabled: checkIfWeNeedToDisableFilterByFilterKey( filterKey, dependentFilters ?? {}, - dashboard + dashboard, + preferences ), handleResourcesSelection: handleResourceChange, label, @@ -282,7 +296,8 @@ export const checkIfWeNeedToDisableFilterByFilterKey = ( dependentFilters: { [key: string]: FilterValueType | TimeDuration; }, - dashboard: Dashboard + dashboard: Dashboard, + preferences?: AclpConfig ): boolean | undefined => { if (dashboard?.service_type) { const serviceTypeConfig = FILTER_CONFIG.get(dashboard.service_type); @@ -304,6 +319,15 @@ export const checkIfWeNeedToDisableFilterByFilterKey = ( return filter.configuration.dependency?.some((dependent) => { const dependentFilter = dependentFilters[dependent]; + if ( + preferences && + preferences[dependent] && + (!dependentFilter || + (Array.isArray(dependentFilter) && dependentFilter.length === 0)) + ) { + return true; // Since filters are set one by one, disabled will be true until the values that are defined inside the preferences are populated in the dependent filters as well + } + return ( !optionalFilters.has(dependent) && (!dependentFilter || diff --git a/packages/manager/src/features/CloudPulse/Utils/FilterConfig.ts b/packages/manager/src/features/CloudPulse/Utils/FilterConfig.ts index 3ac7cf87bdc..62c1d5dca2c 100644 --- a/packages/manager/src/features/CloudPulse/Utils/FilterConfig.ts +++ b/packages/manager/src/features/CloudPulse/Utils/FilterConfig.ts @@ -11,30 +11,31 @@ export const LINODE_CONFIG: Readonly = { filters: [ { configuration: { - filterKey: 'tags', + filterKey: 'region', filterType: 'string', isFilterable: false, isMetricsFilter: false, - isMultiSelect: true, - isOptional: true, - name: 'Tags', + name: 'Region', neededInServicePage: false, - placeholder: 'Select Tags', - priority: 4, + priority: 1, }, - name: 'Tags', + name: 'Region', }, { configuration: { - filterKey: 'region', + dependency: ['region'], + filterKey: 'tags', filterType: 'string', isFilterable: false, isMetricsFilter: false, - name: 'Region', + isMultiSelect: true, + isOptional: true, + name: 'Tags', neededInServicePage: false, - priority: 1, + placeholder: 'Select Tags', + priority: 4, }, - name: 'Region', + name: 'Tags', }, { configuration: { diff --git a/packages/manager/src/features/CloudPulse/shared/CloudPulseDashboardFilterBuilder.tsx b/packages/manager/src/features/CloudPulse/shared/CloudPulseDashboardFilterBuilder.tsx index cf9d91f0dd7..c7479590091 100644 --- a/packages/manager/src/features/CloudPulse/shared/CloudPulseDashboardFilterBuilder.tsx +++ b/packages/manager/src/features/CloudPulse/shared/CloudPulseDashboardFilterBuilder.tsx @@ -138,6 +138,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo( selectedTags, savePref, { + [RESOURCE_ID]: undefined, [TAGS]: selectedTags, } ); @@ -171,6 +172,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo( const updatedPreferenceData = { [REGION]: region, [RESOURCES]: undefined, + [TAGS]: undefined, }; emitFilterChangeByFilterKey( REGION, @@ -209,6 +211,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo( { config, dashboard, + dependentFilters: dependentFilterReference.current, isServiceAnalyticsIntegration, preferences, }, diff --git a/packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx b/packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx index b6dee07b2cb..9aebcec1d62 100644 --- a/packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx +++ b/packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx @@ -102,6 +102,10 @@ export const CloudPulseResourcesSelect = React.memo( // Once the data is loaded, set the state variable with value stored in preferences React.useEffect(() => { + if (disabled && !selectedResources) { + return; + } + // To save default values, go through side effects if disabled is false if (resources && savePreferences && !selectedResources) { const defaultResources = defaultValue && Array.isArray(defaultValue) diff --git a/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.test.tsx b/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.test.tsx index d4cd45f2468..34c77706bf2 100644 --- a/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.test.tsx +++ b/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.test.tsx @@ -6,8 +6,18 @@ import { renderWithTheme } from 'src/utilities/testHelpers'; import { CloudPulseTagsSelect } from './CloudPulseTagsFilter'; +import type { CloudPulseTagsSelectProps } from './CloudPulseTagsFilter'; import type { useAllLinodesQuery } from 'src/queries/linodes/linodes'; +const props: CloudPulseTagsSelectProps = { + disabled: false, + handleTagsChange: vi.fn(), + label: 'Tags', + optional: true, + region: 'us-east', + resourceType: 'linode', +}; + const queryMocks = vi.hoisted(() => ({ useAllLinodesQuery: vi.fn().mockReturnValue({}), })); @@ -20,7 +30,6 @@ vi.mock('src/queries/linodes/linodes', async () => { }; }); -const mockTagsHandler = vi.fn(); const SELECT_ALL = 'Select All'; const ARIA_SELECTED = 'aria-selected'; const LABEL_SUBTITLE = 'Tags (optional)'; @@ -41,37 +50,55 @@ describe('CloudPulseTagsSelect component tests', () => { getByLabelText, getByPlaceholderText, getByTestId, - } = renderWithTheme( - - ); + } = renderWithTheme(); expect(getByTestId('tags-select')).toBeInTheDocument(); expect(getByLabelText(LABEL_SUBTITLE)).toBeInTheDocument(); expect(getByPlaceholderText('Select Tags')).toBeInTheDocument(); }); + it('should render a disabled Tags Select component when disabled is true or region is undefined', () => { + queryMocks.useAllLinodesQuery.mockReturnValue({ + data: linodeFactory.buildList(2), + isError: false, + isLoading: false, + status: 'success', + }); + const { getByRole } = renderWithTheme( + + ); + + expect(getByRole('button', { name: 'Open' })).toBeDisabled(); + }); + it('should render a Tags Select component with error message on api call failure', () => { queryMocks.useAllLinodesQuery.mockReturnValue({ data: undefined, isError: true, isLoading: false, } as ReturnType); - const { getByText } = renderWithTheme( - - ); + const { getByText } = renderWithTheme(); expect(getByText('Failed to fetch Tags.')); }); - it('should select multiple tags', async () => { + it('should render a Tags Select component with tags filtered based on the selected region', async () => { + const user = userEvent.setup(); + + queryMocks.useAllLinodesQuery.mockReturnValue({ + data: [], + isError: false, + isLoading: false, + status: 'success', + }); + const { getByRole, queryAllByRole } = renderWithTheme( + + ); + await user.click(getByRole('button', { name: 'Open' })); + const options = queryAllByRole('option'); + expect(options).toHaveLength(0); // no tags for the selected region + }); + + it('should select multiple tags from the tags filtered based on the selected region', async () => { const user = userEvent.setup(); queryMocks.useAllLinodesQuery.mockReturnValue({ @@ -83,12 +110,7 @@ describe('CloudPulseTagsSelect component tests', () => { status: 'success', }); const { getByLabelText, getByRole } = renderWithTheme( - + ); await user.click(getByRole('button', { name: 'Open' })); await user.click(getByRole('option', { name: 'tag-2' })); @@ -122,12 +144,7 @@ describe('CloudPulseTagsSelect component tests', () => { status: 'success', }); const { getByLabelText, getByRole } = renderWithTheme( - + ); await user.click(getByRole('button', { name: 'Open' })); await user.click(getByRole('option', { name: SELECT_ALL })); @@ -156,10 +173,8 @@ describe('CloudPulseTagsSelect component tests', () => { const { getByRole } = renderWithTheme( ); diff --git a/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx b/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx index e5bfe1fb9eb..17dc6359939 100644 --- a/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx +++ b/packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx @@ -4,6 +4,7 @@ import React from 'react'; import { useAllLinodesQuery } from 'src/queries/linodes/linodes'; import { themes } from 'src/utilities/theme'; +import type { FilterValueType } from '../Dashboard/CloudPulseDashboardLanding'; import type { FilterValue, Linode } from '@linode/api-v4'; export interface CloudPulseTags { @@ -17,6 +18,7 @@ export interface CloudPulseTagsSelectProps { label: string; optional?: boolean; placeholder?: string; + region: FilterValueType; resourceType: string | undefined; savePreferences?: boolean; } @@ -25,48 +27,63 @@ export const CloudPulseTagsSelect = React.memo( (props: CloudPulseTagsSelectProps) => { const { defaultValue, + disabled, handleTagsChange, label, optional, placeholder, + region, resourceType, savePreferences, } = props; - const { data: linodes, isError, isLoading } = useAllLinodesQuery(); + const regionFilter = region ? (region as string) : undefined; + const { data: linodesByRegion, isError, isLoading } = useAllLinodesQuery( + {}, + { region: regionFilter }, + !disabled && Boolean(region && resourceType) + ); + // fetch all linode instances, consume the associated tags const tags = React.useMemo(() => { - if (!linodes) { - return []; + if (!linodesByRegion) { + return undefined; } + return Array.from( - new Set(linodes.flatMap((linode: Linode) => linode.tags)) + new Set(linodesByRegion.flatMap((linode: Linode) => linode.tags)) ) .sort() .map((tag) => ({ label: tag })) as CloudPulseTags[]; - }, [linodes]); + }, [linodesByRegion]); const [selectedTags, setSelectedTags] = React.useState(); const isAutocompleteOpen = React.useRef(false); // Ref to track the open state of Autocomplete React.useEffect(() => { - if (!tags || !savePreferences) { - setSelectedTags([]); - handleTagsChange([]); + if (disabled && !selectedTags) { return; } - const defaultTags = - defaultValue && Array.isArray(defaultValue) - ? defaultValue.map((tag) => String(tag)) - : []; - const filteredTags = tags.filter((tag) => - defaultTags.includes(String(tag.label)) - ); - handleTagsChange(filteredTags); - setSelectedTags(filteredTags); + // To save default values, go through side effects if disabled is false + if (!tags || !savePreferences || selectedTags) { + if (selectedTags) { + setSelectedTags([]); + handleTagsChange([]); + } + } else { + const defaultTags = + defaultValue && Array.isArray(defaultValue) + ? defaultValue.map((tag) => String(tag)) + : []; + const filteredTags = tags.filter((tag) => + defaultTags.includes(String(tag.label)) + ); + handleTagsChange(filteredTags); + setSelectedTags(filteredTags); + } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [tags, resourceType]); + }, [tags, resourceType, region, disabled]); return ( diff --git a/packages/manager/src/mocks/serverHandlers.ts b/packages/manager/src/mocks/serverHandlers.ts index cfc65d826e7..4a96a2ec512 100644 --- a/packages/manager/src/mocks/serverHandlers.ts +++ b/packages/manager/src/mocks/serverHandlers.ts @@ -687,6 +687,11 @@ export const handlers = [ label: 'linode_with_no_tags', region: 'us-central', }), + linodeFactory.build({ + label: 'linode_with_tag_test4', + region: 'us-east', + tags: ['test4'], + }), linodeFactory.build({ label: 'eu-linode', region: 'eu-west', @@ -704,6 +709,7 @@ export const handlers = [ const headers = JSON.parse(request.headers.get('x-filter') || '{}'); const orFilters = headers['+or']; const andFilters = headers['+and']; + const regionFilter = headers.region; let filteredLinodes = linodes; // Default to the original linodes in case no filters are applied @@ -730,6 +736,12 @@ export const handlers = [ }); } + if (regionFilter) { + filteredLinodes = filteredLinodes.filter((linode) => { + return linode.region === regionFilter; + }); + } + return HttpResponse.json(makeResourcePage(filteredLinodes)); } return HttpResponse.json(makeResourcePage(linodes));