zulip icon indicating copy to clipboard operation
zulip copied to clipboard

api: Add User status endpoint.

Open MSurfer20 opened this issue 3 years ago • 3 comments

Testing plan:

GIFs or screenshots:

MSurfer20 avatar Aug 04 '21 10:08 MSurfer20

@timabbott Could you give this a look?

MSurfer20 avatar Aug 04 '21 22:08 MSurfer20

Thanks for working on this @MSurfer20, and sorry for the very slow review! I merged 356e6e50185cadb12c5a7e10a5ff8936da4b632b as a prep commit to avoid code duplication, and pushed back here (after also squashing the tests into the feature commit). I have a few additional requests:

  • What's the thinking on having a status object in the response rather than including the values on the top-level response? Can you start a thread in #api design about what the right design is here? I'm not sure what's correct myself.
  • In the API documentation, I think we don't need **Changes**: New in Zulip 5.0 (feature level 86). type records for fields; we just need a top-level Changes entry in the description for the endpoint being new. That said, I think it'd be reasonable to share the endpoint response description with the other places we have a Status object, and it's OK if we have text about older feature levels as a result.
  • I also switched this to use access_user_by_id, since that's our standard method for this sort of check. It seems possible we should also add a commit to use make get_presence_backend use that method as well as a small code cleanup.

timabbott avatar Sep 02 '21 23:09 timabbott

Heads up @MSurfer20, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Jun 01 '22 23:06 zulipbot

Closing in favor of #30059, which completes this effort, thanks @MSurfer20!

timabbott avatar May 23 '24 00:05 timabbott