Commit 1cc41ce1 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'normalize-slack-hipchat-messages' into 'master'

 Add tag push notifications to HipChat and Slack messages and normalize output of messages to be consistent

Add tag_push event notification to HipChat and Slack services.

For each service, normalize message output to use:
- User name instead of username
- Include first line of title in message description
- Link to "Issue #X" instead of "#X"

See merge request !370
parents 0625d68f ad14ed5e
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
......
......@@ -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
......
......@@ -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)
......
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
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
......
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
......
......@@ -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
......
......@@ -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>" \
......
......@@ -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
......
......@@ -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
......
......@@ -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 = [
......
......@@ -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'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment