-
Notifications
You must be signed in to change notification settings - Fork 432
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
Update docs with recent changes to membership APIs #9210
Conversation
@@ -102,7 +102,7 @@ info: | |||
| 500 | Server Error | An error occurred with our API | | |||
|
|||
servers: | |||
- url: https://api.hypothes.is/api | |||
- url: https://hypothes.is/api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought I'd fix this while I was here: all the example URLs in our docs have been wrong, presumably since we first moved to redoc.
@@ -111,6 +111,7 @@ tags: | |||
- name: general | |||
- name: annotations | |||
- name: groups | |||
- name: memberships |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The membership APIs used to be listed under groups in the docs. I'm moving them out into their own separate section since they do form a separate set of endpoints (list-memberships, add-membership, get-membership, edit-membership and delete-membership). Like annotation, user and group, "membership" is now a first-class noun in the h model and API.
PageNumber: | ||
name: "page[number]" | ||
in: query | ||
required: false | ||
description: Which page of results to get | ||
schema: | ||
type: int | ||
default: 1 | ||
minimum: 1 | ||
PageSize: | ||
name: "page[size]" | ||
in: query | ||
required: false | ||
description: How many items to get per page | ||
schema: | ||
type: int | ||
default: 20 | ||
minimum: 1 | ||
maximum: 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting the pagination query params in a reusable place. They're currently only used by one endpoint but if we ever add another endpoint with page-based pagination we'll want to re-use the same query params. That said I don't know if/how you can customize the default
and maximum
values when reusing a parameter in OpenAPI. 🤷♂️ A problem for another day
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said I don't know if/how you can customize the
default
andmaximum
values when reusing a parameter in OpenAPI. 🤷♂️ A problem for another day
You can use allOf
where one of the "sides" is a $ref
to the common parts of the schema, and the rest are the parts that are different.
Membership: | ||
$ref: './schemas/membership.yaml#/Membership' | ||
MembershipCreate: | ||
$ref: './schemas/membership-create.yaml#/Membership' | ||
PaginationMeta: | ||
$ref: './schemas/pagination-meta.yaml#/PaginationMeta' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reusable schemas for various parts of membership API request and response bodies.
@@ -100,7 +100,7 @@ info: | |||
| 500 | Server Error | An error occurred with our API | | |||
|
|||
servers: | |||
- url: https://api.hypothes.is/api | |||
- url: https://hypothes.is/api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every change made to hypothesis-v1.yaml
has to be duplicated in hypothesis-v2.yaml
as well.
ede43c9
to
68c3e71
Compare
68c3e71
to
7f8c0f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 5 changed files in this pull request and generated no suggestions.
Files not reviewed (2)
- docs/_extra/api-reference/hypothesis-v1.yaml: Language not supported
- docs/_extra/api-reference/hypothesis-v2.yaml: Language not supported
Comments skipped due to low confidence (1)
docs/_extra/api-reference/schemas/membership.yaml:27
- The example for the 'actions' property includes 'updates.roles.member', which is inconsistent with the defined roles ('member', 'moderator', 'admin', 'owner'). Update the example to match the defined roles.
example: ["delete", "updates.roles.member"]
If the `page[number]` query param is included in the request then the response will be paginated. | ||
If there's no `page[number]` query param in the request then a legacy, | ||
un-paginated response format will be used. In the future this legacy | ||
un-paginated response format will be removed and the paginated response | ||
format will be used even when the `page[number]` query param is not | ||
present. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a PR ready to remove the un-paginated API when we're ready to do that, and I've included removing this stuff from the docs: #9183
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
type: array | ||
items: | ||
$ref: '#/components/schemas/Membership' | ||
- title: "Unpaginated (deprecated)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is deprecated, I reckon you can also add deprecated: true
, but I would need to check the docs to confirm this in fact possible in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually did try this but it didn't seem to do anything
Fixes #9134.
You can preview the new version of the docs built from this PR on readthedocs:
This PR depends on several other open PRs to actually make some of the API changes that're documented here: #9188, #9190, #9197, #9200, #9207.