Skip to content

Commit

Permalink
fix: Cannot rely of Object.values to return ordered properties
Browse files Browse the repository at this point in the history
* Since pods are fetched from an Object hashed by their uids, it is not
  certain that the pods will be correctly sorted when the sort button is
  clicked. Need to explicitly sort them.

* Discover.tsx
* context.ts
 * Adds a podOrder state object to context so that the DiscoverToolbar can
   update it and have it used when groupPods() is called

* DiscoverToolbar.tsx
 * Set the pod order before telling the mgmt service to sort

* discover-project.ts
 * When refreshing have it take into account the pod order that has been
   requested
  • Loading branch information
phantomjinx committed Oct 1, 2024
1 parent ab5c1ab commit aae104a
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 15 deletions.
16 changes: 14 additions & 2 deletions packages/online-shell/src/discover/Discover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,18 @@ import { DiscoverLoadingPanel } from './DiscoverLoadingPanel'
import { DiscoverEmptyContent } from './DiscoverEmptyContent'

export const Discover: React.FunctionComponent = () => {
const { error, isLoading, refreshing, setRefreshing, discoverProjects, setDiscoverProjects, filter, setFilter } =
useDisplayItems()
const {
error,
isLoading,
refreshing,
setRefreshing,
discoverProjects,
setDiscoverProjects,
filter,
setFilter,
podOrder,
setPodOrder,
} = useDisplayItems()
const [activeTabKey, setActiveTabKey] = React.useState<string>('')

const handleTabClick = (
Expand Down Expand Up @@ -76,6 +86,8 @@ export const Discover: React.FunctionComponent = () => {
setDiscoverProjects,
filter,
setFilter,
podOrder,
setPodOrder,
}}
>
<DiscoverToolbar />
Expand Down
4 changes: 3 additions & 1 deletion packages/online-shell/src/discover/DiscoverToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ const sortMeta = {
}

export const DiscoverToolbar: React.FunctionComponent = () => {
const { discoverProjects, setDiscoverProjects, filter, setFilter, setRefreshing } = useContext(DiscoverContext)
const { discoverProjects, setDiscoverProjects, filter, setFilter, setRefreshing, setPodOrder } =
useContext(DiscoverContext)
// Ref for toggle of filter type Select control
const filterTypeToggleRef = useRef<HTMLButtonElement | null>()

Expand Down Expand Up @@ -201,6 +202,7 @@ export const DiscoverToolbar: React.FunctionComponent = () => {
* in order to correctly sort all pods and get back the right
* set according to the paging limit
*/
setPodOrder(newSortOrderIcon.type)
setRefreshing(true)
mgmtService.sort(newSortOrderIcon.type)
}
Expand Down
26 changes: 22 additions & 4 deletions packages/online-shell/src/discover/context.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createContext, useCallback, useEffect, useRef, useState } from 'react'
import { TypeFilter } from '@hawtio/online-kubernetes-api'
import { TypeFilter, SortOrder } from '@hawtio/online-kubernetes-api'
import { MgmtActions, isMgmtApiRegistered, mgmtService } from '@hawtio/online-management-api'
import { discoverService } from './discover-service'
import { DiscoverProject } from './discover-project'
Expand All @@ -20,14 +20,15 @@ export function useDisplayItems() {

// type filter created by filter control and displayed as chips
const [filter, setFilter] = useState<TypeFilter>(new TypeFilter())
const [podOrder, setPodOrder] = useState<SortOrder>()

const organisePods = useCallback(() => {
const discoverProjects = discoverService.groupPods()
const discoverProjects = discoverService.groupPods(podOrder)
setDiscoverProjects([...discoverProjects])

setIsLoading(false)
setRefreshing(false)
}, [])
}, [podOrder])

useEffect(() => {
const waitLoading = async () => {
Expand Down Expand Up @@ -64,7 +65,18 @@ export function useDisplayItems() {
}
}, [organisePods])

return { error, isLoading, refreshing, setRefreshing, discoverProjects, setDiscoverProjects, filter, setFilter }
return {
error,
isLoading,
refreshing,
setRefreshing,
discoverProjects,
setDiscoverProjects,
filter,
setFilter,
podOrder,
setPodOrder,
}
}

