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) Fix broken dataPlatformInstance references in browseV2 #8485

Conversation

chriscollins3456
Copy link
Collaborator

With the new search and browse experiences, we introduced dataPlatformInstances to a new place where they are treated like full-blown entities. For example, a dataPlatformInstance urn will often be in browse paths. We then try to use the entity registry in many different places to render an entity name, but would get an error thrown if we tried to do that for a dataPlatformInstance.

This PR adds DataPlatformInstanceEntitiy to the frontend entity registry so we can render the display name properly. The other pieces are unimportant and will be unused entirely (we never try to render a profile for a data platform instance). What's more, is that if we ever tried to do any of these methods anywhere else in the app, the app would just crash like it did when I tried to get entity name so this is definitely a win.

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

@chriscollins3456 chriscollins3456 changed the title fix(ui) Fix broken dataPlatformInstance references fix(ui) Fix broken dataPlatformInstance references in browseV2 Jul 21, 2023
@github-actions github-actions bot added the product PR or Issue related to the DataHub UI/UX label Jul 21, 2023
@@ -42,7 +43,7 @@ const BrowseNode = () => {
const platformAggregation = usePlatformAggregation();
const browseResultGroup = useBrowseResultGroup();
const { count, entity } = browseResultGroup;
const hasEntityLink = !!entity;
const hasEntityLink = !!entity && entity.type !== EntityType.DataPlatformInstance;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch

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.

LGTM. i've been expecting we'd have to do this. evetually we can finish it out and have a proper container-style page for this

@anshbansal anshbansal merged commit 4f961da into datahub-project:master Jul 26, 2023
31 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants