Skip to content

Commit 2800da6

Browse files
committed
fix(navigation): make kindToPlural idempotent on 'hpas' shortname
Bugbot HIGH: kindToPlural('hpas') was falling through to englishPlural which appends 'es' (trailing s) → 'hpases', which matched nothing in KNOWN_KINDS or the dispatch table. Since ResourcesSidebar uses { kind: 'hpas' } as a primary key (the dispatch even hard-codes 'hpas' || 'horizontalpodautoscalers' for HPARenderer), clicking HPAs from the sidebar silently rendered nothing — the same 'URL updates but no detail panel' regression this PR set out to fix for CronJobs. Add 'hpas' to BUILTIN_PLURAL_TO_KIND so kindToPlural('hpas') hits the idempotence path and returns unchanged. Pin with a regression test referencing the dispatch coupling. Also: extract formatZoomLabel to packages/k8s-ui/src/utils/zoom-label.ts so the test imports the production implementation instead of duplicating it (the inline copy gave no regression protection). TimelineSwimlanes re-exports from the new shared module. Strip SKY-826 / bug-9 trailers from the renderer-dispatch comments per CLAUDE.md (same pattern flagged on #584 / #572). Made-with: Cursor
1 parent 63adf11 commit 2800da6

6 files changed

Lines changed: 55 additions & 50 deletions

File tree

packages/k8s-ui/src/components/shared/ResourceRendererDispatch.tsx

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -354,17 +354,13 @@ export function ResourceRendererDispatch({
354354
resolvedEnvFrom,
355355
rendererOverrides,
356356
}: ResourceRendererDispatchProps) {
357-
// Normalize the incoming `kind` to the lowercase plural form the
358-
// dispatch table is keyed on. Without this, a caller that passes
359-
// "CronJob" (PascalCase singular — e.g. from a topology node click,
360-
// or any code path that hands the registry the singular kind name)
361-
// produces `kind = 'cronjob'` after lowercasing, which doesn't
362-
// match `'cronjobs'` and so silently renders nothing — the user
363-
// saw the URL update but no detail panel. (SKY-826 bug 9)
364-
//
365-
// `kindToPlural` is idempotent: it returns already-plural inputs
366-
// unchanged, so callers that already pass `'cronjobs'` are not
367-
// double-pluralised.
357+
// Callers reach this dispatch from two shapes: lowercase plural
358+
// (resource list rows: 'cronjobs') and PascalCase singular
359+
// (topology node clicks: 'CronJob'). The dispatch table is keyed
360+
// only on lowercase plurals, so a raw .toLowerCase() leaves the
361+
// singular form mismatched and nothing renders. kindToPlural
362+
// covers both shapes idempotently (already-plural inputs return
363+
// unchanged), so this single normalisation handles every caller.
368364
const kind = kindToPlural(resource.kind)
369365

370366
const isKnownKind = KNOWN_KINDS.has(kind)
@@ -567,9 +563,8 @@ export function ResourceRendererDispatch({
567563

568564
export function getResourceStatus(kind: string, data: any): { text: string; color: string } | null {
569565
if (!data) return null
570-
// Normalise to lowercase plural for the same reason the renderer
571-
// dispatch does — singular kinds like "CronJob" otherwise produce
572-
// a no-match here too. (SKY-826 bug 9)
566+
// Same normalisation as the renderer dispatch above — singular
567+
// kinds like "CronJob" otherwise produce a no-match here too.
573568
const k = kindToPlural(kind)
574569

575570
if (k === 'pods') return getPodStatus(data)

packages/k8s-ui/src/utils/navigation.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,19 @@ describe('kindToPlural', () => {
5656
expect(kindToPlural('horizontalpodautoscalers')).toBe('horizontalpodautoscalers')
5757
})
5858

59+
test('idempotent on dispatch-keyed shortnames (hpas)', () => {
60+
// Bugbot regression for PR #585: ResourcesSidebar uses
61+
// `{ kind: 'hpas' }` as a primary key (not a query alias) and the
62+
// dispatch table at ResourceRendererDispatch matches both 'hpas'
63+
// AND 'horizontalpodautoscalers'. Without an explicit entry in
64+
// BUILTIN_PLURAL_TO_KIND, kindToPlural('hpas') falls through to
65+
// englishPlural which sees a trailing 's' and adds 'es' →
66+
// 'hpases', matching nothing. The detail panel then silently
67+
// renders empty for HPAs — the exact CronJob-shaped bug this
68+
// PR exists to prevent, just for a different kind. Pin it.
69+
expect(kindToPlural('hpas')).toBe('hpas')
70+
})
71+
5972
test('handles aliases', () => {
6073
expect(kindToPlural('HorizontalPodAutoscaler')).toBe('horizontalpodautoscalers')
6174
expect(kindToPlural('pvc')).toBe('persistentvolumeclaims')

packages/k8s-ui/src/utils/navigation.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ const BUILTIN_PLURAL_TO_KIND: Record<string, string> = {
3434
clusterrolebindings: 'ClusterRoleBinding',
3535
serviceaccounts: 'ServiceAccount',
3636
networkpolicies: 'NetworkPolicy',
37+
// The codebase uses the kubectl shortname `hpas` as a *primary* dispatch
38+
// key (see ResourcesSidebar `{ kind: 'hpas' }` and the dispatch table in
39+
// ResourceRendererDispatch). Listing it here makes `kindToPlural('hpas')`
40+
// return `'hpas'` unchanged via the idempotence check below — without
41+
// this entry, `englishPlural('hpas')` adds "es" → `'hpases'` (ends in s)
42+
// and the HPA detail panel renders nothing.
43+
hpas: 'HorizontalPodAutoscaler',
3744
}
3845

3946
// Dynamic map built from API discovery — populated by initNavigationMap().
Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,10 @@
11
import { describe, it, expect } from 'vitest'
2+
import { formatZoomLabel } from './zoom-label'
23

3-
// Inline copy of the helper from web/src/components/timeline/TimelineSwimlanes.tsx
4-
// (same module — re-declared here to avoid pulling the swimlanes
5-
// React component into the unit-test boundary). If the rules drift,
6-
// update both. The shared rule it pins:
7-
//
8-
// < 1h → "<m>m"
9-
// < 24h → "<n>h"
10-
// >= 24h → "<n>d"
11-
//
12-
// SKY-826 bug 12 turned a static "1h window" label into a real
13-
// dropdown of preset windows. The picker and the previous-render
14-
// label have to format identically or the user sees flicker between
15-
// "60m" (label) and "1h" (popover option) for the same zoom.
16-
function formatZoomLabel(zoom: number): string {
17-
if (zoom < 1) return `${Math.round(zoom * 60)}m`
18-
if (zoom >= 24) return `${Math.round(zoom / 24)}d`
19-
return `${zoom}h`
20-
}
21-
4+
// Pin the timeline-window label format. The picker and the
5+
// previous-render label MUST format identically or the user sees
6+
// flicker between "60m" (label) and "1h" (popover option) for the
7+
// same zoom.
228
describe('formatZoomLabel (timeline window picker)', () => {
239
it('formats sub-hour zooms as minutes', () => {
2410
expect(formatZoomLabel(0.25)).toBe('15m')
@@ -39,9 +25,7 @@ describe('formatZoomLabel (timeline window picker)', () => {
3925
})
4026

4127
it('rounds (does not truncate) values at the bucket boundaries', () => {
42-
// 0.49 * 60 = 29.4 → rounds to 29
4328
expect(formatZoomLabel(0.49)).toBe('29m')
44-
// 35h / 24 = 1.458... → rounds to 1d (closer to 1 than 2)
4529
expect(formatZoomLabel(35)).toBe('1d')
4630
})
4731
})
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Format a zoom-window value (in hours) for display next to the
3+
* timeline. Pure helper so the popover and the rest of the toolbar
4+
* agree on the same string.
5+
*
6+
* Rules:
7+
* < 1h → "<m>m"
8+
* < 24h → "<n>h"
9+
* >= 24h → "<n>d"
10+
*
11+
* Lives in the shared utils package (not the web/ component file)
12+
* so the unit test can import the production implementation
13+
* directly instead of duplicating it.
14+
*/
15+
export function formatZoomLabel(zoom: number): string {
16+
if (zoom < 1) return `${Math.round(zoom * 60)}m`
17+
if (zoom >= 24) return `${Math.round(zoom / 24)}d`
18+
return `${zoom}h`
19+
}

web/src/components/timeline/TimelineSwimlanes.tsx

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -173,21 +173,8 @@ function calculateInterestingnessWithBreakdown(lane: ResourceLane): ScoreBreakdo
173173
return breakdown
174174
}
175175

176-
/**
177-
* Format a zoom-window value (in hours) for display next to the
178-
* timeline. Pure helper so the popover and the rest of the toolbar
179-
* agree on the same string. (SKY-826 bug 12)
180-
*
181-
* Rules:
182-
* < 1h → "<m>m"
183-
* < 24h → "<n>h"
184-
* >= 24h → "<n>d"
185-
*/
186-
export function formatZoomLabel(zoom: number): string {
187-
if (zoom < 1) return `${Math.round(zoom * 60)}m`
188-
if (zoom >= 24) return `${Math.round(zoom / 24)}d`
189-
return `${zoom}h`
190-
}
176+
import { formatZoomLabel } from '@skyhook-io/k8s-ui/utils/zoom-label'
177+
export { formatZoomLabel }
191178

192179
interface ZoomWindowPickerProps {
193180
zoom: number

0 commit comments

Comments
 (0)