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

feat: Channel and Group #kick APIs #39

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lucasthomazoni
Copy link

Added:

@abrom Since both API's do not allow user#username and room#name, I'm sending it as nil on user_params and room_params respectively.

@abrom
Copy link
Owner

abrom commented Oct 9, 2022

The kick APIs are also already supported by the gem. See https://github.com/abrom/rocketchat-ruby/blob/main/lib/rocket_chat/messages/room.rb#L200-L207

@abrom
Copy link
Owner

abrom commented Oct 9, 2022

Still happy to accept a PR improving the docs and specs, although another spec or two to test the username/room name options would be nice (and removing the channel/group kick methods).

@lucasthomazoni
Copy link
Author

@abrom Sorry I was looking at https://developer.rocket.chat/ docs instead of the actual code on GitHub and there is no mention of the user#username param.

I will undo the changes made and improve specs as well, just one unrelated question: Why are custom_fields not marked as settable attributes on channel and group APIs? Can I add it?

Thanks!

@abrom
Copy link
Owner

abrom commented Oct 12, 2022

No problems! Yeah the rocketchat docs are a little misleading on that front. I had to go hunting through the code base to find the real story 😉

Regarding custom fields, are you referring to https://developer.rocket.chat/reference/api/rest-api/endpoints/core-endpoints/channels-endpoints/setcustomfields ? (and similar for groups of course)

If so, it looks like that also supports finding the channel by ID or name (same as kick and others). See https://github.com/RocketChat/Rocket.Chat/blob/404cfffe3fcee219579efce47ee0cab114162c69/apps/meteor/app/api/server/v1/channels.js#L692-L712

But yes, absolutely. PR's welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants