From 07d69d8a6763984536a590690bb58527f6175bc1 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 13 Dec 2016 19:58:26 +0800 Subject: [PATCH] Fix Slack pipeline message by API Pipelines triggered from API don't have a corresponding user, so we show that it's from API, same as from the web UI Closes #25609 --- .../slack_service/pipeline_message.rb | 2 +- .../fix-slack-pipeline-message-by-api.yml | 4 +++ .../slack_service/pipeline_message_spec.rb | 30 +++++++++++++------ 3 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 changelogs/unreleased/fix-slack-pipeline-message-by-api.yml diff --git a/app/models/project_services/slack_service/pipeline_message.rb b/app/models/project_services/slack_service/pipeline_message.rb index f8d03c0e2fa..b6355fc4171 100644 --- a/app/models/project_services/slack_service/pipeline_message.rb +++ b/app/models/project_services/slack_service/pipeline_message.rb @@ -13,7 +13,7 @@ class SlackService @project_name = data[:project][:path_with_namespace] @project_url = data[:project][:web_url] - @user_name = data[:user] && data[:user][:name] + @user_name = (data[:user] && data[:user][:name]) || 'API' end def pretext diff --git a/changelogs/unreleased/fix-slack-pipeline-message-by-api.yml b/changelogs/unreleased/fix-slack-pipeline-message-by-api.yml new file mode 100644 index 00000000000..aa5ad5cd8d6 --- /dev/null +++ b/changelogs/unreleased/fix-slack-pipeline-message-by-api.yml @@ -0,0 +1,4 @@ +--- +title: Fix Slack pipeline message from pipelines made by API +merge_request: 8059 +author: diff --git a/spec/models/project_services/slack_service/pipeline_message_spec.rb b/spec/models/project_services/slack_service/pipeline_message_spec.rb index 363138a9454..4098500122f 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe SlackService::PipelineMessage do subject { SlackService::PipelineMessage.new(args) } + let(:user) { { name: 'hacker' } } let(:args) do { @@ -15,7 +16,7 @@ describe SlackService::PipelineMessage do }, project: { path_with_namespace: 'project_name', web_url: 'example.gitlab.com' }, - user: { name: 'hacker' } + user: user } end @@ -28,9 +29,7 @@ describe SlackService::PipelineMessage do let(:message) { build_message('passed') } it 'returns a message with information about succeeded build' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) + verify_message end end @@ -40,16 +39,29 @@ describe SlackService::PipelineMessage do let(:duration) { 10 } it 'returns a message with information about failed build' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) + verify_message + end + + context 'when triggered by API therefore lacking user' do + let(:user) { nil } + let(:message) { build_message(status, 'API') } + + it 'returns a message stating it is by API' do + verify_message + end end end - def build_message(status_text = status) + def verify_message + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + + def build_message(status_text = status, name = user[:name]) ":" \ " Pipeline " \ " of branch" \ - " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" + " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" end end