diff options
author | Stan Hu <stanhu@gmail.com> | 2015-03-06 08:31:49 -0800 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2015-03-06 08:31:49 -0800 |
commit | ad14ed5e49a956511ae8f4d3bf377457fdb1075d (patch) | |
tree | d7884a56ecea18ea867b63bbbca478a4be96c650 | |
parent | 76802012e62a70bd040ec5cb8efa7e357dda0eee (diff) | |
download | gitlab-ce-ad14ed5e49a956511ae8f4d3bf377457fdb1075d.tar.gz |
Add tag_push event notification to HipChat and Slack services.
Normalize output to use:
- User name instead of username
- Include first line of title in message description
- Link to "Issue #X" instead of "#X"
12 files changed, 134 insertions, 64 deletions
diff --git a/CHANGELOG b/CHANGELOG index 611c6c77d54..c0d38a9abf0 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 d24351a7b13..90ba7e080f1 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 <a href=\""\ + message << "pushed new #{ref_type} <a href=\""\ "#{project_url}/commits/#{URI.escape(ref)}\">#{ref}</a>"\ - " to <a href=\"#{project_url}\">"\ - "#{project_url}</a>\n" + " to #{project_link}\n" elsif after.include?('000000') - message << "removed branch #{ref} from <a href=\"#{project.web_url}\">#{project.name_with_namespace.gsub!(/\s/,'')}</a> \n" + message << "removed #{ref_type} <b>#{ref}</b> from <a href=\"#{project.web_url}\">#{project_name}</a> \n" else - message << "pushed to branch <a href=\""\ + message << "pushed to #{ref_type} <a href=\""\ "#{project.web_url}/commits/#{URI.escape(ref)}\">#{ref}</a> " message << "of <a href=\"#{project.web_url}\">#{project.name_with_namespace.gsub!(/\s/,'')}</a> " message << "(<a href=\"#{project.web_url}/compare/#{before}...#{after}\">Compare changes</a>)" @@ -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 = "<a href=\"#{issue_url}\">##{issue_iid}</a>" - message = "#{username} #{state} issue #{issue_link} in #{project_link}: <b>#{title}</b>" + issue_link = "<a href=\"#{issue_url}\">issue ##{issue_iid}</a>" + message = "#{user_name} #{state} #{issue_link} in #{project_link}: <b>#{title}</b>" 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 = "<a href=\"#{merge_request_url}\">##{merge_request_id}</a>" - message = "#{username} #{state} merge request #{merge_request_link} in " \ + merge_request_link = "<a href=\"#{merge_request_url}\">merge request ##{merge_request_id}</a>" + message = "#{user_name} #{state} #{merge_request_link} in " \ "#{project_link}: <b>#{title}</b>" 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 = "<a href=\"#{note_url}\">#{subject_type} #{subject_desc}</a>" - 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 a58840116f4..36d9874edd3 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 e2fed0bb1b9..5af24a80609 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 4dcce1d15a0..e792c258f73 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 f93dc358f61..074478b292d 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 2e566bc317b..3dc2df04764 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 95ce4f8e4aa..b9f2bee148d 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 " \ + "<a href=\"#{project.web_url}/commits/#{branch}\">#{branch}</a> of " \ + "<a href=\"#{project.web_url}\">#{project_name}</a>") + 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 " \ + "<a href=\"#{project.web_url}/commits/test\">test</a> to " \ + "<a href=\"#{project.web_url}\">#{project_name}</a>\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 " \ - "<a href=\"#{obj_attr[:url]}\">##{obj_attr["iid"]}</a> in " \ + expect(message).to eq("#{user.name} opened " \ + "<a href=\"#{obj_attr[:url]}\">issue ##{obj_attr["iid"]}</a> in " \ "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ "<b>Awesome issue</b>" \ "<pre>please fix</pre>") @@ -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 " \ - "<a href=\"#{obj_attr[:url]}\">##{obj_attr["iid"]}</a> in " \ + expect(message).to eq("#{user.name} opened " \ + "<a href=\"#{obj_attr[:url]}\">merge request ##{obj_attr["iid"]}</a> in " \ "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ "<b>Awesome merge request</b>" \ "<pre>please fix</pre>") @@ -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 " \ "<a href=\"#{obj_attr[:url]}\">commit #{commit_id}</a> in " \ "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ "#{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 " \ "<a href=\"#{obj_attr[:url]}\">merge request ##{merge_id}</a> in " \ "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ "<b>#{title}</b>" \ @@ -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 " \ "<a href=\"#{obj_attr[:url]}\">issue ##{issue_id}</a> in " \ "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ "<b>#{title}</b>" \ @@ -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 " \ "<a href=\"#{obj_attr[:url]}\">snippet ##{snippet_id}</a> in " \ "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ "<b>#{title}</b>" \ 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 a23a7cc068e..8bca1fef44c 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 <url|#100> in <somewhere.com|project_name>: '\ - 'Issue title') + 'Test User opened <url|issue #100> in <somewhere.com|project_name>: '\ + '*Issue title*') expect(subject.attachments).to eq([ { text: "issue description", @@ -47,8 +48,8 @@ describe SlackService::IssueMessage do end it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - 'username closed issue <url|#100> in <somewhere.com|project_name>: '\ - 'Issue title') + 'Test User closed <url|issue #100> in <somewhere.com|project_name>: '\ + '*Issue title*') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/slack_service/merge_message_spec.rb b/spec/models/project_services/slack_service/merge_message_spec.rb index 25d03cd8736..aeb408aa766 100644 --- a/spec/models/project_services/slack_service/merge_message_spec.rb +++ b/spec/models/project_services/slack_service/merge_message_spec.rb @@ -6,13 +6,14 @@ describe SlackService::MergeMessage do let(:args) { { user: { - username: 'username' + name: 'Test User', + username: 'Test User' }, project_name: 'project_name', project_url: 'somewhere.com', object_attributes: { - title: 'Issue title', + title: "Issue title\nSecond line", id: 10, iid: 100, assignee_id: 1, @@ -30,8 +31,8 @@ describe SlackService::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'username opened merge request <somewhere.com/merge_requests/100|#100> '\ - 'in <somewhere.com|project_name>') + 'Test User opened <somewhere.com/merge_requests/100|merge request #100> '\ + 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty end end @@ -42,8 +43,8 @@ describe SlackService::MergeMessage do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'username closed merge request <somewhere.com/merge_requests/100|#100> '\ - 'in <somewhere.com|project_name>') + 'Test User closed <somewhere.com/merge_requests/100|merge request #100> '\ + 'in <somewhere.com|project_name>: *Issue title*') expect(subject.attachments).to be_empty 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 index f2516c10008..21fb575480b 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -37,7 +37,7 @@ describe SlackService::NoteMessage do it 'returns a message regarding notes on commits' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("username commented on " \ + expect(message.pretext).to eq("Test User commented on " \ "<url|commit 5f163b2b> in <somewhere.com|project_name>: " \ "*Added a commit message*") expected_attachments = [ @@ -62,7 +62,7 @@ describe SlackService::NoteMessage do 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 " \ + expect(message.pretext).to eq("Test User commented on " \ "<url|merge request #30> in <somewhere.com|project_name>: " \ "*merge request title*") expected_attachments = [ @@ -89,7 +89,7 @@ describe SlackService::NoteMessage do it 'returns a message regarding notes on an issue' do message = SlackService::NoteMessage.new(@args) expect(message.pretext).to eq( - "username commented on " \ + "Test User commented on " \ "<url|issue #20> in <somewhere.com|project_name>: " \ "*issue title*") expected_attachments = [ @@ -114,7 +114,7 @@ describe SlackService::NoteMessage do it 'returns a message regarding notes on a project snippet' do message = SlackService::NoteMessage.new(@args) - expect(message.pretext).to eq("username commented on " \ + expect(message.pretext).to eq("Test User commented on " \ "<url|snippet #5> in <somewhere.com|project_name>: " \ "*snippet title*") expected_attachments = [ diff --git a/spec/models/project_services/slack_service/push_message_spec.rb b/spec/models/project_services/slack_service/push_message_spec.rb index ef0e7a6ee30..3ef065459d8 100644 --- a/spec/models/project_services/slack_service/push_message_spec.rb +++ b/spec/models/project_services/slack_service/push_message_spec.rb @@ -39,6 +39,26 @@ describe SlackService::PushMessage do end end + context 'tag push' do + let(:args) { + { + after: 'after', + before: '000000', + project_name: 'project_name', + ref: 'refs/tags/new_tag', + user_name: 'user_name', + project_url: 'url' + } + } + + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq('user_name pushed new tag ' \ + '<url/commits/new_tag|new_tag> to ' \ + '<url|project_name>') + expect(subject.attachments).to be_empty + end + end + context 'new branch' do before do args[:before] = '000000' |