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

Create a way to remove default commands. #59

Open
dblock opened this issue Mar 21, 2016 · 22 comments
Open

Create a way to remove default commands. #59

dblock opened this issue Mar 21, 2016 · 22 comments

Comments

@dblock
Copy link
Collaborator

dblock commented Mar 21, 2016

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

@dramalho
Copy link
Contributor

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 ?

@dblock
Copy link
Collaborator Author

dblock commented Mar 23, 2016

👍 @dramalho Want to give it a shot?

@dramalho
Copy link
Contributor

: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
Copy link
Contributor

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

@dblock
Copy link
Collaborator Author

dblock commented Mar 23, 2016

What are we paying you for @dramalho ! :)

@dramalho
Copy link
Contributor

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

:)

FINE

I'll pick this up :)

@dblock
Copy link
Collaborator Author

dblock commented Mar 23, 2016

@zenmatt
Copy link

zenmatt commented Apr 20, 2016

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.

@dramalho
Copy link
Contributor

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?

@zenmatt
Copy link

zenmatt commented Apr 21, 2016

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?

@dblock
Copy link
Collaborator Author

dblock commented Apr 21, 2016

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
Copy link
Collaborator Author

dblock commented Jun 4, 2016

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

@dramalho
Copy link
Contributor

dramalho commented Jun 4, 2016

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, <
notifications@github.com> 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
.

@m5rk
Copy link

m5rk commented Jun 16, 2016

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

@gconklin
Copy link

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

@laertispappas
Copy link
Contributor

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?

@dblock
Copy link
Collaborator Author

dblock commented Jun 26, 2017

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

@chuckremes
Copy link
Collaborator

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
Copy link
Collaborator

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.

@dblock
Copy link
Collaborator Author

dblock commented Jul 8, 2017

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

@TallOrderDev
Copy link

@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.)

@lamtha
Copy link

lamtha commented Feb 15, 2019

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

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

No branches or pull requests

9 participants