Skip to content

Commit

Permalink
fix(ui) Add backwards compatibility to the UI for old policy filters (#…
Browse files Browse the repository at this point in the history 
…12017)
  • Loading branch information
@chriscollins3456
chriscollins3456 authored Dec 11, 2024
1 parent d062411 commit 11c49ec
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 16 deletions.
8 changes: 5 additions & 3 deletions datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 21 additions & 8 deletions datahub-web-react/src/app/permissions/policy/PolicyPrivilegeForm.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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),
),
});
Expand All @@ -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,
Expand All @@ -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),
),
});
Expand Down
160 changes: 159 additions & 1 deletion datahub-web-react/src/app/permissions/policy/_tests_/policyUtils.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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' }],
},
],
});
});
});
4 changes: 4 additions & 0 deletions datahub-web-react/src/app/permissions/policy/constants.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const TYPE = 'TYPE';
export const RESOURCE_TYPE = 'RESOURCE_TYPE';
export const URN = 'URN';
export const RESOURCE_URN = 'RESOURCE_URN';
24 changes: 20 additions & 4 deletions datahub-web-react/src/app/permissions/policy/policyUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 11c49ec

Please sign in to comment.