chore: extract subscriptions.read tmid migration into separate PR

The thread read-marker migration (subscriptions.read tmid endpoint +
ThreadChat/useThreadMessagesQuery callers) moves to its own PR #40957.

Author: ggazzo

.changeset/ddp-migrate-batch2-callers.md

@@ -2,12 +2,11 @@
 '@rocket.chat/meteor': patch
 ---
 
-Migrate seven client DDP callers to their REST equivalents (the DDP methods stay registered on the server for external SDK/mobile clients, with a deprecation log pointing at the REST route until 9.0.0 removes them):
+Migrate six client DDP callers to their REST equivalents (the DDP methods stay registered on the server for external SDK/mobile clients, with a deprecation log pointing at the REST route until 9.0.0 removes them):
 
 - `loadMissedMessages` → `GET /v1/chat.syncMessages`
 - `joinRoom` → `POST /v1/channels.join` (channel-only; non-`c` rooms now error via REST the same way they used to via DDP)
 - `userSetUtcOffset` → `POST /v1/users.setPreferences` (new `utcOffset` field)
 - `deleteFileMessage` → `POST /v1/chat.delete` (new `fileId` body shape)
-- `readThreads` → `POST /v1/subscriptions.read` (new `tmid` field)
 - `spotlight` → `GET /v1/spotlight` (new `usernames` / `type` / `rid` query params)
 - `listCustomSounds` → `GET /v1/custom-sounds.list`

.changeset/rest-subscriptions-read-tmid.md

@@ -1,5 +1,5 @@
 import type { ISubscription } from '@rocket.chat/core-typings';
-import { Messages, Rooms, Subscriptions } from '@rocket.chat/models';
+import { Rooms, Subscriptions } from '@rocket.chat/models';
 import {
 	ajv,
 	isSubscriptionsGetProps,
@@ -14,7 +14,6 @@ import { Meteor } from 'meteor/meteor';
 import { readMessages } from '../../../../server/lib/readMessages';
 import { getSubscriptions } from '../../../../server/publications/subscription';
 import { unreadMessages } from '../../../message-mark-as-unread/server/unreadMessages';
-import { readThreadMethod } from '../../../threads/server/functions';
 import { API } from '../api';
 
 const subscriptionsGetResponseSchema = ajv.compile<{
@@ -140,23 +139,14 @@ API.v1.post(
 		},
 	},
 	async function action() {
-		const { readThreads = false, tmid } = this.bodyParams;
+		const { readThreads = false } = this.bodyParams;
 		const roomId = 'rid' in this.bodyParams ? this.bodyParams.rid : this.bodyParams.roomId;
 
 		const room = await Rooms.findOneById(roomId);
 		if (!room) {
 			throw new Error('error-invalid-subscription');
 		}
 
-		if (tmid) {
-			const thread = await Messages.findOneById(tmid);
-			if (thread?.rid !== room._id) {
-				throw new Error('error-invalid-thread');
-			}
-			await readThreadMethod({ user: this.user, tmid });
-			return API.v1.success();
-		}
-
 		await readMessages(room, this.userId, readThreads);
 
 		return API.v1.success();

apps/meteor/app/api/server/v1/subscriptions.ts

@@ -1,16 +1,13 @@
 import type { IMessage, IRoom, IUser } from '@rocket.chat/core-typings';
 import { isEditedMessage } from '@rocket.chat/core-typings';
-import { Messages, Rooms, Subscriptions, NotificationQueue } from '@rocket.chat/models';
-import { Meteor } from 'meteor/meteor';
+import { Messages, Subscriptions, NotificationQueue } from '@rocket.chat/models';
 
 import { callbacks } from '../../../server/lib/callbacks';
-import { canAccessRoomAsync } from '../../authorization/server';
 import {
 	notifyOnSubscriptionChangedByRoomIdAndUserIds,
 	notifyOnSubscriptionChangedByRoomIdAndUserId,
 } from '../../lib/server/lib/notifyListener';
 import { getMentions, getUserIdsFromHighlights } from '../../lib/server/lib/notifyUsersOnMessage';
-import { settings } from '../../settings/server';
 
 export async function reply({ tmid }: { tmid?: string }, message: IMessage, parentMessage: IMessage, followers: string[]) {
 	if (!tmid || isEditedMessage(message)) {
@@ -101,32 +98,3 @@ export const readThread = async ({ user, room, tmid }: { user: IUser; room: IRoo
 
 	callbacks.runAsync('afterReadMessages', room, { uid: user._id, tmid });
 };
-
-/**
- * Marks a thread as read for a user, replicating the full behavior of the
- * legacy `readThreads` DDP method: it validates that threads are enabled, that
- * the thread and its room exist, that the user can access the room, and runs
- * the `beforeReadMessages` callback before clearing the thread unread state.
- */
-export const readThreadMethod = async ({ user, tmid }: { user: IUser; tmid: IMessage['_id'] }) => {
-	if (!settings.get('Threads_enabled')) {
-		throw new Meteor.Error('error-not-allowed', 'Threads Disabled', { method: 'readThreads' });
-	}
-
-	const thread = await Messages.findOneById(tmid);
-	if (!thread) {
-		return;
-	}
-
-	const room = await Rooms.findOneById(thread.rid);
-	if (!room) {
-		throw new Meteor.Error('error-room-does-not-exist', 'This room does not exist', { method: 'readThreads' });
-	}
-
-	if (!(await canAccessRoomAsync(room, user))) {
-		throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'readThreads' });
-	}
-
-	await callbacks.run('beforeReadMessages', thread.rid, user._id);
-	await readThread({ user, room, tmid });
-};

apps/meteor/app/threads/server/functions.ts

@@ -2,7 +2,7 @@ import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
 import { isEditedMessage } from '@rocket.chat/core-typings';
 import { Box, CheckBox, Field, FieldLabel, FieldRow } from '@rocket.chat/fuselage';
 import { clientCallbacks, ContextualbarContent } from '@rocket.chat/ui-client';
-import { useEndpoint, useTranslation, useUserPreference, useRoomToolbox } from '@rocket.chat/ui-contexts';
+import { useMethod, useTranslation, useUserPreference, useRoomToolbox } from '@rocket.chat/ui-contexts';
 import { useState, useEffect, useCallback, useId } from 'react';
 
 import ThreadMessageList from './ThreadMessageList';
@@ -62,7 +62,7 @@ const ThreadChat = ({ mainMessage }: ThreadChatProps) => {
 	}, [chat?.messageEditing]);
 
 	const room = useRoom();
-	const markThreadRead = useEndpoint('POST', '/v1/subscriptions.read');
+	const readThreads = useMethod('readThreads');
 	useEffect(() => {
 		clientCallbacks.add(
 			'streamNewMessage',
@@ -71,7 +71,7 @@ const ThreadChat = ({ mainMessage }: ThreadChatProps) => {
 					return;
 				}
 
-				void markThreadRead({ rid: room._id, tmid: mainMessage._id });
+				readThreads(mainMessage._id);
 			},
 			clientCallbacks.priority.MEDIUM,
 			`thread-${room._id}`,
@@ -80,7 +80,7 @@ const ThreadChat = ({ mainMessage }: ThreadChatProps) => {
 		return () => {
 			clientCallbacks.remove('streamNewMessage', `thread-${room._id}`);
 		};
-	}, [mainMessage._id, markThreadRead, room._id]);
+	}, [mainMessage._id, readThreads, room._id]);
 
 	const subscription = useRoomSubscription();
 	const sendToChannelID = useId();

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

@@ -1,5 +1,5 @@
 import { isThreadMessage, type IMessage, type IRoom, type IThreadMainMessage, type IThreadMessage } from '@rocket.chat/core-typings';
-import { useEndpoint, useStream } from '@rocket.chat/ui-contexts';
+import { useEndpoint, useMethod, useStream } from '@rocket.chat/ui-contexts';
 import { useQuery, useQueryClient } from '@tanstack/react-query';
 import { useEffect, useRef } from 'react';
 
@@ -26,7 +26,10 @@ export const useThreadMessagesQuery = (tmid: IThreadMainMessage['_id'], rid?: IR
 	const queryClient = useQueryClient();
 	const queryKey = roomsQueryKeys.threadMessages(roomId, tmid);
 	const getThreadMessages = useEndpoint('GET', '/v1/chat.getThreadMessages');
-	const markThreadRead = useEndpoint('POST', '/v1/subscriptions.read');
+	// REST has no per-thread read-marker endpoint yet; fall back to the
+	// `readThreads` DDP method so the side effect that DDP getThreadMessages
+	// used to do server-side keeps happening for callers.
+	const readThreads = useMethod('readThreads');
 
 	const subscribeToRoomMessages = useStream('room-messages');
 	const subscribeToNotifyRoom = useStream('notify-room');
@@ -108,7 +111,7 @@ export const useThreadMessagesQuery = (tmid: IThreadMainMessage['_id'], rid?: IR
 			const cachedMessages = queryClient.getQueryData<IThreadMessage[]>(queryKey) || [];
 
 			const { messages } = await getThreadMessages({ tmid });
-			void markThreadRead({ rid: roomId, tmid }).catch(() => undefined);
+			void Promise.resolve(readThreads(tmid)).catch(() => undefined);
 			const filtered = messages
 				.map((m) => mapMessageFromApi(m))
 				.filter((msg): msg is IThreadMessage => isThreadMessage(msg) && msg.tmid === tmid && msg._id !== tmid && msg._hidden !== true);

apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts

@@ -1,10 +1,13 @@
 import type { IMessage, IUser } from '@rocket.chat/core-typings';
 import type { ServerMethods } from '@rocket.chat/ddp-client';
+import { Messages, Rooms } from '@rocket.chat/models';
 import { check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
 
-import { methodDeprecationLogger } from '../../app/lib/server/lib/deprecationWarningLogger';
-import { readThreadMethod } from '../../app/threads/server/functions';
+import { canAccessRoomAsync } from '../../app/authorization/server';
+import { settings } from '../../app/settings/server';
+import { readThread } from '../../app/threads/server/functions';
+import { callbacks } from '../lib/callbacks';
 
 declare module '@rocket.chat/ddp-client' {
 	// eslint-disable-next-line @typescript-eslint/naming-convention
@@ -15,14 +18,33 @@ declare module '@rocket.chat/ddp-client' {
 
 Meteor.methods<ServerMethods>({
 	async readThreads(tmid) {
-		methodDeprecationLogger.method('readThreads', '9.0.0', '/v1/subscriptions.read');
 		check(tmid, String);
 
-		const user = (await Meteor.userAsync()) as IUser | null;
-		if (!user) {
-			throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'readThreads' });
+		if (!Meteor.userId() || !settings.get('Threads_enabled')) {
+			throw new Meteor.Error('error-not-allowed', 'Threads Disabled', {
+				method: 'getThreadMessages',
+			});
 		}
 
-		await readThreadMethod({ user, tmid });
+		const thread = await Messages.findOneById(tmid);
+		if (!thread) {
+			return;
+		}
+
+		const user = (await Meteor.userAsync()) ?? undefined;
+
+		const room = await Rooms.findOneById(thread.rid);
+		if (!room) {
+			throw new Meteor.Error('error-room-does-not-exist', 'This room does not exist', { method: 'getThreadMessages' });
+		}
+
+		if (!(await canAccessRoomAsync(room, user))) {
+			throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'getThreadMessages' });
+		}
+
+		await callbacks.run('beforeReadMessages', thread.rid, user?._id);
+		if (user?._id) {
+			await readThread({ user: user as IUser, room, tmid });
+		}
 	},
 });

apps/meteor/server/methods/readThreads.ts

@@ -394,60 +394,6 @@ describe('[Subscriptions]', () => {
 						expect(res.body.subscription.tunread).to.deep.equal([threadId]);
 					});
 			});
-
-			it('should mark a single thread as read when a tmid is provided', async () => {
-				await request
-					.post(api('chat.sendMessage'))
-					.set(threadUserCredentials)
-					.send({
-						message: {
-							rid: testChannel._id,
-							msg: `@${adminUsername} making admin follow this thread`,
-							tmid: threadId,
-						},
-					});
-
-				await request
-					.post(api('subscriptions.read'))
-					.set(credentials)
-					.send({
-						rid: testChannel._id,
-						tmid: threadId,
-					})
-					.expect(200)
-					.expect((res) => {
-						expect(res.body).to.have.property('success', true);
-					});
-
-				await request
-					.get(api('subscriptions.getOne'))
-					.set(credentials)
-					.query({
-						roomId: testChannel._id,
-					})
-					.expect(200)
-					.expect((res) => {
-						expect(res.body).to.have.property('success', true);
-						// removeUnreadThreadByRoomIdAndUserId only $pulls the tmid, leaving an empty array
-						expect(res.body.subscription).to.have.property('tunread').that.is.an('array').and.does.not.include(threadId);
-					});
-			});
-
-			it('should fail when the tmid does not belong to the provided room', (done) => {
-				void request
-					.post(api('subscriptions.read'))
-					.set(credentials)
-					.send({
-						rid: testGroup._id,
-						tmid: threadId,
-					})
-					.expect(400)
-					.expect((res) => {
-						expect(res.body).to.have.property('success', false);
-						expect(res.body).to.have.property('error');
-					})
-					.end(done);
-			});
 		});
 	});
 

apps/meteor/tests/end-to-end/api/subscriptions.ts

@@ -6,9 +6,7 @@ type SubscriptionsGet = { updatedSince?: string };
 
 type SubscriptionsGetOne = { roomId: IRoom['_id'] };
 
-type SubscriptionsRead =
-	| { rid: IRoom['_id']; readThreads?: boolean; tmid?: IMessage['_id'] }
-	| { roomId: IRoom['_id']; readThreads?: boolean; tmid?: IMessage['_id'] };
+type SubscriptionsRead = { rid: IRoom['_id']; readThreads?: boolean } | { roomId: IRoom['_id']; readThreads?: boolean };
 
 type SubscriptionsUnread = { roomId: IRoom['_id'] } | { firstUnreadMessage: Pick<IMessage, '_id'> };
 
@@ -51,10 +49,6 @@ const SubscriptionsReadSchema = {
 					type: 'boolean',
 					nullable: true,
 				},
-				tmid: {
-					type: 'string',
-					nullable: true,
-				},
 			},
 			required: ['rid'],
 			additionalProperties: false,
@@ -69,10 +63,6 @@ const SubscriptionsReadSchema = {
 					type: 'boolean',
 					nullable: true,
 				},
-				tmid: {
-					type: 'string',
-					nullable: true,
-				},
 			},
 			required: ['roomId'],
 			additionalProperties: false,