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

Spec /createRoom is_direct flag, is_direct in member event and m.direct #389

Merged
merged 25 commits into from Oct 6, 2016

Conversation

Projects
None yet
3 participants
@erikjohnston
Member

erikjohnston commented Sep 29, 2016

No description provided.

@richvdh

Needs changelog too, please.

is_direct:
type: boolean
description: |-
This flag shows that the intent for this room is to be a

This comment has been minimized.

@richvdh

richvdh Sep 29, 2016

Member

but what does the server actually do on the createRoom? does it set room state, or what?

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

@richvdh richvdh assigned erikjohnston and unassigned richvdh Sep 29, 2016

@erikjohnston erikjohnston assigned richvdh and unassigned erikjohnston Sep 30, 2016

as should the inviter. This is to keep the intent for new rooms
synchronised at first. Both users may subsequently choose to
tag the room differently.
This adds an ``is_direct`` flag to the content of any invites

This comment has been minimized.

@richvdh

richvdh Sep 30, 2016

Member

"The server should set an internal flag indicating that this is a direct room. It should then add an is_direct flag to the content of any invites sent" ?

Is that it? it only affects invites? If a user joins the room proactively they don't get to hear about the flag?

What happens if @alice:alice.com invites @bob:bob.com, and bob invites @charlie:charlie.com? did bob.com remember the flag so that it can include it in the invite to charlie?

Does the flag end up in https://github.com/matrix-org/matrix-doc/blob/master/api/client-server/sync.yaml#L174? Whose job is it to update that?

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

So it turns out I got completely the wrong end of the stick from this. It needs to make it much clearer what the server is supposed to do.

How about just:

"This flag makes the server set the is_direct flag on the m.room.member events sent to the users in invite and invite_3pid. See (Direct Messaging) for more information."

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

@richvdh richvdh assigned erikjohnston and unassigned richvdh Sep 30, 2016

@erikjohnston

This comment has been minimized.

Member

erikjohnston commented Sep 30, 2016

The plan is for @dbkr to use this PR when he specs the rest of the DM stuff

@erikjohnston erikjohnston assigned dbkr and unassigned erikjohnston Sep 30, 2016

@dbkr dbkr changed the title from Add /createRoom is_direct flag to Spec /createRoom is_direct flag, is_direct in member event and m.direct Oct 3, 2016

@dbkr dbkr assigned richvdh and unassigned dbkr Oct 3, 2016

@@ -37,6 +37,9 @@
- Add top-level ``account_data`` key to the responses to ``GET /sync`` and
``GET /initialSync``
(`#380 <https://github.com/matrix-org/matrix-doc/pull/380>`_).
- Add ``is_direct`` flag to ``/createRoom`` and invite member event.

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

link to PR?

This comment has been minimized.

@dbkr

dbkr Oct 4, 2016

Member

They're the same PR as below - what syntax do I use for that?

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

just link it twice.

This comment has been minimized.

@richvdh
.. _module:dm:
All communications over Matrix happens within a room. It is sometimes

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

"communications happens"

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

.. _module:dm:
All communications over Matrix happens within a room. It is sometimes
desireable to offer users the concept of speaking directly to one

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

s/desireable/desirable/

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

Events
------
A map of which rooms are considered 'direct' rooms for specific users

This comment has been minimized.

@richvdh

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

(done)

When creating a room, the ``is_direct`` flag may be specified to signal
to the invitee that this is a direct chat. See
`GET /_matrix/client/unstable/initialSync`_. This flag appears as

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

initialSync is deprecated

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

@@ -39,6 +39,9 @@ properties:
- leave
- ban
type: string
is_direct:
description: Flag indicating if this room was created with the intention of being a direct chat

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

Needs a trailing "." for consistency.

"this room" is unclear here. "the room containing this event", maybe?

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

as should the inviter. This is to keep the intent for new rooms
synchronised at first. Both users may subsequently choose to
tag the room differently.
This adds an ``is_direct`` flag to the content of any invites

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

