-
Notifications
You must be signed in to change notification settings - Fork 8
TSV output to aid MMs #169
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
|
|
||
| const challengeId = (queryDto.challengeId ?? '').trim(); | ||
| if (!challengeId) { | ||
| throw new BadRequestException({ |
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.
[correctness]
The BadRequestException is thrown with a custom message and code. Ensure that this exception handling is consistent with the rest of the application and that the client is prepared to handle this specific error code (TSV_CHALLENGE_ID_REQUIRED).
| } | ||
|
|
||
| private requestWantsTabSeparated(req: Request): boolean { | ||
| const acceptHeader = Array.isArray(req.headers.accept) |
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.
[correctness]
The acceptHeader is processed by joining it with a comma if it's an array. Ensure that this behavior aligns with how the rest of the application handles multi-value headers, as this could lead to unexpected results if the header is not handled consistently.
|
|
||
| const lines = [headers.join('\t')]; | ||
|
|
||
| payload.data.forEach((entry) => { |
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.
[correctness]
The forEach loop inside buildReviewSummationTsv processes each entry's metadata. Consider handling potential errors or unexpected data structures within entry.metadata to prevent runtime exceptions.
| } | ||
|
|
||
| const record = value as Record<string, unknown>; | ||
| const hasScore = Object.prototype.hasOwnProperty.call(record, 'score'); |
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.
[💡 readability]
The use of Object.prototype.hasOwnProperty.call is correct for checking properties. However, consider using TypeScript's optional chaining and nullish coalescing operators for cleaner and more readable code.
|
|
||
| const primitiveValue = value as string | number | boolean | bigint; | ||
| const asString = String(primitiveValue); | ||
| return asString.replace(/[\t\n\r]+/g, ' '); |
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.
[security]
The replace method is used to sanitize values by replacing tabs, newlines, and carriage returns. Ensure that this sanitization is sufficient for all expected input data, especially if the data can come from untrusted sources.
No description provided.