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

feat(glossary) Display Incoming 'IsA' Glossary related entities #5063

Conversation

chriscollins3456
Copy link
Collaborator

@chriscollins3456 chriscollins3456 commented Jun 1, 2022

Now displays related entities that are related to any Incoming "IsA" glossary terms on a parent term's related entities tab.

For example:
TermA inherits from TermB (TermA has an Outgoing "IsA" relationship defined with TermA) and therefore shows TermA in its Inherits tab. If you go to TermB's profile, you will see all entities related to TermA in TermB's Related Entities tab.

The way the the Related Terms tab works, though, is that we only show Outgoing "IsA" and "HasA" relationships (Inherits and Contains, respectively). So TermB may not show TermA in its Related Terms tab.

Note: This is only 1 level down, so we only show direct Incoming "IsA" relationship children's related entities (much easier and quicker to implement than going all the way through the tree)

A visual example - Container Term has two Incoming "IsA" relationships with Child Term and 2nd Child Term and therefore will show all of their related entities in its own tab as well as its own.
image

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@xiphl
Copy link
Contributor

xiphl commented Jun 1, 2022

Just want to take the opportunity to clarify, to add relatedTerms to a term, we would still need to do ingestion via backend right? There is no way/plans to add via UI?

@github-actions
Copy link

github-actions bot commented Jun 1, 2022

Unit Test Results (build & test)

334 tests  ±0   334 ✔️ ±0   3m 37s ⏱️ +14s
  78 suites ±0       0 💤 ±0 
  78 files   ±0       0 ±0 

Results for commit 243e617. ± Comparison against base commit 3c7c83f.

♻️ This comment has been updated with latest results.

@chriscollins3456
Copy link
Collaborator Author

Just want to take the opportunity to clarify, to add relatedTerms to a term, we would still need to do ingestion via backend right? There is no way/plans to add via UI?

@xiphl that is correct - for now! We recognize that that is desired and is scheduled as a followup to happen soon. So you should be able to Add/Delete Contains/Inherits relationships via the UI.

@nesho25
Copy link

nesho25 commented Jun 6, 2022

Just want to take the opportunity to clarify, to add relatedTerms to a term, we would still need to do ingestion via backend right? There is no way/plans to add via UI?

@xiphl that is correct - for now! We recognize that that is desired and is scheduled as a followup to happen soon. So you should be able to Add/Delete Contains/Inherits relationships via the UI.

This is highly needed from my side. Looking forward to it so i could let my data stewards work with business glossary independently and effectively via UI. Hope it will arrive soon!

@@ -28,6 +28,19 @@ query getGlossaryTerm($urn: String!, $start: Int, $count: Int) {
}
}
}
childTerms: relationships(input: { types: ["IsA"], direction: INCOMING, start: $start, count: $count }) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: This field may be confusing because there are technically both "IsA" and "HasA" children terms from this one.

Would it be clearer to say

isAChildren?

or

hasAChildren?

Just curious to hear your thoughts. We are also okay to do this as a followup

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that makes sense - I think isAChildren for this case is pretty understandable and clear - will make a quick change and push up

Copy link
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM

@chriscollins3456 chriscollins3456 force-pushed the cc--glossary-related-entities branch from d9f5ae1 to 243e617 Compare June 6, 2022 15:41
@jjoyce0510
Copy link
Collaborator

Will merge once CI passes.

@jjoyce0510 jjoyce0510 merged commit 64c4f51 into datahub-project:master Jun 6, 2022
maggiehays pushed a commit to maggiehays/datahub that referenced this pull request Aug 1, 2022
…hub-project#5063)

* feat(glossary) Display Incoming 'IsA' Glossary related entities

* update name of field

Co-authored-by: Chris Collins <[email protected]>
Co-authored-by: Chris Collins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants