Skip to content

Commit 5896e3b

Browse files
abhinavkrinabhinavkrin
authored
fix: router failing to match channel route when 'First Channel After Login' starts with a hash (#37656)
1 parent 99dcf8c commit 5896e3b

File tree

5 files changed

+287
-8
lines changed

5 files changed

+287
-8
lines changed

‎.changeset/sixty-bikes-know.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@rocket.chat/meteor': patch
3+
---
4+
5+
Fixes an issue where the client failed to load properly when the “First Channel After Login” setting began with a hash (#), ensuring users are routed to the correct channel.

‎apps/meteor/client/views/root/MainLayout/LayoutWithSidebar.spec.tsx‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { mockAppRoot } from '@rocket.chat/mock-providers';
2+
import { useCurrentRoutePath, useRoute } from '@rocket.chat/ui-contexts';
3+
import { render } from '@testing-library/react';
4+
5+
import LayoutWithSidebar from './LayoutWithSidebar';
6+
7+
jest.mock('@rocket.chat/ui-contexts', () => ({
8+
...jest.requireActual('@rocket.chat/ui-contexts'),
9+
useCurrentRoutePath: jest.fn(),
10+
useRoute: jest.fn(),
11+
}));
12+
13+
jest.mock('../../../sidebar', () => () => <div>Sidebar</div>);
14+
15+
const mockedUseCurrentRoutePath = useCurrentRoutePath as jest.MockedFunction<typeof useCurrentRoutePath>;
16+
const mockedUseRoute = useRoute as jest.MockedFunction<typeof useRoute>;
17+
18+
describe('LayoutWithSidebar - First_Channel_After_Login navigation', () => {
19+
beforeEach(() => {
20+
jest.clearAllMocks();
21+
});
22+
23+
const setupChannelRouteMock = () => {
24+
const push = jest.fn();
25+
mockedUseRoute.mockImplementation((routeName) => {
26+
if (routeName === 'channel') {
27+
return { push } as any;
28+
}
29+
return {} as any;
30+
});
31+
return push;
32+
};
33+
34+
it('redirects to First_Channel_After_Login on "/"', () => {
35+
const push = setupChannelRouteMock();
36+
mockedUseCurrentRoutePath.mockReturnValue('/');
37+
38+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
39+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
40+
});
41+
42+
expect(push).toHaveBeenCalledWith({ name: 'general' });
43+
});
44+
45+
it('strips leading "#" from First_Channel_After_Login before redirecting', () => {
46+
const push = setupChannelRouteMock();
47+
mockedUseCurrentRoutePath.mockReturnValue('/');
48+
49+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
50+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '#general').build(),
51+
});
52+
53+
expect(push).toHaveBeenCalledWith({ name: 'general' });
54+
});
55+
56+
it('does NOT redirect if First_Channel_After_Login starts with "?"', () => {
57+
const push = setupChannelRouteMock();
58+
mockedUseCurrentRoutePath.mockReturnValue('/');
59+
60+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
61+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '?general').build(),
62+
});
63+
64+
expect(push).not.toHaveBeenCalled();
65+
});
66+
67+
it('does NOT redirect if First_Channel_After_Login starts with "##"', () => {
68+
const push = setupChannelRouteMock();
69+
mockedUseCurrentRoutePath.mockReturnValue('/');
70+
71+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
72+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '##general').build(),
73+
});
74+
75+
expect(push).not.toHaveBeenCalled();
76+
});
77+
78+
it('redirects when route is "/home"', () => {
79+
const push = setupChannelRouteMock();
80+
mockedUseCurrentRoutePath.mockReturnValue('/home');
81+
82+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
83+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
84+
});
85+
86+
expect(push).toHaveBeenCalled();
87+
});
88+
89+
it('does NOT redirect if First_Channel_After_Login is empty', () => {
90+
const push = setupChannelRouteMock();
91+
mockedUseCurrentRoutePath.mockReturnValue('/');
92+
93+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
94+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '').build(),
95+
});
96+
97+
expect(push).not.toHaveBeenCalled();
98+
});
99+
100+
it('does NOT redirect on non-home routes (e.g. /admin)', () => {
101+
const push = setupChannelRouteMock();
102+
mockedUseCurrentRoutePath.mockReturnValue('/admin' as any);
103+
104+
render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
105+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
106+
});
107+
108+
expect(push).not.toHaveBeenCalled();
109+
});
110+
111+
it('redirects only once even if component re-renders', () => {
112+
const push = setupChannelRouteMock();
113+
mockedUseCurrentRoutePath.mockReturnValue('/');
114+
115+
const { rerender } = render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
116+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
117+
});
118+
119+
rerender(<LayoutWithSidebar>content again</LayoutWithSidebar>);
120+
121+
expect(push).toHaveBeenCalledTimes(1);
122+
expect(push).toHaveBeenCalledWith({ name: 'general' });
123+
});
124+
});

