search

hedwig-im / hedwig_slack

Slack Adapter for Hedwig
MIT License
45 stars 30 forks source link

Add Web API and handle responder identity #10

Closed rjanja closed 7 years ago

rjanja commented 7 years ago

Hi, this is a first go at using the Slack Web API to allow for responders to define identities (custom name and emoji/thumbnails) they will use when sending messages. There are some limits based on spotty (buggy) areas of the Slack API but for most things it works well. Additionally, for emotes, since the RTC doesn't support message formatting, I've switched out the use of it for the Web API.

There is an accompanying changeset to the base Hedwig package as well.

scrogson commented 7 years ago

Hi @rjanja,

All of these changes seem like they would be better suited else where. The adapter for Slack is designed to send messages over the WebSocket connection, not over HTTP.

You could move the HTTP functionality into its own package and use it from within your responders, instead of sending the message via the Hedwig.Responder.{send,reply,emote} functions.

I would be interested in a PR that brings in the WebAPI module similar to how you have in this PR. If that sounds like something you would be interested in that would be awesome!

If you do decide to send a PR for that, please do not replace :hackney with HTTPoison.

Thank you!

rjanja commented 7 years ago

Hi @scrogson, thanks for the feedback and for all your work on the hedwig projects.

The WebSocket (RTM) API lacks message formatting support which makes it less useful and interesting to us. That said, there may be merit to having the Web API fully supported by a separate adapter instead of intermingling both APIs as I have done.

Are you suggesting that the Web API might fit within this adapter but be called directly from individual responders? I've tried to keep everything separate and independent thus far, but I do see a need to access the Web API directly instead of injecting message properties to trigger those calls (for things like adding reactions to messages or working with message attachments).

I'm happy to contribute back to this project, and I'm hoping our goals of easily integrating more advanced Slack features into bots will fit within the aims of your project. I'm open to suggestions and as much feedback as you care to provide; this has so far been an excellent learning experience.

I did struggle with the decision to replace :hackney. It wasn't a necessity but a preference, so that's not a big deal to leave as-is.

One thing that wasn't present in this PR but I can see value in is documenting the shortcomings of the APIs (both Web and RTM). Some things, like the emote (me_message) not working as intended are not obvious until after doing a bit of research. I'm also happy to help with this effort.