Merge pull request #6705 from razielgn/slack-service-attachments

Improved Slack integration with message attachments.

app/models/project_services/slack_message.rb

@@ -11,10 +11,16 @@ class SlackMessage
     @username = params.fetch(:user_name)
   end
 
-  def compose
+  def pretext
     format(message)
   end
 
+  def attachments
+    return [] if new_branch? || removed_branch?
+
+    commit_message_attachments
+  end
+
   private
 
   attr_reader :after
@@ -31,7 +37,7 @@ class SlackMessage
     elsif removed_branch?
       removed_branch_message
     else
-      push_message << commit_messages
+      push_message
     end
   end
 
@@ -54,15 +60,20 @@ class SlackMessage
   def commit_messages
     commits.each_with_object('') do |commit, str|
       str << compose_commit_message(commit)
+    end.chomp
   end
+
+  def commit_message_attachments
+    [{ text: format(commit_messages), color: attachment_color }]
   end
 
   def compose_commit_message(commit)
-    id = commit.fetch(:id)[0..5]
+    author = commit.fetch(:author).fetch(:name)
+    id = commit.fetch(:id)[0..8]
     message = commit.fetch(:message)
     url = commit.fetch(:url)
 
-    "\n - #{message} ([#{id}](#{url}))"
+    "[#{id}](#{url}): #{message} - #{author}\n"
   end
 
   def new_branch?
@@ -92,4 +103,8 @@ class SlackMessage
   def compare_link
     "[Compare changes](#{compare_url})"
   end
+
+  def attachment_color
+    '#345'
+  end
 end

app/models/project_services/slack_service.rb

@@ -53,7 +53,7 @@ class SlackService < Service
     notifier = Slack::Notifier.new(subdomain, token)
     notifier.channel = room
     notifier.username = 'GitLab'
-    notifier.ping(message.compose)
+    notifier.ping(message.pretext, attachments: message.attachments)
   end
 
   private

spec/models/slack_message_spec.rb

@@ -14,20 +14,27 @@ describe SlackMessage do
     }
   }
 
+  let(:color) { '#345' }
+
   context 'push' do
     before do
       args[:commits] = [
-        { message: 'message1', url: 'url1', id: 'abcdefghi' },
-        { message: 'message2', url: 'url2', id: '123456789' },
+        { message: 'message1', url: 'url1', id: 'abcdefghijkl', author: { name: 'author1' } },
+        { message: 'message2', url: 'url2', id: '123456789012', author: { name: 'author2' } },
       ]
     end
 
     it 'returns a message regarding pushes' do
-      subject.compose.should ==
+      subject.pretext.should ==
         'user_name pushed to branch <url/commits/master|master> of ' <<
-        '<url|project_name> (<url/compare/before...after|Compare changes>)' <<
-        "\n - message1 (<url1|abcdef>)" <<
-        "\n - message2 (<url2|123456>)"
+        '<url|project_name> (<url/compare/before...after|Compare changes>)'
+      subject.attachments.should == [
+        {
+          text: "<url1|abcdefghi>: message1 - author1\n" <<
+                "<url2|123456789>: message2 - author2",
+          color: color,
+        }
+      ]
     end
   end
 
@@ -37,9 +44,10 @@ describe SlackMessage do
     end
 
     it 'returns a message regarding a new branch' do
-      subject.compose.should ==
+      subject.pretext.should ==
         'user_name pushed new branch <url/commits/master|master> to ' <<
         '<url|project_name>'
+      subject.attachments.should be_empty
     end
   end
 
@@ -49,8 +57,9 @@ describe SlackMessage do
     end
 
     it 'returns a message regarding a removed branch' do
-      subject.compose.should ==
+      subject.pretext.should ==
         'user_name removed branch master from <url|project_name>'
+      subject.attachments.should be_empty
     end
   end
 end