Fix: removed duplicate toggleCamera function and cleaned TeamDebateRo… #124
Conversation
WalkthroughThe changes introduce a new WebSocket handler for team-based debate connections in the backend, update the server to route debates through this handler, promote Casbin dependencies to direct declarations in go.mod, and remove the Changes
Sequence DiagramsequenceDiagram
participant Client as Client Request
participant Handler as TeamDebateWebsocketHandler
participant Gin as Gin Context
participant Upgrader as WebSocket Upgrader
participant Hub as Debate Hub
participant Client_obj as TeamDebateClient
Client->>Handler: Request with debateID, teamId, userId, isTeam1
Handler->>Gin: Extract parameters from context
Gin-->>Handler: Parsed IDs & boolean
Handler->>Upgrader: Upgrade HTTP to WebSocket
Upgrader-->>Handler: WebSocket connection
Handler->>Client_obj: Construct TeamDebateClient
Handler->>Hub: Register client
Hub-->>Handler: Client registered
Handler->>Client_obj: Start read pump
Handler->>Client_obj: Start write pump
Client_obj-->>Client: WebSocket established
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/websocket/team_debate_handler.go (1)
257-260: Add logging for WebSocket upgrade failures.The handler silently returns when the WebSocket upgrade fails, making debugging connection issues difficult.
Apply this diff to add error logging:
conn, err := upgrader.Upgrade(c.Writer, c.Request, nil) if err != nil { + log.Printf("Failed to upgrade WebSocket connection for debate %s: %v", debateIDHex, err) return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/go.sumis excluded by!**/*.sumfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
backend/cmd/server/main.go(2 hunks)backend/go.mod(1 hunks)backend/websocket/team_debate_handler.go(2 hunks)frontend/src/Pages/TeamDebateRoom.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/Pages/TeamDebateRoom.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
backend/cmd/server/main.go (1)
backend/websocket/team_debate_handler.go (1)
TeamDebateWebsocketHandler(247-275)
🔇 Additional comments (2)
backend/go.mod (1)
8-9: I'll search for the latest versions of these Casbin dependencies to verify they are current.Based on my verification:
Security advisories status verified: No known vulnerabilities found.
The latest version of casbin/casbin/v2 was published November 14, 2025, and both
v2.132.0andv3.7.0of the mongodb-adapter are valid Go module versions with no known CVEs. Both v3 and v4 versions of the mongodb-adapter remain documented and supported, so using v3.7.0 is acceptable.The dependencies are current and free from known vulnerabilities.
backend/cmd/server/main.go (1)
150-151: The review comment is based on incorrect analysis of the handler implementation.The
TeamDebateWebsocketHandlerextractsuserIdandteamIdas query parameters, but these parameters are optional. When missing,c.Query()returns an empty string, whichprimitive.ObjectIDFromHex()silently converts to a zero ObjectID without validation. The client is then registered with zero IDs regardless. There is no requirement check for these fields.The code therefore already supports anonymous spectators exactly as the comment states. Spectators can connect without providing
userIdorteamIdquery parameters, and the handler will accept the connection with zero-value ObjectIDs.While there is a minor code quality concern (the anonymous access model could be more explicit rather than implicit via empty/zero IDs), the endpoint's functionality aligns with its documented behavior.
Likely an incorrect or invalid review comment.
| func TeamDebateWebsocketHandler(c *gin.Context) { | ||
| debateIDHex := c.Param("debateID") | ||
| teamIDHex := c.Query("teamId") | ||
| userIDHex := c.Query("userId") | ||
| isTeam1 := c.Query("isTeam1") == "true" | ||
|
|
||
| debateID, _ := primitive.ObjectIDFromHex(debateIDHex) | ||
| teamID, _ := primitive.ObjectIDFromHex(teamIDHex) | ||
| userID, _ := primitive.ObjectIDFromHex(userIDHex) | ||
|
|
||
| conn, err := upgrader.Upgrade(c.Writer, c.Request, nil) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| client := &TeamDebateClient{ | ||
| conn: conn, | ||
| send: make(chan []byte, 256), | ||
| debateID: debateID, | ||
| teamID: teamID, | ||
| userID: userID, | ||
| isTeam1: isTeam1, | ||
| } | ||
|
|
||
| teamDebateHub.register <- client | ||
|
|
||
| go client.writePump() | ||
| go client.readPump() | ||
| } |
There was a problem hiding this comment.
Add authentication and authorization checks.
The handler doesn't verify that the requesting user has permission to join the debate or that they belong to the specified team. This could allow:
- Unauthorized users to join debates
- Users to impersonate other users by providing different
userIdparameters - Users to access debates they shouldn't be able to see
Consider implementing one of these approaches:
-
Add JWT authentication: Extract and validate the user's JWT token, then verify the token's user ID matches the
userIdparameter. -
Verify team membership: Query the database to confirm the user belongs to the specified team and that team is participating in the debate.
-
Apply AuthMiddleware: If the route should be protected, register it within an authenticated route group in
main.go.
Example authorization check:
func TeamDebateWebsocketHandler(c *gin.Context) {
// ... existing parameter extraction and validation ...
// Verify user belongs to the team
teamCollection := db.GetCollection("teams")
var team models.Team
err := teamCollection.FindOne(nil, bson.M{
"_id": teamID,
"members": bson.M{"$elemMatch": bson.M{"user_id": userID}},
}).Decode(&team)
if err != nil {
c.JSON(403, gin.H{"error": "User is not a member of the specified team"})
return
}
// Verify team is part of the debate
debateCollection := db.GetCollection("team_debates")
var debate models.TeamDebate
err = debateCollection.FindOne(nil, bson.M{
"_id": debateID,
"$or": []bson.M{
{"team1_id": teamID},
{"team2_id": teamID},
},
}).Decode(&debate)
if err != nil {
c.JSON(403, gin.H{"error": "Team is not participating in this debate"})
return
}
// ... rest of handler ...
}🤖 Prompt for AI Agents
In backend/websocket/team_debate_handler.go around lines 247-275, the handler
upgrades to a websocket without authenticating or authorizing the requester; fix
by (1) validating and parsing the authenticated user (e.g., extract/verify JWT
or require the route to be behind AuthMiddleware) and ensure the token's user ID
matches the userId query parameter, (2) check the parsed ObjectIDs for errors
and return 400 on invalid IDs before attempting an upgrade, (3) query the DB to
verify the user is a member of the specified team, and (4) confirm that the team
is a participant in the given debate; if any check fails, return an appropriate
4xx response and do not call upgrader.Upgrade or register the client.
| debateIDHex := c.Param("debateID") | ||
| teamIDHex := c.Query("teamId") | ||
| userIDHex := c.Query("userId") | ||
| isTeam1 := c.Query("isTeam1") == "true" | ||
|
|
||
| debateID, _ := primitive.ObjectIDFromHex(debateIDHex) | ||
| teamID, _ := primitive.ObjectIDFromHex(teamIDHex) | ||
| userID, _ := primitive.ObjectIDFromHex(userIDHex) |
There was a problem hiding this comment.
Add input validation and error handling for required parameters.
The handler extracts query parameters and converts them to ObjectIDs without validation or error handling. This creates several risks:
- Missing required parameters (
teamId,userId) will result in empty strings being parsed, creating zero-value ObjectIDs - Invalid ObjectID hex strings will silently fail conversion, also resulting in zero-value ObjectIDs
- Zero-value ObjectIDs could cause incorrect database lookups or matches with unintended documents
Apply this diff to add proper validation and error handling:
func TeamDebateWebsocketHandler(c *gin.Context) {
debateIDHex := c.Param("debateID")
teamIDHex := c.Query("teamId")
userIDHex := c.Query("userId")
isTeam1 := c.Query("isTeam1") == "true"
+
+ // Validate required parameters
+ if debateIDHex == "" || teamIDHex == "" || userIDHex == "" {
+ c.JSON(400, gin.H{"error": "Missing required parameters: debateID, teamId, and userId"})
+ return
+ }
- debateID, _ := primitive.ObjectIDFromHex(debateIDHex)
- teamID, _ := primitive.ObjectIDFromHex(teamIDHex)
- userID, _ := primitive.ObjectIDFromHex(userIDHex)
+ debateID, err := primitive.ObjectIDFromHex(debateIDHex)
+ if err != nil {
+ c.JSON(400, gin.H{"error": "Invalid debateID format"})
+ return
+ }
+
+ teamID, err := primitive.ObjectIDFromHex(teamIDHex)
+ if err != nil {
+ c.JSON(400, gin.H{"error": "Invalid teamId format"})
+ return
+ }
+
+ userID, err := primitive.ObjectIDFromHex(userIDHex)
+ if err != nil {
+ c.JSON(400, gin.H{"error": "Invalid userId format"})
+ return
+ }📝 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.
| debateIDHex := c.Param("debateID") | |
| teamIDHex := c.Query("teamId") | |
| userIDHex := c.Query("userId") | |
| isTeam1 := c.Query("isTeam1") == "true" | |
| debateID, _ := primitive.ObjectIDFromHex(debateIDHex) | |
| teamID, _ := primitive.ObjectIDFromHex(teamIDHex) | |
| userID, _ := primitive.ObjectIDFromHex(userIDHex) | |
| debateIDHex := c.Param("debateID") | |
| teamIDHex := c.Query("teamId") | |
| userIDHex := c.Query("userId") | |
| isTeam1 := c.Query("isTeam1") == "true" | |
| // Validate required parameters | |
| if debateIDHex == "" || teamIDHex == "" || userIDHex == "" { | |
| c.JSON(400, gin.H{"error": "Missing required parameters: debateID, teamId, and userId"}) | |
| return | |
| } | |
| debateID, err := primitive.ObjectIDFromHex(debateIDHex) | |
| if err != nil { | |
| c.JSON(400, gin.H{"error": "Invalid debateID format"}) | |
| return | |
| } | |
| teamID, err := primitive.ObjectIDFromHex(teamIDHex) | |
| if err != nil { | |
| c.JSON(400, gin.H{"error": "Invalid teamId format"}) | |
| return | |
| } | |
| userID, err := primitive.ObjectIDFromHex(userIDHex) | |
| if err != nil { | |
| c.JSON(400, gin.H{"error": "Invalid userId format"}) | |
| return | |
| } |
🤖 Prompt for AI Agents
In backend/websocket/team_debate_handler.go around lines 248 to 255, the handler
currently converts params to ObjectIDs without validation causing zero-value IDs
on missing/invalid input; update the code to: verify debateID, teamId and userId
are present (non-empty), call primitive.ObjectIDFromHex for each and check the
returned error, and if any conversion fails return a 400 response (with a clear
message indicating which param is missing/invalid) instead of proceeding; do not
use the zero-value ObjectIDs for DB lookups and keep isTeam1 parsed from the
query with a safe default.
1 participant
This pull request includes several frontend updates to improve layout consistency and component behavior.
No major functionality was changed—these are mostly structural and minor UI adjustments.
Changes Made
Refactored frontend layout for better organization
Updated component structure and improved readability
Minor styling fixes and alignment improvements
Cleaned up unnecessary code artifacts
✅ Note
Small frontend errors or warnings are not critical and do not affect core functionality. They can be refined in later passes.
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.