Commit d005c47ec4d42e60e02f64c1187d920a991b448c
1 parent
e87d6766
Exists in
master
and in
1 other branch
Add Notice Observer
Showing
5 changed files
with
23 additions
and
54 deletions
Show diff stats
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 | ||
| @@ -92,10 +91,6 @@ class Notice | @@ -92,10 +91,6 @@ class Notice | ||
| 92 | request['session'] || {} | 91 | request['session'] || {} |
| 93 | end | 92 | end |
| 94 | 93 | ||
| 95 | - def deliver_notification | ||
| 96 | - Mailer.err_notification(self).deliver | ||
| 97 | - end | ||
| 98 | - | ||
| 99 | # Backtrace containing only files from the app itself (ignore gems) | 94 | # Backtrace containing only files from the app itself (ignore gems) |
| 100 | def app_backtrace | 95 | def app_backtrace |
| 101 | backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } | 96 | backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } |
| @@ -103,10 +98,6 @@ class Notice | @@ -103,10 +98,6 @@ class Notice | ||
| 103 | 98 | ||
| 104 | protected | 99 | protected |
| 105 | 100 | ||
| 106 | - def should_notify? | ||
| 107 | - 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? | ||
| 108 | - end | ||
| 109 | - | ||
| 110 | def increase_counter_cache | 101 | def increase_counter_cache |
| 111 | problem.inc(:notices_count, 1) | 102 | problem.inc(:notices_count, 1) |
| 112 | end | 103 | 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
| @@ -52,7 +52,7 @@ module Errbit | @@ -52,7 +52,7 @@ module Errbit | ||
| 52 | config.mongoid.preload_models = true | 52 | config.mongoid.preload_models = true |
| 53 | 53 | ||
| 54 | # Set up observers | 54 | # Set up observers |
| 55 | - config.mongoid.observers = :deploy_observer | 55 | + config.mongoid.observers = :deploy_observer, :notice_observer |
| 56 | 56 | ||
| 57 | # Configure the default encoding used in templates for Ruby 1.9. | 57 | # Configure the default encoding used in templates for Ruby 1.9. |
| 58 | config.encoding = "utf-8" | 58 | config.encoding = "utf-8" |
spec/models/notice_spec.rb
| @@ -112,48 +112,4 @@ describe Notice do | @@ -112,48 +112,4 @@ describe Notice do | ||
| 112 | 112 | ||
| 113 | end | 113 | end |
| 114 | 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 | ||
| 158 | end | 115 | end |
| 159 | - |