Merge branch 'add-notes-notifications-to-services' into 'master'

Add notes notifications to services

This merge request builds on top of a few other merge requests:

* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/237
* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/350

The purpose of this merge request is to add service notification support when a user comments on one of the following:

* commits
* issues
* merge requests
* code snippets

Currently HipChat and Slack are only supported. The admin can select which events to toggle on or off.

## Questions

All note events are bundled under the `note` event type. Is this okay, or does it make more sense to have individual events and/or change the `object_type` attribute in the hook data?

Also, does it look better to link the entire `merge request #X` as opposed to just `#X`? I like the former.

## Before

![Screen Shot 2015-03-01 at 3.05.56 PM](https://gitlab.com/uploads/stanhu/gitlab-ce/8ae828e652/Screen_Shot_2015-03-01_at_3.05.56_PM.png)

## After

![Screen Shot 2015-03-06 at 6.24.40 AM](https://gitlab.com/uploads/gitlab-org/gitlab-ce/7b866ae02b/Screen_Shot_2015-03-06_at_6.24.40_AM.png)

### Slack

![Screen Shot 2015-03-01 at 2.59.00 PM](https://gitlab.com/uploads/stanhu/gitlab-ce/0147c2dc38/Screen_Shot_2015-03-01_at_2.59.00_PM.png)

### HipChat

![Screen Shot 2015-03-01 at 2.59.19 PM](https://gitlab.com/uploads/stanhu/gitlab-ce/14e78fc61c/Screen_Shot_2015-03-01_at_2.59.19_PM.png)

See merge request !358

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.9.0 (unreleased)
-  - Added issue and merge request events to HipChat and Slack service (Stan Hu)
+  - Added comment notification events to HipChat and Slack services (Stan Hu)
+  - Added issue and merge request events to HipChat and Slack services (Stan Hu)
   - Fix merge request URL passed to Webhooks. (Stan Hu)
   - Fix bug that caused a server error when editing a comment to "+1" or "-1" (Stan Hu)
   - Move labels/milestones tabs to sidebar

app/controllers/projects/services_controller.rb

@@ -52,7 +52,8 @@ class Projects::ServicesController < Projects::ApplicationController
       :build_key, :server, :teamcity_url, :build_type,
       :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
       :colorize_messages, :channels,
-      :push_events, :issues_events, :merge_requests_events, :tag_push_events
+      :push_events, :issues_events, :merge_requests_events, :tag_push_events,
+      :note_events
     )
   end
 end

app/helpers/gitlab_routing_helper.rb

@@ -44,4 +44,8 @@ module GitlabRoutingHelper
   def merge_request_url(entity, *args)
     namespace_project_merge_request_url(entity.project.namespace, entity.project, entity, *args)
   end
+
+  def snippet_url(entity, *args)
+    namespace_project_snippet_url(entity.project.namespace, entity.project, entity, *args)
+  end
 end

app/models/concerns/mentionable.rb

@@ -99,5 +99,4 @@ module Mentionable
     preexisting = references(p, original)
     create_cross_references!(p, a, preexisting)
   end
-
 end

app/models/note.rb

@@ -308,6 +308,10 @@ class Note < ActiveRecord::Base
     end
   end
 
+  def hook_attrs
+    attributes
+  end
+
   def set_diff
     # First lets find notes with same diff
     # before iterating over all mr diffs
@@ -466,6 +470,10 @@ class Note < ActiveRecord::Base
     for_merge_request? && for_diff_line?
   end
 
+  def for_project_snippet?
+    noteable_type == "Snippet"
+  end
+
   # override to return commits, which are not active record
   def noteable
     if for_commit?

app/models/project_services/asana_service.rb

@@ -15,8 +15,8 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
-
 require 'asana'
 
 class AsanaService < Service

app/models/project_services/assembla_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class AssemblaService < Service

app/models/project_services/bamboo_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class BambooService < CiService

app/models/project_services/buildbox_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 require "addressable/uri"

app/models/project_services/campfire_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class CampfireService < Service

app/models/project_services/gitlab_issue_tracker_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class GitlabIssueTrackerService < IssueTrackerService

app/models/project_services/hipchat_service.rb

@@ -45,7 +45,7 @@ class HipchatService < Service
   end
 
   def supported_events
-    %w(push issue merge_request)
+    %w(push issue merge_request note)
   end
 
   def execute(data)
@@ -73,6 +73,8 @@ class HipchatService < Service
         create_issue_message(data) unless is_update?(data)
       when "merge_request"
         create_merge_request_message(data) unless is_update?(data)
+      when "note"
+        create_note_message(data)
       end
   end
 
@@ -108,6 +110,14 @@ class HipchatService < Service
     message
   end
 
+  def format_body(body)
+    if body
+      body = body.truncate(200, separator: ' ', omission: '...')
+    end
+
+    "<pre>#{body}</pre>"
+  end
+
   def create_issue_message(data)
     username = data[:user][:username]
 
@@ -123,8 +133,8 @@ class HipchatService < Service
     message = "#{username} #{state} issue #{issue_link} in #{project_link}: <b>#{title}</b>"
 
     if description
-      description = description.truncate(200, separator: ' ', omission: '...')
-      message << "<pre>#{description}</pre>"
+      description = format_body(description)
+      message << description
     end
 
     message
@@ -148,8 +158,62 @@ class HipchatService < Service
       "#{project_link}: <b>#{title}</b>"
 
     if description
-      description = description.truncate(200, separator: ' ', omission: '...')
-      message << "<pre>#{description}</pre>"
+      description = format_body(description)
+      message << description
+    end
+
+    message
+  end
+
+  def format_title(title)
+    "<b>" + title.lines.first.chomp + "</b>"
+  end
+
+  def create_note_message(data)
+    data = HashWithIndifferentAccess.new(data)
+    username = data[:user][:username]
+
+    repo_attr = HashWithIndifferentAccess.new(data[:repository])
+
+    obj_attr = HashWithIndifferentAccess.new(data[:object_attributes])
+    note = obj_attr[:note]
+    note_url = obj_attr[:url]
+    noteable_type = obj_attr[:noteable_type]
+
+    case noteable_type
+    when "Commit"
+      commit_attr = HashWithIndifferentAccess.new(data[:commit])
+      subject_desc = commit_attr[:id]
+      subject_desc = Commit.truncate_sha(subject_desc)
+      subject_type = "commit"
+      title = format_title(commit_attr[:message])
+    when "Issue"
+      subj_attr = HashWithIndifferentAccess.new(data[:issue])
+      subject_id = subj_attr[:iid]
+      subject_desc = "##{subject_id}"
+      subject_type = "issue"
+      title = format_title(subj_attr[:title])
+    when "MergeRequest"
+      subj_attr = HashWithIndifferentAccess.new(data[:merge_request])
+      subject_id = subj_attr[:iid]
+      subject_desc = "##{subject_id}"
+      subject_type = "merge request"
+      title = format_title(subj_attr[:title])
+    when "Snippet"
+      subj_attr = HashWithIndifferentAccess.new(data[:snippet])
+      subject_id = subj_attr[:id]
+      subject_desc = "##{subject_id}"
+      subject_type = "snippet"
+      title = format_title(subj_attr[:title])
+    end
+
+    subject_html = "<a href=\"#{note_url}\">#{subject_type} #{subject_desc}</a>"
+    message = "#{username} commented on #{subject_html} in #{project_link}: "
+    message << title
+
+    if note
+      note = format_body(note)
+      message << note
     end
 
     message

app/models/project_services/issue_tracker_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class IssueTrackerService < Service

app/models/project_services/jira_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class JiraService < IssueTrackerService

app/models/project_services/pivotaltracker_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class PivotaltrackerService < Service

app/models/project_services/pushover_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class PushoverService < Service

app/models/project_services/redmine_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class RedmineService < IssueTrackerService

app/models/project_services/slack_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class SlackService < Service
@@ -43,7 +44,7 @@ class SlackService < Service
   end
 
   def supported_events
-    %w(push issue merge_request)
+    %w(push issue merge_request note)
   end
 
   def execute(data)
@@ -69,6 +70,8 @@ class SlackService < Service
         IssueMessage.new(data) unless is_update?(data)
       when "merge_request"
         MergeMessage.new(data) unless is_update?(data)
+      when "note"
+        NoteMessage.new(data)
       end
 
     opt = {}
@@ -99,3 +102,4 @@ end
 require "slack_service/issue_message"
 require "slack_service/push_message"
 require "slack_service/merge_message"
+require "slack_service/note_message"

app/models/project_services/slack_service/note_message.rb

+class SlackService
+  class NoteMessage < BaseMessage
+    attr_reader :message
+    attr_reader :username
+    attr_reader :project_name
+    attr_reader :project_link
+    attr_reader :note
+    attr_reader :note_url
+    attr_reader :title
+
+    def initialize(params)
+      params = HashWithIndifferentAccess.new(params)
+      @username = params[:user][:username]
+      @project_name = params[:project_name]
+      @project_url = params[:project_url]
+
+      obj_attr = params[:object_attributes]
+      obj_attr = HashWithIndifferentAccess.new(obj_attr)
+      @note = obj_attr[:note]
+      @note_url = obj_attr[:url]
+      noteable_type = obj_attr[:noteable_type]
+
+      case noteable_type
+      when "Commit"
+        create_commit_note(HashWithIndifferentAccess.new(params[:commit]))
+      when "Issue"
+        create_issue_note(HashWithIndifferentAccess.new(params[:issue]))
+      when "MergeRequest"
+        create_merge_note(HashWithIndifferentAccess.new(params[:merge_request]))
+      when "Snippet"
+        create_snippet_note(HashWithIndifferentAccess.new(params[:snippet]))
+      end
+    end
+
+    def attachments
+      description_message
+    end
+
+    private
+
+    def format_title(title)
+      title.lines.first.chomp
+    end
+
+    def create_commit_note(commit)
+      commit_sha = commit[:id]
+      commit_sha = Commit.truncate_sha(commit_sha)
+      commit_link = "[commit #{commit_sha}](#{@note_url})"
+      title = format_title(commit[:message])
+      @message = "#{@username} commented on #{commit_link} in #{project_link}: *#{title}*"
+    end
+
+    def create_issue_note(issue)
+      issue_iid = issue[:iid]
+      note_link = "[issue ##{issue_iid}](#{@note_url})"
+      title = format_title(issue[:title])
+      @message = "#{@username} commented on #{note_link} in #{project_link}: *#{title}*"
+    end
+
+    def create_merge_note(merge_request)
+      merge_request_id = merge_request[:iid]
+      merge_request_link = "[merge request ##{merge_request_id}](#{@note_url})"
+      title = format_title(merge_request[:title])
+      @message = "#{@username} commented on #{merge_request_link} in #{project_link}: *#{title}*"
+    end
+
+    def create_snippet_note(snippet)
+      snippet_id = snippet[:id]
+      snippet_link = "[snippet ##{snippet_id}](#{@note_url})"
+      title = format_title(snippet[:title])
+      @message = "#{@username} commented on #{snippet_link} in #{project_link}: *#{title}*"
+    end
+
+    def description_message
+      [{ text: format(@note), color: attachment_color }]
+    end
+
+    def project_link
+      "[#{@project_name}](#{@project_url})"
+    end
+  end
+end

app/models/project_services/teamcity_service.rb

@@ -15,6 +15,7 @@
 #  issues_events         :boolean          default(TRUE)
 #  merge_requests_events :boolean          default(TRUE)
 #  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
 #
 
 class TeamcityService < CiService

app/models/service.rb

@@ -28,6 +28,7 @@ class Service < ActiveRecord::Base
   default_value_for :issues_events, true
   default_value_for :merge_requests_events, true
   default_value_for :tag_push_events, true
+  default_value_for :note_events, true
 
   after_initialize :initialize_properties
 
@@ -42,6 +43,7 @@ class Service < ActiveRecord::Base
   scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) }
   scope :issue_hooks, -> { where(issues_events: true, active: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
+  scope :note_hooks, -> { where(note_events: true, active: true) }
 
   def activated?
     active

app/models/snippet.rb

@@ -59,6 +59,10 @@ class Snippet < ActiveRecord::Base
     content
   end
 
+  def hook_attrs
+    attributes
+  end
+
   def size
     0
   end

app/services/notes/create_service.rb

@@ -17,10 +17,22 @@ module Notes
           note.references.each do |mentioned|
             Note.create_cross_reference_note(mentioned, note.noteable, note.author, note.project)
           end
+
+          execute_hooks(note)
         end
       end
 
       note
     end
+
+    def hook_data(note)
+      Gitlab::NoteDataBuilder.build(note, current_user)
+    end
+
+    def execute_hooks(note)
+      note_data = hook_data(note)
+      # TODO: Support Webhooks
+      note.project.execute_services(note_data, :note_hooks)
+    end
   end
 end

app/views/projects/services/_form.html.haml

@@ -47,6 +47,14 @@
                 %strong Tag push events
               %p.light
                 This url will be triggered when a new tag is pushed to the repository
+        - if @service.supported_events.include?("note")
+          %div
+            = f.check_box :note_events, class: 'pull-left'
+            .prepend-left-20
+              = f.label :note_events, class: 'list-label' do
+                %strong Comments
+              %p.light
+                This url will be triggered when someone adds a comment
         - if @service.supported_events.include?("issue")
           %div
             = f.check_box :issues_events, class: 'pull-left'

db/migrate/20150225065047_add_note_events_to_services.rb

+class AddNoteEventsToServices < ActiveRecord::Migration
+  def change
+    add_column :services, :note_events, :boolean, default: true, null: false
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150223022001) do
+ActiveRecord::Schema.define(version: 20150225065047) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -371,6 +371,7 @@ ActiveRecord::Schema.define(version: 20150223022001) do
     t.boolean  "issues_events",         default: true
     t.boolean  "merge_requests_events", default: true
     t.boolean  "tag_push_events",       default: true
+    t.boolean  "note_events",           default: true,  null: false
   end
 
   add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree

lib/gitlab/note_data_builder.rb

+module Gitlab
+  class NoteDataBuilder
+    class << self
+      # Produce a hash of post-receive data
+      #
+      # For all notes:
+      #
+      # data = {
+      #   object_kind: "note",
+      #   user: {
+      #     name: String,
+      #     username: String,
+      #     avatar_url: String
+      #   }
+      #   project_id: Integer,
+      #   repository: {
+      #     name: String,
+      #     url: String,
+      #     description: String,
+      #     homepage: String,
+      #   }
+      #  object_attributes: {
+      #    <hook data for note>
+      #  }
+      #  <note-specific data>: {
+      # }
+      # note-specific data is a hash with one of the following keys and contains
+      # the hook data for that type.
+      #  - commit
+      #  - issue
+      #  - merge_request
+      #  - snippet
+      #
+      def build(note, user)
+        project = note.project
+        data = build_base_data(project, user, note)
+
+        if note.for_commit?
+          data[:commit] = build_data_for_commit(project, user, note)
+        elsif note.for_issue?
+          data[:issue] = note.noteable.hook_attrs
+        elsif note.for_merge_request?
+          data[:merge_request] = note.noteable.hook_attrs
+        elsif note.for_project_snippet?
+          data[:snippet] = note.noteable.hook_attrs
+        end
+
+        data
+      end
+
+      def build_base_data(project, user, note)
+        base_data = {
+          object_kind: "note",
+          user: user.hook_attrs,
+          project_id: project.id,
+          repository: {
+            name: project.name,
+            url: project.url_to_repo,
+            description: project.description,
+            homepage: project.web_url,
+          },
+          object_attributes: note.hook_attrs
+        }
+
+        base_data[:object_attributes][:url] =
+             Gitlab::UrlBuilder.new(:note).build(note.id)
+        base_data
+      end
+
+      def build_data_for_commit(project, user, note)
+        # commit_id is the SHA hash
+        commit = project.repository.commit(note.commit_id)
+        commit.hook_attrs(project)
+      end
+    end
+  end
+end

lib/gitlab/url_builder.rb

@@ -13,6 +13,9 @@ module Gitlab
         build_issue_url(id)
       when :merge_request
         build_merge_request_url(id)
+      when :note
+        build_note_url(id)
+
       end
     end
 
@@ -27,5 +30,31 @@ module Gitlab
       merge_request = MergeRequest.find(id)
       merge_request_url(merge_request, host: Gitlab.config.gitlab['url'])
     end
+
+    def build_note_url(id)
+      note = Note.find(id)
+      if note.for_commit?
+        namespace_project_commit_url(namespace_id: note.project.namespace,
+                                     id: note.commit_id,
+                                     project_id: note.project,
+                                     host: Gitlab.config.gitlab['url'],
+                                     anchor: "note_#{note.id}")
+      elsif note.for_issue?
+        issue = Issue.find(note.noteable_id)
+        issue_url(issue,
+                  host: Gitlab.config.gitlab['url'],
+                  anchor: "note_#{note.id}")
+      elsif note.for_merge_request?
+        merge_request = MergeRequest.find(note.noteable_id)
+        merge_request_url(merge_request,
+                          host: Gitlab.config.gitlab['url'],
+                          anchor: "note_#{note.id}")
+      elsif note.for_project_snippet?
+        snippet = Snippet.find(note.noteable_id)
+        snippet_url(snippet,
+                    host: Gitlab.config.gitlab['url'],
+                    anchor: "note_#{note.id}")
+      end
+    end
   end
 end

spec/factories/merge_requests.rb

@@ -40,7 +40,7 @@ FactoryGirl.define do
     source_branch "master"
     target_branch "feature"
 
-    merge_status :can_be_merged
+    merge_status "can_be_merged"
 
     trait :with_diffs do
     end

spec/factories/notes.rb

@@ -30,6 +30,7 @@ FactoryGirl.define do
     factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note]
     factory :note_on_merge_request, traits: [:on_merge_request]
     factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff]
