[DO NOT MERGE] Challenge Winners Processing -> dev #64
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| FROM ${submissionTable} | ||
| WHERE "challengeId" IS NOT NULL | ||
| AND "type" = ${CONTEST_SUBMISSION_TYPE} | ||
| AND "type" = ${CONTEST_SUBMISSION_TYPE}::reviews."SubmissionType" |
There was a problem hiding this comment.
[performance]
The cast to reviews."SubmissionType" might not be necessary if the column type already matches CONTEST_SUBMISSION_TYPE. Ensure this cast is required to avoid potential performance overhead.
| FROM ${tables.submission} s | ||
| INNER JOIN ${tables.reviewSummation} rs ON rs."submissionId" = s.id | ||
| WHERE s."challengeId" = ${challenge.id} | ||
| AND s."type" = ${CONTEST_SUBMISSION_TYPE}::reviews."SubmissionType" |
There was a problem hiding this comment.
[performance]
The cast to reviews."SubmissionType" might not be necessary if the column type already matches CONTEST_SUBMISSION_TYPE. Ensure this cast is required to avoid potential performance overhead.
| FROM ${tables.submission} s | ||
| INNER JOIN ${tables.review} r ON r."submissionId" = s.id | ||
| WHERE s."challengeId" = ${challenge.id} | ||
| AND s."type" = ${CONTEST_SUBMISSION_TYPE}::reviews."SubmissionType" |
There was a problem hiding this comment.
[performance]
The cast to reviews."SubmissionType" might not be necessary if the column type already matches CONTEST_SUBMISSION_TYPE. Ensure this cast is required to avoid potential performance overhead.
| row.challengeId, | ||
| submitterHandle, | ||
| toIsoString(row.submittedAt), | ||
| toIsoString(row.submittedAt || row.createdAt), |
There was a problem hiding this comment.
[💡 readability]
Consider using row.submittedDate directly instead of row.submittedAt || row.createdAt to avoid potential confusion between submission and creation dates.
winners1.csv
Outdated
| LnxpmyLx2f0eBc,ffc62a9b-94ca-454b-a532-708b33db04ac,rdzianach,2013-09-28T04:26:41.983Z,90,20,1,PLACEMENT | ||
| pIzn45Te-rM06c,ffc62a9b-94ca-454b-a532-708b33db04ac,sdgun,2013-09-29T04:31:00.546Z,60,20,2,PLACEMENT | ||
| tD9We164PgpRnI,ffc62a9b-94ca-454b-a532-708b33db04ac,Kaumad,2013-09-28T18:02:55.554Z,20,20,1,PASSED_REVIEW | ||
| PrH9NO0kTiBtUM,ffc62a9b-94ca-454b-a532-708b33db04ac,Valarmathi1,2013-09-28T03:17:27.736Z,10,20,, |
There was a problem hiding this comment.
[❗❗ correctness]
The entry on this line has a missing value for 'Prize type'. Ensure that this is intentional and that the data processing logic can handle missing values appropriately.
| const isMarathonMatch = challengeType === "Marathon Match"; | ||
| const phaseFilterClause = | ||
| reviewPhaseIds && reviewPhaseIds.length > 0 | ||
| ? Prisma.sql`r."phaseId" IN (${Prisma.join(reviewPhaseIds)})` |
There was a problem hiding this comment.
[❗❗ security]
The use of Prisma.sql with Prisma.join for dynamic SQL generation can be risky if reviewPhaseIds contains untrusted input. Ensure that reviewPhaseIds is properly validated and sanitized before being used here to prevent SQL injection vulnerabilities.
| (rows || []).map((row) => { | ||
| const minimumPassingScore = toNumber(row.minimumPassingScore); | ||
| const minScore = toNumber(row.minScore); | ||
| const passingScore = minimumPassingScore !== null ? minimumPassingScore : (minScore ?? 0); |
There was a problem hiding this comment.
[maintainability]
The logic for determining passingScore could be simplified by using a default value for minimumPassingScore directly in the SQL query, reducing the need for post-processing logic.
| const challengeType = CHALLENGE_TYPE_BY_ID.get(challenge.typeId); | ||
| const isMarathonMatch = challengeType === "Marathon Match"; | ||
| const reviewPhaseIds = getReviewPhaseIds(challenge); | ||
| if (!isMarathonMatch && reviewPhaseIds.length === 0) { |
There was a problem hiding this comment.
[💡 design]
The check for reviewPhaseIds.length === 0 and subsequent warning could be moved into getReviewPhaseIds to encapsulate the logic and reduce repetition.
| if (!challenge || !Array.isArray(challenge.phases)) { | ||
| return []; | ||
| } | ||
| const eligiblePhases = new Set(["review", "iterative review"]); |
There was a problem hiding this comment.
[maintainability]
Consider using a constant or configuration for the phase names to avoid hardcoding them in multiple places. This will improve maintainability and reduce the risk of typos or inconsistencies.
No description provided.