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

api-docs: Document bulk mark as read endpoints as deprecated. #27349

Merged

Conversation

laurynmm
Copy link
Collaborator

Adds a deprecated changes note to the three bulk mark as read endpoints in the API documentation and removes the out of date link/reference to the bulk mark messages as read endpoints in the main description for the update personal message flags endpoint.

See CZO discussion.

Part of work on #23598.

Screenshots and screen captures:

Mark messages in topic as read endpoint main description

Current documentation
Screenshot from 2023-10-24 16-31-07

Mark messages in stream as read endpoint main description

Current documentation
Screenshot from 2023-10-24 16-31-03

Mark all as read endpoint main description

Current documentation
Screenshot from 2023-10-24 16-30-55


Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@zulipbot
Copy link
Member

Hello @zulip/server-api members, this pull request was labeled with the "area: documentation (api and integrations)" label, so you may want to check it out!

@gnprice
Copy link
Member

gnprice commented Oct 24, 2023

LGTM modulo Tim's comments above. Thanks @laurynmm for this cleanup!

Previously, we had checked that deprecated parameters and return
values had been marked as `deprecated: true` in the OpenAPI
documentation and had a description with a deprecated note.

Here we extend that check at the top level to deprecated endpoints.

The backend test that catches a failed assertion for this check
is `test_api_doc_endpoints` in zerver/tests/test_docs.py as that
test checks for a success response all pages linked the sidebar of
the API documentation.
@laurynmm laurynmm force-pushed the api-docs-deprecate-bulk-mark-as-read-endpoints branch from 4c37306 to a9805d9 Compare October 25, 2023 15:16
@zulipbot zulipbot added size: M and removed size: S labels Oct 25, 2023
@laurynmm
Copy link
Collaborator Author

Updates:

  • Added deprecated: true to these endpoints/paths in the OpenAPI spec, as well as the other deprecated endpoint api/mute-topic.
  • Extended the assertion check we use for parameters and return values in the OpenAPI spec to also be done at the top level of the endpoint.

A follow-up to these changes may be to add a stronger visual indication that the endpoint is deprecated in the rendered documentation. For example, here's the deprecated use_first_unread_anchor parameter in get-messages: https://zulip.com/api/get-messages#parameter-use_first_unread_anchor.

@timabbott timabbott merged commit f755fd1 into zulip:main Oct 25, 2023
7 checks passed
@timabbott
Copy link
Sponsor Member

Merged, thanks @laurynmm!

@laurynmm laurynmm deleted the api-docs-deprecate-bulk-mark-as-read-endpoints branch October 31, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants