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

add user_directory #1096

Merged
merged 7 commits into from Dec 29, 2017

Conversation

Projects
None yet
3 participants
@t3chguy
Contributor

t3chguy commented Dec 17, 2017

No description provided.

t3chguy added some commits Dec 17, 2017

@turt2live

This comment has been minimized.

Member

turt2live commented Dec 18, 2017

operationId is missing in /user_directory/search, verb post

@ara4n ara4n requested a review from richvdh Dec 18, 2017

@richvdh richvdh self-assigned this Dec 18, 2017

@richvdh

This comment has been minimized.

Member

richvdh commented Dec 18, 2017

I think this fixes #953

@@ -0,0 +1,93 @@
# Copyright 2016 OpenMarket Ltd

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

it's not 2016, and you don't work for openmarket. (you may be after New Vector Ltd or Michael Telatynski, depending which hat you feel like wearing)

This comment has been minimized.

@t3chguy

t3chguy Dec 18, 2017

Contributor

the woes of Copying entire files and my IDE collapsing the header and never actually seeing this

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

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

not the profile api

@@ -76,6 +76,9 @@ r0.3.0
- ``GET /media/{version}/preview_url``
(`#1064 <https://github.com/matrix-org/matrix-doc/pull/1064>`_).
- ``POST /user_directory/search``

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

Needs to go under 'unreleased changes', please.

$ref: definitions/security.yaml
paths:
"/user_directory/search":
post:

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

as Travis says, this is missing its operationId.

This comment has been minimized.

@t3chguy

t3chguy Dec 18, 2017

Contributor

done

required: ["search_term"]
responses:
200:
description: The results of the paginated search.

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

erm, "the paginated results of the search", surely. Or probably just "The results of the search".

example: "foo"
limit:
type: number
description: The maximum number of results to return

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

if this is optional, what's the behaviour when it's omitted?

post:
summary: Searches the user directory.
description: |-
This API paginates over search results of the user directory.

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

I think this emphasises the pagination too strongly. Not least because aiui pagination isn't currently supported. How about "Performs a server-side search over all users registered on the server".

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

could you also give some details about how the search works. Is it case-sensitive? Does it search displayname, mxid, both?

items:
title: User
type: object
properties:

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

is user_id not required here?

type: object
required: ["results", "limited"]
properties:
results:

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

do we know anything about the ordering of the results?

This comment has been minimized.

@t3chguy

t3chguy Dec 18, 2017

Contributor

vaguely, seems implementation specific

@@ -1335,6 +1335,13 @@ Listing rooms
{{list_public_rooms_cs_http_api}}
Users

This comment has been minimized.

@richvdh

richvdh Dec 18, 2017

Member

I think it might be worth merging this and profiles into a generic user data section.

t3chguy added some commits Dec 18, 2017

@t3chguy

This comment has been minimized.

Contributor

t3chguy commented Dec 18, 2017

back to you @richvdh

This API paginates over search results of the user directory.
This API performs a server-side search over all users registered on the server.
Searches MXID and displayname case-insesitively for users that you share a room with or that are in public rooms.
operationId: postUserDirectorySearch

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

why postUserDirectorySearch? seems inconsistent with everything else.

@@ -31,7 +31,9 @@ paths:
post:
summary: Searches the user directory.
description: |-
This API paginates over search results of the user directory.
This API performs a server-side search over all users registered on the server.
Searches MXID and displayname case-insesitively for users that you share a room with or that are in public rooms.

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

insesitively

@@ -31,7 +31,9 @@ paths:
post:
summary: Searches the user directory.
description: |-
This API paginates over search results of the user directory.
This API performs a server-side search over all users registered on the server.
Searches MXID and displayname case-insesitively for users that you share a room with or that are in public rooms.

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

full sentence here please.

@@ -46,12 +48,12 @@ paths:
example: "foo"
limit:
type: number
description: The maximum number of results to return
description: The maximum number of results to return (10 if omitted), with a maximum of 50

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

I'm not sure that the maximum here needs speccing, and if it does, it probably belongs in a note in the api description rather than that of the summary.

@@ -46,12 +48,12 @@ paths:
example: "foo"
limit:
type: number
description: The maximum number of results to return
description: The maximum number of results to return (10 if omitted), with a maximum of 50

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

suggest you use 'Defaults to 10' instead of (10 if omitted).

rav@fred:~/work/matrix-doc (master $%=)$ grep -ir 'if omitted' api/client-server/ | wc -l
1
rav@fred:~/work/matrix-doc (master $%=)$ grep -ir 'Defaults to' api/client-server/ | wc -l
6
@@ -46,12 +48,12 @@ paths:
example: "foo"
limit:
type: number
description: The maximum number of results to return
description: The maximum number of results to return (10 if omitted), with a maximum of 50

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

we seem to be using full-stops on these descriptions in general

@@ -31,7 +31,9 @@ paths:
post:
summary: Searches the user directory.
description: |-
This API paginates over search results of the user directory.
This API performs a server-side search over all users registered on the server.
Searches MXID and displayname case-insesitively for users that you share a room with or that are in public rooms.

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

MXID isn't actually used very much in the spec (and where it is, I think it's a bug). Suggest 'user ID'.

properties:
user_id:
type: string
example: "@foo:bar.com"
description: The MXID of the user.

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

"The user's matrix user ID"

{{users_cs_http_api}}
Profiles
--------
~~~~~~~~

This comment has been minimized.

@richvdh

richvdh Dec 20, 2017

Member

"Events on Change of Profile Information" is no longer included under "Profiles", which seems odd

@richvdh

\o/ perfect

@richvdh richvdh merged commit 73118b6 into matrix-org:master Dec 29, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@richvdh

This comment has been minimized.

Member

richvdh commented Feb 20, 2018

This was added to synapse in matrix-org/synapse#2252, ftr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment