-
Notifications
You must be signed in to change notification settings - Fork 30
fix(frontend): improve TeamSelector synchronization with task detail #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,36 +41,61 @@ export default function TeamSelector({ | |
| useEffect(() => { | ||
| 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, | ||
| }); | ||
|
|
||
| // Wait for teams to load before syncing | ||
| if (teams.length === 0) { | ||
| console.log('[TeamSelector] Teams not loaded yet, skipping sync'); | ||
| return; | ||
| } | ||
|
Comment on lines
+51
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard may prevent sync when teams load after task detail. The Consider using the - // 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 |
||
|
|
||
| // Priority 1: Set team from task detail if viewing a task | ||
| if ( | ||
| selectedTaskDetail && | ||
| 'team' in selectedTaskDetail && | ||
| selectedTaskDetail.team && | ||
| teams.length > 0 | ||
| ) { | ||
| const foundTeam = | ||
| teams.find(t => t.id === (selectedTaskDetail.team as { id: number }).id) || null; | ||
| if (foundTeam && (!selectedTeam || selectedTeam.id !== foundTeam.id)) { | ||
| console.log('[TeamSelector] Setting team from task detail:', foundTeam.name, foundTeam.id); | ||
| setSelectedTeam(foundTeam); | ||
| if (selectedTaskDetail && 'team' in selectedTaskDetail) { | ||
| const taskTeam = selectedTaskDetail.team; | ||
|
|
||
| // 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; | ||
|
|
||
| if (foundTeam) { | ||
| // Only update if team changed to avoid unnecessary re-renders | ||
| if (!selectedTeam || selectedTeam.id !== foundTeam.id) { | ||
| console.log('[TeamSelector] Setting team from task detail:', foundTeam.name, foundTeam.id); | ||
| setSelectedTeam(foundTeam); | ||
| } | ||
| return; // Team synced successfully, exit early | ||
| } else { | ||
| // Team exists in task detail but not in user's team list (deleted or no access) | ||
| console.log('[TeamSelector] Task team not found in user teams, keeping current selection'); | ||
| // Keep current selectedTeam unchanged - don't clear it | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Case 2: Task has no team (team was deleted or never set) | ||
| if (taskTeam === null) { | ||
| console.log('[TeamSelector] Task has no team, clearing team selection'); | ||
| if (selectedTeam !== null) { | ||
| setSelectedTeam(null); | ||
| } | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Priority 2: Validate selected team still exists in list | ||
| if (selectedTeam && teams.length > 0) { | ||
| // Priority 2: Validate selected team still exists in list (only when not viewing a task detail) | ||
| if (!selectedTaskDetail && selectedTeam && teams.length > 0) { | ||
| const exists = teams.some(team => team.id === selectedTeam.id); | ||
| if (!exists) { | ||
| console.log('[TeamSelector] Selected team not in list, clearing selection'); | ||
| setSelectedTeam(null); | ||
| } | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [selectedTaskDetail, teams]); | ||
|
|
||
| const handleChange = (value: string) => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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:
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