fix(frontend): improve TeamSelector synchronization with task detail #189
Conversation
- Add comprehensive logging for task team changes and selection state - Wait for teams list to load before attempting synchronization - Handle three distinct cases: 1. Task has valid team: sync if found in user's teams, otherwise keep current selection 2. Task has no team (null): clear team selection 3. No task detail: validate current selection exists in teams list - Add proper dependency array to useEffect (include selectedTeam and setSelectedTeam) - Prevent unnecessary re-renders by checking if team actually changed - Only validate selected team against teams list when not viewing a task detail - Improve edge case handling for deleted teams and access restrictions
|
Warning Rate limit exceeded@qdaxb has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe TeamSelector component's useEffect was refactored to robustly synchronize the selected team from task details. The update adds guards for data readiness, implements multi-case branching for team existence scenarios (valid team found, team not in user list, or null team), and adjusts dependencies to prevent stale closures and unnecessary re-renders. Changes
Sequence DiagramsequenceDiagram
participant Effect as useEffect Hook
participant Teams as Teams Data
participant TaskDetail as Task Detail
participant Selection as Selected Team State
Effect->>Teams: Check if teams loaded?
alt Teams not loaded
Effect->>Effect: Exit early, wait
end
Effect->>TaskDetail: Task detail exists?
alt Task detail exists
Effect->>TaskDetail: Get task team ID
alt Task team is valid object
Effect->>Teams: Find matching team in list
alt Found
Effect->>Selection: Set selected team
Effect->>Effect: Exit early
else Not found
Effect->>Selection: Keep current selection
Effect->>Effect: Exit early
end
else Task team is null
Effect->>Selection: Clear selection
Effect->>Effect: Exit early
end
else No task detail
Effect->>Selection: Validate current team exists
alt Team missing from list
Effect->>Selection: Clear selection
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 3
🧹 Nitpick comments (2)
frontend/src/features/tasks/components/TeamSelector.tsx (2)
62-79: Improve type safety by leveraging TypeScript's type narrowing.The type assertion on line 63 bypasses TypeScript's type safety. After the type guard on line 62, TypeScript should be able to narrow the type automatically if
taskTeamis properly typed. If the Team type is correctly defined, this assertion should be unnecessary.Refactor to use type narrowing:
- // Case 1: Task has a valid team - if (taskTeam && typeof taskTeam === 'object' && 'id' in taskTeam) { - const taskTeamId = (taskTeam as { id: number }).id; - const foundTeam = teams.find(t => t.id === taskTeamId) || null; + // Case 1: Task has a valid team + if (taskTeam && typeof taskTeam.id === 'number') { + const foundTeam = teams.find(t => t.id === taskTeam.id) || null;This eliminates the type assertion and makes the code safer by directly accessing
taskTeam.idafter verifying it exists and is a number.
91-98: Optional: Remove redundantteams.length > 0check.The
teams.length > 0condition on line 92 is redundant because the early return on line 52 already handles the empty teams case. However, if you modify the earlier guard to useisLoadingas suggested in my previous comment, this check becomes meaningful again for the edge case where a user legitimately has no teams.If keeping the current guard at line 52, simplify to:
- // Priority 2: Validate selected team still exists in list (only when not viewing a task detail) - if (!selectedTaskDetail && selectedTeam && teams.length > 0) { + // Priority 2: Validate selected team still exists in list (only when not viewing a task detail) + if (!selectedTaskDetail && selectedTeam) { const exists = teams.some(team => team.id === selectedTeam.id); if (!exists) { console.log('[TeamSelector] Selected team not in list, clearing selection'); setSelectedTeam(null); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/features/tasks/components/TeamSelector.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: TypeScript strict mode must be enabled in Frontend
Use functional components with hooks in React
Use Prettier formatter in Frontend with single quotes and no semicolons
Use ESLint with Next.js configuration in Frontend
Useconstoverlet, never usevarin TypeScript
Component names must use PascalCase; file names must use kebab-case in Frontend
Use mobile-first responsive design approach in Frontend components
Frontend design: Low saturation + low contrast for reduced eye strain with generous whitespace
Frontend design: Minimal shadows, subtle component differentiation (<10% background variance)
Frontend design: Use mint blue (#14B8A6) as primary accent sparingly
Use Tailwind CSS variables for colors: --color-bg-base, --color-text-primary, --color-border, etc.
Frontend spacing: Use standard units - p-2 (8px), p-4 (16px), p-6 (24px), gap-3 (12px)
Frontend border-radius: rounded-2xl (16px) for large containers, rounded-lg (12px) for cards, rounded-md (6px) for buttons
Frontend typography: H1 text-xl font-semibold, H2 text-lg font-semibold, H3 text-base font-medium, Body text-sm
Use shadcn/ui Button component with variants: default, secondary, ghost, outline, link
Use shadcn/ui Form component with react-hook-form + zod validation
Files:
frontend/src/features/tasks/components/TeamSelector.tsx
frontend/**
📄 CodeRabbit inference engine (AGENTS.md)
Frontend: Only use
NEXT_PUBLIC_*for client-safe environment variables
Files:
frontend/src/features/tasks/components/TeamSelector.tsx
| console.log('[TeamSelector] Effect triggered', { | ||
| hasSelectedTaskDetail: !!selectedTaskDetail, | ||
| taskDetailTeamId: selectedTaskDetail?.team?.id || 'null', | ||
| taskDetailTeamName: selectedTaskDetail?.team?.name || 'null', | ||
| selectedTeam: selectedTeam?.name || 'null', | ||
| selectedTeamId: selectedTeam?.id || 'null', | ||
| teamsLength: teams.length, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove console.log statements or make them development-only.
Console logs should not be present in production code as they clutter the browser console and may expose internal logic. Consider removing them entirely or wrapping them in a development check.
Apply this pattern to conditionally log only in development:
- console.log('[TeamSelector] Effect triggered', {
- hasSelectedTaskDetail: !!selectedTaskDetail,
- taskDetailTeamId: selectedTaskDetail?.team?.id || 'null',
- taskDetailTeamName: selectedTaskDetail?.team?.name || 'null',
- selectedTeam: selectedTeam?.name || 'null',
- selectedTeamId: selectedTeam?.id || 'null',
- teamsLength: teams.length,
- });
+ if (process.env.NODE_ENV === 'development') {
+ console.log('[TeamSelector] Effect triggered', {
+ hasSelectedTaskDetail: !!selectedTaskDetail,
+ taskDetailTeamId: selectedTaskDetail?.team?.id || 'null',
+ taskDetailTeamName: selectedTaskDetail?.team?.name || 'null',
+ selectedTeam: selectedTeam?.name || 'null',
+ selectedTeamId: selectedTeam?.id || 'null',
+ teamsLength: teams.length,
+ })
+ }Apply the same pattern to all other console.log statements in the effect (lines 53, 69, 75, 83, 95).
Also applies to: 53-53, 69-69, 75-75, 83-83, 95-95
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/TeamSelector.tsx around lines 42-95,
there are console.log calls used for debugging (lines 42, 53, 69, 75, 83, 95);
remove these console.log statements from production by either deleting them or
wrapping each in a development-only guard (e.g., check process.env.NODE_ENV ===
'development' or your app's equivalent) so logs only run in development, or
replace with a centralized dev-only logger helper and use it for all the logs in
this effect.
| // Wait for teams to load before syncing | ||
| if (teams.length === 0) { | ||
| console.log('[TeamSelector] Teams not loaded yet, skipping sync'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Guard may prevent sync when teams load after task detail.
The teams.length === 0 check assumes teams always load before selectedTaskDetail. If task detail arrives first (e.g., from URL navigation), the effect will skip synchronization even after teams eventually load, because the effect doesn't re-run when teams goes from empty to populated if other dependencies haven't changed.
Consider using the isLoading prop to determine data readiness:
- // Wait for teams to load before syncing
- if (teams.length === 0) {
- console.log('[TeamSelector] Teams not loaded yet, skipping sync');
- return;
- }
+ // Skip sync while teams are still loading
+ if (isLoading) {
+ return;
+ }This ensures the effect waits for loading to complete rather than checking array length, which handles both the loading state and the legitimate "no teams" case correctly.
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/TeamSelector.tsx around lines 51-55,
the effect currently guards with teams.length === 0 which can skip sync
permanently if selectedTaskDetail arrives before teams; replace that guard with
the teams loading flag (e.g., isLoading or teamsIsLoading) so the effect waits
for loading to complete and still re-runs when teams transition from loading to
loaded, and ensure the effect's dependency array includes the loading flag (and
selectedTaskDetail) so synchronization runs after teams finish loading and
correctly handles both "still loading" and "loaded but empty" cases.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
Fixed the issue where TeamSelector component fails to automatically switch teams when users navigate between different conversations from the history message list after using the lite API.
Changes
Core Logic Improvements
Enhanced State Synchronization
Three-Case Handling
Edge Cases Handled
Performance Optimization
Technical Details
Modified File: frontend/src/features/tasks/components/TeamSelector.tsx
Key Changes:
Testing Scenarios
Verification
The fix ensures:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.