Rewrite notifications to work without observers

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

10 files changed, 35 insertions, 50 deletions

diff --git a/Gemfile b/Gemfile
index 4f85cf1..9c0b2de 100644
--- a/Gemfile
+++ b/Gemfile
@@ -14,7 +14,6 @@ gem 'actionpack-page_caching'
 gem 'actionpack-action_caching'
 gem 'activerecord-deprecated_finders'
 gem 'activerecord-session_store'
-gem "rails-observers"
 
 # DB
 gem 'mysql2', group: :mysql
@@ -71,6 +70,7 @@ gem "font-awesome-sass-rails", '~> 3.0.2'
 group :development do
   gem 'annotate'
   gem 'quiet_assets'
+  gem "letter_opener"
 end
 
 
diff --git a/Gemfile.lock b/Gemfile.lock
index d838e6a..776f53a 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -142,6 +142,10 @@ GEM
     kaminari (0.15.0)
       actionpack (>= 3.0.0)
       activesupport (>= 3.0.0)
+    launchy (2.4.2)
+      addressable (~> 2.3)
+    letter_opener (1.1.2)
+      launchy (~> 2.2)
     libv8 (3.16.14.3)
     listen (2.4.0)
       celluloid (>= 0.15.2)
@@ -194,8 +198,6 @@ GEM
       bundler (>= 1.3.0, < 2.0)
       railties (= 4.0.2)
       sprockets-rails (~> 2.0.0)
-    rails-observers (0.1.2)
-      activemodel (~> 4.0)
     railties (4.0.2)
       actionpack (= 4.0.2)
       activesupport (= 4.0.2)
@@ -325,6 +327,7 @@ DEPENDENCIES
   httparty (= 0.11.0)
   jquery-rails
   kaminari
+  letter_opener
   minitest (= 4.3.2)
   mysql2
   pg
@@ -334,7 +337,6 @@ DEPENDENCIES
   puma (~> 2.7.1)
   quiet_assets
   rails (= 4.0.2)
-  rails-observers
   rake
   rb-fsevent
   rb-inotify
diff --git a/app/models/build.rb b/app/models/build.rb
index 44f46e0..99193aa 100644
--- a/app/models/build.rb
+++ b/app/models/build.rb
@@ -75,6 +75,13 @@ class Build < ActiveRecord::Base
 
     after_transition any => [:success, :failed, :canceled] do |build, transition|
       build.update_attributes finished_at: Time.now
+      project = build.project
+
+      if project.email_notification?
+        if build.status.to_sym == :failed || project.email_all_broken_builds
+          NotificationService.new.build_ended(build)
+        end
+      end
     end
 
     state :pending, value: 'pending'
@@ -166,11 +173,10 @@ class Build < ActiveRecord::Base
   def project_name
     project.name
   end
-  
+
   def project_recipients
     recipients = 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 ca35b5b..1a72f7e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -18,14 +18,14 @@
 #  gitlab_id                 :integer
 #  allow_git_fetch           :boolean          default(TRUE), not null
 #  email_recipients          :string(255)
-#  email_add_committer       :boolean          default(TRUE), not null 
-#  email_all_broken_builds   :boolean          default(TRUE), not null 
+#  email_add_committer       :boolean          default(TRUE), not null
+#  email_all_broken_builds   :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_all_broken_builds
 
   has_many :builds, dependent: :destroy
@@ -127,11 +127,11 @@ class Project < ActiveRecord::Base
   def success?
     last_build.success? if last_build
   end
-  
+
   def broken_or_success?
-    broken? || success? 
+    broken? || success?
   end
-  
+
   def last_build
     builds.last
   end
@@ -184,17 +184,16 @@ 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?) && broken_or_success?
+    email_add_committer || email_recipients.present?
   end
-  
+
   # onlu check for toggling build status within same ref.
   def last_build_changed_status?
-    ref = last_build.ref 
+    ref = last_build.ref
     last_builds = builds.where(ref: ref).order('id DESC').limit(2)
-    return false if last_builds.size < 2 
+    return false if last_builds.size < 2
     return last_builds[0].status != last_builds[1].status
   end
-  
 end
diff --git a/app/observers/base_observer.rb b/app/observers/base_observer.rb
deleted file mode 100644
index 1ba725d..0000000
--- a/app/observers/base_observer.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-class BaseObserver < ActiveRecord::Observer
-  def notification
-    NotificationService.new
-  end
-
-end
diff --git a/app/observers/build_observer.rb b/app/observers/build_observer.rb
deleted file mode 100644
index 274497e..0000000
--- a/app/observers/build_observer.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-class BuildObserver < BaseObserver
-  def after_save(build)
-    project = build.project
-    if project.email_notification?
-      if (project.email_all_broken_builds? && project.broken?) || project.last_build_changed_status?
-        notification.build_ended(build)
-      end
-    end
-  end
-
-end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 6f392be..acd2e54 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -6,23 +6,20 @@
 #   NotificationService.new.build_ended(build)
 #
 class NotificationService
-
   def build_ended(build)
     build.project_recipients.each do |recipient|
-      if build.status == :success 
-        mailer.build_success_email(build.id, recipient) 
-      else
+      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
   end
 
   protected
-  
-  # Do we need to delay these emails?
+
   def mailer
-    # Notify.delay
-    Notify
+    Notify.delay
   end
-  
 end
diff --git a/config/application.rb b/config/application.rb
index 588e3c7..ad29b57 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -17,9 +17,6 @@ module GitlabCi
     # :all can be used as a placeholder for all plugins not explicitly named.
     # config.plugins = [ :exception_notification, :ssl_requirement, :all ]
 
-    # Activate observers that should always be running.
-    config.active_record.observers = :build_observer
-
     # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
     # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
     # config.time_zone = 'Central Time (US & Canada)'
diff --git a/config/environments/development.rb b/config/environments/development.rb
index 723ed89..1ec813b 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -29,4 +29,6 @@ GitlabCi::Application.configure do
   config.assets.debug = true
 
   config.eager_load = false
+
+  config.action_mailer.delivery_method = :letter_opener
 end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 1f2038e..fff9c87 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -37,7 +37,7 @@ describe NotificationService do
     describe 'successfull build and project has email_recipients' do
       let(:project) { FactoryGirl.create(:project, :email_recipients => "jeroen@example.com")}
       let(:build) { FactoryGirl.create(:build, :status => :success, :project => project) }
-      
+
       it do
         should_email(build.git_author_email)
         should_email("jeroen@example.com")
@@ -50,5 +50,4 @@ describe NotificationService do
       end
     end
   end
-
 end