feat: replace numeric table assignment with dynamic two-floor lettere…#426
feat: replace numeric table assignment with dynamic two-floor lettere…#426jackzheng-cs wants to merge 7 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates CSV ingestion table assignment to generate two-floor letter-number table labels (e.g., A3) and prioritize “Best Hardware Hack” teams on floor 1, aligning with Issue #381’s new table labeling scheme.
Changes:
- Added a two-floor table assignment algorithm that distributes teams across lettered rows.
- Prioritized hardware teams on floor 1 and filled remaining floor 1 capacity with non-hardware teams.
- Replaced the previous sequential numeric table numbering assignment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
migrations/20260324120000-update-table-number-to-letter-number.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { storedValue: tableNumber } = useTableNumberContext(); | ||
| const { team, judges, loading, error, fetchTeamJudges } = | ||
| useTeamJudgesFromTableNumber(tableNumber ?? -1); | ||
| useTeamJudgesFromTableNumber(String(tableNumber ?? '')); |
There was a problem hiding this comment.
useTableNumberContext() currently normalizes the stored value with Number(storedValue). Once table numbers become letter-number strings (e.g. A13), that conversion becomes NaN, which then turns into the literal string 'NaN' here and also prevents the if (tableNumber) gating from working. This call site should pass the actual string table label, and the context/provider should stop coercing to number so judging works with the new format.
| useTeamJudgesFromTableNumber(String(tableNumber ?? '')); | |
| useTeamJudgesFromTableNumber(tableNumber ?? ''); |
| const rowLetter = secondFloorStartLabel.trim().toUpperCase().charAt(0); | ||
| const secondFloorStart = (rowLetter.charCodeAt(0) - 64) * 1000; |
There was a problem hiding this comment.
secondFloorStartLabel is treated as a row letter, but the function doesn’t validate it; charCodeAt(0) on an empty string yields NaN, and non-letter input yields incorrect thresholds. Also, toComparableTableNumber supports legacy numeric table numbers, but the threshold computation cannot accept a numeric cutoff anymore. Consider parsing secondFloorStartLabel as either (a) a number cutoff when it’s numeric, or (b) a validated A-L row letter cutoff when it’s alphabetic.
| const rowLetter = secondFloorStartLabel.trim().toUpperCase().charAt(0); | |
| const secondFloorStart = (rowLetter.charCodeAt(0) - 64) * 1000; | |
| const normalizedLabel = secondFloorStartLabel.trim(); | |
| let secondFloorStart: number; | |
| if (/^\d+$/.test(normalizedLabel)) { | |
| // Numeric cutoff (supports legacy numeric table numbers) | |
| secondFloorStart = Number(normalizedLabel); | |
| } else if (/^[A-La-l]$/.test(normalizedLabel)) { | |
| // Single row letter cutoff (A–L), converted to the comparable number space | |
| const rowLetter = normalizedLabel.toUpperCase(); | |
| secondFloorStart = (rowLetter.charCodeAt(0) - 64) * 1000; | |
| } else { | |
| throw new Error( | |
| `Invalid secondFloorStartLabel: "${secondFloorStartLabel}". Expected a numeric cutoff or a single row letter A-L.` | |
| ); | |
| } |
Summary