feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole #34153

abhinavkrin · 2024-12-10T12:54:36Z

Proposed changes (including videos or screenshots)

Adds groups.membersOrderedByRole and channels.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.

A sample request

curl --location --globoff 'http://localhost:3000/api/v1/channels.membersOrderedByRole?roomId=GENERAL&sort={%22username%22%3A%201}&offset=0&rolesOrder[]=leader&rolesOrder[]=owner' \
--header 'X-Auth-Token: Ce_2eOJ19mnW59wVO5IfSv4s650MF1w7mqY6i52uPG5' \
--header 'X-User-Id: u29m4aQNjZujdE4r5' \
--data ''

Issue(s)

Steps to test or reproduce

Further comments

CORE-846

dionisio-bot · 2024-12-10T12:54:40Z

Looks like this PR is not ready to merge, because of the following issues:

This PR is missing the 'stat: QA assured' label
This PR is targeting the wrong base branch. It should target 7.3.0, but it targets 7.2.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

changeset-bot · 2024-12-10T12:54:40Z

🦋 Changeset detected

Latest commit: e8d16bf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 35 packages

Name	Type
@rocket.chat/meteor	Minor
@rocket.chat/rest-typings	Minor
@rocket.chat/api-client	Patch
@rocket.chat/core-services	Patch
@rocket.chat/ddp-client	Patch
@rocket.chat/ui-contexts	Major
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/stream-hub-service	Patch
@rocket.chat/omnichannel-services	Patch
@rocket.chat/presence	Patch
rocketchat-services	Patch
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/network-broker	Patch
@rocket.chat/livechat	Patch
@rocket.chat/uikit-playground	Patch
@rocket.chat/fuselage-ui-kit	Major
@rocket.chat/gazzodown	Major
@rocket.chat/ui-avatar	Major
@rocket.chat/ui-client	Major
@rocket.chat/ui-video-conf	Major
@rocket.chat/ui-voip	Major
@rocket.chat/web-ui-registration	Major
@rocket.chat/core-typings	Minor
@rocket.chat/apps	Patch
@rocket.chat/cron	Patch
@rocket.chat/freeswitch	Patch
@rocket.chat/model-typings	Patch
@rocket.chat/license	Patch
@rocket.chat/pdf-worker	Patch
@rocket.chat/models	Patch
@rocket.chat/instance-status	Patch

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

github-actions · 2024-12-10T13:04:48Z

PR Preview Action v1.4.8
🚀 Deployed preview to https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-34153/
on branch `gh-pages` at 2024-12-20 14:42 UTC

codecov · 2024-12-10T13:37:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.27%. Comparing base (7edc3db) to head (6ceb8e3).
Report is 30 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #34153      +/-   ##
===========================================
- Coverage    75.83%   75.27%   -0.57%     
===========================================
  Files          511      516       +5     
  Lines        22171    22532     +361     
  Branches      5397     5485      +88     
===========================================
+ Hits         16813    16960     +147     
- Misses        4710     4911     +201     
- Partials       648      661      +13

