-
Notifications
You must be signed in to change notification settings - Fork 167
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
[RHOAIENG-2987] Artifacts - Details Page #2765
[RHOAIENG-2987] Artifacts - Details Page #2765
Conversation
0d90b66
to
1e4968f
Compare
frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactDetails.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good! just the two comments
frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactUriLink.tsx
Outdated
Show resolved
Hide resolved
bc85867
to
93687a8
Compare
All lgtm, one last thing, could you please add the link to the artifact details page here? https://github.com/opendatahub-io/odh-dashboard/blob/main/frontend/src/pages/pipelines/global/experiments/executions/details/ExecutionDetailsInputOutputSection.tsx#L75-L76. I left it in my previously merged PR because the artifact details page was not created at that time. Thanks a lot! |
93687a8
to
6782833
Compare
tested again and the json looks good now /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Gkrumbach07 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactOverviewDetails.tsx
Outdated
Show resolved
Hide resolved
...rc/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue.tsx
Show resolved
Hide resolved
6782833
to
9eeeae6
Compare
993a2fd
to
36bb0fe
Compare
36bb0fe
to
76bea1b
Compare
/lgtm |
Closes: RHOAIENG-2987
Description
Details page for artifacts accessed by clicking the table, then clicking on the name of the artifact. The page only renders the Overview tab and its data. The JIRA asked for an empty state for Lineage explorer, but since there is no design screen showing what that might look like, I've just disabled the tab for now.
How Has This Been Tested?
Added unit tests that verifies data being rendered on the page since Cypress does not support intercepting application/grpc-web+proto content type requests.
I've also added a simple unit test for the artifacts list table which mocks and renders some data.
How to test
Navigate to the artifacts table from the left nav, click on an artifact name, verify you're routed to the details page, verify the visual matches the design and the data is correct.
(cc @yannnz)Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main