diff --git a/CHANGELOG b/CHANGELOG index 611c6c77d5452d3d562722fb236f9efb44066d8d..c0d38a9abf01e60db248b35c4361c73bb1037424 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,8 +1,9 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.9.0 (unreleased) - - Added comment notification events to HipChat and Slack services (Stan Hu) - - Added issue and merge request events to HipChat and Slack services (Stan Hu) + - Add tag push notifications and normalize HipChat and Slack messages to be consistent (Stan Hu) + - Add comment notification events to HipChat and Slack services (Stan Hu) + - Add 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 @@ -35,7 +36,6 @@ v 7.8.2 - Fix response of push to repository to return "Not found" if user doesn't have access - Fix check if user is allowed to view the file attachment - Fix import check for case sensetive namespaces - - Added issue and merge request events to Slack service (Stan Hu) - Increase timeout for Git-over-HTTP requests to 1 hour since large pulls/pushes can take a long time. v 7.8.1 diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index d24351a7b13cac60b39c5bf44859713dd15557c5..90ba7e080f15a79e67bee41c343ed2eb5d62a0cd 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 note) + %w(push issue merge_request note tag_push) end def execute(data) @@ -67,7 +67,7 @@ class HipchatService < Service message = \ case object_kind - when "push" + when "push", "tag_push" create_push_message(data) when "issue" create_issue_message(data) unless is_update?(data) @@ -79,21 +79,27 @@ class HipchatService < Service end def create_push_message(push) - ref = push[:ref].gsub("refs/heads/", "") + if push[:ref].starts_with?('refs/tags/') + ref_type = 'tag' + ref = push[:ref].gsub('refs/tags/', '') + else + ref_type = 'branch' + ref = push[:ref].gsub('refs/heads/', '') + end + before = push[:before] after = push[:after] message = "" message << "#{push[:user_name]} " if before.include?('000000') - message << "pushed new branch #{ref}"\ - " to "\ - "#{project_url}\n" + " to #{project_link}\n" elsif after.include?('000000') - message << "removed branch #{ref} from #{project.name_with_namespace.gsub!(/\s/,'')} \n" + message << "removed #{ref_type} #{ref} from #{project_name} \n" else - message << "pushed to branch #{ref} " message << "of #{project.name_with_namespace.gsub!(/\s/,'')} " message << "(Compare changes)" @@ -119,7 +125,7 @@ class HipchatService < Service end def create_issue_message(data) - username = data[:user][:username] + user_name = data[:user][:name] obj_attr = data[:object_attributes] obj_attr = HashWithIndifferentAccess.new(obj_attr) @@ -129,8 +135,8 @@ class HipchatService < Service issue_url = obj_attr[:url] description = obj_attr[:description] - issue_link = "##{issue_iid}" - message = "#{username} #{state} issue #{issue_link} in #{project_link}: #{title}" + issue_link = "issue ##{issue_iid}" + message = "#{user_name} #{state} #{issue_link} in #{project_link}: #{title}" if description description = format_body(description) @@ -141,7 +147,7 @@ class HipchatService < Service end def create_merge_request_message(data) - username = data[:user][:username] + user_name = data[:user][:name] obj_attr = data[:object_attributes] obj_attr = HashWithIndifferentAccess.new(obj_attr) @@ -153,8 +159,8 @@ class HipchatService < Service title = obj_attr[:title] merge_request_url = "#{project_url}/merge_requests/#{merge_request_id}" - merge_request_link = "##{merge_request_id}" - message = "#{username} #{state} merge request #{merge_request_link} in " \ + merge_request_link = "merge request ##{merge_request_id}" + message = "#{user_name} #{state} #{merge_request_link} in " \ "#{project_link}: #{title}" if description @@ -171,7 +177,7 @@ class HipchatService < Service def create_note_message(data) data = HashWithIndifferentAccess.new(data) - username = data[:user][:username] + user_name = data[:user][:name] repo_attr = HashWithIndifferentAccess.new(data[:repository]) @@ -208,7 +214,7 @@ class HipchatService < Service end subject_html = "#{subject_type} #{subject_desc}" - message = "#{username} commented on #{subject_html} in #{project_link}: " + message = "#{user_name} commented on #{subject_html} in #{project_link}: " message << title if note diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index a58840116f44ef829d22e017d5b78a7a123801ef..36d9874edd32d3b8d8a3f22c377458817858b935 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -44,7 +44,7 @@ class SlackService < Service end def supported_events - %w(push issue merge_request note) + %w(push issue merge_request note tag_push) end def execute(data) @@ -64,7 +64,7 @@ class SlackService < Service message = \ case object_kind - when "push" + when "push", "tag_push" PushMessage.new(data) when "issue" IssueMessage.new(data) unless is_update?(data) diff --git a/app/models/project_services/slack_service/issue_message.rb b/app/models/project_services/slack_service/issue_message.rb index e2fed0bb1b9a8ee9c35971ee4af6fcd5a31d1aaa..5af24a80609352547ab9b5b29549f6cd463a3a07 100644 --- a/app/models/project_services/slack_service/issue_message.rb +++ b/app/models/project_services/slack_service/issue_message.rb @@ -1,6 +1,6 @@ class SlackService class IssueMessage < BaseMessage - attr_reader :username + attr_reader :user_name attr_reader :title attr_reader :project_name attr_reader :project_url @@ -11,7 +11,7 @@ class SlackService attr_reader :description def initialize(params) - @username = params[:user][:username] + @user_name = params[:user][:name] @project_name = params[:project_name] @project_url = params[:project_url] @@ -34,7 +34,7 @@ class SlackService private def message - "#{username} #{state} issue #{issue_link} in #{project_link}: #{title}" + "#{user_name} #{state} #{issue_link} in #{project_link}: *#{title}*" end def opened_issue? @@ -50,7 +50,7 @@ class SlackService end def issue_link - "[##{issue_iid}](#{issue_url})" + "[issue ##{issue_iid}](#{issue_url})" end end end diff --git a/app/models/project_services/slack_service/merge_message.rb b/app/models/project_services/slack_service/merge_message.rb index 4dcce1d15a0ec0ea55f09b084a45888088929593..e792c258f7394a5e6e1c12dd222c75cd93d39620 100644 --- a/app/models/project_services/slack_service/merge_message.rb +++ b/app/models/project_services/slack_service/merge_message.rb @@ -1,15 +1,16 @@ class SlackService class MergeMessage < BaseMessage - attr_reader :username + attr_reader :user_name attr_reader :project_name attr_reader :project_url attr_reader :merge_request_id attr_reader :source_branch attr_reader :target_branch attr_reader :state + attr_reader :title def initialize(params) - @username = params[:user][:username] + @user_name = params[:user][:name] @project_name = params[:project_name] @project_url = params[:project_url] @@ -19,6 +20,7 @@ class SlackService @source_branch = obj_attr[:source_branch] @target_branch = obj_attr[:target_branch] @state = obj_attr[:state] + @title = format_title(obj_attr[:title]) end def pretext @@ -31,6 +33,10 @@ class SlackService private + def format_title(title) + '*' + title.lines.first.chomp + '*' + end + def message merge_request_message end @@ -40,11 +46,11 @@ class SlackService end def merge_request_message - "#{username} #{state} merge request #{merge_request_link} in #{project_link}" + "#{user_name} #{state} #{merge_request_link} in #{project_link}: #{title}" end def merge_request_link - "[##{merge_request_id}](#{merge_request_url})" + "[merge request ##{merge_request_id}](#{merge_request_url})" end def merge_request_url diff --git a/app/models/project_services/slack_service/note_message.rb b/app/models/project_services/slack_service/note_message.rb index f93dc358f61756e5dc07bc08c7c54d8e9b75b705..074478b292da5ef73cd6cd7860bf0893404d1cfa 100644 --- a/app/models/project_services/slack_service/note_message.rb +++ b/app/models/project_services/slack_service/note_message.rb @@ -1,7 +1,7 @@ class SlackService class NoteMessage < BaseMessage attr_reader :message - attr_reader :username + attr_reader :user_name attr_reader :project_name attr_reader :project_link attr_reader :note @@ -10,7 +10,7 @@ class SlackService def initialize(params) params = HashWithIndifferentAccess.new(params) - @username = params[:user][:username] + @user_name = params[:user][:name] @project_name = params[:project_name] @project_url = params[:project_url] @@ -47,28 +47,28 @@ class SlackService 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}*" + @message = "#{@user_name} 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}*" + @message = "#{@user_name} 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}*" + @message = "#{@user_name} 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}*" + @message = "#{@user_name} commented on #{snippet_link} in #{project_link}: *#{title}*" end def description_message diff --git a/app/models/project_services/slack_service/push_message.rb b/app/models/project_services/slack_service/push_message.rb index 2e566bc317b33361641edf08009b2575b2b471c0..3dc2df04764b8d0b74385a545471b817b397cd1a 100644 --- a/app/models/project_services/slack_service/push_message.rb +++ b/app/models/project_services/slack_service/push_message.rb @@ -6,7 +6,8 @@ class SlackService attr_reader :project_name attr_reader :project_url attr_reader :ref - attr_reader :username + attr_reader :ref_type + attr_reader :user_name def initialize(params) @after = params[:after] @@ -14,8 +15,14 @@ class SlackService @commits = params.fetch(:commits, []) @project_name = params[:project_name] @project_url = params[:project_url] - @ref = params[:ref].gsub('refs/heads/', '') - @username = params[:user_name] + if params[:ref].starts_with?('refs/tags/') + @ref_type = 'tag' + @ref = params[:ref].gsub('refs/tags/', '') + else + @ref_type = 'branch' + @ref = params[:ref].gsub('refs/heads/', '') + end + @user_name = params[:user_name] end def pretext @@ -45,15 +52,15 @@ class SlackService end def new_branch_message - "#{username} pushed new branch #{branch_link} to #{project_link}" + "#{user_name} pushed new #{ref_type} #{branch_link} to #{project_link}" end def removed_branch_message - "#{username} removed branch #{ref} from #{project_link}" + "#{user_name} removed #{ref_type} #{ref} from #{project_link}" end def push_message - "#{username} pushed to branch #{branch_link} of #{project_link} (#{compare_link})" + "#{user_name} pushed to #{ref_type} #{branch_link} of #{project_link} (#{compare_link})" end def commit_messages diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 95ce4f8e4aaba25477447cdc68620245eed8b062..b9f2bee148d2b1f9ee383f155c7ded5d2dda0f8e 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -50,6 +50,35 @@ describe HipchatService do expect(WebMock).to have_requested(:post, api_url).once end + + it "should create a push message" do + message = hipchat.send(:create_push_message, push_sample_data) + + obj_attr = push_sample_data[:object_attributes] + branch = push_sample_data[:ref].gsub('refs/heads/', '') + expect(message).to include("#{user.name} pushed to branch " \ + "#{branch} of " \ + "#{project_name}") + end + end + + context 'tag_push events' do + let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, '000000', '111111', 'refs/tags/test', []) } + + it "should call Hipchat API for tag push events" do + hipchat.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "should create a tag push message" do + message = hipchat.send(:create_push_message, push_sample_data) + + obj_attr = push_sample_data[:object_attributes] + expect(message).to eq("#{user.name} pushed new tag " \ + "test to " \ + "#{project_name}\n") + end end context 'issue events' do @@ -67,8 +96,8 @@ describe HipchatService do message = hipchat.send(:create_issue_message, issues_sample_data) obj_attr = issues_sample_data[:object_attributes] - expect(message).to eq("#{user.username} opened issue " \ - "##{obj_attr["iid"]} in " \ + expect(message).to eq("#{user.name} opened " \ + "issue ##{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome issue" \ "
please fix") @@ -91,8 +120,8 @@ describe HipchatService do merge_sample_data) obj_attr = merge_sample_data[:object_attributes] - expect(message).to eq("#{user.username} opened merge request " \ - "##{obj_attr["iid"]} in " \ + expect(message).to eq("#{user.name} opened " \ + "merge request ##{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome merge request" \ "
please fix") @@ -122,7 +151,7 @@ describe HipchatService do commit_id = Commit.truncate_sha(data[:commit][:id]) title = hipchat.send(:format_title, data[:commit][:message]) - expect(message).to eq("#{user.username} commented on " \ + expect(message).to eq("#{user.name} commented on " \ "commit #{commit_id} in " \ "#{project_name}: " \ "#{title}" \ @@ -141,7 +170,7 @@ describe HipchatService do merge_id = data[:merge_request]['iid'] title = data[:merge_request]['title'] - expect(message).to eq("#{user.username} commented on " \ + expect(message).to eq("#{user.name} commented on " \ "merge request ##{merge_id} in " \ "#{project_name}: " \ "#{title}" \ @@ -158,7 +187,7 @@ describe HipchatService do issue_id = data[:issue]['iid'] title = data[:issue]['title'] - expect(message).to eq("#{user.username} commented on " \ + expect(message).to eq("#{user.name} commented on " \ "issue ##{issue_id} in " \ "#{project_name}: " \ "#{title}" \ @@ -177,7 +206,7 @@ describe HipchatService do snippet_id = data[:snippet]['id'] title = data[:snippet]['title'] - expect(message).to eq("#{user.username} commented on " \ + expect(message).to eq("#{user.name} commented on " \ "snippet ##{snippet_id} in " \ "#{project_name}: " \ "#{title}" \ diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb index a23a7cc068ecdf3c798ec0fbe2ac627fcff8a0c5..8bca1fef44c50f1c281ad2c405fee18e8db6b2cb 100644 --- a/spec/models/project_services/slack_service/issue_message_spec.rb +++ b/spec/models/project_services/slack_service/issue_message_spec.rb @@ -6,7 +6,8 @@ describe SlackService::IssueMessage do let(:args) { { user: { - username: 'username' + name: 'Test User', + username: 'Test User' }, project_name: 'project_name', project_url: 'somewhere.com', @@ -29,8 +30,8 @@ describe SlackService::IssueMessage do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - 'username opened issue