So it turns out I got completely the wrong end of the stick from this. It needs to make it much clearer what the server is supposed to do.

How about just:

"This flag makes the server set the is_direct flag on the m.room.member events sent to the users in invite and invite_3pid. See (Direct Messaging) for more information."

A room may not necessarily be considered 'direct' by all members of the
room, but a signalling mechanism exists to propagate the information of
whether a chat is 'direct' to an invitee. The invitee's client may

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

"The invitee's client... " belongs under "Client behaviour"

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

(done)

.. See the License for the specific language governing permissions and
.. limitations under the License.
Direct Messaging

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

We're missing "Client behaviour" and "Server behaviour" here. Both could usefully exist and say things.

This comment has been minimized.

@richvdh

richvdh Oct 4, 2016

Member

(done)

@richvdh richvdh assigned dbkr and unassigned richvdh Oct 4, 2016

@dbkr dbkr assigned richvdh and unassigned dbkr Oct 5, 2016

@richvdh richvdh assigned dbkr and unassigned richvdh Oct 5, 2016

@dbkr dbkr assigned richvdh and unassigned dbkr Oct 5, 2016

@@ -37,6 +37,9 @@
- Add top-level ``account_data`` key to the responses to ``GET /sync`` and
``GET /initialSync``
(`#380 <https://github.com/matrix-org/matrix-doc/pull/380>`_).
- Add ``is_direct`` flag to ``/createRoom`` and invite member event.

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

this is still missing its link

This comment has been minimized.

@dbkr

dbkr Oct 6, 2016

Member

ah yes. done

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

I actually meant its link to the PR. Which turned out not to matter any more, since you'd merged it with the next point in the changelog, which I'd missed somewhere in the 24 commits it's taken to get here.

Normally I haven't bothered linkifying API endpoints in the changelog. But this is fine!

The invitee's client may use the ``is_direct`` flag in `m.room.member`_ to
automatically mark the room as a direct message but this is not required: it
may for example, prompt the user, ignore the flag altogether. To do this, it
stores this event in account data using the ``account_data`` API: see `Client

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

Please link to the /account_data API (rather than the section) here.

This comment has been minimized.

@dbkr

dbkr Oct 6, 2016

Member

done

stores this event in account data using the ``account_data`` API: see `Client
Config`_.
The inviter's client should set the ``is_direct`` flag to .. |/createRoom|_

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

no dots needed here

This comment has been minimized.

@dbkr

dbkr Oct 6, 2016

Member

done

Server behaviour
----------------
When the ``is_direct`` flag is given to .. |/createRoom|_, the home

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

or here

This comment has been minimized.

@dbkr

dbkr Oct 6, 2016

Member

done

Client behaviour
----------------
The invitee's client may use the ``is_direct`` flag in `m.room.member`_ to

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

"in the m.room.member event"?

This comment has been minimized.

@dbkr

dbkr Oct 6, 2016

Member

done

----------------
When the ``is_direct`` flag is given to .. |/createRoom|_, the home
server must set the ``is_direct`` flag in the invite member event for any users
invited in the ``createRoom`` call.

This comment has been minimized.

@richvdh

richvdh Oct 6, 2016

Member

/createRoom rather than just createRoom, for consistency

This comment has been minimized.

@dbkr

dbkr Oct 6, 2016

Member

done

@richvdh richvdh assigned dbkr and unassigned richvdh Oct 6, 2016

dbkr added some commits Oct 6, 2016

Linkify account_data API
Also change other links because it turns out the .. isn't part
of the syntax

@dbkr dbkr assigned richvdh and unassigned dbkr Oct 6, 2016

@richvdh

This comment has been minimized.

Member

richvdh commented Oct 6, 2016

Right. 25 commits and 66 comments later, I think it will do.

@richvdh

richvdh approved these changes Oct 6, 2016

@richvdh richvdh merged commit 909aef2 into master Oct 6, 2016

2 checks passed

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