From 7e204cf389346d23e71bc4c2fa9e14cf82a7ed2e Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 5 Mar 2015 10:38:23 -0800
Subject: [PATCH] Added comment notification events to HipChat and Slack
services.
Supports four different event types all bundled under the "note" event type:
- comments on a commit
- comments on an issue
- comments on a merge request
- comments on a code snippet
---
CHANGELOG | 3 +-
.../projects/services_controller.rb | 3 +-
app/helpers/gitlab_routing_helper.rb | 4 +
app/models/concerns/mentionable.rb | 1 -
app/models/note.rb | 8 ++
app/models/project_services/asana_service.rb | 2 +-
.../project_services/assembla_service.rb | 1 +
app/models/project_services/bamboo_service.rb | 1 +
.../project_services/buildbox_service.rb | 1 +
.../project_services/campfire_service.rb | 1 +
.../gitlab_issue_tracker_service.rb | 1 +
.../project_services/hipchat_service.rb | 74 +++++++++-
.../project_services/issue_tracker_service.rb | 1 +
app/models/project_services/jira_service.rb | 1 +
.../pivotaltracker_service.rb | 1 +
.../project_services/pushover_service.rb | 1 +
.../project_services/redmine_service.rb | 1 +
app/models/project_services/slack_service.rb | 6 +-
.../slack_service/note_message.rb | 82 +++++++++++
.../project_services/teamcity_service.rb | 1 +
app/models/service.rb | 2 +
app/models/snippet.rb | 4 +
app/services/notes/create_service.rb | 12 ++
app/views/projects/services/_form.html.haml | 8 ++
...50225065047_add_note_events_to_services.rb | 5 +
db/schema.rb | 3 +-
lib/gitlab/note_data_builder.rb | 77 +++++++++++
lib/gitlab/url_builder.rb | 29 ++++
spec/factories/merge_requests.rb | 2 +-
spec/factories/notes.rb | 6 +
spec/lib/gitlab/note_data_builder_spec.rb | 73 ++++++++++
spec/lib/gitlab/url_builder_spec.rb | 58 ++++++++
.../project_services/hipchat_service_spec.rb | 86 ++++++++++++
.../slack_service/note_message_spec.rb | 129 ++++++++++++++++++
.../project_services/slack_service_spec.rb | 67 ++++++++-
35 files changed, 736 insertions(+), 19 deletions(-)
create mode 100644 app/models/project_services/slack_service/note_message.rb
create mode 100644 db/migrate/20150225065047_add_note_events_to_services.rb
create mode 100644 lib/gitlab/note_data_builder.rb
create mode 100644 spec/lib/gitlab/note_data_builder_spec.rb
create mode 100644 spec/models/project_services/slack_service/note_message_spec.rb
diff --git a/CHANGELOG b/CHANGELOG
index b927b60140e..611c6c77d54 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,7 +1,8 @@
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
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index 087579de106..382d63d053b 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/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
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index ac37f909ce9..8518a47a3a0 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/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
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index 50be458bf24..74900d4675d 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -99,5 +99,4 @@ module Mentionable
preexisting = references(p, original)
create_cross_references!(p, a, preexisting)
end
-
end
diff --git a/app/models/note.rb b/app/models/note.rb
index e6c258ffbe9..b19d7b0f256 100644
--- a/app/models/note.rb
+++ b/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?
diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb
index 8dce33e670b..6a622207385 100644
--- a/app/models/project_services/asana_service.rb
+++ b/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
diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb
index 6dc2500e770..fb7e0c0fb0d 100644
--- a/app/models/project_services/assembla_service.rb
+++ b/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
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 50b7cb795d7..0100f1e4a10 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/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
diff --git a/app/models/project_services/buildbox_service.rb b/app/models/project_services/buildbox_service.rb
index 1270484ff6d..270863c1576 100644
--- a/app/models/project_services/buildbox_service.rb
+++ b/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"
diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb
index 21e1ca603bb..1c63444fbf9 100644
--- a/app/models/project_services/campfire_service.rb
+++ b/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
diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb
index 90be1e42b26..84346350a6c 100644
--- a/app/models/project_services/gitlab_issue_tracker_service.rb
+++ b/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
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index 9d094eaf146..d24351a7b13 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/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
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index 7fe1326890b..16876335b67 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/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
diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index 4a76f23c693..fcd9dc2f336 100644
--- a/app/models/project_services/jira_service.rb
+++ b/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
diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb
index 13cbb9bdbcf..ade9ee97873 100644
--- a/app/models/project_services/pivotaltracker_service.rb
+++ b/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
diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb
index a67abb34831..0ce324434db 100644
--- a/app/models/project_services/pushover_service.rb
+++ b/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
diff --git a/app/models/project_services/redmine_service.rb b/app/models/project_services/redmine_service.rb
index 7d7d7d7660b..dd9ba97ee1f 100644
--- a/app/models/project_services/redmine_service.rb
+++ b/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
diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb
index c529a784017..a58840116f4 100644
--- a/app/models/project_services/slack_service.rb
+++ b/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"
diff --git a/app/models/project_services/slack_service/note_message.rb b/app/models/project_services/slack_service/note_message.rb
new file mode 100644
index 00000000000..f93dc358f61
--- /dev/null
+++ b/app/models/project_services/slack_service/note_message.rb
@@ -0,0 +1,82 @@
+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
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index cd9388de873..038c200adc7 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/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
diff --git a/app/models/service.rb b/app/models/service.rb
index 8f6a9d57d3b..33734e97c55 100644
--- a/app/models/service.rb
+++ b/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
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 82c1ab94446..3fb2ec1d66c 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -59,6 +59,10 @@ class Snippet < ActiveRecord::Base
content
end
+ def hook_attrs
+ attributes
+ end
+
def size
0
end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index f64006a4edc..e969061f229 100644
--- a/app/services/notes/create_service.rb
+++ b/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
diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml
index 55ac85c32b9..defcdbe268e 100644
--- a/app/views/projects/services/_form.html.haml
+++ b/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'
diff --git a/db/migrate/20150225065047_add_note_events_to_services.rb b/db/migrate/20150225065047_add_note_events_to_services.rb
new file mode 100644
index 00000000000..d54ba9e482f
--- /dev/null
+++ b/db/migrate/20150225065047_add_note_events_to_services.rb
@@ -0,0 +1,5 @@
+class AddNoteEventsToServices < ActiveRecord::Migration
+ def change
+ add_column :services, :note_events, :boolean, default: true, null: false
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 1a9b512e159..a686bb4b3cd 100644
--- a/db/schema.rb
+++ b/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
diff --git a/lib/gitlab/note_data_builder.rb b/lib/gitlab/note_data_builder.rb
new file mode 100644
index 00000000000..644dec45dca
--- /dev/null
+++ b/lib/gitlab/note_data_builder.rb
@@ -0,0 +1,77 @@
+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
diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb
index ab7c8ad89f3..6830d15875a 100644
--- a/lib/gitlab/url_builder.rb
+++ b/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
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 6ce1d7446fe..77cd37c22d9 100644
--- a/spec/factories/merge_requests.rb
+++ b/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
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index 83d0cc62dbf..f1c33461b55 100644
--- a/spec/factories/notes.rb
+++ b/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
diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/note_data_builder_spec.rb
new file mode 100644
index 00000000000..448cd0c6880
--- /dev/null
+++ b/spec/lib/gitlab/note_data_builder_spec.rb
@@ -0,0 +1,73 @@
+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
diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb
index 94b2fd5508e..5153ed15af3 100644
--- a/spec/lib/gitlab/url_builder_spec.rb
+++ b/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
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index 804c3d5ddd9..95ce4f8e4aa 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/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
diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb
new file mode 100644
index 00000000000..f2516c10008
--- /dev/null
+++ b/spec/models/project_services/slack_service/note_message_spec.rb
@@ -0,0 +1,129 @@
+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
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index 4e8a96ec730..c36506644b3 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/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
--
GitLab