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

document joinedMembers and joinedRooms endpoints #999

Merged
merged 13 commits into from Oct 10, 2017

Conversation

Projects
None yet
4 participants
@t3chguy
Contributor

t3chguy commented Sep 16, 2017

Draft for #734
Based on matrix-org/synapse#1680

document endpoints from matrix-org/synapse#1680
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@matrixbot

This comment has been minimized.

Member

matrixbot commented Sep 16, 2017

Can one of the admins verify this patch?

@t3chguy t3chguy changed the title from document endpoints from https://github.com/matrix-org/synapse/pull/1680 to document joined_members and joined_rooms endpoints Sep 16, 2017

@richvdh richvdh self-assigned this Sep 17, 2017

@richvdh

This comment has been minimized.

Member

richvdh commented Sep 26, 2017

matrixbot: ok to test

@richvdh

Thanks for the PR! Sorry this stuff tends to be rather fiddly.

One other thing in addition to the below: could you add an entry to https://github.com/matrix-org/matrix-doc/blob/master/changelogs/client_server.rst ?

# limitations under the License.
swagger: '2.0'
info:
title: "Matrix Client-Server Room Creation API"

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

s/creation/listing/, I guess

This comment has been minimized.

@t3chguy

t3chguy Oct 2, 2017

Contributor

.

- application/json
produces:
- application/json
paths:

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

needs a securityDefinitions

paths:
"/joined_rooms":
get:
summary: Lists the rooms that the authenticated user is in.

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

This is a bit tortuous. "Lists the user's current rooms". Likewise below.

@@ -0,0 +1,55 @@
# Copyright 2017 Michael Telatynski <7t3chguy@gmail.com>

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

7t3chguy ?

This comment has been minimized.

@t3chguy

t3chguy Sep 27, 2017

Contributor

yeah t3chguy@gmail.com was already taken xD

get:
summary: Lists the rooms that the authenticated user is in.
description: |-
Lists the rooms that the authenticated user is in.

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

Can we make this a proper sentence: "This API returns a list of ..."

get:
summary: Get the m.room.member events for the room.
description:
Get the list of currently joined users and their profile data.

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

Could you explain why it is advantageous to use this instead of /rooms/roomid/members? (basically: it can be implemented more efficiently on the server, so is faster to respond)

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

Also: proper sentence here please

- in: path
type: string
name: roomId
description: The room to get the member events for.

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

s/ event// ?

responses:
200:
examples:
application/json: |-

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

again, make this a real object

avatar_url:
type: string
description: The mxc avatar url of the user this object is representing.
description: A map from user ID to a RoomMember object.

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

Can you make it clear what the (intended) behaviour is if the user has left the room.

The /state endpoint has: "If the user is a member of the room this will be the current state of the room as a list of events. If the user has left the room then this will be the state of the room when they left as a list of events." I suggest you base a description on that.

This comment has been minimized.

@t3chguy

t3chguy Oct 2, 2017

Contributor

Currently when you call it after leaving the room you get:
{"errcode":"M_UNKNOWN","error":"Internal server error"}

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

hence "(intended)". We can specify something sane here even if synapse doesn't implement it.

This comment has been minimized.

@t3chguy

t3chguy Oct 10, 2017

Contributor

is it not implied by the 403 response?

This comment has been minimized.

@t3chguy

t3chguy Oct 10, 2017

Contributor

Done

@@ -1327,6 +1327,7 @@ before they can re-join the room or be re-invited.
Listing rooms
~~~~~~~~~~~~~
{{list_joined_rooms_cs_http_api}}

This comment has been minimized.

@richvdh

richvdh Sep 27, 2017

Member

suggest this belongs under "Room membership" rather than here.

This comment has been minimized.

@t3chguy

t3chguy Oct 2, 2017

Contributor

it is under Room Membership, in a subcategory, should it get its own or not be in a subcat or?

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

errr, no it's not. I have "7.4 Room membership"; "7.5 Listing rooms". It is under 7.5. I suggest you stick it directly under 7.4 (ie line 1262 or so)

@turt2live

This comment has been minimized.

Member

turt2live commented Sep 28, 2017

Is it worth documenting how AS users differ for this endpoint? matrix-org/synapse#2476

@t3chguy

This comment has been minimized.

Contributor

t3chguy commented Oct 2, 2017

@t3chguy t3chguy changed the title from document joined_members and joined_rooms endpoints to document joinedMembers and joinedRooms endpoints Oct 2, 2017

@t3chguy

This comment has been minimized.

Contributor

t3chguy commented Oct 2, 2017

@turt2live detail plox?

t3chguy added some commits Oct 2, 2017

Apply most of rich's feedback
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
example response has a null, I copied it without reading...
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
missed a joined_rooms -> joinedRooms
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@turt2live

This comment has been minimized.

Member

turt2live commented Oct 2, 2017

If an application service requests the joined members for a room, synapse will see if any user under the application service's namespace is in the room. For everyone else, the requesting user has to be in the room.

fix indentation
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@richvdh

also, still needs a changelog please

paths:
"/joined_rooms":
"/joinedRooms":

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

nope, joined_rooms please.

{
"joined_rooms": [
application/json: {
"joinedRooms": [

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

joined_rooms

responses:
200:
description: A list of the rooms the user is in.
schema:
type: object
description: A list of the rooms the user is in.
required: ["joined_rooms"]
required: ["joinedRooms"]

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

joined_rooms

@@ -299,27 +299,29 @@ paths:
member of the room.
tags:
- Room participation
"/rooms/{roomId}/joined_members":
"/rooms/{roomId}/joinedMembers":

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

joined_members

description:
Get the list of currently joined users and their profile data.
This API returns a map of MXIDs to member info objects for members of the room.
Primarily for Application Services, can be implemented more efficiently on the server so faster to respond.

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

this sentence no verb

required: true
x-example: "!636q39766251:example.com"
security:
- accessToken: []
responses:
200:
examples:

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

needs a description here

avatar_url:
type: string
description: The mxc avatar url of the user this object is representing.
description: A map from user ID to a RoomMember object.

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

hence "(intended)". We can specify something sane here even if synapse doesn't implement it.

@@ -1327,6 +1327,7 @@ before they can re-join the room or be re-invited.
Listing rooms
~~~~~~~~~~~~~
{{list_joined_rooms_cs_http_api}}

This comment has been minimized.

@richvdh

richvdh Oct 5, 2017

Member

errr, no it's not. I have "7.4 Room membership"; "7.5 Listing rooms". It is under 7.5. I suggest you stick it directly under 7.4 (ie line 1262 or so)

t3chguy added some commits Oct 10, 2017

revert casing change
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Append note about usability of endpoint and move location in rst
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
this sentence no verb
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
fix phrasing
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
make intended behaviour consistent
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
add changes to changelog
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
add missing description
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
- New endpoints:
- ``GET /joined_rooms``
(`#343 <https://github.com/matrix-org/matrix-doc/pull/999>`_).

This comment has been minimized.

@richvdh

richvdh Oct 10, 2017

Member

s/343/999/

(`#343 <https://github.com/matrix-org/matrix-doc/pull/999>`_).
- ``GET /rooms/{roomId}/joined_members``
(`#346 <https://github.com/matrix-org/matrix-doc/pull/999>`_).

This comment has been minimized.

@richvdh

richvdh Oct 10, 2017

Member

s/346/999/

This API returns a map of MXIDs to member info objects for members of the room.
Primarily for Application Services, can be implemented more efficiently on the server so faster to respond.
This API returns a map of MXIDs to member info objects for members of the room. The current user must be in the room for it to work, unless it is an Application Service in which case any of the AS's users must be in the room.
This API is primarily for Application Services and should be faster to respond than ``/members`` as it can be implemented more efficiently on the server.

This comment has been minimized.

@richvdh

richvdh Oct 10, 2017

Member

perfect 🎖

@richvdh richvdh merged commit bf3b49f into matrix-org:master Oct 10, 2017

1 check passed

Docs (Merged PR) Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment