Commit f471cc6769fc9b1b358a562c0885ee5ca9ce2646
Exists in
master
and in
1 other branch
Merge pull request #165 from mathias/observers
Add Observers to Models
Showing
9 changed files
with
97 additions
and
76 deletions
Show diff stats
app/models/deploy.rb
| @@ -12,16 +12,11 @@ class Deploy | @@ -12,16 +12,11 @@ class Deploy | ||
| 12 | 12 | ||
| 13 | embedded_in :app, :inverse_of => :deploys | 13 | embedded_in :app, :inverse_of => :deploys |
| 14 | 14 | ||
| 15 | - after_create :deliver_notification, :if => :should_notify? | ||
| 16 | after_create :resolve_app_errs, :if => :should_resolve_app_errs? | 15 | after_create :resolve_app_errs, :if => :should_resolve_app_errs? |
| 17 | after_create :store_cached_attributes_on_problems | 16 | after_create :store_cached_attributes_on_problems |
| 18 | 17 | ||
| 19 | validates_presence_of :username, :environment | 18 | validates_presence_of :username, :environment |
| 20 | 19 | ||
| 21 | - def deliver_notification | ||
| 22 | - Mailer.deploy_notification(self).deliver | ||
| 23 | - end | ||
| 24 | - | ||
| 25 | def resolve_app_errs | 20 | def resolve_app_errs |
| 26 | app.problems.unresolved.in_env(environment).each {|problem| problem.resolve!} | 21 | app.problems.unresolved.in_env(environment).each {|problem| problem.resolve!} |
| 27 | end | 22 | end |
| @@ -32,10 +27,6 @@ class Deploy | @@ -32,10 +27,6 @@ class Deploy | ||
| 32 | 27 | ||
| 33 | protected | 28 | protected |
| 34 | 29 | ||
| 35 | - def should_notify? | ||
| 36 | - app.notify_on_deploys? && app.notification_recipients.any? | ||
| 37 | - end | ||
| 38 | - | ||
| 39 | def should_resolve_app_errs? | 30 | def should_resolve_app_errs? |
| 40 | app.resolve_errs_on_deploy? | 31 | app.resolve_errs_on_deploy? |
| 41 | end | 32 | end |
app/models/notice.rb
| @@ -23,7 +23,6 @@ class Notice | @@ -23,7 +23,6 @@ class Notice | ||
| 23 | ) | 23 | ) |
| 24 | 24 | ||
| 25 | after_create :increase_counter_cache, :cache_attributes_on_problem, :unresolve_problem | 25 | after_create :increase_counter_cache, :cache_attributes_on_problem, :unresolve_problem |
| 26 | - after_create :deliver_notification, :if => :should_notify? | ||
| 27 | before_save :sanitize | 26 | before_save :sanitize |
| 28 | before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem | 27 | before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem |
| 29 | 28 | ||
| @@ -93,10 +92,6 @@ class Notice | @@ -93,10 +92,6 @@ class Notice | ||
| 93 | request['session'] || {} | 92 | request['session'] || {} |
| 94 | end | 93 | end |
| 95 | 94 | ||
| 96 | - def deliver_notification | ||
| 97 | - Mailer.err_notification(self).deliver | ||
| 98 | - end | ||
| 99 | - | ||
| 100 | # Backtrace containing only files from the app itself (ignore gems) | 95 | # Backtrace containing only files from the app itself (ignore gems) |
| 101 | def app_backtrace | 96 | def app_backtrace |
| 102 | backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } | 97 | backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } |
| @@ -104,10 +99,6 @@ class Notice | @@ -104,10 +99,6 @@ class Notice | ||
| 104 | 99 | ||
| 105 | protected | 100 | protected |
| 106 | 101 | ||
| 107 | - def should_notify? | ||
| 108 | - app.notify_on_errs? && (Errbit::Config.per_app_email_at_notices && app.email_at_notices || Errbit::Config.email_at_notices).include?(problem.notices_count) && app.notification_recipients.any? | ||
| 109 | - end | ||
| 110 | - | ||
| 111 | def increase_counter_cache | 102 | def increase_counter_cache |
| 112 | problem.inc(:notices_count, 1) | 103 | problem.inc(:notices_count, 1) |
| 113 | end | 104 | end |
| @@ -0,0 +1,19 @@ | @@ -0,0 +1,19 @@ | ||
| 1 | +class NoticeObserver < Mongoid::Observer | ||
| 2 | + observe :notice | ||
| 3 | + | ||
| 4 | + def after_create notice | ||
| 5 | + return unless should_notify? notice | ||
| 6 | + | ||
| 7 | + Mailer.err_notification(notice).deliver | ||
| 8 | + end | ||
| 9 | + | ||
| 10 | + private | ||
| 11 | + | ||
| 12 | + def should_notify? notice | ||
| 13 | + app = notice.app | ||
| 14 | + app.notify_on_errs? && | ||
| 15 | + (Errbit::Config.per_app_email_at_notices && app.email_at_notices || Errbit::Config.email_at_notices).include?(notice.problem.notices_count) && | ||
| 16 | + app.notification_recipients.any? | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | +end |
config/application.rb
| @@ -51,6 +51,9 @@ module Errbit | @@ -51,6 +51,9 @@ module Errbit | ||
| 51 | # IssueTracker subclasses use inheritance, so preloading models provides querying consistency in dev mode. | 51 | # IssueTracker subclasses use inheritance, so preloading models provides querying consistency in dev mode. |
| 52 | config.mongoid.preload_models = true | 52 | config.mongoid.preload_models = true |
| 53 | 53 | ||
| 54 | + # Set up observers | ||
| 55 | + config.mongoid.observers = :deploy_observer, :notice_observer | ||
| 56 | + | ||
| 54 | # Configure the default encoding used in templates for Ruby 1.9. | 57 | # Configure the default encoding used in templates for Ruby 1.9. |
| 55 | config.encoding = "utf-8" | 58 | config.encoding = "utf-8" |
| 56 | 59 |
| @@ -0,0 +1,20 @@ | @@ -0,0 +1,20 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe DeployObserver do | ||
| 4 | + context 'when a Deploy is saved' do | ||
| 5 | + context 'and the app should notify on deploys' do | ||
| 6 | + it 'should send an email notification' do | ||
| 7 | + Mailer.should_receive(:deploy_notification). | ||
| 8 | + and_return(mock('email', :deliver => true)) | ||
| 9 | + Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => true)) | ||
| 10 | + end | ||
| 11 | + end | ||
| 12 | + | ||
| 13 | + context 'and the app is not set to notify on deploys' do | ||
| 14 | + it 'should not send an email notification' do | ||
| 15 | + Mailer.should_not_receive(:deploy_notification) | ||
| 16 | + Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => false)) | ||
| 17 | + end | ||
| 18 | + end | ||
| 19 | + end | ||
| 20 | +end |
spec/models/deploy_spec.rb
| @@ -17,12 +17,6 @@ describe Deploy do | @@ -17,12 +17,6 @@ describe Deploy do | ||
| 17 | end | 17 | end |
| 18 | 18 | ||
| 19 | context 'being created' do | 19 | context 'being created' do |
| 20 | - it 'should send an email notification' do | ||
| 21 | - Mailer.should_receive(:deploy_notification). | ||
| 22 | - and_return(mock('email', :deliver => true)) | ||
| 23 | - Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => true)) | ||
| 24 | - end | ||
| 25 | - | ||
| 26 | context 'when the app has resolve_errs_on_deploy set to false' do | 20 | context 'when the app has resolve_errs_on_deploy set to false' do |
| 27 | it 'should not resolve the apps errs' do | 21 | it 'should not resolve the apps errs' do |
| 28 | app = Fabricate(:app, :resolve_errs_on_deploy => false) | 22 | app = Fabricate(:app, :resolve_errs_on_deploy => false) |
| @@ -43,12 +37,6 @@ describe Deploy do | @@ -43,12 +37,6 @@ describe Deploy do | ||
| 43 | end | 37 | end |
| 44 | end | 38 | end |
| 45 | 39 | ||
| 46 | - context 'when the app has deploy notifications set to false' do | ||
| 47 | - it 'should not send an email notification' do | ||
| 48 | - Mailer.should_not_receive(:deploy_notification) | ||
| 49 | - Fabricate(:deploy, :app => Fabricate(:app_with_watcher, :notify_on_deploys => false)) | ||
| 50 | - end | ||
| 51 | - end | ||
| 52 | end | 40 | end |
| 53 | 41 | ||
| 54 | it "should produce a shortened revision with 7 characters" do | 42 | it "should produce a shortened revision with 7 characters" do |
| @@ -0,0 +1,46 @@ | @@ -0,0 +1,46 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe NoticeObserver do | ||
| 4 | + describe "email notifications (configured individually for each app)" do | ||
| 5 | + custom_thresholds = [2, 4, 8, 16, 32, 64] | ||
| 6 | + | ||
| 7 | + before do | ||
| 8 | + Errbit::Config.per_app_email_at_notices = true | ||
| 9 | + @app = Fabricate(:app_with_watcher, :email_at_notices => custom_thresholds) | ||
| 10 | + @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app)) | ||
| 11 | + end | ||
| 12 | + | ||
| 13 | + after do | ||
| 14 | + Errbit::Config.per_app_email_at_notices = false | ||
| 15 | + end | ||
| 16 | + | ||
| 17 | + custom_thresholds.each do |threshold| | ||
| 18 | + it "sends an email notification after #{threshold} notice(s)" do | ||
| 19 | + @err.problem.stub(:notices_count).and_return(threshold) | ||
| 20 | + Mailer.should_receive(:err_notification). | ||
| 21 | + and_return(mock('email', :deliver => true)) | ||
| 22 | + Fabricate(:notice, :err => @err) | ||
| 23 | + end | ||
| 24 | + end | ||
| 25 | + end | ||
| 26 | + | ||
| 27 | + describe "email notifications for a resolved issue" do | ||
| 28 | + | ||
| 29 | + before do | ||
| 30 | + Errbit::Config.per_app_email_at_notices = true | ||
| 31 | + @app = Fabricate(:app_with_watcher, :email_at_notices => [1]) | ||
| 32 | + @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app, :notices_count => 100)) | ||
| 33 | + end | ||
| 34 | + | ||
| 35 | + after do | ||
| 36 | + Errbit::Config.per_app_email_at_notices = false | ||
| 37 | + end | ||
| 38 | + | ||
| 39 | + it "should send email notification after 1 notice since an error has been resolved" do | ||
| 40 | + @err.problem.resolve! | ||
| 41 | + Mailer.should_receive(:err_notification). | ||
| 42 | + and_return(mock('email', :deliver => true)) | ||
| 43 | + Fabricate(:notice, :err => @err) | ||
| 44 | + end | ||
| 45 | + end | ||
| 46 | +end |
spec/models/notice_spec.rb
| @@ -109,51 +109,5 @@ describe Notice do | @@ -109,51 +109,5 @@ describe Notice do | ||
| 109 | notice = Fabricate.build(:notice, :request => {}) | 109 | notice = Fabricate.build(:notice, :request => {}) |
| 110 | notice.host.should == 'N/A' | 110 | notice.host.should == 'N/A' |
| 111 | end | 111 | end |
| 112 | - | ||
| 113 | - end | ||
| 114 | - | ||
| 115 | - | ||
| 116 | - describe "email notifications (configured individually for each app)" do | ||
| 117 | - custom_thresholds = [2, 4, 8, 16, 32, 64] | ||
| 118 | - | ||
| 119 | - before do | ||
| 120 | - Errbit::Config.per_app_email_at_notices = true | ||
| 121 | - @app = Fabricate(:app_with_watcher, :email_at_notices => custom_thresholds) | ||
| 122 | - @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app)) | ||
| 123 | - end | ||
| 124 | - | ||
| 125 | - after do | ||
| 126 | - Errbit::Config.per_app_email_at_notices = false | ||
| 127 | - end | ||
| 128 | - | ||
| 129 | - custom_thresholds.each do |threshold| | ||
| 130 | - it "sends an email notification after #{threshold} notice(s)" do | ||
| 131 | - @err.problem.stub(:notices_count).and_return(threshold) | ||
| 132 | - Mailer.should_receive(:err_notification). | ||
| 133 | - and_return(mock('email', :deliver => true)) | ||
| 134 | - Fabricate(:notice, :err => @err) | ||
| 135 | - end | ||
| 136 | - end | ||
| 137 | - end | ||
| 138 | - | ||
| 139 | - describe "email notifications for a resolved issue" do | ||
| 140 | - | ||
| 141 | - before do | ||
| 142 | - Errbit::Config.per_app_email_at_notices = true | ||
| 143 | - @app = Fabricate(:app_with_watcher, :email_at_notices => [1]) | ||
| 144 | - @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app, :notices_count => 100)) | ||
| 145 | - end | ||
| 146 | - | ||
| 147 | - after do | ||
| 148 | - Errbit::Config.per_app_email_at_notices = false | ||
| 149 | - end | ||
| 150 | - | ||
| 151 | - it "should send email notification after 1 notice since an error has been resolved" do | ||
| 152 | - @err.problem.resolve! | ||
| 153 | - Mailer.should_receive(:err_notification). | ||
| 154 | - and_return(mock('email', :deliver => true)) | ||
| 155 | - Fabricate(:notice, :err => @err) | ||
| 156 | - end | ||
| 157 | end | 112 | end |
| 158 | end | 113 | end |
| 159 | - |