api: Refactor GET /users endpoint. #546
Conversation
akashaviator
force-pushed
the
get-user
branch
2 times, most recently
from
b7aed01
to
57b0226
Compare
zulip/zulip/__init__.py
Outdated
| ''' | ||
| Example usage: | ||
|
|
||
| >>> client.get_member(8,{'include_custom_profile_fields': True}) |
There was a problem hiding this comment.
I'd put a space after the , here, as that's standard Python style. Otherwise lgtm.
|
@timabbott added the space after |
zulip/zulip/__init__.py
Outdated
| @@ -1076,6 +1076,21 @@ def delete_stream(self, stream_id): | |||
| method='DELETE', | |||
| ) | |||
|
|
|||
| def get_member(self, user_id, request=None): | |||
There was a problem hiding this comment.
Oh, let's call this get_user_by_id, not get_member. That's more consistent naming with the rest of the product.
(And I think we'll want to rename get_members to get_users)
zulip/zulip/__init__.py
Outdated
| {'result': 'success', 'msg': '', 'user': [{...}, {...}]} | ||
| ''' | ||
| return self.call_endpoint( | ||
| url='users/'+str(user_id), |
There was a problem hiding this comment.
We generally do space around operators like +; see PEP-8 which we follow on this.
But probably it's better to do string formatting, rather than +, here.
akashaviator
force-pushed
the
get-user
branch
3 times, most recently
from
d90ffe4
to
eca22c7
Compare
|
@timabbott made those changes. I was trying to rename examples/list-members to list-users , but got "conflicting files" warning here. |
|
I merged the first commit. For the rename commit, we should make the old function name work for backwards-compatibility. Otherwise, any existing API clients will break on upgrade. (And once we merge that and do a release, we should make a PR to update the documentation) |
|
@timabbott So do you mean that I should create a new wrapper for the same endpoint, named as get_users ? |
|
I was suggesting we rename (That would be more clear about which is the preferred option, the one that's not a wrapper). |
akashaviator
changed the title
|
@timabbott I've updated the PR. |
| ''' | ||
| return self.call_endpoint( | ||
| url='users', | ||
| method='GET', | ||
| request=request, | ||
| ) | ||
|
|
||
| def get_members(self, request: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: | ||
| return self.get_users(request=request) |
There was a problem hiding this comment.
Let's add a comment that this is a backwards-compatibility name. I can take care of it before merging.
|
Merged after adding the comment discussed above, thanks @akashaviator! We'll probably want to do similar renames to the other items we wanted to change in @orientor's |
This adds binding for
GET ../users/{id}endpoint.