Skip to content

Commit c0ce2fb

Browse files
gtryusGreg Trihus
andauthored
TT-6998 Improve navigation logic in ProjectsScreen based on teamId state (#178)
- 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. Refactor ProjectsScreen to handle missing teamId with early return and useEffect for navigation Co-authored-by: Greg Trihus <[email protected]>
1 parent 467aa10 commit c0ce2fb

File tree

2 files changed

+170
-65
lines changed

2 files changed

+170
-65
lines changed

src/renderer/src/routes/ProjectsScreen.cy.tsx

Lines changed: 145 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -262,15 +262,89 @@ describe('ProjectsScreen', () => {
262262
);
263263
};
264264

265+
// Helper function to create a standard test team
266+
const createTestTeam = (teamId = 'test-team-id', teamName = 'Test Team') => ({
267+
id: teamId,
268+
type: 'organization',
269+
attributes: { name: teamName },
270+
});
271+
272+
// Helper function to set up localStorage with teamId
273+
const setupTeamInLocalStorage = (teamId: string, planId?: string) => {
274+
cy.window().then((win) => {
275+
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
276+
if (planId) {
277+
win.localStorage.setItem(LocalKey.plan, planId);
278+
}
279+
});
280+
};
281+
282+
// Helper function to mount ProjectsScreen with standard team setup
283+
const mountWithTeam = (
284+
teamId = 'test-team-id',
285+
options: {
286+
isAdmin?: (team: any) => boolean;
287+
personalTeam?: string;
288+
initialState?: ReturnType<typeof createInitialState>;
289+
initialEntries?: string[];
290+
} = {}
291+
) => {
292+
const {
293+
isAdmin = () => false,
294+
personalTeam = 'personal-team-id',
295+
initialState = createInitialState(),
296+
initialEntries = ['/projects'],
297+
} = options;
298+
299+
const mockTeam = createTestTeam(teamId);
300+
setupTeamInLocalStorage(teamId);
301+
302+
mountProjectsScreen(initialState, initialEntries, {
303+
isAdmin,
304+
personalTeam,
305+
teams: [mockTeam],
306+
});
307+
};
308+
309+
// Helper function to mount ProjectsScreen without teamId (for testing missing teamId scenario)
310+
const mountWithoutTeam = (
311+
options: {
312+
planId?: string;
313+
personalTeam?: string;
314+
initialState?: ReturnType<typeof createInitialState>;
315+
initialEntries?: string[];
316+
} = {}
317+
) => {
318+
const {
319+
planId,
320+
personalTeam = 'personal-team-id',
321+
initialState = createInitialState(),
322+
initialEntries = ['/projects'],
323+
} = options;
324+
325+
cy.window().then((win) => {
326+
win.localStorage.removeItem(localUserKey(LocalKey.team));
327+
if (planId) {
328+
win.localStorage.setItem(LocalKey.plan, planId);
329+
}
330+
});
331+
332+
mountProjectsScreen(initialState, initialEntries, {
333+
isAdmin: () => false,
334+
personalTeam,
335+
teams: [],
336+
});
337+
};
338+
265339
it('should render ProjectsScreen', () => {
266-
mountProjectsScreen(createInitialState());
340+
mountWithTeam();
267341

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

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

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

280354
it('should show "Add New Project..." button when isAdmin returns true', () => {
281355
const teamId = 'test-team-id';
282-
const mockTeam = {
283-
id: teamId,
284-
type: 'organization',
285-
attributes: { name: 'Test Team' },
286-
};
287-
288-
cy.window().then((win) => {
289-
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
290-
});
291-
292-
mountProjectsScreen(createInitialState(), ['/projects'], {
356+
mountWithTeam(teamId, {
293357
isAdmin: (team: any) => team?.id === teamId,
294-
personalTeam: 'personal-team-id',
295-
teams: [mockTeam],
296358
});
297359

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

302364
it('should open ProjectDialog when "Add New Project..." button is clicked', () => {
303365
const teamId = 'test-team-id';
304-
const mockTeam = {
305-
id: teamId,
306-
type: 'organization',
307-
attributes: { name: 'Test Team' },
308-
};
309-
310-
cy.window().then((win) => {
311-
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
312-
});
313-
314-
mountProjectsScreen(createInitialState(), ['/projects'], {
366+
mountWithTeam(teamId, {
315367
isAdmin: (team: any) => team?.id === teamId,
316-
personalTeam: 'personal-team-id',
317-
teams: [mockTeam],
318368
});
319369

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

326376
it('should not show "Add New Project..." button when isAdmin returns false', () => {
327-
const teamId = 'test-team-id';
328-
const mockTeam = {
329-
id: teamId,
330-
type: 'organization',
331-
attributes: { name: 'Test Team' },
332-
};
333-
334-
cy.window().then((win) => {
335-
win.localStorage.setItem(localUserKey(LocalKey.team), teamId);
336-
});
337-
338-
mountProjectsScreen(createInitialState(), ['/projects'], {
339-
isAdmin: () => false, // Not admin
340-
personalTeam: 'personal-team-id',
341-
teams: [mockTeam],
342-
});
377+
mountWithTeam();
343378

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

347382
it('should show "Switch Teams" button', () => {
348-
mountProjectsScreen(createInitialState());
383+
mountWithTeam();
349384

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

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

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

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

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

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

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

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

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

388-
mountProjectsScreen(createInitialState(), ['/projects']);
421+
mountWithTeam();
389422

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

397-
mountProjectsScreen(createInitialState());
430+
mountWithTeam();
398431

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

403436
it('should set home to true on mount', () => {
404-
mountProjectsScreen(createInitialState({ home: false }));
437+
mountWithTeam('test-team-id', {
438+
initialState: createInitialState({ home: false }),
439+
});
405440

406441
// The component sets home to true in useEffect
407442
// We can verify this by checking that navigation doesn't happen immediately
408443
cy.wait(100);
409444
cy.get('#ProjectsScreen').should('exist');
410445
});
446+
447+
it('should navigate to /switch-teams when teamId is undefined', () => {
448+
mountWithoutTeam({ planId: 'test-plan-id' });
449+
450+
// Wait for the navigation effect to occur
451+
// The component calls handleSwitchTeams() in useEffect when teamId is undefined,
452+
// which removes the plan from localStorage and navigates to /switch-teams
453+
cy.wait(200).then(() => {
454+
// Verify that navigation occurred by checking side effects of handleSwitchTeams
455+
// handleSwitchTeams removes LocalKey.plan before navigating to /switch-teams
456+
cy.window().then((win) => {
457+
// The plan should be removed (side effect of handleSwitchTeams, proving it was called)
458+
// This confirms that navigation to /switch-teams was triggered
459+
expect(win.localStorage.getItem(LocalKey.plan)).to.be.null;
460+
// Verify teamId is still undefined
461+
expect(win.localStorage.getItem(localUserKey(LocalKey.team))).to.be
462+
.null;
463+
});
464+
});
465+
466+
// The component should not render its content when teamId is missing
467+
// (it returns null early)
468+
cy.get('#ProjectsScreen').should('not.exist');
469+
});
470+
471+
it('should not navigate to /switch-teams when teamId is defined', () => {
472+
const teamId = 'test-team-id';
473+
474+
// Set teamId and plan to verify handleSwitchTeams is not called
475+
setupTeamInLocalStorage(teamId, 'test-plan-id');
476+
477+
const mockTeam = createTestTeam(teamId);
478+
479+
// Mount with teamId defined - this should NOT trigger navigation
480+
// The component has: if (!teamId) handleSwitchTeams();
481+
// Since teamId is defined, handleSwitchTeams should not be called
482+
mountProjectsScreen(createInitialState(), ['/projects'], {
483+
isAdmin: () => false,
484+
personalTeam: 'personal-team-id',
485+
teams: [mockTeam],
486+
});
487+
488+
// Wait a bit to ensure navigation doesn't happen
489+
cy.wait(200);
490+
491+
// Verify ProjectsScreen renders (navigation did not happen)
492+
cy.get('#ProjectsScreen').should('exist');
493+
494+
// Verify that handleSwitchTeams was NOT called by checking localStorage
495+
cy.window().then((win) => {
496+
expect(win.localStorage.getItem(localUserKey(LocalKey.team))).to.equal(
497+
teamId
498+
);
499+
// Plan should still be set (handleSwitchTeams was not called, so no navigation occurred)
500+
expect(win.localStorage.getItem(LocalKey.plan)).to.equal('test-plan-id');
501+
});
502+
});
411503
});

src/renderer/src/routes/ProjectsScreen.tsx

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,26 @@ export const ProjectsScreenInner: React.FC = () => {
6060
const theme = useTheme();
6161
const isMobileWidth = useMediaQuery(theme.breakpoints.down('sm'));
6262

63+
const handleSwitchTeams = React.useCallback(() => {
64+
localStorage.removeItem(LocalKey.plan);
65+
navigate('/switch-teams');
66+
}, [navigate]);
67+
68+
// Handle missing teamId with useEffect to prevent infinite render loops
69+
React.useEffect(() => {
70+
if (!teamId) {
71+
handleSwitchTeams();
72+
}
73+
// eslint-disable-next-line react-hooks/exhaustive-deps
74+
}, [handleSwitchTeams]);
75+
76+
React.useEffect(() => {
77+
startClear();
78+
setHome(true);
79+
// we intentionally do not reset project/plan here; selection will set them
80+
// eslint-disable-next-line react-hooks/exhaustive-deps
81+
}, []);
82+
6383
const isPersonal = teamId === personalTeam;
6484
const projects = React.useMemo(
6585
() => (isPersonal ? personalProjects : teamId ? teamProjects(teamId) : []),
@@ -160,18 +180,6 @@ export const ProjectsScreenInner: React.FC = () => {
160180
setAddOpen(false);
161181
};
162182

163-
const handleSwitchTeams = () => {
164-
localStorage.removeItem(LocalKey.plan);
165-
navigate('/switch-teams');
166-
};
167-
168-
React.useEffect(() => {
169-
startClear();
170-
setHome(true);
171-
// we intentionally do not reset project/plan here; selection will set them
172-
// eslint-disable-next-line react-hooks/exhaustive-deps
173-
}, []);
174-
175183
// Navigate to plan page only after user explicitly leaves home (card click triggers leaveHome)
176184
React.useEffect(() => {
177185
if (!plan) return; // no selection yet
@@ -191,6 +199,11 @@ export const ProjectsScreenInner: React.FC = () => {
191199
return thisTeam && isAdmin(thisTeam);
192200
}, [thisTeam, isAdmin]);
193201

202+
// Early return when teamId is missing to prevent errors in derived values
203+
if (!teamId) {
204+
return null; // or a loading state
205+
}
206+
194207
return (
195208
<Box sx={{ width: '100%' }}>
196209
<AppHead />

0 commit comments

Comments
 (0)