Rocket.Chat icon indicating copy to clipboard operation
Rocket.Chat copied to clipboard

feat: Add `membersByHighestRole` endpoints

Open matheusbsilva137 opened this issue 1 year ago • 6 comments

Proposed changes (including videos or screenshots)

  • Add channels.membersByHighestRole and groups.membersByHighestRole endpoints to enable users to retrieve members of a room ordered by their highest (most important) role.

Issue(s)

Steps to test or reproduce

The endpoint's usage is the same as the current .members:

curl -H "X-Auth-Token: 9HqLlyZOugoStsXCUfD_0YdwnNnunAJF8V47U3QHXSq" \
     -H "X-User-Id: aobEdbYhXfu5hkeqG" \
     http://localhost:3000/api/v1/groups.membersByHighestRole?roomId=ByehQjC44FwMeiLbX

And results are as follows:

{
  "members": [
    {
      "_id": "JmR3T4TFfppJCfmq9",
      "status": "offline",
      "_updatedAt": "2023-07-20T20:43:05.895Z",
      "name": "Matheus Barbosa Silva",
      "statusConnection": "offline",
      "username": "matheus.barbosa",
      "avatarETag": "fdJn5SqCwg4ZwbywD",
      "highestRole": {
        "role": "owner",
        "level": 0
      }
    },
    {
      "_id": "edjvbnb7pGC3MZNSP",
      "status": "offline",
      "_updatedAt": "2023-07-17T21:42:31.845Z",
      "username": "john",
      "name": "John",
      "statusConnection": "offline",
      "highestRole": {
        "role": "member",
        "level": 2
      }
    }
  ],
  "count": 2,
  "offset": 0,
  "total": 2,
  "success": true
}
  • The members array contains a list of users ordered from highest to lowest role.
  • A numeric representation of the highest role level is given in the roleLevel field (inside the highestRole object), which ranges from 0 to 2. 0 is used for owners, 1 is used for moderators and 2 is used for any other room scoped roles.
  • The fields returned for each user are similar to those from the .members endpoints, except from the new highestRole object (which contains the level and role fields) and statusConnection. The roles field returned within each user contains only room scoped roles (and not global roles, as in the .members endpoints). Note: The same applies to the channels.membersByHighestRole endpoint

Further comments

The following rank between room scoped roles is used: Owner > Moderator > Other room scoped roles. TC-837

matheusbsilva137 avatar Jul 19 '23 17:07 matheusbsilva137

🦋 Changeset detected

Latest commit: cd44cfc09ea5cb6f1ba4f46617a468aa384e7e78

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

This PR includes changesets to release 31 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/core-typings Minor
@rocket.chat/model-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Major
@rocket.chat/livechat Patch
@rocket.chat/ui-contexts Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/ui-avatar Major
@rocket.chat/ui-client Major
@rocket.chat/ui-video-conf Major
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Major
@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

changeset-bot[bot] avatar Jul 19 '23 17:07 changeset-bot[bot]

Codecov Report

Attention: 39 lines in your changes are missing coverage. Please review.

Comparison is base (16cb0ea) 53.18% compared to head (cd44cfc) 54.35%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #29870      +/-   ##
===========================================
+ Coverage    53.18%   54.35%   +1.16%     
===========================================
  Files         2263     2276      +13     
  Lines        49675    50207     +532     
  Branches     10114    10373     +259     
===========================================
+ Hits         26422    27291     +869     
+ Misses       20835    20422     -413     
- Partials      2418     2494      +76     
Flag Coverage Δ
e2e 53.12% <ø> (+2.11%) :arrow_up:
e2e-api 39.93% <42.64%> (+0.08%) :arrow_up:
unit 76.47% <ø> (ø)

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

codecov[bot] avatar Jul 19 '23 18:07 codecov[bot]

following your description, this is then basically a filter right? I'm not understanding the connection between the description (filtering by role field) and the name of the endpoint membersByHighestRole - what does it mean by by highest role and how does the endpoint respect that definition?

and if it's just a filter why can it not be added to existing members endpoints?

debdutdeb avatar Jul 20 '23 10:07 debdutdeb

following your description, this is then basically a filter right?

and if it's just a filter why can it not be added to existing members endpoints?

Hey @debdutdeb !

Yeah, it is some sort of filter, but since this filter wouldn't be applied to a field that is returned in the endpoint, I felt like this should be something else (new). Just like we have the channels.list and channels.list.joined endpoints -- the latter can be seen as a "filter", but since it works different internally, this is a whole new endpoint.

Although I've taken the decision to split this in a new endpoint for now, this still needs to be discussed with the Archicteture team, so it may change :thinking: But I'm sure that adding a new param to the members endpoints is a feasible approach too.

I'm not understanding the connection between the description (filtering by role field) and the name of the endpoint membersByHighestRole - what does it mean by by highest role and how does the endpoint respect that definition?

We're not just filtering by role in this endpoint. Instead, we've defined a hierarchy between room scoped roles and we're using it to define which is the highest (most important) role of a user in a room. A user can have multiple roles in a room (eg they can be moderator and owner at the same time), but when filtering by highest role, we'll only return users that do not have any role above the one specified (if moderator is given, we won't return owners; or if leader is given, we won't return moderators or owners).

There is an improvement planned to members lists in rooms: instead of displaying a single list with all the users, we're going to split it in multiple lists (one for owners, other for moderators, other for leaders, and other list for any other role or no role at all) that's where this new endpoint fits.

Note: we changed the approach, so this comment is outdated. Read the PR's description.

matheusbsilva137 avatar Jul 20 '23 14:07 matheusbsilva137

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

kodiakhq[bot] avatar Oct 02 '23 12:10 kodiakhq[bot]

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

kodiakhq[bot] avatar Oct 05 '23 23:10 kodiakhq[bot]

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 missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

dionisio-bot[bot] avatar Apr 12 '24 19:04 dionisio-bot[bot]