-
Notifications
You must be signed in to change notification settings - Fork 18
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
[3/n] Refactor workflow details page to use v2 nodes route #1310
[3/n] Refactor workflow details page to use v2 nodes route #1310
Conversation
4196cf7
to
80babf5
Compare
… eng-2699-m1-refactor-wf-detail-pages-to-use-new-3
… eng-2699-m1-refactor-wf-detail-pages-to-use-new-3
(state: RootState) => state.nodeSelectionReducer.selected | ||
const selectedNodeState = useSelector( | ||
(state: RootState) => | ||
state.workflowPageReducer.perWorkflowPageStates[workflowId]?.SelectedNode |
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.
what does this perWorkflowPageStates do? Does it manage the sidesheet state?
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.
After getting deep into most refactors, I think the only thing it's managing is the selected node (which controls whether to show and which sidesheet to show). It doesn't further manage any page states, which will be controlled by page components or already handled by RTK
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.
sounds good
@@ -120,6 +120,7 @@ func NewArtifactFromDBObject(dbArtifactNode *views.ArtifactNode) *Artifact { | |||
type ArtifactResult struct { | |||
// Contains only the `result`. It mostly mirrors 'artifact_result' schema. | |||
ID uuid.UUID `json:"id"` | |||
ArtifactID uuid.UUID `json:"artifact_id"` |
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.
… eng-2699-m1-refactor-wf-detail-pages-to-use-new-3
Describe your changes and why you are making these changes
This PR refactors workflow details page to use v2 nodes route. We mainly handle node selection and retrieval logic to directly use
nodes
from V2 endpoints, rather than the convolutedoperators
andartifacts
objects inselectedDag
reducer state.We are not yet touching the node rendering part, which is a huge change and will include in a separate PR.
Related issue number (if any)
Loom demo (if any)
Checklist before requesting a review
python3 scripts/run_linters.py -h
for usage).run_integration_test
: Runs integration testsskip_integration_test
: Skips integration tests (Should be used when changes are ONLY documentation/UI)