regression: unfollowed thread message marked as unread

apps/meteor/client/lib/chats/readStateManager.ts

@@ -72,7 +72,8 @@ export class ReadStateManager extends Emitter {
 			(record) =>
 				record.rid === this.subscription?.rid &&
 				record.ts.getTime() > (this.subscription.ls?.getTime() ?? 0) &&
-				record.u._id !== getUserId(),
+				record.u._id !== getUserId() &&
+				(!record.tmid || record.tshow === true),
 			(a, b) => a.ts.getTime() - b.ts.getTime(),
 		);
 

apps/meteor/client/lib/utils/setMessageJumpQueryStringParameter.ts

@@ -3,14 +3,14 @@ import type { LocationPathname } from '@rocket.chat/ui-contexts';
 
 import { router } from '../../providers/RouterProvider';
 
-export const setMessageJumpQueryStringParameter = async (msg: IMessage['_id'] | null) => {
-	const { msg: _, ...search } = router.getSearchParameters();
+export const setMessageJumpQueryStringParameter = async (msg: IMessage['_id'] | null, context?: 'jumpToUnread') => {
+	const { msg: _msg, jumpContext: _jumpContext, ...search } = router.getSearchParameters();
 	const locationPathname = new URL(window.location.href).pathname as LocationPathname;
 
 	router.navigate(
 		{
 			pathname: locationPathname,
-			search: msg ? { ...search, msg } : search,
+			search: msg ? { ...search, msg, ...(context && { jumpContext: context }) } : search,
 		},
 		{ replace: true },
 	);

apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts

@@ -42,7 +42,8 @@ const useTryToJumpToMessage = ({ rid, virtualizerRef, setIsJumpingToMessage, mes
 			return;
 		}
 		// Thread deep links are handled by useTryToJumpToThreadMessage; do not use the main list virtualizer
-		if (message && isThreadMessage(message) && !isThreadMainMessage(message)) {
+		// If tshow is true, there is a preview on the main list, in this case we scroll to it
+		if (message && isThreadMessage(message) && !isThreadMainMessage(message) && message.tshow !== true) {
 			setIsJumpingToMessage(false);
 			return;
 		}

apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToThreadMessage.spec.ts

@@ -0,0 +1,132 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { renderHook, waitFor } from '@testing-library/react';
+
+import useTryToJumpToThreadMessage from './useTryToJumpToThreadMessage';
+import { RoomHistoryManager } from '../../../../../app/ui-utils/client';
+
+jest.mock('../../../../../app/ui-utils/client', () => ({
+	RoomHistoryManager: {
+		getSurroundingMessages: jest.fn().mockResolvedValue(undefined),
+		isLoaded: jest.fn().mockReturnValue(false),
+		getMore: jest.fn().mockResolvedValue(undefined),
+	},
+}));
+
+jest.mock('../../../../lib/RoomManager', () => ({
+	RoomManager: {
+		opened: undefined as string | undefined,
+	},
+}));
+
+jest.mock('../../../../lib/rooms/roomCoordinator', () => ({
+	roomCoordinator: {
+		openRouteLink: jest.fn(),
+	},
+}));
+
+jest.mock('../../../../providers/RouterProvider', () => ({
+	router: {
+		getSearchParameters: jest.fn().mockReturnValue({}),
+		getRouteParameters: jest.fn().mockReturnValue({}),
+	},
+}));
+
+jest.mock('../../../../stores', () => ({
+	Subscriptions: {
+		state: {
+			find: jest.fn().mockReturnValue(undefined),
+		},
+	},
+}));
+
+const mockedRoomHistoryManager = jest.mocked(RoomHistoryManager);
+
+afterEach(() => {
+	jest.clearAllMocks();
+});
+
+describe('useTryToJumpToThreadMessage', () => {
+	describe('early return when msg param is absent or jumpContext is jumpToUnread', () => {
+		it('should not navigate or load messages when msg search parameter is absent', () => {
+			const endpointSpy = jest.fn();
+
+			renderHook(() => useTryToJumpToThreadMessage(), {
+				wrapper: mockAppRoot().withEndpoint('GET', '/v1/chat.getMessage', endpointSpy).build(),
+			});
+
+			expect(endpointSpy).not.toHaveBeenCalled();
+			expect(mockedRoomHistoryManager.getSurroundingMessages).not.toHaveBeenCalled();
+			expect(mockedRoomHistoryManager.getMore).not.toHaveBeenCalled();
+		});
+
+		it('should not navigate or load messages when jumpContext is jumpToUnread', async () => {
+			const threadMessage = {
+				_id: 'msg-1',
+				rid: 'room-1',
+				tmid: 'parent-msg-1',
+				ts: new Date('2024-01-01T00:00:00Z').toISOString(),
+				u: { _id: 'user-1', username: 'john' },
+				msg: 'Thread reply',
+				_updatedAt: new Date('2024-01-01T00:00:00Z').toISOString(),
+			};
+
+			const endpointSpy = jest.fn().mockResolvedValue({ message: threadMessage });
+
+			renderHook(() => useTryToJumpToThreadMessage(), {
+				wrapper: mockAppRoot()
+					.withRouter({ getSearchParameters: () => ({ msg: 'msg-1', jumpContext: 'jumpToUnread' }) })
+					.withEndpoint('GET', '/v1/chat.getMessage', endpointSpy)
+					.withMethod('getRoomById', () => ({ _id: 'room-1', t: 'c', name: 'general' }) as any)
+					.build(),
+			});
+
+			await waitFor(() => {
+				expect(endpointSpy).toHaveBeenCalledWith({ msgId: 'msg-1' });
+			});
+
+			await waitFor(async () => {
+				await new Promise((resolve) => setTimeout(resolve, 300));
+				expect(mockedRoomHistoryManager.getMore).not.toHaveBeenCalled();
+			});
+
+			expect(mockedRoomHistoryManager.getSurroundingMessages).not.toHaveBeenCalled();
+			expect(mockedRoomHistoryManager.getMore).not.toHaveBeenCalled();
+		});
+	});
+
+	describe('when jumpContext is absent', () => {
+		it('should load more messages when jumpContext is null', async () => {
+			const threadMessage = {
+				_id: 'msg-1',
+				rid: 'room-1',
+				tmid: 'parent-msg-1',
+				ts: new Date('2024-01-01T00:00:00Z').toISOString(),
+				u: { _id: 'user-1', username: 'john' },
+				msg: 'Thread reply',
+				_updatedAt: new Date('2024-01-01T00:00:00Z').toISOString(),
+			};
+
+			mockedRoomHistoryManager.isLoaded.mockReturnValue(false);
+
+			const endpointSpy = jest.fn().mockResolvedValue({ message: threadMessage });
+
+			renderHook(() => useTryToJumpToThreadMessage(), {
+				wrapper: mockAppRoot()
+					.withRouter({ getSearchParameters: () => ({ msg: 'msg-1' }) })
+					.withEndpoint('GET', '/v1/chat.getMessage', endpointSpy)
+					.withMethod('getRoomById', () => ({ _id: 'room-1', t: 'c', name: 'general' }) as any)
+					.build(),
+			});
+
+			await waitFor(() => {
+				expect(endpointSpy).toHaveBeenCalledWith({ msgId: 'msg-1' });
+			});
+
+			await waitFor(() => {
+				expect(mockedRoomHistoryManager.getMore).toHaveBeenCalledWith('room-1');
+			});
+
+			expect(mockedRoomHistoryManager.getSurroundingMessages).not.toHaveBeenCalled();
+		});
+	});
+});

apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToThreadMessage.ts

@@ -11,6 +11,7 @@ import { useGoToRoom } from '../../hooks/useGoToRoom';
 
 const useTryToJumpToThreadMessage = (): void => {
 	const messageJumpParam = useSearchParameter('msg');
+	const messageJumpContext = useSearchParameter('jumpContext');
 	const goToRoom = useGoToRoom();
 	const tab = useRouteParameter('tab');
 	const context = useRouteParameter('context');
@@ -28,7 +29,7 @@ const useTryToJumpToThreadMessage = (): void => {
 	});
 
 	useEffect(() => {
-		if (!messageJumpParam) {
+		if (!messageJumpParam || messageJumpContext === 'jumpToUnread') {
 			return;
 		}
 
@@ -56,7 +57,7 @@ const useTryToJumpToThreadMessage = (): void => {
 				await RoomHistoryManager.getMore(message.rid);
 			}
 		})();
-	}, [messageJumpParam, message, goToRoom, tab, context]);
+	}, [messageJumpParam, message, goToRoom, tab, context, messageJumpContext]);
 };
 
 export default useTryToJumpToThreadMessage;

