Skip to content

Commit

Permalink
Merge pull request datahub-project#983 from acryldata/gabe--fixingTwo…
Browse files Browse the repository at this point in the history
…NitsForRH

fix(robinhood): two ux fixes for robinhood (also going to OSS)
  • Loading branch information
gabe-lyons authored Nov 1, 2022
2 parents 52d7456 + 5c7e8c9 commit 42e88e0
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { TagSummary } from './shared/TagSummary';
import { TermSummary } from './shared/TermSummary';
import { FIELDS_TO_HIGHLIGHT } from './search/highlights';
import { getMatchPrioritizingPrimary } from '../shared/utils';
import { downgradeV2FieldPath } from './profile/schema/utils/utils';

type Props = {
matchedFields: MatchedField[];
Expand All @@ -23,6 +24,8 @@ export const DatasetSearchSnippet = ({ matchedFields }: Props) => {
snippet = <TagSummary urn={matchedField.value} />;
} else if (matchedField.value.includes('urn:li:glossaryTerm')) {
snippet = <TermSummary urn={matchedField.value} />;
} else if (matchedField.name === 'fieldPaths') {
snippet = <b>{downgradeV2FieldPath(matchedField.value)}</b>;
} else {
snippet = <b>{matchedField.value}</b>;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { dataset3WithLineage, dataset4WithLineage } from '../../../../Mocks';
import { EntityType } from '../../../../types.generated';
import { combineEntityDataWithSiblings, combineSiblingsInSearchResults } from '../siblingUtils';
import {
combineEntityDataWithSiblings,
combineSiblingsInSearchResults,
shouldEntityBeTreatedAsPrimary,
} from '../siblingUtils';

const usageStats = {
buckets: [
Expand Down Expand Up @@ -70,7 +74,11 @@ const datasetPrimary = {
},
],
},
siblings: {
isPrimary: true,
},
};

const datasetUnprimary = {
...dataset4WithLineage,
usageStats,
Expand Down Expand Up @@ -98,6 +106,9 @@ const datasetUnprimary = {
},
],
},
siblings: {
isPrimary: false,
},
};

const datasetPrimaryWithSiblings = {
Expand All @@ -108,6 +119,22 @@ const datasetPrimaryWithSiblings = {
},
};

const datasetUnprimaryWithPrimarySiblings = {
...datasetUnprimary,
siblings: {
isPrimary: false,
siblings: [datasetPrimary],
},
};

const datasetUnprimaryWithNoPrimarySiblings = {
...datasetUnprimary,
siblings: {
isPrimary: false,
siblings: [datasetUnprimary],
},
};

const searchResultWithSiblings = [
{
entity: {
Expand Down Expand Up @@ -430,17 +457,9 @@ const searchResultWithSiblings = [
},
];

// const datasetUnprimaryWithSiblings = {
// ...datasetUnprimary,
// siblings: {
// isPrimary: true,
// siblings: [datasetPrimary],
// },
// };

describe('siblingUtils', () => {
describe('combineEntityDataWithSiblings', () => {
it('combines my metadata with my siblings', () => {
it('combines my metadata with my siblings as primary', () => {
const baseEntity = { dataset: datasetPrimaryWithSiblings };
expect(baseEntity.dataset.usageStats).toBeNull();
const combinedData = combineEntityDataWithSiblings(baseEntity);
Expand All @@ -457,6 +476,17 @@ describe('siblingUtils', () => {

// will take secondary string properties in the case of empty string
expect(combinedData.dataset.properties.description).toEqual('primary description');

// will stay primary
expect(combinedData.dataset.siblings.isPrimary).toBeTruthy();
});

it('combines my metadata with my siblings as secondary', () => {
const baseEntity = { dataset: datasetUnprimaryWithPrimarySiblings };
const combinedData = combineEntityDataWithSiblings(baseEntity);

// will stay secondary
expect(combinedData.dataset.siblings.isPrimary).toBeFalsy();
});
});

Expand All @@ -473,4 +503,18 @@ describe('siblingUtils', () => {
);
});
});

describe('shouldEntityBeTreatedAsPrimary', () => {
it('will say a primary entity is primary', () => {
expect(shouldEntityBeTreatedAsPrimary(datasetPrimaryWithSiblings)).toBeTruthy();
});

it('will say a un-primary entity is not primary', () => {
expect(shouldEntityBeTreatedAsPrimary(datasetUnprimaryWithPrimarySiblings)).toBeFalsy();
});

it('will say a un-primary entity is primary if it has no primary sibling', () => {
expect(shouldEntityBeTreatedAsPrimary(datasetUnprimaryWithNoPrimarySiblings)).toBeTruthy();
});
});
});
24 changes: 22 additions & 2 deletions datahub-web-react/src/app/entity/shared/siblingUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ const customMerge = (isPrimary, key) => {
if (key === 'upstream' || key === 'downstream') {
return (_secondary, primary) => primary;
}
if (key === 'platform') {
// take the platform & siblings of whichever entity we're merging with, rather than the primary
if (key === 'platform' || key === 'siblings') {
return (secondary, primary) => (isPrimary ? primary : secondary);
}
if (key === 'testResults') {
Expand Down Expand Up @@ -141,6 +142,24 @@ export const getEntitySiblingData = <T>(baseEntity: T): Maybe<SiblingProperties>
return extractedBaseEntity?.['siblings'];
};

// should the entity's metadata win out against its siblings?
export const shouldEntityBeTreatedAsPrimary = (extractedBaseEntity: { siblings?: SiblingProperties | null }) => {
const siblingAspect = extractedBaseEntity?.siblings;

const siblingsList = siblingAspect?.siblings || [];

// if the entity is marked as primary, take its metadata first
const isPrimarySibling = !!siblingAspect?.isPrimary;

// if no entity in the cohort is primary, just have the entity whos urn is navigated
// to be primary
const hasAnyPrimarySibling = siblingsList.find((sibling) => !!(sibling as any)?.siblings?.isPrimary) !== undefined;

const isPrimary = isPrimarySibling || !hasAnyPrimarySibling;

return isPrimary;
};

export const combineEntityDataWithSiblings = <T>(baseEntity: T): T => {
if (!baseEntity) {
return baseEntity;
Expand All @@ -156,7 +175,8 @@ export const combineEntityDataWithSiblings = <T>(baseEntity: T): T => {

// eslint-disable-next-line @typescript-eslint/dot-notation
const siblings: T[] = siblingAspect?.siblings || [];
const isPrimary = !!extractedBaseEntity?.siblings?.isPrimary;

const isPrimary = shouldEntityBeTreatedAsPrimary(extractedBaseEntity);

const combinedBaseEntity: any = siblings.reduce(
(prev, current) =>
Expand Down
3 changes: 3 additions & 0 deletions datahub-web-react/src/graphql/dataset.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@ fragment nonSiblingDatasetFields on Dataset {
totalIncidents: incidents(start: 0, count: 1) {
total
}
siblings {
isPrimary
}
}

fragment siblingFields on Dataset {
Expand Down

0 comments on commit 42e88e0

Please sign in to comment.