Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ First you need to create a Discord bot user, which you can do by following the i
// Makes the bot hide the username prefix for messages that start
// with one of these characters (commands):
"commandCharacters": ["!", "."],
"ircStatusNotices": true // Enables notifications in Discord when people join/part in the relevant IRC channel
"ircStatusNotices": true, // Enables notifications in Discord when people join/part in the relevant IRC channel
// List of webhooks per channel
"webhooks": {
"#discord": "https://discordapp.com/api/webhooks/id/token"
}
}
]
```
Expand All @@ -94,6 +98,15 @@ The `ircOptions` object is passed directly to irc-upd ([available options](https

To retrieve a discord channel ID, write `\#channel` on the relevant server – it should produce something of the form `<#1234567890>`, which you can then use in the `channelMapping` config.

### Webhooks
Webhooks allow nickname and avatar override, so messages coming from IRC will appear almost as regular Discord messages.

See [here (part 1 only)](https://support.discordapp.com/hc/en-us/articles/228383668-Intro-to-Webhooks) to create a webhook for a channel.

Example result:

![discord-webhook](http://i.imgur.com/lNeJIUI.jpg)

## Tests
Run the tests with:
```bash
Expand Down
61 changes: 60 additions & 1 deletion lib/bot.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Bot {
this.channels = _.values(options.channelMapping);
this.ircStatusNotices = options.ircStatusNotices;
this.announceSelfJoin = options.announceSelfJoin;
this.webhookOptions = options.webhooks;

// "{$keyName}" => "variableValue"
// author/nickname: nickname of the user who sent the message
Expand Down Expand Up @@ -65,6 +66,7 @@ class Bot {
this.channelUsers = {};

this.channelMapping = {};
this.webhooks = {};

// Remove channel passwords from the mapping and lowercase IRC channel names
_.forOwn(options.channelMapping, (ircChan, discordChan) => {
Expand All @@ -79,6 +81,16 @@ class Bot {
logger.debug('Connecting to IRC and Discord');
this.discord.login(this.discordToken);

// Extract id and token from Webhook urls and connect.
_.forOwn(this.webhookOptions, (url, channel) => {
const [id, token] = url.split('/').slice(-2);
const client = new discord.WebhookClient(id, token);
this.webhooks[channel] = {
id,
client
};
});

const ircOptions = {
userName: this.nickname,
realName: this.nickname,
Expand Down Expand Up @@ -258,7 +270,9 @@ class Bot {
sendToIRC(message) {
const author = message.author;
// Ignore messages sent by the bot itself:
if (author.id === this.discord.user.id) return;
if (author.id === this.discord.user.id ||
Object.keys(this.webhooks).some(channel => this.webhooks[channel].id === author.id)
) return;

const channelName = `#${message.channel.name}`;
const ircChannel = this.channelMapping[message.channel.id] ||
Expand Down Expand Up @@ -335,6 +349,38 @@ class Bot {
return null;
}

findWebhook(ircChannel) {
const discordChannelName = this.invertedMapping[ircChannel.toLowerCase()];
return discordChannelName && this.webhooks[discordChannelName];
}

getDiscordAvatar(nick, channel) {
const guildMembers = this.findDiscordChannel(channel).guild.members;
const findByNicknameOrUsername = caseSensitive =>
(member) => {
if (caseSensitive) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are discord nicknames case sensitive or insensitive? Can there be two users with the same username/nickname with different casing? I think we can simplify this logic as long as we know what assumptions we're making.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insensitive and yes, which is why I suggested (I think I suggested?) matching case sensitively first and then falling back to case insensitive if there's no match found. (Since IRC users have no way to, in a dropdown, select the correct one of the various case sensitivities – Discord users have this ability since Discord translates the mention in its UI – I thought it best to allow them to force the right one by picking the right case, where relevant.)

Copy link
Contributor Author

@Fiaxhs Fiaxhs Aug 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can even have two users with the exact same nickname (same case): http://i.imgur.com/ACuYlcS.png + http://i.imgur.com/oD3JEfK.png
(And @maorninja suggested the change :) )

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Yes but I suggested here the logic for how to implement it. :P)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( right :D )

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would blame Discord because they allow 2 users with the exact same nickname, but that's just my opinion. I wish there was a bot that could block that.

return member.user.username === nick || member.nickname === nick;
}
const nickLowerCase = nick.toLowerCase();
return member.user.username.toLowerCase() === nickLowerCase
|| (member.nickname && member.nickname.toLowerCase() === nickLowerCase);
};

