[IMPROVE] Add support to queries in channels.members and `groups.me…

…mbers` endpoints (#21414) Co-authored-by: Diego Sampaio <chinello@gmail.com>
RocketChat · May 14, 2021 · 35b497c · 35b497c
1 parent c2e99d3
commit 35b497c
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 70 deletions.
diff --git a/app/api/server/v1/channels.js b/app/api/server/v1/channels.js
@@ -1,13 +1,15 @@
 import { Meteor } from 'meteor/meteor';
+import { Match, check } from 'meteor/check';
 import _ from 'underscore';
 
-import { Rooms, Subscriptions, Messages, Uploads, Integrations, Users } from '../../../models';
+import { Rooms, Subscriptions, Messages, Uploads, Integrations, Users } from '../../../models/server';
 import { hasPermission, hasAtLeastOnePermission, hasAllPermission } from '../../../authorization/server';
 import { mountIntegrationQueryBasedOnPermissions } from '../../../integrations/server/lib/mountQueriesBasedOnPermission';
 import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { API } from '../api';
-import { settings } from '../../../settings';
+import { settings } from '../../../settings/server';
 import { Team } from '../../../../server/sdk';
+import { findUsersOfRoom } from '../../../../server/lib/findUsersOfRoom';
 
 
 // Returns the channel IF found otherwise it will return the failure of why it didn't. Check the `statusCode` property
@@ -575,29 +577,31 @@ API.v1.addRoute('channels.members', { authRequired: true }, {
 			return API.v1.unauthorized();
 		}
 
-		const { offset, count } = this.getPaginationItems();
+		const { offset: skip, count: limit } = this.getPaginationItems();
 		const { sort = {} } = this.parseJsonQuery();
 
-		const subscriptions = Subscriptions.findByRoomId(findResult._id, {
-			fields: { 'u._id': 1 },
-			sort: { 'u.username': sort.username != null ? sort.username : 1 },
-			skip: offset,
-			limit: count,
-		});
-
-		const total = subscriptions.count();
+		check(this.queryParams, Match.ObjectIncluding({
+			status: Match.Maybe([String]),
+			filter: Match.Maybe(String),
+		}));
+		const { status, filter } = this.queryParams;
 
-		const members = subscriptions.fetch().map((s) => s.u && s.u._id);
+		const cursor = findUsersOfRoom({
+			rid: findResult._id,
+			...status && { status: { $in: status } },
+			skip,
+			limit,
+			filter,
+			...sort?.username && { sort: { username: sort.username } },
+		});
 
-		const users = Users.find({ _id: { $in: members } }, {
-			fields: { _id: 1, username: 1, name: 1, status: 1, statusText: 1, utcOffset: 1 },
-			sort: { username: sort.username != null ? sort.username : 1 },
-		}).fetch();
+		const total = cursor.count();
+		const members = cursor.fetch();
 
 		return API.v1.success({
-			members: users,
-			count: users.length,
-			offset,
+			members,
+			count: members.length,
+			offset: skip,
 			total,
 		});
 	},

diff --git a/app/api/server/v1/groups.js b/app/api/server/v1/groups.js
@@ -1,13 +1,14 @@
 import _ from 'underscore';
 import { Meteor } from 'meteor/meteor';
-import { Match } from 'meteor/check';
+import { Match, check } from 'meteor/check';
 
 import { mountIntegrationQueryBasedOnPermissions } from '../../../integrations/server/lib/mountQueriesBasedOnPermission';
 import { Subscriptions, Rooms, Messages, Uploads, Integrations, Users } from '../../../models/server';
 import { hasPermission, hasAtLeastOnePermission, canAccessRoom, hasAllPermission } from '../../../authorization/server';
 import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { API } from '../api';
 import { Team } from '../../../../server/sdk';
+import { findUsersOfRoom } from '../../../../server/lib/findUsersOfRoom';
 
 // Returns the private group subscription IF found otherwise it will return the failure of why it didn't. Check the `statusCode` property
 export function findPrivateGroupByIdOrName({ params, userId, checkedArchived = true }) {
@@ -23,6 +24,7 @@ export function findPrivateGroupByIdOrName({ params, userId, checkedArchived = t
 			fname: 1,
 			prid: 1,
 			archived: 1,
+			broadcast: 1,
 		},
 	};
 	const room = params.roomId
@@ -54,6 +56,7 @@ export function findPrivateGroupByIdOrName({ params, userId, checkedArchived = t
 		ro: room.ro,
 		t: room.t,
 		name: roomName,
+		broadcast: room.broadcast,
 	};
 }
 
@@ -491,36 +494,40 @@ API.v1.addRoute('groups.listAll', { authRequired: true }, {
 
 API.v1.addRoute('groups.members', { authRequired: true }, {
 	get() {
-		const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId });
-		const room = Rooms.findOneById(findResult.rid, { fields: { broadcast: 1 } });
+		const findResult = findPrivateGroupByIdOrName({
+			params: this.requestParams(),
+			userId: this.userId,
+		});
 
-		if (room.broadcast && !hasPermission(this.userId, 'view-broadcast-member-list')) {
+		if (findResult.broadcast && !hasPermission(this.userId, 'view-broadcast-member-list')) {
 			return API.v1.unauthorized();
 		}
 
-		const { offset, count } = this.getPaginationItems();
+		const { offset: skip, count: limit } = this.getPaginationItems();
 		const { sort = {} } = this.parseJsonQuery();
 
-		const subscriptions = Subscriptions.findByRoomId(findResult.rid, {
-			fields: { 'u._id': 1 },
-			sort: { 'u.username': sort.username != null ? sort.username : 1 },
-			skip: offset,
-			limit: count,
-		});
-
-		const total = subscriptions.count();
+		check(this.queryParams, Match.ObjectIncluding({
+			status: Match.Maybe([String]),
+			filter: Match.Maybe(String),
+		}));
+		const { status, filter } = this.queryParams;
 
-		const members = subscriptions.fetch().map((s) => s.u && s.u._id);
+		const cursor = findUsersOfRoom({
+			rid: findResult.rid,
+			...status && { status: { $in: status } },
+			skip,
+			limit,
+			filter,
+			...sort?.username && { sort: { username: sort.username } },
+		});
 
-		const users = Users.find({ _id: { $in: members } }, {
-			fields: { _id: 1, username: 1, name: 1, status: 1, statusText: 1, utcOffset: 1 },
-			sort: { username: sort.username != null ? sort.username : 1 },
-		}).fetch();
+		const total = cursor.count();
+		const members = cursor.fetch();
 
 		return API.v1.success({
-			members: users,
-			count: users.length,
-			offset,
+			members,
+			count: members.length,
+			offset: skip,
 			total,
 		});
 	},

diff --git a/app/models/server/models/Users.js b/app/models/server/models/Users.js
@@ -774,7 +774,7 @@ export class Users extends Base {
 		}
 
 		// if the search term is empty, don't need to have the $or statement (because it would be an empty regex)
-		if (searchTerm === '') {
+		if (!searchTerm) {
 			const query = {
 				$and: [
 					{

diff --git a/server/lib/findUsersOfRoom.ts b/server/lib/findUsersOfRoom.ts
@@ -0,0 +1,38 @@
+import type { Mongo } from 'meteor/mongo';
+
+import { Users } from '../../app/models/server';
+import { settings } from '../../app/settings/server';
+import { IUser } from '../../definition/IUser';
+
+type FindUsersParam = {
+	rid: string;
+	status?: string;
+	skip?: number;
+	limit?: number;
+	filter?: string;
+	sort?: Record<string, any>;
+};
+
+export function findUsersOfRoom({ rid, status, skip = 0, limit = 0, filter = '', sort = {} }: FindUsersParam): Mongo.Cursor<IUser> {
+	const options = {
+		fields: {
+			name: 1,
+			username: 1,
+			nickname: 1,
+			status: 1,
+			avatarETag: 1,
+			_updatedAt: 1,
+		},
+		sort: {
+			statusConnection: -1,
+			...sort || { [settings.get('UI_Use_Real_Name') ? 'name' : 'username']: 1 },
+		},
+		...skip > 0 && { skip },
+		...limit > 0 && { limit },
+	};
+
+	return Users.findByActiveUsersExcept(filter, undefined, options, undefined, [{
+		__rooms: rid,
+		...status && { status },
+	}]);
+}
diff --git a/server/methods/getUsersOfRoom.js b/server/methods/getUsersOfRoom.js
@@ -1,35 +1,11 @@
 import { Meteor } from 'meteor/meteor';
 
-import { Subscriptions, Users } from '../../app/models/server';
-import { hasPermission } from '../../app/authorization';
-import { settings } from '../../app/settings';
-
-function findUsers({ rid, status, skip, limit, filter = '' }) {
-	const options = {
-		fields: {
-			name: 1,
-			username: 1,
-			nickname: 1,
-			status: 1,
-			avatarETag: 1,
-			_updatedAt: 1,
-		},
-		sort: {
-			statusConnection: -1,
-			[settings.get('UI_Use_Real_Name') ? 'name' : 'username']: 1,
-		},
-		...skip > 0 && { skip },
-		...limit > 0 && { limit },
-	};
-
-	return Users.findByActiveUsersExcept(filter, undefined, options, undefined, [{
-		__rooms: rid,
-		...status && { status },
-	}]).fetch();
-}
+import { Subscriptions } from '../../app/models/server';
+import { hasPermission } from '../../app/authorization/server';
+import { findUsersOfRoom } from '../lib/findUsersOfRoom';
 
 Meteor.methods({
-	async getUsersOfRoom(rid, showAll, { limit, skip } = {}, filter) {
+	getUsersOfRoom(rid, showAll, { limit, skip } = {}, filter) {
 		const userId = Meteor.userId();
 		if (!userId) {
 			throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'getUsersOfRoom' });
@@ -46,7 +22,7 @@ Meteor.methods({
 
 		const total = Subscriptions.findByRoomIdWhenUsernameExists(rid).count();
 
-		const users = await findUsers({ rid, status: !showAll ? { $ne: 'offline' } : undefined, limit, skip, filter });
+		const users = findUsersOfRoom({ rid, status: !showAll ? { $ne: 'offline' } : undefined, limit, skip, filter }).fetch();
 
 		return {
 			total,