[FIX] Marking room as read with unread threads still (#18410)

app/models/server/models/Subscriptions.js

@@ -1396,17 +1396,23 @@ export class Subscriptions extends Base {
 		}, { multi: true });
 	}
 
-	removeUnreadThreadByRoomIdAndUserId(rid, userId, tmid) {
-		return this.update({
-			'u._id': userId,
-			rid,
-		}, {
+	removeUnreadThreadByRoomIdAndUserId(rid, userId, tmid, clearAlert = false) {
+		const update = {
 			$pull: {
 				tunread: tmid,
 				tunreadGroup: tmid,
 				tunreadUser: tmid,
 			},
-		});
+		};
+
+		if (clearAlert) {
+			update.$set = { alert: false };
+		}
+
+		return this.update({
+			'u._id': userId,
+			rid,
+		}, update);
 	}
 
 	removeAllUnreadThreadsByRoomIdAndUserId(rid, userId) {

app/models/server/raw/Subscriptions.js

@@ -34,4 +34,24 @@ export class SubscriptionsRaw extends BaseRaw {
 
 		return this.findOne(query, { fields: { roles: 1 } });
 	}
+
+	setAsReadByRoomIdAndUserId(rid, uid, alert = false) {
+		const query = {
+			rid,
+			'u._id': uid,
+		};
+
+		const update = {
+			$set: {
+				open: true,
+				alert,
+				unread: 0,
+				userMentions: 0,
+				groupMentions: 0,
+				ls: new Date(),
+			},
+		};
+
+		return this.col.update(query, update);
+	}
 }

app/threads/server/functions.js

@@ -67,6 +67,14 @@ export const unfollow = ({ tmid, rid, uid }) => {
 	return Messages.removeThreadFollowerByThreadId(tmid, uid);
 };
 
-export const readThread = ({ userId, rid, tmid }) => Subscriptions.removeUnreadThreadByRoomIdAndUserId(rid, userId, tmid);
+export const readThread = ({ userId, rid, tmid }) => {
+	const fields = { tunread: 1 };
+	const sub = Subscriptions.findOneByRoomIdAndUserId(rid, userId, { fields });
+
+	// if the thread being marked as read is the last one unread also clear the unread subscription flag
+	const clearAlert = sub.tunread?.length <= 1 && sub.tunread.includes(tmid);
+
+	Subscriptions.removeUnreadThreadByRoomIdAndUserId(rid, userId, tmid, clearAlert);
+};
 
 export const readAllThreads = (rid, userId) => Subscriptions.removeAllUnreadThreadsByRoomIdAndUserId(rid, userId);

server/lib/markRoomAsRead.ts

@@ -0,0 +1,21 @@
+import { callbacks } from '../../app/callbacks/server';
+import { NotificationQueue, Subscriptions } from '../../app/models/server/raw';
+
+export async function markRoomAsRead(rid: string, uid: string): Promise<void> {
+	callbacks.run('beforeReadMessages', rid, uid);
+
+	const projection = { ls: 1, tunread: 1, alert: 1 };
+	const sub = await Subscriptions.findOneByRoomIdAndUserId(rid, uid, { projection });
+	if (!sub) {
+		throw new Error('error-invalid-subscription');
+	}
+
+	// do not mark room as read if there are still unread threads
+	const alert = sub.alert && sub.tunread?.length > 0;
+
+	await Subscriptions.setAsReadByRoomIdAndUserId(rid, uid, alert);
+
+	await NotificationQueue.clearQueueByUserId(uid);
+
+	callbacks.runAsync('afterReadMessages', rid, { uid, lastSeen: sub.ls });
+}

server/methods/readMessages.js

@@ -1,39 +1,27 @@
 import { Meteor } from 'meteor/meteor';
 import { check } from 'meteor/check';
 
-import { callbacks } from '../../app/callbacks/server';
-import { Subscriptions } from '../../app/models/server';
-import { NotificationQueue } from '../../app/models/server/raw';
+import { markRoomAsRead } from '../lib/markRoomAsRead';
+import { canAccessRoom } from '../../app/authorization/server';
+import { Rooms } from '../../app/models/server';
 
 Meteor.methods({
 	readMessages(rid) {
 		check(rid, String);
 
 		const userId = Meteor.userId();
-
 		if (!userId) {
 			throw new Meteor.Error('error-invalid-user', 'Invalid user', {
 				method: 'readMessages',
 			});
 		}
 
-		callbacks.run('beforeReadMessages', rid, userId);
-
-		// TODO: move this calls to an exported function
-		const userSubscription = Subscriptions.findOneByRoomIdAndUserId(rid, userId, { fields: { ls: 1 } });
-
-		if (!userSubscription) {
-			throw new Meteor.Error('error-invalid-subscription', 'Invalid subscription', {
-				method: 'readMessages',
-			});
+		const user = Meteor.user();
+		const room = Rooms.findOneById(rid);
+		if (!canAccessRoom(room, user)) {
+			throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'readMessages' });
 		}
 
-		Subscriptions.setAsReadByRoomIdAndUserId(rid, userId);
-
-		NotificationQueue.clearQueueByUserId(userId);
-
-		Meteor.defer(() => {
-			callbacks.run('afterReadMessages', rid, { userId, lastSeen: userSubscription.ls });
-		});
+		markRoomAsRead(rid, userId);
 	},
 });