-
Notifications
You must be signed in to change notification settings - Fork 471
fix(sonarqube): sorting fails on missing data in SonarQubeRelatedEntitiesOverview #5297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Changed Packages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you @ric03 🙏
| describe('datetimeSort', () => { | ||
| [ | ||
| { a: new Date(2025, 1).toISOString(), b: new Date(2026, 2).toISOString() }, | ||
| { a: new Date(2019, 3).toISOString(), b: new Date(2024, 5).toISOString() }, | ||
| { | ||
| a: new Date(2019, 2, 1).toISOString(), | ||
| b: new Date(2019, 2, 2).toISOString(), | ||
| }, | ||
| ].forEach(({ a, b }) => { | ||
| it(`should return negative number when ${a} < ${b}`, () => { | ||
| const result = datetimeSort<string>(x => x)(a, b, undefined); | ||
| expect(result).toBeLessThan(0); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest can help a bit here:
| describe('datetimeSort', () => { | |
| [ | |
| { a: new Date(2025, 1).toISOString(), b: new Date(2026, 2).toISOString() }, | |
| { a: new Date(2019, 3).toISOString(), b: new Date(2024, 5).toISOString() }, | |
| { | |
| a: new Date(2019, 2, 1).toISOString(), | |
| b: new Date(2019, 2, 2).toISOString(), | |
| }, | |
| ].forEach(({ a, b }) => { | |
| it(`should return negative number when ${a} < ${b}`, () => { | |
| const result = datetimeSort<string>(x => x)(a, b, undefined); | |
| expect(result).toBeLessThan(0); | |
| }); | |
| }); | |
| 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<string>(x => x)( | |
| a.toISOString(), | |
| b.toISOString(), | |
| undefined, | |
| ); | |
| expect(result).toBeLessThan(0); | |
| }); |
same below 👇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thank you. I have applied your suggestion. Additionally i have wrapped the invalid params in a nested describe.
…itiesOverview` 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 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you @ric03 🚢
…itiesOverview` (backstage#5297) 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 <[email protected]>
Hey, I just made a Pull Request!
Background: The SonarQubeRelatedEntitiesOverview collects all components of a system and displays them in a table. Those components may or may not have a sonarqube-annotation. The sonarqube-annotation is used to query the API. If there is no annotation, no data is displayed.
Problem: If there are rows without an annotation, meaning no data, then the sorting does not work as intended.
Current faulty behaviour:

Note about the fix:
I did not dig deep enough, but i suspect that the deeply-nested object structure of the resolved object (with potentially undefined values) causes the problem. I opted to add the two sort-function. I am happy for alternative suggestions.
✔️ Checklist
Signed-off-byline in the message. (more info)