api: Document /users/me/alert_words API endpoint. #22627
Conversation
akashaviator
force-pushed
the
alert_words
branch
from
274db15
to
1c6fc9b
Compare
There was a problem hiding this comment.
@akashaviator - Thanks for documenting these endpoints! I made a few comments and had a few questions.
There are a few places where I think it'd be good to note that "alert words" are also phrases, which I marked with comments.
As a bonus, would you also be up for updating the "alert_words" event in the /get-events endpoint documentation to be more similar to your description/phrasing for the alert_words response values? I think you might need to modify 'requesting user' to be 'user' and 'specified' to be 'configured', but other than that it should be good.
It'd probably be good to post screenshots of the rendered documentation after making these updates. Thanks!
zerver/openapi/zulip.yaml
Outdated
| description: | | ||
| List all of the requesting user's configured [alert words][alert-words]. | ||
|
|
||
| [alert words]: /help/pm-mention-alert-notifications#alert-words |
There was a problem hiding this comment.
The reference link here is broken. You need a dash instead of a space.
zerver/openapi/zulip.yaml
Outdated
| items: | ||
| type: string | ||
| example: ["foo", "bar"] | ||
| required: false |
There was a problem hiding this comment.
Is this an optional parameter? With curl in the dev environment I get this when I don't send the alert_words parameter ...
{
"code" : "REQUEST_VARIABLE_MISSING",
"msg" : "Missing 'alert_words' argument",
"result" : "error",
"var_name" : "alert_words"
}
There was a problem hiding this comment.
I think I missed it while copy-pasting the schema. I think this should ideally be checked by the tests, whether parameters marked as required: false return an error in case they are not provided in the request.
zerver/openapi/zulip.yaml
Outdated
| - name: alert_words | ||
| in: query | ||
| description: | | ||
| An array of strings to be removed from the requesting user's set of alert words. |
There was a problem hiding this comment.
Does it make sense to explicitly say that strings that are not in the user's set of alert words are ignored? Or is that implicit / not important since the updated list is sent in the response?
There was a problem hiding this comment.
I've included this information now.
zerver/openapi/zulip.yaml
Outdated
| summary: Remove alert words | ||
| tags: ["users"] | ||
| description: | | ||
| Remove words from the requesting user's set of [alert words][alert-words]. |
There was a problem hiding this comment.
I'd update to say, "Remove words (or phrases) from ...", since that's clearly stated in the help center documentation.
akashaviator
force-pushed
the
alert_words
branch
3 times, most recently
from
817e356
to
7814b55
Compare
|
@laurynmm Thanks for the review! I've updated the PR and added the screenshots in the description. |
akashaviator
force-pushed
the
alert_words
branch
from
7814b55
to
b4cd399
Compare
|
This is great, merged after adding a sentence about alert words being case-insensitive in a couple endpoint descriptions, and fixing one typo. |
This documents
/users/me/alert_wordsAPI endpoint.(UPDATED)
Screenshots:
GET /users/me/alert_words
POST /users/me/alert_words
DELETE /users/me/alert_words
GET /events