Commit ccb5daee authored by Robert Speicher's avatar Robert Speicher

Merge branch '5571-hipchat-notifications-missing-colors' into 'master'

Resolve "HipChat notifications missing colors"

_Originally opened at !2322 by @eisnerd._

- - -

## What does this MR do?

This MR restores the colors mentioned in #5571 by overriding the color setting with green/red just for build events.

## What are the relevant issue numbers?

Closes #5571.

See merge request !5498
parents a8bbeb48 386478d8
......@@ -11,6 +11,7 @@ v 8.11.0 (unreleased)
- Add green outline to New Branch button. !5447 (winniehell)
- Retrieve rendered HTML from cache in one request
- Nokogiri's various parsing methods are now instrumented
- Add build event color in HipChat messages (David Eisner)
- Make fork counter always clickable. !5463 (winniehell)
- Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le)
- Load project invited groups and members eagerly in `ProjectTeam#fetch_members`
......
......@@ -46,7 +46,7 @@ class HipchatService < Service
return unless supported_events.include?(data[:object_kind])
message = create_message(data)
return unless message.present?
gate[room].send('GitLab', message, message_options)
gate[room].send('GitLab', message, message_options(data))
end
def test(data)
......@@ -67,8 +67,8 @@ class HipchatService < Service
@gate ||= HipChat::Client.new(token, options)
end
def message_options
{ notify: notify.present? && notify == '1', color: color || 'yellow' }
def message_options(data = nil)
{ notify: notify.present? && notify == '1', color: message_color(data) }
end
def create_message(data)
......@@ -240,6 +240,21 @@ class HipchatService < Service
"#{project_link}: Commit #{commit_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status(status)} in #{duration} second(s)"
end
def message_color(data)
build_status_color(data) || color || 'yellow'
end
def build_status_color(data)
return unless data && data[:object_kind] == 'build'
case data[:commit][:status]
when 'success'
'green'
else
'red'
end
end
def project_name
project.name_with_namespace.gsub(/\s/, '')
end
......
......@@ -340,18 +340,36 @@ describe HipchatService, models: true do
end
context "#message_options" do
it "should be set to the defaults" do
expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'yellow' })
it "is set to the defaults" do
expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'yellow' })
end
it "should set notfiy to true" do
it "sets notify to true" do
allow(hipchat).to receive(:notify).and_return('1')
expect(hipchat.send(:message_options)).to eq({ notify: true, color: 'yellow' })
expect(hipchat.__send__(:message_options)).to eq({ notify: true, color: 'yellow' })
end
it "should set the color" do
it "sets the color" do
allow(hipchat).to receive(:color).and_return('red')
expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'red' })
expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'red' })
end
context 'with a successful build' do
it 'uses the green color' do
build_data = { object_kind: 'build', commit: { status: 'success' } }
expect(hipchat.__send__(:message_options, build_data)).to eq({ notify: false, color: 'green' })
end
end
context 'with a failed build' do
it 'uses the red color' do
build_data = { object_kind: 'build', commit: { status: 'failed' } }
expect(hipchat.__send__(:message_options, build_data)).to eq({ notify: false, color: 'red' })
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