summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/services_controller.rb6
-rw-r--r--app/models/build.rb6
-rw-r--r--app/models/project.rb7
-rw-r--r--app/models/project_services/mail_service.rb86
-rw-r--r--app/models/project_services/slack_service.rb8
-rw-r--r--app/models/service.rb12
-rw-r--r--app/services/notification_service.rb25
-rw-r--r--app/views/projects/_form.html.haml22
-rw-r--r--app/views/services/_form.html.haml3
-rw-r--r--lib/api/projects.rb1
-rw-r--r--spec/factories/commits.rb24
-rw-r--r--spec/models/mail_service_spec.rb192
-rw-r--r--spec/services/notification_service_spec.rb56
13 files changed, 324 insertions, 124 deletions
diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb
index 655d959..b24f512 100644
--- a/app/controllers/services_controller.rb
+++ b/app/controllers/services_controller.rb
@@ -49,10 +49,8 @@ class ServicesController < ApplicationController
def service_params
params.require(:service).permit(
- :title, :token, :type, :active, :api_key, :subdomain,
- :room, :recipients, :project_url, :webhook,
- :user_key, :device, :priority, :sound,
- :notify_only_broken_builds
+ :type, :active, :webhook, :notify_only_broken_builds,
+ :email_recipients, :email_only_broken_builds, :email_add_committer
)
end
end
diff --git a/app/models/build.rb b/app/models/build.rb
index 2616752..dc53255 100644
--- a/app/models/build.rb
+++ b/app/models/build.rb
@@ -119,12 +119,6 @@ class Build < ActiveRecord::Base
project.execute_services(build)
- if project.email_notification?
- if build.status.to_sym == :failed || !project.email_only_broken_builds
- NotificationService.new.build_ended(build)
- end
- end
-
if project.coverage_enabled?
build.update_coverage
end
diff --git a/app/models/project.rb b/app/models/project.rb
index ed10044..a31153d 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -42,6 +42,7 @@ class Project < ActiveRecord::Base
# Project services
has_many :services, dependent: :destroy
has_one :slack_service, dependent: :destroy
+ has_one :mail_service, dependent: :destroy
accepts_nested_attributes_for :jobs, allow_destroy: true
@@ -84,7 +85,9 @@ ls -la
email_only_broken_builds: GitlabCi.config.gitlab_ci.all_broken_builds,
}
- Project.new(params)
+ project = Project.new(params)
+ project.build_missing_services
+ project
end
def from_gitlab(user, page, per_page, scope = :owned)
@@ -192,7 +195,7 @@ ls -la
end
def available_services_names
- %w(slack)
+ %w(slack mail)
end
def build_missing_services
diff --git a/app/models/project_services/mail_service.rb b/app/models/project_services/mail_service.rb
new file mode 100644
index 0000000..6eaf1a8
--- /dev/null
+++ b/app/models/project_services/mail_service.rb
@@ -0,0 +1,86 @@
+# == 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
+#
+
+class MailService < Service
+ delegate :email_recipients, :email_recipients=,
+ :email_add_committer, :email_add_committer=,
+ :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_committer', label: 'Add committer to recipients list'},
+ {type: 'checkbox', name: 'email_only_broken_builds', label: 'Notify only broken builds'}
+ ]
+ end
+
+ def can_test?
+ # e-mail notification is useful only for builds either successful or failed
+ project.builds.order(id: :desc).any? do |build|
+ return false unless build.commit.project_recipients.any?
+
+ case build.status.to_sym
+ when :failed
+ true
+ when :success
+ !email_only_broken_builds
+ else
+ false
+ end
+ end
+ end
+
+ def execute(build)
+ # it doesn't make sense to send emails for retried builds
+ commit = build.commit
+ return unless commit
+ return unless commit.builds_without_retry.include?(build)
+
+ commit.project_recipients.each do |recipient|
+ case build.status.to_sym
+ when :success
+ return if email_only_broken_builds
+ mailer.build_success_email(build.id, recipient)
+ when :failed
+ mailer.build_fail_email(build.id, recipient)
+ end
+ end
+ end
+
+ private
+
+ def update_project
+ project.save!
+ end
+
+ def mailer
+ Notify.delay
+ end
+end
diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb
index 44f0cd4..29174ec 100644
--- a/app/models/project_services/slack_service.rb
+++ b/app/models/project_services/slack_service.rb
@@ -14,13 +14,11 @@
class SlackService < Service
prop_accessor :webhook
- prop_accessor :notify_only_broken_builds
+ boolean_accessor :notify_only_broken_builds
validates :webhook, presence: true, if: :activated?
default_value_for :notify_only_broken_builds, true
- NOTIFY_ONLY_BROKEN_BUILDS_ENABLED = '1'
-
def title
'Slack'
end
@@ -44,10 +42,6 @@ class SlackService < Service
]
end
- def notify_only_broken_builds?
- notify_only_broken_builds == NOTIFY_ONLY_BROKEN_BUILDS_ENABLED
- end
-
def can_test?
# slack notification is useful only for builds either successful or failed
project.commits.order(id: :desc).any? do |commit|
diff --git a/app/models/service.rb b/app/models/service.rb
index 20a2c15..9586060 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -82,4 +82,16 @@ class Service < ActiveRecord::Base
}
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
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
deleted file mode 100644
index db52c02..0000000
--- a/app/services/notification_service.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-# NotificationService class
-#
-# Used for notifying users with emails about different events
-#
-# Ex.
-# NotificationService.new.build_ended(build)
-#
-class NotificationService
- def build_ended(build)
- build.commit.project_recipients.each do |recipient|
- case build.status.to_sym
- when :success
- mailer.build_success_email(build.id, recipient)
- when :failed
- mailer.build_fail_email(build.id, recipient)
- end
- end
- end
-
- protected
-
- def mailer
- Notify.delay
- end
-end
diff --git a/app/views/projects/_form.html.haml b/app/views/projects/_form.html.haml
index ea9328a..63d34e1 100644
--- a/app/views/projects/_form.html.haml
+++ b/app/views/projects/_form.html.haml
@@ -78,28 +78,6 @@
%fieldset
- %legend Email notification
- .form-group
- = f.label :email_recipients, "Recipients", class: 'control-label'
- .col-sm-10
- = f.text_field :email_recipients, class: 'form-control'
- .light Whitespace-separated list of recipient addresses
- .form-group
- .col-sm-2
- .col-sm-10
- .checkbox
- = f.label :email_add_committer do
- = f.check_box :email_add_committer
- %span Add committer to recipients list
- .form-group
- .col-sm-2
- .col-sm-10
- .checkbox
- = f.label :email_only_broken_builds do
- = f.check_box :email_only_broken_builds
- %span Send notification only for broken builds
-
- %fieldset
%legend Advanced settings
.form-group
= f.label :name, class: 'control-label'
diff --git a/app/views/services/_form.html.haml b/app/views/services/_form.html.haml
index 5e4bb76..3eacf90 100644
--- a/app/views/services/_form.html.haml
+++ b/app/views/services/_form.html.haml
@@ -34,6 +34,7 @@
- placeholder = field[:placeholder]
- choices = field[:choices]
- default_choice = field[:default_choice]
+ - help = field[:help]
.form-group
= f.label label, class: "control-label"
@@ -46,6 +47,8 @@
= 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'
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 632aa51..a3cc9f3 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -94,6 +94,7 @@ module API
}
project = Project.new(filtered_params)
+ project.build_missing_services
project.build_default_job
if project.save
diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb
index f767cac..156fabe 100644
--- a/spec/factories/commits.rb
+++ b/spec/factories/commits.rb
@@ -22,8 +22,28 @@ FactoryGirl.define do
push_data do
{
ref: 'refs/heads/master',
- before_sha: '76de212e80737a608d939f648d959671fb0a0142',
- after_sha: '97de212e80737a608d939f648d959671fb0a0142'
+ before: '76de212e80737a608d939f648d959671fb0a0142',
+ after: '97de212e80737a608d939f648d959671fb0a0142',
+ user_name: 'Git User',
+ repository: {
+ name: 'test-data',
+ url: 'ssh://git@gitlab.com/test/test-data.git',
+ description: '',
+ homepage: 'http://gitlab.com/test/test-data'
+ },
+ commits: [
+ {
+ id: '97de212e80737a608d939f648d959671fb0a0142',
+ message: 'Test commit message',
+ timestamp: '2014-09-23T13:12:25+02:00',
+ url: 'https://gitlab.com/test/test-data/commit/97de212e80737a608d939f648d959671fb0a0142',
+ author: {
+ name: 'Git User',
+ email: 'git@user.com'
+ }
+ }
+ ],
+ total_commits_count: 1
}
end
end
diff --git a/spec/models/mail_service_spec.rb b/spec/models/mail_service_spec.rb
new file mode 100644
index 0000000..7a43978
--- /dev/null
+++ b/spec/models/mail_service_spec.rb
@@ -0,0 +1,192 @@
+require 'spec_helper'
+
+describe MailService do
+ describe "Associations" do
+ it { should 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) { MailService.new }
+
+ describe 'failed build' do
+ let(:project) { FactoryGirl.create(:project, email_add_committer: true) }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :failed, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ end
+
+ it do
+ should_email(commit.git_author_email)
+ mail.execute(build)
+ end
+
+ def should_email(email)
+ Notify.should_receive(:build_fail_email).with(build.id, email)
+ Notify.should_not_receive(:build_success_email).with(build.id, email)
+ end
+ end
+
+ describe 'successfull build' do
+ let(:project) { FactoryGirl.create(:project, email_add_committer: true, email_only_broken_builds: false) }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ end
+
+ it do
+ should_email(commit.git_author_email)
+ mail.execute(build)
+ end
+
+ def should_email(email)
+ Notify.should_receive(:build_success_email).with(build.id, email)
+ Notify.should_not_receive(:build_fail_email).with(build.id, email)
+ end
+ end
+
+ describe 'successfull build and project has email_recipients' do
+ let(:project) {
+ FactoryGirl.create(:project,
+ email_add_committer: true,
+ email_only_broken_builds: false,
+ email_recipients: "jeroen@example.com")
+ }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ end
+
+ it do
+ should_email(commit.git_author_email)
+ should_email("jeroen@example.com")
+ mail.execute(build)
+ end
+
+ def should_email(email)
+ Notify.should_receive(:build_success_email).with(build.id, email)
+ Notify.should_not_receive(:build_fail_email).with(build.id, email)
+ end
+ end
+
+ describe 'successful build and notify only broken builds' do
+ let(:project) {
+ FactoryGirl.create(:project,
+ email_add_committer: true,
+ email_only_broken_builds: true,
+ email_recipients: "jeroen@example.com")
+ }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ end
+
+ it do
+ should_email(commit.git_author_email)
+ should_email("jeroen@example.com")
+ mail.execute(build)
+ end
+
+ def should_email(email)
+ Notify.should_not_receive(:build_success_email).with(build.id, email)
+ Notify.should_not_receive(:build_fail_email).with(build.id, email)
+ end
+ end
+
+ describe 'successful build and can test service' do
+ let(:project) {
+ FactoryGirl.create(:project,
+ email_add_committer: true,
+ email_only_broken_builds: false,
+ email_recipients: "jeroen@example.com")
+ }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ build
+ end
+
+ it do
+ mail.can_test?.should == true
+ end
+ end
+
+ describe 'successful build and cannot test service' do
+ let(:project) {
+ FactoryGirl.create(:project,
+ email_add_committer: true,
+ email_only_broken_builds: true,
+ email_recipients: "jeroen@example.com")
+ }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ build
+ end
+
+ it do
+ mail.can_test?.should == false
+ end
+ end
+
+ describe 'retried build should not receive email' do
+ let(:project) {
+ FactoryGirl.create(:project,
+ email_add_committer: true,
+ email_only_broken_builds: true,
+ email_recipients: "jeroen@example.com")
+ }
+ let(:commit) { FactoryGirl.create(:commit, project: project) }
+ let(:build) { FactoryGirl.create(:build, status: :failed, commit: commit) }
+
+ before do
+ mail.stub(
+ project: project
+ )
+ end
+
+ it do
+ Build.retry(build)
+ should_email(commit.git_author_email)
+ should_email("jeroen@example.com")
+ mail.execute(build)
+ end
+
+ def should_email(email)
+ Notify.should_not_receive(:build_success_email).with(build.id, email)
+ Notify.should_not_receive(:build_fail_email).with(build.id, email)
+ end
+ end
+ end
+end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
deleted file mode 100644
index 413523c..0000000
--- a/spec/services/notification_service_spec.rb
+++ /dev/null
@@ -1,56 +0,0 @@
-require 'spec_helper'
-
-describe NotificationService do
- let(:notification) { NotificationService.new }
-
- describe 'Builds' do
-
- describe 'failed build' do
- let(:project) { FactoryGirl.create(:project) }
- let(:commit) { FactoryGirl.create(:commit, project: project) }
- let(:build) { FactoryGirl.create(:build, status: :failed, commit: commit) }
-
- it do
- should_email(commit.git_author_email)
- notification.build_ended(build)
- end
-
- def should_email(email)
- Notify.should_receive(:build_fail_email).with(build.id, email)
- Notify.should_not_receive(:build_success_email).with(build.id, email)
- end
- end
-
- describe 'successfull build' do
- let(:project) { FactoryGirl.create(:project) }
- let(:commit) { FactoryGirl.create(:commit, project: project) }
- let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
- it do
- should_email(commit.git_author_email)
- notification.build_ended(build)
- end
-
- def should_email(email)
- Notify.should_receive(:build_success_email).with(build.id, email)
- Notify.should_not_receive(:build_fail_email).with(build.id, email)
- end
- end
-
- describe 'successfull build and project has email_recipients' do
- let(:project) { FactoryGirl.create(:project, email_recipients: "jeroen@example.com") }
- let(:commit) { FactoryGirl.create(:commit, project: project) }
- let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
-
- it do
- should_email(commit.git_author_email)
- should_email("jeroen@example.com")
- notification.build_ended(build)
- end
-
- def should_email(email)
- Notify.should_receive(:build_success_email).with(build.id, email)
- Notify.should_not_receive(:build_fail_email).with(build.id, email)
- end
- end
- end
-end
View Lorry Controller Status