// Try to find exact matching case
let users = guildMembers.filter(findByNicknameOrUsername(true));

// Now let's search case insensitive.
if (users.size === 0) {
users = guildMembers.filter(findByNicknameOrUsername(false));
}

// No matching user or more than one => no avatar
if (users && users.size === 1) {
return users.first().user.avatarURL;
}
return null;
}

sendToDiscord(author, channel, text) {
const discordChannel = this.findDiscordChannel(channel);
if (!discordChannel) return;
Expand Down Expand Up @@ -390,6 +436,19 @@ class Bot {
return match;
});

// Webhooks first
const webhook = this.findWebhook(channel);
if (webhook) {
logger.debug('Sending message to Discord via webhook', withMentions, channel, '->', `#${discordChannel.name}`);
const avatarURL = this.getDiscordAvatar(author, channel);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During netsplits author is nil. This would usually send some message like <${username}> ${message} (or something similar, I can't recall. but the message would contain ${} sent verbatim). Now this.getDiscordAvatar crashes with TypeError: Cannot read property 'toLowerCase' of undefined

webhook.client.sendMessage(withMentions, {
username: author,
text,
avatarURL
}).catch(logger.error);
return;
}

Copy link
Collaborator

@Throne3d Throne3d Jul 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… I also failed to notice that this seems to discard the custom formatting when sending with a webhook; I think the 'Webhooks first' code should go after the const withAuthor… bit, here, and use withAuthor instead of withMentions.

Edit: Or, rather… make a new pattern for webhook messages?

Edit2: … I'm not sure. Uh. It seems plausibly useful to be able to change the formatting for a webhook message, if people were changing the withAuthor thing drastically. Maybe. But defeats the point of a webhook a little. Uh. Maybe ignore this. We can address it later if anybody complains. (Above thing should still use nicknames and maybe case insensitively check.)

patternMap.withMentions = withMentions;

// Add bold formatting:
Expand Down
79 changes: 79 additions & 0 deletions test/bot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import discord from 'discord.js';
import Bot from '../lib/bot';
import createDiscordStub from './stubs/discord-stub';
import ClientStub from './stubs/irc-client-stub';
import createWebhookStub from './stubs/webhook-stub';
import config from './fixtures/single-test-config.json';
import configMsgFormatDefault from './fixtures/msg-formats-default.json';

Expand Down Expand Up @@ -40,6 +41,8 @@ describe('Bot', function () {
ClientStub.prototype.say = sandbox.stub();
ClientStub.prototype.send = sandbox.stub();
ClientStub.prototype.join = sandbox.stub();
this.sendWebhookMessageStub = sandbox.stub();
discord.WebhookClient = createWebhookStub(this.sendWebhookMessageStub);
this.bot = new Bot(config);
this.bot.connect();

Expand Down Expand Up @@ -807,4 +810,80 @@ describe('Bot', function () {
this.sendStub.should.have.been.calledOnce;
this.sendStub.getCall(0).args.should.deep.equal([msg]);
});

it('should create webhooks clients for each webhook url in the config', function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of tests - nice!

this.bot.webhooks.should.have.property('#withwebhook');
});

it('should extract id and token from webhook urls', function () {
this.bot.webhooks['#withwebhook'].id.should.equal('id');
});

it('should find the matching webhook when it exists', function () {
this.bot.findWebhook('#ircwebhook').should.not.equal(null);
});

it('should prefer webhooks to send a message when possible', function () {
const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
const bot = new Bot(newConfig);
bot.connect();
bot.sendToDiscord('nick', '#irc', 'text');
this.sendWebhookMessageStub.should.have.been.called;
});

it('should find a matching username, case sensitive, when looking for an avatar', function () {
const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
const bot = new Bot(newConfig);
bot.connect();
const userObj = { id: 123, username: 'Nick', avatar: 'avatarURL' };
const memberObj = { nickname: 'Different' };
this.addUser(userObj, memberObj);
this.bot.getDiscordAvatar('Nick', '#irc').should.equal('/avatars/123/avatarURL.png?size=2048');
});

it('should find a matching username, case insensitive, when looking for an avatar', function () {
const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
const bot = new Bot(newConfig);
bot.connect();
const userObj = { id: 124, username: 'nick', avatar: 'avatarURL' };
const memberObj = { nickname: 'Different' };
this.addUser(userObj, memberObj);
this.bot.getDiscordAvatar('Nick', '#irc').should.equal('/avatars/124/avatarURL.png?size=2048');
});

it('should find a matching nickname, case sensitive, when looking for an avatar', function () {
const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
const bot = new Bot(newConfig);
bot.connect();
const userObj = { id: 125, username: 'Nick', avatar: 'avatarURL' };
const memberObj = { nickname: 'Different' };
this.addUser(userObj, memberObj);
this.bot.getDiscordAvatar('Different', '#irc').should.equal('/avatars/125/avatarURL.png?size=2048');
});

it('should not return an avatar with two matching usernames when looking for an avatar', function () {
const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
const bot = new Bot(newConfig);
bot.connect();
const userObj1 = { id: 126, username: 'common', avatar: 'avatarURL' };
const userObj2 = { id: 127, username: 'Nick', avatar: 'avatarURL' };
const memberObj1 = { nickname: 'Different' };
const memberObj2 = { nickname: 'common' };
this.addUser(userObj1, memberObj1);
this.addUser(userObj2, memberObj2);
chai.should().equal(this.bot.getDiscordAvatar('common', '#irc'), null);
});

it('should not return an avatar when no users match and should handle lack of nickname, when looking for an avatar', function () {
const newConfig = { ...config, webhooks: { '#discord': 'https://discordapp.com/api/webhooks/id/token' } };
const bot = new Bot(newConfig);
bot.connect();
const userObj1 = { id: 128, username: 'common', avatar: 'avatarURL' };
const userObj2 = { id: 129, username: 'Nick', avatar: 'avatarURL' };
const memberObj1 = {};
const memberObj2 = { nickname: 'common' };
this.addUser(userObj1, memberObj1);
this.addUser(userObj2, memberObj2);
chai.should().equal(this.bot.getDiscordAvatar('nonexistent', '#irc'), null);
});
});
6 changes: 5 additions & 1 deletion test/fixtures/single-test-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
"channelMapping": {
"#discord": "#irc channelKey",
"#notinchannel": "#otherIRC",
"1234": "#channelforid"
"1234": "#channelforid",
"#withwebhook": "#ircwebhook"
},
"webhooks": {
"#withwebhook": "https://discordapp.com/api/webhooks/id/token"
}
}
5 changes: 5 additions & 0 deletions test/stubs/discord-stub.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export default function createDiscordStub(sendStub, guild, discordUsers) {
id: 'testid'
};
this.channels = this.guildChannels();
this.options = {
http: {
cdn: ''
}
};

this.users = discordUsers;
}
Expand Down
14 changes: 14 additions & 0 deletions test/stubs/webhook-stub.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/* eslint-disable class-methods-use-this */
export default function createWebhookStub(sendWebhookMessage) {
return class WebhookStub {
constructor(id, token) {
this.id = id;
this.token = token;
}

sendMessage() {
sendWebhookMessage();
return new Promise(() => {});
}
};
}