Skip to content
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 a v2 api for setting tags on rooms #132

Merged
merged 23 commits into from
Nov 27, 2015
Merged

Conversation

NegativeMjark
Copy link
Contributor

Todo:

  • Describe how tags appear in v1 /initialSync and v1 /events
  • Describe how tags appear in v2 /sync
  • Describe how tags can be used in v2 filters to group and filter rooms in the v2 sync.
  • Describe how tags can be used to control push notifications.

@@ -332,6 +337,16 @@ paths:
description: |-
Whether this room is visible to the ``/publicRooms`` API
or not."
private_user_data:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay! :)

@ara4n
Copy link
Member

ara4n commented Oct 30, 2015

This LGTM modulo comments above - thanks :)

Mark Haines added 3 commits November 2, 2015 10:13
…ll as v2 sync. Only add the room_id for v1 /events since it is redundant in v1 /initialSync
@ara4n
Copy link
Member

ara4n commented Nov 2, 2015

Final query here was whether to use m.roomtag or m.room.tag to avoid "m.tag" colliding horribly with message tag names etc in future. Unless we keep m.tag for rooms, and m.room.message.tag or something for message tags in future.

{
"type": "m.tag",
"content": {
"tags": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a list of tags? Or if it's a single tag, shouldn't the key be called "tag"? Any chance we can show an example of metadata for the tag - e.g. { order: 1 } rather than {}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done d538140

name: access_token
in: query
paths:
"/user/{userId}/rooms/{roomId}/tags":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the rationale for forcing clients to know their own userId in the path? We can completely infer this from the token.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the /user/user_id/filter APIs

Mark Haines added 2 commits November 16, 2015 16:56
Conflicts:
	api/client-server/v1/rooms.yaml
	specification/targets.yaml
@ara4n
Copy link
Member

ara4n commented Nov 16, 2015

in vector this is implemented with java style namespacing for the tag names - eg m.favourite. i still think we should name the tag events themselves m.room.tag rather than m.tag but implemented as the latter for now.

@NegativeMjark
Copy link
Contributor Author

@ara4n

How does vector handle tags it doesn't know about?
If we are using m.* namespaced tags it would be good to included them in this specification.

Conflicts:
	api/client-server/v2_alpha/sync.yaml
"account_data": {
"events": [
{
"type": "m.tags",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am tempted to use "m.tags" too but the chosen terminology is "m.tag", no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed d39494b

NegativeMjark added a commit that referenced this pull request Nov 27, 2015
Document a v2 api for setting tags on rooms
@NegativeMjark NegativeMjark merged commit c5f457c into master Nov 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants