fix: resolved merge conflicts #138
Conversation
WalkthroughThe PR resolves merge conflicts and refactors multiple frontend components: centralizing WebSocket message handling in Game.tsx with a dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/Pages/TeamDebateRoom.tsx (1)
1492-1503: Hardcoded localhost URL will break in production.The transcript submission endpoint uses a hardcoded
http://localhost:1313URL, butBASE_URLis already defined and used elsewhere for WebSocket connections. This will fail in any non-local environment.🔎 Proposed fix
- const response = await fetch(`http://localhost:1313/submit-transcripts`, { + const response = await fetch(`${BASE_URL}/submit-transcripts`, {frontend/src/Pages/MatchLogs.tsx (1)
219-223: Bug: Tiebreaker note condition will never match.The
stagevariable can only be"First Round","Semifinal", or"Final"(computed at lines 105-109), but this condition checks for"First Round Match 3"which is never assigned tostage. The tiebreaker note will never display.🔎 Proposed fix - check the original match string instead
- {stage === "First Round Match 3" && ( + {log.match.includes("First Round Match 3") && ( <p className="text-xs text-muted-foreground mt-2"> * Ayaan Khanna advanced via tiebreaker </p> )}
🧹 Nitpick comments (1)
frontend/src/Pages/TeamDebateRoom.tsx (1)
643-668: MissinghandlePhaseDonein dependency array may cause stale closure.The timer countdown effect calls
handlePhaseDone()on expiration, buthandlePhaseDoneis not included in the dependency array. This could cause stale closures ifhandlePhaseDonecaptures state that changes during the timer.🔎 Proposed fix - wrap handlePhaseDone with useCallback and add to deps
- }, [timer, debatePhase, isMyTurn, speechTranscripts, localRole, debateId]); + }, [timer, debatePhase, isMyTurn, speechTranscripts, localRole, debateId, handlePhaseDone]);Also wrap
handlePhaseDonewithuseCallback:- const handlePhaseDone = () => { + const handlePhaseDone = useCallback(() => { const currentIndex = phaseOrder.indexOf(debatePhase); ... - }; + }, [debatePhase, phaseOrder, logMessageHistory]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/Pages/Game.tsxfrontend/src/Pages/MatchLogs.tsxfrontend/src/Pages/TeamBuilder.tsxfrontend/src/Pages/TeamDebateRoom.tsx
💤 Files with no reviewable changes (1)
- frontend/src/Pages/Game.tsx
🔇 Additional comments (7)
frontend/src/Pages/TeamDebateRoom.tsx (2)
268-308: LGTM! -toggleCamerafunction is properly consolidated.The PR objective mentioned removing duplicate
toggleCamerafunction definitions. This implementation is now singular and handles camera toggle correctly with proper error handling and stream management.
145-148: LGTM! - Formatting changes are consistent.The whitespace adjustments and formatting changes throughout the file maintain consistency with the codebase style. The
isMyTurnmemo and other logic remain correct.Also applies to: 540-546
frontend/src/Pages/TeamBuilder.tsx (3)
234-249: LGTM! - Robust captain ID comparison with backwards compatibility.The
isCaptainfunction correctly handles both string IDs and MongoDB ObjectId-style objects ({ $oid: ... }). This defensive approach prevents comparison failures when ID formats differ.
821-824: Good defensive improvement to capacity calculation.The updated logic correctly guards against
maxSizebeing0or negative, which the previous|| 4fallback wouldn't handle correctly (since0 || 4would incorrectly default).
769-793: LGTM! - Captain action conditionals are correctly structured.The nested conditionals properly verify captain status and prevent captains from removing themselves. The
|| nullfallback ensures type safety for theisCaptaincall.frontend/src/Pages/MatchLogs.tsx (2)
105-109: LGTM! - Stage determination logic is correct.The ternary chain correctly categorizes matches into "First Round", "Semifinal", or "Final" stages based on the match string.
198-217: LGTM! - Score highlighting logic is correct.The total score cells correctly apply
text-primarystyling to the winning score. The formatting changes improve readability of the template literals.
|
Hey @bhavik-mangla, |
Brother I have already tested in local then raised PR, I think there is no unnecessary thing changed...I raised both issue and PR, If still have any type of issue I will definitely recheck and work on that, Thanks for the information... |
|
Hey @bhavik-mangla |
2 participants
Fixes: #123 #131 #135 #136
Changes i made -
Fixed Build-Breaking Syntax, i removed all conflict markers across these 4 files =>
Game.tsxMatchLogs.tsxTeamBuilder.tsxTeamDebateRoom.tsxTeamDebateRoom.tsx: Removed 3 duplicate function definitions (multiple toggleCamera functions), consolidated redundant useEffect hooks, and fixed a syntax error caused by a duplicated case "ready" block.Thanks : )
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.