fix: router failing to match channel route when 'First Channel After Login' starts with a hash

.changeset/sixty-bikes-know.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+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

@@ -0,0 +1,124 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { useCurrentRoutePath, useRoute } from '@rocket.chat/ui-contexts';
+import { render } from '@testing-library/react';
+
+import LayoutWithSidebar from './LayoutWithSidebar';
+
+jest.mock('@rocket.chat/ui-contexts', () => ({
+	...jest.requireActual('@rocket.chat/ui-contexts'),
+	useCurrentRoutePath: jest.fn(),
+	useRoute: jest.fn(),
+}));
+
+jest.mock('../../../sidebar', () => () => <div>Sidebar</div>);
+
+const mockedUseCurrentRoutePath = useCurrentRoutePath as jest.MockedFunction<typeof useCurrentRoutePath>;
+const mockedUseRoute = useRoute as jest.MockedFunction<typeof useRoute>;
+
+describe('LayoutWithSidebar - First_Channel_After_Login navigation', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+	});
+
+	const setupChannelRouteMock = () => {
+		const push = jest.fn();
+		mockedUseRoute.mockImplementation((routeName) => {
+			if (routeName === 'channel') {
+				return { push } as any;
+			}
+			return {} as any;
+		});
+		return push;
+	};
+
+	it('redirects to First_Channel_After_Login on "/"', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		expect(push).toHaveBeenCalledWith({ name: 'general' });
+	});
+
+	it('strips leading "#" from First_Channel_After_Login before redirecting', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '#general').build(),
+		});
+
+		expect(push).toHaveBeenCalledWith({ name: 'general' });
+	});
+
+	it('does NOT redirect if First_Channel_After_Login starts with "?"', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '?general').build(),
+		});
+
+		expect(push).not.toHaveBeenCalled();
+	});
+
+	it('does NOT redirect if First_Channel_After_Login starts with "##"', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '##general').build(),
+		});
+
+		expect(push).not.toHaveBeenCalled();
+	});
+
+	it('redirects when route is "/home"', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/home');
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		expect(push).toHaveBeenCalled();
+	});
+
+	it('does NOT redirect if First_Channel_After_Login is empty', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '').build(),
+		});
+
+		expect(push).not.toHaveBeenCalled();
+	});
+
+	it('does NOT redirect on non-home routes (e.g. /admin)', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/admin' as any);
+
+		render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		expect(push).not.toHaveBeenCalled();
+	});
+
+	it('redirects only once even if component re-renders', () => {
+		const push = setupChannelRouteMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		const { rerender } = render(<LayoutWithSidebar>content</LayoutWithSidebar>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		rerender(<LayoutWithSidebar>content again</LayoutWithSidebar>);
+
+		expect(push).toHaveBeenCalledTimes(1);
+		expect(push).toHaveBeenCalledWith({ name: 'general' });
+	});
+});

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

@@ -8,14 +8,17 @@ import MainContent from './MainContent';
 import { MainLayoutStyleTags } from './MainLayoutStyleTags';
 import Sidebar from '../../../sidebar';
 
+const INVALID_ROOM_NAME_PREFIXES = ['#', '?'] as const;
+
 const LayoutWithSidebar = ({ children }: { children: ReactNode }): ReactElement => {
 	const { isEmbedded: embeddedLayout } = useLayout();
 
 	const currentRoutePath = useCurrentRoutePath();
 	const channelRoute = useRoute('channel');
 	const removeSidenav = embeddedLayout && !currentRoutePath?.startsWith('/admin');
 
-	const firstChannelAfterLogin = useSetting('First_Channel_After_Login');
+	const firstChannelAfterLogin = useSetting<string>('First_Channel_After_Login', '');
+	const roomName = (firstChannelAfterLogin.startsWith('#') ? firstChannelAfterLogin.slice(1) : firstChannelAfterLogin).trim();
 
 	const redirected = useRef(false);
 
@@ -26,17 +29,23 @@ const LayoutWithSidebar = ({ children }: { children: ReactNode }): ReactElement
 			return;
 		}
 
