fix: Pass context to resource tree (#5468) #9401
Conversation
|
Hi @rbreeze, please review this. Two of the menu actions on the pod within the pod group do not work because of it. I also included the fix for the sonarcloud warning. While fixing this, I noticed the pod's menu is not consistent. The new |
Codecov Report
@@ Coverage Diff @@
## master #9401 +/- ##
==========================================
- Coverage 46.22% 45.78% -0.45%
==========================================
Files 218 220 +2
Lines 25914 26170 +256
==========================================
+ Hits 11978 11981 +3
- Misses 12278 12531 +253
Partials 1658 1658
Continue to review full report at Codecov.
|
keithchong
force-pushed
the
5468-bugPassContextToResourceTree
branch
from
d055096
to
007df3e
Compare
| @@ -327,6 +327,7 @@ export const describeNode = (node: ResourceTreeNode) => { | |||
|
|
|||
| function processPodGroup(targetPodGroup: ResourceTreeNode, child: ResourceTreeNode, props: ApplicationResourceTreeProps) { | |||
| const statusByKey = new Map<string, models.ResourceStatus>(); | |||
| props.app.status?.resources?.forEach(res => statusByKey.set(nodeKey(res), res)); | |||
There was a problem hiding this comment.
Is it possible to construct the map more expressively in a single line, rather than creating and then populating the dictionary?
There was a problem hiding this comment.
Hi @crenshaw-dev , I cleaned this up some more. So a bit of context first. This pod group is different than the pod group in the Pods view. I have a part two issue, #9338, to make the pod group a common component. The pod group in the tree/network view is different currently, because the layout makes the animation a lot nicer, and the placement of the various statuses are consistent with the other tree nodes. (See videos in the original PR #8996)
The resourceStatus and status in question are actually not used here when I render the pod group. I used the 'existing' way to render the status, here:
Compare it to the pods view, where it uses this resourceStatus property:
and
So to finally answer your question, for now, we can just get rid of that map. I've updated the PR. We'll have to refactor this when we do: #9338
Signed-off-by: Keith Chong <kykchong@redhat.com>
keithchong
force-pushed
the
5468-bugPassContextToResourceTree
branch
from
007df3e
to
8be597d
Compare
Signed-off-by: Keith Chong kykchong@redhat.com
For the PR #8996, the details page is supposed to pass the
contextto the resource tree. This is needed otherwise the menu actions on the pod will not work. (It's missing perhaps when I rebased (?) or for some other reason. It's already defined in theApplicationResourceTreePropsproperties for this very purpose.) This fix also includes the change to fix the sonarcloud issue where the map is empty.This is a bug fix with no open issue. It (the context) should have been part of the above PR to fix issue #5468.
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: