diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-03-10 18:26:11 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-03-10 18:26:11 +0000 |
commit | f8ebe264ee725a3d98bf04ef8c7a1f22cfd55907 (patch) | |
tree | db78dfbea34360c4c017c73b20204bf5e44a116c | |
parent | 52a12893747ddac4a38dfa5becd3231e5092619c (diff) | |
parent | 7499901c2421074b1730b8a803b9435a3f234506 (diff) | |
download | gitlab-ci-f8ebe264ee725a3d98bf04ef8c7a1f22cfd55907.tar.gz |
Merge branch 'notify_based_on_pusher_email' into 'master'
Notify only pusher instead of commiter
Fixes #161
!!! Dependent MR on GitLab - https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/1630
See merge request !126
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/services_controller.rb | 2 | ||||
-rw-r--r-- | app/models/commit.rb | 6 | ||||
-rw-r--r-- | app/models/project.rb | 8 | ||||
-rw-r--r-- | app/models/project_services/mail_service.rb | 4 | ||||
-rw-r--r-- | app/services/create_commit_service.rb | 1 | ||||
-rw-r--r-- | app/views/admin/projects/show.html.haml | 2 | ||||
-rw-r--r-- | config/application.yml.example | 4 | ||||
-rw-r--r-- | config/initializers/1_settings.rb | 2 | ||||
-rw-r--r-- | db/migrate/20150310001733_rename_committer_to_pusher.rb | 5 | ||||
-rw-r--r-- | db/schema.rb | 4 | ||||
-rw-r--r-- | spec/factories/commits.rb | 1 | ||||
-rw-r--r-- | spec/factories/projects.rb | 2 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 24 | ||||
-rw-r--r-- | spec/models/mail_service_spec.rb | 20 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 16 |
16 files changed, 56 insertions, 46 deletions
@@ -11,6 +11,7 @@ v7.9.0 v7.8.2 - Fix the broken build failed email + - Notify only pusher instead of commiter v7.8.0 - Fix OAuth login with GitLab installed in relative URL diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb index 99b4d65..39cf306 100644 --- a/app/controllers/services_controller.rb +++ b/app/controllers/services_controller.rb @@ -50,7 +50,7 @@ class ServicesController < ApplicationController def service_params params.require(:service).permit( :type, :active, :webhook, :notify_only_broken_builds, - :email_recipients, :email_only_broken_builds, :email_add_committer + :email_recipients, :email_only_broken_builds, :email_add_pusher ) end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 11b61d4..2e1bd52 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -92,7 +92,11 @@ class Commit < ActiveRecord::Base def project_recipients recipients = project.email_recipients.split(' ') - recipients << git_author_email if project.email_add_committer? + + if project.email_add_pusher? && push_data[:user_email].present? + recipients << push_data[:user_email] + end + recipients.uniq end diff --git a/app/models/project.rb b/app/models/project.rb index bf14dc5..d615f7e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -17,7 +17,7 @@ # gitlab_id :integer # allow_git_fetch :boolean default(TRUE), not null # email_recipients :string(255) default(""), not null -# email_add_committer :boolean default(TRUE), not null +# email_add_pusher :boolean default(TRUE), not null # email_only_broken_builds :boolean default(TRUE), not null # skip_refs :string(255) # coverage_regex :string(255) @@ -29,7 +29,7 @@ class Project < ActiveRecord::Base attr_accessible :name, :path, :timeout, :token, :timeout_in_minutes, :default_ref, :gitlab_url, :always_build, :polling_interval, :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, :skip_refs, - :email_recipients, :email_add_committer, :email_only_broken_builds, :coverage_regex, + :email_recipients, :email_add_pusher, :email_only_broken_builds, :coverage_regex, :jobs_attributes has_many :commits, dependent: :destroy @@ -81,7 +81,7 @@ ls -la gitlab_url: project.web_url, default_ref: project.default_branch || 'master', ssh_url_to_repo: project.ssh_url_to_repo, - email_add_committer: GitlabCi.config.gitlab_ci.add_committer, + email_add_pusher: GitlabCi.config.gitlab_ci.add_pusher, email_only_broken_builds: GitlabCi.config.gitlab_ci.all_broken_builds, } @@ -137,7 +137,7 @@ ls -la end def email_notification? - email_add_committer || email_recipients.present? + email_add_pusher || email_recipients.present? end def web_hooks? diff --git a/app/models/project_services/mail_service.rb b/app/models/project_services/mail_service.rb index 380d2b6..a920968 100644 --- a/app/models/project_services/mail_service.rb +++ b/app/models/project_services/mail_service.rb @@ -14,7 +14,7 @@ class MailService < Service delegate :email_recipients, :email_recipients=, - :email_add_committer, :email_add_committer=, + :email_add_pusher, :email_add_pusher=, :email_only_broken_builds, :email_only_broken_builds=, to: :project, prefix: false before_save :update_project @@ -36,7 +36,7 @@ class MailService < Service 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_add_pusher', label: 'Add pusher to recipients list' }, { type: 'checkbox', name: 'email_only_broken_builds', label: 'Notify only broken builds' } ] end diff --git a/app/services/create_commit_service.rb b/app/services/create_commit_service.rb index 6526445..b7a5903 100644 --- a/app/services/create_commit_service.rb +++ b/app/services/create_commit_service.rb @@ -32,6 +32,7 @@ class CreateCommitService after: sha, ref: ref, user_name: params[:user_name], + user_email: params[:user_email], repository: params[:repository], commits: params[:commits], total_commits_count: params[:total_commits_count] diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 8204a90..f8e6b9f 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -42,7 +42,7 @@ %strong= @project.email_recipients %p Send mail to commiter: - %strong= @project.email_add_committer + %strong= @project.email_add_pusher %p Send on all broken builds: %strong= @project.email_only_broken_builds diff --git a/config/application.yml.example b/config/application.yml.example index acdc993..124d147 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -23,8 +23,8 @@ defaults: &defaults # Send emails only on broken builds (default: true) # all_broken_builds: true # - # Add committer to recipients list (default: false) - # add_committer: true + # Add pusher to recipients list (default: false) + # add_pusher: true gravatar: enabled: true diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 8b9703a..8e77da8 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -38,7 +38,7 @@ Settings.gitlab_ci['protocol'] ||= Settings.gitlab_ci.https ? "https" Settings.gitlab_ci['email_from'] ||= "gitlab-ci@#{Settings.gitlab_ci.host}" Settings.gitlab_ci['support_email'] ||= Settings.gitlab_ci.email_from Settings.gitlab_ci['all_broken_builds'] = true if Settings.gitlab_ci['all_broken_builds'].nil? -Settings.gitlab_ci['add_committer'] = false if Settings.gitlab_ci['add_committer'].nil? +Settings.gitlab_ci['add_pusher'] = false if Settings.gitlab_ci['add_pusher'].nil? Settings.gitlab_ci['url'] ||= Settings.send(:build_gitlab_ci_url) # Compatibility with old config diff --git a/db/migrate/20150310001733_rename_committer_to_pusher.rb b/db/migrate/20150310001733_rename_committer_to_pusher.rb new file mode 100644 index 0000000..7e18367 --- /dev/null +++ b/db/migrate/20150310001733_rename_committer_to_pusher.rb @@ -0,0 +1,5 @@ +class RenameCommitterToPusher < ActiveRecord::Migration + def change + rename_column :projects, :email_add_committer, :email_add_pusher + end +end diff --git a/db/schema.rb b/db/schema.rb index 804984c..b76eaa0 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: 20150306135341) do +ActiveRecord::Schema.define(version: 20150310001733) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -87,7 +87,7 @@ ActiveRecord::Schema.define(version: 20150306135341) do t.integer "gitlab_id" t.boolean "allow_git_fetch", default: true, null: false t.string "email_recipients", default: "", null: false - t.boolean "email_add_committer", default: true, null: false + t.boolean "email_add_pusher", default: true, null: false t.boolean "email_only_broken_builds", default: true, null: false t.string "skip_refs" t.string "coverage_regex" diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index 156fabe..3674643 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -25,6 +25,7 @@ FactoryGirl.define do before: '76de212e80737a608d939f648d959671fb0a0142', after: '97de212e80737a608d939f648d959671fb0a0142', user_name: 'Git User', + user_email: 'git@example.com', repository: { name: 'test-data', url: 'ssh://git@gitlab.com/test/test-data.git', diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 552cd4e..55e2cdd 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -17,7 +17,7 @@ # gitlab_id :integer # allow_git_fetch :boolean default(TRUE), not null # email_recipients :string(255) default(""), not null -# email_add_committer :boolean default(TRUE), not null +# email_add_pusher :boolean default(TRUE), not null # email_only_broken_builds :boolean default(TRUE), not null # skip_refs :string(255) # coverage_regex :string(255) diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 891cd52..da57109 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -79,43 +79,41 @@ describe Commit do describe :project_recipients do context 'always sending notification' do - it 'should return git_author_email as only recipient when no additional recipients are given' do + it 'should return commit_pusher_email as only recipient when no additional recipients are given' do project = FactoryGirl.create :project, - email_add_committer: true, + email_add_pusher: true, email_recipients: '' commit = FactoryGirl.create :commit, project: project - expected = 'git_author_email' - commit.stub(:git_author_email) { expected } + expected = 'commit_pusher_email' + commit.stub(:push_data) { { user_email: expected } } commit.project_recipients.should == [expected] end - it 'should return git_author_email and additional recipients' do + it 'should return commit_pusher_email and additional recipients' do project = FactoryGirl.create :project, - email_add_committer: true, + email_add_pusher: true, email_recipients: 'rec1 rec2' commit = FactoryGirl.create :commit, project: project - expected = 'git_author_email' - commit.stub(:git_author_email) { expected } + expected = 'commit_pusher_email' + commit.stub(:push_data) { { user_email: expected } } commit.project_recipients.should == ['rec1', 'rec2', expected] end it 'should return recipients' do project = FactoryGirl.create :project, - email_add_committer: false, + email_add_pusher: false, email_recipients: 'rec1 rec2' commit = FactoryGirl.create :commit, project: project - expected = 'git_author_email' - commit.stub(:git_author_email) { expected } commit.project_recipients.should == ['rec1', 'rec2'] end it 'should return unique recipients only' do project = FactoryGirl.create :project, - email_add_committer: true, + email_add_pusher: true, email_recipients: 'rec1 rec1 rec2' commit = FactoryGirl.create :commit, project: project expected = 'rec2' - commit.stub(:git_author_email) { expected } + commit.stub(:push_data) { { user_email: expected } } commit.project_recipients.should == ['rec1', 'rec2'] end end diff --git a/spec/models/mail_service_spec.rb b/spec/models/mail_service_spec.rb index 7a43978..a24fca2 100644 --- a/spec/models/mail_service_spec.rb +++ b/spec/models/mail_service_spec.rb @@ -17,7 +17,7 @@ describe MailService do let(:mail) { MailService.new } describe 'failed build' do - let(:project) { FactoryGirl.create(:project, email_add_committer: true) } + let(:project) { FactoryGirl.create(:project, email_add_pusher: true) } let(:commit) { FactoryGirl.create(:commit, project: project) } let(:build) { FactoryGirl.create(:build, status: :failed, commit: commit) } @@ -28,7 +28,7 @@ describe MailService do end it do - should_email(commit.git_author_email) + should_email("git@example.com") mail.execute(build) end @@ -39,7 +39,7 @@ describe MailService do end describe 'successfull build' do - let(:project) { FactoryGirl.create(:project, email_add_committer: true, email_only_broken_builds: false) } + let(:project) { FactoryGirl.create(:project, email_add_pusher: true, email_only_broken_builds: false) } let(:commit) { FactoryGirl.create(:commit, project: project) } let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) } @@ -50,7 +50,7 @@ describe MailService do end it do - should_email(commit.git_author_email) + should_email("git@example.com") mail.execute(build) end @@ -63,7 +63,7 @@ describe MailService do describe 'successfull build and project has email_recipients' do let(:project) { FactoryGirl.create(:project, - email_add_committer: true, + email_add_pusher: true, email_only_broken_builds: false, email_recipients: "jeroen@example.com") } @@ -77,7 +77,7 @@ describe MailService do end it do - should_email(commit.git_author_email) + should_email("git@example.com") should_email("jeroen@example.com") mail.execute(build) end @@ -91,7 +91,7 @@ describe MailService do describe 'successful build and notify only broken builds' do let(:project) { FactoryGirl.create(:project, - email_add_committer: true, + email_add_pusher: true, email_only_broken_builds: true, email_recipients: "jeroen@example.com") } @@ -119,7 +119,7 @@ describe MailService do describe 'successful build and can test service' do let(:project) { FactoryGirl.create(:project, - email_add_committer: true, + email_add_pusher: true, email_only_broken_builds: false, email_recipients: "jeroen@example.com") } @@ -141,7 +141,7 @@ describe MailService do describe 'successful build and cannot test service' do let(:project) { FactoryGirl.create(:project, - email_add_committer: true, + email_add_pusher: true, email_only_broken_builds: true, email_recipients: "jeroen@example.com") } @@ -163,7 +163,7 @@ describe MailService do describe 'retried build should not receive email' do let(:project) { FactoryGirl.create(:project, - email_add_committer: true, + email_add_pusher: true, email_only_broken_builds: true, email_recipients: "jeroen@example.com") } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4f7f299..af7d162 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -17,7 +17,7 @@ # gitlab_id :integer # allow_git_fetch :boolean default(TRUE), not null # email_recipients :string(255) default(""), not null -# email_add_committer :boolean default(TRUE), not null +# email_add_pusher :boolean default(TRUE), not null # email_only_broken_builds :boolean default(TRUE), not null # skip_refs :string(255) # coverage_regex :string(255) @@ -63,45 +63,45 @@ describe Project do describe '#email_notification?' do it do - project = FactoryGirl.create :project, email_add_committer: true + project = FactoryGirl.create :project, email_add_pusher: true project.email_notification?.should == true end it do - project = FactoryGirl.create :project, email_add_committer: false, email_recipients: 'test tesft' + project = FactoryGirl.create :project, email_add_pusher: false, email_recipients: 'test tesft' project.email_notification?.should == true end it do - project = FactoryGirl.create :project, email_add_committer: false, email_recipients: '' + project = FactoryGirl.create :project, email_add_pusher: false, email_recipients: '' project.email_notification?.should == false end end describe '#broken_or_success?' do it { - project = FactoryGirl.create :project, email_add_committer: true + project = FactoryGirl.create :project, email_add_pusher: true project.stub(:broken?).and_return(true) project.stub(:success?).and_return(true) project.broken_or_success?.should == true } it { - project = FactoryGirl.create :project, email_add_committer: true + project = FactoryGirl.create :project, email_add_pusher: true project.stub(:broken?).and_return(true) project.stub(:success?).and_return(false) project.broken_or_success?.should == true } it { - project = FactoryGirl.create :project, email_add_committer: true + project = FactoryGirl.create :project, email_add_pusher: true project.stub(:broken?).and_return(false) project.stub(:success?).and_return(true) project.broken_or_success?.should == true } it { - project = FactoryGirl.create :project, email_add_committer: true + project = FactoryGirl.create :project, email_add_pusher: true project.stub(:broken?).and_return(false) project.stub(:success?).and_return(false) project.broken_or_success?.should == false |