+    factory :note_on_project_snippet, traits: [:on_project_snippet]
 
     trait :on_commit do
       project factory: :project
@@ -52,6 +53,11 @@ FactoryGirl.define do
       noteable_type "Issue"
     end
 
+    trait :on_project_snippet do
+      noteable_id 1
+      noteable_type "Snippet"
+    end
+
     trait :with_attachment do
       attachment { fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "`/png") }
     end

spec/lib/gitlab/note_data_builder_spec.rb

+require 'spec_helper'
+
+describe 'Gitlab::NoteDataBuilder' do
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+  let(:data) { Gitlab::NoteDataBuilder.build(note, user) }
+  let(:note_url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+  let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors
+
+  before(:each) do
+    expect(data).to have_key(:object_attributes)
+    expect(data[:object_attributes]).to have_key(:url)
+    expect(data[:object_attributes][:url]).to eq(note_url)
+    expect(data[:object_kind]).to eq('note')
+    expect(data[:user]).to eq(user.hook_attrs)
+  end
+
+  describe 'When asking for a note on commit' do
+    let(:note) { create(:note_on_commit) }
+
+    it 'returns the note and commit-specific data' do
+      expect(data).to have_key(:commit)
+    end
+  end
+
+  describe 'When asking for a note on commit diff' do
+    let(:note) { create(:note_on_commit_diff) }
+
+    it 'returns the note and commit-specific data' do
+      expect(data).to have_key(:commit)
+    end
+  end
+
+  describe 'When asking for a note on issue' do
+    let(:issue) { create(:issue, created_at: fixed_time, updated_at: fixed_time) }
+    let(:note) { create(:note_on_issue, noteable_id: issue.id) }
+
+    it 'returns the note and issue-specific data' do
+      expect(data).to have_key(:issue)
+      expect(data[:issue]).to eq(issue.hook_attrs)
+    end
+  end
+
+  describe 'When asking for a note on merge request' do
+    let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) }
+    let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id) }
+
+    it 'returns the note and merge request data' do
+      expect(data).to have_key(:merge_request)
+      expect(data[:merge_request]).to eq(merge_request.hook_attrs)
+    end
+  end
+
+  describe 'When asking for a note on merge request diff' do
+    let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) }
+    let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id) }
+
+    it 'returns the note and merge request diff data' do
+      expect(data).to have_key(:merge_request)
+      expect(data[:merge_request]).to eq(merge_request.hook_attrs)
+    end
+  end
+
+  describe 'When asking for a note on project snippet' do
+    let!(:snippet) { create(:project_snippet, created_at: fixed_time, updated_at: fixed_time) }
+    let!(:note) { create(:note_on_project_snippet, noteable_id: snippet.id) }
+
+    it 'returns the note and project snippet data' do
+      expect(data).to have_key(:snippet)
+      expect(data[:snippet]).to eq(snippet.hook_attrs)
+    end
+  end
+end

spec/lib/gitlab/url_builder_spec.rb

@@ -16,4 +16,62 @@ describe Gitlab::UrlBuilder do
       expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}"
     end
   end
+
+  describe 'When asking for a note on commit' do
+    let(:note) { create(:note_on_commit) }
+    let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+
+    it 'returns the note url' do
+      expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+    end
+  end
+
+  describe 'When asking for a note on commit diff' do
+    let(:note) { create(:note_on_commit_diff) }
+    let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+
+    it 'returns the note url' do
+      expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+    end
+  end
+
+  describe 'When asking for a note on issue' do
+    let(:issue) { create(:issue) }
+    let(:note) { create(:note_on_issue, noteable_id: issue.id) }
+    let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+
+    it 'returns the note url' do
+      expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}#note_#{note.id}"
+    end
+  end
+
+  describe 'When asking for a note on merge request' do
+    let(:merge_request) { create(:merge_request) }
+    let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id) }
+    let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+
+    it 'returns the note url' do
+      expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
+    end
+  end
+
+  describe 'When asking for a note on merge request diff' do
+    let(:merge_request) { create(:merge_request) }
+    let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id) }
+    let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+
+    it 'returns the note url' do
+      expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
+    end
+  end
+
+  describe 'When asking for a note on project snippet' do
+    let(:snippet) { create(:project_snippet) }
+    let(:note) { create(:note_on_project_snippet, noteable_id: snippet.id) }
+    let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+
+    it 'returns the note url' do
+      expect(url).to eq "#{Settings.gitlab['url']}/#{snippet.project.path_with_namespace}/snippets/#{note.noteable_id}#note_#{note.id}"
+    end
+  end
 end

