Email notification settings can now be managed in settings dialog.

13 files changed, 281 insertions, 116 deletions

diff --git a/app/mailers/emails/builds.rb b/app/mailers/emails/builds.rb
index 70e3980..1c3cb8a 100644
--- a/app/mailers/emails/builds.rb
+++ b/app/mailers/emails/builds.rb
@@ -1,15 +1,15 @@
 module Emails
   module Builds
-    def build_fail_email(build_id)
+    def build_fail_email(build_id, to)
       @build = Build.find(build_id)
       @project = @build.project
-      mail(to: @build.git_author_name, subject: subject("Build failed for #{@project.name}", @build.short_sha))
+      mail(to: to, subject: subject("Build failed for #{@project.name}", @build.short_sha))
     end
 
-    def build_success_email(build_id)
+    def build_success_email(build_id, to)
       @build = Build.find(build_id)
       @project = @build.project
-      mail(to: @build.git_author_name, subject: subject("Build success for #{@project.name}", @build.short_sha))
+      mail(to: to, subject: subject("Build success for #{@project.name}", @build.short_sha))
     end
   end
 end
diff --git a/app/models/build.rb b/app/models/build.rb
index 7604a84..04ab456 100644
--- a/app/models/build.rb
+++ b/app/models/build.rb
@@ -165,4 +165,13 @@ class Build < ActiveRecord::Base
   def project_name
     project.name
   end
+  
+  def project_recipients
+    recipients = []
+    return recipients if project.email_only_breaking_build && status != :failed
+    recipients.concat(project.email_recipients.split(' '))
+    recipients << git_author_email if project.email_add_committer?
+    recipients.uniq
+  end
+  
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index b7e7d45..3db8ef5 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -2,27 +2,31 @@
 #
 # Table name: projects
 #
-#  id               :integer          not null, primary key
-#  name             :string(255)      not null
-#  timeout          :integer          default(1800), not null
-#  scripts          :text             default(""), not null
-#  created_at       :datetime         not null
-#  updated_at       :datetime         not null
-#  token            :string(255)
-#  default_ref      :string(255)
-#  gitlab_url       :string(255)
-#  always_build     :boolean          default(FALSE), not null
-#  polling_interval :integer
-#  public           :boolean          default(FALSE), not null
-#  ssh_url_to_repo  :string(255)
-#  gitlab_id        :integer
-#  allow_git_fetch  :boolean          default(TRUE), not null
+#  id                        :integer          not null, primary key
+#  name                      :string(255)      not null
+#  timeout                   :integer          default(1800), not null
+#  scripts                   :text             default(""), not null
+#  created_at                :datetime         not null
+#  updated_at                :datetime         not null
+#  token                     :string(255)
+#  default_ref               :string(255)
+#  gitlab_url                :string(255)
+#  always_build              :boolean          default(FALSE), not null
+#  polling_interval          :integer
+#  public                    :boolean          default(FALSE), not null
+#  ssh_url_to_repo           :string(255)
+#  gitlab_id                 :integer
+#  allow_git_fetch           :boolean          default(TRUE), not null
+#  email_recipients          :string(255)
+#  email_add_committer       :boolean          default(TRUE), not null 
+#  email_only_breaking_build :boolean          default(TRUE), not null 
 #
 
 class Project < ActiveRecord::Base
   attr_accessible :name, :path, :scripts, :timeout, :token,
     :default_ref, :gitlab_url, :always_build, :polling_interval,
-    :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch
+    :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, 
+    :email_recipients, :email_add_committer, :email_only_breaking_build
 
   has_many :builds, dependent: :destroy
   has_many :runner_projects, dependent: :destroy
@@ -54,7 +58,9 @@ class Project < ActiveRecord::Base
         gitlab_url: project.web_url,
         scripts: 'ls -la',
         default_ref: project.default_branch || 'master',
-        ssh_url_to_repo: project.ssh_url_to_repo
+        ssh_url_to_repo: project.ssh_url_to_repo,
+        email_add_committer:       GitlabCi.config.gitlab_ci.add_committer,
+        email_only_breaking_build: GitlabCi.config.gitlab_ci.only_breaking_build,
       }
 
       Project.new(params)
@@ -166,20 +172,9 @@ class Project < ActiveRecord::Base
     # Get running builds not later than 3 days ago to ignore hangs
     builds.running.where("updated_at > ?", 3.days.ago).empty?
   end
+  
+  def email_notification?
+    email_add_committer || !email_recipients.blank?
+  end
+  
 end
-
-
-# == Schema Information
-#
-# Table name: projects
-#
-#  id          :integer(4)      not null, primary key
-#  name        :string(255)     not null
-#  path        :string(255)     not null
-#  timeout     :integer(4)      default(1800), not null
-#  scripts     :text            default(""), not null
-#  created_at  :datetime        not null
-#  updated_at  :datetime        not null
-#  token       :string(255)
-#  default_ref :string(255)
-#
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 72f64d7..6f392be 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -8,11 +8,13 @@
 class NotificationService
 
   def build_ended(build)
-    if !GitlabCi.config.gitlab_ci.only_fail_notifications
-      build.status == :success ? mailer.build_success_email(build.id) : mailer.build_fail_email(build.id)
-    elsif build.status == :failed  
-      mailer.build_fail_email(build.id)
-    end
+    build.project_recipients.each do |recipient|
+      if build.status == :success 
+        mailer.build_success_email(build.id, recipient) 
+      else
+        mailer.build_fail_email(build.id, recipient)
+      end
+    end      
   end
 
   protected
@@ -22,4 +24,5 @@ class NotificationService
     # Notify.delay
     Notify
   end
+  
 end
diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml
index 5539d8b..2a3ee3a 100644
--- a/app/views/admin/projects/show.html.haml
+++ b/app/views/admin/projects/show.html.haml
@@ -31,6 +31,17 @@
         Last build:
         - if @project.last_build
           = time_ago_in_words @project.last_build_date
+    %fieldset
+      %legend Email notification
+      %p
+        Repicients:
+        %strong= @project.email_recipients
+      %p
+        Send mail to commiter:
+        %strong= @project.email_add_committer
+      %p
+        Send only on broken builds:
+        %strong= @project.email_only_breaking_build
 
   .span6
     %fieldset
diff --git a/app/views/projects/_form.html.haml b/app/views/projects/_form.html.haml
index a9e9025..542eacf 100644
--- a/app/views/projects/_form.html.haml
+++ b/app/views/projects/_form.html.haml
@@ -30,6 +30,7 @@
             %span.light (faster)
 
 
+      %br
       %fieldset
         %legend Build Schedule
         .field
@@ -41,6 +42,7 @@
           = f.label :polling_interval, "Build interval"
           = f.number_field :polling_interval, placeholder: '5'
           %span Hours
+
     .span5
       %fieldset
         %legend Project settings
@@ -53,6 +55,24 @@
             = f.check_box :public, class: 'input-xlarge'
             %span Public (Anyone can see project and builds list)
         %br
+
+      %fieldset
+        %legend Email notification
+        .field
+          = f.label :email_recipients, "Whitespace-separated list of recipient addresses."
+          = f.text_field :email_recipients, class: 'input-xlarge'
+        %br
+        .field
+          = f.label :email_add_committer do
+            = f.check_box :email_add_committer
+            %span Add committer to recipients list
+        %br
+        .field
+          = f.label :email_only_breaking_build do
+            = f.check_box :email_only_breaking_build
+            %span Send notification on breaking builds only
+
+      %br
       %fieldset
         %legend Advanced settings
         .field
diff --git a/config/application.yml.example b/config/application.yml.example
index 9c8795c..24f96f8 100644
--- a/config/application.yml.example
+++ b/config/application.yml.example
@@ -17,8 +17,11 @@ defaults: &defaults
     # Email address of your support contact (default: same as email_from)
     support_email: support@localhost
   
-    # Only send emails for failing builds
-    # only_fail_notifications: true
+    # Only send emails for breaking builds
+    # only_breaking_build: true
+    
+    # Add committer to recipients list
+    # add_committer: true
     
   gravatar:
     enabled: true
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 11ed59d..314ea37 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -30,15 +30,16 @@ end
 # GitlabCi
 #
 Settings['gitlab_ci'] ||= Settingslogic.new({})
-Settings.gitlab_ci['https']             = false if Settings.gitlab_ci['https'].nil?
-Settings.gitlab_ci['host']              ||= 'localhost'
-Settings.gitlab_ci['port']              ||= Settings.gitlab_ci.https ? 443 : 80
-Settings.gitlab_ci['relative_url_root'] ||= ENV['RAILS_RELATIVE_URL_ROOT'] || ''
-Settings.gitlab_ci['protocol']          ||= Settings.gitlab_ci.https ? "https" : "http"
-Settings.gitlab_ci['email_from']        ||= "gitlab-ci@#{Settings.gitlab_ci.host}"
-Settings.gitlab_ci['support_email']     ||= Settings.gitlab_ci.email_from
-Settings.gitlab_ci['only_fail_notifications'] ||= true
-Settings.gitlab_ci['url']               ||= Settings.send(:build_gitlab_ci_url)
+Settings.gitlab_ci['https']               = false if Settings.gitlab_ci['https'].nil?
+Settings.gitlab_ci['host']                ||= 'localhost'
+Settings.gitlab_ci['port']                ||= Settings.gitlab_ci.https ? 443 : 80
+Settings.gitlab_ci['relative_url_root']   ||= ENV['RAILS_RELATIVE_URL_ROOT'] || ''
+Settings.gitlab_ci['protocol']            ||= Settings.gitlab_ci.https ? "https" : "http"
+Settings.gitlab_ci['email_from']          ||= "gitlab-ci@#{Settings.gitlab_ci.host}"
+Settings.gitlab_ci['support_email']       ||= Settings.gitlab_ci.email_from
+Settings.gitlab_ci['only_breaking_build'] ||= true
+Settings.gitlab_ci['add_committer']       ||= true
+Settings.gitlab_ci['url']                 ||= Settings.send(:build_gitlab_ci_url)
 
 
 #
diff --git a/db/migrate/20131120155545_add_email_notification_fields_to_project.rb b/db/migrate/20131120155545_add_email_notification_fields_to_project.rb
new file mode 100644
index 0000000..ff2e71a
--- /dev/null
+++ b/db/migrate/20131120155545_add_email_notification_fields_to_project.rb
@@ -0,0 +1,7 @@
+class AddEmailNotificationFieldsToProject < ActiveRecord::Migration
+  def change
+    add_column :projects, :email_recipients, :string, default: '', null: false
+    add_column :projects, :email_add_committer, :boolean, default: true, null: false
+    add_column :projects, :email_only_breaking_build, :boolean, default: true, null: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9ab222f..92b5b76 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20131023103430) do
+ActiveRecord::Schema.define(:version => 20131120155545) do
 
   create_table "builds", :force => true do |t|
     t.integer  "project_id"
@@ -33,20 +33,23 @@ ActiveRecord::Schema.define(:version => 20131023103430) do
   add_index "builds", ["runner_id"], :name => "index_builds_on_runner_id"
 
   create_table "projects", :force => true do |t|
-    t.string   "name",                                :null => false
-    t.integer  "timeout",          :default => 1800,  :null => false
-    t.text     "scripts",                             :null => false
-    t.datetime "created_at",                          :null => false
-    t.datetime "updated_at",                          :null => false
+    t.string   "name",                                         :null => false
+    t.integer  "timeout",                   :default => 1800,  :null => false
+    t.text     "scripts",                                      :null => false
+    t.datetime "created_at",                                   :null => false
+    t.datetime "updated_at",                                   :null => false
     t.string   "token"
     t.string   "default_ref"
     t.string   "gitlab_url"
-    t.boolean  "always_build",     :default => false, :null => false
+    t.boolean  "always_build",              :default => false, :null => false
     t.integer  "polling_interval"
-    t.boolean  "public",           :default => false, :null => false
+    t.boolean  "public",                    :default => false, :null => false
     t.string   "ssh_url_to_repo"
     t.integer  "gitlab_id"
-    t.boolean  "allow_git_fetch",  :default => true,  :null => false
+    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_only_breaking_build", :default => true,  :null => false
   end
 
   create_table "runner_projects", :force => true do |t|
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index a1891c8..f254fe4 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -33,6 +33,7 @@ describe Build do
   it { should respond_to :running? }
   it { should respond_to :pending? }
   it { should respond_to :git_author_name }
+  it { should respond_to :git_author_email }
   it { should respond_to :short_sha }
   it { should respond_to :trace_html }
 
@@ -49,6 +50,98 @@ describe Build do
       build.ci_skip?.should == false
     end
   end
+  
+  describe '#project_recipients' do
+    # before(:each) do
+    #   @settings = double("settings")
+    #   @settings.stub(:only_breaking_build) { true }
+    #   stub_const("Settings", Class.new)
+    #   Settings.stub_chain(:gitlab_ci).and_return(@settings)
+    # end
+    context 'always sending notification' do
+      it 'should return git_author_email as only recipient when no additional recipients are given' do
+        project = FactoryGirl.create :project, 
+                                           email_only_breaking_build: false,
+                                           email_add_committer: true,
+                                           email_recipients: '' 
+        build =  FactoryGirl.create :build, 
+                                         status: :success, 
+                                         project: project 
+        expected = 'git_author_email'
+        build.stub(:git_author_email) { expected }
+        build.project_recipients.should == [expected]
+      end
+
+      it 'should return git_author_email and additional recipients' do
+        project = FactoryGirl.create :project, 
+                                           email_only_breaking_build: false,
+                                           email_add_committer: true,
+                                           email_recipients: 'rec1 rec2' 
+        build = FactoryGirl.create :build, 
+                                         status: :success, 
+                                         project: project 
+        expected = 'git_author_email'
+        build.stub(:git_author_email) { expected }
+        build.project_recipients.should == ['rec1', 'rec2', expected]
+      end
+
+      it 'should return recipients' do
+        project = FactoryGirl.create :project, 
+                                           email_only_breaking_build: false,
+                                           email_add_committer: false,
+                                           email_recipients: 'rec1 rec2' 
+        build = FactoryGirl.create :build, 
+                                         status: :success, 
+                                         project: project 
+        expected = 'git_author_email'
+        build.stub(:git_author_email) { expected }
+        build.project_recipients.should == ['rec1', 'rec2']
+      end
+
+      it 'should return unique recipients only' do
+        project = FactoryGirl.create :project, 
+                                           email_only_breaking_build: false,
+                                           email_add_committer: true,
+                                           email_recipients: 'rec1 rec1 rec2' 
+        build = FactoryGirl.create :build, 
+                                         status: :success, 
+                                         project: project 
+        expected = 'rec2'
+        build.stub(:git_author_email) { expected }
+        build.project_recipients.should == ['rec1', 'rec2']
+      end
+    end
+
+    describe 'only on breaking build sending notification' do
+      it 'should return git_author_email and additional recipients' do
+        project = FactoryGirl.create :project, 
+                                           email_only_breaking_build: true,
+                                           email_add_committer: true,
+                                           email_recipients: 'rec1 rec2' 
+        build = FactoryGirl.create :build, 
+                                         status: :failed, 
+                                         project: project 
+        expected = 'git_author_email'
+        build.stub(:git_author_email) { expected }
+        build.project_recipients.should == ['rec1', 'rec2', expected]
+      end
+
+      it 'should return an empty array because the build was successful' do
+        project = FactoryGirl.create :project, 
+                                           email_only_breaking_build: true,
+                                           email_add_committer: true,
+                                           email_recipients: 'rec1 rec2' 
+        build = FactoryGirl.create :build, 
+                                         status: :success, 
+                                         project: project 
+        expected = 'git_author_email'
+        build.stub(:git_author_email) { expected }
+        build.project_recipients.should == []
+      end
+    end
+  
+  end
+  
 end
 
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 68007aa..9778dc8 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2,21 +2,24 @@
 #
 # Table name: projects
 #
-#  id               :integer          not null, primary key
-#  name             :string(255)      not null
-#  timeout          :integer          default(1800), not null
-#  scripts          :text             default(""), not null
-#  created_at       :datetime         not null
-#  updated_at       :datetime         not null
-#  token            :string(255)
-#  default_ref      :string(255)
-#  gitlab_url       :string(255)
-#  always_build     :boolean          default(FALSE), not null
-#  polling_interval :integer
-#  public           :boolean          default(FALSE), not null
-#  ssh_url_to_repo  :string(255)
-#  gitlab_id        :integer
-#  allow_git_fetch  :boolean          default(TRUE), not null
+#  id                        :integer          not null, primary key
+#  name                      :string(255)      not null
+#  timeout                   :integer          default(1800), not null
+#  scripts                   :text             default(""), not null
+#  created_at                :datetime         not null
+#  updated_at                :datetime         not null
+#  token                     :string(255)
+#  default_ref               :string(255)
+#  gitlab_url                :string(255)
+#  always_build              :boolean          default(FALSE), not null
+#  polling_interval          :integer
+#  public                    :boolean          default(FALSE), not null
+#  ssh_url_to_repo           :string(255)
+#  gitlab_id                 :integer
+#  allow_git_fetch           :boolean          default(TRUE), not null
+#  email_recipients          :string(255)
+#  email_add_committer       :boolean          default(TRUE), not null 
+#  email_only_breaking_build :boolean          default(TRUE), not null 
 #
 
 require 'spec_helper'
