-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
feat: add endpoint rooms.membersOrderedByRole #34153
feat: add endpoint rooms.membersOrderedByRole #34153
Conversation
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: dbe0674 The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
4a3be37
to
aa02c7a
Compare
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #34153 +/- ##
===========================================
- Coverage 59.18% 59.18% -0.01%
===========================================
Files 2819 2819
Lines 67717 67716 -1
Branches 15080 15080
===========================================
- Hits 40077 40076 -1
Misses 24818 24818
Partials 2822 2822
Flags with carried forward coverage won't be shown. Click here to find out more. |
7c92475
to
df876a0
Compare
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.
I think we could create a rooms.membersOrderedByRole
instead of creating one for each room type.
Also, we need to create some mock data to test the aggregation part and its performance.
@ricardogarim can you help us with that.
packages/rest-typings/src/v1/groups/GroupsMembersByOrderedRole.ts
Outdated
Show resolved
Hide resolved
packages/rest-typings/src/v1/channels/ChannelsMembersByOrderedRole.ts
Outdated
Show resolved
Hide resolved
ae9d2e1
to
c5fa198
Compare
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.
It looks good to me, we just need to do some perf tests with some fake data to ensure the query perf
6ceb8e3
to
e8d16bf
Compare
e8d16bf
to
ff2748b
Compare
1736241
to
89ea1e5
Compare
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
apps/meteor/app/lib/server/functions/syncRolePrioritiesForRoomIfRequired.ts
Outdated
Show resolved
Hide resolved
apps/meteor/app/lib/server/functions/syncRolePrioritiesForRoomIfRequired.ts
Show resolved
Hide resolved
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]> Co-authored-by: Abhinav Kumar <[email protected]>
7ee0f4b
…yRole-channels.membersOrderedByRole
Proposed changes (including videos or screenshots)
Adds
groups.membersOrderedByRole
andchannels.membersOrderedByRole
endpoints to retrieve members of groups and channels sorted according to their respective role in the room.This will help in grouping of users according to their roles in the room like below.
The endpoint works similar as groups.members and channels.members, but accepts an optional rolesOrder[] param which can be used for defining roles order while sorting. The index of the role will signify its priority. By default, owner > moderator > rest of members.The endpoint works similar as groups.members and channels.members, but sorts the members according to their role in the room. Owners > moderators > all other members. This can be reversed using
sort={"rolePriority":-1}
.The changes introduces an additional field roomRolePriorities to user collection. It will store the roomId and rolePriority score mapping which will be helpful in bringing down the sort time when there are a large number of users in a room.
This PR also adds a migration to assign the roomRolePriority to already existing room membersI have adopted a soft migration approach, as we only include migrations during breaking changes. In the endpoint, we first check whether the role priorities for the room have already been created. If not, we proceed to create them. For subsequent calls, this process is skipped.
I conducted performance testing using a room with 30,000 owners and moderators and a total of 100,000 members. The first call took approximately ~1 second, while subsequent calls were as fast as the channels.members endpoint. In most practical cases, where room sizes are smaller, the response time is significantly shorter.
A sample request
Issue(s)
Steps to test or reproduce
Further comments
CORE-846