slack-ruby-bot icon indicating copy to clipboard operation
slack-ruby-bot copied to clipboard

Create a way to remove default commands.

Open dblock opened this issue 9 years ago • 22 comments

See https://github.com/dblock/slack-ruby-bot/pull/58.

dblock avatar Mar 21 '16 21:03 dblock

Well, maybe we could follow the exact pattern we do for hooks now. Have a command set with a default initialization and remove the magic command discovery ?

dramalho avatar Mar 23 '16 08:03 dramalho

:+1: @dramalho Want to give it a shot?

dblock avatar Mar 23 '16 12:03 dblock

:P replicate the hook work ....... well, not a pressing issue for me, but sure, I can take one for the team. You know it will probably take me three weeks again :P

dramalho avatar Mar 23 '16 12:03 dramalho

(kidding, should be quicker, making a lot of assumptions on a parallel approach to the hook set)

dramalho avatar Mar 23 '16 12:03 dramalho

What are we paying you for @dramalho ! :)

dblock avatar Mar 23 '16 12:03 dblock

GOD DAMN IT I WANT TO BE RICH DOING NOTHING, THIS WORK THING CRAMPS MY STYLE!!!!

:)

FINE

I'll pick this up :)

dramalho avatar Mar 23 '16 12:03 dramalho

dblock avatar Mar 23 '16 12:03 dblock

What is the best way to override a default command class? I was trying to override Unknown and it worked fine in dev, but in production, commands that had a class/match began matching Unknown instead. It was almost like the Unknown route suddenly had a higher precedence than other command routes.

zenmatt avatar Apr 20 '16 21:04 zenmatt

Sorry, I've been swamped and I haven't come back to refactoring this, however :)

take a look at the message.rb hook file, that's the one that calls the command classes and what it does is

      def call(client, data)
        return if message_to_self_not_allowed? && message_to_self?(client, data)
        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

so there's precedence, anything in the child_command_classes list that matches comes first. Unknown is a last ditch call.

My very long shot guess is you've created a command class that falls in the child_command_classes list and it's matching every message?

dramalho avatar Apr 20 '16 21:04 dramalho

Thank you for response, I think your guess is right. I failed to override Unknown properly and it was being detected in the child_command_classes and matching before other classes. Weirdly enough it must have been matching last in dev, but precedence got moved up in production so other commands were being ignored.

What do you think is the best approach to handle this in the short term before this issue gets addressed?

zenmatt avatar Apr 21 '16 16:04 zenmatt

I'd love it if everyone here spent a bit of time refactoring my mess into something better instead of hacking on top of it :)

dblock avatar Apr 21 '16 21:04 dblock

@dramalho Still would love you to take a stab at this! Thanks.

dblock avatar Jun 04 '16 12:06 dblock

I know, I got into a freaking work cycle and a bunch of things got left behind - my conscience however is heavy😢

this problem is slightly different from the hooks one, that approach may or may not be ideal. I remember thinking up that much 😁

On Sat, 4 Jun 2016, 13:26 Daniel Doubrovkine (dB.) @dblockdotorg, < [email protected]> wrote:

@dramalho https://github.com/dramalho Still would love you to take a stab at this! Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dblock/slack-ruby-bot/issues/59#issuecomment-223752969, or mute the thread https://github.com/notifications/unsubscribe/AAA-Md-s1rHZLlifhre2YOiOw3fhyv_Bks5qIW7wgaJpZM4H1mPT .

dramalho avatar Jun 04 '16 19:06 dramalho

I was able to override the built-in Unknown class simply by monkey patching it.

m5rk avatar Jun 16 '16 14:06 m5rk

This is how I monkey patched the defaults. Might help someone else in the mean-time. Not the greatest if more defaults are introduced, but works for the version I've been using.

File: commands/hide_defaults.rb

module MyBot
  module Commands
    class Unknown < SlackRubyBot::Commands::Base
      match(/^(?<bot>\S*)[\s]*(?<expression>.*)$/)

      def self.call(client, data, _match)
      end
    end

    class About < SlackRubyBot::Commands::Base
      command 'about', 'hi', 'help' do |client, data, match|
      end
    end
  end
end

gconklin avatar Oct 26 '16 00:10 gconklin

How about adding a config flag for now in order to control the built in command invocation and refactor the tightly coupled components on a later time?

laertispappas avatar Jun 24 '17 16:06 laertispappas

I don't love this idea, might as well monkey patch. I'll take a clean fix though.

dblock avatar Jun 26 '17 15:06 dblock

Ha, this is similar to my desire to "mute" the built-in commands. Now that we have the permitted? method in Commands::Base I am working on a separate authorization gem that will make it easy to disable the built-in commands. That's how I'm tackling it.

When I release the gem (this week?) I'll drop a note in here with a link to it.

chuckremes avatar Jun 26 '17 16:06 chuckremes

So I implemented a whitelist authorization framework. I haven't published it as a gem yet but a complete and working repository is available here: https://github.com/stax-dog/slack_ruby_bot_authorization

If you require this in your project then it, by default, DENIES all command requests. You must explicitly setup roles and add users and commands to those roles. This includes the builtin commands such as hi, about, and help.

chuckremes avatar Jul 07 '17 19:07 chuckremes

@chuckremes If you want that repo in the slack-ruby org we can make it happen!

dblock avatar Jul 08 '17 13:07 dblock

@gconklin I've added the file and the code you posted.

It doesn't seem to be working, and I'm guessing it's because I don't have it tied in correctly.

Are there any other steps beside add folder/file, add code, save, "rails s" in console.

Thanks in advanced! (sorry to add clutter to an issue.)

TallOrderDev avatar Jul 20 '17 22:07 TallOrderDev

This worked for me, it proxies to the 'MySlackBot' help block.

  module Commands
    class Unknown < SlackRubyBot::Commands::Base
      match(/^(?<bot>\S*)[\s]*(?<expression>.*)$/)
      
      def self.call(client, data, _match)
        MySlackBot.slack_response(client, "#{SlackRubyBot::Commands::Support::Help.instance.bot_desc_and_commands.join('\n')}", data.channel)
      end

    end

    class About < SlackRubyBot::Commands::Base
      command 'about', 'hi', 'help' do |client, data, match|
      end

    end

  end

lamtha avatar Feb 15 '19 23:02 lamtha