From 35b497c1753629b5c1c7923e683e57d0f6a1a1fb Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Fri, 14 May 2021 10:43:06 -0300 Subject: [PATCH] [IMPROVE] Add support to queries in `channels.members` and `groups.members` endpoints (#21414) Co-authored-by: Diego Sampaio --- app/api/server/v1/channels.js | 42 ++++++++++++++------------ app/api/server/v1/groups.js | 49 ++++++++++++++++++------------- app/models/server/models/Users.js | 2 +- server/lib/findUsersOfRoom.ts | 38 ++++++++++++++++++++++++ server/methods/getUsersOfRoom.js | 34 ++++----------------- 5 files changed, 95 insertions(+), 70 deletions(-) create mode 100644 server/lib/findUsersOfRoom.ts diff --git a/app/api/server/v1/channels.js b/app/api/server/v1/channels.js index ab7ea3350068..58a75e2667df 100644 --- 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 index f0f77ea0c437..ce0f06fc1813 100644 --- a/app/api/server/v1/groups.js +++ b/app/api/server/v1/groups.js @@ -1,6 +1,6 @@ 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'; @@ -8,6 +8,7 @@ import { hasPermission, hasAtLeastOnePermission, canAccessRoom, hasAllPermission 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 index 6a31157e71cf..575e71254660 100644 --- 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 new file mode 100644 index 000000000000..7c6d61433eca --- /dev/null +++ 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; +}; + +export function findUsersOfRoom({ rid, status, skip = 0, limit = 0, filter = '', sort = {} }: FindUsersParam): Mongo.Cursor { + 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 index efeea2233a45..d17d91013cb6 100644 --- 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,