Skip to content

Commit

Permalink
upcoming: [DI-22941] - Update dependency and order for tags (#11615)
Browse files Browse the repository at this point in the history
* upcoming: [DI-22941] - Update dependency and order for tags

* upcoming: [DI-22941] - Type fix

* upcoming: [DI-22941] - Xfilter update

* upcoming: [DI-22941] - Small revert

* upcoming: [DI-22941] - Preferences fix

* upcoming: [DI-22941] - PR comment

* upcoming: [DI-22941] - Mock enhancement

* upcoming: [DI-22941]: Preferences fix for resources

* upcoming: [DI-22941] - UT fix

* upcoming: [DI-22941] - Linting fix

* upcoming: [DI-22941] - Comments

* upcoming: [DI-22941] - Comments

* upcoming: [DI-22941] - Comments

* upcoming: [DI-22941] - Comments

* upcoming: [DI-22941] - UT fix

* upcoming: [DI-22941] - Add changeset

* upcoming: [DI-22941] - PR comment - improve quality of unit test

* upcoming: [DI-22941] - PR comment - improve comment

---------

Co-authored-by: vmangalr <[email protected]>
  • Loading branch information
ankita-akamai and vmangalr authored Feb 6, 2025
1 parent 8914a3b commit c14b95b
Show file tree
Hide file tree
Showing 9 changed files with 238 additions and 67 deletions.
Original file line number Diff line number Diff line change
@@ -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))
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
getMetricsCallCustomFilters,
getRegionProperties,
getResourcesProperties,
getTagsProperties,
getTimeDurationProperties,
} from './FilterBuilder';
import { deepEqual, getFilters } from './FilterBuilder';
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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',
Expand All @@ -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', () => {
Expand Down
32 changes: 28 additions & 4 deletions packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,21 +54,34 @@ 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
*/
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,
};
Expand Down Expand Up @@ -133,7 +146,8 @@ export const getResourcesProperties = (
disabled: checkIfWeNeedToDisableFilterByFilterKey(
filterKey,
dependentFilters ?? {},
dashboard
dashboard,
preferences
),
handleResourcesSelection: handleResourceChange,
label,
Expand Down Expand Up @@ -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);
Expand All @@ -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 ||
Expand Down
23 changes: 12 additions & 11 deletions packages/manager/src/features/CloudPulse/Utils/FilterConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,31 @@ export const LINODE_CONFIG: Readonly<CloudPulseServiceTypeFilterMap> = {
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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo(
selectedTags,
savePref,
{
[RESOURCE_ID]: undefined,
[TAGS]: selectedTags,
}
);
Expand Down Expand Up @@ -171,6 +172,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo(
const updatedPreferenceData = {
[REGION]: region,
[RESOURCES]: undefined,
[TAGS]: undefined,
};
emitFilterChangeByFilterKey(
REGION,
Expand Down Expand Up @@ -209,6 +211,7 @@ export const CloudPulseDashboardFilterBuilder = React.memo(
{
config,
dashboard,
dependentFilters: dependentFilterReference.current,
isServiceAnalyticsIntegration,
preferences,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit c14b95b

Please sign in to comment.