Merge pull request #261 from cjc343/issue_260

Prevent call to create_notification if notification service has not been configured

Merge pull request #261 from cjc343/issue_260
Prevent call to create_notification if notification service has not been configured
Vasiliy Ermolovich
2 parents 0a43e83b 5834dadb
Showing 2 changed files with 23 additions and 1 deletions Show diff stats
app/models/notice_observer.rb
spec/models/notice_observer_spec.rb
@@ -5,7 +5,7 @@ class NoticeObserver &lt; Mongoid::Observer
     return unless should_notify? notice
  
     # if the app has a notficiation service, fire it off
-    unless notice.app.notification_service.nil?
+    if notice.app.notification_service_configured?
       notice.app.notification_service.create_notification(notice.problem)
     end
  
@@ -66,4 +66,26 @@ describe NoticeObserver do
     end
   end
  
+  describe "should not send a notification if a notification service is not configured" do
+    let(:app) { app = Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))}
+    let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) }
+
+    before do
+      Errbit::Config.per_app_email_at_notices = true
+    end
+
+    after do
+      Errbit::Config.per_app_email_at_notices = false
+    end
+
+    it "should not create a campfire notification" do
+      err.problem.stub(:notices_count) { 1 }
+      app.stub!(:notification_recipients => %w('ryan@system88.com'))
+      app.notification_service.should_not_receive(:create_notification)
+
+      Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'},
+                     :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' })
+    end
+  end
+
 end
...	...	@@ -5,7 +5,7 @@ class NoticeObserver < Mongoid::Observer
5	5	return unless should_notify? notice
6	6
7	7	# if the app has a notficiation service, fire it off
8		- unless notice.app.notification_service.nil?
	8	+ if notice.app.notification_service_configured?
9	9	notice.app.notification_service.create_notification(notice.problem)
10	10	end
11	11
...	...
...	...	@@ -66,4 +66,26 @@ describe NoticeObserver do
66	66	end
67	67	end
68	68
	69	+ describe "should not send a notification if a notification service is not configured" do
	70	+ let(:app) { app = Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))}
	71	+ let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) }
	72	+
	73	+ before do
	74	+ Errbit::Config.per_app_email_at_notices = true
	75	+ end
	76	+
	77	+ after do
	78	+ Errbit::Config.per_app_email_at_notices = false
	79	+ end
	80	+
	81	+ it "should not create a campfire notification" do
	82	+ err.problem.stub(:notices_count) { 1 }
	83	+ app.stub!(:notification_recipients => %w('ryan@system88.com'))
	84	+ app.notification_service.should_not_receive(:create_notification)
	85	+
	86	+ Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'},
	87	+ :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' })
	88	+ end
	89	+ end
	90	+
69	91	end
...	...