Skip to content

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

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.

Sign up for GitHub

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

Merged
gtryus merged 1 commit into from
Dec 23, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 145 additions & 53 deletions src/renderer/src/routes/ProjectsScreen.cy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,89 @@ describe('ProjectsScreen', () => {
);
};

// Helper function to create a standard test team
const createTestTeam = (teamId = 'test-team-id', teamName = 'Test Team') => ({
id: teamId,
type: 'organization',
attributes: { name: teamName },
});

// Helper function to set up localStorage with teamId
const setupTeamInLocalStorage = (teamId: string, planId?: string) => {
cy.window().then((win) => {
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
if (planId) {
win.localStorage.setItem(LocalKey.plan, planId);
}
});
};

// Helper function to mount ProjectsScreen with standard team setup
const mountWithTeam = (
teamId = 'test-team-id',
options: {
isAdmin?: (team: any) => boolean;
personalTeam?: string;
initialState?: ReturnType<typeof createInitialState>;
initialEntries?: string[];
} = {}
) => {
const {
isAdmin = () => false,
personalTeam = 'personal-team-id',
initialState = createInitialState(),
initialEntries = ['/projects'],
} = options;

const mockTeam = createTestTeam(teamId);
setupTeamInLocalStorage(teamId);

mountProjectsScreen(initialState, initialEntries, {
isAdmin,
personalTeam,
teams: [mockTeam],
});
Comment on lines +299 to +306
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
};

// Helper function to mount ProjectsScreen without teamId (for testing missing teamId scenario)
const mountWithoutTeam = (
options: {
planId?: string;
personalTeam?: string;
initialState?: ReturnType<typeof createInitialState>;
initialEntries?: string[];
} = {}
) => {
const {
planId,
personalTeam = 'personal-team-id',
initialState = createInitialState(),
initialEntries = ['/projects'],
} = options;

cy.window().then((win) => {
win.localStorage.removeItem(localUserKey(LocalKey.team));
if (planId) {
win.localStorage.setItem(LocalKey.plan, planId);
}
});

mountProjectsScreen(initialState, initialEntries, {
isAdmin: () => false,
personalTeam,
teams: [],
});
};

it('should render ProjectsScreen', () => {
mountProjectsScreen(createInitialState());
mountWithTeam();

cy.get('#ProjectsScreen').should('exist');
cy.get('header').should('exist'); // AppHead should render
});

it('should display "No projects yet." when there are no projects', () => {
mountProjectsScreen(createInitialState(), ['/projects']);
mountWithTeam();

// TeamProvider will query memory for projects, which will be empty
// So we should see the "No projects yet." message
Expand All @@ -279,20 +353,8 @@ describe('ProjectsScreen', () => {

it('should show "Add New Project..." button when isAdmin returns true', () => {
const teamId = 'test-team-id';
const mockTeam = {
id: teamId,
type: 'organization',
attributes: { name: 'Test Team' },
};

cy.window().then((win) => {
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
});

mountProjectsScreen(createInitialState(), ['/projects'], {
mountWithTeam(teamId, {
isAdmin: (team: any) => team?.id === teamId,
personalTeam: 'personal-team-id',
teams: [mockTeam],
});

cy.get('#ProjectActAdd').should('be.visible');
Expand All @@ -301,20 +363,8 @@ describe('ProjectsScreen', () => {

it('should open ProjectDialog when "Add New Project..." button is clicked', () => {
const teamId = 'test-team-id';
const mockTeam = {
id: teamId,
type: 'organization',
attributes: { name: 'Test Team' },
};

cy.window().then((win) => {
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
});

mountProjectsScreen(createInitialState(), ['/projects'], {
mountWithTeam(teamId, {
isAdmin: (team: any) => team?.id === teamId,
personalTeam: 'personal-team-id',
teams: [mockTeam],
});

cy.get('#ProjectActAdd').click();
Expand All @@ -324,35 +374,20 @@ describe('ProjectsScreen', () => {
});

it('should not show "Add New Project..." button when isAdmin returns false', () => {
const teamId = 'test-team-id';
const mockTeam = {
id: teamId,
type: 'organization',
attributes: { name: 'Test Team' },
};

cy.window().then((win) => {
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
});

mountProjectsScreen(createInitialState(), ['/projects'], {
isAdmin: () => false, // Not admin
personalTeam: 'personal-team-id',
teams: [mockTeam],
});
mountWithTeam();

cy.get('#ProjectActAdd').should('not.exist');
});

it('should show "Switch Teams" button', () => {
mountProjectsScreen(createInitialState());
mountWithTeam();

cy.get('#ProjectActSwitch').should('be.visible');
cy.contains('Switch Teams').should('be.visible');
});

it('should navigate to switch-teams when "Switch Teams" button is clicked', () => {
mountProjectsScreen(createInitialState(), ['/projects']);
mountWithTeam();

cy.get('#ProjectActSwitch').click();

Expand All @@ -365,16 +400,14 @@ describe('ProjectsScreen', () => {
});

it('should not show "Edit Workflow" button when not admin', () => {
mountProjectsScreen(createInitialState(), ['/projects']);
mountWithTeam();

// Edit Workflow button should not exist for non-admin users
cy.get('#ProjectActEditWorkflow').should('not.exist');
});

it('should not show "Edit Workflow" button when viewing personal projects', () => {
cy.window().then((win) => {
win.localStorage.setItem(localUserKey(LocalKey.team), 'personal-team-id');
});
setupTeamInLocalStorage('personal-team-id');

mountProjectsScreen(createInitialState(), ['/projects']);

Expand All @@ -385,7 +418,7 @@ describe('ProjectsScreen', () => {
it('should not show "Edit Workflow" button on mobile devices', () => {
cy.viewport(400, 800); // Mobile viewport

mountProjectsScreen(createInitialState(), ['/projects']);
mountWithTeam();

// Edit Workflow button should not be visible on mobile
cy.get('#ProjectActEditWorkflow').should('not.exist');
Expand All @@ -394,18 +427,77 @@ describe('ProjectsScreen', () => {
it('should apply mobile styling when on mobile device', () => {
cy.viewport(400, 800); // Mobile viewport

mountProjectsScreen(createInitialState());
mountWithTeam();

cy.get('#ProjectsScreen').should('exist');
// Mobile styling is applied via isMobile prop
});

it('should set home to true on mount', () => {
mountProjectsScreen(createInitialState({ home: false }));
mountWithTeam('test-team-id', {
initialState: createInitialState({ home: false }),
});

// The component sets home to true in useEffect
// We can verify this by checking that navigation doesn't happen immediately
cy.wait(100);
cy.get('#ProjectsScreen').should('exist');
});

it('should navigate to /switch-teams when teamId is undefined', () => {
mountWithoutTeam({ planId: 'test-plan-id' });

// Wait for the navigation effect to occur
// The component calls handleSwitchTeams() in useEffect when teamId is undefined,
// which removes the plan from localStorage and navigates to /switch-teams
cy.wait(200).then(() => {
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
// Verify that navigation occurred by checking side effects of handleSwitchTeams
// handleSwitchTeams removes LocalKey.plan before navigating to /switch-teams
cy.window().then((win) => {
// The plan should be removed (side effect of handleSwitchTeams, proving it was called)
// This confirms that navigation to /switch-teams was triggered
expect(win.localStorage.getItem(LocalKey.plan)).to.be.null;
// Verify teamId is still undefined
expect(win.localStorage.getItem(localUserKey(LocalKey.team))).to.be
.null;
});
});

// The component should not render its content when teamId is missing
// (it returns null early)
cy.get('#ProjectsScreen').should('not.exist');
});

it('should not navigate to /switch-teams when teamId is defined', () => {
const teamId = 'test-team-id';

// Set teamId and plan to verify handleSwitchTeams is not called
setupTeamInLocalStorage(teamId, 'test-plan-id');

const mockTeam = createTestTeam(teamId);

// Mount with teamId defined - this should NOT trigger navigation
// The component has: if (!teamId) handleSwitchTeams();
// Since teamId is defined, handleSwitchTeams should not be called
mountProjectsScreen(createInitialState(), ['/projects'], {
isAdmin: () => false,
personalTeam: 'personal-team-id',
teams: [mockTeam],
});

// Wait a bit to ensure navigation doesn't happen
cy.wait(200);

// Verify ProjectsScreen renders (navigation did not happen)
Comment on lines +488 to +491
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
cy.get('#ProjectsScreen').should('exist');

// Verify that handleSwitchTeams was NOT called by checking localStorage
cy.window().then((win) => {
expect(win.localStorage.getItem(localUserKey(LocalKey.team))).to.equal(
teamId
);
// Plan should still be set (handleSwitchTeams was not called, so no navigation occurred)
expect(win.localStorage.getItem(LocalKey.plan)).to.equal('test-plan-id');
});
});
});
37 changes: 25 additions & 12 deletions src/renderer/src/routes/ProjectsScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,26 @@ export const ProjectsScreenInner: React.FC = () => {
const theme = useTheme();
const isMobileWidth = useMediaQuery(theme.breakpoints.down('sm'));

const handleSwitchTeams = React.useCallback(() => {
localStorage.removeItem(LocalKey.plan);
navigate('/switch-teams');
}, [navigate]);

// Handle missing teamId with useEffect to prevent infinite render loops
React.useEffect(() => {
if (!teamId) {
handleSwitchTeams();
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [handleSwitchTeams]);

React.useEffect(() => {
startClear();
setHome(true);
// we intentionally do not reset project/plan here; selection will set them
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const isPersonal = teamId === personalTeam;
const projects = React.useMemo(
() => (isPersonal ? personalProjects : teamId ? teamProjects(teamId) : []),
Expand Down Expand Up @@ -160,18 +180,6 @@ export const ProjectsScreenInner: React.FC = () => {
setAddOpen(false);
};

const handleSwitchTeams = () => {
localStorage.removeItem(LocalKey.plan);
navigate('/switch-teams');
};

React.useEffect(() => {
startClear();
setHome(true);
// we intentionally do not reset project/plan here; selection will set them
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

// Navigate to plan page only after user explicitly leaves home (card click triggers leaveHome)
React.useEffect(() => {
if (!plan) return; // no selection yet
Expand All @@ -191,6 +199,11 @@ export const ProjectsScreenInner: React.FC = () => {
return thisTeam && isAdmin(thisTeam);
}, [thisTeam, isAdmin]);

// Early return when teamId is missing to prevent errors in derived values
if (!teamId) {
return null; // or a loading state
}

return (
<Box sx={{ width: '100%' }}>
<AppHead />
Expand Down