search

Inumedia / SlackAPI

.NET Implementation of the Slack team communication platform API.
MIT License
452 stars 243 forks source link

Add Group object and various Groups methods #1

Closed joncamp closed 9 years ago

joncamp commented 9 years ago

Fix channel.mark RequestPath bug Add is_channel to Channel object Update Response object to return reason-code as string Add filetype to UploadFile

Inumedia commented 9 years ago

So briefly looking at this, if I recall correctly, channels and groups are interchangeable and there's no downside to grouping them together as a single object. Has this changed since I first implemented channel support?

joncamp commented 9 years ago

Looking at a couple of the various methods, it depends. Some return back a group object such as https://api.slack.com/methods/groups.create, https://api.slack.com/methods/groups.createChild, https://api.slack.com/methods/groups.invite. Others return a channel object, such as https://api.slack.com/methods/groups.rename, return back a channel object.

Inumedia commented 9 years ago

The two other things that stand out are RPCMessages/GroupResponse.cs seemingly never being used, and account_inactive field being removed.

Re: Channel vs Group object, it looks like they resemble each other a lot, maybe some inheritance. I'd rather reduce the number of duplicate fields where possible for simplicity or at least having them easily interchangeable.

Re: groups.rename, it looks like it returns a partial view of the groups object since it contains the is_group = true field. This might fall back to the interchangeable thing where groups and channels are very similar, if not interchangeable.

Could you look into making channels / groups interchangeable? If not I'll look into it later this week. Once it's done or it's determined to not be a good idea to make them interchangeable, I'll pull it in.

Inumedia commented 9 years ago

See branch: https://github.com/Inumedia/SlackAPI/tree/joncamp-groups

If this looks alright to you and checks out, I'll pull it in.

joncamp commented 9 years ago

Yeah, seems reasonable - thanks!

Inumedia commented 9 years ago

Not a problem, and thanks for the help. If you have any other suggestions feel free to put them here or email me. :)