Merge pull request slackapi#111 from slackhq/long-messages

Improve long message splitting and add tests
b3nj4m · Dec 10, 2014 · 12c1f5e · 12c1f5e
2 parents c57f7ec + 3ef146f
commit 12c1f5e
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 9 deletions.
diff --git a/src/slack.coffee b/src/slack.coffee
@@ -3,9 +3,9 @@
 SlackClient = require 'slack-client'
 Util = require 'util'
 
-MAX_MESSAGE_LENGTH = 4000
-
 class SlackBot extends Adapter
+  @MAX_MESSAGE_LENGTH: 4000
+
   constructor: (robot) ->
     @robot = robot
 
@@ -109,28 +109,36 @@ class SlackBot extends Adapter
     for msg in messages
       @robot.logger.debug "Sending to #{envelope.room}: #{msg}"
 
-      if msg.length <= MAX_MESSAGE_LENGTH
+      if msg.length <= SlackBot.MAX_MESSAGE_LENGTH
         channel.send msg
 
       # If message is greater than MAX_MESSAGE_LENGTH, split it into multiple messages
       else
         submessages = []
 
         while msg.length > 0
-          if msg.length <= MAX_MESSAGE_LENGTH
+          if msg.length <= SlackBot.MAX_MESSAGE_LENGTH
             submessages.push msg
             msg = ''
 
           else
             # Split message at last line break, if it exists
-            chunk = msg.substring(0, MAX_MESSAGE_LENGTH)
-            breakIndex = chunk.lastIndexOf('\n')
-            breakIndex = MAX_MESSAGE_LENGTH if breakIndex is -1
+            maxSizeChunk = msg.substring(0, SlackBot.MAX_MESSAGE_LENGTH)
+
+            lastLineBreak = maxSizeChunk.lastIndexOf('\n')
+            lastWordBreak = maxSizeChunk.match(/\W\w+$/)?.index
+
+            breakIndex = if lastLineBreak > -1
+              lastLineBreak
+            else if lastWordBreak
+              lastWordBreak
+            else
+              SlackBot.MAX_MESSAGE_LENGTH
 
             submessages.push msg.substring(0, breakIndex)
 
-            # Skip char if split on line break
-            breakIndex++ if breakIndex isnt MAX_MESSAGE_LENGTH
+            # Skip char if split on line or word break
+            breakIndex++ if breakIndex isnt SlackBot.MAX_MESSAGE_LENGTH
 
             msg = msg.substring(breakIndex, msg.length)
 

diff --git a/test/slack.coffee b/test/slack.coffee
@@ -12,15 +12,23 @@ should = require 'should'
 stubs = null
 beforeEach ->
   stubs =
+    # Slack client
+    channel:
+      send: (msg) -> msg
+    client:
+      getChannelGroupOrDMByName: () ->
+        stubs.channel
     # Hubot.Robot instance
     robot:
       logger:
         info: ->
+        debug: ->
 
 # Generate a new slack instance for each test.
 slackbot = null
 beforeEach ->
   slackbot = new SlackBot stubs.robot
+  slackbot.client = stubs.client
 
 
 ###################################################################
@@ -38,3 +46,33 @@ describe 'Login', ->
       name: 'bot'
     slackbot.loggedIn(user, team)
     slackbot.robot.name.should.equal 'bot'
+
+describe 'Send Messages', ->
+  it 'Should send multiple messages', ->
+    sentMessages = slackbot.send {room: 'room-name'}, 'one', 'two', 'three'
+    sentMessages.length.should.equal 3
+
+  it 'Should split long messages', ->
+    lines = 'Hello, Slackbot\nHow are you?\n'
+    # Make a very long message
+    msg = lines
+    len = 10000
+    msg += lines while msg.length < len
+
+    sentMessages = slackbot.send {room: 'room-name'}, msg
+    sentMessage = sentMessages.pop()
+    sentMessage.length.should.equal Math.ceil(len / SlackBot.MAX_MESSAGE_LENGTH)
+
+  it 'Should try to split on word breaks', ->
+    msg = 'Foo bar baz'
+    slackbot.constructor.MAX_MESSAGE_LENGTH = 10
+    sentMessages = slackbot.send {room: 'room-name'}, msg
+    sentMessage = sentMessages.pop()
+    sentMessage.length.should.equal 2
+
+  it 'Should split into max length chunks if there are no breaks', ->
+    msg = 'Foobar'
+    slackbot.constructor.MAX_MESSAGE_LENGTH = 3
+    sentMessages = slackbot.send {room: 'room-name'}, msg
+    sentMessage = sentMessages.pop()
+    sentMessage.should.eql ['Foo', 'bar']