Fix workload overview status sorting #3486
Conversation
Signed-off-by: Alex Andreev <[email protected]>
Signed-off-by: Alex Andreev <[email protected]>
Signed-off-by: Alex Andreev <[email protected]>
aleksfront
requested review from
nevalla and
Nokel81
and removed request for
a team
| @@ -80,7 +80,7 @@ export class PodsStore extends KubeObjectStore<Pod> { | |||
| } | |||
|
|
|||
| getStatuses(pods: Pod[]) { | |||
| return countBy(pods.map(pod => pod.getStatus())); | |||
| return countBy(pods.map(pod => pod.getStatus()).sort().reverse()); | |||
There was a problem hiding this comment.
This will automatically sort statuses in a needed order:
Succeeded -> Running -> Pending -> Failed -> Evicted
| @@ -0,0 +1,115 @@ | |||
| /** | |||
There was a problem hiding this comment.
All tests sit in parent /components/ folder until this #3481 won't be resolved. Otherwise it's hard run (read impossible) tests locally.
| @@ -34,7 +34,7 @@ export class CronJobStore extends KubeObjectStore<CronJob> { | |||
| } | |||
|
|
|||
| getStatuses(cronJobs?: CronJob[]) { | |||
| const status = { suspended: 0, scheduled: 0 }; | |||
There was a problem hiding this comment.
Why does the order matter?
There was a problem hiding this comment.
This is an object, so I think we should specify the order by extracting to an array. Or at least add a comment above all of these saying that the order is important.
There was a problem hiding this comment.
Since we're decided to put all "green" statuses on top (show them first), I've moved scheduled to be first one since it's "green".
There was a problem hiding this comment.
I still think that should be something that is commented or specified somewhere else. Since objects are key/value structures which don't really have a conceptual ordering.
There was a problem hiding this comment.
Yes, objects didn't have conceptual ordering, but they're actually have an order for now. The results are always the same + we have tests now checking ordering.
Here's mentions about object ordering https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map#objects_vs._maps
As an alternative, I've converted statuses to use Maps, but this looks more messy and verbose without visible benefits.
There was a problem hiding this comment.
Okay, I was thinking more of having a defined ordering in overview-workload-status.tsx but that might be a bit too much. Maybe something for the future.
I don't particularly like the fact that this ordering is required to be static but I concur that should be for the future.
Running,Succeeded,Scheduledgoes on top).Before:
After:
Fixes #3423