apps/meteor/client/views/room/body/hooks/useUnreadMessages.ts

@@ -64,14 +64,15 @@ export const useHandleUnread = (
 		let message = firstUnread;
 		if (!message) {
 			message = findFirstMessage(
-				(record) => record.rid === rid && record.ts.getTime() > (unread?.since.getTime() ?? -Infinity),
+				(record) =>
+					record.rid === rid && record.ts.getTime() > (unread?.since.getTime() ?? -Infinity) && (!record.tmid || record.tshow === true),
 				(a, b) => a.ts.getTime() - b.ts.getTime(),
 			);
 		}
 		if (!message) {
 			return;
 		}
-		setMessageJumpQueryStringParameter(message?._id);
+		setMessageJumpQueryStringParameter(message?._id, 'jumpToUnread');
 		chat.readStateManager.markAsRead();
 		setUnreadCount(0);
 	}, [room._id, setUnreadCount, findFirstMessage, unread?.since, chat.readStateManager]);

apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx

@@ -73,7 +73,8 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
 	const { messageListRef } = useMessageListNavigation();
 
 	const virtualizerRef = useRef<VirtualizerHandle | null>(null);
-	const isAtBottom = useRef(true);
+	const isAtBottom = useRef<boolean | null>(null);
+
 	const lastScrollSizeRef = useRef(0);
 
 	const items = loading ? [] : [mainMessage, ...messages];
@@ -104,7 +105,7 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
 			setShouldJumpToBottom(false);
 			return;
 		}
-		if (isAtBottom.current && lastScrollSizeRef.current !== handle?.scrollSize) {
+		if (isAtBottom.current === true && lastScrollSizeRef.current !== handle?.scrollSize) {
 			lastScrollSizeRef.current = handle?.scrollSize ?? 0;
 			setShouldJumpToBottom(true);
 		}
@@ -130,12 +131,29 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
 		setShouldJumpToBottom(false);
 		handle.scrollToIndex(threadMsgTargetIndex, { align: 'center' });
 		setHighlightMessage(msgJumpParam);
-		setMessageJumpQueryStringParameter(null);
 		setTimeout(() => {
 			clearHighlightMessage();
 		}, 2000);
 	}, [threadMsgTargetIndex, msgJumpParam, mainMessage._id, setShouldJumpToBottom]);
 
+	useEffect(() => {
+		if (!msgJumpParam) {
+			return;
+		}
+		const clearMsgJumpParam = () => {
+			if (messages.find((m) => m._id === msgJumpParam) && mainMessage._id !== msgJumpParam) {
+				setMessageJumpQueryStringParameter(null);
+			}
+		};
+		const timeoutId = setTimeout(() => {
+			clearMsgJumpParam();
+		}, 500);
+		return () => {
+			clearMsgJumpParam();
+			clearTimeout(timeoutId);
+		};
+	}, [msgJumpParam, messages, mainMessage._id]);
+
 	useEffect(() => {
 		const handlerId = `thread-scroll-${mainMessage._id}`;
 		clientCallbacks.add(