Flag	Coverage Δ
unit	`75.27% <ø> (-0.57%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

MarcosSpessatto

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.

.changeset/silly-kings-approve.md

apps/meteor/tests/end-to-end/api/channels.ts

packages/rest-typings/src/v1/groups/GroupsMembersByOrderedRole.ts

packages/rest-typings/src/v1/channels/ChannelsMembersByOrderedRole.ts

KevLehman · 2024-12-17T19:28:45Z

apps/meteor/tests/end-to-end/api/rooms.ts

@@ -3647,4 +3648,179 @@ describe('[Rooms]', () => {
 				});
 		});
 	});
+
+	describe('[/rooms.membersOrderedByRole]', () => {


Add test cases for:

Trying to fetch a private room by a user not part of room

Trying to fetch a public room by a user not part of room

The same 2, but with users that are part of the rooms

Trying to fetch a private channel inside a public team for someone not part of the room

Trying to fetch a public channel inside a private team for someone not part of the room

The same 2, but with users that are part of the rooms

Just to ensure we don't show more data than what we should 🙈

I have added the following tests:

[Rooms]

[/rooms.membersOrderedByRole]

✔ should return a list of members ordered by owner, moderator, then members by default

✔ should allow custom role order

✔ should support pagination

✔ should return matched members when using filter param

✔ should return empty list if no matches (e.g., filter by status that no one has)

✔ should support custom sorting by username descending

Additional Visibility Tests

✔ should not fetch private room members by user not part of room

✔ should fetch private room members by user who is part of the room

✔ should fetch public room members by user who is part of the room

✔ should fetch public room members by user not part of room - because public (135ms)

✔ should fetch a private channel members inside a public team by someone part of the room

✔ should not fetch a private channel members inside a public team by someone not part of the room, but part of team

✔ should not fetch a private channel members inside a public team by someone not part of the team

✔ should fetch a public channel members inside a public team by someone part of the room

✔ should fetch a public channel members inside a public team by someone not part of the room, but part of team

✔ should fetch a public channel members inside a public team by someone not part of the team

✔ should fetch a public channel members inside a private team by someone part of the room

✔ should fetch a public channel members inside a private team by someone not part of the room, but part of team

✔ should not fetch a public channel members inside a private team by someone not part of team

✔ should fetch a private channel members inside a private team by someone part of the room

✔ should not fetch a private channel members inside a private team by someone not part of the room, but part of team

✔ should not fetch a private channel members inside a private team by someone not part of team

KevLehman · 2024-12-17T19:29:37Z

apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts

+		},
+	];
+
+	const [result] = await Subscriptions.col.aggregate(facetPipeline, { allowDiskUse: true }).toArray();


@MarcosSpessatto , should we run this on the secondary like the other aggregations?

I think it makes sense

KevLehman · 2024-12-17T19:30:10Z

apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts

+	roles: IRole['_id'][];
+};
+
+export async function findUsersOfRoomOrderedByRole({


Can you add some explains with some decent amount of data to check on the perf of the aggregation? :please:

apps/meteor/server/lib/findUsersOfRoomOrderedByRole.ts

apps/meteor/app/api/server/v1/rooms.ts

MarcosSpessatto

It looks good to me, we just need to do some perf tests with some fake data to ensure the query perf

Signed-off-by: Abhinav Kumar <[email protected]>

abhinavkrin added this to the 7.2.0 milestone Dec 10, 2024

abhinavkrin force-pushed the feat/add-endpoints-groups.membersOrderedByRole-channels.membersOrderedByRole branch from 4a3be37 to aa02c7a Compare December 10, 2024 12:55

abhinavkrin force-pushed the feat/add-endpoints-groups.membersOrderedByRole-channels.membersOrderedByRole branch from 7c92475 to df876a0 Compare December 13, 2024 17:58

abhinavkrin changed the title ~~[WIP] feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole~~ feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole Dec 16, 2024

abhinavkrin marked this pull request as ready for review December 16, 2024 15:48

abhinavkrin requested review from a team as code owners December 16, 2024 15:48

MarcosSpessatto requested changes Dec 16, 2024

View reviewed changes

abhinavkrin force-pushed the feat/add-endpoints-groups.membersOrderedByRole-channels.membersOrderedByRole branch 2 times, most recently from ae9d2e1 to c5fa198 Compare December 17, 2024 16:54

KevLehman requested changes Dec 17, 2024

View reviewed changes

MarcosSpessatto reviewed Dec 19, 2024

View reviewed changes

scuciatto modified the milestones: 7.2.0, 7.3.0 Dec 20, 2024

abhinavkrin added 6 commits December 20, 2024 20:26

feat: added endpoint rooms.membersOrderedByRole

26fda8c

Signed-off-by: Abhinav Kumar <[email protected]>

modified changeset

819fdf9

Signed-off-by: Abhinav Kumar <[email protected]>

fix tests

6860222

Signed-off-by: Abhinav Kumar <[email protected]>

added tests and fixed permissions

c93d16b

Signed-off-by: Abhinav Kumar <[email protected]>

minor code refactor

53de4b6

Signed-off-by: Abhinav Kumar <[email protected]>

aggregation changes

e8d16bf

Signed-off-by: Abhinav Kumar <[email protected]>

abhinavkrin force-pushed the feat/add-endpoints-groups.membersOrderedByRole-channels.membersOrderedByRole branch from 6ceb8e3 to e8d16bf Compare December 20, 2024 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole #34153

feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole #34153

abhinavkrin commented Dec 10, 2024 •

edited by jira bot

Loading

dionisio-bot bot commented Dec 10, 2024 •

edited

Loading

changeset-bot bot commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

codecov bot commented Dec 10, 2024 •

edited

Loading

MarcosSpessatto left a comment

KevLehman Dec 17, 2024

abhinavkrin Dec 18, 2024

KevLehman Dec 17, 2024

MarcosSpessatto Dec 19, 2024

KevLehman Dec 17, 2024

MarcosSpessatto left a comment

feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole #34153

Are you sure you want to change the base?

feat: add endpoints groups.membersOrderedByRole channels.membersOrderedByRole #34153

Conversation

abhinavkrin commented Dec 10, 2024 • edited by jira bot Loading

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

dionisio-bot bot commented Dec 10, 2024 • edited Loading

changeset-bot bot commented Dec 10, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Dec 10, 2024 • edited Loading

codecov bot commented Dec 10, 2024 • edited Loading

Codecov Report

MarcosSpessatto left a comment

Choose a reason for hiding this comment

KevLehman Dec 17, 2024

Choose a reason for hiding this comment

abhinavkrin Dec 18, 2024

Choose a reason for hiding this comment

KevLehman Dec 17, 2024

Choose a reason for hiding this comment

MarcosSpessatto Dec 19, 2024

Choose a reason for hiding this comment

KevLehman Dec 17, 2024

Choose a reason for hiding this comment

MarcosSpessatto left a comment

Choose a reason for hiding this comment

abhinavkrin commented Dec 10, 2024 •

edited by jira bot

Loading

dionisio-bot bot commented Dec 10, 2024 •

edited

Loading

changeset-bot bot commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

codecov bot commented Dec 10, 2024 •

edited

Loading