[FIX] Replace query param by name, username and status on the teams.members endpoint#21539
[FIX] Replace query param by name, username and status on the teams.members endpoint#21539sampaiodiego merged 18 commits intodevelopfrom
query param by name, username and status on the teams.members endpoint#21539Conversation
app/api/server/v1/teams.ts
Outdated
| const canSeeAllMembers = hasPermission(this.userId, 'view-all-teams', team.roomId); | ||
|
|
||
| const query = { | ||
| username, |
There was a problem hiding this comment.
Is it expected that username/name are not RegExp here?
There was a problem hiding this comment.
Yes, so I should use check to assure this. Right?
There was a problem hiding this comment.
I don't think so 🤔 I was thinking that most username/name filters support regexes for searching, but if that's not needed here, then it's fine
There was a problem hiding this comment.
Not sure, but I think that supporting regexes would require some extra work (such as in the getUsersOfRoom method). @sampaiodiego , should this endpoint support RegExp?
There was a problem hiding this comment.
oops, sry, I forgot to answer the last mention..
so I think the main idea is to be able to use those *.members endpoints wherever getUsersOfRooms is being currently used:
in that sense it should accept a regex as well..
There was a problem hiding this comment.
In that sense, i guess the im.members PR (#21471) should be updated too to support a regex 🤔
app/api/server/v1/teams.ts
Outdated
| const canSeeAllMembers = hasPermission(this.userId, 'view-all-teams', team.roomId); | ||
|
|
||
| const query = { | ||
| username, |
There was a problem hiding this comment.
I don't think so 🤔 I was thinking that most username/name filters support regexes for searching, but if that's not needed here, then it's fine
app/api/server/v1/teams.ts
Outdated
| username: username ? new RegExp(username, 'i') : undefined, | ||
| name: name ? new RegExp(name, 'i') : undefined, | ||
| status: status ? { $in: status } : undefined, | ||
| } as Partial<FindOneOptions<IUser>>; |
There was a problem hiding this comment.
You forgot to change this coercion 🙈
There was a problem hiding this comment.
Ohhh, you're right 😬. That's now fixed
The SecondOptionalParam check was also giving some errors in the tests -- so I decided to remove it until we can think of a better way to check this kind of params (maybe in V2).
3 participants
Proposed changes (including videos or screenshots)
queryparam byname,usernameandstatuson theteams.membersendpoint.Issue(s)
Task - ClickUp
Steps to test or reproduce
Example input (using cURL):
Further comments