diff options
35 files changed, 736 insertions, 19 deletions
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 |