‎apps/meteor/client/views/root/MainLayout/LayoutWithSidebar.tsx‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ import MainContent from './MainContent';
88
import { MainLayoutStyleTags } from './MainLayoutStyleTags';
99
import Sidebar from '../../../sidebar';
1010

11+
const INVALID_ROOM_NAME_PREFIXES = ['#', '?'] as const;
12+
1113
const LayoutWithSidebar = ({ children }: { children: ReactNode }): ReactElement => {
1214
const { isEmbedded: embeddedLayout } = useLayout();
1315

1416
const currentRoutePath = useCurrentRoutePath();
1517
const channelRoute = useRoute('channel');
1618
const removeSidenav = embeddedLayout && !currentRoutePath?.startsWith('/admin');
1719

18-
const firstChannelAfterLogin = useSetting('First_Channel_After_Login');
20+
const firstChannelAfterLogin = useSetting<string>('First_Channel_After_Login', '');
21+
const roomName = (firstChannelAfterLogin.startsWith('#') ? firstChannelAfterLogin.slice(1) : firstChannelAfterLogin).trim();
1922

2023
const redirected = useRef(false);
2124

@@ -26,17 +29,23 @@ const LayoutWithSidebar = ({ children }: { children: ReactNode }): ReactElement
2629
return;
2730
}
2831

29-
if (!firstChannelAfterLogin || typeof firstChannelAfterLogin !== 'string') {
32+
if (!roomName) {
33+
return;
34+
}
35+
36+
if (INVALID_ROOM_NAME_PREFIXES.some((prefix) => roomName.startsWith(prefix))) {
37+
// Because this will break url routing. Eg: /channel/#roomName and /channel/?roomName which will route to path /channel
3038
return;
3139
}
3240

3341
if (redirected.current) {
3442
return;
3543
}
44+
3645
redirected.current = true;
3746

38-
channelRoute.push({ name: firstChannelAfterLogin });
39-
}, [channelRoute, currentRoutePath, firstChannelAfterLogin]);
47+
channelRoute.push({ name: roomName });
48+
}, [channelRoute, currentRoutePath, roomName]);
4049

4150
return (
4251
<Box

‎apps/meteor/client/views/root/MainLayout/LayoutWithSidebarV2.spec.tsx‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import { mockAppRoot } from '@rocket.chat/mock-providers';
2+
import { useCurrentRoutePath, useRouter } from '@rocket.chat/ui-contexts';
3+
import { render } from '@testing-library/react';
4+
import React from 'react';
5+
6+
import LayoutWithSidebarV2 from './LayoutWithSidebarV2';
7+
8+
jest.mock('@rocket.chat/ui-contexts', () => ({
9+
...jest.requireActual('@rocket.chat/ui-contexts'),
10+
useCurrentRoutePath: jest.fn(),
11+
useRouter: jest.fn(),
12+
}));
13+
14+
jest.mock('../../../NavBarV2', () => () => <div>NavBarV2</div>);
15+
jest.mock('../../../sidebarv2', () => () => <div>SidebarV2</div>);
16+
jest.mock('../../navigation', () => () => <div>NavigationRegion</div>);
17+
jest.mock('./AccessibilityShortcut', () => () => <div>AccessibilityShortcut</div>);
18+
jest.mock('../../navigation/providers/RoomsNavigationProvider', () => ({
19+
__esModule: true,
20+
default: ({ children }: { children: React.ReactNode }) => <>{children}</>,
21+
}));
22+
23+
jest.mock('@rocket.chat/ui-client', () => ({
24+
FeaturePreview: ({ children }: { children: React.ReactNode }) => <>{children}</>,
25+
FeaturePreviewOn: ({ children }: { children: React.ReactNode }) => <>{children}</>,
26+
FeaturePreviewOff: ({ children }: { children: React.ReactNode }) => <>{children}</>,
27+
}));
28+
29+
const mockedUseCurrentRoutePath = useCurrentRoutePath as jest.MockedFunction<typeof useCurrentRoutePath>;
30+
const mockedUseRouter = useRouter as jest.MockedFunction<typeof useRouter>;
31+
32+
describe('LayoutWithSidebarV2 - First_Channel_After_Login navigation', () => {
33+
beforeEach(() => {
34+
jest.clearAllMocks();
35+
});
36+
37+
const setupRouterMock = () => {
38+
const navigate = jest.fn();
39+
mockedUseRouter.mockReturnValue({ navigate } as any);
40+
return navigate;
41+
};
42+
43+
it('redirects to First_Channel_After_Login on "/"', () => {
44+
const navigate = setupRouterMock();
45+
mockedUseCurrentRoutePath.mockReturnValue('/');
46+
47+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
48+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
49+
});
50+
51+
expect(navigate).toHaveBeenCalledWith({ name: '/channel/general' });
52+
});
53+
54+
it('strips leading "#" from First_Channel_After_Login before redirecting', () => {
55+
const navigate = setupRouterMock();
56+
mockedUseCurrentRoutePath.mockReturnValue('/');
57+
58+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
59+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '#general').build(),
60+
});
61+
62+
expect(navigate).toHaveBeenCalledWith({ name: '/channel/general' });
63+
});
64+
65+
it('does NOT redirect if First_Channel_After_Login starts with "?"', () => {
66+
const navigate = setupRouterMock();
67+
mockedUseCurrentRoutePath.mockReturnValue('/');
68+
69+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
70+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '?general').build(),
71+
});
72+
73+
expect(navigate).not.toHaveBeenCalled();
74+
});
75+
76+
it('does NOT redirect if First_Channel_After_Login starts with "##"', () => {
77+
const navigate = setupRouterMock();
78+
mockedUseCurrentRoutePath.mockReturnValue('/');
79+
80+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
81+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '##general').build(),
82+
});
83+
84+
expect(navigate).not.toHaveBeenCalled();
85+
});
86+
87+
it('redirects when route is "/home"', () => {
88+
const navigate = setupRouterMock();
89+
mockedUseCurrentRoutePath.mockReturnValue('/home');
90+
91+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
92+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
93+
});
94+
95+
expect(navigate).toHaveBeenCalled();
96+
});
97+
98+
it('does NOT redirect if First_Channel_After_Login is empty', () => {
99+
const navigate = setupRouterMock();
100+
mockedUseCurrentRoutePath.mockReturnValue('/');
101+
102+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
103+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '').build(),
104+
});
105+
106+
expect(navigate).not.toHaveBeenCalled();
107+
});
108+
109+
it('does NOT redirect on non-home routes (e.g. /admin)', () => {
110+
const navigate = setupRouterMock();
111+
mockedUseCurrentRoutePath.mockReturnValue('/admin' as any);
112+
113+
render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
114+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
115+
});
116+
117+
expect(navigate).not.toHaveBeenCalled();
118+
});
119+
120+
it('redirects only once even if component re-renders', () => {
121+
const navigate = setupRouterMock();
122+
mockedUseCurrentRoutePath.mockReturnValue('/');
123+
124+
const { rerender } = render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
125+
wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
126+
});
127+
128+
rerender(<LayoutWithSidebarV2>content again</LayoutWithSidebarV2>);
129+
130+
expect(navigate).toHaveBeenCalledTimes(1);
131+
expect(navigate).toHaveBeenCalledWith({ name: '/channel/general' });
132+
});
133+
});

