Commit 0ce6572c3aa877aff3b51ce30fbdd05b307f96ae
1 parent
d74fdaec
Exists in
master
and in
28 other branches
Do not send email to comment author
Noosfero sends an email to all admins of a commnunity when a new comment is posted. This commit removes the author of the comment from the recipient list. Additionally, the comment notifier tests were rewritten. The way it was implemented, the author of the article was always the same as the author of the comment. Now, these authors are different.
Showing
2 changed files
with
26 additions
and
11 deletions
Show diff stats
app/models/comment.rb
... | ... | @@ -67,6 +67,10 @@ class Comment < ActiveRecord::Base |
67 | 67 | self.find(:all, :order => 'created_at desc, id desc', :limit => limit) |
68 | 68 | end |
69 | 69 | |
70 | + def notification_emails | |
71 | + self.article.profile.notification_emails - [self.author_email || self.email] | |
72 | + end | |
73 | + | |
70 | 74 | after_save :notify_article |
71 | 75 | after_destroy :notify_article |
72 | 76 | def notify_article |
... | ... | @@ -74,7 +78,7 @@ class Comment < ActiveRecord::Base |
74 | 78 | end |
75 | 79 | |
76 | 80 | after_create do |comment| |
77 | - if comment.article.notify_comments? && !comment.article.profile.notification_emails.empty? | |
81 | + if comment.article.notify_comments? && !comment.notification_emails.empty? | |
78 | 82 | Comment::Notifier.deliver_mail(comment) |
79 | 83 | end |
80 | 84 | end |
... | ... | @@ -111,7 +115,7 @@ class Comment < ActiveRecord::Base |
111 | 115 | class Notifier < ActionMailer::Base |
112 | 116 | def mail(comment) |
113 | 117 | profile = comment.article.profile |
114 | - recipients profile.notification_emails | |
118 | + recipients comment.notification_emails | |
115 | 119 | from "#{profile.environment.name} <#{profile.environment.contact_email}>" |
116 | 120 | subject _("[%s] you got a new comment!") % [profile.environment.name] |
117 | 121 | body :recipient => profile.nickname || profile.name, | ... | ... |
test/unit/comment_notifier_test.rb
... | ... | @@ -9,23 +9,24 @@ class CommentNotifierTest < Test::Unit::TestCase |
9 | 9 | ActionMailer::Base.perform_deliveries = true |
10 | 10 | ActionMailer::Base.deliveries = [] |
11 | 11 | @profile = create_user('user_comment_test').person |
12 | + @profile2 = create_user('user_comment_test2').person | |
12 | 13 | @article = fast_create(Article, :name => 'Article test', :profile_id => @profile.id, :notify_comments => true) |
13 | 14 | end |
14 | 15 | |
15 | 16 | should 'deliver mail after make aarticle commment' do |
16 | 17 | assert_difference ActionMailer::Base.deliveries, :size do |
17 | - @article.comments << Comment.new(:author => @profile, :title => 'test comment', :body => 'you suck!') | |
18 | + @article.comments << Comment.new(:author => @profile2, :title => 'test comment', :body => 'you suck!') | |
18 | 19 | end |
19 | 20 | end |
20 | 21 | |
21 | - should 'deliver mail to owner of article' do | |
22 | - @article.comments << Comment.new(:author => @profile, :title => 'test comment', :body => 'you suck!') | |
22 | + should 'deliver mail to owner of article with a another person' do | |
23 | + @article.comments << Comment.new(:author => @profile2, :title => 'test comment', :body => 'you suck!') | |
23 | 24 | sent = ActionMailer::Base.deliveries.first |
24 | 25 | assert_equal [@profile.email], sent.to |
25 | 26 | end |
26 | 27 | |
27 | 28 | should 'display author name in delivered mail' do |
28 | - @article.comments << Comment.new(:author => @profile, :title => 'test comment', :body => 'you suck!') | |
29 | + @article.comments << Comment.new(:author => @profile2, :title => 'test comment', :body => 'you suck!') | |
29 | 30 | sent = ActionMailer::Base.deliveries.first |
30 | 31 | assert_match /user_comment_test/, sent.body |
31 | 32 | end |
... | ... | @@ -40,18 +41,18 @@ class CommentNotifierTest < Test::Unit::TestCase |
40 | 41 | should 'not deliver mail if notify comments is false' do |
41 | 42 | @article.update_attribute(:notify_comments, false) |
42 | 43 | assert_no_difference ActionMailer::Base.deliveries, :size do |
43 | - @article.comments << Comment.new(:author => @profile, :title => 'test comment', :body => 'you suck!') | |
44 | + @article.comments << Comment.new(:author => @profile2, :title => 'test comment', :body => 'you suck!') | |
44 | 45 | end |
45 | 46 | end |
46 | 47 | |
47 | 48 | should 'include comment title in the e-mail' do |
48 | - @article.comments << Comment.new(:author => @profile, :title => 'comment title', :body => 'comment title') | |
49 | + @article.comments << Comment.new(:author => @profile2, :title => 'comment title', :body => 'comment title') | |
49 | 50 | sent = ActionMailer::Base.deliveries.first |
50 | 51 | assert_match /comment title/, sent.body |
51 | 52 | end |
52 | 53 | |
53 | 54 | should 'include comment text in the e-mail' do |
54 | - @article.comments << Comment.new(:author => @profile, :title => 'comment title', :body => 'comment body') | |
55 | + @article.comments << Comment.new(:author => @profile2, :title => 'comment title', :body => 'comment body') | |
55 | 56 | sent = ActionMailer::Base.deliveries.first |
56 | 57 | assert_match /comment body/, sent.body |
57 | 58 | end |
... | ... | @@ -65,12 +66,22 @@ class CommentNotifierTest < Test::Unit::TestCase |
65 | 66 | end |
66 | 67 | end |
67 | 68 | |
68 | - private | |
69 | + should 'not deliver mail to comments author' do | |
70 | + community = fast_create(Community) | |
71 | + community.add_admin @profile | |
72 | + community.add_admin @profile2 | |
73 | + | |
74 | + article = fast_create(Article, :name => 'Article test', :profile_id => community.id, :notify_comments => true) | |
75 | + article.comments << Comment.new(:author => @profile, :title => 'test comment', :body => 'there is no addresses to send notification') | |
76 | + sent = ActionMailer::Base.deliveries.first | |
77 | + assert_not_includes sent.to, @profile.email | |
78 | + end | |
79 | + | |
80 | + private | |
69 | 81 | |
70 | 82 | def read_fixture(action) |
71 | 83 | IO.readlines("#{FIXTURES_PATH}/mail_sender/#{action}") |
72 | 84 | end |
73 | - | |
74 | 85 | def encode(subject) |
75 | 86 | quoted_printable(subject, CHARSET) |
76 | 87 | end | ... | ... |