Repository: reactiflux/discord-irc
Pull request #33 (2016-02-24 20:46:19)

Fix sending attachments to IRC
Now that Discord actually got the ability to send both text messages and attachments in single message, here's the code to support that.

This time with tests right off the bat!

rce: I wonder what went wrong with the frigg build? Results look correct other than the return code `255` which I can't reproduce on my machine.
ekmartin: Not sure, seems like it passed after a restart. Would it be better to make it clearer that the message actually is a comment to the attachment? Something like "ekmartin posted an attachment to #general on Discord, with the comment "here's the file": <url> - or something like that?
ekmartin: Any update on this? :)
rce: I've been running a few variations of this myself because I wasn't sure what would be the best format to post the attachments and comment to IRC.

We have ended up with sending the comment and attachment link(s) as if they were simple normal messages like this:

```
<rce> So good cola!
<rce> https://cdn.discordapp.com/attachments/141649840923869184/164022249014165506/IMG_0620.JPG
```

Instead of this

```
<rce> So good cola!
rce posted an attachment to #channel on Discord: https://cdn.discordapp.com/attachments/141649840923869184/164022249014165506/IMG_0620.JPG
```

or this

```
rce posted an attachment to #channel on Discord, with the comment "So good cola!": https://cdn.discordapp.com/attachments/141649840923869184/164022249014165506/IMG_0620.JPG
```

We found that the extra text "X posted an attachment to Y on Discord" is very much noise especially if the channel receives attachments often. The links itself are pretty damn long by themselves. None of the other messages to IRC include similar extra info either. What the IRC user wants to see is just the comment and link to attachment anyway.

FWIW Discord doesn't distinguish between an attachment with comment and a text message and attachment sent separately.

![image](https://cloud.githubusercontent.com/assets/4427896/14247268/968fc3b4-fa5c-11e5-8038-985d842fe319.png)

What do you think?
ekmartin: Sounds good, your points about too much noise makes sense. Care to rebase with master after changing to what you're suggesting? So the branch is mergeable :)

Patch:
diff --git a/lib/bot.js b/lib/bot.js
index 9296792..1645804 100644
--- a/lib/bot.js
+++ b/lib/bot.js
@@ -147,17 +147,20 @@ class Bot {
         const prelude = `Command sent from Discord by ${username}:`;
         this.ircClient.say(ircChannel, prelude);
         this.ircClient.say(ircChannel, text);
-      } else if (message.attachments && message.attachments.length) {
-        message.attachments.forEach(a => {
-          const urlMessage =
-            `${displayUsername} posted an attachment to ${channelName} on Discord: ${a.url}`;
-          logger.debug('Sending attachment URL to IRC', ircChannel, urlMessage);
-          this.ircClient.say(ircChannel, urlMessage);
-        });
       } else {
-        text = `<${displayUsername}> ${text}`;
-        logger.debug('Sending message to IRC', ircChannel, text);
-        this.ircClient.say(ircChannel, text);
+        if (text !== '') {
+          text = `<${displayUsername}> ${text}`;
+          logger.debug('Sending message to IRC', ircChannel, text);
+          this.ircClient.say(ircChannel, text);
+        }
+
+        if (message.attachments && message.attachments.length) {
+          message.attachments.forEach(a => {
+            const urlMessage = `<${displayUsername}> ${a.url}`;
+            logger.debug('Sending attachment URL to IRC', ircChannel, urlMessage);
+            this.ircClient.say(ircChannel, urlMessage);
+          });
+        }
       }
     }
   }


Test patch:
diff --git a/test/bot.test.js b/test/bot.test.js
index 37d0e0e..e3861a2 100644
--- a/test/bot.test.js
+++ b/test/bot.test.js
@@ -125,12 +125,58 @@ describe('Bot', function() {
     };
 
     this.bot.sendToIRC(message);
-    // Wrap in colors:
-    const expected = `\u000304${message.author.username}\u000f posted an attachment to ` +
-      `#${message.channel.name} on Discord: ${attachmentUrl}`;
+    const expected = `<\u000304${message.author.username}\u000f> ${attachmentUrl}`;
     ClientStub.prototype.say.should.have.been.calledWith('#irc', expected);
   });
 
+  it('should send text message and attachment URL to IRC if both exist', function() {
+    const text = 'Look at this cute cat picture!';
+    const attachmentUrl = 'https://image/url.jpg';
+    const message = {
+      content: text,
+      attachments: [{
+        url: attachmentUrl
+      }],
+      mentions: [],
+      channel: {
+        name: 'discord'
+      },
+      author: {
+        username: 'otherauthor',
+        id: 'not bot id'
+      }
+    };
+
+    this.bot.sendToIRC(message);
+
+    ClientStub.prototype.say.should.have.been.calledWith('#irc',
+      `<\u000304${message.author.username}\u000f> ${text}`);
+
+    const expected = `<\u000304${message.author.username}\u000f> ${attachmentUrl}`;
+    ClientStub.prototype.say.should.have.been.calledWith('#irc', expected);
+  });
+
+  it('should not send an empty text message with an attachment to IRC', function() {
+    const message = {
+      content: '',
+      attachments: [{
+        url: 'https://image/url.jpg'
+      }],
+      mentions: [],
+      channel: {
+        name: 'discord'
+      },
+      author: {
+        username: 'otherauthor',
+        id: 'not bot id'
+      }
+    };
+
+    this.bot.sendToIRC(message);
+
+    ClientStub.prototype.say.should.have.been.calledOnce;
+  });
+
   it('should not send its own messages to irc', function() {
     const message = {
       author: {