spec/models/project_services/hipchat_service_spec.rb

@@ -98,5 +98,91 @@ describe HipchatService do
             "<pre>please fix</pre>")
       end
     end
+
+    context "Note events" do
+      let(:user) { create(:user) }
+      let(:project) { create(:project, creator_id: user.id) }
+      let(:issue)         { create(:issue, project: project) }
+      let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+      let(:snippet)       { create(:project_snippet, project: project) }
+      let(:commit_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') }
+      let(:merge_request_note) { create(:note_on_merge_request, noteable_id: merge_request.id, note: "merge request note") }
+      let(:issue_note) { create(:note_on_issue, noteable_id: issue.id, note: "issue note")}
+      let(:snippet_note) { create(:note_on_project_snippet, noteable_id: snippet.id, note: "snippet note") }
+
+      it "should call Hipchat API for commit comment events" do
+        data = Gitlab::NoteDataBuilder.build(commit_note, user)
+        hipchat.execute(data)
+
+        expect(WebMock).to have_requested(:post, api_url).once
+
+        message = hipchat.send(:create_message, data)
+
+        obj_attr = data[:object_attributes]
+        commit_id = Commit.truncate_sha(data[:commit][:id])
+        title = hipchat.send(:format_title, data[:commit][:message])
+
+        expect(message).to eq("#{user.username} commented on " \
+            "<a href=\"#{obj_attr[:url]}\">commit #{commit_id}</a> in " \
+            "<a href=\"#{project.web_url}\">#{project_name}</a>: " \
+            "#{title}" \
+            "<pre>a comment on a commit</pre>")
+      end
+
+      it "should call Hipchat API for merge request comment events" do
+        data = Gitlab::NoteDataBuilder.build(merge_request_note, user)
+        hipchat.execute(data)
+
+        expect(WebMock).to have_requested(:post, api_url).once
+
+        message = hipchat.send(:create_message, data)
+
+        obj_attr = data[:object_attributes]
+        merge_id = data[:merge_request]['iid']
+        title = data[:merge_request]['title']
+
+        expect(message).to eq("#{user.username} commented on " \
+            "<a href=\"#{obj_attr[:url]}\">merge request ##{merge_id}</a> in " \
+            "<a href=\"#{project.web_url}\">#{project_name}</a>: " \
+            "<b>#{title}</b>" \
+            "<pre>merge request note</pre>")
+      end
+
+      it "should call Hipchat API for issue comment events" do
+        data = Gitlab::NoteDataBuilder.build(issue_note, user)
+        hipchat.execute(data)
+
+        message = hipchat.send(:create_message, data)
+
+        obj_attr = data[:object_attributes]
+        issue_id = data[:issue]['iid']
+        title = data[:issue]['title']
+
+        expect(message).to eq("#{user.username} commented on " \
+            "<a href=\"#{obj_attr[:url]}\">issue ##{issue_id}</a> in " \
+            "<a href=\"#{project.web_url}\">#{project_name}</a>: " \
+            "<b>#{title}</b>" \
+            "<pre>issue note</pre>")
+      end
+
+      it "should call Hipchat API for snippet comment events" do
+        data = Gitlab::NoteDataBuilder.build(snippet_note, user)
+        hipchat.execute(data)
+
+        expect(WebMock).to have_requested(:post, api_url).once
+
+        message = hipchat.send(:create_message, data)
+
+        obj_attr = data[:object_attributes]
+        snippet_id = data[:snippet]['id']
+        title = data[:snippet]['title']
+
+        expect(message).to eq("#{user.username} commented on " \
+            "<a href=\"#{obj_attr[:url]}\">snippet ##{snippet_id}</a> in " \
+            "<a href=\"#{project.web_url}\">#{project_name}</a>: " \
+            "<b>#{title}</b>" \
+            "<pre>snippet note</pre>")
+      end
+    end
   end
 end

spec/models/project_services/slack_service/note_message_spec.rb

+require 'spec_helper'
+
+describe SlackService::NoteMessage do
+  let(:color) { '#345' }
+
+  before do
+    @args = {
+        user: {
+            name: 'Test User',
+            username: 'username',
+            avatar_url: 'http://fakeavatar'
+        },
+        project_name: 'project_name',
+        project_url: 'somewhere.com',
+        repository: {
+            name: 'project_name',
+            url: 'somewhere.com',
+        },
+        object_attributes: {
+            id: 10,
+            note: 'comment on a commit',
+            url: 'url',
+            noteable_type: 'Commit'
+        }
+    }
+  end
+
+  context 'commit notes' do
+    before do
+      @args[:object_attributes][:note] = 'comment on a commit'
+      @args[:object_attributes][:noteable_type] = 'Commit'
+      @args[:commit] = {
+          id: '5f163b2b95e6f53cbd428f5f0b103702a52b9a23',
+          message: "Added a commit message\ndetails\n123\n"
+      }
+    end
+
+    it 'returns a message regarding notes on commits' do
+      message = SlackService::NoteMessage.new(@args)
+      expect(message.pretext).to eq("username commented on " \
+      "<url|commit 5f163b2b> in <somewhere.com|project_name>: " \
+      "*Added a commit message*")
+      expected_attachments = [
+          {
+              text: "comment on a commit",
+              color: color,
+          }
+      ]
+      expect(message.attachments).to eq(expected_attachments)
+    end
+  end
+
+  context 'merge request notes' do
+    before do
+      @args[:object_attributes][:note] = 'comment on a merge request'
+      @args[:object_attributes][:noteable_type] = 'MergeRequest'
+      @args[:merge_request] = {
+          id: 1,
+          iid: 30,
+          title: "merge request title\ndetails\n"
+      }
+    end
+    it 'returns a message regarding notes on a merge request' do
+      message = SlackService::NoteMessage.new(@args)
+      expect(message.pretext).to eq("username commented on " \
+      "<url|merge request #30> in <somewhere.com|project_name>: " \
+      "*merge request title*")
+      expected_attachments =  [
+          {
+              text: "comment on a merge request",
+              color: color,
+          }
+      ]
+      expect(message.attachments).to eq(expected_attachments)
+    end
+  end
+
+  context 'issue notes' do
+    before do
+      @args[:object_attributes][:note] = 'comment on an issue'
+      @args[:object_attributes][:noteable_type] = 'Issue'
+      @args[:issue] = {
+          id: 1,
+          iid: 20,
+          title: "issue title\ndetails\n"
+      }
+    end
+
+    it 'returns a message regarding notes on an issue' do
+      message = SlackService::NoteMessage.new(@args)
+      expect(message.pretext).to eq(
+                                     "username commented on " \
+      "<url|issue #20> in <somewhere.com|project_name>: " \
+      "*issue title*")
+      expected_attachments =  [
+          {
+              text: "comment on an issue",
+              color: color,
+          }
+      ]
+      expect(message.attachments).to eq(expected_attachments)
+    end
+  end
+
+  context 'project snippet notes' do
+    before do
+      @args[:object_attributes][:note] = 'comment on a snippet'
+      @args[:object_attributes][:noteable_type] = 'Snippet'
+      @args[:snippet] = {
+          id: 5,
+          title: "snippet title\ndetails\n"
+      }
+    end
+
+    it 'returns a message regarding notes on a project snippet' do
+      message = SlackService::NoteMessage.new(@args)
+      expect(message.pretext).to eq("username commented on " \
+      "<url|snippet #5> in <somewhere.com|project_name>: " \
+      "*snippet title*")
+      expected_attachments =  [
+          {
+              text: "comment on a snippet",
+              color: color,
+          }
+      ]
+      expect(message.attachments).to eq(expected_attachments)
+    end
+  end
+end

spec/models/project_services/slack_service_spec.rb

@@ -76,16 +76,16 @@ describe SlackService do
                                                    'open')
     end
 
