From ae5f124e96504f20500df282d34dce7b0627c212 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 28 Jul 2016 01:16:31 +0800 Subject: [PATCH 01/34] WIP, initial work to implement pipeline hooks: I might be squashing this commit in the future. --- app/controllers/projects/hooks_controller.rb | 1 + app/models/ci/pipeline.rb | 6 ++ app/models/hooks/project_hook.rb | 1 + app/models/hooks/web_hook.rb | 1 + app/models/service.rb | 1 + ...081025_add_pipeline_events_to_web_hooks.rb | 16 +++++ ...8103734_add_pipeline_events_to_services.rb | 16 +++++ .../data_builder/pipeline_data_builder.rb | 65 +++++++++++++++++++ 8 files changed, 107 insertions(+) create mode 100644 db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb create mode 100644 db/migrate/20160728103734_add_pipeline_events_to_services.rb create mode 100644 lib/gitlab/data_builder/pipeline_data_builder.rb diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index a60027ff477..b5624046387 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -56,6 +56,7 @@ class Projects::HooksController < Projects::ApplicationController def hook_params params.require(:hook).permit( :build_events, + :pipeline_events, :enable_ssl_verification, :issues_events, :merge_requests_events, diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bce6a992af6..bf6d872dece 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -213,6 +213,12 @@ module Ci ] end + def execute_hooks + pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self) + project.execute_hooks(pipeline_data.dup, :pipeline_hooks) + project.execute_services(pipeline_data.dup, :pipeline_hooks) + end + private def build_builds_for_stages(stages, user, status, trigger_request) diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index ba42a8eeb70..836a75b0608 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -5,5 +5,6 @@ class ProjectHook < WebHook scope :note_hooks, -> { where(note_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) } scope :build_hooks, -> { where(build_events: true) } + scope :pipeline_hooks, -> { where(pipeline_events: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true) } end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 8b87b6c3d64..f365dee3141 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -8,6 +8,7 @@ class WebHook < ActiveRecord::Base default_value_for :merge_requests_events, false default_value_for :tag_push_events, false default_value_for :build_events, false + default_value_for :pipeline_events, false default_value_for :enable_ssl_verification, true scope :push_hooks, -> { where(push_events: true) } diff --git a/app/models/service.rb b/app/models/service.rb index 40cd9b861f0..4a63abb2724 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -36,6 +36,7 @@ class Service < ActiveRecord::Base scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } + scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } diff --git a/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb new file mode 100644 index 00000000000..b800e6d7283 --- /dev/null +++ b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb @@ -0,0 +1,16 @@ +class AddPipelineEventsToWebHooks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:web_hooks, :pipeline_events, :boolean, + default: false, allow_null: false) + end + + def down + remove_column(:web_hooks, :pipeline_events) + end +end diff --git a/db/migrate/20160728103734_add_pipeline_events_to_services.rb b/db/migrate/20160728103734_add_pipeline_events_to_services.rb new file mode 100644 index 00000000000..bcd24fe1566 --- /dev/null +++ b/db/migrate/20160728103734_add_pipeline_events_to_services.rb @@ -0,0 +1,16 @@ +class AddPipelineEventsToServices < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:services, :pipeline_events, :boolean, + default: false, allow_null: false) + end + + def down + remove_column(:services, :pipeline_events) + end +end diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb new file mode 100644 index 00000000000..a9c1bc7acee --- /dev/null +++ b/lib/gitlab/data_builder/pipeline_data_builder.rb @@ -0,0 +1,65 @@ +module Gitlab + module DataBuilder + module PipelineDataBuilder + module_function + + def build(pipeline) + { + object_kind: 'pipeline', + user: pipeline.user.hook_attrs, + project: pipeline.project.hook_attrs(backward: false), + commit: pipeline.commit.hook_attrs, + object_attributes: hook_attrs(pipeline), + builds: pipeline.builds.map(&method(:build_hook_attrs)) + } + end + + def hook_attrs(pipeline) + first_pending_build = pipeline.builds.first_pending + config_processor = pipeline.config_processor + + { + id: pipeline.id, + ref: pipeline.ref, + tag: pipeline.tag, + sha: pipeline.sha, + before_sha: pipeline.before_sha, + status: pipeline.status, + stage: first_pending_build.try(:stage), + stages: config_processor.try(:stages), + created_at: pipeline.created_at, + finished_at: pipeline.finished_at + } + end + + def build_hook_attrs(build) + { + id: build.id, + stage: build.stage, + name: build.name, + status: build.status, + created_at: build.created_at, + started_at: build.started_at, + finished_at: build.finished_at, + when: build.when, + manual: build.manual?, + user: build.user.hook_attrs, + runner: runner_hook_attrs(build.runner), + artifacts_file: { + filename: build.artifacts_file.filename, + size: build.artifacts_size + } + } + end + + def runner_hook_attrs(runner) + { + id: runner.id, + description: runner.description, + active: runner.active?, + is_shared: runner.is_shared? + } + end + end + end +end From 751be82ee189cad83c59516881edfac51f1ce379 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 28 Jul 2016 19:33:40 +0800 Subject: [PATCH 02/34] Add views for pipeline events --- app/views/projects/hooks/_project_hook.html.haml | 2 +- app/views/shared/web_hooks/_form.html.haml | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml index 8151187d499..3fcf1692e09 100644 --- a/app/views/projects/hooks/_project_hook.html.haml +++ b/app/views/projects/hooks/_project_hook.html.haml @@ -3,7 +3,7 @@ .col-md-8.col-lg-7 %strong.light-header= hook.url %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events wiki_page_events).each do |trigger| + - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray.deploy-project-label= trigger.titleize .col-md-4.col-lg-5.text-right-lg.prepend-top-5 diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index 2585ed9360b..106161d6515 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -65,6 +65,13 @@ %strong Build events %p.light This url will be triggered when the build status changes + %li + = f.check_box :pipeline_events, class: 'pull-left' + .prepend-left-20 + = f.label :pipeline_events, class: 'list-label' do + %strong Pipeline events + %p.light + This url will be triggered when the pipeline status changes %li = f.check_box :wiki_page_events, class: 'pull-left' .prepend-left-20 From 6d82e3f15f8de3bae316cdc2f204b3cdea88b55f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 28 Jul 2016 19:43:36 +0800 Subject: [PATCH 03/34] We're not using original hash anyway, we could use it --- app/models/ci/pipeline.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bf6d872dece..7d23456cdac 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -215,7 +215,7 @@ module Ci def execute_hooks pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self) - project.execute_hooks(pipeline_data.dup, :pipeline_hooks) + project.execute_hooks(pipeline_data, :pipeline_hooks) project.execute_services(pipeline_data.dup, :pipeline_hooks) end From 755301a2ad1d307c727e3c2642c9e234a7ddb05d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 29 Jul 2016 00:37:35 +0800 Subject: [PATCH 04/34] Also touch the pipeline so we could just hook into update_state --- app/models/ci/pipeline.rb | 13 +++++++------ app/services/ci/create_pipeline_service.rb | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 7d23456cdac..cd6ead4ded2 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -213,12 +213,6 @@ module Ci ] end - def execute_hooks - pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self) - project.execute_hooks(pipeline_data, :pipeline_hooks) - project.execute_services(pipeline_data.dup, :pipeline_hooks) - end - private def build_builds_for_stages(stages, user, status, trigger_request) @@ -244,6 +238,13 @@ module Ci self.finished_at = statuses.finished_at self.duration = statuses.latest.duration save + execute_hooks + end + + def execute_hooks + pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self) + project.execute_hooks(pipeline_data, :pipeline_hooks) + project.execute_services(pipeline_data.dup, :pipeline_hooks) end def keep_around_commits diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index be91bf0db85..7a8b0683acb 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -27,6 +27,7 @@ module Ci end pipeline.save! + pipeline.touch unless pipeline.create_builds(current_user) pipeline.errors.add(:base, 'No builds for this pipeline.') From b831ef716b088fa5f0892ececd00d4a383267979 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 29 Jul 2016 02:05:50 +0800 Subject: [PATCH 05/34] They could be nil --- lib/gitlab/data_builder/pipeline_data_builder.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb index a9c1bc7acee..fed9bd92ba4 100644 --- a/lib/gitlab/data_builder/pipeline_data_builder.rb +++ b/lib/gitlab/data_builder/pipeline_data_builder.rb @@ -6,9 +6,9 @@ module Gitlab def build(pipeline) { object_kind: 'pipeline', - user: pipeline.user.hook_attrs, + user: pipeline.user.try(:hook_attrs), project: pipeline.project.hook_attrs(backward: false), - commit: pipeline.commit.hook_attrs, + commit: pipeline.commit.try(:hook_attrs), object_attributes: hook_attrs(pipeline), builds: pipeline.builds.map(&method(:build_hook_attrs)) } @@ -43,8 +43,8 @@ module Gitlab finished_at: build.finished_at, when: build.when, manual: build.manual?, - user: build.user.hook_attrs, - runner: runner_hook_attrs(build.runner), + user: build.user.try(:hook_attrs), + runner: build.runner && runner_hook_attrs(build.runner), artifacts_file: { filename: build.artifacts_file.filename, size: build.artifacts_size From d41e83e91a2b2c90ab51feda61e105818e1713be Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 29 Jul 2016 17:06:09 +0800 Subject: [PATCH 06/34] Don't execute hooks if ci was supposed to be skipped And we should preserve the return value --- app/models/ci/pipeline.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index cd6ead4ded2..4e6ccf48c68 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -237,8 +237,9 @@ module Ci self.started_at = statuses.started_at self.finished_at = statuses.finished_at self.duration = statuses.latest.duration - save - execute_hooks + saved = save + execute_hooks if saved && !skip_ci? + saved end def execute_hooks From 5fee7ec1a0016fb8789abf2f3b7e2faaf24518f5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 29 Jul 2016 17:58:26 +0800 Subject: [PATCH 07/34] It was never used --- app/models/project_services/slack_service/build_message.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/project_services/slack_service/build_message.rb b/app/models/project_services/slack_service/build_message.rb index 69c21b3fc38..0fca4267bad 100644 --- a/app/models/project_services/slack_service/build_message.rb +++ b/app/models/project_services/slack_service/build_message.rb @@ -9,7 +9,7 @@ class SlackService attr_reader :user_name attr_reader :duration - def initialize(params, commit = true) + def initialize(params) @sha = params[:sha] @ref_type = params[:tag] ? 'tag' : 'branch' @ref = params[:ref] @@ -36,7 +36,7 @@ class SlackService def message "#{project_link}: Commit #{commit_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status} in #{duration} #{'second'.pluralize(duration)}" - end + end def format(string) Slack::Notifier::LinkFormatter.format(string) From 4a431a266f8773c54a0f47292d4b9470a7d2acd3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 29 Jul 2016 20:20:04 +0800 Subject: [PATCH 08/34] Fix that tricky side-effect issue in the test --- app/models/ci/build.rb | 2 +- spec/models/build_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index aac78d75f57..e5523c42a3b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -460,7 +460,7 @@ module Ci def build_attributes_from_config return {} unless pipeline.config_processor - + pipeline.config_processor.build_attributes(name) end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index dc88697199b..47c489e6af1 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -275,7 +275,8 @@ describe Ci::Build, models: true do context 'when yaml_variables are undefined' do before do - build.yaml_variables = nil + build.update(yaml_variables: nil) + build.reload # reload pipeline so that it resets config_processor end context 'use from gitlab-ci.yml' do @@ -854,7 +855,8 @@ describe Ci::Build, models: true do context 'if is undefined' do before do - build.when = nil + build.update(when: nil) + build.reload # reload pipeline so that it resets config_processor end context 'use from gitlab-ci.yml' do From d27095a36a5d8c83182771d7548a13fcc9188e3c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 30 Jul 2016 00:59:54 +0800 Subject: [PATCH 09/34] Add pipeline_events to Slack service: Also add Service#event_names so that we don't have to hard code all event names in ServiceParams. Note that I don't know why we want to call plural on issue_event and merge_request_event so that we still need to hard code them for issues_event and merge_requests_event. See app/helpers/services_helper.rb for those special rules. --- app/controllers/concerns/service_params.rb | 15 ++-- app/models/project_services/slack_service.rb | 20 ++++- .../slack_service/pipeline_message.rb | 77 +++++++++++++++++++ app/models/service.rb | 4 + .../data_builder/pipeline_data_builder.rb | 3 +- .../slack_service/pipeline_message_spec.rb | 58 ++++++++++++++ 6 files changed, 166 insertions(+), 11 deletions(-) create mode 100644 app/models/project_services/slack_service/pipeline_message.rb create mode 100644 spec/models/project_services/slack_service/pipeline_message_spec.rb diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index 471d15af913..58877c5ad5d 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -7,11 +7,12 @@ module ServiceParams :build_key, :server, :teamcity_url, :drone_url, :build_type, :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel, :colorize_messages, :channels, - :push_events, :issues_events, :merge_requests_events, :tag_push_events, - :note_events, :build_events, :wiki_page_events, - :notify_only_broken_builds, :add_pusher, - :send_from_committer_email, :disable_diffs, :external_wiki_url, - :notify, :color, + # See app/helpers/services_helper.rb + # for why we need issues_events and merge_requests_events. + :issues_events, :merge_requests_events, + :notify_only_broken_builds, :notify_only_broken_pipelines, + :add_pusher, :send_from_committer_email, :disable_diffs, + :external_wiki_url, :notify, :color, :server_host, :server_port, :default_irc_uri, :enable_ssl_verification, :jira_issue_transition_id] @@ -19,9 +20,7 @@ module ServiceParams FILTER_BLANK_PARAMS = [:password] def service_params - dynamic_params = [] - dynamic_params.concat(@service.event_channel_names) - + dynamic_params = @service.event_channel_names + @service.event_names service_params = params.permit(:id, service: ALLOWED_PARAMS + dynamic_params) if service_params[:service].is_a?(Hash) diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index abbc780dc1a..6584646e998 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -1,6 +1,6 @@ class SlackService < Service prop_accessor :webhook, :username, :channel - boolean_accessor :notify_only_broken_builds + boolean_accessor :notify_only_broken_builds, :notify_only_broken_pipelines validates :webhook, presence: true, url: true, if: :activated? def initialize_properties @@ -10,6 +10,7 @@ class SlackService < Service if properties.nil? self.properties = {} self.notify_only_broken_builds = true + self.notify_only_broken_pipelines = true end end @@ -38,13 +39,14 @@ class SlackService < Service { type: 'text', name: 'username', placeholder: 'username' }, { type: 'text', name: 'channel', placeholder: "#general" }, { type: 'checkbox', name: 'notify_only_broken_builds' }, + { type: 'checkbox', name: 'notify_only_broken_pipelines' }, ] default_fields + build_event_channels end def supported_events - %w(push issue merge_request note tag_push build wiki_page) + %w(push issue merge_request note tag_push build pipeline wiki_page) end def execute(data) @@ -74,6 +76,8 @@ class SlackService < Service NoteMessage.new(data) when "build" BuildMessage.new(data) if should_build_be_notified?(data) + when "pipeline" + PipelineMessage.new(data) if should_pipeline_be_notified?(data) when "wiki_page" WikiPageMessage.new(data) end @@ -142,6 +146,17 @@ class SlackService < Service false end end + + def should_pipeline_be_notified?(data) + case data[:object_attributes][:status] + when 'success' + !notify_only_broken_builds? + when 'failed' + true + else + false + end + end end require "slack_service/issue_message" @@ -149,4 +164,5 @@ require "slack_service/push_message" require "slack_service/merge_message" require "slack_service/note_message" require "slack_service/build_message" +require "slack_service/pipeline_message" require "slack_service/wiki_page_message" diff --git a/app/models/project_services/slack_service/pipeline_message.rb b/app/models/project_services/slack_service/pipeline_message.rb new file mode 100644 index 00000000000..3d12af3fa7e --- /dev/null +++ b/app/models/project_services/slack_service/pipeline_message.rb @@ -0,0 +1,77 @@ +class SlackService + class PipelineMessage < BaseMessage + attr_reader :sha, :ref_type, :ref, :status, :project_name, :project_url, + :user_name, :duration, :pipeline_id + + def initialize(data) + @sha = data[:sha] + @ref_type = data[:tag] ? 'tag' : 'branch' + @ref = data[:ref] + @status = data[:status] + @project_name = data[:project][:path_with_namespace] + @project_url = data[:project][:web_url] + @user_name = data[:commit] && data[:commit][:author_name] + @duration = data[:object_attributes][:duration] + @pipeline_id = data[:object_attributes][:id] + end + + def pretext + '' + end + + def fallback + format(message) + end + + def attachments + [{ text: format(message), color: attachment_color }] + end + + private + + def message + "#{project_link}: Pipeline #{pipeline_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status} in #{duration} #{'second'.pluralize(duration)}" + end + + def format(string) + Slack::Notifier::LinkFormatter.format(string) + end + + def humanized_status + case status + when 'success' + 'passed' + else + status + end + end + + def attachment_color + if status == 'success' + 'good' + else + 'danger' + end + end + + def branch_url + "#{project_url}/commits/#{ref}" + end + + def branch_link + "[#{ref}](#{branch_url})" + end + + def project_link + "[#{project_name}](#{project_url})" + end + + def pipeline_url + "#{project_url}/pipelines/#{pipeline_id}" + end + + def pipeline_link + "[#{Commit.truncate_sha(sha)}](#{pipeline_url})" + end + end +end diff --git a/app/models/service.rb b/app/models/service.rb index 4a63abb2724..e4cd44f542a 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -87,6 +87,10 @@ class Service < ActiveRecord::Base [] end + def event_names + supported_events.map { |event| "#{event}_events" } + end + def event_field(event) nil end diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb index fed9bd92ba4..46f8a1857b4 100644 --- a/lib/gitlab/data_builder/pipeline_data_builder.rb +++ b/lib/gitlab/data_builder/pipeline_data_builder.rb @@ -28,7 +28,8 @@ module Gitlab stage: first_pending_build.try(:stage), stages: config_processor.try(:stages), created_at: pipeline.created_at, - finished_at: pipeline.finished_at + finished_at: pipeline.finished_at, + duration: pipeline.duration } end diff --git a/spec/models/project_services/slack_service/pipeline_message_spec.rb b/spec/models/project_services/slack_service/pipeline_message_spec.rb new file mode 100644 index 00000000000..2960a200e9d --- /dev/null +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe SlackService::PipelineMessage do + subject { SlackService::PipelineMessage.new(args) } + + let(:args) do + { + sha: '97de212e80737a608d939f648d959671fb0a0142', + tag: false, + ref: 'develop', + status: status, + project: { path_with_namespace: 'project_name', + web_url: 'somewhere.com' }, + commit: { author_name: 'hacker' }, + object_attributes: { duration: duration, + id: 123 } + } + end + + context 'succeeded' do + let(:status) { 'success' } + let(:color) { 'good' } + let(:duration) { 10 } + + it 'returns a message with information about succeeded build' do + message = ': Pipeline of branch by hacker passed in 10 seconds' + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end + + context 'failed' do + let(:status) { 'failed' } + let(:color) { 'danger' } + let(:duration) { 10 } + + it 'returns a message with information about failed build' do + message = ': Pipeline of branch by hacker failed in 10 seconds' + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end + + describe '#seconds_name' do + let(:status) { 'failed' } + let(:color) { 'danger' } + let(:duration) { 1 } + + it 'returns seconds as singular when there is only one' do + message = ': Pipeline of branch by hacker failed in 1 second' + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end +end From 3caf7bffdc9cffb0ff60af25cb97b19f672cdb63 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 30 Jul 2016 01:18:35 +0800 Subject: [PATCH 10/34] Reduce complexity by extracting it to a method --- app/models/project_services/slack_service.rb | 37 +++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 6584646e998..4e14a979833 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -64,23 +64,7 @@ class SlackService < Service # 'close' action. Ignore update events for now to prevent duplicate # messages from arriving. - message = \ - case object_kind - when "push", "tag_push" - PushMessage.new(data) - when "issue" - IssueMessage.new(data) unless is_update?(data) - when "merge_request" - MergeMessage.new(data) unless is_update?(data) - when "note" - NoteMessage.new(data) - when "build" - BuildMessage.new(data) if should_build_be_notified?(data) - when "pipeline" - PipelineMessage.new(data) if should_pipeline_be_notified?(data) - when "wiki_page" - WikiPageMessage.new(data) - end + message = get_message(object_kind, data) opt = {} @@ -109,6 +93,25 @@ class SlackService < Service private + def get_message(object_kind, data) + case object_kind + when "push", "tag_push" + PushMessage.new(data) + when "issue" + IssueMessage.new(data) unless is_update?(data) + when "merge_request" + MergeMessage.new(data) unless is_update?(data) + when "note" + NoteMessage.new(data) + when "build" + BuildMessage.new(data) if should_build_be_notified?(data) + when "pipeline" + PipelineMessage.new(data) if should_pipeline_be_notified?(data) + when "wiki_page" + WikiPageMessage.new(data) + end + end + def get_channel_field(event) field_name = event_channel_name(event) self.public_send(field_name) From cc4b0a55bd4a2ef5992c8b5ea0102b0dc40cf616 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Sat, 30 Jul 2016 01:24:03 +0800 Subject: [PATCH 11/34] Fix test for checking everything for Slack service --- features/steps/admin/settings.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb index 03f87df7a60..11dc7f580f0 100644 --- a/features/steps/admin/settings.rb +++ b/features/steps/admin/settings.rb @@ -33,6 +33,7 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps page.check('Issue') page.check('Merge request') page.check('Build') + page.check('Pipeline') click_on 'Save' end From 7209ea4fd7aac3517472851d107567e89eccb7e8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 16:06:57 +0800 Subject: [PATCH 12/34] Slack pipeline events test and fix some issues along the way --- app/models/project_services/slack_service.rb | 20 +++--- .../slack_service/pipeline_message.rb | 14 ++-- .../project_services/slack_service_spec.rb | 66 +++++++++++++++++-- 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 4e14a979833..a46ff475750 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -66,16 +66,20 @@ class SlackService < Service message = get_message(object_kind, data) - opt = {} - - event_channel = get_channel_field(object_kind) || channel - - opt[:channel] = event_channel if event_channel - opt[:username] = username if username - if message + opt = {} + + event_channel = get_channel_field(object_kind) || channel + + opt[:channel] = event_channel if event_channel + opt[:username] = username if username + notifier = Slack::Notifier.new(webhook, opt) notifier.ping(message.pretext, attachments: message.attachments, fallback: message.fallback) + + true + else + false end end @@ -153,7 +157,7 @@ class SlackService < Service def should_pipeline_be_notified?(data) case data[:object_attributes][:status] when 'success' - !notify_only_broken_builds? + !notify_only_broken_pipelines? when 'failed' true else diff --git a/app/models/project_services/slack_service/pipeline_message.rb b/app/models/project_services/slack_service/pipeline_message.rb index 3d12af3fa7e..f06b3562965 100644 --- a/app/models/project_services/slack_service/pipeline_message.rb +++ b/app/models/project_services/slack_service/pipeline_message.rb @@ -4,15 +4,17 @@ class SlackService :user_name, :duration, :pipeline_id def initialize(data) - @sha = data[:sha] - @ref_type = data[:tag] ? 'tag' : 'branch' - @ref = data[:ref] - @status = data[:status] + pipeline_attributes = data[:object_attributes] + @sha = pipeline_attributes[:sha] + @ref_type = pipeline_attributes[:tag] ? 'tag' : 'branch' + @ref = pipeline_attributes[:ref] + @status = pipeline_attributes[:status] + @duration = pipeline_attributes[:duration] + @pipeline_id = pipeline_attributes[:id] + @project_name = data[:project][:path_with_namespace] @project_url = data[:project][:web_url] @user_name = data[:commit] && data[:commit][:author_name] - @duration = data[:object_attributes][:duration] - @pipeline_id = data[:object_attributes][:id] end def pretext diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index df511b1bc4c..9fc097f2ce0 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -21,6 +21,9 @@ require 'spec_helper' describe SlackService, models: true do + let(:slack) { SlackService.new } + let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } + describe "Associations" do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } @@ -42,11 +45,9 @@ describe SlackService, models: true do end describe "Execute" do - let(:slack) { SlackService.new } let(:user) { create(:user) } let(:project) { create(:project) } let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } - let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } let(:username) { 'slack_username' } let(:channel) { 'slack_channel' } @@ -210,10 +211,8 @@ describe SlackService, models: true do end describe "Note events" do - let(:slack) { SlackService.new } let(:user) { create(:user) } let(:project) { create(:project, creator_id: user.id) } - let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } before do allow(slack).to receive_messages( @@ -283,4 +282,63 @@ describe SlackService, models: true do end end end + + describe 'Pipeline events' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:pipeline) do + create(:ci_pipeline, + project: project, status: status, + sha: project.commit.sha, ref: project.default_branch) + end + let(:status) { raise NotImplementedError } + + before do + allow(slack).to receive_messages( + project: project, + service_hook: true, + webhook: webhook_url + ) + end + + shared_examples 'call Slack API' do + before do + WebMock.stub_request(:post, webhook_url) + end + + it 'calls Slack API for pipeline events' do + data = Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) + slack.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with failed pipeline' do + let(:status) { 'failed' } + + it_behaves_like 'call Slack API' + end + + context 'with succeeded pipeline' do + let(:status) { 'success' } + + context 'with default to notify_only_broken_pipelines' do + it 'does not call Slack API for pipeline events' do + data = Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) + result = slack.execute(data) + + expect(result).to be_falsy + end + end + + context 'with setting notify_only_broken_pipelines to false' do + before do + slack.notify_only_broken_pipelines = false + end + + it_behaves_like 'call Slack API' + end + end + end end From 3b943fbb9603dd5babe7f9348aef79a52ac10c45 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 17:09:26 +0800 Subject: [PATCH 13/34] Fix the test due to the format changed --- .../slack_service/pipeline_message_spec.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) 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 2960a200e9d..a292defee66 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -5,15 +5,17 @@ describe SlackService::PipelineMessage do let(:args) do { - sha: '97de212e80737a608d939f648d959671fb0a0142', - tag: false, - ref: 'develop', - status: status, + object_attributes: { + id: 123, + sha: '97de212e80737a608d939f648d959671fb0a0142', + tag: false, + ref: 'develop', + status: status, + duration: duration + }, project: { path_with_namespace: 'project_name', web_url: 'somewhere.com' }, - commit: { author_name: 'hacker' }, - object_attributes: { duration: duration, - id: 123 } + commit: { author_name: 'hacker' } } end From ef0613716c3e2a3cbfedb9b6652e501dafd6804b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 18:02:53 +0800 Subject: [PATCH 14/34] Add test for PipelineDataBuilder --- .../data_builder/pipeline_data_builder.rb | 2 +- .../pipeline_data_builder_spec.rb | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb index 46f8a1857b4..13417ba09eb 100644 --- a/lib/gitlab/data_builder/pipeline_data_builder.rb +++ b/lib/gitlab/data_builder/pipeline_data_builder.rb @@ -6,10 +6,10 @@ module Gitlab def build(pipeline) { object_kind: 'pipeline', + object_attributes: hook_attrs(pipeline), user: pipeline.user.try(:hook_attrs), project: pipeline.project.hook_attrs(backward: false), commit: pipeline.commit.try(:hook_attrs), - object_attributes: hook_attrs(pipeline), builds: pipeline.builds.map(&method(:build_hook_attrs)) } end diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb new file mode 100644 index 00000000000..24d39b318c0 --- /dev/null +++ b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::DataBuilder::PipelineDataBuilder do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:pipeline) do + create(:ci_pipeline, + project: project, status: 'success', + sha: project.commit.sha, ref: project.default_branch) + end + let!(:build) { create(:ci_build, pipeline: pipeline) } + + describe '.build' do + let(:data) { Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) } + let(:attributes) { data[:object_attributes] } + let(:build_data) { data[:builds].first } + let(:project_data) { data[:project] } + + it { expect(attributes).to be_a(Hash) } + it { expect(attributes[:ref]).to eq(pipeline.ref) } + it { expect(attributes[:sha]).to eq(pipeline.sha) } + it { expect(attributes[:tag]).to eq(pipeline.tag) } + it { expect(attributes[:id]).to eq(pipeline.id) } + it { expect(attributes[:status]).to eq(pipeline.status) } + + it { expect(build_data).to be_a(Hash) } + it { expect(build_data[:id]).to eq(build.id) } + it { expect(build_data[:status]).to eq(build.status) } + + it { expect(project_data).to eq(project.hook_attrs(backward: false)) } + end +end From db0cdee6d59f86fe196e4a10c2d1195a45b25cce Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 18:03:05 +0800 Subject: [PATCH 15/34] Expose pipeline_events in the API --- lib/api/entities.rb | 6 ++++-- lib/api/project_hooks.rb | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e76e7304674..06e94d953fe 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -48,7 +48,8 @@ module API class ProjectHook < Hook expose :project_id, :push_events - expose :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events + expose :issues_events, :merge_requests_events, :tag_push_events + expose :note_events, :build_events, :pipeline_events expose :enable_ssl_verification end @@ -340,7 +341,8 @@ module API class ProjectService < Grape::Entity expose :id, :title, :created_at, :updated_at, :active - expose :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events + expose :push_events, :issues_events, :merge_requests_events + expose :tag_push_events, :note_events, :build_events, :pipeline_events # Expose serialized properties expose :properties do |service, options| field_names = service.fields. diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 6bb70bc8bc3..3f63cd678e8 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -45,6 +45,7 @@ module API :tag_push_events, :note_events, :build_events, + :pipeline_events, :enable_ssl_verification ] @hook = user_project.hooks.new(attrs) @@ -78,6 +79,7 @@ module API :tag_push_events, :note_events, :build_events, + :pipeline_events, :enable_ssl_verification ] From 016d4f6b03b5afab34ef93b205ecd78dc8697497 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 18:11:59 +0800 Subject: [PATCH 16/34] Add API tests for pipeline_events --- spec/requests/api/project_hooks_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index fd1fffa6223..504deed81f9 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -8,8 +8,9 @@ describe API::API, 'ProjectHooks', api: true do let!(:hook) do create(:project_hook, project: project, url: "http://example.com", - push_events: true, merge_requests_events: true, tag_push_events: true, - issues_events: true, note_events: true, build_events: true, + push_events: true, merge_requests_events: true, + tag_push_events: true, issues_events: true, note_events: true, + build_events: true, pipeline_events: true, enable_ssl_verification: true) end @@ -33,6 +34,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response.first['tag_push_events']).to eq(true) expect(json_response.first['note_events']).to eq(true) expect(json_response.first['build_events']).to eq(true) + expect(json_response.first['pipeline_events']).to eq(true) expect(json_response.first['enable_ssl_verification']).to eq(true) end end @@ -91,6 +93,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['tag_push_events']).to eq(false) expect(json_response['note_events']).to eq(false) expect(json_response['build_events']).to eq(false) + expect(json_response['pipeline_events']).to eq(false) expect(json_response['enable_ssl_verification']).to eq(true) end From f9075eb04083030b70b6b58b6c4042d7d74c701c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 19:16:59 +0800 Subject: [PATCH 17/34] Add test for running hooks for pipeline after touched --- spec/models/ci/pipeline_spec.rb | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 0d4c86955ce..aa05fc78f94 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -513,7 +513,7 @@ describe Ci::Pipeline, models: true do create :ci_build, :success, pipeline: pipeline, name: 'rspec' create :ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop' end - + it 'returns true' do is_expected.to be_truthy end @@ -524,7 +524,7 @@ describe Ci::Pipeline, models: true do create :ci_build, :success, pipeline: pipeline, name: 'rspec' create :ci_build, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop' end - + it 'returns false' do is_expected.to be_falsey end @@ -542,4 +542,33 @@ describe Ci::Pipeline, models: true do end end end + + describe '#execute_hooks' do + let!(:hook) do + create(:project_hook, project: project, pipeline_events: enabled) + end + let(:enabled) { raise NotImplementedError } + + before do + WebMock.stub_request(:post, hook.url) + pipeline.touch + ProjectWebHookWorker.drain + end + + context 'with pipeline hooks enabled' do + let(:enabled) { true } + + it 'executes pipeline_hook after touched' do + expect(WebMock).to have_requested(:post, hook.url).once + end + end + + context 'with pipeline hooks disabled' do + let(:enabled) { false } + + it 'did not execute pipeline_hook after touched' do + expect(WebMock).not_to have_requested(:post, hook.url) + end + end + end end From 6fb25bef8e55958c6b75f9c699645969d1b1a8f7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 1 Aug 2016 19:23:40 +0800 Subject: [PATCH 18/34] Add a CHANGELOG entry for pipeline events --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index d555d23860d..3b1c83fb841 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.11.0 (unreleased) - Fix renaming repository when name contains invalid chararacters under project settings - Nokogiri's various parsing methods are now instrumented - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 + - Add pipeline events and a corresponding Slack integration !5525 - Add build event color in HipChat messages (David Eisner) - Make fork counter always clickable. !5463 (winniehell) - All created issues, API or WebUI, can be submitted to Akismet for spam check !5333 From 4384e53ec56fa361014fc8e49a84608536596949 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 2 Aug 2016 19:08:40 +0800 Subject: [PATCH 19/34] Update CHANGELOG due to the introduction of !5620 --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 65283f93ed7..85396cb4e42 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,7 +21,7 @@ v 8.11.0 (unreleased) - Optimize checking if a user has read access to a list of issues !5370 - Nokogiri's various parsing methods are now instrumented - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 - - Add pipeline events and a corresponding Slack integration !5525 + - Add pipeline events to Slack integration !5525 - Include old revision in merge request update hooks (Ben Boeckel) - Add build event color in HipChat messages (David Eisner) - Make fork counter always clickable. !5463 (winniehell) From 59b84b6ef5cdf2904840dfa583ddd5642a231a21 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 24 Aug 2016 21:07:55 +0800 Subject: [PATCH 20/34] Fix CHANGELOG --- CHANGELOG | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c4d2f79b1d9..ce2fc5b194f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.12.0 (unreleased) - Change merge_error column from string to text type - Optimistic locking for Issues and Merge Requests (title and description overriding prevention) - Added tests for diff notes + - Add pipeline events to Slack integration !5525 v 8.11.1 (unreleased) - Fix file links on project page when default view is Files !5933 @@ -95,8 +96,6 @@ v 8.11.0 - Fix syntax highlighting in file editor - Support slash commands in issue and merge request descriptions as well as comments. !5021 - Nokogiri's various parsing methods are now instrumented - - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 - - Add pipeline events to Slack integration !5525 - Add archived badge to project list !5798 - Add simple identifier to public SSH keys (muteor) - Admin page now references docs instead of a specific file !5600 (AnAverageHuman) From 386bffb8ca2ea471432e0a57e27044f871610845 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 24 Aug 2016 21:08:01 +0800 Subject: [PATCH 21/34] Fix caption --- app/views/shared/web_hooks/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index f99fbe56277..8189e0b645d 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -78,7 +78,7 @@ = f.label :pipeline_events, class: 'list-label' do %strong Pipeline events %p.light - This url will be triggered when the pipeline status changes + This URL will be triggered when the pipeline status changes %li = f.check_box :wiki_page_events, class: 'pull-left' .prepend-left-20 From c6f09f4d88b0fcd11d8b8c2b4fdd48d0c7e3cd61 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 24 Aug 2016 21:08:42 +0800 Subject: [PATCH 22/34] Fix spacing --- spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb index 24d39b318c0..fb5d07bb563 100644 --- a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb +++ b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb @@ -3,12 +3,13 @@ require 'spec_helper' describe Gitlab::DataBuilder::PipelineDataBuilder do let(:user) { create(:user) } let(:project) { create(:project) } + let!(:build) { create(:ci_build, pipeline: pipeline) } + let(:pipeline) do create(:ci_pipeline, project: project, status: 'success', sha: project.commit.sha, ref: project.default_branch) end - let!(:build) { create(:ci_build, pipeline: pipeline) } describe '.build' do let(:data) { Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) } From a79cd2a275893635d9ee719495fbb33aa7649497 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 24 Aug 2016 21:09:53 +0800 Subject: [PATCH 23/34] No longer needed --- spec/models/build_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index f63183f5df1..ee2c3d04984 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -275,8 +275,7 @@ describe Ci::Build, models: true do context 'when yaml_variables are undefined' do before do - build.update(yaml_variables: nil) - build.reload # reload pipeline so that it resets config_processor + build.yaml_variables = nil end context 'use from gitlab-ci.yml' do @@ -902,8 +901,7 @@ describe Ci::Build, models: true do context 'when `when` is undefined' do before do - build.update(when: nil) - build.reload # reload pipeline so that it resets config_processor + build.when = nil end context 'use from gitlab-ci.yml' do From 4323d24abf78a33f8d53b62fe976a6b4a486de8d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 24 Aug 2016 22:01:34 +0800 Subject: [PATCH 24/34] Remove old stuffs --- .../data_builder/pipeline_data_builder.rb | 66 ------------------- .../pipeline_data_builder_spec.rb | 33 ---------- .../project_services/slack_service_spec.rb | 4 +- 3 files changed, 2 insertions(+), 101 deletions(-) delete mode 100644 lib/gitlab/data_builder/pipeline_data_builder.rb delete mode 100644 spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb deleted file mode 100644 index 13417ba09eb..00000000000 --- a/lib/gitlab/data_builder/pipeline_data_builder.rb +++ /dev/null @@ -1,66 +0,0 @@ -module Gitlab - module DataBuilder - module PipelineDataBuilder - module_function - - def build(pipeline) - { - object_kind: 'pipeline', - object_attributes: hook_attrs(pipeline), - user: pipeline.user.try(:hook_attrs), - project: pipeline.project.hook_attrs(backward: false), - commit: pipeline.commit.try(:hook_attrs), - builds: pipeline.builds.map(&method(:build_hook_attrs)) - } - end - - def hook_attrs(pipeline) - first_pending_build = pipeline.builds.first_pending - config_processor = pipeline.config_processor - - { - id: pipeline.id, - ref: pipeline.ref, - tag: pipeline.tag, - sha: pipeline.sha, - before_sha: pipeline.before_sha, - status: pipeline.status, - stage: first_pending_build.try(:stage), - stages: config_processor.try(:stages), - created_at: pipeline.created_at, - finished_at: pipeline.finished_at, - duration: pipeline.duration - } - end - - def build_hook_attrs(build) - { - id: build.id, - stage: build.stage, - name: build.name, - status: build.status, - created_at: build.created_at, - started_at: build.started_at, - finished_at: build.finished_at, - when: build.when, - manual: build.manual?, - user: build.user.try(:hook_attrs), - runner: build.runner && runner_hook_attrs(build.runner), - artifacts_file: { - filename: build.artifacts_file.filename, - size: build.artifacts_size - } - } - end - - def runner_hook_attrs(runner) - { - id: runner.id, - description: runner.description, - active: runner.active?, - is_shared: runner.is_shared? - } - end - end - end -end diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb deleted file mode 100644 index fb5d07bb563..00000000000 --- a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -require 'spec_helper' - -describe Gitlab::DataBuilder::PipelineDataBuilder do - let(:user) { create(:user) } - let(:project) { create(:project) } - let!(:build) { create(:ci_build, pipeline: pipeline) } - - let(:pipeline) do - create(:ci_pipeline, - project: project, status: 'success', - sha: project.commit.sha, ref: project.default_branch) - end - - describe '.build' do - let(:data) { Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) } - let(:attributes) { data[:object_attributes] } - let(:build_data) { data[:builds].first } - let(:project_data) { data[:project] } - - it { expect(attributes).to be_a(Hash) } - it { expect(attributes[:ref]).to eq(pipeline.ref) } - it { expect(attributes[:sha]).to eq(pipeline.sha) } - it { expect(attributes[:tag]).to eq(pipeline.tag) } - it { expect(attributes[:id]).to eq(pipeline.id) } - it { expect(attributes[:status]).to eq(pipeline.status) } - - it { expect(build_data).to be_a(Hash) } - it { expect(build_data[:id]).to eq(build.id) } - it { expect(build_data[:status]).to eq(build.status) } - - it { expect(project_data).to eq(project.hook_attrs(backward: false)) } - end -end diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 9b7899270d0..975c18fee1b 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -310,7 +310,7 @@ describe SlackService, models: true do end it 'calls Slack API for pipeline events' do - data = Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) + data = Gitlab::DataBuilder::Pipeline.build(pipeline) slack.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once @@ -328,7 +328,7 @@ describe SlackService, models: true do context 'with default to notify_only_broken_pipelines' do it 'does not call Slack API for pipeline events' do - data = Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) + data = Gitlab::DataBuilder::Pipeline.build(pipeline) result = slack.execute(data) expect(result).to be_falsy From b99263fbd120dcfea4159e800d8949fb6ad453e2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 26 Aug 2016 09:35:42 +0000 Subject: [PATCH 25/34] Fix CHANGELOG --- CHANGELOG | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e317b1f6ce0..a59c45a0fda 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,8 +15,6 @@ v 8.12.0 (unreleased) - Fix groups sort dropdown alignment (ClemMakesApps) - Added tests for diff notes - Add pipeline events to Slack integration !5525 - -v 8.11.1 (unreleased) - Add delimiter to project stars and forks count (ClemMakesApps) - Fix badge count alignment (ClemMakesApps) - Fix branch title trailing space on hover (ClemMakesApps) From 5067d9d388589da39416d818358ec3e1211c6ed7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 15:21:50 +0800 Subject: [PATCH 26/34] Empty line between message =, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5525#note_14664714 --- .../project_services/slack_service/build_message_spec.rb | 9 ++++++--- .../slack_service/pipeline_message_spec.rb | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb index 7fcfdf0eacd..a3ef121a093 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -24,9 +24,10 @@ describe SlackService::BuildMessage do let(:status) { 'success' } let(:color) { 'good' } let(:duration) { 10 } - + it 'returns a message with information about succeeded build' do message = ': Commit of branch by hacker passed in 10 seconds' + expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -40,12 +41,13 @@ describe SlackService::BuildMessage do it 'returns a message with information about failed build' do message = ': Commit of branch by hacker failed in 10 seconds' + expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) end - end - + end + describe '#seconds_name' do let(:status) { 'failed' } let(:color) { 'danger' } @@ -53,6 +55,7 @@ describe SlackService::BuildMessage do it 'returns seconds as singular when there is only one' do message = ': Commit of branch by hacker failed in 1 second' + expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) 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 a292defee66..c27b7c82dd6 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -26,6 +26,7 @@ describe SlackService::PipelineMessage do it 'returns a message with information about succeeded build' do message = ': Pipeline of branch by hacker passed in 10 seconds' + expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -39,6 +40,7 @@ describe SlackService::PipelineMessage do it 'returns a message with information about failed build' do message = ': Pipeline of branch by hacker failed in 10 seconds' + expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -52,6 +54,7 @@ describe SlackService::PipelineMessage do it 'returns seconds as singular when there is only one' do message = ': Pipeline of branch by hacker failed in 1 second' + expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) From 4fc478a4e0cb88dc72db627cd6081c1456f7ddf8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 15:23:49 +0800 Subject: [PATCH 27/34] somewhere -> example.gitlab, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5525#note_14664661 --- .../project_services/slack_service/build_message_spec.rb | 8 ++++---- .../slack_service/pipeline_message_spec.rb | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb index a3ef121a093..5ffa724e7a4 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -10,7 +10,7 @@ describe SlackService::BuildMessage do tag: false, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'example.gitlab.com', commit: { status: status, @@ -26,7 +26,7 @@ describe SlackService::BuildMessage do let(:duration) { 10 } it 'returns a message with information about succeeded build' do - message = ': Commit of branch by hacker passed in 10 seconds' + message = ': Commit of branch by hacker passed in 10 seconds' expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) @@ -40,7 +40,7 @@ describe SlackService::BuildMessage do let(:duration) { 10 } it 'returns a message with information about failed build' do - message = ': Commit of branch by hacker failed in 10 seconds' + message = ': Commit of branch by hacker failed in 10 seconds' expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) @@ -54,7 +54,7 @@ describe SlackService::BuildMessage do let(:duration) { 1 } it 'returns seconds as singular when there is only one' do - message = ': Commit of branch by hacker failed in 1 second' + message = ': Commit of branch by hacker failed in 1 second' expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) 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 c27b7c82dd6..8258f118c13 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -14,7 +14,7 @@ describe SlackService::PipelineMessage do duration: duration }, project: { path_with_namespace: 'project_name', - web_url: 'somewhere.com' }, + web_url: 'example.gitlab.com' }, commit: { author_name: 'hacker' } } end @@ -25,7 +25,7 @@ describe SlackService::PipelineMessage do let(:duration) { 10 } it 'returns a message with information about succeeded build' do - message = ': Pipeline of branch by hacker passed in 10 seconds' + message = ': Pipeline of branch by hacker passed in 10 seconds' expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) @@ -39,7 +39,7 @@ describe SlackService::PipelineMessage do let(:duration) { 10 } it 'returns a message with information about failed build' do - message = ': Pipeline of branch by hacker failed in 10 seconds' + message = ': Pipeline of branch by hacker failed in 10 seconds' expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) @@ -53,7 +53,7 @@ describe SlackService::PipelineMessage do let(:duration) { 1 } it 'returns seconds as singular when there is only one' do - message = ': Pipeline of branch by hacker failed in 1 second' + message = ': Pipeline of branch by hacker failed in 1 second' expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) From a97133a6b13b0357f4037d2678569b738046acdb Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 15:33:36 +0800 Subject: [PATCH 28/34] Shorten the line and use methods, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5525#note_14664637 --- .../slack_service/build_message_spec.rb | 17 +++++++++++------ .../slack_service/pipeline_message_spec.rb | 16 ++++++++++------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb index 5ffa724e7a4..16fe1c93039 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -20,14 +20,15 @@ describe SlackService::BuildMessage do } end + let(:message) { build_message } + context 'succeeded' do let(:status) { 'success' } let(:color) { 'good' } let(:duration) { 10 } + let(:message) { build_message('passed') } it 'returns a message with information about succeeded build' do - message = ': Commit of branch by hacker passed in 10 seconds' - expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -40,8 +41,6 @@ describe SlackService::BuildMessage do let(:duration) { 10 } it 'returns a message with information about failed build' do - message = ': Commit of branch by hacker failed in 10 seconds' - expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -54,11 +53,17 @@ describe SlackService::BuildMessage do let(:duration) { 1 } it 'returns seconds as singular when there is only one' do - message = ': Commit of branch by hacker failed in 1 second' - expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) end end + + def build_message(status_text=status) + ":" \ + " Commit " \ + " of branch" \ + " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" + end end 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 8258f118c13..1f1701202b4 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -19,14 +19,15 @@ describe SlackService::PipelineMessage do } end + let(:message) { build_message } + context 'succeeded' do let(:status) { 'success' } let(:color) { 'good' } let(:duration) { 10 } + let(:message) { build_message('passed') } it 'returns a message with information about succeeded build' do - message = ': Pipeline of branch by hacker passed in 10 seconds' - expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -39,8 +40,6 @@ describe SlackService::PipelineMessage do let(:duration) { 10 } it 'returns a message with information about failed build' do - message = ': Pipeline of branch by hacker failed in 10 seconds' - expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) @@ -53,11 +52,16 @@ describe SlackService::PipelineMessage do let(:duration) { 1 } it 'returns seconds as singular when there is only one' do - message = ': Pipeline of branch by hacker failed in 1 second' - expect(subject.pretext).to be_empty expect(subject.fallback).to eq(message) expect(subject.attachments).to eq([text: message, color: color]) end end + + def build_message(status_text=status) + ":" \ + " Pipeline " \ + " of branch" \ + " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" + end end From 08c3e9e7849c9f303d3df5f4afb9986eae7a9973 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 15:35:04 +0800 Subject: [PATCH 29/34] pipeline/build succeeded/failed, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5525#note_14664613 --- .../project_services/slack_service/build_message_spec.rb | 4 ++-- .../project_services/slack_service/pipeline_message_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb index 16fe1c93039..f690b6c7b3b 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -22,7 +22,7 @@ describe SlackService::BuildMessage do let(:message) { build_message } - context 'succeeded' do + context 'build succeeded' do let(:status) { 'success' } let(:color) { 'good' } let(:duration) { 10 } @@ -35,7 +35,7 @@ describe SlackService::BuildMessage do end end - context 'failed' do + context 'build failed' do let(:status) { 'failed' } let(:color) { 'danger' } let(:duration) { 10 } 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 1f1701202b4..fedf6eb1317 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -21,7 +21,7 @@ describe SlackService::PipelineMessage do let(:message) { build_message } - context 'succeeded' do + context 'pipeline succeeded' do let(:status) { 'success' } let(:color) { 'good' } let(:duration) { 10 } @@ -34,7 +34,7 @@ describe SlackService::PipelineMessage do end end - context 'failed' do + context 'pipeline failed' do let(:status) { 'failed' } let(:color) { 'danger' } let(:duration) { 10 } From 71981151f817712f6f6dcd30651d32fb1bdb0abc Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 15:37:15 +0800 Subject: [PATCH 30/34] empty lines between blocks --- spec/models/project_services/slack_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 975c18fee1b..f920dc263ef 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -289,12 +289,12 @@ describe SlackService, models: true do describe 'Pipeline events' do let(:user) { create(:user) } let(:project) { create(:project) } + let(:pipeline) do create(:ci_pipeline, project: project, status: status, sha: project.commit.sha, ref: project.default_branch) end - let(:status) { raise NotImplementedError } before do allow(slack).to receive_messages( From 41384db1c6a267b344259cd350b6161fe79605e8 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 15:45:43 +0800 Subject: [PATCH 31/34] Use a completely fake webhook URL, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5525#note_14665966 --- spec/models/project_services/slack_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index f920dc263ef..5afdc4b2f7b 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -22,7 +22,7 @@ require 'spec_helper' describe SlackService, models: true do let(:slack) { SlackService.new } - let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } + let(:webhook_url) { 'https://example.gitlab.com/' } describe "Associations" do it { is_expected.to belong_to :project } From 867b64c87dc1e5e9d43a91e9cd7ba48c1025683e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 16:03:03 +0800 Subject: [PATCH 32/34] I am too used to have no spaces around default args --- .../models/project_services/slack_service/build_message_spec.rb | 2 +- .../project_services/slack_service/pipeline_message_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb index f690b6c7b3b..0b876f4fd26 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -59,7 +59,7 @@ describe SlackService::BuildMessage do end end - def build_message(status_text=status) + def build_message(status_text = status) ":" \ " Commit " \ 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 fedf6eb1317..384b22b7103 100644 --- a/spec/models/project_services/slack_service/pipeline_message_spec.rb +++ b/spec/models/project_services/slack_service/pipeline_message_spec.rb @@ -58,7 +58,7 @@ describe SlackService::PipelineMessage do end end - def build_message(status_text=status) + def build_message(status_text = status) ":" \ " Pipeline " \ " of branch" \ From da1acf04cef4cd7ee53f9da7bb870ba226ed1441 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 29 Aug 2016 17:14:16 +0800 Subject: [PATCH 33/34] Not sure why there's an extra one!? --- app/views/shared/web_hooks/_form.html.haml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index 8189e0b645d..d2ec6c3ddef 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -72,13 +72,6 @@ %strong Pipeline events %p.light This URL will be triggered when the pipeline status changes - %li - = f.check_box :pipeline_events, class: 'pull-left' - .prepend-left-20 - = f.label :pipeline_events, class: 'list-label' do - %strong Pipeline events - %p.light - This URL will be triggered when the pipeline status changes %li = f.check_box :wiki_page_events, class: 'pull-left' .prepend-left-20 From c2bcfab18af1cf9253a47d4ffd3ea48e43cd19be Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 6 Sep 2016 19:53:02 +0800 Subject: [PATCH 34/34] Remove redundant tests, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5525#note_14993999 --- .../slack_service/build_message_spec.rb | 12 ------------ .../slack_service/pipeline_message_spec.rb | 12 ------------ 2 files changed, 24 deletions(-) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb index 0b876f4fd26..452f4e2782c 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -47,18 +47,6 @@ describe SlackService::BuildMessage do end end - describe '#seconds_name' do - let(:status) { 'failed' } - let(:color) { 'danger' } - let(:duration) { 1 } - - it 'returns seconds as singular when there is only one' do - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) - end - end - def build_message(status_text = status) ":" \ " Commit :" \ " Pipeline " \