@@ -64,24 +67,52 @@ describe Project do
       it { project.status_image.should == 'running.png' }
     end
   end
+  
+  describe '#email_notification?' do
+    it { 
+      project = FactoryGirl.create :project, email_add_committer: true 
+      project.email_notification?.should == true 
+    }
+
+    it { 
+      project = FactoryGirl.create :project, email_add_committer: false 
+      project.email_notification?.should == false 
+    }
+
+    it { 
+      project = FactoryGirl.create :project, email_add_committer: false, email_recipients: 'test tesft' 
+      project.email_notification?.should == true 
+    }
+
+    it { 
+      project = FactoryGirl.create :project, email_add_committer: false, email_recipients: ''  
+      project.email_notification?.should == false 
+    }
+  end
+  
 end
 
 # == Schema Information
 #
 # Table name: projects
 #
-#  id               :integer(4)      not null, primary key
-#  name             :string(255)     not null
-#  path             :string(255)     not null
-#  timeout          :integer(4)      default(1800), not null
-#  scripts          :text            default(""), not null
-#  created_at       :datetime        not null
-#  updated_at       :datetime        not null
-#  token            :string(255)
-#  default_ref      :string(255)
-#  gitlab_url       :string(255)
-#  always_build     :boolean(1)      default(FALSE), not null
-#  polling_interval :integer(4)
-#  public           :boolean(1)      default(FALSE), not null
+#  id                        :integer          not null, primary key
+#  name                      :string(255)      not null
+#  timeout                   :integer          default(1800), not null
+#  scripts                   :text             default(""), not null
+#  created_at                :datetime         not null
+#  updated_at                :datetime         not null
+#  token                     :string(255)
+#  default_ref               :string(255)
+#  gitlab_url                :string(255)
+#  always_build              :boolean          default(FALSE), not null
+#  polling_interval          :integer
+#  public                    :boolean          default(FALSE), not null
+#  ssh_url_to_repo           :string(255)
+#  gitlab_id                 :integer
+#  allow_git_fetch           :boolean          default(TRUE), not null
+#  email_recipients          :string(255)
+#  email_add_committer       :boolean          default(TRUE), not null 
+#  email_only_breaking_build :boolean          default(TRUE), not null 
 #
 
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 2ef34b9..841763a 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -4,49 +4,38 @@ describe NotificationService do
   let(:notification) { NotificationService.new }
 
   describe 'Builds' do
-    let(:project) { FactoryGirl.create(:project)}
 
     describe 'failed build' do
+      let(:project) { FactoryGirl.create(:project, :email_only_breaking_build => true)}
       let(:build) { FactoryGirl.create(:build, :status => :failed, :project => project) }
+
       it do
         should_email(build.git_author_email)
         notification.build_ended(build)
       end
 
-      def should_email(user_id)
-        Notify.should_receive(:build_fail_email).with(build.id)
-        Notify.should_not_receive(:build_success_email).with(build.id)
+      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 with default settings' do
-      before(:each) do
-        @settings = double("settings")
-        @settings.stub(:only_fail_notifications) { true }
-        stub_const("Settings", Class.new)
-        Settings.stub_chain(:gitlab_ci).and_return(@settings)
-      end
-
+    describe 'successfull build when only breaking builds should trigger email' do
+      let(:project) { FactoryGirl.create(:project, :email_only_breaking_build => true)}
       let(:build) { FactoryGirl.create(:build, :status => :success, :project => project) }
       it do
         should_not_email(build.git_author_email)
         notification.build_ended(build)
       end
 
-      def should_not_email(user_id)
-        Notify.should_not_receive(:build_success_email).with(build.id)
-        Notify.should_not_receive(:build_fail_email).with(build.id)
+      def should_not_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 'successfull build with changed settings' do
-      before(:each) do
-        @settings = double("settings")
-        @settings.stub(:only_fail_notifications) { false }
-        stub_const("Settings", Class.new)
-        Settings.stub_chain(:gitlab_ci).and_return(@settings)
-      end
-
+    describe 'successfull build when all builds should trigger email' do
+      let(:project) { FactoryGirl.create(:project, :email_only_breaking_build => false)}
       let(:build) { FactoryGirl.create(:build, :status => :success, :project => project) }
       
       it do
@@ -54,9 +43,9 @@ describe NotificationService do
         notification.build_ended(build)
       end
 
-      def should_email(user_id)
-        Notify.should_receive(:build_success_email).with(build.id)
-        Notify.should_not_receive(:build_fail_email).with(build.id)
+      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