From a0bda5712ca326d7232334084e053ec4b82b6178 Mon Sep 17 00:00:00 2001 From: Tom Schneider Date: Wed, 10 Sep 2025 00:54:45 +0200 Subject: [PATCH] fix(sonarqube): sorting fails on missing data in `SonarQubeRelatedEntitiesOverview` if there are rows without an annotation, meaning no data, then the sorting does not work as intended. The sort-behavior mimics `Array.prototype.sort()`, with undefined values sorted to the end. Signed-off-by: Tom Schneider --- .../sonarqube/.changeset/curly-plums-own.md | 6 + .../SonarQubeTable/Columns.test.tsx | 186 ++++++++++++++++++ .../src/components/SonarQubeTable/Columns.tsx | 69 ++++++- 3 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 workspaces/sonarqube/.changeset/curly-plums-own.md create mode 100644 workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.test.tsx diff --git a/workspaces/sonarqube/.changeset/curly-plums-own.md b/workspaces/sonarqube/.changeset/curly-plums-own.md new file mode 100644 index 0000000000..b386a6fe76 --- /dev/null +++ b/workspaces/sonarqube/.changeset/curly-plums-own.md @@ -0,0 +1,6 @@ +--- +'@backstage-community/plugin-sonarqube': patch +--- + +Fixed bug in `SonarQubeRelatedEntitiesOverview` table component where the sorting of columns does not work properly when +there are components without an annotation diff --git a/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.test.tsx b/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.test.tsx new file mode 100644 index 0000000000..6d35dd03e0 --- /dev/null +++ b/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.test.tsx @@ -0,0 +1,186 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { datetimeSort, numericSort } from './Columns.tsx'; + +describe('datetimeSort', () => { + it.each([ + [new Date(2025, 1), new Date(2026, 2)], + [new Date(2019, 3), new Date(2024, 5)], + [new Date(2019, 2, 1), new Date(2019, 2, 2)], + ])(`should return negative number when %o < %o`, (a, b) => { + const result = datetimeSort(x => x)( + a.toISOString(), + b.toISOString(), + undefined, + ); + expect(result).toBeLessThan(0); + }); + + it.each([ + [new Date(2026, 1), new Date(2026, 1)], + [new Date(2011, 9, 13), new Date(2011, 9, 13)], + ])(`should return 0 when %o === %o`, (a, b) => { + const result = datetimeSort(x => x)( + a.toISOString(), + b.toISOString(), + undefined, + ); + expect(result).toBe(0); + }); + + it.each([ + [new Date(2026, 7), new Date(2025, 1)], + [new Date(2024, 2), new Date(2019, 10)], + [new Date(2019, 2, 2), new Date(2019, 2, 1)], + ])(`should return positive number when %o > %o`, (a, b) => { + const result = datetimeSort(x => x)( + a.toISOString(), + b.toISOString(), + undefined, + ); + expect(result).toBeGreaterThan(0); + }); + + describe('invalid params', () => { + const validDateString = '2025-09-08T10:04:46+0200'; + const invalidParams = [ + undefined, + null, + '', + '.', + 'text', + '$/}', + '1970--01--01', + ]; + + it.each(invalidParams)( + `should return negative number when second param is not-a-number %o`, + invalid => { + const result = datetimeSort(x => x)( + validDateString, + invalid, + undefined, + ); + expect(result).toBeLessThan(0); + }, + ); + + it.each(invalidParams)( + `should return 0 when both params are not-a-number %o`, + invalid => { + const result = datetimeSort(x => x)(invalid, invalid, undefined); + expect(result).toBe(0); + }, + ); + + it.each(invalidParams)( + `should return positive number when first param is not-a-number %o`, + invalid => { + const result = datetimeSort(x => x)( + invalid, + validDateString, + undefined, + ); + expect(result).toBeGreaterThan(0); + }, + ); + + it(`should return 0 when both params are different not-a-number`, () => { + const result = datetimeSort(x => x)('a', 'b', undefined); + expect(result).toBe(0); + }); + }); + + it('should execute a deeply nested accessor', () => { + const obj = { one: { two: { three: '4' } } }; + const accessor = (x: any) => x.one.two.three; + const result = datetimeSort(accessor)(obj, obj, undefined); + expect(result).toBe(0); + }); +}); + +describe('numericSort', () => { + it.each([ + ['0.9', '1'], + ['1', '2'], + ['2', '5'], + ])(`should return negative number when %d < %d`, (a, b) => { + const result = numericSort(x => x)(a, b, undefined); + expect(result).toBeLessThan(0); + }); + + it.each([ + ['', ''], + ['0', '0'], + ['75.1', '75.1'], + ])(`should return 0 when %o === %o`, (a, b) => { + const result = numericSort(x => x)(a, b, undefined); + expect(result).toBe(0); + }); + + it.each([ + ['2.2', '2'], + ['2', '1'], + ['9', '5'], + ])(`should return positive number when %d > %d`, (a, b) => { + const result = numericSort(x => x)(a, b, undefined); + expect(result).toBeGreaterThan(0); + }); + + describe('invalid params', () => { + const invalidParams = [undefined, 'text', '.', '$/}']; + + it.each(invalidParams)( + `should return negative number when second param is not-a-number %o`, + notNumber => { + const result = numericSort(x => x)('1', notNumber, undefined); + expect(result).toBeLessThan(0); + }, + ); + + it.each(invalidParams)( + `should return 0 when both params are not-a-number %o`, + notNumber => { + const result = numericSort(x => x)( + notNumber, + notNumber, + undefined, + ); + expect(result).toBe(0); + }, + ); + + it.each(invalidParams)( + `should return positive number when first param is not-a-number %o`, + notNumber => { + const result = numericSort(x => x)(notNumber, '1', undefined); + expect(result).toBeGreaterThan(0); + }, + ); + + it(`should return 0 when both params are different not-a-number`, () => { + const result = numericSort(x => x)('a', 'b', undefined); + expect(result).toBe(0); + }); + }); + + it('should execute a deeply nested accessor', () => { + const obj = { one: { two: { three: '1970-01-01' } } }; + const accessor = (x: any) => x.one.two.three; + const result = numericSort(accessor)(obj, obj, undefined); + expect(result).toBe(0); + }); +}); diff --git a/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.tsx b/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.tsx index 83181bf80e..c7a962ba45 100644 --- a/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.tsx +++ b/workspaces/sonarqube/plugins/sonarqube/src/components/SonarQubeTable/Columns.tsx @@ -33,6 +33,56 @@ import { SonarQubeTableRow } from './types'; import { TranslationFunction } from '@backstage/core-plugin-api/alpha'; import { sonarqubeTranslationRef } from '../../translation'; +/** + * Sort function for datetime columns. + * + * The dates are sorted from oldest to newest. + * All undefined values are sorted to the end. + * @internal + */ +export function datetimeSort( + dataAccessor: (data: T) => string | undefined, +) { + return (data1: T, data2: T, _type: any) => { + const a: number = Date.parse(dataAccessor(data1) || ''); + const b: number = Date.parse(dataAccessor(data2) || ''); + + if (isNaN(a) && isNaN(b)) { + return 0; // both NaN + } else if (isNaN(a)) { + return 1; // only first NaN + } else if (isNaN(b)) { + return -1; // only second NaN + } + return a - b; + }; +} + +/** + * Sort function for numeric columns. + * + * The numbers are sorted from lowest to highest. + * All undefined values are sorted to the end. + * @internal + */ +export function numericSort( + dataAccessor: (data: T) => string | undefined, +) { + return (data1: T, data2: T, _type: any) => { + const a: number = Number(dataAccessor(data1)); + const b: number = Number(dataAccessor(data2)); + + if (isNaN(a) && isNaN(b)) { + return 0; // both NaN + } else if (isNaN(a)) { + return 1; // only first NaN + } else if (isNaN(b)) { + return -1; // only second NaN + } + return a - b; + }; +} + export const getColumns = ( t: TranslationFunction, ): TableColumn[] => { @@ -68,10 +118,11 @@ export const getColumns = ( }, { title: t('sonarQubeTable.columnsTitle.lastAnalysis'), - field: 'resolved.findings.metrics.lastAnalysis', + field: 'resolved.findings.lastAnalysis', align: 'right', type: 'datetime', width: '8%', + customSort: datetimeSort(data => data.resolved.findings?.lastAnalysis), render: ({ resolved }) => resolved?.findings?.metrics && ( @@ -83,6 +134,7 @@ export const getColumns = ( align: 'center', type: 'numeric', width: '7%', + customSort: numericSort(data => data.resolved.findings?.metrics?.bugs), render: ({ resolved }) => resolved?.findings?.metrics && ( @@ -94,6 +146,9 @@ export const getColumns = ( align: 'center', width: '7%', type: 'numeric', + customSort: numericSort( + data => data.resolved.findings?.metrics?.vulnerabilities, + ), render: ({ resolved }) => resolved?.findings?.metrics && ( @@ -105,6 +160,9 @@ export const getColumns = ( align: 'center', type: 'numeric', width: '7%', + customSort: numericSort( + data => data.resolved.findings?.metrics?.code_smells, + ), render: ({ resolved }) => resolved?.findings?.metrics && ( @@ -116,6 +174,9 @@ export const getColumns = ( align: 'center', type: 'numeric', width: '7%', + customSort: numericSort( + data => data.resolved.findings?.metrics?.security_hotspots_reviewed, + ), render: ({ resolved }) => resolved?.findings?.metrics && ( @@ -127,6 +188,9 @@ export const getColumns = ( align: 'center', type: 'numeric', width: '7%', + customSort: numericSort( + data => data.resolved.findings?.metrics?.coverage, + ), render: ({ resolved }) => resolved?.findings?.metrics && ( @@ -138,6 +202,9 @@ export const getColumns = ( align: 'center', type: 'numeric', width: '7%', + customSort: numericSort( + data => data.resolved.findings?.metrics?.duplicated_lines_density, + ), render: ({ resolved }) => resolved?.findings?.metrics && (