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.

Sign up for GitHub

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

Jump to bottom

feat(ui): add option to add picture link for groups #9882

Merged
merged 16 commits into from
Mar 5, 2024
Merged

feat(ui): add option to add picture link for groups #9882

merged 16 commits into from
Mar 5, 2024

Conversation

akarsh991
Copy link
Contributor

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

@github-actions github-actions bot added product PR or Issue related to the DataHub UI/UX community-contribution PR or Issue raised by member(s) of DataHub Community labels Feb 19, 2024
@akarsh991 akarsh991 marked this pull request as draft February 19, 2024 10:49
@akarsh991 akarsh991 marked this pull request as ready for review February 20, 2024 13:15
@asikowitz asikowitz removed the community-contribution PR or Issue raised by member(s) of DataHub Community label Feb 22, 2024
anshbansal
anshbansal previously approved these changes Feb 26, 2024
View reviewed changes
Copy link
Collaborator

@anshbansal anshbansal left a comment

Choose a reason for hiding this comment

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

LGTM. Wait for CI before merge

@anshbansal anshbansal added the merge-pending-ci A PR that has passed review and should be merged once CI is green. label Feb 26, 2024
@anshbansal anshbansal changed the title feat(group-info): add picture link for update group info feat(graphql): add picture link for update group info Feb 26, 2024
@anshbansal anshbansal dismissed their stale review February 27, 2024 14:05

Failing CI

@anshbansal anshbansal removed the merge-pending-ci A PR that has passed review and should be merged once CI is green. label Feb 27, 2024
@jjoyce0510
Copy link
Collaborator

nice PR. once comments are addressed this is looking good

anshbansal
anshbansal reviewed Feb 28, 2024
View reviewed changes
.../com/linkedin/datahub/graphql/types/corpgroup/mappers/CorpGroupEditablePropertiesMapper.java Outdated
Comment on lines 34 to 36
result.setPictureLink(
(Objects.requireNonNull(corpGroupEditableInfo.getPictureLink(GetMode.DEFAULT))).toString());

Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens for existing cases where picture link might not be set? Have you tested opening the UI for an existing group?

@anshbansal anshbansal mentioned this pull request Feb 28, 2024
Closed
5 tasks
@anshbansal anshbansal changed the title feat(graphql): add picture link for update group info feat(ui): add option to add picture link for groups Mar 5, 2024
@anshbansal anshbansal merged commit a85db60 into datahub-project:master Mar 5, 2024
38 checks passed
dushayntAW pushed a commit to dushayntAW/datahub that referenced this pull request Mar 7, 2024
@akarsh991 @gaurav2733 @dushayntAW
feat(ui): add option to add picture link for groups (datahub-project#…
35d9b78 
…9882)

Co-authored-by: gaurav2733 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anshbansal anshbansal anshbansal approved these changes

Assignees
No one assigned
Labels
product PR or Issue related to the DataHub UI/UX
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@akarsh991 @jjoyce0510 @anshbansal @asikowitz @gaurav2733