Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ui): two small ux fixes #6335

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -92,7 +92,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 === 'tags' || key === 'terms' || key === 'assertions' || key === 'customProperties' || key === 'owners') {
Expand Down Expand Up @@ -122,6 +123,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 @@ -137,7 +156,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 @@ -206,6 +206,9 @@ fragment nonSiblingDatasetFields on Dataset {
}
}
}
siblings {
isPrimary
}
}

mutation updateDataset($urn: String!, $input: DatasetUpdateInput!) {
Expand Down