‎apps/meteor/client/views/root/MainLayout/LayoutWithSidebarV2.tsx‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,17 @@ import Sidebar from '../../../sidebarv2';
1313
import NavigationRegion from '../../navigation';
1414
import RoomsNavigationProvider from '../../navigation/providers/RoomsNavigationProvider';
1515

16+
const INVALID_ROOM_NAME_PREFIXES = ['#', '?'] as const;
17+
1618
const LayoutWithSidebarV2 = ({ children }: { children: ReactNode }): ReactElement => {
1719
const { isEmbedded: embeddedLayout } = useLayout();
1820

1921
const currentRoutePath = useCurrentRoutePath();
2022
const router = useRouter();
2123
const removeSidenav = embeddedLayout && !currentRoutePath?.startsWith('/admin');
2224

23-
const firstChannelAfterLogin = useSetting('First_Channel_After_Login');
25+
const firstChannelAfterLogin = useSetting<string>('First_Channel_After_Login', '');
26+
const roomName = (firstChannelAfterLogin.startsWith('#') ? firstChannelAfterLogin.slice(1) : firstChannelAfterLogin).trim();
2427

2528
const redirected = useRef(false);
2629

@@ -31,7 +34,12 @@ const LayoutWithSidebarV2 = ({ children }: { children: ReactNode }): ReactElemen
3134
return;
3235
}
3336

34-
if (!firstChannelAfterLogin || typeof firstChannelAfterLogin !== 'string') {
37+
if (!roomName) {
38+
return;
39+
}
40+
41+
if (INVALID_ROOM_NAME_PREFIXES.some((prefix) => roomName.startsWith(prefix))) {
42+
// Because this will break url routing. Eg: /channel/#roomName and /channel/?roomName which will route to path /channel
3543
return;
3644
}
3745

@@ -40,8 +48,8 @@ const LayoutWithSidebarV2 = ({ children }: { children: ReactNode }): ReactElemen
4048
}
4149
redirected.current = true;
4250

43-
router.navigate({ name: `/channel/${firstChannelAfterLogin}` as keyof IRouterPaths });
44-
}, [router, currentRoutePath, firstChannelAfterLogin]);
51+
router.navigate({ name: `/channel/${roomName}` as keyof IRouterPaths });
52+
}, [router, currentRoutePath, roomName]);
4553

4654
return (
4755
<>

0 commit comments

Comments
 (0)