diff --git a/CHANGELOG b/CHANGELOG index b927b60140e1ca2ed907bc683b7371f730c752e9..611c6c77d5452d3d562722fb236f9efb44066d8d 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 087579de1064853f658bc4c5378e93bf9945fc70..382d63d053b6e60a984cc94b19de0c95918edc99 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 ac37f909ce97c6189bef3aef95423233ac9cf87c..8518a47a3a006ab085e1e42f5022ba5b9595fc76 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 50be458bf246bc9f247cdd31d70c6141f8a2e5ce..74900d4675d788675f11069320eac5ed190be1e7 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 e6c258ffbe95d8b47723ce845361749c31374ade..b19d7b0f256d1c62e6888ae230ff52851b53fc82 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 8dce33e670bc72c316957a8ce40ca95b77580fdc..6a622207385936450f9cf18dc76dd6dcabe65aa0 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 6dc2500e7707e56a9b30c6706665e8cd24bc5f0e..fb7e0c0fb0d1580c5b36217e6f5f44b9e7f3b289 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 50b7cb795d76e5d65f246ae09cfde58214fb6d4e..0100f1e4a10be3aa1b83ee8a714c9a7886c3d8d2 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 1270484ff6d0682afbbfefc63f88775abb63cf90..270863c1576c9cfde71e3d1124f503797ae28efc 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 21e1ca603bb4e113ee15fc5fa7c6b7d64bd58bf8..1c63444fbf9ab41f0dd078a605e3a5cd598470fd 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 90be1e42b263d1d66b0727456976d86113ed2b91..84346350a6c9e7879bc85cdd3dbdf8b7c6acfe29 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 9d094eaf14604f9ddcb2f47fd66df9beb825ceca..d24351a7b13cac60b39c5bf44859713dd15557c5 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 7fe1326890bca3ba53d6563faacf0fd6b23a1ed1..16876335b67562a94e3decc0e9721b54ace5f970 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 4a76f23c69326a51b323a77903bfe19e8d83560e..fcd9dc2f33674ccd1fbd781434c542e1dc3cc298 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 13cbb9bdbcf135b5fb265e6a29e32f77f4939e19..ade9ee97873999daa81fba4880cd736b9d6319aa 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 a67abb3483107887f0cd7acbcc0f52d8f7d85e1c..0ce324434db0b9535d6183672442f7060b6a3a51 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 7d7d7d7660bbb647624b1a8a51879c3aac31b844..dd9ba97ee1fac487d66f5315955002abef9da6d9 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 c529a784017fec93ffbd4ae7879bb831545f3e0e..a58840116f44ef829d22e017d5b78a7a123801ef 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 0000000000000000000000000000000000000000..f93dc358f61756e5dc07bc08c7c54d8e9b75b705 --- /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 cd9388de873be574a9456c0a0a76948e019edd55..038c200adc73331d9cd95a37b058531803a58c2d 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 8f6a9d57d3b393a78624ccf0160881683236ca9a..33734e97c552354258608adbc62ffe970cc73ea1 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 82c1ab94446d071ef16b3209b6019bef225becf0..3fb2ec1d66cc655c29a784dbe386f827c62f16d0 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 f64006a4edcceaed27155a2b8ceb20f0420a2af3..e969061f229d8a8ee6ce266cdf9a88a50c79af45 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 55ac85c32b9db9321f9abb6259093cc1770ba90f..defcdbe268eee27f10993addd0c33276bfa5b9a9 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 0000000000000000000000000000000000000000..d54ba9e482f846fb8170227a0fd80197e9438b0f --- /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 1a9b512e15967d836c9c4690349c246aa46d88ab..a686bb4b3cd07fd7d0a7f4a2c47b4f2bf71a3b93 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 0000000000000000000000000000000000000000..644dec45dca6db0eec39da424f2d443c2a2abc27 --- /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 ab7c8ad89f35c998125ed03aaec51e5558ac5e86..6830d15875affba7929b02dae9e69fee9549dc56 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 6ce1d7446fed65731edf836f6ed268ed0b02a3cc..77cd37c22d93c0c85d7cf2fcfacca2d392396667 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 83d0cc62dbf9d5b86269264b7f22290f3714c752..f1c33461b55ee93bc294c4be19ed93f0154c49ef 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 0000000000000000000000000000000000000000..448cd0c6880dcd58c23174cc62de6deec7453c2a --- /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 94b2fd5508ebc80aa5b39e5c973a1e06df5d534e..5153ed15af3f865fed83d30545d9d99a82685639 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 804c3d5ddd9aac800c57c68aedffb50000e2cf12..95ce4f8e4aaba25477447cdc68620245eed8b062 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 0000000000000000000000000000000000000000..f2516c100089d48ba8ae8bdb3f02d004c506a058 --- /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 4e8a96ec730fe9fa783618d8cc5820238821f6ca..c36506644b30c74508a91abaf5e44357a7d9ad7c 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