slack-ruby / slack-ruby-bot

The easiest way to write a Slack bot in Ruby.
MIT License
1.12k stars 187 forks source link

Where can i find more info on the client.say method? #259

Open dsusviela opened 4 years ago

dsusviela commented 4 years ago

Hey evreyone, i'm using this gem for a personal project, nothing too fancy.

I'm stuck trying to get the bot send a message to the general channel when a "broadcast" command is invoked. This is my code snippet:

class Broadcast < SlackRubyBot::Commands::Base
  def self.call(client, data, _match)
    subs = Subscription.all()
    subs.each do |sub|
      client.say(channel: "#general", text: "<@#{sub.user_id}>")
    end
  end
end

But this doesn't seem to work. On the other hand if i simply put the channel id, it works like a charm. Any clues on what I'm doing wrong? Also, what other methods do i have access to with client? I didn't find anything in the readme.

dblock commented 4 years ago

The say method is implemented here and is a wrapper on message which itself calls send_json which itself calls send_data to the slack connected real-time socket.

What you're looking for is slack RTM docs on sending message, which explains the requirements for doing so.

Other methods are documented in RTM parts of slack-ruby-client.

I would greatly appreciate any README contributions that do a good job at linking/explaining this!

dsusviela commented 4 years ago

Sorry it took me a while to get back to you. I looked at the implementation of the say method prior to making this issue but didnt find the message method, so thank you very much on that clarification.

Just a follow up question, my app was actually created through the steps of the slack-ruby-server gem, because I didn't want to have a legacy app. However if i got you right i'm still using the "old" RTM API with the say method, right?

dblock commented 4 years ago

Just a follow up question, my app was actually created through the steps of the slack-ruby-server gem, because I didn't want to have a legacy app. However if i got you right i'm still using the "old" RTM API with the say method, right?

yes

dsusviela commented 4 years ago

Hey its been a while. I've started writing something for the readme on my forked repo, and I've taken the liberty to create an index and i was wondering if that's okay or if its outside of the scope of what you're looking for.

Also, this issue made me delve deeper into the code of the gem trying to figure out whats going on. I noticed that SlackRubyBot's client inherits from Slack::Realtime::Client so we have access to some parts of the API namely message, message_id, ping, and typing which is fine. However Slack::Realtime::Client has an attribute named web_client which is basically an instance of Slack::Web::Client which in turn grants me access to the API::Endpoints. Therefore i can change the above snippet to the following to achieve my desired result:

class Broadcast < SlackRubyBot::Commands::Base
  def self.call(client, data, _match)
    channels = client.web_client.channels_list
    # parse the response and get the 'general' ch id
    subs = Subscription.all()
    subs.each do |sub|
      client.say(channel: general_channel_id, text: "<@#{sub.user_id}>")
    end
  end
end

Is this intentional? Am i supposed to use the web_client attribute?

dblock commented 4 years ago

Hey its been a while. I've started writing something for the readme on my forked repo, and I've taken the liberty to create an index and i was wondering if that's okay or if its outside of the scope of what you're looking for.

You mean a TOC? You should add https://github.com/dblock/danger-toc and it will auto-generate and enforce one for you.

Also, this issue made me delve deeper into the code of the gem trying to figure out whats going on. I noticed that SlackRubyBot's client inherits from Slack::Realtime::Client so we have access to some parts of the API namely message, message_id, ping, and typing which is fine. However Slack::Realtime::Client has an attribute named web_client which is basically an instance of Slack::Web::Client which in turn grants me access to the API::Endpoints. Therefore i can change the above snippet to the following to achieve my desired result:

class Broadcast < SlackRubyBot::Commands::Base
  def self.call(client, data, _match)
    channels = client.web_client.channels_list
    # parse the response and get the 'general' ch id
    subs = Subscription.all()
    subs.each do |sub|
      client.say(channel: general_channel_id, text: "<@#{sub.user_id}>")
    end
  end
end

Is this intentional? Am i supposed to use the web_client attribute?

This is intentional. We hold the token that lets you call the Web API, so .web_client makes it easy. So in your case you call a web api to get a list fo channels, and it's A-OK.

Note that say sends the data over an already open websocket, while web_client will open a new connection every time you call an API (reuse is addressed in https://github.com/slack-ruby/slack-ruby-client/pull/322), which makes it is significantly slower.

dblock commented 4 years ago

One more note, channels_list is a paginated API and in the example above you only get page 1 that might not contain #general. If you do paginate, depending on how many channels you have this is very slow.

The web API web_client.chat_postMessage supports '#general' AFAIK, so I would rewrite the code above to do web_client.chat_postMessage(channel: '#general', ...) and I think that's that.

dsusviela commented 4 years ago

You mean a TOC? You should add https://github.com/dblock/danger-toc and it will auto-generate and enforce one for you.

Yes! That's exactly what i meant.

Thank you so much, everything is clearer now :)