Download as
Browse Code »

Commit 6baaac4cb37dd27ef5c913ec519be56c3e9a3ab7

Authored by Alvaro Bautista
1 parent 59a1c553
Exists in master and in 1 other branch production

Comment emailable logic to decide when to send notifications 

Test coverage for comment observer

[Fixes #452]
Showing 5 changed files with 99 additions and 3 deletions   Show diff stats
app/models/comment.rb
  Diff comments   View file @6baaac4
@@ -14,6 +14,14 @@ class Comment @@ -14,6 +14,14 @@ class Comment
14 14
15 validates_presence_of :body 15 validates_presence_of :body
16 16
  17 + def notification_recipients
  18 + app.notification_recipients - [user.email]
  19 + end
  20 +
  21 + def emailable?
  22 + app.emailable? && notification_recipients.any?
  23 + end
  24 +
17 protected 25 protected
18 def increase_counter_cache 26 def increase_counter_cache
19 err.inc(:comments_count, 1) 27 err.inc(:comments_count, 1)
@@ -24,4 +32,3 @@ class Comment @@ -24,4 +32,3 @@ class Comment
24 end 32 end
25 33
26 end 34 end
27 -  
app/models/comment_observer.rb
  Diff comments   View file @6baaac4
@@ -2,7 +2,7 @@ class CommentObserver < Mongoid::Observer @@ -2,7 +2,7 @@ class CommentObserver < Mongoid::Observer
2 observe :comment 2 observe :comment
3 3
4 def after_create(comment) 4 def after_create(comment)
5 - Mailer.comment_notification(comment).deliver if comment.app.emailable? 5 + Mailer.comment_notification(comment).deliver if comment.emailable?
6 end 6 end
7 7
8 end 8 end
spec/models/app_spec.rb
  Diff comments   View file @6baaac4
@@ -124,6 +124,25 @@ describe App do @@ -124,6 +124,25 @@ describe App do
124 end 124 end
125 end 125 end
126 126
  127 + context "emailable?" do
  128 + it "should be true if notify on errs and there are notification recipients" do
  129 + app = Fabricate(:app, :notify_on_errs => true, :notify_all_users => false)
  130 + 2.times { Fabricate(:watcher, :app => app) }
  131 + app.emailable?.should be_true
  132 + end
  133 +
  134 + it "should be false if notify on errs is disabled" do
  135 + app = Fabricate(:app, :notify_on_errs => false, :notify_all_users => false)
  136 + 2.times { Fabricate(:watcher, :app => app) }
  137 + app.emailable?.should be_false
  138 + end
  139 +
  140 + it "should be false if there are no notification recipients" do
  141 + app = Fabricate(:app, :notify_on_errs => true, :notify_all_users => false)
  142 + app.watchers.should be_empty
  143 + app.emailable?.should be_false
  144 + end
  145 + end
127 146
128 context "copying attributes from existing app" do 147 context "copying attributes from existing app" do
129 it "should only copy the necessary fields" do 148 it "should only copy the necessary fields" do
spec/models/comment_observer_spec.rb 0 → 100644
  Diff comments   View file @6baaac4
@@ -0,0 +1,27 @@ @@ -0,0 +1,27 @@
  1 +require 'spec_helper'
  2 +
  3 +describe CommentObserver do
  4 + context 'when a Comment is saved' do
  5 + let(:comment) { Fabricate.build(:comment) }
  6 +
  7 + context 'and it is emailable?' do
  8 + before { comment.stub(:emailable?).and_return(true) }
  9 +
  10 + it 'should send an email notification' do
  11 + Mailer.should_receive(:comment_notification).
  12 + with(comment).
  13 + and_return(mock('email', :deliver => true))
  14 + comment.save
  15 + end
  16 + end
  17 +
  18 + context 'and it is not emailable?' do
  19 + before { comment.stub(:emailable?).and_return(false) }
  20 +
  21 + it 'should not send an email notification' do
  22 + Mailer.should_not_receive(:comment_notification)
  23 + comment.save
  24 + end
  25 + end
  26 + end
  27 +end
spec/models/comment_spec.rb
  Diff comments   View file @6baaac4
@@ -8,5 +8,48 @@ describe Comment do @@ -8,5 +8,48 @@ describe Comment do
8 comment.errors[:body].should include("can't be blank") 8 comment.errors[:body].should include("can't be blank")
9 end 9 end
10 end 10 end
11 -end  
12 11
  12 + context 'notification_recipients' do
  13 + let(:app) { Fabricate(:app) }
  14 + let!(:watcher) { Fabricate(:watcher, :app => app) }
  15 + let(:err) { Fabricate(:problem, :app => app) }
  16 + let(:comment_user) { Fabricate(:user, :email => 'author@example.com') }
  17 + let(:comment) { Fabricate.build(:comment, :err => err, :user => comment_user) }
  18 +
  19 + before do
  20 + Fabricate(:user_watcher, :app => app, :user => comment_user)
  21 + end
  22 +
  23 + it 'includes app notification_recipients except user email' do
  24 + comment.notification_recipients.should == [watcher.address]
  25 + end
  26 + end
  27 +
  28 + context 'emailable?' do
  29 + let(:app) { Fabricate(:app, :notify_on_errs => true) }
  30 + let!(:watcher) { Fabricate(:watcher, :app => app) }
  31 + let(:err) { Fabricate(:problem, :app => app) }
  32 + let(:comment_user) { Fabricate(:user, :email => 'author@example.com') }
  33 + let(:comment) { Fabricate.build(:comment, :err => err, :user => comment_user) }
  34 +
  35 + before do
  36 + Fabricate(:user_watcher, :app => app, :user => comment_user)
  37 + end
  38 +
  39 + it 'should be true if app is emailable? and there are notification recipients' do
  40 + comment.emailable?.should be_true
  41 + end
  42 +
  43 + it 'should be false if app is not emailable?' do
  44 + app.update_attribute(:notify_on_errs, false)
  45 + comment.notification_recipients.should be_any
  46 + comment.emailable?.should be_false
  47 + end
  48 +
  49 + it 'should be false if there are no notification recipients' do
  50 + watcher.destroy
  51 + app.emailable?.should be_true
  52 + comment.emailable?.should be_false
  53 + end
  54 + end
  55 +end