fix/duration-filters - #2127
fix/duration-filters#2127alanpeixinho wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
I think these tests does not cover the POST path used by Tree Details and Hardware Details, right?
worth adding a process_body=True
| if (value === '') { | ||
| delete next[field]; | ||
| } else { | ||
| next[field] = Number.parseInt(value, 10); | ||
| } |
There was a problem hiding this comment.
this guards '' but other values can also become NaN.
Even if the input is of type number, I think it's worth using Number.isFinite to validate the parsed value.
Closes kernelci#428 Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
Closes kernelci#2126 Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
d5b4e70 to
f232d48
Compare
| }); | ||
| }); | ||
|
|
||
| describe('duration filter issue #428', () => { |
There was a problem hiding this comment.
I'd not reference github issues in test names. Give it a description that we can understand without leaving the editor and put a link to the issue as a comment.
// ensures https://github.com/kernelci/dashboard/issues/428
describe('...', () => {});| export const getActiveDurationFilter = (value: unknown): number | undefined => { | ||
| if (value === undefined || value === null || value === '') { | ||
| return undefined; | ||
| } | ||
| const n = typeof value === 'number' ? value : Number(value); | ||
| return Number.isFinite(n) && n !== 0 ? n : undefined; | ||
| }; |
There was a problem hiding this comment.
I'm pondering if this is too permissive. Since value is unknown we can get anything here.
Maybe we should do something like:
export const getActiveDurationFilter = (
value: unknown,
): number | undefined => {
if (typeof value !== 'string' && typeof value !== 'number') {
return undefined;
}
if (typeof value === 'string' && value.trim() === '') {
return undefined;
}
const n = Number(value);
return Number.isFinite(n) && n !== 0 ? n : undefined;
};OR restricting the type of value param
What it is
Fixes duration filter handling in the dashboard and API:
longer truncates multi-digit values (e.g. 3600 → 3) when filters come from the query string.
Closes #428
Closes #2126
How to test