feat: Refactor admin view toggle into a dedicated component and enhan…#516
feat: Refactor admin view toggle into a dedicated component and enhan…#516
Conversation
…ce graph visualization with improved ReactFlow configuration and layout for bracket and group pages.
📝 WalkthroughWalkthroughAdds interactive admin toggles, overhauls ReactFlow graph generation and interaction settings for bracket/group views, restructures page layouts with responsive headers and containers, makes match team names navigable, wraps team creation in a form submit flow, and adds a DB seeding script plus npm script for seeding users/teams. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/components/match/MatchNode.tsx`:
- Around line 135-145: The span in MatchNode.tsx shows hover/cursor styles
unconditionally though navigation only happens when team.id exists, causing
misleading UX; update the span to apply "cursor-pointer" and "hover:underline"
classes only when team.id is truthy and only attach the onClick navigation
handler (router.push(`/events/${eventId}/teams/${team.id}`)) when team.id
exists, preserving e.stopPropagation() where used; reference the span rendering
of formatTeamName(team.name), the onClick that checks team.id, and router.push
to locate and change the JSX to conditionally set classes and the onClick
handler.
🧹 Nitpick comments (8)
frontend/app/events/[id]/groups/actions.tsx (2)
2-2: Remove unuseduseParamsimport.
useParamsis imported but never used in this component.Proposed fix
-import { useParams, useRouter, useSearchParams } from "next/navigation"; +import { useRouter, useSearchParams } from "next/navigation";
6-29: Consider extracting a shared AdminViewToggle component.This component is nearly identical to
frontend/app/events/[id]/bracket/actions.tsx. Both implement the same admin view toggle functionality with identical UI and logic.Consider extracting this into a shared component (e.g.,
@/components/AdminViewToggle) to reduce duplication and ensure consistent behavior across bracket and groups pages.frontend/app/events/[id]/bracket/graphView.tsx (2)
9-9: Remove unusedSwitchimport.The
Switchcomponent is imported but no longer used in this file after the admin toggle was moved toactions.tsx.Proposed fix
-import { Switch } from "@/components/ui/switch";
205-210: Consider removing explicitvariant={undefined}.Setting
variant={undefined}is unusual. If you want the default behavior, simply omit the prop. If you intend to disable the variant pattern, verify this is the correct approach for the ReactFlow Background component.Proposed fix
<Background color="currentColor" className="opacity-10" gap={20} - variant={undefined} />frontend/app/events/[id]/groups/page.tsx (1)
47-52: LGTM, but note prop naming inconsistency.The layout and conditional rendering are well implemented. However, there's a naming inconsistency between bracket and groups GraphView components:
bracket/graphView.tsxusesisEventAdmingroups/graphView.tsxuseseventAdminConsider aligning prop names for consistency across similar components.
frontend/app/events/[id]/groups/graphView.tsx (3)
9-9: Remove unusedSwitchimport.The
Switchcomponent is imported but no longer used after the admin toggle was moved toactions.tsx.Proposed fix
-import { Switch } from "@/components/ui/switch";
112-113: AddsetNodesto the dependency array.While
setNodesfromuseNodesStateis stable and won't cause re-renders, it should be included in the dependency array to satisfy the exhaustive-deps lint rule and maintain consistency.Proposed fix
setNodes(newNodes); - }, [matches, eventAdmin, eventId, router]); + }, [matches, eventAdmin, eventId, router, setNodes]);
138-143: Consider removing explicitvariant={undefined}.Same as in
bracket/graphView.tsx- settingvariant={undefined}is unusual. If default behavior is intended, simply omit the prop.Proposed fix
<Background color="currentColor" className="opacity-10" gap={20} - variant={undefined} />
…view for improved node generation.
…odes and edges, including conditional handle rendering.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@api/src/scripts/seed-users-teams.ts`:
- Around line 56-66: The seed script creates UserEntity records with fixed
emails (user{i}@example.com) which will collide on repeated runs; update the
email generation in the users loop to include the run-unique suffix (use the
existing now or the same githubId/username suffix) so each email is unique
(e.g., `user{i}+{now}@example.com`), or alternatively add a cleanup step that
removes previously seeded users before inserting; modify the loop that
constructs UserEntity (variables users, user.githubId, user.username) to include
the unique suffix in user.email or add a pre-insert cleanup for those seed
records.
- Around line 17-50: The bootstrap function currently calls process.exit on
errors and can leave the DataSource open; refactor bootstrap to initialize and
run seeding inside a try/finally so dataSource.destroy() is always called: move
the seeding logic (including dataSource.initialize(), repository usage like
userRepository/teamRepository/eventRepository/permissionRepository, and the
event lookup) into a try block, perform any validation/errors by throwing
exceptions (not calling process.exit) and in the finally block call await
dataSource.destroy(); let the top-level caller catch errors and exit process if
needed so cleanup always runs.
In `@frontend/app/events/`[id]/bracket/graphView.tsx:
- Around line 47-52: The placeholder-round calculation can produce 0 or
-Infinity when teamCount <= 1 because it uses Math.log2(teamCount); replace the
Math.log2-based computation with the existing getTotalRounds(teamCount) utility
so totalRounds is always a valid positive integer before the for-loop in the
branch that checks `if (!matches || matches.length === 0)` (refer to variables
`matches`, `teamCount`, and `totalRounds` in graphView.tsx and the helper
`getTotalRounds`).
- Around line 103-184: The layout uses a 0-based coordinate system but the data
and some branches treat match.round as 1-based, causing off-by-one positioning
and last-round/handle logic errors; normalize to a 0-based roundIndex used
everywhere for layout and comparisons: compute a roundIndex from the round key
or match.round (e.g., roundIndex = Number(roundKeyOrMatchRound) - 1) and then
use roundIndex for x positioning (ROUND_SPACING * roundIndex), spacing
calculations (2 ** roundIndex * VERTICAL_SPACING), nodeIdsByRound keys,
last-round checks (compare roundIndex to lastRoundIndex), handle visibility
(showTargetHandle/showSourceHandle), and when finding the placementMatch; update
all references that currently use raw round or match.round (including variables
roundKeys, placementMatch selection, and onClick logic that inspects lastRound)
to use this normalized roundIndex.
In `@frontend/components/match/MatchNode.tsx`:
- Around line 161-172: Replace the non-focusable clickable <span> in
MatchNode.tsx with a keyboard-accessible <button type="button"> that preserves
the visual styling (transparent background, no border, reset padding, text-left
alignment) so keyboard/assistive users can navigate; keep the existing onClick
handler behavior (call e.stopPropagation(), check team.id and call
router.push(`/events/${eventId}/teams/${team.id}`)) and continue to render the
team label via formatTeamName(team.name). Ensure the button uses the same
classes used on the span (e.g., hover:underline transition-all duration-200
cursor-pointer and truncate flex-1) to maintain appearance while adding
focusability.
- Around line 65-78: The MatchNode component unsafely casts useParams() to
string; update the call to useParams with a typed param (e.g., useParams<{ id?:
string }>()) and normalize the result before using it: read params.id into a
local const (e.g., const rawId = params.id) and then derive eventId via a safe
normalization (e.g., const eventId = rawId ?? '' or handle missing id
explicitly), so all uses of eventId in MatchNode are type-safe and won't crash
if the route variant changes.
| async function bootstrap() { | ||
| const eventId = process.argv[2]; | ||
| if (!eventId) { | ||
| console.error("Please provide an eventId as the first argument"); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log("Connecting to database..."); | ||
| const configService = new ConfigService(); | ||
| const databaseConfig = new DatabaseConfig(configService); | ||
| const baseConfig = databaseConfig.getConfig(true); | ||
|
|
||
| // Use a glob pattern that works with ts-node in development | ||
| const dataSource = new DataSource({ | ||
| ...baseConfig, | ||
| entities: [join(__dirname, "..", "**", "*.entity.ts")], | ||
| } as DataSourceOptions); | ||
|
|
||
| await dataSource.initialize(); | ||
| console.log("Database connected!"); | ||
|
|
||
| const userRepository = dataSource.getRepository(UserEntity); | ||
| const teamRepository = dataSource.getRepository(TeamEntity); | ||
| const eventRepository = dataSource.getRepository(EventEntity); | ||
| const permissionRepository = dataSource.getRepository( | ||
| UserEventPermissionEntity, | ||
| ); | ||
|
|
||
| const event = await eventRepository.findOne({ where: { id: eventId } }); | ||
| if (!event) { | ||
| console.error(`Event with ID ${eventId} not found`); | ||
| await dataSource.destroy(); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "seed-users-teams.ts" -type fRepository: 42core-team/website
Length of output: 100
🏁 Script executed:
fd "seed-users-teams.ts" --type fRepository: 42core-team/website
Length of output: 98
🏁 Script executed:
cat -n api/src/scripts/seed-users-teams.tsRepository: 42core-team/website
Length of output: 4081
Always close the DataSource on failures.
If any error is thrown after Line 35 (during dataSource.initialize() or the seeding operations), the catch handler at Line 103-105 exits without closing the connection. Wrap the seeding logic in try/finally and throw errors instead of calling process.exit inside bootstrap so cleanup always runs.
🧹 Suggested structure
async function bootstrap() {
const eventId = process.argv[2];
if (!eventId) {
- console.error("Please provide an eventId as the first argument");
- process.exit(1);
+ throw new Error("Please provide an eventId as the first argument");
}
console.log("Connecting to database...");
const configService = new ConfigService();
const databaseConfig = new DatabaseConfig(configService);
const baseConfig = databaseConfig.getConfig(true);
const dataSource = new DataSource({
...baseConfig,
entities: [join(__dirname, "..", "**", "*.entity.ts")],
} as DataSourceOptions);
- await dataSource.initialize();
- console.log("Database connected!");
+ try {
+ await dataSource.initialize();
+ console.log("Database connected!");
const userRepository = dataSource.getRepository(UserEntity);
const teamRepository = dataSource.getRepository(TeamEntity);
const eventRepository = dataSource.getRepository(EventEntity);
const permissionRepository = dataSource.getRepository(
UserEventPermissionEntity,
);
const event = await eventRepository.findOne({ where: { id: eventId } });
if (!event) {
- console.error(`Event with ID ${eventId} not found`);
- await dataSource.destroy();
- process.exit(1);
+ throw new Error(`Event with ID ${eventId} not found`);
}
// ... seed logic ...
- console.log("Seeding completed successfully!");
- await dataSource.destroy();
+ console.log("Seeding completed successfully!");
+ } finally {
+ if (dataSource.isInitialized) {
+ await dataSource.destroy();
+ }
+ }
}🤖 Prompt for AI Agents
In `@api/src/scripts/seed-users-teams.ts` around lines 17 - 50, The bootstrap
function currently calls process.exit on errors and can leave the DataSource
open; refactor bootstrap to initialize and run seeding inside a try/finally so
dataSource.destroy() is always called: move the seeding logic (including
dataSource.initialize(), repository usage like
userRepository/teamRepository/eventRepository/permissionRepository, and the
event lookup) into a try block, perform any validation/errors by throwing
exceptions (not calling process.exit) and in the finally block call await
dataSource.destroy(); let the top-level caller catch errors and exit process if
needed so cleanup always runs.
| const users: UserEntity[] = []; | ||
| const now = Date.now(); | ||
| for (let i = 1; i <= 90; i++) { | ||
| const user = new UserEntity(); | ||
| user.githubId = `seed-user-${i}-${now}`; | ||
| user.githubAccessToken = "dummy-token"; | ||
| user.email = `user${i}@example.com`; | ||
| user.username = `seeduser${i}_${now.toString().slice(-5)}`; | ||
| user.name = `Seed User ${i}`; | ||
| user.profilePicture = `https://api.dicebear.com/7.x/avataaars/svg?seed=${user.username}`; | ||
| users.push(user); |
There was a problem hiding this comment.
Prevent email collisions on repeated seeding runs.
Line 62 uses a fixed email pattern (user{i}@example.com). If UserEntity.email is unique (typical), a second run will fail and leave a partial seed. Add a run-unique suffix (similar to githubId/username) or clean existing seeds before insert.
🔧 Proposed fix
- user.email = `user${i}@example.com`;
+ user.email = `user${i}_${now}@example.com`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const users: UserEntity[] = []; | |
| const now = Date.now(); | |
| for (let i = 1; i <= 90; i++) { | |
| const user = new UserEntity(); | |
| user.githubId = `seed-user-${i}-${now}`; | |
| user.githubAccessToken = "dummy-token"; | |
| user.email = `user${i}@example.com`; | |
| user.username = `seeduser${i}_${now.toString().slice(-5)}`; | |
| user.name = `Seed User ${i}`; | |
| user.profilePicture = `https://api.dicebear.com/7.x/avataaars/svg?seed=${user.username}`; | |
| users.push(user); | |
| const users: UserEntity[] = []; | |
| const now = Date.now(); | |
| for (let i = 1; i <= 90; i++) { | |
| const user = new UserEntity(); | |
| user.githubId = `seed-user-${i}-${now}`; | |
| user.githubAccessToken = "dummy-token"; | |
| user.email = `user${i}_${now}@example.com`; | |
| user.username = `seeduser${i}_${now.toString().slice(-5)}`; | |
| user.name = `Seed User ${i}`; | |
| user.profilePicture = `https://api.dicebear.com/7.x/avataaars/svg?seed=${user.username}`; | |
| users.push(user); |
🤖 Prompt for AI Agents
In `@api/src/scripts/seed-users-teams.ts` around lines 56 - 66, The seed script
creates UserEntity records with fixed emails (user{i}@example.com) which will
collide on repeated runs; update the email generation in the users loop to
include the run-unique suffix (use the existing now or the same
githubId/username suffix) so each email is unique (e.g.,
`user{i}+{now}@example.com`), or alternatively add a cleanup step that removes
previously seeded users before inserting; modify the loop that constructs
UserEntity (variables users, user.githubId, user.username) to include the unique
suffix in user.email or add a pre-insert cleanup for those seed records.
…ce graph visualization with improved ReactFlow configuration and layout for bracket and group pages.
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores