Skip to content

fix(UI): Enforce required URL validation for Institutional Memory #13548

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
Closed
pinakipb2 wants to merge 10 commits into from
Closed

fix(UI): Enforce required URL validation for Institutional Memory #13548

pinakipb2 wants to merge 10 commits into from
+0 −3

Conversation

@pinakipb2
Copy link
Contributor

@pinakipb2 pinakipb2 commented May 19, 2025

Recording of defect

Screen.Recording.2025-05-19.at.12.31.07.PM.mov

Summary

Previously, setting warningOnly: true was causing the required URL validation to be skipped entirely. This PR updates the logic to ensure that validation for required fields is applied.

As a result of this fix:

  • The required URL validation is now properly enforced.
  • The corresponding error is displayed in the UI as expected.

@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 May 19, 2025
@datahub-cyborg datahub-cyborg bot added the needs-review Label for PRs that need review from a maintainer. label May 19, 2025
@codecov
Copy link

codecov bot commented Jun 10, 2025

Bundle Report

Changes will decrease total bundle size by 45 bytes (-0.0%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
datahub-react-web-esm 19.68MB -45 bytes (-0.0%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: datahub-react-web-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/index-*.js -45 bytes 16.06MB -0.0%

Files in assets/index-*.js:

  • ./src/app/entity/shared/tabs/Documentation/components/LinkList.tsx → Total Size: 6.05kB

  • ./src/app/entity/shared/components/styled/AddLinkModal.tsx → Total Size: 3.12kB

  • ./src/app/entityV2/shared/components/styled/AddLinkModal.tsx → Total Size: 3.87kB

@codecov
Copy link

codecov bot commented Jun 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

jjoyce0510
jjoyce0510 reviewed Jun 23, 2025
View reviewed changes
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.

The only thing I'd watch out for is custom URL routing, e.g.

s3://path/to/file

Or something similar that is not HTTP / HTTPS. This might break based on your changes, but I'm assuming some customers are leveraging. Worth consideration before we merge.

@jjoyce0510
Copy link
Collaborator

jjoyce0510 commented Jun 23, 2025

If these will still work, we are fine to approve and merge.

@datahub-cyborg datahub-cyborg bot added pending-submitter-response Issue/request has been reviewed but requires a response from the submitter and removed needs-review Label for PRs that need review from a maintainer. labels Jun 23, 2025
@rtekal
Copy link
Contributor

rtekal commented Jun 30, 2025

Thanks to Meenakshi: With Data Profile V2 - which is a new aspect now, this PR is no longer required.

@datahub-cyborg datahub-cyborg bot added needs-review Label for PRs that need review from a maintainer. and removed pending-submitter-response Issue/request has been reviewed but requires a response from the submitter labels Jun 30, 2025
@pinakipb2 pinakipb2 closed this Jun 30, 2025
@pinakipb2 pinakipb2 deleted the pb-link-error branch June 30, 2025 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjoyce0510 jjoyce0510 jjoyce0510 left review comments

Assignees

No one assigned

Labels

community-contribution PR or Issue raised by member(s) of DataHub Community needs-review Label for PRs that need review from a maintainer. 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.

3 participants

@pinakipb2 @jjoyce0510 @rtekal