fix(ui) Add backwards compatibility to the UI for old policy filters (d…

…atahub-project#12017)
G-Research · Dec 11, 2024 · 11c49ec · 11c49ec
1 parent d062411
commit 11c49ec
Show file tree

Hide file tree

Showing 5 changed files with 209 additions and 16 deletions.
diff --git 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
@@ -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,21 +180,28 @@ 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),
+            ]),
         });
     };
 
     const onDeselectResourceType = (deselectedResourceType: string) => {
         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
@@ -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
@@ -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
@@ -114,12 +114,28 @@ export const convertLegacyResourceFilter = (resourceFilter: Maybe<ResourceFilter
     };
 };
 
-export const getFieldValues = (filter: Maybe<PolicyMatchFilter> | undefined, resourceFieldType: string) => {
-    return filter?.criteria?.find((criterion) => criterion.field === resourceFieldType)?.values || [];
+export const getFieldValues = (
+    filter: Maybe<PolicyMatchFilter> | 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<PolicyMatchFilter> | undefined, resourceFieldType: string) => {
-    return filter?.criteria?.find((criterion) => criterion.field === resourceFieldType)?.condition || null;
+export const getFieldCondition = (
+    filter: Maybe<PolicyMatchFilter> | 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<PolicyMatchFilter> | undefined, resourceFieldType: string) => {