-		if (!firstChannelAfterLogin || typeof firstChannelAfterLogin !== 'string') {
+		if (!roomName) {
+			return;
+		}
+
+		if (INVALID_ROOM_NAME_PREFIXES.some((prefix) => roomName.startsWith(prefix))) {
+			// Because this will break url routing. Eg: /channel/#roomName and /channel/?roomName which will route to path /channel
 			return;
 		}
 
 		if (redirected.current) {
 			return;
 		}
+
 		redirected.current = true;
 
-		channelRoute.push({ name: firstChannelAfterLogin });
-	}, [channelRoute, currentRoutePath, firstChannelAfterLogin]);
+		channelRoute.push({ name: roomName });
+	}, [channelRoute, currentRoutePath, roomName]);
 
 	return (
 		<Box

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

@@ -0,0 +1,133 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { useCurrentRoutePath, useRouter } from '@rocket.chat/ui-contexts';
+import { render } from '@testing-library/react';
+import React from 'react';
+
+import LayoutWithSidebarV2 from './LayoutWithSidebarV2';
+
+jest.mock('@rocket.chat/ui-contexts', () => ({
+	...jest.requireActual('@rocket.chat/ui-contexts'),
+	useCurrentRoutePath: jest.fn(),
+	useRouter: jest.fn(),
+}));
+
+jest.mock('../../../NavBarV2', () => () => <div>NavBarV2</div>);
+jest.mock('../../../sidebarv2', () => () => <div>SidebarV2</div>);
+jest.mock('../../navigation', () => () => <div>NavigationRegion</div>);
+jest.mock('./AccessibilityShortcut', () => () => <div>AccessibilityShortcut</div>);
+jest.mock('../../navigation/providers/RoomsNavigationProvider', () => ({
+	__esModule: true,
+	default: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+}));
+
+jest.mock('@rocket.chat/ui-client', () => ({
+	FeaturePreview: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+	FeaturePreviewOn: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+	FeaturePreviewOff: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+}));
+
+const mockedUseCurrentRoutePath = useCurrentRoutePath as jest.MockedFunction<typeof useCurrentRoutePath>;
+const mockedUseRouter = useRouter as jest.MockedFunction<typeof useRouter>;
+
+describe('LayoutWithSidebarV2 - First_Channel_After_Login navigation', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+	});
+
+	const setupRouterMock = () => {
+		const navigate = jest.fn();
+		mockedUseRouter.mockReturnValue({ navigate } as any);
+		return navigate;
+	};
+
+	it('redirects to First_Channel_After_Login on "/"', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		expect(navigate).toHaveBeenCalledWith({ name: '/channel/general' });
+	});
+
+	it('strips leading "#" from First_Channel_After_Login before redirecting', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '#general').build(),
+		});
+
+		expect(navigate).toHaveBeenCalledWith({ name: '/channel/general' });
+	});
+
+	it('does NOT redirect if First_Channel_After_Login starts with "?"', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '?general').build(),
+		});
+
+		expect(navigate).not.toHaveBeenCalled();
+	});
+
+	it('does NOT redirect if First_Channel_After_Login starts with "##"', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '##general').build(),
+		});
+
+		expect(navigate).not.toHaveBeenCalled();
+	});
+
+	it('redirects when route is "/home"', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/home');
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		expect(navigate).toHaveBeenCalled();
+	});
+
+	it('does NOT redirect if First_Channel_After_Login is empty', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', '').build(),
+		});
+
+		expect(navigate).not.toHaveBeenCalled();
+	});
+
+	it('does NOT redirect on non-home routes (e.g. /admin)', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/admin' as any);
+
+		render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		expect(navigate).not.toHaveBeenCalled();
+	});
+
+	it('redirects only once even if component re-renders', () => {
+		const navigate = setupRouterMock();
+		mockedUseCurrentRoutePath.mockReturnValue('/');
+
+		const { rerender } = render(<LayoutWithSidebarV2>content</LayoutWithSidebarV2>, {
+			wrapper: mockAppRoot().withSetting('First_Channel_After_Login', 'general').build(),
+		});
+
+		rerender(<LayoutWithSidebarV2>content again</LayoutWithSidebarV2>);
+
+		expect(navigate).toHaveBeenCalledTimes(1);
+		expect(navigate).toHaveBeenCalledWith({ name: '/channel/general' });
+	});
+});

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

@@ -13,14 +13,17 @@ import Sidebar from '../../../sidebarv2';
 import NavigationRegion from '../../navigation';
 import RoomsNavigationProvider from '../../navigation/providers/RoomsNavigationProvider';
 
+const INVALID_ROOM_NAME_PREFIXES = ['#', '?'] as const;
+
 const LayoutWithSidebarV2 = ({ children }: { children: ReactNode }): ReactElement => {
 	const { isEmbedded: embeddedLayout } = useLayout();
 
 	const currentRoutePath = useCurrentRoutePath();
 	const router = useRouter();
 	const removeSidenav = embeddedLayout && !currentRoutePath?.startsWith('/admin');
 
-	const firstChannelAfterLogin = useSetting('First_Channel_After_Login');
+	const firstChannelAfterLogin = useSetting<string>('First_Channel_After_Login', '');
+	const roomName = (firstChannelAfterLogin.startsWith('#') ? firstChannelAfterLogin.slice(1) : firstChannelAfterLogin).trim();
 
 	const redirected = useRef(false);
 
@@ -31,7 +34,12 @@ const LayoutWithSidebarV2 = ({ children }: { children: ReactNode }): ReactElemen
 			return;
 		}
 
-		if (!firstChannelAfterLogin || typeof firstChannelAfterLogin !== 'string') {
+		if (!roomName) {
+			return;
+		}
+
+		if (INVALID_ROOM_NAME_PREFIXES.some((prefix) => roomName.startsWith(prefix))) {
+			// Because this will break url routing. Eg: /channel/#roomName and /channel/?roomName which will route to path /channel
 			return;
 		}
 
@@ -40,8 +48,8 @@ const LayoutWithSidebarV2 = ({ children }: { children: ReactNode }): ReactElemen
 		}
 		redirected.current = true;
 
-		router.navigate({ name: `/channel/${firstChannelAfterLogin}` as keyof IRouterPaths });
-	}, [router, currentRoutePath, firstChannelAfterLogin]);
+		router.navigate({ name: `/channel/${roomName}` as keyof IRouterPaths });
+	}, [router, currentRoutePath, roomName]);
 
 	return (
 		<>