-
Notifications
You must be signed in to change notification settings - Fork 389
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
MSC2895: Proposal to improve /members and /joined_rooms #2895
base: old_master
Are you sure you want to change the base?
Conversation
|
||
### Option 1: Query string | ||
|
||
A new query parameter, `membership`, should be added to the `/members` endpoint. The parameter |
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.
Personally I favor this. It's simple to implement and understand at a glance and doesn't over-complicate the matter. Arguably the filters gives you more, but it feels like an over-complication.
|
||
An example filter for getting members/rooms of membership `invite` or `join` would be: | ||
|
||
```json5 |
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.
IMO, it just feels like there isn't enough utlity here to re-use it. Some keys are ignored, and the ones that remain are of limited use. The bridges/bots I've written have usually wanted the full set of senders / rooms and end up filtering the data into various buckets (virtual users / real users / bot rooms / admin rooms) etc locally.
You could potentially want some kind of limit, but then you'd probably want to paginate...and then it feels like this could be something else entirely.
|
||
### Option 3: Even more filters | ||
|
||
Expanding on Option 2, we give `/state` the option of a filter (also from Option 2). This would |
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.
Having filters for /state | /context would be super useful, simply because so many projects of mine do not care about the full state of the room, but only the types it cares about. I don't know if this MSC is the one to solve that though. It might be more elegant though to do it this way.
Funnily enough a lot of bridges in the matrix-org domain do call /state on a lot of rooms to fetch membership because we can't currently fetch membership for invite/leave/ban so this would potentially make for easier upgrades..
|
||
## Alternative solutions | ||
|
||
### Using ?membership=join,invite or ?membership=join+invite instead |
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.
This isn't super uncommon with other REST apis. E.g. Twitter use commas. Agreed it's a bit of a PITA to hex encode, but I suspect relying on undefined behavior is slightly worse.
I with my encodeURIComponent
superpowers would probably favor this.
Rendered
This previously existed at #1337 however through a merge failure it now exists here.