From 2988e1fbf50b3c9e803a9358933e3e969e64dcc3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 7 Dec 2015 13:23:23 +0100 Subject: Migrate CI::Services and CI::WebHooks to Services and WebHooks --- CHANGELOG | 1 + app/controllers/projects/ci_services_controller.rb | 49 ------ .../projects/ci_web_hooks_controller.rb | 45 ------ app/controllers/projects/hooks_controller.rb | 3 +- app/controllers/projects/services_controller.rb | 4 +- app/mailers/ci/emails/builds.rb | 17 -- app/mailers/ci/notify.rb | 46 ------ app/mailers/emails/builds.rb | 15 ++ app/mailers/notify.rb | 1 + app/models/ci/build.rb | 20 ++- app/models/ci/commit.rb | 4 + app/models/ci/project.rb | 41 ----- app/models/ci/service.rb | 105 ------------ app/models/ci/web_hook.rb | 43 ----- app/models/hooks/project_hook.rb | 1 + app/models/hooks/web_hook.rb | 1 + app/models/project.rb | 1 + .../project_services/builds_email_service.rb | 81 ++++++++++ app/models/project_services/ci/hip_chat_message.rb | 73 --------- app/models/project_services/ci/hip_chat_service.rb | 93 ----------- app/models/project_services/ci/mail_service.rb | 84 ---------- app/models/project_services/ci/slack_message.rb | 92 ----------- app/models/project_services/ci/slack_service.rb | 81 ---------- app/models/project_services/hipchat_service.rb | 42 ++++- app/models/project_services/slack_service.rb | 22 ++- .../project_services/slack_service/base_message.rb | 3 + .../slack_service/build_message.rb | 82 ++++++++++ app/models/service.rb | 18 +++ app/services/ci/create_builds_service.rb | 3 +- app/views/ci/notify/build_fail_email.html.haml | 23 --- app/views/ci/notify/build_fail_email.text.erb | 11 -- app/views/ci/notify/build_success_email.html.haml | 24 --- app/views/ci/notify/build_success_email.text.erb | 11 -- app/views/layouts/nav/_project_settings.html.haml | 10 -- app/views/notify/build_fail_email.html.haml | 23 +++ app/views/notify/build_fail_email.text.erb | 11 ++ app/views/notify/build_success_email.html.haml | 24 +++ app/views/notify/build_success_email.text.erb | 11 ++ app/views/projects/ci_services/_form.html.haml | 54 ------- app/views/projects/ci_services/edit.html.haml | 2 - app/views/projects/ci_services/index.html.haml | 23 --- app/views/projects/ci_web_hooks/index.html.haml | 94 ----------- app/views/projects/hooks/index.html.haml | 9 +- app/views/shared/_service_settings.html.haml | 9 ++ app/workers/build_email_worker.rb | 19 +++ app/workers/ci/hip_chat_notifier_worker.rb | 19 --- app/workers/ci/slack_notifier_worker.rb | 10 -- app/workers/ci/web_hook_worker.rb | 9 -- config/routes.rb | 11 -- .../20151203162134_add_build_events_to_services.rb | 6 + db/schema.rb | 4 +- features/project/service.feature | 8 +- features/steps/admin/settings.rb | 2 + features/steps/project/services.rb | 8 +- lib/api/entities.rb | 5 +- lib/api/project_hooks.rb | 2 + lib/ci/api/projects.rb | 38 ----- lib/gitlab/build_data_builder.rb | 64 ++++++++ spec/factories/ci/web_hook.rb | 6 - spec/features/ci_web_hooks_spec.rb | 27 ---- spec/lib/gitlab/build_data_builder_spec.rb | 20 +++ spec/mailers/ci/notify_spec.rb | 35 ---- spec/mailers/notify_spec.rb | 29 ++++ .../ci/project_services/hip_chat_message_spec.rb | 39 ----- .../ci/project_services/hip_chat_service_spec.rb | 73 --------- .../ci/project_services/mail_service_spec.rb | 177 --------------------- .../ci/project_services/slack_message_spec.rb | 43 ----- .../ci/project_services/slack_service_spec.rb | 57 ------- spec/models/ci/project_spec.rb | 2 - spec/models/ci/service_spec.rb | 48 ------ spec/models/ci/web_hook_spec.rb | 63 -------- .../project_services/hipchat_service_spec.rb | 49 ++++++ .../slack_service/build_message_spec.rb | 46 ++++++ spec/requests/api/project_hooks_spec.rb | 12 +- spec/requests/ci/api/projects_spec.rb | 73 --------- spec/services/ci/web_hook_service_spec.rb | 37 ----- spec/workers/build_email_worker_spec.rb | 35 ++++ 77 files changed, 669 insertions(+), 1817 deletions(-) delete mode 100644 app/controllers/projects/ci_services_controller.rb delete mode 100644 app/controllers/projects/ci_web_hooks_controller.rb delete mode 100644 app/mailers/ci/emails/builds.rb delete mode 100644 app/mailers/ci/notify.rb create mode 100644 app/mailers/emails/builds.rb delete mode 100644 app/models/ci/service.rb delete mode 100644 app/models/ci/web_hook.rb create mode 100644 app/models/project_services/builds_email_service.rb delete mode 100644 app/models/project_services/ci/hip_chat_message.rb delete mode 100644 app/models/project_services/ci/hip_chat_service.rb delete mode 100644 app/models/project_services/ci/mail_service.rb delete mode 100644 app/models/project_services/ci/slack_message.rb delete mode 100644 app/models/project_services/ci/slack_service.rb create mode 100644 app/models/project_services/slack_service/build_message.rb delete mode 100644 app/views/ci/notify/build_fail_email.html.haml delete mode 100644 app/views/ci/notify/build_fail_email.text.erb delete mode 100644 app/views/ci/notify/build_success_email.html.haml delete mode 100644 app/views/ci/notify/build_success_email.text.erb create mode 100644 app/views/notify/build_fail_email.html.haml create mode 100644 app/views/notify/build_fail_email.text.erb create mode 100644 app/views/notify/build_success_email.html.haml create mode 100644 app/views/notify/build_success_email.text.erb delete mode 100644 app/views/projects/ci_services/_form.html.haml delete mode 100644 app/views/projects/ci_services/edit.html.haml delete mode 100644 app/views/projects/ci_services/index.html.haml delete mode 100644 app/views/projects/ci_web_hooks/index.html.haml create mode 100644 app/workers/build_email_worker.rb delete mode 100644 app/workers/ci/hip_chat_notifier_worker.rb delete mode 100644 app/workers/ci/slack_notifier_worker.rb delete mode 100644 app/workers/ci/web_hook_worker.rb create mode 100644 db/migrate/20151203162134_add_build_events_to_services.rb create mode 100644 lib/gitlab/build_data_builder.rb delete mode 100644 spec/factories/ci/web_hook.rb delete mode 100644 spec/features/ci_web_hooks_spec.rb create mode 100644 spec/lib/gitlab/build_data_builder_spec.rb delete mode 100644 spec/mailers/ci/notify_spec.rb delete mode 100644 spec/models/ci/project_services/hip_chat_message_spec.rb delete mode 100644 spec/models/ci/project_services/hip_chat_service_spec.rb delete mode 100644 spec/models/ci/project_services/mail_service_spec.rb delete mode 100644 spec/models/ci/project_services/slack_message_spec.rb delete mode 100644 spec/models/ci/project_services/slack_service_spec.rb delete mode 100644 spec/models/ci/service_spec.rb delete mode 100644 spec/models/ci/web_hook_spec.rb create mode 100644 spec/models/project_services/slack_service/build_message_spec.rb delete mode 100644 spec/services/ci/web_hook_service_spec.rb create mode 100644 spec/workers/build_email_worker_spec.rb diff --git a/CHANGELOG b/CHANGELOG index fc1cb1a996f..2f94eb6c66c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.3.0 (unreleased) - Fire update hook from GitLab - Style warning about mentioning many people in a comment - Fix: sort milestones by due date once again (Greg Smethells) + - Migrate all CI::Services and CI::WebHooks to Services and WebHooks - Don't show project fork event as "imported" - Add API endpoint to fetch merge request commits list - Expose events API with comment information and author info diff --git a/app/controllers/projects/ci_services_controller.rb b/app/controllers/projects/ci_services_controller.rb deleted file mode 100644 index 550a019e8e2..00000000000 --- a/app/controllers/projects/ci_services_controller.rb +++ /dev/null @@ -1,49 +0,0 @@ -class Projects::CiServicesController < Projects::ApplicationController - before_action :ci_project - before_action :authorize_admin_project! - - layout "project_settings" - - def index - @ci_project.build_missing_services - @services = @ci_project.services.reload - end - - def edit - service - end - - def update - if service.update_attributes(service_params) - redirect_to edit_namespace_project_ci_service_path(@project.namespace, @project, service.to_param) - else - render 'edit' - end - end - - def test - last_build = @project.ci_builds.last - - if service.execute(last_build) - message = { notice: 'We successfully tested the service' } - else - message = { alert: 'We tried to test the service but error occurred' } - end - - redirect_back_or_default(options: message) - end - - private - - def service - @service ||= @ci_project.services.find { |service| service.to_param == params[:id] } - end - - def service_params - params.require(:service).permit( - :type, :active, :webhook, :notify_only_broken_builds, - :email_recipients, :email_only_broken_builds, :email_add_pusher, - :hipchat_token, :hipchat_room, :hipchat_server - ) - end -end diff --git a/app/controllers/projects/ci_web_hooks_controller.rb b/app/controllers/projects/ci_web_hooks_controller.rb deleted file mode 100644 index a2d470d4a69..00000000000 --- a/app/controllers/projects/ci_web_hooks_controller.rb +++ /dev/null @@ -1,45 +0,0 @@ -class Projects::CiWebHooksController < Projects::ApplicationController - before_action :ci_project - before_action :authorize_admin_project! - - layout "project_settings" - - def index - @web_hooks = @ci_project.web_hooks - @web_hook = Ci::WebHook.new - end - - def create - @web_hook = @ci_project.web_hooks.new(web_hook_params) - @web_hook.save - - if @web_hook.valid? - redirect_to namespace_project_ci_web_hooks_path(@project.namespace, @project) - else - @web_hooks = @ci_project.web_hooks.select(&:persisted?) - render :index - end - end - - def test - Ci::TestHookService.new.execute(hook, current_user) - - redirect_back_or_default(default: { action: 'index' }) - end - - def destroy - hook.destroy - - redirect_to namespace_project_ci_web_hooks_path(@project.namespace, @project) - end - - private - - def hook - @web_hook ||= @ci_project.web_hooks.find(params[:id]) - end - - def web_hook_params - params.require(:web_hook).permit(:url) - end -end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 6a62880cb71..5fd4f855dec 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -53,6 +53,7 @@ class Projects::HooksController < Projects::ApplicationController def hook_params params.require(:hook).permit(:url, :push_events, :issues_events, - :merge_requests_events, :tag_push_events, :note_events, :enable_ssl_verification) + :merge_requests_events, :tag_push_events, :note_events, + :build_events, :enable_ssl_verification) end end diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 42dbb497e01..6e7590260ff 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -6,7 +6,9 @@ class Projects::ServicesController < Projects::ApplicationController :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, :send_from_committer_email, :disable_diffs, :external_wiki_url, + :note_events, :build_events, + :notify_only_broken_builds, :add_pusher, + :send_from_committer_email, :disable_diffs, :external_wiki_url, :notify, :color, :server_host, :server_port, :default_irc_uri, :enable_ssl_verification] diff --git a/app/mailers/ci/emails/builds.rb b/app/mailers/ci/emails/builds.rb deleted file mode 100644 index 6fb4fba85e5..00000000000 --- a/app/mailers/ci/emails/builds.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Ci - module Emails - module Builds - def build_fail_email(build_id, to) - @build = Ci::Build.find(build_id) - @project = @build.project - mail(to: to, subject: subject("Build failed for #{@project.name}", @build.short_sha)) - end - - def build_success_email(build_id, to) - @build = Ci::Build.find(build_id) - @project = @build.project - mail(to: to, subject: subject("Build success for #{@project.name}", @build.short_sha)) - end - end - end -end diff --git a/app/mailers/ci/notify.rb b/app/mailers/ci/notify.rb deleted file mode 100644 index 404842cf213..00000000000 --- a/app/mailers/ci/notify.rb +++ /dev/null @@ -1,46 +0,0 @@ -module Ci - class Notify < ActionMailer::Base - include Ci::Emails::Builds - - add_template_helper Ci::GitlabHelper - - default_url_options[:host] = Gitlab.config.gitlab.host - default_url_options[:protocol] = Gitlab.config.gitlab.protocol - default_url_options[:port] = Gitlab.config.gitlab.port unless Gitlab.config.gitlab_on_standard_port? - default_url_options[:script_name] = Gitlab.config.gitlab.relative_url_root - - default from: Gitlab.config.gitlab.email_from - - # Just send email with 3 seconds delay - def self.delay - delay_for(2.seconds) - end - - private - - # Formats arguments into a String suitable for use as an email subject - # - # extra - Extra Strings to be inserted into the subject - # - # Examples - # - # >> subject('Lorem ipsum') - # => "GitLab-CI | Lorem ipsum" - # - # # Automatically inserts Project name when @project is set - # >> @project = Project.last - # => # - # >> subject('Lorem ipsum') - # => "GitLab-CI | Ruby on Rails | Lorem ipsum " - # - # # Accepts multiple arguments - # >> subject('Lorem ipsum', 'Dolor sit amet') - # => "GitLab-CI | Lorem ipsum | Dolor sit amet" - def subject(*extra) - subject = "GitLab-CI" - subject << (@project ? " | #{@project.name}" : "") - subject << " | " + extra.join(' | ') if extra.present? - subject - end - end -end diff --git a/app/mailers/emails/builds.rb b/app/mailers/emails/builds.rb new file mode 100644 index 00000000000..d58609a2de5 --- /dev/null +++ b/app/mailers/emails/builds.rb @@ -0,0 +1,15 @@ +module Emails + module Builds + def build_fail_email(build_id, to) + @build = Ci::Build.find(build_id) + @project = @build.project + mail(to: to, subject: subject("Build failed for #{@project.name}", @build.short_sha)) + end + + def build_success_email(build_id, to) + @build = Ci::Build.find(build_id) + @project = @build.project + mail(to: to, subject: subject("Build success for #{@project.name}", @build.short_sha)) + end + end +end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 50a409c3754..874769e239a 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -7,6 +7,7 @@ class Notify < BaseMailer include Emails::Projects include Emails::Profile include Emails::Groups + include Emails::Builds add_template_helper MergeRequestsHelper add_template_helper EmailsHelper diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 52ce1b920fa..564041e3214 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -96,21 +96,21 @@ module Ci end state_machine :status, initial: :pending do + after_transition pending: :running do |build, transition| + build.execute_hooks + end + after_transition any => [:success, :failed, :canceled] do |build, transition| return unless build.gl_project project = build.project - if project.web_hooks? - Ci::WebHookService.new.build_end(build) - end - - build.commit.create_next_builds(build) - project.execute_services(build) - if project.coverage_enabled? build.update_coverage end + + build.commit.create_next_builds(build) + build.execute_hooks end end @@ -275,6 +275,12 @@ module Ci end end + def execute_hooks + build_data = Gitlab::BuildDataBuilder.build(self) + gl_project.execute_hooks(build_data.dup, :build_hooks) + gl_project.execute_services(build_data.dup, :build_hooks) + end + private def yaml_variables diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb index 75465685e98..e63f7790946 100644 --- a/app/models/ci/commit.rb +++ b/app/models/ci/commit.rb @@ -178,6 +178,10 @@ module Ci duration_array.reduce(:+).to_i end + def started_at + @started_at ||= statuses.order('started_at ASC').first.try(:started_at) + end + def finished_at @finished_at ||= statuses.order('finished_at DESC').first.try(:finished_at) end diff --git a/app/models/ci/project.rb b/app/models/ci/project.rb index 669ee1cc0d2..79ff7e1dcd4 100644 --- a/app/models/ci/project.rb +++ b/app/models/ci/project.rb @@ -35,17 +35,10 @@ module Ci has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject' has_many :runners, through: :runner_projects, class_name: 'Ci::Runner' - has_many :web_hooks, dependent: :destroy, class_name: 'Ci::WebHook' has_many :events, dependent: :destroy, class_name: 'Ci::Event' has_many :variables, dependent: :destroy, class_name: 'Ci::Variable' has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger' - # Project services - has_many :services, dependent: :destroy, class_name: 'Ci::Service' - has_one :hip_chat_service, dependent: :destroy, class_name: 'Ci::HipChatService' - has_one :slack_service, dependent: :destroy, class_name: 'Ci::SlackService' - has_one :mail_service, dependent: :destroy, class_name: 'Ci::MailService' - accepts_nested_attributes_for :variables, allow_destroy: true delegate :name_with_namespace, :path_with_namespace, :web_url, :http_url_to_repo, :ssh_url_to_repo, to: :gl_project @@ -122,14 +115,6 @@ module Ci email_add_pusher || email_recipients.present? end - def web_hooks? - web_hooks.any? - end - - def services? - services.any? - end - def timeout_in_minutes timeout / 60 end @@ -151,32 +136,6 @@ module Ci end end - def available_services_names - %w(slack mail hip_chat) - end - - def build_missing_services - available_services_names.each do |service_name| - service = services.find { |service| service.to_param == service_name } - - # If service is available but missing in db - # we should create an instance. Ex `create_gitlab_ci_service` - self.send :"create_#{service_name}_service" if service.nil? - end - end - - def execute_services(data) - services.each do |service| - - # Call service hook only if it is active - begin - service.execute(data) if service.active && service.can_execute?(data) - rescue => e - logger.error(e) - end - end - end - def setup_finished? commits.any? end diff --git a/app/models/ci/service.rb b/app/models/ci/service.rb deleted file mode 100644 index 8063c51e82b..00000000000 --- a/app/models/ci/service.rb +++ /dev/null @@ -1,105 +0,0 @@ -# == Schema Information -# -# Table name: ci_services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -# To add new service you should build a class inherited from Service -# and implement a set of methods -module Ci - class Service < ActiveRecord::Base - extend Ci::Model - - serialize :properties, JSON - - default_value_for :active, false - - after_initialize :initialize_properties - - belongs_to :project, class_name: 'Ci::Project' - - validates :project_id, presence: true - - def activated? - active - end - - def category - :common - end - - def initialize_properties - self.properties = {} if properties.nil? - end - - def title - # implement inside child - end - - def description - # implement inside child - end - - def help - # implement inside child - end - - def to_param - # implement inside child - end - - def fields - # implement inside child - [] - end - - def can_test? - project.builds.any? - end - - def can_execute?(build) - true - end - - def execute(build) - # implement inside child - end - - # Provide convenient accessor methods - # for each serialized property. - def self.prop_accessor(*args) - args.each do |arg| - class_eval %{ - def #{arg} - (properties || {})['#{arg}'] - end - - def #{arg}=(value) - self.properties ||= {} - self.properties['#{arg}'] = value - end - } - end - end - - def self.boolean_accessor(*args) - self.prop_accessor(*args) - - args.each do |arg| - class_eval %{ - def #{arg}? - ActiveRecord::ConnectionAdapters::Column::TRUE_VALUES.include?(#{arg}) - end - } - end - end - end -end diff --git a/app/models/ci/web_hook.rb b/app/models/ci/web_hook.rb deleted file mode 100644 index 0dc15eb6683..00000000000 --- a/app/models/ci/web_hook.rb +++ /dev/null @@ -1,43 +0,0 @@ -# == Schema Information -# -# Table name: ci_web_hooks -# -# id :integer not null, primary key -# url :string(255) not null -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# - -module Ci - class WebHook < ActiveRecord::Base - extend Ci::Model - - include HTTParty - - belongs_to :project, class_name: 'Ci::Project' - - # HTTParty timeout - default_timeout 10 - - validates :url, presence: true, url: true - - def execute(data) - parsed_url = URI.parse(url) - if parsed_url.userinfo.blank? - Ci::WebHook.post(url, body: data.to_json, headers: { "Content-Type" => "application/json" }, verify: false) - else - post_url = url.gsub("#{parsed_url.userinfo}@", "") - auth = { - username: URI.decode(parsed_url.user), - password: URI.decode(parsed_url.password), - } - Ci::WebHook.post(post_url, - body: data.to_json, - headers: { "Content-Type" => "application/json" }, - verify: false, - basic_auth: auth) - end - end - end -end diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index 337b3097126..22638057773 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -25,4 +25,5 @@ class ProjectHook < WebHook scope :issue_hooks, -> { where(issues_events: true) } scope :note_hooks, -> { where(note_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) } + scope :build_hooks, -> { where(build_events: true) } end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 715ec5908b7..40eb0e20b4b 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -26,6 +26,7 @@ class WebHook < ActiveRecord::Base default_value_for :note_events, false default_value_for :merge_requests_events, false default_value_for :tag_push_events, false + default_value_for :build_events, false default_value_for :enable_ssl_verification, true # HTTParty timeout diff --git a/app/models/project.rb b/app/models/project.rb index e78868af1cc..60ca2cad6ac 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -81,6 +81,7 @@ class Project < ActiveRecord::Base has_one :campfire_service, dependent: :destroy has_one :drone_ci_service, dependent: :destroy has_one :emails_on_push_service, dependent: :destroy + has_one :builds_email_service, dependent: :destroy has_one :irker_service, dependent: :destroy has_one :pivotaltracker_service, dependent: :destroy has_one :hipchat_service, dependent: :destroy diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb new file mode 100644 index 00000000000..9c9b5a4d08c --- /dev/null +++ b/app/models/project_services/builds_email_service.rb @@ -0,0 +1,81 @@ +# == Schema Information +# +# Table name: services +# +# id :integer not null, primary key +# type :string(255) +# title :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# active :boolean default(FALSE), not null +# properties :text +# template :boolean default(FALSE) +# push_events :boolean default(TRUE) +# issues_events :boolean default(TRUE) +# merge_requests_events :boolean default(TRUE) +# tag_push_events :boolean default(TRUE) +# note_events :boolean default(TRUE), not null +# + +class BuildsEmailService < Service + prop_accessor :recipients + boolean_accessor :add_pusher + boolean_accessor :notify_only_broken_builds + validates :recipients, presence: true, if: :activated? + + def title + 'Builds emails' + end + + def description + 'Email the builds status to a list of recipients.' + end + + def to_param + 'builds_email' + end + + def supported_events + %w(build) + end + + def execute(push_data) + return unless supported_events.include?(push_data[:object_kind]) + + if should_build_be_notified?(push_data) + BuildEmailWorker.perform_async( + push_data[:build_id], + all_recipients(push_data), + push_data, + ) + end + end + + def fields + [ + { type: 'textarea', name: 'recipients', placeholder: 'Emails separated by whitespace' }, + { type: 'checkbox', name: 'add_pusher', label: 'Add pusher to recipients list' }, + { type: 'checkbox', name: 'notify_only_broken_builds' }, + ] + end + + def should_build_be_notified?(data) + case data[:build_status] + when 'success' + !notify_only_broken_builds? + when 'failed' + true + else + false + end + end + + def all_recipients(data) + if add_pusher? && data[:user][:email] + recipients + " #{data[:user][:email]}" + else + recipients + end + end +end diff --git a/app/models/project_services/ci/hip_chat_message.rb b/app/models/project_services/ci/hip_chat_message.rb deleted file mode 100644 index d89466b689f..00000000000 --- a/app/models/project_services/ci/hip_chat_message.rb +++ /dev/null @@ -1,73 +0,0 @@ -module Ci - class HipChatMessage - include Gitlab::Application.routes.url_helpers - - attr_reader :build - - def initialize(build) - @build = build - end - - def to_s - lines = Array.new - lines.push("#{project.name} - ") - lines.push("Commit ##{commit.id}
") - lines.push("#{commit.short_sha} #{commit.git_author_name} - #{commit.git_commit_message}
") - lines.push("#{humanized_status(commit_status)} in #{commit.duration} second(s).") - lines.join('') - end - - def status_color(build_or_commit=nil) - build_or_commit ||= commit_status - case build_or_commit - when :success - 'green' - when :failed, :canceled - 'red' - else # :pending, :running or unknown - 'yellow' - end - end - - def notify? - [:failed, :canceled].include?(commit_status) - end - - - private - - def commit - build.commit - end - - def project - commit.project - end - - def build_status - build.status.to_sym - end - - def commit_status - commit.status.to_sym - end - - def humanized_status(build_or_commit=nil) - build_or_commit ||= commit_status - case build_or_commit - when :pending - "Pending" - when :running - "Running" - when :failed - "Failed" - when :success - "Successful" - when :canceled - "Canceled" - else - "Unknown" - end - end - end -end diff --git a/app/models/project_services/ci/hip_chat_service.rb b/app/models/project_services/ci/hip_chat_service.rb deleted file mode 100644 index 0df03890efb..00000000000 --- a/app/models/project_services/ci/hip_chat_service.rb +++ /dev/null @@ -1,93 +0,0 @@ -# == Schema Information -# -# Table name: ci_services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -module Ci - class HipChatService < Ci::Service - prop_accessor :hipchat_token, :hipchat_room, :hipchat_server - boolean_accessor :notify_only_broken_builds - validates :hipchat_token, presence: true, if: :activated? - validates :hipchat_room, presence: true, if: :activated? - default_value_for :notify_only_broken_builds, true - - def title - "HipChat" - end - - def description - "Private group chat, video chat, instant messaging for teams" - end - - def help - end - - def to_param - 'hip_chat' - end - - def fields - [ - { type: 'text', name: 'hipchat_token', label: 'Token', placeholder: '' }, - { type: 'text', name: 'hipchat_room', label: 'Room', placeholder: '' }, - { type: 'text', name: 'hipchat_server', label: 'Server', placeholder: 'https://hipchat.example.com', help: 'Leave blank for default' }, - { type: 'checkbox', name: 'notify_only_broken_builds', label: 'Notify only broken builds' } - ] - end - - def can_execute?(build) - return if build.allow_failure? - - commit = build.commit - return unless commit - return unless commit.latest_builds.include? build - - case commit.status.to_sym - when :failed - true - when :success - true unless notify_only_broken_builds? - else - false - end - end - - def execute(build) - msg = Ci::HipChatMessage.new(build) - opts = default_options.merge( - token: hipchat_token, - room: hipchat_room, - server: server_url, - color: msg.status_color, - notify: msg.notify? - ) - Ci::HipChatNotifierWorker.perform_async(msg.to_s, opts) - end - - private - - def default_options - { - service_name: 'GitLab CI', - message_format: 'html' - } - end - - def server_url - if hipchat_server.blank? - 'https://api.hipchat.com' - else - hipchat_server - end - end - end -end diff --git a/app/models/project_services/ci/mail_service.rb b/app/models/project_services/ci/mail_service.rb deleted file mode 100644 index bb961d06972..00000000000 --- a/app/models/project_services/ci/mail_service.rb +++ /dev/null @@ -1,84 +0,0 @@ -# == Schema Information -# -# Table name: ci_services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -module Ci - class MailService < Ci::Service - delegate :email_recipients, :email_recipients=, - :email_add_pusher, :email_add_pusher=, - :email_only_broken_builds, :email_only_broken_builds=, to: :project, prefix: false - - before_save :update_project - - default_value_for :active, true - - def title - 'Mail' - end - - def description - 'Email notification' - end - - def to_param - 'mail' - end - - def fields - [ - { type: 'text', name: 'email_recipients', label: 'Recipients', help: 'Whitespace-separated list of recipient addresses' }, - { type: 'checkbox', name: 'email_add_pusher', label: 'Add pusher to recipients list' }, - { type: 'checkbox', name: 'email_only_broken_builds', label: 'Notify only broken builds' } - ] - end - - def can_execute?(build) - return if build.allow_failure? - - # it doesn't make sense to send emails for retried builds - commit = build.commit - return unless commit - return unless commit.latest_builds.include?(build) - - case build.status.to_sym - when :failed - true - when :success - true unless email_only_broken_builds - else - false - end - end - - def execute(build) - build.project_recipients.each do |recipient| - case build.status.to_sym - when :success - mailer.build_success_email(build.id, recipient).deliver_later - when :failed - mailer.build_fail_email(build.id, recipient).deliver_later - end - end - end - - private - - def update_project - project.save! - end - - def mailer - Ci::Notify - end - end -end diff --git a/app/models/project_services/ci/slack_message.rb b/app/models/project_services/ci/slack_message.rb deleted file mode 100644 index 1a6ff8e34c9..00000000000 --- a/app/models/project_services/ci/slack_message.rb +++ /dev/null @@ -1,92 +0,0 @@ -require 'slack-notifier' - -module Ci - class SlackMessage - include Gitlab::Application.routes.url_helpers - - def initialize(commit) - @commit = commit - end - - def pretext - '' - end - - def color - attachment_color - end - - def fallback - format(attachment_message) - end - - def attachments - fields = [] - - commit.latest_builds.each do |build| - next if build.allow_failure? - next unless build.failed? - fields << { - title: build.name, - value: "Build <#{namespace_project_build_url(build.gl_project.namespace, build.gl_project, build)}|\##{build.id}> failed in #{build.duration.to_i} second(s)." - } - end - - [{ - text: attachment_message, - color: attachment_color, - fields: fields - }] - end - - private - - attr_reader :commit - - def attachment_message - out = "<#{ci_project_url(project)}|#{project_name}>: " - out << "Commit <#{builds_namespace_project_commit_url(commit.gl_project.namespace, commit.gl_project, commit.sha)}|\##{commit.id}> " - out << "(<#{commit_sha_link}|#{commit.short_sha}>) " - out << "of <#{commit_ref_link}|#{commit.ref}> " - out << "by #{commit.git_author_name} " if commit.git_author_name - out << "#{commit_status} in " - out << "#{commit.duration} second(s)" - end - - def format(string) - Slack::Notifier::LinkFormatter.format(string) - end - - def project - commit.project - end - - def project_name - project.name - end - - def commit_sha_link - "#{project.gitlab_url}/commit/#{commit.sha}" - end - - def commit_ref_link - "#{project.gitlab_url}/commits/#{commit.ref}" - end - - def attachment_color - if commit.success? - 'good' - else - 'danger' - end - end - - def commit_status - if commit.success? - 'succeeded' - else - 'failed' - end - end - end -end diff --git a/app/models/project_services/ci/slack_service.rb b/app/models/project_services/ci/slack_service.rb deleted file mode 100644 index 7064bfe78db..00000000000 --- a/app/models/project_services/ci/slack_service.rb +++ /dev/null @@ -1,81 +0,0 @@ -# == Schema Information -# -# Table name: ci_services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -module Ci - class SlackService < Ci::Service - prop_accessor :webhook - boolean_accessor :notify_only_broken_builds - validates :webhook, presence: true, if: :activated? - - default_value_for :notify_only_broken_builds, true - - def title - 'Slack' - end - - def description - 'A team communication tool for the 21st century' - end - - def to_param - 'slack' - end - - def help - 'Visit https://www.slack.com/services/new/incoming-webhook. Then copy link and save project!' unless webhook.present? - end - - def fields - [ - { type: 'text', name: 'webhook', label: 'Webhook URL', placeholder: '' }, - { type: 'checkbox', name: 'notify_only_broken_builds', label: 'Notify only broken builds' } - ] - end - - def can_execute?(build) - return if build.allow_failure? - - commit = build.commit - return unless commit - return unless commit.latest_builds.include?(build) - - case commit.status.to_sym - when :failed - true - when :success - true unless notify_only_broken_builds? - else - false - end - end - - def execute(build) - message = Ci::SlackMessage.new(build.commit) - options = default_options.merge( - color: message.color, - fallback: message.fallback, - attachments: message.attachments - ) - Ci::SlackNotifierWorker.perform_async(webhook, message.pretext, options) - end - - private - - def default_options - { - username: 'GitLab CI' - } - end - end -end diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index af2840a57f0..6f96907ec18 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -22,6 +22,7 @@ class HipchatService < Service MAX_COMMITS = 3 prop_accessor :token, :room, :server, :notify, :color, :api_version + boolean_accessor :notify_only_broken_builds validates :token, presence: true, if: :activated? def title @@ -45,12 +46,13 @@ class HipchatService < Service { type: 'text', name: 'api_version', placeholder: 'Leave blank for default (v2)' }, { type: 'text', name: 'server', - placeholder: 'Leave blank for default. https://hipchat.example.com' } + placeholder: 'Leave blank for default. https://hipchat.example.com' }, + { type: 'checkbox', name: 'notify_only_broken_builds' }, ] end def supported_events - %w(push issue merge_request note tag_push) + %w(push issue merge_request note tag_push build) end def execute(data) @@ -94,6 +96,8 @@ class HipchatService < Service create_merge_request_message(data) unless is_update?(data) when "note" create_note_message(data) + when "build" + create_build_message(data) if should_build_be_notified?(data) end end @@ -235,6 +239,20 @@ class HipchatService < Service message end + def create_build_message(data) + ref_type = data[:tag] ? 'tag' : 'branch' + ref = data[:ref] + sha = data[:sha] + user_name = data[:commit][:author_name] + status = data[:commit][:status] + duration = data[:commit][:duration] + + branch_link = "#{ref}" + commit_link = "#{Commit.truncate_sha(sha)}" + + "#{project_link}: Commit #{commit_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status(status)} in #{duration} second(s)" + end + def project_name project.name_with_namespace.gsub(/\s/, '') end @@ -250,4 +268,24 @@ class HipchatService < Service def is_update?(data) data[:object_attributes][:action] == 'update' end + + def humanized_status(status) + case status + when 'success' + 'passed' + else + status + end + end + + def should_build_be_notified?(data) + case data[:commit][:status] + when 'success' + !notify_only_broken_builds? + when 'failed' + true + else + false + end + end end diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 7cd5e892507..35819491575 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -20,6 +20,7 @@ class SlackService < Service prop_accessor :webhook, :username, :channel + boolean_accessor :notify_only_broken_builds validates :webhook, presence: true, if: :activated? def title @@ -45,12 +46,13 @@ class SlackService < Service { type: 'text', name: 'webhook', placeholder: 'https://hooks.slack.com/services/...' }, { type: 'text', name: 'username', placeholder: 'username' }, - { type: 'text', name: 'channel', placeholder: '#channel' } + { type: 'text', name: 'channel', placeholder: '#channel' }, + { type: 'checkbox', name: 'notify_only_broken_builds' }, ] end def supported_events - %w(push issue merge_request note tag_push) + %w(push issue merge_request note tag_push build) end def execute(data) @@ -78,6 +80,8 @@ class SlackService < Service MergeMessage.new(data) unless is_update?(data) when "note" NoteMessage.new(data) + when "build" + BuildMessage.new(data) if should_build_be_notified?(data) end opt = {} @@ -86,7 +90,7 @@ class SlackService < Service if message notifier = Slack::Notifier.new(webhook, opt) - notifier.ping(message.pretext, attachments: message.attachments) + notifier.ping(message.pretext, attachments: message.attachments, fallback: message.fallback) end end @@ -103,9 +107,21 @@ class SlackService < Service def is_update?(data) data[:object_attributes][:action] == 'update' end + + def should_build_be_notified?(data) + case data[:commit][:status] + when 'success' + !notify_only_broken_builds? + when 'failed' + true + else + false + end + end end require "slack_service/issue_message" require "slack_service/push_message" require "slack_service/merge_message" require "slack_service/note_message" +require "slack_service/build_message" diff --git a/app/models/project_services/slack_service/base_message.rb b/app/models/project_services/slack_service/base_message.rb index aa00d6061a1..f1182824687 100644 --- a/app/models/project_services/slack_service/base_message.rb +++ b/app/models/project_services/slack_service/base_message.rb @@ -10,6 +10,9 @@ class SlackService format(message) end + def fallback + end + def attachments raise NotImplementedError end diff --git a/app/models/project_services/slack_service/build_message.rb b/app/models/project_services/slack_service/build_message.rb new file mode 100644 index 00000000000..c124cad4afd --- /dev/null +++ b/app/models/project_services/slack_service/build_message.rb @@ -0,0 +1,82 @@ +class SlackService + class BuildMessage < BaseMessage + attr_reader :sha + attr_reader :ref_type + attr_reader :ref + attr_reader :status + attr_reader :project_name + attr_reader :project_url + attr_reader :user_name + attr_reader :duration + + def initialize(params, commit = true) + @sha = params[:sha] + @ref_type = params[:tag] ? 'tag' : 'branch' + @ref = params[:ref] + @project_name = params[:project_name] + @project_url = params[:project_url] + @status = params[:commit][:status] + @user_name = params[:commit][:author_name] + @duration = params[:commit][:duration] + end + + def pretext + '' + end + + def fallback + format(message) + end + + def attachments + [{ text: format(message), color: attachment_color }] + end + + private + + def message + "#{project_link}: Commit #{commit_link} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status} in #{duration} second(s)" + 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 commit_url + "#{project_url}/commit/#{sha}/builds" + end + + def commit_link + "[#{Commit.truncate_sha(sha)}](#{commit_url})" + end + end +end diff --git a/app/models/service.rb b/app/models/service.rb index d610abd1683..195c4690e8f 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -30,6 +30,7 @@ class Service < ActiveRecord::Base default_value_for :merge_requests_events, true default_value_for :tag_push_events, true default_value_for :note_events, true + default_value_for :build_events, true after_initialize :initialize_properties @@ -47,6 +48,7 @@ class Service < ActiveRecord::Base scope :issue_hooks, -> { where(issues_events: true, active: true) } 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) } def activated? active @@ -133,6 +135,21 @@ class Service < ActiveRecord::Base end end + # Provide convenient boolean accessor methods + # for each serialized property. + # Also keep track of updated properties in a similar way as ActiveModel::Dirty + def self.boolean_accessor(*args) + self.prop_accessor(*args) + + args.each do |arg| + class_eval %{ + def #{arg}? + ActiveRecord::ConnectionAdapters::Column::TRUE_VALUES.include?(#{arg}) + end + } + end + end + # Returns a hash of the properties that have been assigned a new value since last save, # indicating their original values (attr => original value). # ActiveRecord does not provide a mechanism to track changes in serialized keys, @@ -163,6 +180,7 @@ class Service < ActiveRecord::Base assembla bamboo buildkite + builds_email campfire custom_issue_tracker drone_ci diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb index 912eb6258a4..847db2d48a7 100644 --- a/app/services/ci/create_builds_service.rb +++ b/app/services/ci/create_builds_service.rb @@ -31,7 +31,8 @@ module Ci trigger_request: trigger_request, user: user) - commit.builds.create!(build_attrs) + build = commit.builds.create!(build_attrs) + build.execute_hooks end end end diff --git a/app/views/ci/notify/build_fail_email.html.haml b/app/views/ci/notify/build_fail_email.html.haml deleted file mode 100644 index de6291aa914..00000000000 --- a/app/views/ci/notify/build_fail_email.html.haml +++ /dev/null @@ -1,23 +0,0 @@ -- content_for :header do - %h1{style: "background: #c40834; color: #FFF; font: normal 20px Helvetica, Arial, sans-serif; margin: 0; padding: 5px 10px; line-height: 32px; font-size: 16px;"} - GitLab CI (build failed) -%h3 - Project: - = link_to ci_project_url(@project) do - = @project.name - -%p - Commit: #{link_to @build.short_sha, namespace_project_commit_url(@build.gl_project.namespace, @build.gl_project, @build.sha)} -%p - Author: #{@build.commit.git_author_name} -%p - Branch: #{@build.ref} -%p - Stage: #{@build.stage} -%p - Job: #{@build.name} -%p - Message: #{@build.commit.git_commit_message} - -%p - Build details: #{link_to "Build #{@build.id}", namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)} diff --git a/app/views/ci/notify/build_fail_email.text.erb b/app/views/ci/notify/build_fail_email.text.erb deleted file mode 100644 index 17a3b9b1d33..00000000000 --- a/app/views/ci/notify/build_fail_email.text.erb +++ /dev/null @@ -1,11 +0,0 @@ -Build failed for <%= @project.name %> - -Status: <%= @build.status %> -Commit: <%= @build.commit.short_sha %> -Author: <%= @build.commit.git_author_name %> -Branch: <%= @build.ref %> -Stage: <%= @build.stage %> -Job: <%= @build.name %> -Message: <%= @build.commit.git_commit_message %> - -Url: <%= namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build) %> diff --git a/app/views/ci/notify/build_success_email.html.haml b/app/views/ci/notify/build_success_email.html.haml deleted file mode 100644 index 6ef1fd67d89..00000000000 --- a/app/views/ci/notify/build_success_email.html.haml +++ /dev/null @@ -1,24 +0,0 @@ -- content_for :header do - %h1{style: "background: #38CF5B; color: #FFF; font: normal 20px Helvetica, Arial, sans-serif; margin: 0; padding: 5px 10px; line-height: 32px; font-size: 16px;"} - GitLab CI (build successful) - -%h3 - Project: - = link_to ci_project_url(@project) do - = @project.name - -%p - Commit: #{link_to @build.short_sha, namespace_project_commit_url(@build.gl_project.namespace, @build.gl_project, @build.sha)} -%p - Author: #{@build.commit.git_author_name} -%p - Branch: #{@build.ref} -%p - Stage: #{@build.stage} -%p - Job: #{@build.name} -%p - Message: #{@build.commit.git_commit_message} - -%p - Build details: #{link_to "Build #{@build.id}", namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)} diff --git a/app/views/ci/notify/build_success_email.text.erb b/app/views/ci/notify/build_success_email.text.erb deleted file mode 100644 index bc8b978c3d7..00000000000 --- a/app/views/ci/notify/build_success_email.text.erb +++ /dev/null @@ -1,11 +0,0 @@ -Build successful for <%= @project.name %> - -Status: <%= @build.status %> -Commit: <%= @build.commit.short_sha %> -Author: <%= @build.commit.git_author_name %> -Branch: <%= @build.ref %> -Stage: <%= @build.stage %> -Job: <%= @build.name %> -Message: <%= @build.commit.git_commit_message %> - -Url: <%= namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build) %> diff --git a/app/views/layouts/nav/_project_settings.html.haml b/app/views/layouts/nav/_project_settings.html.haml index f0b3f27b626..4b32c631d5c 100644 --- a/app/views/layouts/nav/_project_settings.html.haml +++ b/app/views/layouts/nav/_project_settings.html.haml @@ -50,18 +50,8 @@ = icon('retweet fw') %span Triggers - = nav_link path: 'ci_web_hooks#index' do - = link_to namespace_project_ci_web_hooks_path(@project.namespace, @project), title: 'CI Web Hooks' do - = icon('link fw') - %span - CI Web Hooks = nav_link path: 'ci_settings#edit' do = link_to edit_namespace_project_ci_settings_path(@project.namespace, @project), title: 'CI Settings' do = icon('building fw') %span CI Settings - = nav_link controller: 'ci_services' do - = link_to namespace_project_ci_services_path(@project.namespace, @project), title: 'CI Services' do - = icon('share fw') - %span - CI Services diff --git a/app/views/notify/build_fail_email.html.haml b/app/views/notify/build_fail_email.html.haml new file mode 100644 index 00000000000..3b251dc011a --- /dev/null +++ b/app/views/notify/build_fail_email.html.haml @@ -0,0 +1,23 @@ +- content_for :header do + %h1{style: "background: #c40834; color: #FFF; font: normal 20px Helvetica, Arial, sans-serif; margin: 0; padding: 5px 10px; line-height: 32px; font-size: 16px;"} + GitLab (build failed) +%h3 + Project: + = link_to ci_project_url(@project) do + = @project.name + +%p + Commit: #{link_to @build.short_sha, namespace_project_commit_url(@build.gl_project.namespace, @build.gl_project, @build.sha)} +%p + Author: #{@build.commit.git_author_name} +%p + Branch: #{@build.ref} +%p + Stage: #{@build.stage} +%p + Job: #{@build.name} +%p + Message: #{@build.commit.git_commit_message} + +%p + Build details: #{link_to "Build #{@build.id}", namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)} diff --git a/app/views/notify/build_fail_email.text.erb b/app/views/notify/build_fail_email.text.erb new file mode 100644 index 00000000000..17a3b9b1d33 --- /dev/null +++ b/app/views/notify/build_fail_email.text.erb @@ -0,0 +1,11 @@ +Build failed for <%= @project.name %> + +Status: <%= @build.status %> +Commit: <%= @build.commit.short_sha %> +Author: <%= @build.commit.git_author_name %> +Branch: <%= @build.ref %> +Stage: <%= @build.stage %> +Job: <%= @build.name %> +Message: <%= @build.commit.git_commit_message %> + +Url: <%= namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build) %> diff --git a/app/views/notify/build_success_email.html.haml b/app/views/notify/build_success_email.html.haml new file mode 100644 index 00000000000..c23f4b7e45a --- /dev/null +++ b/app/views/notify/build_success_email.html.haml @@ -0,0 +1,24 @@ +- content_for :header do + %h1{style: "background: #38CF5B; color: #FFF; font: normal 20px Helvetica, Arial, sans-serif; margin: 0; padding: 5px 10px; line-height: 32px; font-size: 16px;"} + GitLab (build successful) + +%h3 + Project: + = link_to ci_project_url(@project) do + = @project.name + +%p + Commit: #{link_to @build.short_sha, namespace_project_commit_url(@build.gl_project.namespace, @build.gl_project, @build.sha)} +%p + Author: #{@build.commit.git_author_name} +%p + Branch: #{@build.ref} +%p + Stage: #{@build.stage} +%p + Job: #{@build.name} +%p + Message: #{@build.commit.git_commit_message} + +%p + Build details: #{link_to "Build #{@build.id}", namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)} diff --git a/app/views/notify/build_success_email.text.erb b/app/views/notify/build_success_email.text.erb new file mode 100644 index 00000000000..bc8b978c3d7 --- /dev/null +++ b/app/views/notify/build_success_email.text.erb @@ -0,0 +1,11 @@ +Build successful for <%= @project.name %> + +Status: <%= @build.status %> +Commit: <%= @build.commit.short_sha %> +Author: <%= @build.commit.git_author_name %> +Branch: <%= @build.ref %> +Stage: <%= @build.stage %> +Job: <%= @build.name %> +Message: <%= @build.commit.git_commit_message %> + +Url: <%= namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build) %> diff --git a/app/views/projects/ci_services/_form.html.haml b/app/views/projects/ci_services/_form.html.haml deleted file mode 100644 index 397832e56db..00000000000 --- a/app/views/projects/ci_services/_form.html.haml +++ /dev/null @@ -1,54 +0,0 @@ -%h3.page-title - = @service.title - = boolean_to_icon @service.activated? - -%p= @service.description - - -%hr - -= form_for(@service, as: :service, url: namespace_project_ci_service_path(@project.namespace, @project, @service.to_param), method: :put, html: { class: 'form-horizontal' }) do |f| - - if @service.errors.any? - .alert.alert-danger - %ul - - @service.errors.full_messages.each do |msg| - %li= msg - - - if @service.help.present? - .bs-callout - = @service.help - - .form-group - = f.label :active, "Active", class: "control-label" - .col-sm-10 - = f.check_box :active - - - @service.fields.each do |field| - - name = field[:name] - - label = field[:label] || name - - value = @service.send(name) - - type = field[:type] - - placeholder = field[:placeholder] - - choices = field[:choices] - - default_choice = field[:default_choice] - - help = field[:help] - - .form-group - = f.label label, class: "control-label" - .col-sm-10 - - if type == 'text' - = f.text_field name, class: "form-control", placeholder: placeholder - - elsif type == 'textarea' - = f.text_area name, rows: 5, class: "form-control", placeholder: placeholder - - elsif type == 'checkbox' - = f.check_box name - - elsif type == 'select' - = f.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control" } - - if help - .light #{help} - - .form-actions - = f.submit 'Save', class: 'btn btn-save' -   - - if @service.valid? && @service.activated? && @service.can_test? - = link_to 'Test settings', test_namespace_project_ci_service_path(@project.namespace, @project, @service.to_param), class: 'btn' diff --git a/app/views/projects/ci_services/edit.html.haml b/app/views/projects/ci_services/edit.html.haml deleted file mode 100644 index dacb6b4f6f4..00000000000 --- a/app/views/projects/ci_services/edit.html.haml +++ /dev/null @@ -1,2 +0,0 @@ -- page_title @service.title, "CI Services" -= render 'form' diff --git a/app/views/projects/ci_services/index.html.haml b/app/views/projects/ci_services/index.html.haml deleted file mode 100644 index 3f26c7851d8..00000000000 --- a/app/views/projects/ci_services/index.html.haml +++ /dev/null @@ -1,23 +0,0 @@ -- page_title "CI Services" -%h3.page-title Project services -%p.light Project services allow you to integrate GitLab CI with other applications - -%table.table - %thead - %tr - %th - %th Service - %th Description - %th Last edit - - @services.sort_by(&:title).each do |service| - %tr - %td - = boolean_to_icon service.activated? - %td - = link_to edit_namespace_project_ci_service_path(@project.namespace, @project, service.to_param) do - %strong= service.title - %td - = service.description - %td.light - = time_ago_in_words service.updated_at - ago diff --git a/app/views/projects/ci_web_hooks/index.html.haml b/app/views/projects/ci_web_hooks/index.html.haml deleted file mode 100644 index 2998fb08ff1..00000000000 --- a/app/views/projects/ci_web_hooks/index.html.haml +++ /dev/null @@ -1,94 +0,0 @@ -- page_title "CI Web Hooks" -%h3.page-title - CI Web hooks - -%p.light - Web Hooks can be used for binding events when build completed. - -%hr.clearfix - -= form_for @web_hook, url: namespace_project_ci_web_hooks_path(@project.namespace, @project), html: { class: 'form-horizontal' } do |f| - -if @web_hook.errors.any? - .alert.alert-danger - - @web_hook.errors.full_messages.each do |msg| - %p= msg - .form-group - = f.label :url, "URL", class: 'control-label' - .col-sm-10 - = f.text_field :url, class: "form-control", placeholder: 'http://example.com/trigger-ci.json' - .form-actions - = f.submit "Add Web Hook", class: "btn btn-create" - --if @web_hooks.any? - %h4 Activated web hooks (#{@web_hooks.count}) - .table-holder - %table.table - - @web_hooks.each do |hook| - %tr - %td - .clearfix - %span.monospace= hook.url - %td - .pull-right - - if @ci_project.commits.any? - = link_to 'Test Hook', test_namespace_project_ci_web_hook_path(@project.namespace, @project, hook), class: "btn btn-sm btn-grouped" - = link_to 'Remove', namespace_project_ci_web_hook_path(@project.namespace, @project, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove btn-sm btn-grouped" - -%h4 Web Hook data example - -:erb - 
-    
-      {
-        "build_id": 2,
-        "build_name":"rspec_linux"
-        "build_status": "failed",
-        "build_started_at": "2014-05-05T18:01:02.563Z",
-        "build_finished_at": "2014-05-05T18:01:07.611Z",
-        "project_id": 1,
-        "project_name": "Brightbox \/ Brightbox Cli",
-        "gitlab_url": "http:\/\/localhost:3000\/brightbox\/brightbox-cli",
-        "ref": "master",
-        "sha": "a26cf5de9ed9827746d4970872376b10d9325f40",
-        "before_sha": "34f57f6ba3ed0c21c5e361bbb041c3591411176c",
-        "push_data": {
-          "before": "34f57f6ba3ed0c21c5e361bbb041c3591411176c",
-          "after": "a26cf5de9ed9827746d4970872376b10d9325f40",
-          "ref": "refs\/heads\/master",
-          "user_id": 1,
-          "user_name": "Administrator",
-          "project_id": 5,
-          "repository": {
-            "name": "Brightbox Cli",
-            "url": "dzaporozhets@localhost:brightbox\/brightbox-cli.git",
-            "description": "Voluptatibus quae error consectetur voluptas dolores vel excepturi possimus.",
-            "homepage": "http:\/\/localhost:3000\/brightbox\/brightbox-cli"
-          },
-          "commits": [
-            {
-              "id": "a26cf5de9ed9827746d4970872376b10d9325f40",
-              "message": "Release v1.2.2",
-              "timestamp": "2014-04-22T16:46:42+03:00",
-              "url": "http:\/\/localhost:3000\/brightbox\/brightbox-cli\/commit\/a26cf5de9ed9827746d4970872376b10d9325f40",
-              "author": {
-                "name": "Paul Thornthwaite",
-                "email": "tokengeek@gmail.com"
-              }
-            },
-            {
-              "id": "34f57f6ba3ed0c21c5e361bbb041c3591411176c",
-              "message": "Fix server user data update\n\nIncorrect condition was being used so Base64 encoding option was having\nopposite effect from desired.",
-              "timestamp": "2014-04-11T18:17:26+03:00",
-              "url": "http:\/\/localhost:3000\/brightbox\/brightbox-cli\/commit\/34f57f6ba3ed0c21c5e361bbb041c3591411176c",
-              "author": {
-                "name": "Paul Thornthwaite",
-                "email": "tokengeek@gmail.com"
-              }
-            }
-          ],
-          "total_commits_count": 2,
-          "ci_yaml_file":"rspec_linux:\r\n  script: ls\r\n"
-        }
-      }
-    
-
diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml index 3702aeaecba..b18d9197d0b 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -55,6 +55,13 @@ %strong Merge Request events %p.light This url will be triggered when a merge request is created + %div + = f.check_box :build_events, class: 'pull-left' + .prepend-left-20 + = f.label :build_events, class: 'list-label' do + %strong Build events + %p.light + This url will be triggered when the build status changes .form-group = f.label :enable_ssl_verification, "SSL verification", class: 'control-label checkbox' .col-sm-10 @@ -78,7 +85,7 @@ .clearfix %span.monospace= hook.url %p - - %w(push_events tag_push_events issues_events note_events merge_requests_events).each do |trigger| + - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray= trigger.titleize SSL Verification: #{hook.enable_ssl_verification ? "enabled" : "disabled"} diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml index 16a98a7233c..28d6f421fea 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_service_settings.html.haml @@ -59,6 +59,15 @@ %strong Merge Request events %p.light This url will be triggered when a merge request is created + - if @service.supported_events.include?("build") + %div + = form.check_box :build_events, class: 'pull-left' + .prepend-left-20 + = form.label :build_events, class: 'list-label' do + %strong Build events + %p.light + This url will be triggered when a build status changes + - @service.fields.each do |field| - type = field[:type] diff --git a/app/workers/build_email_worker.rb b/app/workers/build_email_worker.rb new file mode 100644 index 00000000000..c5c8055bea7 --- /dev/null +++ b/app/workers/build_email_worker.rb @@ -0,0 +1,19 @@ +class BuildEmailWorker + include Sidekiq::Worker + + def perform(build_id, recipients, push_data) + recipients.split(' ').each do |recipient| + begin + case push_data['build_status'] + when 'success' + Notify.build_success_email(build_id, recipient).deliver_now + when 'failed' + Notify.build_fail_email(build_id, recipient).deliver_now + end + # These are input errors and won't be corrected even if Sidekiq retries + rescue Net::SMTPFatalError, Net::SMTPSyntaxError => e + logger.info("Failed to send e-mail for project '#{push_data['project_name']}' to #{recipient}: #{e}") + end + end + end +end diff --git a/app/workers/ci/hip_chat_notifier_worker.rb b/app/workers/ci/hip_chat_notifier_worker.rb deleted file mode 100644 index ebb43570e2a..00000000000 --- a/app/workers/ci/hip_chat_notifier_worker.rb +++ /dev/null @@ -1,19 +0,0 @@ -module Ci - class HipChatNotifierWorker - include Sidekiq::Worker - - def perform(message, options={}) - room = options.delete('room') - token = options.delete('token') - server = options.delete('server') - name = options.delete('service_name') - client_opts = { - api_version: 'v2', - server_url: server - } - - client = HipChat::Client.new(token, client_opts) - client[room].send(name, message, options.symbolize_keys) - end - end -end diff --git a/app/workers/ci/slack_notifier_worker.rb b/app/workers/ci/slack_notifier_worker.rb deleted file mode 100644 index 3bbb9b4bec7..00000000000 --- a/app/workers/ci/slack_notifier_worker.rb +++ /dev/null @@ -1,10 +0,0 @@ -module Ci - class SlackNotifierWorker - include Sidekiq::Worker - - def perform(webhook_url, message, options={}) - notifier = Slack::Notifier.new(webhook_url) - notifier.ping(message, options) - end - end -end diff --git a/app/workers/ci/web_hook_worker.rb b/app/workers/ci/web_hook_worker.rb deleted file mode 100644 index 0bb83845572..00000000000 --- a/app/workers/ci/web_hook_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Ci - class WebHookWorker - include Sidekiq::Worker - - def perform(hook_id, data) - Ci::WebHook.find(hook_id).execute data - end - end -end diff --git a/config/routes.rb b/config/routes.rb index 061a8fd5da4..a104e686ac6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -596,17 +596,6 @@ Rails.application.routes.draw do resource :variables, only: [:show, :update] resources :triggers, only: [:index, :create, :destroy] resource :ci_settings, only: [:edit, :update, :destroy] - resources :ci_web_hooks, only: [:index, :create, :destroy] do - member do - get :test - end - end - - resources :ci_services, constraints: { id: /[^\/]+/ }, only: [:index, :edit, :update] do - member do - get :test - end - end resources :builds, only: [:index, :show] do collection do diff --git a/db/migrate/20151203162134_add_build_events_to_services.rb b/db/migrate/20151203162134_add_build_events_to_services.rb new file mode 100644 index 00000000000..a84be7db3f1 --- /dev/null +++ b/db/migrate/20151203162134_add_build_events_to_services.rb @@ -0,0 +1,6 @@ +class AddBuildEventsToServices < ActiveRecord::Migration + def up + add_column :services, :build_events, :boolean, default: false, null: false + add_column :web_hooks, :build_events, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 94b87040d88..58595f8ae59 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20151203162133) do +ActiveRecord::Schema.define(version: 20151203162134) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -706,6 +706,7 @@ ActiveRecord::Schema.define(version: 20151203162133) do t.boolean "merge_requests_events", default: true t.boolean "tag_push_events", default: true t.boolean "note_events", default: true, null: false + t.boolean "build_events", default: false, null: false end add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree @@ -854,6 +855,7 @@ ActiveRecord::Schema.define(version: 20151203162133) do t.boolean "tag_push_events", default: false t.boolean "note_events", default: false, null: false t.boolean "enable_ssl_verification", default: true + t.boolean "build_events", default: false, null: false end add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree diff --git a/features/project/service.feature b/features/project/service.feature index 5014b52b9f6..13edc6cb2b9 100644 --- a/features/project/service.feature +++ b/features/project/service.feature @@ -55,11 +55,11 @@ Feature: Project Services And I fill Pushover settings Then I should see Pushover service settings saved - Scenario: Activate email on push service + Scenario: Activate email service When I visit project "Shop" services page - And I click email on push service link - And I fill email on push settings - Then I should see email on push service settings saved + And I click email service link + And I fill email settings + Then I should see email service settings saved Scenario: Activate Irker (IRC Gateway) service When I visit project "Shop" services page diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb index 6acbf46eb20..037f7494a77 100644 --- a/features/steps/admin/settings.rb +++ b/features/steps/admin/settings.rb @@ -32,6 +32,7 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps page.check('Comments') page.check('Issues events') page.check('Merge Request events') + page.check('Build events') click_on 'Save' end @@ -39,6 +40,7 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps fill_in 'Webhook', with: 'http://localhost' fill_in 'Username', with: 'test_user' fill_in 'Channel', with: '#test_channel' + page.check('Notify only broken builds') end step 'I should see service template settings saved' do diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index 1c700df0c63..2d564dac498 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -118,16 +118,16 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps expect(find_field('Restrict to branch').value).to eq 'master' end - step 'I click email on push service link' do - click_link 'Emails on push' + step 'I click email service link' do + click_link 'Emails' end - step 'I fill email on push settings' do + step 'I fill email settings' do fill_in 'Recipients', with: 'qa@company.name' click_button 'Save' end - step 'I should see email on push service settings saved' do + step 'I should see email service settings saved' do expect(find_field('Recipients').value).to eq 'qa@company.name' end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 81bf7a8222b..03e3056a87e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -45,7 +45,8 @@ module API class ProjectHook < Hook expose :project_id, :push_events - expose :issues_events, :merge_requests_events, :tag_push_events, :note_events, :enable_ssl_verification + expose :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events + expose :enable_ssl_verification end class ForkedFromProject < Grape::Entity @@ -252,7 +253,7 @@ 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 + expose :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_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 882d1a083ad..cf9938d25a7 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -45,6 +45,7 @@ module API :merge_requests_events, :tag_push_events, :note_events, + :build_events, :enable_ssl_verification ] @hook = user_project.hooks.new(attrs) @@ -77,6 +78,7 @@ module API :merge_requests_events, :tag_push_events, :note_events, + :build_events, :enable_ssl_verification ] diff --git a/lib/ci/api/projects.rb b/lib/ci/api/projects.rb index d719ad9e8d5..23c79f3879d 100644 --- a/lib/ci/api/projects.rb +++ b/lib/ci/api/projects.rb @@ -5,30 +5,6 @@ module Ci before { authenticate! } resource :projects do - # Register new webhook for project - # - # Parameters - # project_id (required) - The ID of a project - # web_hook (required) - WebHook URL - # Example Request - # POST /projects/:project_id/webhooks - post ":project_id/webhooks" do - required_attributes! [:web_hook] - - project = Ci::Project.find(params[:project_id]) - - unauthorized! unless can?(current_user, :admin_project, project.gl_project) - - web_hook = project.web_hooks.new({ url: params[:web_hook] }) - - if web_hook.save - present web_hook, with: Entities::WebHook - else - errors = web_hook.errors.full_messages.join(", ") - render_api_error!(errors, 400) - end - end - # Retrieve all Gitlab CI projects that the user has access to # # Example Request: @@ -121,20 +97,6 @@ module Ci end end - # Remove a Gitlab CI project - # - # Parameters: - # id (required) - The ID of a project - # Example Request: - # DELETE /projects/:id - delete ":id" do - project = Ci::Project.find(params[:id]) - - unauthorized! unless can?(current_user, :admin_project, project.gl_project) - - project.destroy - end - # Link a Gitlab CI project to a runner # # Parameters: diff --git a/lib/gitlab/build_data_builder.rb b/lib/gitlab/build_data_builder.rb new file mode 100644 index 00000000000..fa2cd551cee --- /dev/null +++ b/lib/gitlab/build_data_builder.rb @@ -0,0 +1,64 @@ +module Gitlab + class BuildDataBuilder + class << self + def build(build) + project = build.gl_project + commit = build.commit + user = build.user + + data = { + object_kind: 'build', + + ref: build.ref, + tag: build.tag, + before_sha: build.before_sha, + sha: build.sha, + + # TODO: should this be not prefixed with build_? + # Leaving this way to have backward compatibility + build_id: build.id, + build_name: build.name, + build_stage: build.stage, + build_status: build.status, + build_started_at: build.started_at, + build_finished_at: build.finished_at, + build_duration: build.duration, + + # TODO: do we still need it? + project_id: project.id, + project_name: project.name_with_namespace, + + user: { + id: user.try(:id), + name: user.try(:name), + email: user.try(:email), + }, + + commit: { + id: commit.id, + sha: commit.sha, + message: commit.git_commit_message, + author_name: commit.git_author_name, + author_email: commit.git_author_email, + status: commit.status, + duration: commit.duration, + started_at: commit.started_at, + finished_at: commit.finished_at, + }, + + repository: { + name: project.name, + url: project.url_to_repo, + description: project.description, + homepage: project.web_url, + git_http_url: project.http_url_to_repo, + git_ssh_url: project.ssh_url_to_repo, + visibility_level: project.visibility_level + }, + } + + data + end + end + end +end diff --git a/spec/factories/ci/web_hook.rb b/spec/factories/ci/web_hook.rb deleted file mode 100644 index 40d878ecb3c..00000000000 --- a/spec/factories/ci/web_hook.rb +++ /dev/null @@ -1,6 +0,0 @@ -FactoryGirl.define do - factory :ci_web_hook, class: Ci::WebHook do - sequence(:url) { FFaker::Internet.uri('http') } - project factory: :ci_project - end -end diff --git a/spec/features/ci_web_hooks_spec.rb b/spec/features/ci_web_hooks_spec.rb deleted file mode 100644 index efae0a42c1e..00000000000 --- a/spec/features/ci_web_hooks_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'spec_helper' - -describe 'CI web hooks' do - let(:user) { create(:user) } - before { login_as(user) } - - before do - @project = FactoryGirl.create :ci_project - @gl_project = @project.gl_project - @gl_project.team << [user, :master] - visit namespace_project_ci_web_hooks_path(@gl_project.namespace, @gl_project) - end - - context 'create a trigger' do - before do - fill_in 'web_hook_url', with: 'http://example.com' - click_on 'Add Web Hook' - end - - it { expect(@project.web_hooks.count).to eq(1) } - - it 'revokes the trigger' do - click_on 'Remove' - expect(@project.web_hooks.count).to eq(0) - end - end -end diff --git a/spec/lib/gitlab/build_data_builder_spec.rb b/spec/lib/gitlab/build_data_builder_spec.rb new file mode 100644 index 00000000000..af2de207eba --- /dev/null +++ b/spec/lib/gitlab/build_data_builder_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe 'Gitlab::BuildDataBuilder' do + let(:build) { create(:ci_build) } + + describe :build do + let(:data) do + Gitlab::BuildDataBuilder.build(build) + end + + it { expect(data).to be_a(Hash) } + it { expect(data[:ref]).to eq(build.ref) } + it { expect(data[:sha]).to eq(build.sha) } + it { expect(data[:tag]).to eq(build.tag) } + it { expect(data[:build_id]).to eq(build.id) } + it { expect(data[:build_status]).to eq(build.status) } + it { expect(data[:project_id]).to eq(build.gl_project.id) } + it { expect(data[:project_name]).to eq(build.gl_project.name_with_namespace) } + end +end diff --git a/spec/mailers/ci/notify_spec.rb b/spec/mailers/ci/notify_spec.rb deleted file mode 100644 index b83fb41603b..00000000000 --- a/spec/mailers/ci/notify_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'spec_helper' - -describe Ci::Notify do - include EmailSpec::Helpers - include EmailSpec::Matchers - - before do - @commit = FactoryGirl.create :ci_commit - @build = FactoryGirl.create :ci_build, commit: @commit - end - - describe 'build success' do - subject { Ci::Notify.build_success_email(@build.id, 'wow@example.com') } - - it 'has the correct subject' do - should have_subject /Build success for/ - end - - it 'contains name of project' do - should have_body_text /build successful/ - end - end - - describe 'build fail' do - subject { Ci::Notify.build_fail_email(@build.id, 'wow@example.com') } - - it 'has the correct subject' do - should have_subject /Build failed for/ - end - - it 'contains name of project' do - should have_body_text /build failed/ - end - end -end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 27e509933b2..154901a2fbc 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -13,6 +13,7 @@ describe Notify do let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to } let(:recipient) { create(:user, email: 'recipient@example.com') } let(:project) { create(:project) } + let(:build) { create(:ci_build) } before(:each) do ActionMailer::Base.deliveries.clear @@ -865,4 +866,32 @@ describe Notify do is_expected.to have_body_text /#{diff_path}/ end end + + describe 'build success' do + before { build.success } + + subject { Notify.build_success_email(build.id, 'wow@example.com') } + + it 'has the correct subject' do + should have_subject /Build success for/ + end + + it 'contains name of project' do + should have_body_text build.project_name + end + end + + describe 'build fail' do + before { build.drop } + + subject { Notify.build_fail_email(build.id, 'wow@example.com') } + + it 'has the correct subject' do + should have_subject /Build failed for/ + end + + it 'contains name of project' do + should have_body_text build.project_name + end + end end diff --git a/spec/models/ci/project_services/hip_chat_message_spec.rb b/spec/models/ci/project_services/hip_chat_message_spec.rb deleted file mode 100644 index 7d54b6cf84c..00000000000 --- a/spec/models/ci/project_services/hip_chat_message_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'spec_helper' - -describe Ci::HipChatMessage, models: true do - subject { Ci::HipChatMessage.new(build) } - - let(:commit) { FactoryGirl.create(:ci_commit_with_two_jobs) } - - let(:build) do - commit.builds.first - end - - context 'when all matrix builds succeed' do - it 'returns a successful message' do - commit.create_builds('master', false, nil) - commit.builds.update_all(status: "success") - commit.reload - - expect(subject.status_color).to eq 'green' - expect(subject.notify?).to be_falsey - expect(subject.to_s).to match(/Commit #\d+/) - expect(subject.to_s).to match(/Successful in \d+ second\(s\)\./) - end - end - - context 'when at least one matrix build fails' do - it 'returns a failure message' do - commit.create_builds('master', false, nil) - first_build = commit.builds.first - second_build = commit.builds.last - first_build.update(status: "success") - second_build.update(status: "failed") - - expect(subject.status_color).to eq 'red' - expect(subject.notify?).to be_truthy - expect(subject.to_s).to match(/Commit #\d+/) - expect(subject.to_s).to match(/Failed in \d+ second\(s\)\./) - end - end -end diff --git a/spec/models/ci/project_services/hip_chat_service_spec.rb b/spec/models/ci/project_services/hip_chat_service_spec.rb deleted file mode 100644 index 714f1e17e0b..00000000000 --- a/spec/models/ci/project_services/hip_chat_service_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -# == Schema Information -# -# Table name: services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - - -require 'spec_helper' - -describe Ci::HipChatService, models: true do - - describe "Validations" do - - context "active" do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of :hipchat_room } - it { is_expected.to validate_presence_of :hipchat_token } - - end - end - - describe "Execute" do - - let(:service) { Ci::HipChatService.new } - let(:commit) { FactoryGirl.create :ci_commit } - let(:build) { FactoryGirl.create :ci_build, commit: commit, status: 'failed' } - let(:api_url) { 'https://api.hipchat.com/v2/room/123/notification?auth_token=a1b2c3d4e5f6' } - - before do - allow(service).to receive_messages( - project: commit.project, - project_id: commit.project_id, - notify_only_broken_builds: false, - hipchat_room: 123, - hipchat_token: 'a1b2c3d4e5f6' - ) - - WebMock.stub_request(:post, api_url) - end - - - it "should call the HipChat API" do - service.execute(build) - Ci::HipChatNotifierWorker.drain - - expect( WebMock ).to have_requested(:post, api_url).once - end - - it "calls the worker with expected arguments" do - expect( Ci::HipChatNotifierWorker ).to receive(:perform_async) \ - .with(an_instance_of(String), hash_including( - token: 'a1b2c3d4e5f6', - room: 123, - server: 'https://api.hipchat.com', - color: 'red', - notify: true - )) - - service.execute(build) - end - end -end diff --git a/spec/models/ci/project_services/mail_service_spec.rb b/spec/models/ci/project_services/mail_service_spec.rb deleted file mode 100644 index 638d9a4a626..00000000000 --- a/spec/models/ci/project_services/mail_service_spec.rb +++ /dev/null @@ -1,177 +0,0 @@ -# == Schema Information -# -# Table name: services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -require 'spec_helper' - -describe Ci::MailService, models: true do - describe "Associations" do - it { is_expected.to belong_to :project } - end - - describe "Validations" do - context "active" do - before do - subject.active = true - end - end - end - - describe 'Sends email for' do - let(:mail) { Ci::MailService.new } - let(:user) { User.new(notification_email: 'git@example.com')} - - describe 'failed build' do - let(:project) { FactoryGirl.create(:ci_project, email_add_pusher: true) } - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) } - let(:build) { FactoryGirl.create(:ci_build, status: 'failed', commit: commit, user: user) } - - before do - allow(mail).to receive_messages( - project: project - ) - end - - it do - perform_enqueued_jobs do - expect{ mail.execute(build) }.to change{ ActionMailer::Base.deliveries.size }.by(1) - expect(ActionMailer::Base.deliveries.last.to).to eq(["git@example.com"]) - end - end - end - - describe 'successfull build' do - let(:project) { FactoryGirl.create(:ci_project, email_add_pusher: true, email_only_broken_builds: false) } - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) } - let(:build) { FactoryGirl.create(:ci_build, status: 'success', commit: commit, user: user) } - - before do - allow(mail).to receive_messages( - project: project - ) - end - - it do - perform_enqueued_jobs do - expect{ mail.execute(build) }.to change{ ActionMailer::Base.deliveries.size }.by(1) - expect(ActionMailer::Base.deliveries.last.to).to eq(["git@example.com"]) - end - end - end - - describe 'successfull build and project has email_recipients' do - let(:project) do - FactoryGirl.create(:ci_project, - email_add_pusher: true, - email_only_broken_builds: false, - email_recipients: "jeroen@example.com") - end - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) } - let(:build) { FactoryGirl.create(:ci_build, status: 'success', commit: commit, user: user) } - - before do - allow(mail).to receive_messages( - project: project - ) - end - - it do - perform_enqueued_jobs do - expect{ mail.execute(build) }.to change{ ActionMailer::Base.deliveries.size }.by(2) - expect( - ActionMailer::Base.deliveries.map(&:to).flatten - ).to include("git@example.com", "jeroen@example.com") - end - end - end - - describe 'successful build and notify only broken builds' do - let(:project) do - FactoryGirl.create(:ci_project, - email_add_pusher: true, - email_only_broken_builds: true, - email_recipients: "jeroen@example.com") - end - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) } - let(:build) { FactoryGirl.create(:ci_build, status: 'success', commit: commit, user: user) } - - before do - allow(mail).to receive_messages( - project: project - ) - end - - it do - perform_enqueued_jobs do - expect do - mail.execute(build) if mail.can_execute?(build) - end.to_not change{ ActionMailer::Base.deliveries.size } - end - end - end - - describe 'successful build and can test service' do - let(:project) do - FactoryGirl.create(:ci_project, - email_add_pusher: true, - email_only_broken_builds: false, - email_recipients: "jeroen@example.com") - end - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) } - let(:build) { FactoryGirl.create(:ci_build, status: 'success', commit: commit, user: user) } - - before do - allow(mail).to receive_messages( - project: project - ) - build - end - - it do - expect(mail.can_test?).to eq(true) - end - end - - describe 'retried build should not receive email' do - let(:project) do - FactoryGirl.create(:ci_project, - email_add_pusher: true, - email_only_broken_builds: true, - email_recipients: "jeroen@example.com") - end - let(:gl_project) { FactoryGirl.create(:empty_project, gitlab_ci_project: project) } - let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) } - let(:build) { FactoryGirl.create(:ci_build, status: 'failed', commit: commit, user: user) } - - before do - allow(mail).to receive_messages( - project: project - ) - end - - it do - Ci::Build.retry(build) - perform_enqueued_jobs do - expect do - mail.execute(build) if mail.can_execute?(build) - end.to_not change{ ActionMailer::Base.deliveries.size } - end - end - end - end -end diff --git a/spec/models/ci/project_services/slack_message_spec.rb b/spec/models/ci/project_services/slack_message_spec.rb deleted file mode 100644 index 226032b4cda..00000000000 --- a/spec/models/ci/project_services/slack_message_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'spec_helper' - -describe Ci::SlackMessage, models: true do - subject { Ci::SlackMessage.new(commit) } - - let(:commit) { FactoryGirl.create(:ci_commit_with_two_jobs) } - - context 'when all matrix builds succeeded' do - let(:color) { 'good' } - - it 'returns a message with success' do - commit.create_builds('master', false, nil) - commit.builds.update_all(status: "success") - commit.reload - - expect(subject.color).to eq(color) - expect(subject.fallback).to include('Commit') - expect(subject.fallback).to include("\##{commit.id}") - expect(subject.fallback).to include('succeeded') - expect(subject.attachments.first[:fields]).to be_empty - end - end - - context 'when one of matrix builds failed' do - let(:color) { 'danger' } - - it 'returns a message with information about failed build' do - commit.create_builds('master', false, nil) - first_build = commit.builds.first - second_build = commit.builds.last - first_build.update(status: "success") - second_build.update(status: "failed") - - expect(subject.color).to eq(color) - expect(subject.fallback).to include('Commit') - expect(subject.fallback).to include("\##{commit.id}") - expect(subject.fallback).to include('failed') - expect(subject.attachments.first[:fields].size).to eq(1) - expect(subject.attachments.first[:fields].first[:title]).to eq(second_build.name) - expect(subject.attachments.first[:fields].first[:value]).to include("\##{second_build.id}") - end - end -end diff --git a/spec/models/ci/project_services/slack_service_spec.rb b/spec/models/ci/project_services/slack_service_spec.rb deleted file mode 100644 index e7d7d5d6f4c..00000000000 --- a/spec/models/ci/project_services/slack_service_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -# == Schema Information -# -# Table name: services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -require 'spec_helper' - -describe Ci::SlackService, models: true do - describe "Associations" do - it { is_expected.to belong_to :project } - end - - describe "Validations" do - context "active" do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of :webhook } - end - end - - describe "Execute" do - let(:slack) { Ci::SlackService.new } - let(:commit) { FactoryGirl.create :ci_commit } - let(:build) { FactoryGirl.create :ci_build, commit: commit, status: 'failed' } - let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } - let(:notify_only_broken_builds) { false } - - before do - allow(slack).to receive_messages( - project: commit.project, - project_id: commit.project_id, - webhook: webhook_url, - notify_only_broken_builds: notify_only_broken_builds - ) - - WebMock.stub_request(:post, webhook_url) - end - - it "should call Slack API" do - slack.execute(build) - Ci::SlackNotifierWorker.drain - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end -end diff --git a/spec/models/ci/project_spec.rb b/spec/models/ci/project_spec.rb index 346471aa9b5..e358aa02741 100644 --- a/spec/models/ci/project_spec.rb +++ b/spec/models/ci/project_spec.rb @@ -34,11 +34,9 @@ describe Ci::Project, models: true do it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } - it { is_expected.to have_many(:web_hooks) } it { is_expected.to have_many(:events) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } - it { is_expected.to have_many(:services) } it { is_expected.to validate_presence_of :timeout } it { is_expected.to validate_presence_of :gitlab_id } diff --git a/spec/models/ci/service_spec.rb b/spec/models/ci/service_spec.rb deleted file mode 100644 index 34e3af7f810..00000000000 --- a/spec/models/ci/service_spec.rb +++ /dev/null @@ -1,48 +0,0 @@ -# == Schema Information -# -# Table name: ci_services -# -# id :integer not null, primary key -# type :string(255) -# title :string(255) -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# active :boolean default(FALSE), not null -# properties :text -# - -require 'spec_helper' - -describe Ci::Service, models: true do - - describe "Associations" do - it { is_expected.to belong_to :project } - end - - describe "Mass assignment" do - end - - describe "Test Button" do - before do - @service = Ci::Service.new - end - - describe "Testable" do - let(:commit) { FactoryGirl.create :ci_commit } - let(:build) { FactoryGirl.create :ci_build, commit: commit } - - before do - allow(@service).to receive_messages( - project: commit.project - ) - build - @testable = @service.can_test? - end - - describe :can_test do - it { expect(@testable).to eq(true) } - end - end - end -end diff --git a/spec/models/ci/web_hook_spec.rb b/spec/models/ci/web_hook_spec.rb deleted file mode 100644 index 1a4edec9d4f..00000000000 --- a/spec/models/ci/web_hook_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -# == Schema Information -# -# Table name: ci_web_hooks -# -# id :integer not null, primary key -# url :string(255) not null -# project_id :integer not null -# created_at :datetime -# updated_at :datetime -# - -require 'spec_helper' - -describe Ci::WebHook, models: true do - describe "Associations" do - it { is_expected.to belong_to :project } - end - - describe "Validations" do - it { is_expected.to validate_presence_of(:url) } - - context "url format" do - it { is_expected.to allow_value("http://example.com").for(:url) } - it { is_expected.to allow_value("https://excample.com").for(:url) } - it { is_expected.to allow_value("http://test.com/api").for(:url) } - it { is_expected.to allow_value("http://test.com/api?key=abc").for(:url) } - it { is_expected.to allow_value("http://test.com/api?key=abc&type=def").for(:url) } - - it { is_expected.not_to allow_value("example.com").for(:url) } - it { is_expected.not_to allow_value("ftp://example.com").for(:url) } - it { is_expected.not_to allow_value("herp-and-derp").for(:url) } - end - end - - describe "execute" do - before(:each) do - @web_hook = FactoryGirl.create(:ci_web_hook) - @project = @web_hook.project - @data = { before: 'oldrev', after: 'newrev', ref: 'ref' } - - WebMock.stub_request(:post, @web_hook.url) - end - - it "POSTs to the web hook URL" do - @web_hook.execute(@data) - expect(WebMock).to have_requested(:post, @web_hook.url).once - end - - it "POSTs the data as JSON" do - json = @data.to_json - - @web_hook.execute(@data) - expect(WebMock).to have_requested(:post, @web_hook.url).with(body: json).once - end - - it "catches exceptions" do - expect(Ci::WebHook).to receive(:post).and_raise("Some HTTP Post error") - - expect{ @web_hook.execute(@data) }. - to raise_error(RuntimeError, 'Some HTTP Post error') - end - end -end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index c96ab548149..a5662b08bda 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -247,6 +247,55 @@ describe HipchatService, models: true do end end + context 'build events' do + let(:build) { create(:ci_build) } + let(:data) { Gitlab::BuildDataBuilder.build(build) } + + context 'for failed' do + before { build.drop } + + it "should call Hipchat API" do + hipchat.execute(data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "should create a build message" do + message = hipchat.send(:create_build_message, data) + + project_url = project.web_url + project_name = project.name_with_namespace.gsub(/\s/, '') + sha = data[:sha] + ref = data[:ref] + ref_type = data[:tag] ? 'tag' : 'branch' + duration = data[:commit][:duration] + + expect(message).to eq("#{project_name}: " \ + "Commit #{Commit.truncate_sha(sha)} " \ + "of #{ref} #{ref_type} " \ + "by #{data[:commit][:author_name]} failed in #{duration} second(s)") + end + end + + context 'for succeeded' do + before do + build.success + end + + it "should call Hipchat API" do + hipchat.notify_only_broken_builds = false + hipchat.execute(data) + expect(WebMock).to have_requested(:post, api_url).once + end + + it "should notify only broken" do + hipchat.notify_only_broken_builds = true + hipchat.execute(data) + expect(WebMock).to_not have_requested(:post, api_url).once + end + end + end + context "#message_options" do it "should be set to the defaults" do expect(hipchat.send(:message_options)).to eq({ notify: false, color: 'yellow' }) diff --git a/spec/models/project_services/slack_service/build_message_spec.rb b/spec/models/project_services/slack_service/build_message_spec.rb new file mode 100644 index 00000000000..d64c67da938 --- /dev/null +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe SlackService::BuildMessage do + subject { SlackService::BuildMessage.new(args) } + + let(:args) do + { + sha: '97de212e80737a608d939f648d959671fb0a0142', + ref: 'develop', + tag: false, + + project_name: 'project_name', + project_url: 'somewhere.com', + + commit: { + status: status, + author_name: 'hacker', + duration: 10, + }, + } + end + + context 'succeeded' do + let(:status) { 'success' } + let(:color) { 'good' } + + it 'returns a message with information about succeeded build' do + message = ': Commit of branch by hacker succeeded in 10 second(s)' + 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' } + + it 'returns a message with information about failed build' do + message = ': Commit of branch by hacker failed in 10 second(s)' + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end +end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 606b226ad77..142b637d291 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -1,11 +1,17 @@ require 'spec_helper' -describe API::API, 'ProjectHooks', api: true do +describe API::API, 'ProjectHooks', api: true do include ApiHelpers let(:user) { create(:user) } let(:user3) { create(:user) } let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let!(:hook) { 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, enable_ssl_verification: true) } + 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, + enable_ssl_verification: true) + end before do project.team << [user, :master] @@ -26,6 +32,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response.first['merge_requests_events']).to eq(true) 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['enable_ssl_verification']).to eq(true) end end @@ -83,6 +90,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['merge_requests_events']).to eq(false) 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['enable_ssl_verification']).to eq(true) end diff --git a/spec/requests/ci/api/projects_spec.rb b/spec/requests/ci/api/projects_spec.rb index 893fd168d3e..b1fbdac5970 100644 --- a/spec/requests/ci/api/projects_spec.rb +++ b/spec/requests/ci/api/projects_spec.rb @@ -58,57 +58,6 @@ describe Ci::API::API do end end - describe "POST /projects/:project_id/webhooks" do - let!(:project) { FactoryGirl.create(:ci_project) } - - context "Valid Webhook URL" do - let!(:webhook) { { web_hook: "http://example.com/sth/1/ala_ma_kota" } } - - before do - options.merge!(webhook) - end - - it "should create webhook for specified project" do - project.gl_project.team << [user, :master] - post ci_api("/projects/#{project.id}/webhooks"), options - expect(response.status).to eq(201) - expect(json_response["url"]).to eq(webhook[:web_hook]) - end - - it "fails to create webhook for non existsing project" do - post ci_api("/projects/non-existant-id/webhooks"), options - expect(response.status).to eq(404) - end - - it "non-manager is not authorized" do - post ci_api("/projects/#{project.id}/webhooks"), options - expect(response.status).to eq(401) - end - end - - context "Invalid Webhook URL" do - let!(:webhook) { { web_hook: "ala_ma_kota" } } - - before do - options.merge!(webhook) - end - - it "fails to create webhook for not valid url" do - project.gl_project.team << [user, :master] - post ci_api("/projects/#{project.id}/webhooks"), options - expect(response.status).to eq(400) - end - end - - context "Missed web_hook parameter" do - it "fails to create webhook for not provided url" do - project.gl_project.team << [user, :master] - post ci_api("/projects/#{project.id}/webhooks"), options - expect(response.status).to eq(400) - end - end - end - describe "GET /projects/:id" do let!(:project) { FactoryGirl.create(:ci_project) } @@ -158,28 +107,6 @@ describe Ci::API::API do end end - describe "DELETE /projects/:id" do - let!(:project) { FactoryGirl.create(:ci_project) } - - it "should delete a specific project" do - project.gl_project.team << [user, :master] - delete ci_api("/projects/#{project.id}"), options - expect(response.status).to eq(200) - expect { project.reload }. - to raise_error(ActiveRecord::RecordNotFound) - end - - it "non-manager is not authorized" do - delete ci_api("/projects/#{project.id}"), options - expect(response.status).to eq(401) - end - - it "is getting not found error" do - delete ci_api("/projects/not-existing_id"), options - expect(response.status).to eq(404) - end - end - describe "POST /projects/:id/runners/:id" do let(:project) { FactoryGirl.create(:ci_project) } let(:runner) { FactoryGirl.create(:ci_runner) } diff --git a/spec/services/ci/web_hook_service_spec.rb b/spec/services/ci/web_hook_service_spec.rb deleted file mode 100644 index e7d8ab30652..00000000000 --- a/spec/services/ci/web_hook_service_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -require 'spec_helper' - -describe Ci::WebHookService, services: true do - let(:project) { FactoryGirl.create :ci_project } - let(:gl_project) { FactoryGirl.create :empty_project, gitlab_ci_project: project } - let(:commit) { FactoryGirl.create :ci_commit, gl_project: gl_project } - let(:build) { FactoryGirl.create :ci_build, commit: commit } - let(:hook) { FactoryGirl.create :ci_web_hook, project: project } - - describe :execute do - it "should execute successfully" do - stub_request(:post, hook.url).to_return(status: 200) - expect(Ci::WebHookService.new.build_end(build)).to be_truthy - end - end - - context 'build_data' do - it "contains all needed fields" do - expect(build_data(build)).to include( - :build_id, - :project_id, - :ref, - :build_status, - :build_started_at, - :build_finished_at, - :before_sha, - :project_name, - :gitlab_url, - :build_name - ) - end - end - - def build_data(build) - Ci::WebHookService.new.send :build_data, build - end -end diff --git a/spec/workers/build_email_worker_spec.rb b/spec/workers/build_email_worker_spec.rb new file mode 100644 index 00000000000..7e379502f99 --- /dev/null +++ b/spec/workers/build_email_worker_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe BuildEmailWorker do + include RepoHelpers + + let(:build) { create(:ci_build) } + let(:user) { create(:user) } + let(:data) { Gitlab::BuildDataBuilder.build(build) } + + subject { BuildEmailWorker.new } + + before do + allow(build).to receive(:execute_hooks).and_return(false) + build.success + end + + describe "#perform" do + it "sends mail" do + subject.perform(build.id, user.email, data.stringify_keys) + + email = ActionMailer::Base.deliveries.last + expect(email.subject).to include('Build success for') + expect(email.to).to eq([user.email]) + end + + it "gracefully handles an input SMTP error" do + ActionMailer::Base.deliveries.clear + allow(Notify).to receive(:build_success_email).and_raise(Net::SMTPFatalError) + + subject.perform(build.id, user.email, data.stringify_keys) + + expect(ActionMailer::Base.deliveries.count).to eq(0) + end + end +end -- cgit v1.2.1 From d5c91bb9a601a1a344d94763654f0b0996857497 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 9 Dec 2015 16:31:42 +0100 Subject: Migrate CI WebHooks and Emails to new tables --- app/models/project_services/builds_email_service.rb | 11 +++++++---- app/workers/build_email_worker.rb | 2 +- db/migrate/20151209144329_migrate_ci_web_hooks.rb | 12 ++++++++++++ db/migrate/20151209145909_migrate_ci_emails.rb | 16 ++++++++++++++++ db/schema.rb | 2 +- lib/gitlab/database.rb | 18 ++++++++++++++++++ 6 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20151209144329_migrate_ci_web_hooks.rb create mode 100644 db/migrate/20151209145909_migrate_ci_emails.rb diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index 9c9b5a4d08c..4514f55cf56 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -54,7 +54,7 @@ class BuildsEmailService < Service def fields [ - { type: 'textarea', name: 'recipients', placeholder: 'Emails separated by whitespace' }, + { type: 'textarea', name: 'recipients', placeholder: 'Emails separated by comma' }, { type: 'checkbox', name: 'add_pusher', label: 'Add pusher to recipients list' }, { type: 'checkbox', name: 'notify_only_broken_builds' }, ] @@ -72,10 +72,13 @@ class BuildsEmailService < Service end def all_recipients(data) + all_recipients = [] + all_recipients <<= recipients.split(',') + if add_pusher? && data[:user][:email] - recipients + " #{data[:user][:email]}" - else - recipients + all_recipients << "#{data[:user][:email]}" end + + all_recipients end end diff --git a/app/workers/build_email_worker.rb b/app/workers/build_email_worker.rb index c5c8055bea7..1c7a04a66a8 100644 --- a/app/workers/build_email_worker.rb +++ b/app/workers/build_email_worker.rb @@ -2,7 +2,7 @@ class BuildEmailWorker include Sidekiq::Worker def perform(build_id, recipients, push_data) - recipients.split(' ').each do |recipient| + recipients.each do |recipient| begin case push_data['build_status'] when 'success' diff --git a/db/migrate/20151209144329_migrate_ci_web_hooks.rb b/db/migrate/20151209144329_migrate_ci_web_hooks.rb new file mode 100644 index 00000000000..0293be3f6ce --- /dev/null +++ b/db/migrate/20151209144329_migrate_ci_web_hooks.rb @@ -0,0 +1,12 @@ +class MigrateCiWebHooks < ActiveRecord::Migration + include Gitlab::Database + + def up + execute( + 'INSERT INTO web_hooks (url, project_id, type, created_at, updated_at, push_events, build_events) ' \ + "SELECT ci_web_hooks.url, projects.id, 'ProjectHook', ci_web_hooks.created_at, ci_web_hooks.updated_at, #{false_value}, #{true_value} FROM ci_web_hooks " \ + 'JOIN ci_projects ON ci_web_hooks.project_id = ci_projects.id ' \ + 'JOIN projects ON ci_projects.gitlab_id = projects.id' + ) + end +end diff --git a/db/migrate/20151209145909_migrate_ci_emails.rb b/db/migrate/20151209145909_migrate_ci_emails.rb new file mode 100644 index 00000000000..ebf0e7d264f --- /dev/null +++ b/db/migrate/20151209145909_migrate_ci_emails.rb @@ -0,0 +1,16 @@ +class MigrateCiEmails < ActiveRecord::Migration + include Gitlab::Database + + def up + execute( + 'INSERT INTO services (project_id, type, created_at, updated_at, active, push_events, issues_events, merge_requests_events, tag_push_events, note_events, build_events, properties) ' \ + "SELECT projects.id, 'BuildsEmailService', ci_services.created_at, ci_services.updated_at, #{true_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{true_value}, " \ + "CONCAT('{\"notify_only_broken_builds\":\"', ci_projects.email_only_broken_builds, " \ + "'\",\"add_pusher\":\"', ci_projects.email_add_pusher, '\",\"recipients\":\"', ci_projects.email_recipients, '\"}') " \ + 'FROM ci_services ' \ + 'JOIN ci_projects ON ci_services.project_id = ci_projects.id ' \ + 'JOIN projects ON ci_projects.gitlab_id = projects.id ' \ + "WHERE ci_services.type = 'Ci::MailService' AND ci_services.active" + ) + end +end diff --git a/db/schema.rb b/db/schema.rb index 58595f8ae59..50b6650ac40 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20151203162134) do +ActiveRecord::Schema.define(version: 20151209145909) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 71f37f1fef8..de77a6fbff1 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -7,5 +7,23 @@ module Gitlab def self.postgresql? ActiveRecord::Base.connection.adapter_name.downcase == 'postgresql' end + + def true_value + case ActiveRecord::Base.connection.adapter_name.downcase + when 'postgresql' + "'t'" + else + 1 + end + end + + def false_value + case ActiveRecord::Base.connection.adapter_name.downcase + when 'postgresql' + "'f'" + else + 0 + end + end end end -- cgit v1.2.1 From 80f8074d01a310141984dad9dfe01a27b533e78a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 10 Dec 2015 14:08:09 +0100 Subject: Migrate SlackService and HipChat service --- .../project_services/builds_email_service.rb | 2 ++ app/models/project_services/hipchat_service.rb | 2 ++ app/models/project_services/slack_service.rb | 2 ++ db/migrate/20151209145909_migrate_ci_emails.rb | 6 +++++ .../20151210125232_migrate_ci_slack_service.rb | 29 +++++++++++++++++++++ .../20151210125927_migrate_ci_hip_chat_service.rb | 30 ++++++++++++++++++++++ db/schema.rb | 2 +- 7 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20151210125232_migrate_ci_slack_service.rb create mode 100644 db/migrate/20151210125927_migrate_ci_hip_chat_service.rb diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index 4514f55cf56..e01648f0596 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -24,6 +24,8 @@ class BuildsEmailService < Service boolean_accessor :notify_only_broken_builds validates :recipients, presence: true, if: :activated? + default_value_for :notify_only_broken_builds, true + def title 'Builds emails' end diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 6f96907ec18..4045d16fa22 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -25,6 +25,8 @@ class HipchatService < Service boolean_accessor :notify_only_broken_builds validates :token, presence: true, if: :activated? + default_value_for :notify_only_broken_builds, true + def title 'HipChat' end diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 35819491575..553d4b025e1 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -23,6 +23,8 @@ class SlackService < Service boolean_accessor :notify_only_broken_builds validates :webhook, presence: true, if: :activated? + default_value_for :notify_only_broken_builds, true + def title 'Slack' end diff --git a/db/migrate/20151209145909_migrate_ci_emails.rb b/db/migrate/20151209145909_migrate_ci_emails.rb index ebf0e7d264f..964dde841ad 100644 --- a/db/migrate/20151209145909_migrate_ci_emails.rb +++ b/db/migrate/20151209145909_migrate_ci_emails.rb @@ -2,6 +2,9 @@ class MigrateCiEmails < ActiveRecord::Migration include Gitlab::Database def up + # This inserts a new service: BuildsEmailService + # It also "manually" constructs the properties (JSON-encoded) + # Migrating all ci_projects e-mail related columns execute( 'INSERT INTO services (project_id, type, created_at, updated_at, active, push_events, issues_events, merge_requests_events, tag_push_events, note_events, build_events, properties) ' \ "SELECT projects.id, 'BuildsEmailService', ci_services.created_at, ci_services.updated_at, #{true_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{true_value}, " \ @@ -13,4 +16,7 @@ class MigrateCiEmails < ActiveRecord::Migration "WHERE ci_services.type = 'Ci::MailService' AND ci_services.active" ) end + + def down + end end diff --git a/db/migrate/20151210125232_migrate_ci_slack_service.rb b/db/migrate/20151210125232_migrate_ci_slack_service.rb new file mode 100644 index 00000000000..4a5dfe866a5 --- /dev/null +++ b/db/migrate/20151210125232_migrate_ci_slack_service.rb @@ -0,0 +1,29 @@ +class MigrateCiSlackService < ActiveRecord::Migration + include Gitlab::Database + + def up + properties_query = 'SELECT properties FROM ci_services ' \ + 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ + 'WHERE ci_projects.gitlab_id=services.project_id' + + active_query = 'SELECT 1 FROM ci_services ' \ + 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ + "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::SlackService' AND ci_services.active" + + # We update the service since services are always generated for project, even if they are inactive + # Activate service and migrate properties if currently the service is not active + execute( + "UPDATE services SET properties=(#{properties_query}), build_events=#{true_value}, active=#{true_value} " \ + "WHERE NOT services.active AND services.type='SlackService' AND (#{active_query}) IS NOT NULL" + ) + + # Tick only build_events if the service is already active + execute( + "UPDATE services SET build_events=#{true_value} " \ + "WHERE services.active AND services.type='SlackService' AND (#{active_query}) IS NOT NULL" + ) + end + + def down + end +end diff --git a/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb b/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb new file mode 100644 index 00000000000..2fdaf5fb917 --- /dev/null +++ b/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb @@ -0,0 +1,30 @@ +class MigrateCiHipChatService < ActiveRecord::Migration + include Gitlab::Database + + def up + # From properties strip `hipchat_` key + properties_query = "SELECT REPLACE(properties, '\"hipchat_', '\"') FROM ci_services " \ + 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ + 'WHERE ci_projects.gitlab_id=services.project_id' + + active_query = 'SELECT 1 FROM ci_services ' \ + 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ + "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::HipchatService' AND ci_services.active" + + # We update the service since services are always generated for project, even if they are inactive + # Activate service and migrate properties if currently the service is not active + execute( + "UPDATE services SET properties=(#{properties_query}), build_events=#{true_value}, active=#{true_value} " \ + "WHERE NOT services.active AND services.type='HipchatService' AND (#{active_query}) IS NOT NULL" + ) + + # Tick only build_events if the service is already active + execute( + "UPDATE services SET build_events=#{true_value} " \ + "WHERE services.active AND services.type='HipchatService' AND (#{active_query}) IS NOT NULL" + ) + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index 50b6650ac40..6ccd4404219 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20151209145909) do +ActiveRecord::Schema.define(version: 20151210125927) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" -- cgit v1.2.1 From c4fa894de22a6ba20f3078f490b708c81b6d6464 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 10 Dec 2015 16:16:34 +0100 Subject: Fix specs --- app/models/service.rb | 1 + features/project/service.feature | 8 ++++---- features/steps/project/services.rb | 8 ++++---- spec/models/project_services/slack_service/build_message_spec.rb | 2 +- spec/workers/build_email_worker_spec.rb | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/models/service.rb b/app/models/service.rb index 195c4690e8f..0ccb8b410d1 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -31,6 +31,7 @@ class Service < ActiveRecord::Base default_value_for :tag_push_events, true default_value_for :note_events, true default_value_for :build_events, true + default_value_for :properties, {} after_initialize :initialize_properties diff --git a/features/project/service.feature b/features/project/service.feature index 13edc6cb2b9..5014b52b9f6 100644 --- a/features/project/service.feature +++ b/features/project/service.feature @@ -55,11 +55,11 @@ Feature: Project Services And I fill Pushover settings Then I should see Pushover service settings saved - Scenario: Activate email service + Scenario: Activate email on push service When I visit project "Shop" services page - And I click email service link - And I fill email settings - Then I should see email service settings saved + And I click email on push service link + And I fill email on push settings + Then I should see email on push service settings saved Scenario: Activate Irker (IRC Gateway) service When I visit project "Shop" services page diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index 2d564dac498..1c700df0c63 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -118,16 +118,16 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps expect(find_field('Restrict to branch').value).to eq 'master' end - step 'I click email service link' do - click_link 'Emails' + step 'I click email on push service link' do + click_link 'Emails on push' end - step 'I fill email settings' do + step 'I fill email on push settings' do fill_in 'Recipients', with: 'qa@company.name' click_button 'Save' end - step 'I should see email service settings saved' do + step 'I should see email on push service settings saved' do expect(find_field('Recipients').value).to eq 'qa@company.name' end 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 d64c67da938..621c83c0cda 100644 --- a/spec/models/project_services/slack_service/build_message_spec.rb +++ b/spec/models/project_services/slack_service/build_message_spec.rb @@ -25,7 +25,7 @@ describe SlackService::BuildMessage do let(:color) { 'good' } it 'returns a message with information about succeeded build' do - message = ': Commit of branch by hacker succeeded in 10 second(s)' + message = ': Commit of branch by hacker passed in 10 second(s)' 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/workers/build_email_worker_spec.rb b/spec/workers/build_email_worker_spec.rb index 7e379502f99..98deae0a588 100644 --- a/spec/workers/build_email_worker_spec.rb +++ b/spec/workers/build_email_worker_spec.rb @@ -16,7 +16,7 @@ describe BuildEmailWorker do describe "#perform" do it "sends mail" do - subject.perform(build.id, user.email, data.stringify_keys) + subject.perform(build.id, [user.email], data.stringify_keys) email = ActionMailer::Base.deliveries.last expect(email.subject).to include('Build success for') @@ -27,7 +27,7 @@ describe BuildEmailWorker do ActionMailer::Base.deliveries.clear allow(Notify).to receive(:build_success_email).and_raise(Net::SMTPFatalError) - subject.perform(build.id, user.email, data.stringify_keys) + subject.perform(build.id, [user.email], data.stringify_keys) expect(ActionMailer::Base.deliveries.count).to eq(0) end -- cgit v1.2.1 From dc4e2744ba13cd7d75147787550a1272b9d34a95 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 10 Dec 2015 17:21:06 +0100 Subject: Fix issue tracker service --- app/models/project_services/issue_tracker_service.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 936e574cccd..46ba9808175 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -56,16 +56,12 @@ class IssueTrackerService < Service end def initialize_properties - if properties.nil? + if new_record? if enabled_in_gitlab_config - self.properties = { - title: issues_tracker['title'], - project_url: add_issues_tracker_id(issues_tracker['project_url']), - issues_url: add_issues_tracker_id(issues_tracker['issues_url']), - new_issue_url: add_issues_tracker_id(issues_tracker['new_issue_url']) - } - else - self.properties = {} + self.title = issues_tracker['title'] + self.project_url = add_issues_tracker_id(issues_tracker['project_url']) + self.issues_url = add_issues_tracker_id(issues_tracker['issues_url']) + self.new_issue_url = add_issues_tracker_id(issues_tracker['new_issue_url']) end end end @@ -98,8 +94,8 @@ class IssueTrackerService < Service def enabled_in_gitlab_config Gitlab.config.issues_tracker && - Gitlab.config.issues_tracker.values.any? && - issues_tracker + Gitlab.config.issues_tracker.values.any? && + issues_tracker end def issues_tracker -- cgit v1.2.1 From e365750199802a7c0b45dc88c99e4bf7eb6f111c Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 11 Dec 2015 13:24:38 +0100 Subject: Enhance migrate CI emails --- db/migrate/20151209145909_migrate_ci_emails.rb | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/db/migrate/20151209145909_migrate_ci_emails.rb b/db/migrate/20151209145909_migrate_ci_emails.rb index 964dde841ad..5ee11893582 100644 --- a/db/migrate/20151209145909_migrate_ci_emails.rb +++ b/db/migrate/20151209145909_migrate_ci_emails.rb @@ -3,13 +3,16 @@ class MigrateCiEmails < ActiveRecord::Migration def up # This inserts a new service: BuildsEmailService - # It also "manually" constructs the properties (JSON-encoded) + # It "manually" constructs the properties (JSON-encoded) # Migrating all ci_projects e-mail related columns execute( 'INSERT INTO services (project_id, type, created_at, updated_at, active, push_events, issues_events, merge_requests_events, tag_push_events, note_events, build_events, properties) ' \ - "SELECT projects.id, 'BuildsEmailService', ci_services.created_at, ci_services.updated_at, #{true_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{true_value}, " \ - "CONCAT('{\"notify_only_broken_builds\":\"', ci_projects.email_only_broken_builds, " \ - "'\",\"add_pusher\":\"', ci_projects.email_add_pusher, '\",\"recipients\":\"', ci_projects.email_recipients, '\"}') " \ + "SELECT projects.id, 'BuildsEmailService', ci_services.created_at, ci_services.updated_at, " \ + "#{true_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{true_value}, " \ + "CONCAT('{\"notify_only_broken_builds\":\"', #{convert_bool('ci_projects.email_only_broken_builds')}, " \ + "'\",\"add_pusher\":\"', #{convert_bool('ci_projects.email_add_pusher')}, " \ + "'\",\"recipients\":\"', #{escape_text('ci_projects.email_recipients')}, " \ + "'\"}') " \ 'FROM ci_services ' \ 'JOIN ci_projects ON ci_services.project_id = ci_projects.id ' \ 'JOIN projects ON ci_projects.gitlab_id = projects.id ' \ @@ -19,4 +22,20 @@ class MigrateCiEmails < ActiveRecord::Migration def down end + + # This function escapes double-quotes and slash + def escape_text(name) + "REPLACE(REPLACE(#{name}, '\\', '\\\\'), '\"', '\\\"')" + end + + # This function returns 0 or 1 for column + def convert_bool(name) + if self.postgresql? + # PostgreSQL uses BOOLEAN type + "CASE WHEN #{name} IS TRUE THEN '1' ELSE '0' END;" + else + # MySQL uses TINYINT + "#{name}" + end + end end -- cgit v1.2.1 From 71e6a93db979165073161f0dc13d3bfe7a461e4a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 11 Dec 2015 13:28:40 +0100 Subject: Change default values --- app/models/project_services/builds_email_service.rb | 7 ++++++- app/models/project_services/hipchat_service.rb | 7 ++++++- app/models/project_services/issue_tracker_service.rb | 18 +++++++++++------- app/models/project_services/slack_service.rb | 7 ++++++- app/models/service.rb | 1 - 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index e01648f0596..b8bd48aca69 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -24,7 +24,12 @@ class BuildsEmailService < Service boolean_accessor :notify_only_broken_builds validates :recipients, presence: true, if: :activated? - default_value_for :notify_only_broken_builds, true + def initialize_properties + if properties.nil? + self.properties = {} + self.notify_only_broken_builds = true + end + end def title 'Builds emails' diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 4045d16fa22..1e1686a11c6 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -25,7 +25,12 @@ class HipchatService < Service boolean_accessor :notify_only_broken_builds validates :token, presence: true, if: :activated? - default_value_for :notify_only_broken_builds, true + def initialize_properties + if properties.nil? + self.properties = {} + self.notify_only_broken_builds = true + end + end def title 'HipChat' diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index 46ba9808175..936e574cccd 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -56,12 +56,16 @@ class IssueTrackerService < Service end def initialize_properties - if new_record? + if properties.nil? if enabled_in_gitlab_config - self.title = issues_tracker['title'] - self.project_url = add_issues_tracker_id(issues_tracker['project_url']) - self.issues_url = add_issues_tracker_id(issues_tracker['issues_url']) - self.new_issue_url = add_issues_tracker_id(issues_tracker['new_issue_url']) + self.properties = { + title: issues_tracker['title'], + project_url: add_issues_tracker_id(issues_tracker['project_url']), + issues_url: add_issues_tracker_id(issues_tracker['issues_url']), + new_issue_url: add_issues_tracker_id(issues_tracker['new_issue_url']) + } + else + self.properties = {} end end end @@ -94,8 +98,8 @@ class IssueTrackerService < Service def enabled_in_gitlab_config Gitlab.config.issues_tracker && - Gitlab.config.issues_tracker.values.any? && - issues_tracker + Gitlab.config.issues_tracker.values.any? && + issues_tracker end def issues_tracker diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 553d4b025e1..375b4534d07 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -23,7 +23,12 @@ class SlackService < Service boolean_accessor :notify_only_broken_builds validates :webhook, presence: true, if: :activated? - default_value_for :notify_only_broken_builds, true + def initialize_properties + if properties.nil? + self.properties = {} + self.notify_only_broken_builds = true + end + end def title 'Slack' diff --git a/app/models/service.rb b/app/models/service.rb index 0ccb8b410d1..195c4690e8f 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -31,7 +31,6 @@ class Service < ActiveRecord::Base default_value_for :tag_push_events, true default_value_for :note_events, true default_value_for :build_events, true - default_value_for :properties, {} after_initialize :initialize_properties -- cgit v1.2.1 From 8b4cdc50fca816b4f56f8579e17c4dba836ec797 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 11 Dec 2015 18:01:57 +0100 Subject: Fix indentation and BuildsEmailService --- app/models/project_services/builds_email_service.rb | 3 +-- app/models/service.rb | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index b8bd48aca69..8247c79fc33 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -79,8 +79,7 @@ class BuildsEmailService < Service end def all_recipients(data) - all_recipients = [] - all_recipients <<= recipients.split(',') + all_recipients = recipients.split(',') if add_pusher? && data[:user][:email] all_recipients << "#{data[:user][:email]}" diff --git a/app/models/service.rb b/app/models/service.rb index 195c4690e8f..4159e367d8c 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -143,10 +143,10 @@ class Service < ActiveRecord::Base args.each do |arg| class_eval %{ - def #{arg}? - ActiveRecord::ConnectionAdapters::Column::TRUE_VALUES.include?(#{arg}) - end - } + def #{arg}? + ActiveRecord::ConnectionAdapters::Column::TRUE_VALUES.include?(#{arg}) + end + } end end -- cgit v1.2.1 From a0c2a7b0cbb4567a1f09c4cbc400bf75df47a072 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 14 Dec 2015 11:04:47 +0100 Subject: Fix migrations [ci skip] --- db/migrate/20151209144329_migrate_ci_web_hooks.rb | 5 +++-- db/migrate/20151209145909_migrate_ci_emails.rb | 4 ++-- db/migrate/20151210125232_migrate_ci_slack_service.rb | 10 +++++++--- db/migrate/20151210125927_migrate_ci_hip_chat_service.rb | 10 +++++++--- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/db/migrate/20151209144329_migrate_ci_web_hooks.rb b/db/migrate/20151209144329_migrate_ci_web_hooks.rb index 0293be3f6ce..825ba1973ff 100644 --- a/db/migrate/20151209144329_migrate_ci_web_hooks.rb +++ b/db/migrate/20151209144329_migrate_ci_web_hooks.rb @@ -3,8 +3,9 @@ class MigrateCiWebHooks < ActiveRecord::Migration def up execute( - 'INSERT INTO web_hooks (url, project_id, type, created_at, updated_at, push_events, build_events) ' \ - "SELECT ci_web_hooks.url, projects.id, 'ProjectHook', ci_web_hooks.created_at, ci_web_hooks.updated_at, #{false_value}, #{true_value} FROM ci_web_hooks " \ + 'INSERT INTO web_hooks (url, project_id, type, created_at, updated_at, push_events, issues_events, merge_requests_events, tag_push_events, note_events, build_events) ' \ + "SELECT ci_web_hooks.url, projects.id, 'ProjectHook', ci_web_hooks.created_at, ci_web_hooks.updated_at, " \ + "#{false_value}, #{false_value}, #{false_value}, #{false_value}, #{false_value}, #{true_value} FROM ci_web_hooks " \ 'JOIN ci_projects ON ci_web_hooks.project_id = ci_projects.id ' \ 'JOIN projects ON ci_projects.gitlab_id = projects.id' ) diff --git a/db/migrate/20151209145909_migrate_ci_emails.rb b/db/migrate/20151209145909_migrate_ci_emails.rb index 5ee11893582..202fac8e3fc 100644 --- a/db/migrate/20151209145909_migrate_ci_emails.rb +++ b/db/migrate/20151209145909_migrate_ci_emails.rb @@ -30,9 +30,9 @@ class MigrateCiEmails < ActiveRecord::Migration # This function returns 0 or 1 for column def convert_bool(name) - if self.postgresql? + if Gitlab::Database.postgresql? # PostgreSQL uses BOOLEAN type - "CASE WHEN #{name} IS TRUE THEN '1' ELSE '0' END;" + "CASE WHEN #{name} IS TRUE THEN '1' ELSE '0' END" else # MySQL uses TINYINT "#{name}" diff --git a/db/migrate/20151210125232_migrate_ci_slack_service.rb b/db/migrate/20151210125232_migrate_ci_slack_service.rb index 4a5dfe866a5..f14efa3e95d 100644 --- a/db/migrate/20151210125232_migrate_ci_slack_service.rb +++ b/db/migrate/20151210125232_migrate_ci_slack_service.rb @@ -4,16 +4,20 @@ class MigrateCiSlackService < ActiveRecord::Migration def up properties_query = 'SELECT properties FROM ci_services ' \ 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ - 'WHERE ci_projects.gitlab_id=services.project_id' + "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::SlackService' AND ci_services.active " \ + 'LIMIT 1' active_query = 'SELECT 1 FROM ci_services ' \ 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ - "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::SlackService' AND ci_services.active" + "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::SlackService' AND ci_services.active " \ + 'LIMIT 1' # We update the service since services are always generated for project, even if they are inactive # Activate service and migrate properties if currently the service is not active execute( - "UPDATE services SET properties=(#{properties_query}), build_events=#{true_value}, active=#{true_value} " \ + "UPDATE services SET properties=(#{properties_query}), active=#{true_value}, " \ + "push_events=#{false_value}, issues_events=#{false_value}, merge_requests_events=#{false_value}, " \ + "tag_push_events=#{false_value}, note_events=#{false_value}, build_events=#{true_value} " \ "WHERE NOT services.active AND services.type='SlackService' AND (#{active_query}) IS NOT NULL" ) diff --git a/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb b/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb index 2fdaf5fb917..b9e04323576 100644 --- a/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb +++ b/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb @@ -5,16 +5,20 @@ class MigrateCiHipChatService < ActiveRecord::Migration # From properties strip `hipchat_` key properties_query = "SELECT REPLACE(properties, '\"hipchat_', '\"') FROM ci_services " \ 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ - 'WHERE ci_projects.gitlab_id=services.project_id' + "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::HipChatService' AND ci_services.active " \ + 'LIMIT 1' active_query = 'SELECT 1 FROM ci_services ' \ 'JOIN ci_projects ON ci_services.project_id=ci_projects.id ' \ - "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::HipchatService' AND ci_services.active" + "WHERE ci_projects.gitlab_id=services.project_id AND ci_services.type='Ci::HipChatService' AND ci_services.active " \ + 'LIMIT 1' # We update the service since services are always generated for project, even if they are inactive # Activate service and migrate properties if currently the service is not active execute( - "UPDATE services SET properties=(#{properties_query}), build_events=#{true_value}, active=#{true_value} " \ + "UPDATE services SET properties=(#{properties_query}), active=#{true_value}, " \ + "push_events=#{false_value}, issues_events=#{false_value}, merge_requests_events=#{false_value}, " \ + "tag_push_events=#{false_value}, note_events=#{false_value}, build_events=#{true_value} " \ "WHERE NOT services.active AND services.type='HipchatService' AND (#{active_query}) IS NOT NULL" ) -- cgit v1.2.1