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

Option to allow bot messages #50

Open
rubiesonthesky opened this issue Aug 13, 2015 · 11 comments
Open

Option to allow bot messages #50

rubiesonthesky opened this issue Aug 13, 2015 · 11 comments

Comments

@rubiesonthesky
Copy link

Would be nice if you could add option to allow bot messages to get thru to irc. I'm trying to use this for Jira notifications (and maybe github) via slack. :)

@ekmartin
Copy link
Owner

Definitely, looks like you're moving in the correct direction in your fork. Removing the subtype filtering completely will make it include channel joins/quits too though (see https://api.slack.com/events/message). I guess for now we're interested in the messages without a subtype and those with the subtype bot_message?

@rubiesonthesky
Copy link
Author

The bot crashes now because it doesn't get name for the bot. Bot's don't have userid, they have bot_id. Seems that the slack-client doesn't have API for bot yet but this for has https://github.com/guiceolin/node-slack-client. So either you would need to depency on this fork or wait official slack to accept the pull request (there's no commits after May). Not holding my breath for that.

@ekmartin
Copy link
Owner

There's a lot of open node-slack-client pull requests that slack-irc could use, so I'm afraid relying on a single of the forks wouldn't help much in the long term. Hopefully Slack will start giving it some more focus soon, otherwise I guess a fork that merged in some of the open pull requests (and maybe added a decent test suite, which is lacking in node-slack-client at the moment) could work.

@pmclanahan
Copy link

I'm doing this with some success in my fork by simply adding bot_message to ALLOWED_SUBTYPES, but I do get crashes and it may be related to this. Anyone know of updates from node-slack-client?

@rubiesonthesky
Copy link
Author

Yes I think it's the same problem. The bot crashes because it doesn't get all needed arguments from node-slack-client. I haven't checked have they updated their code since August.

@pmclanahan
Copy link

Looks like there's a beta 2.0 version out. Not sure yet if it fixes this though.

@ekmartin
Copy link
Owner

Yep @pmclanahan, rewrote slack-irc to use version 2 a while back, but there was some stuff that needed to be fixed first. Think it should be good to go now though, so gonna take a look at it soon.

@pmclanahan
Copy link

Cool. Thanks for the update!

@ekmartin
Copy link
Owner

ekmartin commented Apr 5, 2016

Sorry for the delay, but slack-irc has now been released with version 2 of node-slack-client (slack-irc v3.7.8).

This means that this issue is solvable, but I'm a bit unsure about the execution in terms of backwards compatibility. The most straightforward solution would of course just be to add another config option, something like muteBotMessages, and default it to true.

However, there already is an option named muteSlackbot, which defaults to false. I feel like it might be a bit overkill to have both of these options, and that they could be merged to one - e.g. like muteSlackBots. I guess you could consider this as breaking backwards compatibility though - even if you preserve muteSlackbot as a deprecated alias, as it would result in slack-irc starting to forward bot messages to IRC (which hasn't happened previously) - but it's a bit of a grey area.

@iandennismiller
Copy link

I've been trying to get this working too. According to the documentation, it appears the bot specifies a username.

https://api.slack.com/events/message/bot_message

{
    "type": "message",
    "subtype": "bot_message",
    "ts": "1358877455.000010",
    "text": "Pushing is the answer",
    "bot_id": "BB12033",
    "username": "github",
    "icons": {}
}

Within node-slack-sdk, I think that value is available to the process when the message subtype is bot_message. Thus, in bot.js sendToIRC() I've added code to handle the bot_message subtype:

      } else if (message.subtype === 'bot_message') {
        text = `Bot: ${username} ${text}`;
      }

However, upon reading this thread, it sounds to me like we are waiting for guidance on a configuration option, and I realize I'd better not proceed until you've made a choice about the semantics of muting. Would you mind picking a config flag for the botting behavior?

My thoughts on the matter:

  • make a new option called muteBots. The default for muteBots is true, so old configurations do not unexpectedly begin mirroring bot messages.
  • Deprecate muteSlackbot and make it an alias for muteBots.

There is now one case breaking backwards compatibility: among users who had unmuted Slackbot (i.e. muteSlackbot is false), then when this becomes an alias for muteBots it will still permit Slackbut but additionally permit other bots, which changes the behavior in an unexpected way with old, unmodified configurations. I'm not sure about your numbering policy, but this may warrant a version bump.

@guruofquality
Copy link

Hey all, just wanted to share my modification to slack-irc to support bot messages. So I setup github notifications to one of my slack channels and I needed the messages from github to make it into the irc channel. I made the following changes:

const ALLOWED_SUBTYPES = ['me_message', 'bot_message'];

This allows messages from bots to be handled by sendToIRC()

else if (message.subtype === 'bot_message' && message.attachments && message.attachments[0] && message.attachments[0].fallback) {
text = \x0303${message.attachments[0].fallback.replace(/\|/g, " ")}\x03;

This is a bit of a one-off change, but bots might not have text. They might use attachments which use fallback text for chat display purposes.

And this replace(/\|/g, " ") replaces the bar in the "url|title" format so that links sent to irc are click-able. Otherwise the bar messes up the url.

else if (message.subtype === 'bot_message') {
return;

And this line prevents the echoing of our own bot messages from slack-irc since we now inspect bot_message types. Not very crafty, but I was only interested in the messages with attachments, so this works for me to stop echos. But it could be smarter.

Anyway, I just wanted to bring up the case of supporting attachments from a slack based bot and what sort of changes might be needed. Perhaps its a use-case thats worth supporting. The full diff is below:

diff --git a/lib/bot.js b/lib/bot.js
index 9c8ecbf..29ae31c 100644
--- a/lib/bot.js
+++ b/lib/bot.js
@@ -7,7 +7,7 @@ import emojis from '../assets/emoji.json';
 import { validateChannelMapping } from './validators';
 import { highlightUsername } from './helpers';

-const ALLOWED_SUBTYPES = ['me_message'];
+const ALLOWED_SUBTYPES = ['me_message', 'bot_message'];
 const REQUIRED_FIELDS = ['server', 'nickname', 'channelMapping', 'token'];

 /**
@@ -209,6 +209,10 @@ class Bot {
         text = `<${user.name}> ${text}`;
       } else if (message.subtype === 'me_message') {
         text = `Action: ${user.name} ${text}`;
+      } else if (message.subtype === 'bot_message' && message.attachments && message.attachments[0] && message.attachments[0].fallback) {
+        text = `\x0303${message.attachments[0].fallback.replace(/\|/g, " ")}\x03`;
+      } else if (message.subtype === 'bot_message') {
+        return;
       }
       logger.debug('Sending message to IRC', channelName, text);
       this.ircClient.say(ircChannel, text);

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

5 participants