Add channel members API #34
Conversation
lib/rocket_chat/messages/channel.rb
Outdated
| # @return [Room[]] | ||
| # @raise [HTTPError, StatusError] | ||
| # | ||
| def members(room_id: nil, name: nil, offset: nil, count: nil, sort: nil, fields: nil, query: {}) |
There was a problem hiding this comment.
I added room_id and name as first arguments for this method, I think wold be consistent with the other methods in this file.
This is a method which is also a list and the arguments offset: nil, count: nil, sort: nil, fields: nil, query: {} make sense.
I'm not sure about the fields looking at the RocketChat API docs this is not supported, I added it for consistency.
We could also omit it, and use {} or nil when building the params for list build_list_body(offset, count, sort, {}, query).
I noticed that fields is required build_list_body method.
I'm having doubts on what is the best approach, @abrom could you please advise when you have the chance?
Thank you 🙇
There was a problem hiding this comment.
Correct, the API would support both the room ID and name.
The API does not support either fields or query parameters so please remove these. We wouldn't want someone to think that because the method accepts these parameters that the API does too!
If you just pass through the fields value as nil it will not be included.
Given that the groups.members API also supports the same parameters and returns the same response I would suggest you put this API code in the parent class Room. That way it will be available to both. As per the other methods in the Room class you'd pass the URI for the API through using the api_path helper: self.class.api_path('members')
There was a problem hiding this comment.
@abrom Thank you for the feedback 🙇♀️ I applied your suggestions.
alinavancea
force-pushed
the
am-add-channel-members-api
branch
from
98097eb to
b16f0ca
Compare
lib/rocket_chat/messages/channel.rb
Outdated
| # @return [Room[]] | ||
| # @raise [HTTPError, StatusError] | ||
| # | ||
| def members(room_id: nil, name: nil, offset: nil, count: nil, sort: nil, fields: nil, query: {}) |
There was a problem hiding this comment.
Correct, the API would support both the room ID and name.
The API does not support either fields or query parameters so please remove these. We wouldn't want someone to think that because the method accepts these parameters that the API does too!
If you just pass through the fields value as nil it will not be included.
Given that the groups.members API also supports the same parameters and returns the same response I would suggest you put this API code in the parent class Room. That way it will be available to both. As per the other methods in the Room class you'd pass the URI for the API through using the api_path helper: self.class.api_path('members')
alinavancea
force-pushed
the
am-add-channel-members-api
branch
from
70800de to
4eae9ef
Compare
|
FYI this has been released in v0.1.20 |
|
Thank you @abrom 🙇♀️ |
Issue #33
Summary of the changes
Add
membersmethod to underchannel.rbAdd docs
Add
rspectestsrspec is passing
rubocop has some offenses