diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index 37349585fa4c92..1988fea3496992 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -12,6 +12,7 @@ import { mapResourceTypeToDisplayName, } from './policyUtils'; import AvatarsGroup from '../AvatarsGroup'; +import { RESOURCE_TYPE, RESOURCE_URN, TYPE, URN } from './constants'; type PrivilegeOptionType = { type?: string; @@ -72,10 +73,11 @@ export default function PolicyDetailsModal({ policy, open, onClose, privileges } const isMetadataPolicy = policy?.type === PolicyType.Metadata; const resources = convertLegacyResourceFilter(policy?.resources); - const resourceTypes = getFieldValues(resources?.filter, 'TYPE') || []; + const resourceTypes = getFieldValues(resources?.filter, TYPE, RESOURCE_TYPE) || []; const dataPlatformInstances = getFieldValues(resources?.filter, 'DATA_PLATFORM_INSTANCE') || []; - const resourceEntities = getFieldValues(resources?.filter, 'URN') || []; - const resourceFilterCondition = getFieldCondition(resources?.filter, 'URN') || PolicyMatchCondition.Equals; + const resourceEntities = getFieldValues(resources?.filter, URN, RESOURCE_URN) || []; + const resourceFilterCondition = + getFieldCondition(resources?.filter, URN, RESOURCE_URN) || PolicyMatchCondition.Equals; const domains = getFieldValues(resources?.filter, 'DOMAIN') || []; const { diff --git a/datahub-web-react/src/app/permissions/policy/PolicyPrivilegeForm.tsx b/datahub-web-react/src/app/permissions/policy/PolicyPrivilegeForm.tsx index 7a0de67f414192..414346c2776db8 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyPrivilegeForm.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyPrivilegeForm.tsx @@ -28,6 +28,7 @@ import ClickOutside from '../../shared/ClickOutside'; import { TagTermLabel } from '../../shared/tags/TagTermLabel'; import { ENTER_KEY_CODE } from '../../shared/constants'; import { useGetRecommendations } from '../../shared/recommendation'; +import { RESOURCE_TYPE, RESOURCE_URN, TYPE, URN } from './constants'; type Props = { policyType: PolicyType; @@ -102,8 +103,9 @@ export default function PolicyPrivilegeForm({ } = useAppConfig(); const resources: ResourceFilter = convertLegacyResourceFilter(maybeResources) || EMPTY_POLICY.resources; - const resourceTypes = getFieldValues(resources.filter, 'TYPE') || []; - const resourceEntities = getFieldValues(resources.filter, 'URN') || []; + // RESOURCE_TYPE and RESOURCE_URN are deprecated, but need to get them for backwards compatibility + const resourceTypes = getFieldValues(resources.filter, TYPE, RESOURCE_TYPE) || []; + const resourceEntities = getFieldValues(resources.filter, URN, RESOURCE_URN) || []; const getDisplayName = (entity) => { if (!entity) { @@ -178,9 +180,14 @@ export default function PolicyPrivilegeForm({ const filter = resources.filter || { criteria: [], }; + // remove the deprecated RESOURCE_TYPE field and replace with TYPE field + const filterWithoutDeprecatedField = setFieldValues(filter, RESOURCE_TYPE, []); setResources({ ...resources, - filter: setFieldValues(filter, 'TYPE', [...resourceTypes, createCriterionValue(selectedResourceType)]), + filter: setFieldValues(filterWithoutDeprecatedField, TYPE, [ + ...resourceTypes, + createCriterionValue(selectedResourceType), + ]), }); }; @@ -188,11 +195,13 @@ export default function PolicyPrivilegeForm({ const filter = resources.filter || { criteria: [], }; + // remove the deprecated RESOURCE_TYPE field and replace with TYPE field + const filterWithoutDeprecatedField = setFieldValues(filter, RESOURCE_TYPE, []); setResources({ ...resources, filter: setFieldValues( - filter, - 'TYPE', + filterWithoutDeprecatedField, + TYPE, resourceTypes?.filter((criterionValue) => criterionValue.value !== deselectedResourceType), ), }); @@ -203,9 +212,11 @@ export default function PolicyPrivilegeForm({ const filter = resources.filter || { criteria: [], }; + // remove the deprecated RESOURCE_URN field and replace with URN field + const filterWithoutDeprecatedField = setFieldValues(filter, RESOURCE_URN, []); setResources({ ...resources, - filter: setFieldValues(filter, 'URN', [ + filter: setFieldValues(filterWithoutDeprecatedField, URN, [ ...resourceEntities, createCriterionValueWithEntity( resource, @@ -220,11 +231,13 @@ export default function PolicyPrivilegeForm({ const filter = resources.filter || { criteria: [], }; + // remove the deprecated RESOURCE_URN field and replace with URN field + const filterWithoutDeprecatedField = setFieldValues(filter, RESOURCE_URN, []); setResources({ ...resources, filter: setFieldValues( - filter, - 'URN', + filterWithoutDeprecatedField, + URN, resourceEntities?.filter((criterionValue) => criterionValue.value !== resource), ), }); diff --git a/datahub-web-react/src/app/permissions/policy/_tests_/policyUtils.test.tsx b/datahub-web-react/src/app/permissions/policy/_tests_/policyUtils.test.tsx index 1c9884e5fcf09c..eae735b3477f0f 100644 --- a/datahub-web-react/src/app/permissions/policy/_tests_/policyUtils.test.tsx +++ b/datahub-web-react/src/app/permissions/policy/_tests_/policyUtils.test.tsx @@ -1,4 +1,12 @@ -import { addOrUpdatePoliciesInList, updateListPoliciesCache, removeFromListPoliciesCache } from '../policyUtils'; +import { PolicyMatchCondition } from '../../../../types.generated'; +import { + addOrUpdatePoliciesInList, + updateListPoliciesCache, + removeFromListPoliciesCache, + getFieldValues, + getFieldCondition, + setFieldValues, +} from '../policyUtils'; // Mock the Apollo Client readQuery and writeQuery methods const mockReadQuery = vi.fn(); @@ -103,3 +111,153 @@ describe('removeFromListPoliciesCache', () => { }); }); }); + +describe('getFieldValues', () => { + it('should get field values for a given field', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataset' }, { value: 'dataJob' }], + }, + ], + }; + + expect(getFieldValues(filter, 'TYPE')).toMatchObject([{ value: 'dataset' }, { value: 'dataJob' }]); + }); + + it('should get field values for a alternate field (for deprecated fields)', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'RESOURCE_TYPE', + values: [{ value: 'dataset' }, { value: 'dataJob' }], + }, + ], + }; + + expect(getFieldValues(filter, 'TYPE', 'RESOURCE_TYPE')).toMatchObject([ + { value: 'dataset' }, + { value: 'dataJob' }, + ]); + }); + + it('should get field values for main field with alternative field given and has values', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'RESOURCE_TYPE', + values: [{ value: 'container' }, { value: 'dataFlow' }], + }, + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataset' }, { value: 'dataJob' }], + }, + ], + }; + + // should only return values from main field + expect(getFieldValues(filter, 'TYPE', 'RESOURCE_TYPE')).toMatchObject([ + { value: 'dataset' }, + { value: 'dataJob' }, + ]); + }); +}); + +describe('getFieldCondition', () => { + it('should get field values for a given field', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataset' }], + }, + ], + }; + + expect(getFieldCondition(filter, 'TYPE')).toBe(PolicyMatchCondition.Equals); + }); + + it('should get field values for a alternate field (for deprecated fields)', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'RESOURCE_TYPE', + values: [{ value: 'dataset' }], + }, + ], + }; + + expect(getFieldCondition(filter, 'TYPE', 'RESOURCE_TYPE')).toBe(PolicyMatchCondition.Equals); + }); + + it('should get field values for main field with alternative field given and has values', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.StartsWith, + field: 'RESOURCE_TYPE', + values: [{ value: 'container' }, { value: 'dataFlow' }], + }, + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataset' }], + }, + ], + }; + + // should only return values from main field + expect(getFieldCondition(filter, 'TYPE', 'RESOURCE_TYPE')).toBe(PolicyMatchCondition.Equals); + }); +}); +describe('setFieldValues', () => { + it('should remove a field if you pass in an empty array', () => { + const filter = { + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'RESOURCE_TYPE', + values: [{ value: 'dataset' }], + }, + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataJob' }], + }, + ], + }; + + expect(setFieldValues(filter, 'RESOURCE_TYPE', [])).toMatchObject({ + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataJob' }], + }, + ], + }); + }); + + it('should set values for a field properly', () => { + const filter = { + criteria: [], + }; + + expect(setFieldValues(filter, 'TYPE', [{ value: 'dataFlow' }])).toMatchObject({ + criteria: [ + { + condition: PolicyMatchCondition.Equals, + field: 'TYPE', + values: [{ value: 'dataFlow' }], + }, + ], + }); + }); +}); diff --git a/datahub-web-react/src/app/permissions/policy/constants.ts b/datahub-web-react/src/app/permissions/policy/constants.ts new file mode 100644 index 00000000000000..cdd20bf9b50d6a --- /dev/null +++ b/datahub-web-react/src/app/permissions/policy/constants.ts @@ -0,0 +1,4 @@ +export const TYPE = 'TYPE'; +export const RESOURCE_TYPE = 'RESOURCE_TYPE'; +export const URN = 'URN'; +export const RESOURCE_URN = 'RESOURCE_URN'; diff --git a/datahub-web-react/src/app/permissions/policy/policyUtils.ts b/datahub-web-react/src/app/permissions/policy/policyUtils.ts index b71a38f80fc256..d6221a0a9293af 100644 --- a/datahub-web-react/src/app/permissions/policy/policyUtils.ts +++ b/datahub-web-react/src/app/permissions/policy/policyUtils.ts @@ -114,12 +114,28 @@ export const convertLegacyResourceFilter = (resourceFilter: Maybe | undefined, resourceFieldType: string) => { - return filter?.criteria?.find((criterion) => criterion.field === resourceFieldType)?.values || []; +export const getFieldValues = ( + filter: Maybe | undefined, + resourceFieldType: string, + alternateResourceFieldType?: string, +) => { + return ( + filter?.criteria?.find((criterion) => criterion.field === resourceFieldType)?.values || + filter?.criteria?.find((criterion) => criterion.field === alternateResourceFieldType)?.values || + [] + ); }; -export const getFieldCondition = (filter: Maybe | undefined, resourceFieldType: string) => { - return filter?.criteria?.find((criterion) => criterion.field === resourceFieldType)?.condition || null; +export const getFieldCondition = ( + filter: Maybe | undefined, + resourceFieldType: string, + alternateResourceFieldType?: string, +) => { + return ( + filter?.criteria?.find((criterion) => criterion.field === resourceFieldType)?.condition || + filter?.criteria?.find((criterion) => criterion.field === alternateResourceFieldType)?.condition || + null + ); }; export const getFieldValuesOfTags = (filter: Maybe | undefined, resourceFieldType: string) => {