Skip to content

Commit

Permalink
fix(ui) Fix duplicate schema field rendering with siblings (datahub-p…
Browse files Browse the repository at this point in the history
  • Loading branch information
chriscollins3456 authored and shirshanka committed Jan 18, 2023
1 parent 5cf0d75 commit bdc13ce
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { dataset3WithLineage, dataset4WithLineage } from '../../../../Mocks';
import { EntityType } from '../../../../types.generated';
import { EntityType, SchemaFieldDataType } from '../../../../types.generated';
import {
combineEntityDataWithSiblings,
combineSiblingsInSearchResults,
Expand Down Expand Up @@ -106,6 +106,26 @@ const datasetUnprimary = {
},
],
},
schemaMetadata: {
...dataset4WithLineage.schemaMetadata,
fields: [
{
__typename: 'SchemaField',
nullable: false,
recursive: false,
fieldPath: 'new_one',
description: 'Test to make sure fields merge works',
type: SchemaFieldDataType.String,
nativeDataType: 'varchar(100)',
isPartOfKey: false,
jsonPath: null,
globalTags: null,
glossaryTerms: null,
label: 'hi',
},
...(dataset4WithLineage.schemaMetadata?.fields || []),
],
},
siblings: {
isPrimary: false,
},
Expand Down Expand Up @@ -471,6 +491,12 @@ describe('siblingUtils', () => {
expect(combinedData.dataset.globalTags.tags[0].tag.urn).toEqual('urn:li:tag:unprimary-tag');
expect(combinedData.dataset.globalTags.tags[1].tag.urn).toEqual('urn:li:tag:primary-tag');

// merges schema metadata properly by fieldPath
expect(combinedData.dataset.schemaMetadata?.fields).toHaveLength(3);
expect(combinedData.dataset.schemaMetadata?.fields[0].fieldPath).toEqual('new_one');
expect(combinedData.dataset.schemaMetadata?.fields[1].fieldPath).toEqual('user_id');
expect(combinedData.dataset.schemaMetadata?.fields[2].fieldPath).toEqual('user_name');

// will overwrite string properties w/ primary
expect(combinedData.dataset.editableProperties.description).toEqual('secondary description');

Expand Down
22 changes: 20 additions & 2 deletions datahub-web-react/src/app/entity/shared/siblingUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import merge from 'deepmerge';
import { unionBy } from 'lodash';
import { unionBy, keyBy, values } from 'lodash';
import { useLocation } from 'react-router-dom';
import * as QueryString from 'query-string';
import { Entity, MatchedField, Maybe, SiblingProperties } from '../../../types.generated';
Expand Down Expand Up @@ -51,6 +51,11 @@ const combineMerge = (target, source, options) => {
return destination;
};

// use when you want to merge and array of objects by key in the object as opposed to by index of array
const mergeArrayOfObjectsByKey = (destinationArray: any[], sourceArray: any[], key: string) => {
return values(merge(keyBy(destinationArray, key), keyBy(sourceArray, key)));
};

const mergeTags = (destinationArray, sourceArray, _options) => {
return unionBy(destinationArray, sourceArray, 'tag.urn');
};
Expand All @@ -71,6 +76,10 @@ const mergeOwners = (destinationArray, sourceArray, _options) => {
return unionBy(destinationArray, sourceArray, 'owner.urn');
};

const mergeFields = (destinationArray, sourceArray, _options) => {
return mergeArrayOfObjectsByKey(destinationArray, sourceArray, 'fieldPath');
};

function getArrayMergeFunction(key) {
switch (key) {
case 'tags':
Expand All @@ -83,6 +92,8 @@ function getArrayMergeFunction(key) {
return mergeProperties;
case 'owners':
return mergeOwners;
case 'fields':
return mergeFields;
default:
return undefined;
}
Expand All @@ -96,7 +107,14 @@ const customMerge = (isPrimary, key) => {
if (key === 'platform' || key === 'siblings') {
return (secondary, primary) => (isPrimary ? primary : secondary);
}
if (key === 'tags' || key === 'terms' || key === 'assertions' || key === 'customProperties' || key === 'owners') {
if (
key === 'tags' ||
key === 'terms' ||
key === 'assertions' ||
key === 'customProperties' ||
key === 'owners' ||
key === 'fields'
) {
return (secondary, primary) => {
return merge(secondary, primary, {
arrayMerge: getArrayMergeFunction(key),
Expand Down

0 comments on commit bdc13ce

Please sign in to comment.