Migrate E-mail notification to Services

10 files changed, 148 insertions, 71 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 68dafed..4877bed 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
 
@@ -63,6 +64,8 @@ class Project < ActiveRecord::Base
 
   before_validation :set_default_values
 
+  after_initialize :build_missing_services
+
   class << self
     def base_build_script
       <<-eos
@@ -188,7 +191,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 123d15f..db850d8 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/spec/services/notification_service_spec.rb b/spec/models/mail_service_spec.rb
index 413523c..70f4253 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/models/mail_service_spec.rb
@@ -1,18 +1,36 @@
 require 'spec_helper'
 
-describe NotificationService do
-  let(:notification) { NotificationService.new }
+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 'Builds' do
+  describe 'Sends email for' do
+    let(:mail)   { MailService.new }
 
     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) }
 
+      before do
+        mail.stub(
+          project: project,
+          project_id: project.id
+        )
+      end
+
       it do
         should_email(commit.git_author_email)
-        notification.build_ended(build)
+        mail.execute(build)
       end
 
       def should_email(email)
@@ -25,9 +43,17 @@ describe NotificationService do
       let(:project) { FactoryGirl.create(:project) }
       let(:commit) { FactoryGirl.create(:commit, project: project) }
       let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) }
+
+      before do
+        mail.stub(
+          project: project,
+          project_id: project.id
+        )
+      end
+
       it do
         should_email(commit.git_author_email)
-        notification.build_ended(build)
+        mail.execute(build)
       end
 
       def should_email(email)
@@ -40,11 +66,19 @@ describe NotificationService 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) }
+      let(:recipients) { "jeroen@example.com" }
+
+      before do
+        mail.stub(
+          project: project,
+          project_id: project.id
+        )
+      end
 
       it do
         should_email(commit.git_author_email)
         should_email("jeroen@example.com")
-        notification.build_ended(build)
+        mail.execute(build)
       end
 
       def should_email(email)