type DiscoverContext = {
Expand All @@ -74,6 +86,8 @@ type DiscoverContext = {
setDiscoverProjects: (projects: DiscoverProject[]) => void
filter: TypeFilter
setFilter: (filter: TypeFilter) => void
podOrder?: SortOrder
setPodOrder: (podOrder: SortOrder) => void
}

export const DiscoverContext = createContext<DiscoverContext>({
Expand All @@ -89,4 +103,8 @@ export const DiscoverContext = createContext<DiscoverContext>({
setFilter: () => {
// no-op
},
podOrder: undefined,
setPodOrder: () => {
// no-op
},
})
35 changes: 30 additions & 5 deletions packages/online-shell/src/discover/discover-project.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ManagedPod } from '@hawtio/online-management-api'
import { OwnerReference } from '@hawtio/online-kubernetes-api'
import { OwnerReference, SortOrder } from '@hawtio/online-kubernetes-api'
import { DiscoverGroup, DiscoverPod, DiscoverType } from './globals'

export type DiscoverProjects = {
Expand All @@ -15,8 +15,9 @@ export class DiscoverProject {
private projectName: string,
fullPodCount: number,
mgmtPods: ManagedPod[],
podOrder?: SortOrder,
) {
this.refresh(fullPodCount, mgmtPods)
this.refresh(fullPodCount, mgmtPods, podOrder)
}

private podOwner(pod: ManagedPod): OwnerReference | null {
Expand Down Expand Up @@ -67,7 +68,7 @@ export class DiscoverProject {
return values[key]
}

refresh(fullPodCount: number, pods: ManagedPod[]) {
refresh(fullPodCount: number, pods: ManagedPod[], podOrder?: SortOrder) {
const discoverPods: DiscoverPod[] = []
const discoverGroups: DiscoverGroup[] = []

Expand All @@ -94,8 +95,32 @@ export class DiscoverProject {
discoverGroups.push(this.toDiscoverGroup(pod, ownerRef, replicas))
}

this.discoverGroups = discoverGroups
this.discoverPods = discoverPods
if (!podOrder) {
// No sort order
this.discoverGroups = discoverGroups
this.discoverPods = discoverPods
} else {
// Sort order imposed so sort groups, pods-in-groups and pods
const sortPodFn = (pod1: DiscoverPod, pod2: DiscoverPod) => {
const name1 = pod1.mPod.metadata?.name || ''
const name2 = pod2.mPod.metadata?.name || ''

let value = name1.localeCompare(name2)
return podOrder === SortOrder.DESC ? (value *= -1) : value
}

this.discoverGroups = discoverGroups.sort((grp1: DiscoverGroup, grp2: DiscoverGroup) => {
let value = grp1.name.localeCompare(grp2.name)
return podOrder === SortOrder.DESC ? (value *= -1) : value
})

this.discoverGroups.forEach(group => {
group.replicas = group.replicas.sort(sortPodFn)
})

this.discoverPods = discoverPods.sort(sortPodFn)
}

this._fullPodCount = fullPodCount

/**
Expand Down
9 changes: 6 additions & 3 deletions packages/online-shell/src/discover/discover-service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { mgmtService, ManagedPod } from '@hawtio/online-management-api'
import { DiscoverPod } from './globals'
import { DiscoverProject, DiscoverProjects } from './discover-project'
import { SortOrder } from '@hawtio/online-kubernetes-api'

export enum ViewType {
listView = 'listView',
Expand All @@ -10,17 +11,19 @@ export enum ViewType {
class DiscoverService {
private discoverProjects: DiscoverProjects = {}

groupPods(): DiscoverProject[] {
groupPods(podOrder?: SortOrder): DiscoverProject[] {
const projectNames: string[] = []

Object.values(mgmtService.projects).forEach(mgmtProject => {
const pods: ManagedPod[] = Object.values(mgmtProject.pods)

projectNames.push(mgmtProject.name)

if (!this.discoverProjects[mgmtProject.name]) {
const discoverProject = new DiscoverProject(mgmtProject.name, mgmtProject.fullPodCount, pods)
const discoverProject = new DiscoverProject(mgmtProject.name, mgmtProject.fullPodCount, pods, podOrder)
this.discoverProjects[mgmtProject.name] = discoverProject
} else {
this.discoverProjects[mgmtProject.name].refresh(mgmtProject.fullPodCount, pods)
this.discoverProjects[mgmtProject.name].refresh(mgmtProject.fullPodCount, pods, podOrder)
}
})

Expand Down

0 comments on commit aae104a

Please sign in to comment.