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

Loops with ephemeral messages #226

Open
sihu opened this issue Jun 24, 2019 · 3 comments
Open

Loops with ephemeral messages #226

sihu opened this issue Jun 24, 2019 · 3 comments
Labels

Comments

@sihu
Copy link

sihu commented Jun 24, 2019

I noticed that my Bot had problems with ephemeral messages, in particular with unfolding URLs.

How it can be reproduced

  1. bot posts an URL to a channel
  2. slackbot unfolds it
  3. bot posts the same URL to the channel
  4. slackbot sends message: "Pssst! I didn\u2019t unfurl <https:\/\/example.com> because it was already shared in this channel quite recently (within the last hour) and I didn\u2019t want to clutter things up."
  5. my bot responds with a Sorry @slackbot, I don't understand that command!
  6. now slackbot responds that he didn't understand
  7. ...there is a pingpong

Possible solution

The message hook should filter out those ephemeral messages. I think it will be something like:

lib/slack-ruby-bot/hooks/message.rb:

module SlackRubyBot
  module Hooks
    class Message
      def call(client, data)
        return if message_to_self_not_allowed? && message_to_self?(client, data)
        return if ephemeral_message?(data) # this would be new
        data.text = data.text.strip if data.text
        result = child_command_classes.detect { |d| d.invoke(client, data) }
        result ||= built_in_command_classes.detect { |d| d.invoke(client, data) }
        result ||= SlackRubyBot::Commands::Unknown.tap { |d| d.invoke(client, data) }
        result
      end

      ...

      private

      def  ephemeral_message?(data)
          data.is_ephemeral
          # and you could also check for the data.subtype == 'bot_message'
      end

I would be willing to contribute this fix if you like the solution.

@dblock
Copy link
Collaborator

dblock commented Jun 25, 2019

That seems consistent with what I saw. Would love a PR with proper README/UPGRADING documentation and an option to override this behavior just like for bot messages.

@dblock dblock added the bug? label Jun 25, 2019
@sihu
Copy link
Author

sihu commented Jun 25, 2019

@dblock that's fine! I have two open questions before i start:

  1. you said override this behavior just like for bot messages., did you mean, just like message_to_self?
  2. i asked myself what the check should include. Should it never respond to ephemeral messages or just not to ephemeral messages from bots? Or should it not reply to bots at all? This could bring some breaking changes. An alternative solution could also be to catch it in SlackRubyBot::Commands::Unknown ... so an unknown-message should never go to a bot.

@dblock
Copy link
Collaborator

dblock commented Jun 26, 2019

  1. I think that yes, that's what I meant. But really I only care about being able to turn off the behavior you're introducing.
  2. Both? Either? Make it configurable and let people decide!

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

No branches or pull requests

2 participants