Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 17, 2025
Merged

TSV output to aid MMs #169

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 221 additions & 3 deletions src/api/review-summation/review-summation.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
HttpCode,
HttpStatus,
Req,
Res,
BadRequestException,
} from '@nestjs/common';
import {
ApiOperation,
Expand Down Expand Up @@ -38,13 +40,15 @@ import { PaginatedResponse, PaginationDto } from '../../dto/pagination.dto';
import { SortDto } from '../../dto/sort.dto';
import { ReviewSummationService } from './review-summation.service';
import { JwtUser } from 'src/shared/modules/global/jwt.service';
import { Request } from 'express';
import { Request, Response } from 'express';

@ApiTags('ReviewSummations')
@ApiBearerAuth()
@Controller('/reviewSummations')
export class ReviewSummationController {
private readonly logger: LoggerService;
private static readonly TSV_CONTENT_TYPE = 'text/tab-separated-values';
private static readonly TSV_FILENAME_PREFIX = 'review-summations';

constructor(private readonly service: ReviewSummationService) {
this.logger = LoggerService.forRoot(ReviewSummationController.name);
Expand Down Expand Up @@ -150,22 +154,60 @@ export class ReviewSummationController {
description: 'List of review summations.',
type: [ReviewSummationResponseDto],
})
@ApiResponse({
status: 200,
description:
'Tab-delimited representation when Accept includes text/tab-separated-values.',
content: {
'text/tab-separated-values': {
schema: {
type: 'string',
example:
'submissionId\tsubmitterId\tsubmitterHandle\taggregateScore\tisFinal\tisProvisional\tisExample\treviewedDate\tcreatedAt\tupdatedAt\tscore\ttestcase\nsid123\t123456\tmember\t99.5\ttrue\tfalse\tfalse\t2024-02-01T10:00:00.000Z\t2024-02-02T12:00:00.000Z\t2024-02-02T13:00:00.000Z\t99.5\tSample test case',
},
},
},
})
async listReviewSummations(
@Req() req: Request,
@Res({ passthrough: true }) res: Response,
@Query() queryDto: ReviewSummationQueryDto,
@Query() paginationDto?: PaginationDto,
@Query() sortDto?: SortDto,
): Promise<PaginatedResponse<ReviewSummationResponseDto>> {
): Promise<PaginatedResponse<ReviewSummationResponseDto> | string> {
this.logger.log(
`Getting review summations with filters - ${JSON.stringify(queryDto)}`,
);
const authUser: JwtUser = req['user'] as JwtUser;
return this.service.searchSummation(
const results = await this.service.searchSummation(
authUser,
queryDto,
paginationDto,
sortDto,
);

if (!this.requestWantsTabSeparated(req)) {
return results;
}

const challengeId = (queryDto.challengeId ?? '').trim();
if (!challengeId) {
throw new BadRequestException({
Copy link

@github-actions github-actions bot Nov 17, 2025

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).

message:
'challengeId is required when requesting tab-delimited review summations.',
code: 'TSV_CHALLENGE_ID_REQUIRED',
});
}

const payload = this.buildReviewSummationTsv(results);
const safeChallengeSlug = this.buildFilenameSlug(challengeId);
const filename = `${ReviewSummationController.TSV_FILENAME_PREFIX}-${safeChallengeSlug}.tsv`;
res.setHeader(
'Content-Type',
`${ReviewSummationController.TSV_CONTENT_TYPE}; charset=utf-8`,
);
res.setHeader('Content-Disposition', `attachment; filename="${filename}"`);
return payload;
}

@Get('/:reviewSummationId')
Expand Down Expand Up @@ -278,4 +320,180 @@ export class ReviewSummationController {
message: `Review type ${reviewSummationId} deleted successfully.`,
};
}

private requestWantsTabSeparated(req: Request): boolean {
const acceptHeader = Array.isArray(req.headers.accept)
Copy link

@github-actions github-actions bot Nov 17, 2025

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.

? req.headers.accept.join(',')
: (req.headers.accept ?? '');
if (acceptHeader) {
const lowered = acceptHeader
.split(',')
.map((value) => value.trim().toLowerCase());
const matchesHeader = lowered.some((value) =>
value.startsWith(
ReviewSummationController.TSV_CONTENT_TYPE.toLowerCase(),
),
);
if (matchesHeader) {
return true;
}
}

const formatParam = req.query['format'];
if (
typeof formatParam === 'string' &&
formatParam.trim().toLowerCase() === 'tsv'
) {
return true;
}

return false;
}

private buildReviewSummationTsv(
payload: PaginatedResponse<ReviewSummationResponseDto>,
): string {
const headers = [
'submissionId',
'submitterId',
'submitterHandle',
'aggregateScore',
'isFinal',
'isProvisional',
'isExample',
'reviewedDate',
'createdAt',
'updatedAt',
'score',
'testcase',
];

const lines = [headers.join('\t')];

payload.data.forEach((entry) => {
Copy link

@github-actions github-actions bot Nov 17, 2025

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 scoreRows = this.extractTestScoreEntries(entry.metadata);
if (!scoreRows.length) {
lines.push(
this.toTabSeparatedRow(entry, {
score: '',
testcase: '',
}),
);
return;
}

scoreRows.forEach((scoreEntry) => {
lines.push(this.toTabSeparatedRow(entry, scoreEntry));
});
});

return lines.join('\n');
}

private toTabSeparatedRow(
entry: ReviewSummationResponseDto,
metadataEntry: { score: unknown; testcase: unknown },
): string {
const values: Array<unknown> = [
entry.submissionId,
entry.submitterId,
entry.submitterHandle,
entry.aggregateScore,
entry.isFinal,
entry.isProvisional,
entry.isExample,
entry.reviewedDate,
entry.createdAt,
entry.updatedAt,
metadataEntry.score,
metadataEntry.testcase,
];

return values
.map((value) => this.sanitizeTabSeparatedValue(value))
.join('\t');
}

private extractTestScoreEntries(
metadata: ReviewSummationResponseDto['metadata'],
): Array<{ score: unknown; testcase: unknown }> {
if (metadata === null || metadata === undefined) {
return [];
}

const results: Array<{ score: unknown; testcase: unknown }> = [];

const visit = (value: unknown, inTestsScope = false) => {
if (Array.isArray(value)) {
value.forEach((entry) => visit(entry, inTestsScope));
return;
}

if (!value || typeof value !== 'object') {
return;
}

const record = value as Record<string, unknown>;
const hasScore = Object.prototype.hasOwnProperty.call(record, 'score');
Copy link

@github-actions github-actions bot Nov 17, 2025

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 hasTestCase =
Object.prototype.hasOwnProperty.call(record, 'testcase') ||
Object.prototype.hasOwnProperty.call(record, 'testCase');

if (inTestsScope && (hasScore || hasTestCase)) {
results.push({
score: record['score'] ?? null,
testcase: record['testcase'] ?? record['testCase'] ?? null,
});
}

Object.entries(record).forEach(([key, child]) => {
const normalizedKey = key.toLowerCase();
const isTestKey =
normalizedKey === 'tests' || normalizedKey === 'testscores';
visit(child, inTestsScope || isTestKey);
});
};

visit(metadata);
return results;
}

private sanitizeTabSeparatedValue(value: unknown): string {
if (value === null || value === undefined) {
return '';
}

if (value instanceof Date) {
return value.toISOString();
}

if (typeof value === 'object') {
try {
return JSON.stringify(value);
} catch (error) {
this.logger.warn(`Failed to stringify TSV value: ${error}`);
return '';
}
}

if (typeof value === 'function') {
// Functions shouldn't appear in tab-separated exports; fall back to empty string.
this.logger.warn(
'Encountered function value while sanitizing TSV export',
);
return '';
}

if (typeof value === 'symbol') {
return value.toString();
}

const primitiveValue = value as string | number | boolean | bigint;
const asString = String(primitiveValue);
return asString.replace(/[\t\n\r]+/g, ' ');
Copy link

@github-actions github-actions bot Nov 17, 2025

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.

}

private buildFilenameSlug(challengeId: string): string {
return challengeId.replace(/[^A-Za-z0-9-_]+/g, '_') || 'export';
}
}
Loading