-
Notifications
You must be signed in to change notification settings - Fork 11.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IMPROVE] Add groups to the directory channels list #21687
Changes from 14 commits
6b6fc58
3fab5d8
1b23181
c1e67b9
f2c8194
55c9520
bb3ebc6
7f73092
fae1b98
2578b5d
0749ffd
834abcc
cef097e
c5aca1a
6ed1c4d
1f2f1bc
ed70cd2
e2b2a3c
1af0618
d453e96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -535,6 +535,10 @@ export class Rooms extends Base { | |||||
return this._db.find(query, options); | ||||||
} | ||||||
|
||||||
findTeamMainRooms(options) { | ||||||
return this._db.find({ teamMain: { $exists: true, $eq: true } }, options); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just a small change as both are equivalent but this is much easier to read:
Suggested change
|
||||||
} | ||||||
|
||||||
findByNameOrFNameAndType(name, type, options) { | ||||||
const query = { | ||||||
t: type, | ||||||
|
@@ -552,38 +556,40 @@ export class Rooms extends Base { | |||||
return this._db.find(query, options); | ||||||
} | ||||||
|
||||||
findByNameOrFNameAndTypeIncludingTeamRooms(name, type, teamIds, options) { | ||||||
findByNameOrFNameAndRoomIdsIncludingTeamRooms(text, teamIds, roomIds, options) { | ||||||
const searchTerm = text && new RegExp(text, 'i'); | ||||||
|
||||||
const query = { | ||||||
t: type, | ||||||
teamMain: { | ||||||
$exists: false, | ||||||
}, | ||||||
$and: [ | ||||||
{ teamMain: { $exists: false } }, | ||||||
{ prid: { $exists: false } }, | ||||||
{ | ||||||
$or: [ | ||||||
{ | ||||||
teamId: { | ||||||
$exists: false, | ||||||
}, | ||||||
t: 'c', | ||||||
teamId: { $exists: false }, | ||||||
}, | ||||||
{ | ||||||
teamId: { | ||||||
$in: teamIds, | ||||||
}, | ||||||
t: 'c', | ||||||
teamId: { $in: teamIds }, | ||||||
}, | ||||||
...roomIds?.length > 0 ? [{ | ||||||
_id: { | ||||||
$in: roomIds, | ||||||
}, | ||||||
}] : [], | ||||||
], | ||||||
}, | ||||||
{ | ||||||
...searchTerm ? [{ | ||||||
KevLehman marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
$or: [{ | ||||||
name, | ||||||
name: searchTerm, | ||||||
}, { | ||||||
fname: name, | ||||||
fname: searchTerm, | ||||||
}], | ||||||
}, | ||||||
}] : [], | ||||||
], | ||||||
}; | ||||||
|
||||||
// do not use cache | ||||||
return this._db.find(query, options); | ||||||
} | ||||||
|
||||||
|
@@ -603,7 +609,7 @@ export class Rooms extends Base { | |||||
}; | ||||||
|
||||||
if (text) { | ||||||
const regex = new RegExp(s.trim(escapeRegExp(text)), 'i'); | ||||||
const regex = new RegExp(text, 'i'); | ||||||
|
||||||
query.$and.push({ | ||||||
$or: [{ | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,15 +44,24 @@ const sortUsers = function(field, direction) { | |
} | ||
}; | ||
|
||
const getChannels = (user, canViewAnon, searchTerm, sort, pagination) => { | ||
const getChannelsAndGroups = (user, canViewAnon, searchTerm, sort, pagination) => { | ||
if ((!user && !canViewAnon) || (user && !hasPermission(user._id, 'view-c-room'))) { | ||
return; | ||
} | ||
|
||
const teams = Promise.await(Team.getAllPublicTeams()); | ||
const teamIds = teams.map(({ _id }) => _id); | ||
const teamsMains = Rooms.findTeamMainRooms({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @KevLehman 's suggestion was great, but finding and storing all main rooms here even though none of them might be used if "result" doesn't have any team doesn't seem the best approach. My suggestion would be:
rough example const result = Rooms.findByNameOrFNameAndRoomIdsIncludingTeamRooms( ..... );
const results = result.fetch();
const teamIds = results.filter(({ teamId }) => teamId).map(({ teamId }) => teamId);
const teams = Rooms.findByIds(teamIds, { fields: { name: 1 } });
return results.map((room) => {
if (room.teamId) {
const team = teams.find((team) => team._id === room.teamId);
if (team) room.belongsTo = team.name;
}
}); |
||
fields: { | ||
name: 1, | ||
teamId: 1, | ||
}, | ||
}).fetch(); | ||
|
||
const userTeamsIds = Promise.await(Team.listTeamsBySubscriberUserId(user._id, { projection: { teamId: 1 } }))?.map(({ teamId }) => teamId) || []; | ||
const userRooms = user.__rooms; | ||
|
||
const result = Rooms.findByNameOrFNameAndTypeIncludingTeamRooms(searchTerm, 'c', teamIds, { | ||
const result = Rooms.findByNameOrFNameAndRoomIdsIncludingTeamRooms(searchTerm, [...userTeamsIds, ...teamIds], userRooms, { | ||
...pagination, | ||
sort: { | ||
featured: -1, | ||
|
@@ -78,7 +87,7 @@ const getChannels = (user, canViewAnon, searchTerm, sort, pagination) => { | |
const total = result.count(); // count ignores the `skip` and `limit` options | ||
const results = result.fetch().map((room) => { | ||
if (room.teamId) { | ||
const team = teams.find((team) => team._id === room.teamId); | ||
const team = teamsMains.find((mainRoom) => mainRoom.teamId === room.teamId); | ||
if (team) { | ||
room.belongsTo = team.name; | ||
sampaiodiego marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
@@ -202,7 +211,7 @@ const getUsers = (user, text, workspace, sort, pagination) => { | |
|
||
Meteor.methods({ | ||
browseChannels({ text = '', workspace = '', type = 'channels', sortBy = 'name', sortDirection = 'asc', page, offset, limit = 10 }) { | ||
const regex = new RegExp(s.trim(escapeRegExp(text)), 'i'); | ||
const searchTerm = s.trim(escapeRegExp(text)); | ||
|
||
if (!['channels', 'users', 'teams'].includes(type) || !['asc', 'desc'].includes(sortDirection) || ((!page && page !== 0) && (!offset && offset !== 0))) { | ||
return; | ||
|
@@ -230,9 +239,9 @@ Meteor.methods({ | |
|
||
switch (type) { | ||
case 'channels': | ||
return getChannels(user, canViewAnonymous, regex, sortChannels(sortBy, sortDirection), pagination); | ||
return getChannelsAndGroups(user, canViewAnonymous, searchTerm, sortChannels(sortBy, sortDirection), pagination); | ||
case 'teams': | ||
return getTeams(user, text, sortChannels(sortBy, sortDirection), pagination); | ||
return getTeams(user, searchTerm, sortChannels(sortBy, sortDirection), pagination); | ||
case 'users': | ||
return getUsers(user, text, workspace, sortUsers(sortBy, sortDirection), pagination); | ||
default: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sry, a picky naming suggestion, since this is already on "rooms" model, I think we don't need to say we're finding rooms:
if my other suggestion is accepted, this method might not be needed anymore though