-    it "should call Slack API for pull requests" do
+    it "should call Slack API for push events" do
       slack.execute(push_sample_data)
 
-      WebMock.should have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, webhook_url).once
     end
 
     it "should call Slack API for issue events" do
       slack.execute(@issues_sample_data)
 
-      WebMock.should have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, webhook_url).once
     end
 
     it "should call Slack API for merge requests events" do
@@ -97,10 +97,10 @@ describe SlackService do
     it 'should use the username as an option for slack when configured' do
       slack.stub(username: username)
       expect(Slack::Notifier).to receive(:new).
-        with(webhook_url, username: username).
-        and_return(
-          double(:slack_service).as_null_object
-        )
+       with(webhook_url, username: username).
+       and_return(
+         double(:slack_service).as_null_object
+       )
       slack.execute(push_sample_data)
     end
 
@@ -114,4 +114,57 @@ describe SlackService do
       slack.execute(push_sample_data)
     end
   end
+
+  describe "Note events" do
+    let(:slack)   { SlackService.new }
+    let(:user) { create(:user) }
+    let(:project) { create(:project, creator_id: user.id) }
+    let(:issue)         { create(:issue, project: project) }
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+    let(:snippet)       { create(:project_snippet, project: project) }
+    let(:commit_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') }
+    let(:merge_request_note) { create(:note_on_merge_request, noteable_id: merge_request.id, note: "merge request note") }
+    let(:issue_note) { create(:note_on_issue, noteable_id: issue.id, note: "issue note")}
+    let(:snippet_note) { create(:note_on_project_snippet, noteable_id: snippet.id, note: "snippet note") }
+    let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
+
+    before do
+      slack.stub(
+          project: project,
+          project_id: project.id,
+          service_hook: true,
+          webhook: webhook_url
+      )
+
+      WebMock.stub_request(:post, webhook_url)
+    end
+
+    it "should call Slack API for commit comment events" do
+      data = Gitlab::NoteDataBuilder.build(commit_note, user)
+      slack.execute(data)
+
+      expect(WebMock).to have_requested(:post, webhook_url).once
+    end
+
+    it "should call Slack API for merge request comment events" do
+      data = Gitlab::NoteDataBuilder.build(merge_request_note, user)
+      slack.execute(data)
+
+      expect(WebMock).to have_requested(:post, webhook_url).once
+    end
+
+    it "should call Slack API for issue comment events" do
+      data = Gitlab::NoteDataBuilder.build(issue_note, user)
+      slack.execute(data)
+
+      expect(WebMock).to have_requested(:post, webhook_url).once
+    end
+
+    it "should call Slack API for snippet comment events" do
+      data = Gitlab::NoteDataBuilder.build(snippet_note, user)
+      slack.execute(data)
+
+      expect(WebMock).to have_requested(:post, webhook_url).once
+    end
+  end
 end