From fcbd4e8c3876bc156440ad65fbd649da12bdf77e Mon Sep 17 00:00:00 2001 From: Akanksha Trehun Date: Tue, 18 Aug 2026 15:24:53 +0530 Subject: [PATCH 1/2] fix(keda): sort Jobs list by completions, not parallelism sortByCompletions in the Jobs list actually sorted by parallelism first and only fell back to completions on a tie, despite the name saying otherwise. Also swapped in nullish coalescing so a job missing either field doesn't turn the comparator into NaN. Exported it so it's testable on its own. Signed-off-by: Akanksha Trehun --- .../common/CommonComponents.test.ts | 25 +++++++++++++++++++ .../components/common/CommonComponents.tsx | 16 ++++++------ 2 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 keda/src/components/common/CommonComponents.test.ts diff --git a/keda/src/components/common/CommonComponents.test.ts b/keda/src/components/common/CommonComponents.test.ts new file mode 100644 index 0000000000..ba71f40daf --- /dev/null +++ b/keda/src/components/common/CommonComponents.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from 'vitest'; +import { sortByCompletions } from './CommonComponents'; + +function job(completions?: number, parallelism?: number) { + return { spec: { completions, parallelism } } as any; +} + +describe('sortByCompletions', () => { + it('sorts by completions, not parallelism, when they disagree', () => { + // job1 has fewer completions but more parallelism than job2 — a completions-first + // sort must put job1 before job2, the opposite of what a parallelism-first sort would do. + expect(sortByCompletions(job(1, 5), job(5, 1))).toBeLessThan(0); + }); + + it('falls back to parallelism when completions are equal', () => { + expect(sortByCompletions(job(2, 1), job(2, 4))).toBeLessThan(0); + }); + + it('treats missing completions/parallelism as 0 instead of NaN', () => { + expect(sortByCompletions(job(undefined, undefined), job(1, 1))).toBeLessThan(0); + expect( + Number.isNaN(sortByCompletions(job(undefined, undefined), job(undefined, undefined))) + ).toBe(false); + }); +}); diff --git a/keda/src/components/common/CommonComponents.tsx b/keda/src/components/common/CommonComponents.tsx index f25cfea7b8..ed51e4e3c8 100644 --- a/keda/src/components/common/CommonComponents.tsx +++ b/keda/src/components/common/CommonComponents.tsx @@ -564,6 +564,14 @@ export interface JobsListRendererProps { noNamespaceFilter?: boolean; } +export function sortByCompletions(job1: Job, job2: Job) { + const completionsSorted = (job1.spec.completions ?? 0) - (job2.spec.completions ?? 0); + if (completionsSorted === 0) { + return (job1.spec.parallelism ?? 0) - (job2.spec.parallelism ?? 0); + } + return completionsSorted; +} + export function JobsListRenderer(props: JobsListRendererProps) { const { jobs, errors, hideColumns = [], reflectTableInURL = 'jobs', noNamespaceFilter } = props; const { t } = useTranslation(); @@ -572,14 +580,6 @@ export function JobsListRenderer(props: JobsListRendererProps) { return `${job.spec.completions}/${job.spec.parallelism}`; } - function sortByCompletions(job1: Job, job2: Job) { - const parallelismSorted = job1.spec.parallelism - job2.spec.parallelism; - if (parallelismSorted === 0) { - return job1.spec.completions - job2.spec.completions; - } - return parallelismSorted; - } - return ( Date: Wed, 19 Aug 2026 14:52:45 +0530 Subject: [PATCH 2/2] fix(keda): guard against undefined completions/parallelism in the display too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit good catch on review — the sort treated missing fields as 0 but the cell still rendered them as literal "undefined", so the two disagreed about what a Job with neither field set should look like. same guard, applied to the render path this time. pulled getCompletions out to module scope so it's actually testable, same as sortByCompletions already was. Signed-off-by: Akanksha Trehun --- keda/src/components/common/CommonComponents.test.ts | 12 +++++++++++- keda/src/components/common/CommonComponents.tsx | 8 ++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/keda/src/components/common/CommonComponents.test.ts b/keda/src/components/common/CommonComponents.test.ts index ba71f40daf..647cd97798 100644 --- a/keda/src/components/common/CommonComponents.test.ts +++ b/keda/src/components/common/CommonComponents.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { sortByCompletions } from './CommonComponents'; +import { getCompletions, sortByCompletions } from './CommonComponents'; function job(completions?: number, parallelism?: number) { return { spec: { completions, parallelism } } as any; @@ -23,3 +23,13 @@ describe('sortByCompletions', () => { ).toBe(false); }); }); + +describe('getCompletions', () => { + it('renders both fields when set', () => { + expect(getCompletions(job(1, 5))).toBe('1/5'); + }); + + it('renders 0 instead of undefined for unset fields, matching how they sort', () => { + expect(getCompletions(job(undefined, undefined))).toBe('0/0'); + }); +}); diff --git a/keda/src/components/common/CommonComponents.tsx b/keda/src/components/common/CommonComponents.tsx index ed51e4e3c8..9258d8734d 100644 --- a/keda/src/components/common/CommonComponents.tsx +++ b/keda/src/components/common/CommonComponents.tsx @@ -572,14 +572,14 @@ export function sortByCompletions(job1: Job, job2: Job) { return completionsSorted; } +export function getCompletions(job: Job) { + return `${job.spec.completions ?? 0}/${job.spec.parallelism ?? 0}`; +} + export function JobsListRenderer(props: JobsListRendererProps) { const { jobs, errors, hideColumns = [], reflectTableInURL = 'jobs', noNamespaceFilter } = props; const { t } = useTranslation(); - function getCompletions(job: Job) { - return `${job.spec.completions}/${job.spec.parallelism}`; - } - return (