TT-6998 Improve navigation logic in ProjectsScreen based on teamId state

[gtryus]

• Added tests to verify navigation to /switch-teams when teamId is undefined, ensuring proper handling of localStorage.
• Implemented logic in ProjectsScreen to call handleSwitchTeams if teamId is not set, enhancing user flow for team management.

These changes improve the navigation experience by ensuring users are directed to the appropriate screen based on their team selection.

[Copilot]

Pull request overview

This PR improves the navigation logic in ProjectsScreen by ensuring users without a selected team are automatically redirected to the team selection screen. The implementation adds a check for undefined teamId and corresponding test coverage.

• Added conditional navigation logic to redirect users to /switch-teams when teamId is not set
• Implemented two test cases to verify the navigation behavior with and without a defined teamId

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/renderer/src/routes/ProjectsScreen.tsx	Added check to call handleSwitchTeams() when teamId is undefined, redirecting users to team selection
src/renderer/src/routes/ProjectsScreen.cy.tsx	Added two test cases verifying navigation occurs when teamId is undefined and doesn't occur when teamId is defined

---

Critical Issues Identified:

The implementation in ProjectsScreen.tsx has a critical bug that will cause an infinite render loop. The handleSwitchTeams() function is being called unconditionally in the component body (line 194), which will execute on every render. This creates an infinite loop: render → call handleSwitchTeams → navigate → re-render → call handleSwitchTeams again.

Additionally, the component continues to render its JSX after calling handleSwitchTeams, which can lead to errors since the component depends on teamId being present for computing derived values like thisTeam and projects.

The proper solution is to wrap this logic in a useEffect hook with appropriate dependencies, and return early (e.g., render null or a loading state) to prevent rendering the rest of the component when teamId is missing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

There's significant code duplication across multiple test cases where the same teamId setup, mockTeam creation, and localStorage initialization is repeated. Consider extracting this common setup into a helper function or beforeEach hook to improve maintainability and reduce duplication.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test relies on cy.wait() with hardcoded delays (200ms) to verify that navigation doesn't occur. This approach is brittle and can lead to flaky tests. The test should instead use Cypress's retry-ability by checking for the expected state directly without arbitrary waits.

Suggested change

	// Wait a bit to ensure navigation doesn't happen
	cy.wait(200);
	// Verify ProjectsScreen renders (navigation did not happen)
	// Verify ProjectsScreen renders (navigation did not happen)
	// Cypress will automatically retry this assertion until it passes or times out.

[Copilot]

The test uses cy.window().then() to set up localStorage asynchronously before mounting the component. However, Cypress commands are asynchronous and queued, while mountProjectsScreen is called synchronously. This creates a race condition where the component might mount before localStorage is properly set up. The setupTeamInLocalStorage call inside mountWithTeam should use cy.then() to ensure the mount happens after localStorage setup is complete, or the localStorage setup should happen before calling mountWithTeam.

[Copilot]

The test relies on cy.wait() with hardcoded delays (200ms) to verify asynchronous behavior. This approach is brittle and can lead to flaky tests. The test should instead use Cypress's retry-ability by checking for the expected state changes directly, or use cy.waitUntil() or other deterministic approaches to verify the navigation occurred.