Commit 43493eca authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'slack-issue-format' into 'master'

Improve issue formatting in Slack service

This will improve the looks of the message that gets send to Slack.

**Before**

![image](https://gitlab.com/jeroenvanbaarsen/gitlab-ce/uploads/670442d4d5cef5aa36753671b8894a75/image.png)

**After**

![image](https://gitlab.com/jeroenvanbaarsen/gitlab-ce/uploads/ab6e40a0c22ebe89ebd5b3652821c6dd/image.png)

There is one thing I'm not certain about, thats sending the logo with the payload. Because this means we need to host the GitLab logo somewhere public, and every time a payload from any GitLab instance is send, we use that logo url. This might overload the server, or drive the bandwidth bill through the roof.
This is something @sytses or @dzaporozhets have to decide about.

@stanhu, you told me you were the author of the original code, maybe you can review?

See merge request !1308
parents 5da1c23b 78e2d0ac
...@@ -59,6 +59,7 @@ v 8.8.0 (unreleased) ...@@ -59,6 +59,7 @@ v 8.8.0 (unreleased)
- Add counter metrics for rails cache - Add counter metrics for rails cache
- Import pull requests from GitHub where the source or target branches were removed - Import pull requests from GitHub where the source or target branches were removed
- All Grape API helpers are now instrumented - All Grape API helpers are now instrumented
- Improve Issue formatting for the Slack Service (Jeroen van Baarsen)
v 8.7.6 v 8.7.6
- Fix links on wiki pages for relative url setups. !4131 (Artem Sidorenko) - Fix links on wiki pages for relative url setups. !4131 (Artem Sidorenko)
...@@ -893,7 +894,7 @@ v 8.1.3 ...@@ -893,7 +894,7 @@ v 8.1.3
- Use issue editor as cross reference comment author when issue is edited with a new mention - Use issue editor as cross reference comment author when issue is edited with a new mention
- Add Facebook authentication - Add Facebook authentication
v 8.1.2 v 8.1.1
- Fix cloning Wiki repositories via HTTP (Stan Hu) - Fix cloning Wiki repositories via HTTP (Stan Hu)
- Add migration to remove satellites directory - Add migration to remove satellites directory
- Fix specific runners visibility - Fix specific runners visibility
......
...@@ -34,7 +34,12 @@ class SlackService ...@@ -34,7 +34,12 @@ class SlackService
private private
def message def message
"#{user_name} #{state} #{issue_link} in #{project_link}: *#{title}*" case state
when "opened"
"[#{project_link}] Issue #{state} by #{user_name}"
else
"[#{project_link}] Issue #{issue_link} #{state} by #{user_name}"
end
end end
def opened_issue? def opened_issue?
...@@ -42,7 +47,11 @@ class SlackService ...@@ -42,7 +47,11 @@ class SlackService
end end
def description_message def description_message
[{ text: format(description), color: attachment_color }] [{
title: issue_title,
title_link: issue_url,
text: format(description),
color: "#C95823" }]
end end
def project_link def project_link
...@@ -50,7 +59,11 @@ class SlackService ...@@ -50,7 +59,11 @@ class SlackService
end end
def issue_link def issue_link
"[issue ##{issue_iid}](#{issue_url})" "[#{issue_title}](#{issue_url})"
end
def issue_title
"##{issue_iid} #{title}"
end end
end end
end end
...@@ -25,7 +25,7 @@ describe SlackService::IssueMessage, models: true do ...@@ -25,7 +25,7 @@ describe SlackService::IssueMessage, models: true do
} }
end end
let(:color) { '#345' } let(:color) { '#C95823' }
context '#initialize' do context '#initialize' do
before do before do
...@@ -40,10 +40,11 @@ describe SlackService::IssueMessage, models: true do ...@@ -40,10 +40,11 @@ describe SlackService::IssueMessage, models: true do
context 'open' do context 'open' do
it 'returns a message regarding opening of issues' do it 'returns a message regarding opening of issues' do
expect(subject.pretext).to eq( expect(subject.pretext).to eq(
'Test User opened <url|issue #100> in <somewhere.com|project_name>: '\ '<somewhere.com|[project_name>] Issue opened by Test User')
'*Issue title*')
expect(subject.attachments).to eq([ expect(subject.attachments).to eq([
{ {
title: "#100 Issue title",
title_link: "url",
text: "issue description", text: "issue description",
color: color, color: color,
} }
...@@ -56,10 +57,10 @@ describe SlackService::IssueMessage, models: true do ...@@ -56,10 +57,10 @@ describe SlackService::IssueMessage, models: true do
args[:object_attributes][:action] = 'close' args[:object_attributes][:action] = 'close'
args[:object_attributes][:state] = 'closed' args[:object_attributes][:state] = 'closed'
end end
it 'returns a message regarding closing of issues' do it 'returns a message regarding closing of issues' do
expect(subject.pretext). to eq( expect(subject.pretext). to eq(
'Test User closed <url|issue #100> in <somewhere.com|project_name>: '\ '<somewhere.com|[project_name>] Issue <url|#100 Issue title> closed by Test User')
'*Issue title*')
expect(subject.attachments).to be_empty expect(subject.attachments).to be_empty
end end
end end
......
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