Add general clarity to the /createRoom endpoint #1518

turt2live · 2018-08-15T23:30:13Z

Rendered: see 'docs' status check

This commit does a number of things:

Minor formatting/alignment changes
Document the room_alias response key. This could be deprecated now, or forfeited, if needed.
Remove the guest_can_join parameter - it is not actually supported
Document the previously undocumented power_level_content_override parameter
Clarify that the room_id is required on the response
More clearly spell out which events are created as part of the request
Clarify how the room alias becomes the canonical alias
Clarify how the visibility may be used to determine a default preset to apply
Document the m.federate creation content parameter, adding an option for the homeserver to define a default value

References:

Preset being inferred by the visibility: https://github.com/matrix-org/synapse/blob/cd32c19a604549b4518d748c07149d140bc9e063/synapse/handlers/room.py#L172-L177
Power level content overrides:
- https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L198
- https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L335-L359
Aliases becoming canonical: https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L366-L370
m.federate landing in the create event: https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L311-L315

Fixes #1243
Fixes #1471
Inspired by #1213
Fixes #681

This commit does a number of things: * Minor formatting/alignment changes * Document the room_alias response key. This could be deprecated now, or forfeited, if needed. * Remove the guest_can_join parameter - it is not actually supported * Document the previously undocumented power_level_content_override parameter * Clarify that the room_id is required on the response * More clearly spell out which events are created as part of the request * Clarify how the room alias becomes the canonical alias * Clarify how the `visibility` may be used to determine a default preset to apply * Document the `m.federate` creation content parameter, adding an option for the homeserver to define a default value References: * Preset being inferred by the visibility: https://github.com/matrix-org/synapse/blob/cd32c19a604549b4518d748c07149d140bc9e063/synapse/handlers/room.py#L172-L177 * Power level content overrides: * https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L198 * https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L335-L359 * Aliases becoming canonical: https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L366-L370 * `m.federate` landing in the create event: https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room.py#L311-L315 Fixes https://github.com/matrix-org/matrix-doc/issues/1243 Fixes matrix-org#1471 Inspired by matrix-org#1213

richvdh

This endpoint might get a prize for being the most byzantine API in the whole of matrix, and there is some competition :/

richvdh · 2018-08-20T09:45:50Z

api/client-server/create_room.yaml

@@ -196,10 +223,17 @@ paths:
                type: string
                description: |-
                  The created room's ID.
+              room_alias:


it sounds like we need to reach an agreement over what to do with this. I can't entirely remember why I was in favour of removing it, but either way I'd like us to discuss it before committing to it.

#1243 makes it sound like your opinion was that we aren't this helpful in other endpoints.

richvdh · 2018-08-20T09:53:40Z

api/client-server/create_room.yaml

@@ -89,6 +96,11 @@ paths:
                  ``private`` visibility if this key is not included. NB: This
                  should not be confused with ``join_rules`` which also uses the
                  word ``public``.
+
+                  If no ``preset`` is specificed, the server may use the visbility


I think this might be clearer specified against the preset property rather than here.

Also, a "may" specification here is kinda useless imho; the only reason for speccing it would be if the client could rely on it happening. Assuming we don't want to deprecate this behaviour, this should be at least a "should".

will go with 'should' and move it to the preset. This is one of those cases where I was looking at synapse and ran across the if statement that does this and thought it was worth including.

richvdh · 2018-08-20T09:58:09Z

api/client-server/create_room.yaml

@@ -145,6 +160,14 @@ paths:
                  The server will clobber the following keys: ``creator``. Future
                  versions of the specification may allow the server to clobber
                  other keys.
+                properties:


speccing this fails to make it clear that m.federate is just one example of a property that can be set here. It might be better just to say "extra keys... such as m.federate" and link to the m.room.create event schema spec.

richvdh · 2018-08-20T10:02:08Z

api/client-server/create_room.yaml

+                  event. This object is applied on top of the generated power
+                  level event prior to it being sent to the room. Defaults
+                  to overriding nothing.
+                $ref: "definitions/event-schemas/power_level_content_schema.yaml"


Can we just link to the m.room.power_levels event rather than duplicating the whole content in the spec - not least because anyone implementing this endpoint needs to inspect the two sections carefully to make sure that they actually are the same.

tbh I'm not a huge fan of linking between different sections, as it leads to infinite new tabs when trying to implement something. Would a link + keeping the $ref be okay?

actually it kinda makes more sense to put it as a link only, similar to m.federate. Will do that instead.

The argument is that this isn't really needed at this time.

turt2live · 2018-08-20T18:26:04Z

I have removed room_alias from the PR for now. I'll leave this open for a day or so (assuming approved) to give people a chance at disagreeing strongly with not having a room_alias response field.

richvdh · 2018-08-21T17:55:59Z

hum, why is there no docs status ? :'(

turt2live · 2018-08-21T17:58:30Z

Good question. The docs are here though: https://515-24998719-gh.circle-artifacts.com/0/root/project/scripts/gen/index.html (as of writing this comment)

richvdh

an example for power_levels_content wouldn't have gone amiss, but lgtm

The words already say this is deprecated, but it was missing the flag.

turt2live added 2 commits August 15, 2018 17:25

Changelog

ef7570e

turt2live requested a review from a team August 15, 2018 23:30

richvdh requested changes Aug 20, 2018

View reviewed changes

turt2live added 4 commits August 20, 2018 12:21

Move the power level event schema to where it was

7d14309

Move description about which preset to use when none is specified

6c7a93d

Clarify that the creation event can have other keys in it

1fd7c99

Take out the room_alias response field

7e6ca5f

The argument is that this isn't really needed at this time.

Define the type of the power_level_content_override field

8dc6f09

turt2live assigned richvdh Aug 20, 2018

turt2live requested a review from richvdh August 20, 2018 20:40

richvdh approved these changes Aug 21, 2018

View reviewed changes

turt2live merged commit e4f5c3d into matrix-org:master Aug 21, 2018

turt2live deleted the travis/c2s/create-room-improvements branch August 21, 2018 18:05

RiotTranslateBot pushed a commit to RiotTranslateBot/matrix-doc that referenced this pull request Aug 22, 2023

Add 'deprecated:true' to /v1/send_{join,leave} (matrix-org#1518)

188d568

The words already say this is deprecated, but it was missing the flag.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add general clarity to the /createRoom endpoint #1518

Add general clarity to the /createRoom endpoint #1518

turt2live commented Aug 15, 2018 •

edited

Loading

richvdh left a comment

richvdh Aug 20, 2018

turt2live Aug 20, 2018

richvdh Aug 20, 2018

richvdh Aug 20, 2018

turt2live Aug 20, 2018

richvdh Aug 20, 2018

richvdh Aug 20, 2018

turt2live Aug 20, 2018

turt2live Aug 20, 2018

turt2live commented Aug 20, 2018

richvdh commented Aug 21, 2018

turt2live commented Aug 21, 2018

richvdh left a comment

Add general clarity to the /createRoom endpoint #1518

Add general clarity to the /createRoom endpoint #1518

Conversation

turt2live commented Aug 15, 2018 • edited Loading

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live commented Aug 20, 2018

richvdh commented Aug 21, 2018

turt2live commented Aug 21, 2018

richvdh left a comment

Choose a reason for hiding this comment

turt2live commented Aug 15, 2018 •

edited

Loading