-
Notifications
You must be signed in to change notification settings - Fork 2
CSV ingestion fixes #354
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
base: main
Are you sure you want to change the base?
CSV ingestion fixes #354
Conversation
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.
Pull request overview
This pull request addresses CSV ingestion issues by removing the 6-track limit and adding comprehensive validation functionality to the CSV upload process. The changes enable teams to have more than 6 tracks and provide detailed error reporting before database insertion.
Changes:
- Removed
maxItems: 6constraint from teams' tracks array in database schema - Added two-step CSV validation and upload workflow with detailed error/warning reporting
- Implemented contact information extraction for easier follow-up on validation issues
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/create-teams.mjs | Removes maxItems: 6 from initial teams collection schema |
| migrations/20260105090000-remove-team-tracks-limit.mjs | New migration to remove track limit from existing databases |
| migrations/20250420035636-update-teams.mjs | Updates existing migration to remove track limit (concerning - modifies historical migration) |
| app/(pages)/admin/csv/page.tsx | Complete rewrite of CSV upload UI with validation, error reporting, and two-step workflow |
| app/(api)/_utils/csv-ingestion/csvAlgorithm.ts | Major enhancement with validation functions, contact extraction, and detailed issue reporting |
| app/(api)/_actions/logic/validateCSV.ts | New server action for CSV validation |
| app/(api)/_actions/logic/ingestTeams.ts | New server action for team ingestion |
| app/(api)/_actions/logic/checkTeamsPopulated.ts | New server action to check if teams exist |
| tests/csvValidation.test.ts | Tests for CSV validation behavior |
| tests/csvAlgorithm.test.ts | Tests for track matching and filtering |
| .prettierignore | Adds commented-out pattern for .mjs files (has syntax issue) |
Comments suppressed due to low confidence (1)
migrations/create-teams.mjs:42
- The maxItems constraint is being removed from the initial teams collection creation. While this is fine for new database setups, if this migration has already been run on production databases, this change won't affect them. The new migration 20260105090000-remove-team-tracks-limit.mjs is the correct approach for existing databases. However, for consistency in new setups, this change makes sense.
tracks: {
bsonType: 'array',
items: {
enum: tracks,
description: 'track must be one of the valid tracks',
},
description: 'tracks must be an array of strings',
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| # **.mjs No newline at end of file | |||
Copilot
AI
Jan 14, 2026
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.
The comment in .prettierignore is ineffective. In .prettierignore files, comments should start on their own line with a hash (#), not inline. The pattern "# **.mjs" will be treated as a literal pattern to ignore files named "# .mjs" rather than as a comment. If you want to ignore .mjs files, remove the "# " prefix to use ".mjs" as the pattern, or if you want a comment, move it to a separate line.
| # **.mjs | |
| **.mjs |
|
|
||
| import { CreateManyTeams } from "@datalib/teams/createTeams"; | ||
|
|
||
| export default async function ingestTeams(body: object) { |
Copilot
AI
Jan 14, 2026
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.
The parameter type 'object' is too generic. Consider using a more specific type such as ParsedRecord[] to ensure type safety and prevent invalid data from being passed to CreateManyTeams.
| export default async function ingestTeams(body: object) { | |
| type IngestTeamsBody = Parameters<typeof CreateManyTeams>[0]; | |
| export default async function ingestTeams(body: IngestTeamsBody) { |
| const res = (await validateCSV(formData)) as ValidationResponse; | ||
| setValidation(res); | ||
| setResponse(""); | ||
| setValidating(false); |
Copilot
AI
Jan 14, 2026
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.
The validateHandler function lacks error handling. If validateCSV throws an error, the validating state will remain true and the UI will be stuck. Add a try-catch block to handle errors and reset the validating state in a finally block.
| const res = (await validateCSV(formData)) as ValidationResponse; | |
| setValidation(res); | |
| setResponse(""); | |
| setValidating(false); | |
| try { | |
| const res = (await validateCSV(formData)) as ValidationResponse; | |
| setValidation(res); | |
| setResponse(""); | |
| } catch (error) { | |
| console.error("Error validating CSV file:", error); | |
| setResponse("An error occurred while validating the CSV file. Please try again."); | |
| } finally { | |
| setValidating(false); | |
| } |
| const res = await ingestTeams(validation.validBody); | ||
| setResponse(JSON.stringify(res, null, 2)); | ||
| setPending(false); |
Copilot
AI
Jan 14, 2026
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.
The uploadValidHandler function lacks error handling. If ingestTeams throws an error, the pending state will remain true and the UI will be stuck. Add a try-catch block to handle errors and reset the pending state in a finally block.
| const res = await ingestTeams(validation.validBody); | |
| setResponse(JSON.stringify(res, null, 2)); | |
| setPending(false); | |
| try { | |
| const res = await ingestTeams(validation.validBody); | |
| setResponse(JSON.stringify(res, null, 2)); | |
| } catch (error: unknown) { | |
| const message = | |
| error instanceof Error ? error.message : "An unknown error occurred during upload."; | |
| setResponse(`Error uploading teams: ${message}`); | |
| } finally { | |
| setPending(false); | |
| } |
| const res = await ingestTeams(validation.body); | ||
| setResponse(JSON.stringify(res, null, 2)); | ||
| setPending(false); |
Copilot
AI
Jan 14, 2026
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.
The uploadAllHandler function lacks error handling. If ingestTeams throws an error, the pending state will remain true and the UI will be stuck. Add a try-catch block to handle errors and reset the pending state in a finally block.
| const res = await ingestTeams(validation.body); | |
| setResponse(JSON.stringify(res, null, 2)); | |
| setPending(false); | |
| try { | |
| const res = await ingestTeams(validation.body); | |
| setResponse(JSON.stringify(res, null, 2)); | |
| } catch (error) { | |
| setResponse( | |
| `Failed to upload teams: ${ | |
| error instanceof Error ? error.message : String(error) | |
| }` | |
| ); | |
| } finally { | |
| setPending(false); | |
| } |
| <input | ||
| type="file" | ||
| accept=".csv" | ||
| onChange={(e) => { | ||
| const next = e.target.files?.[0] ?? null; | ||
| setFile(next); | ||
| setValidation(null); | ||
| setResponse(""); | ||
| }} | ||
| /> |
Copilot
AI
Jan 14, 2026
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.
The file input lacks an associated label element. For better accessibility, add a label with a htmlFor attribute pointing to the input's id, or wrap the input in a label element. This helps screen reader users understand the purpose of the input.
| <button | ||
| onClick={uploadValidHandler} | ||
| disabled={pending || validating || !validation.validBody?.length} | ||
| > | ||
| {pending ? "Uploading..." : "Upload Valid Teams Only"} | ||
| </button> | ||
|
|
||
| <button | ||
| onClick={uploadAllHandler} | ||
| disabled={pending || validating || !validation.body?.length} | ||
| > | ||
| Force Upload All Teams (Ignore Errors) | ||
| </button> | ||
| </div> |
Copilot
AI
Jan 14, 2026
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.
Both upload buttons should have explicit type="button" attributes to prevent unintended form submission behavior.
| output.push({ | ||
| name: data['Project Title'], | ||
| teamNumber: parseInt(data['Table Number']), | ||
| tableNumber: 0, // doing it later (on end) | ||
| tracks: tracksInOrder, | ||
| name: projectTitle, | ||
| teamNumber: parsedTeamNumber, | ||
| tableNumber: 0, // assigned after ordering | ||
| tracks: canonicalTracks, | ||
| active: true, | ||
| }); |
Copilot
AI
Jan 14, 2026
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.
Teams with validation errors (missing projectTitle, invalid teamNumber, or no tracks) are still added to the output array. This means that when "Force Upload All Teams" is used, teams with invalid data will be sent to the database, which will likely cause CreateManyTeams to fail. Consider filtering out teams with error-level issues from the body array, or ensure CreateManyTeams handles partial failures gracefully with detailed error reporting.
| track1: string, | ||
| track2: string, | ||
| track3: string, | ||
| chosentracks: string |
Copilot
AI
Jan 14, 2026
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.
The parameter name 'chosentracks' doesn't follow the camelCase convention used elsewhere in the codebase. Consider renaming it to 'chosenTracks' for consistency.
| .map((i: any) => ( | ||
| <li key={`${i.rowIndex}-${i.teamNumberRaw}`}> | ||
| Team {i.teamNumberRaw} — {i.projectTitle} | ||
| {i.contactNames?.length ? ( | ||
| <> (Submitter: {i.contactNames.join(", ")})</> | ||
| ) : null} | ||
| {i.missingFields?.length ? ( | ||
| <> (Missing: {i.missingFields.join(", ")})</> | ||
| ) : null} | ||
| {i.invalidTracks?.length ? ( | ||
| <> (Invalid tracks: {i.invalidTracks.join(", ")})</> | ||
| ) : null} | ||
| {buildTeamMemberLines(i).length ? ( | ||
| <pre className="mt-2 text-xs whitespace-pre-wrap break-words"> | ||
| {buildTeamMemberLines(i) | ||
| .map((l) => `Member: ${l}`) | ||
| .join("\n")} | ||
| </pre> | ||
| ) : null} | ||
| </li> | ||
| ))} |
Copilot
AI
Jan 14, 2026
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.
The buildTeamMemberLines function is called twice for the same issue in the render - once at line 246 to check the length and again at line 248-250 to render the lines. Consider storing the result in a variable to avoid recalculating it.
| .map((i: any) => ( | |
| <li key={`${i.rowIndex}-${i.teamNumberRaw}`}> | |
| Team {i.teamNumberRaw} — {i.projectTitle} | |
| {i.contactNames?.length ? ( | |
| <> (Submitter: {i.contactNames.join(", ")})</> | |
| ) : null} | |
| {i.missingFields?.length ? ( | |
| <> (Missing: {i.missingFields.join(", ")})</> | |
| ) : null} | |
| {i.invalidTracks?.length ? ( | |
| <> (Invalid tracks: {i.invalidTracks.join(", ")})</> | |
| ) : null} | |
| {buildTeamMemberLines(i).length ? ( | |
| <pre className="mt-2 text-xs whitespace-pre-wrap break-words"> | |
| {buildTeamMemberLines(i) | |
| .map((l) => `Member: ${l}`) | |
| .join("\n")} | |
| </pre> | |
| ) : null} | |
| </li> | |
| ))} | |
| .map((i: any) => { | |
| const memberLines = buildTeamMemberLines(i); | |
| return ( | |
| <li key={`${i.rowIndex}-${i.teamNumberRaw}`}> | |
| Team {i.teamNumberRaw} — {i.projectTitle} | |
| {i.contactNames?.length ? ( | |
| <> (Submitter: {i.contactNames.join(", ")})</> | |
| ) : null} | |
| {i.missingFields?.length ? ( | |
| <> (Missing: {i.missingFields.join(", ")})</> | |
| ) : null} | |
| {i.invalidTracks?.length ? ( | |
| <> (Invalid tracks: {i.invalidTracks.join(", ")})</> | |
| ) : null} | |
| {memberLines.length ? ( | |
| <pre className="mt-2 text-xs whitespace-pre-wrap break-words"> | |
| {memberLines | |
| .map((l) => `Member: ${l}`) | |
| .join("\n")} | |
| </pre> | |
| ) : null} | |
| </li> | |
| ); | |
| })} |
Closes #349