Commit 54db230565fb109482cf36d6ffa431798f6fb3f7
Exists in
master
and in
23 other branches
Merge commit 'refs/merge-requests/95' of git://gitorious.org/noosfero/noosfero i…
…nto merge-requests/95 Since this patch is very old, I needed to fix the correction in the model a little. Also replaced the "profile2" variable name used to "author". Conflicts: app/models/comment.rb test/unit/comment_notifier_test.rb
Showing
2 changed files
with
27 additions
and
11 deletions
Show diff stats
app/models/comment.rb
| @@ -74,6 +74,10 @@ class Comment < ActiveRecord::Base | @@ -74,6 +74,10 @@ class Comment < ActiveRecord::Base | ||
| 74 | self.find(:all, :order => 'created_at desc, id desc', :limit => limit) | 74 | self.find(:all, :order => 'created_at desc, id desc', :limit => limit) |
| 75 | end | 75 | end |
| 76 | 76 | ||
| 77 | + def notification_emails | ||
| 78 | + self.article.profile.notification_emails - [self.author_email || self.email] | ||
| 79 | + end | ||
| 80 | + | ||
| 77 | after_save :notify_article | 81 | after_save :notify_article |
| 78 | after_destroy :notify_article | 82 | after_destroy :notify_article |
| 79 | def notify_article | 83 | def notify_article |
| @@ -114,7 +118,7 @@ class Comment < ActiveRecord::Base | @@ -114,7 +118,7 @@ class Comment < ActiveRecord::Base | ||
| 114 | 118 | ||
| 115 | def notify_by_mail | 119 | def notify_by_mail |
| 116 | if source.kind_of?(Article) && article.notify_comments? | 120 | if source.kind_of?(Article) && article.notify_comments? |
| 117 | - if !article.profile.notification_emails.empty? | 121 | + if !notification_emails.empty? |
| 118 | Comment::Notifier.deliver_mail(self) | 122 | Comment::Notifier.deliver_mail(self) |
| 119 | end | 123 | end |
| 120 | emails = article.followers - [author_email] | 124 | emails = article.followers - [author_email] |
| @@ -174,7 +178,7 @@ class Comment < ActiveRecord::Base | @@ -174,7 +178,7 @@ class Comment < ActiveRecord::Base | ||
| 174 | class Notifier < ActionMailer::Base | 178 | class Notifier < ActionMailer::Base |
| 175 | def mail(comment) | 179 | def mail(comment) |
| 176 | profile = comment.article.profile | 180 | profile = comment.article.profile |
| 177 | - recipients profile.notification_emails | 181 | + recipients comment.notification_emails |
| 178 | from "#{profile.environment.name} <#{profile.environment.contact_email}>" | 182 | from "#{profile.environment.name} <#{profile.environment.contact_email}>" |
| 179 | subject _("[%s] you got a new comment!") % [profile.environment.name] | 183 | subject _("[%s] you got a new comment!") % [profile.environment.name] |
| 180 | body :recipient => profile.nickname || profile.name, | 184 | body :recipient => profile.nickname || profile.name, |
test/unit/comment_notifier_test.rb
| @@ -8,26 +8,27 @@ class CommentNotifierTest < ActiveSupport::TestCase | @@ -8,26 +8,27 @@ class CommentNotifierTest < ActiveSupport::TestCase | ||
| 8 | ActionMailer::Base.delivery_method = :test | 8 | ActionMailer::Base.delivery_method = :test |
| 9 | ActionMailer::Base.perform_deliveries = true | 9 | ActionMailer::Base.perform_deliveries = true |
| 10 | ActionMailer::Base.deliveries = [] | 10 | ActionMailer::Base.deliveries = [] |
| 11 | - @profile = create_user('user_comment_test').person | 11 | + @profile = create_user('content_owner').person |
| 12 | + @author = create_user('author').person | ||
| 12 | @article = fast_create(Article, :name => 'Article test', :profile_id => @profile.id, :notify_comments => true) | 13 | @article = fast_create(Article, :name => 'Article test', :profile_id => @profile.id, :notify_comments => true) |
| 13 | end | 14 | end |
| 14 | 15 | ||
| 15 | should 'deliver mail after make an article comment' do | 16 | should 'deliver mail after make an article comment' do |
| 16 | assert_difference ActionMailer::Base.deliveries, :size do | 17 | assert_difference ActionMailer::Base.deliveries, :size do |
| 17 | - create_comment_and_notify(:author => @profile, :title => 'test comment', :body => 'you suck!', :source => @article ) | 18 | + create_comment_and_notify(:author => @author, :title => 'test comment', :body => 'you suck!', :source => @article ) |
| 18 | end | 19 | end |
| 19 | end | 20 | end |
| 20 | 21 | ||
| 21 | should 'deliver mail to owner of article' do | 22 | should 'deliver mail to owner of article' do |
| 22 | - create_comment_and_notify(:author => @profile, :title => 'test comment', :body => 'you suck!', :source => @article ) | 23 | + create_comment_and_notify(:author => @author, :title => 'test comment', :body => 'you suck!', :source => @article ) |
| 23 | sent = ActionMailer::Base.deliveries.first | 24 | sent = ActionMailer::Base.deliveries.first |
| 24 | assert_equal [@profile.email], sent.to | 25 | assert_equal [@profile.email], sent.to |
| 25 | end | 26 | end |
| 26 | 27 | ||
| 27 | should 'display author name in delivered mail' do | 28 | should 'display author name in delivered mail' do |
| 28 | - create_comment_and_notify(:author => @profile, :title => 'test comment', :body => 'you suck!', :source => @article) | 29 | + create_comment_and_notify(:author => @author, :title => 'test comment', :body => 'you suck!', :source => @article) |
| 29 | sent = ActionMailer::Base.deliveries.first | 30 | sent = ActionMailer::Base.deliveries.first |
| 30 | - assert_match /user_comment_test/, sent.body | 31 | + assert_match /#{@author.name}/, sent.body |
| 31 | end | 32 | end |
| 32 | 33 | ||
| 33 | should 'display unauthenticated author name and email in delivered mail' do | 34 | should 'display unauthenticated author name and email in delivered mail' do |
| @@ -40,18 +41,18 @@ class CommentNotifierTest < ActiveSupport::TestCase | @@ -40,18 +41,18 @@ class CommentNotifierTest < ActiveSupport::TestCase | ||
| 40 | should 'not deliver mail if notify comments is false' do | 41 | should 'not deliver mail if notify comments is false' do |
| 41 | @article.update_attribute(:notify_comments, false) | 42 | @article.update_attribute(:notify_comments, false) |
| 42 | assert_no_difference ActionMailer::Base.deliveries, :size do | 43 | assert_no_difference ActionMailer::Base.deliveries, :size do |
| 43 | - create_comment_and_notify(:author => @profile, :title => 'test comment', :body => 'you suck!', :source => @article) | 44 | + create_comment_and_notify(:author => @author, :title => 'test comment', :body => 'you suck!', :source => @article) |
| 44 | end | 45 | end |
| 45 | end | 46 | end |
| 46 | 47 | ||
| 47 | should 'include comment title in the e-mail' do | 48 | should 'include comment title in the e-mail' do |
| 48 | - create_comment_and_notify(:author => @profile, :title => 'comment title', :body => 'comment body', :source => @article) | 49 | + create_comment_and_notify(:author => @author, :title => 'comment title', :body => 'comment body', :source => @article) |
| 49 | sent = ActionMailer::Base.deliveries.first | 50 | sent = ActionMailer::Base.deliveries.first |
| 50 | assert_match /comment title/, sent.body | 51 | assert_match /comment title/, sent.body |
| 51 | end | 52 | end |
| 52 | 53 | ||
| 53 | should 'include comment text in the e-mail' do | 54 | should 'include comment text in the e-mail' do |
| 54 | - create_comment_and_notify(:author => @profile, :title => 'comment title', :body => 'comment body', :source => @article) | 55 | + create_comment_and_notify(:author => @author, :title => 'comment title', :body => 'comment body', :source => @article) |
| 55 | sent = ActionMailer::Base.deliveries.first | 56 | sent = ActionMailer::Base.deliveries.first |
| 56 | assert_match /comment body/, sent.body | 57 | assert_match /comment body/, sent.body |
| 57 | end | 58 | end |
| @@ -61,7 +62,7 @@ class CommentNotifierTest < ActiveSupport::TestCase | @@ -61,7 +62,7 @@ class CommentNotifierTest < ActiveSupport::TestCase | ||
| 61 | assert_equal [], community.notification_emails | 62 | assert_equal [], community.notification_emails |
| 62 | article = fast_create(Article, :name => 'Article test', :profile_id => community.id, :notify_comments => true) | 63 | article = fast_create(Article, :name => 'Article test', :profile_id => community.id, :notify_comments => true) |
| 63 | assert_no_difference ActionMailer::Base.deliveries, :size do | 64 | assert_no_difference ActionMailer::Base.deliveries, :size do |
| 64 | - create_comment_and_notify(:author => @profile, :title => 'test comment', :body => 'there is no addresses to send notification', :source => article) | 65 | + create_comment_and_notify(:author => @author, :title => 'test comment', :body => 'there is no addresses to send notification', :source => article) |
| 65 | end | 66 | end |
| 66 | end | 67 | end |
| 67 | 68 | ||
| @@ -80,6 +81,17 @@ class CommentNotifierTest < ActiveSupport::TestCase | @@ -80,6 +81,17 @@ class CommentNotifierTest < ActiveSupport::TestCase | ||
| 80 | assert_not_includes ActionMailer::Base.deliveries.map(&:bcc).flatten, follower.email | 81 | assert_not_includes ActionMailer::Base.deliveries.map(&:bcc).flatten, follower.email |
| 81 | end | 82 | end |
| 82 | 83 | ||
| 84 | + should 'not deliver mail to comments author' do | ||
| 85 | + community = fast_create(Community) | ||
| 86 | + community.add_admin @profile | ||
| 87 | + community.add_admin @author | ||
| 88 | + | ||
| 89 | + article = fast_create(Article, :name => 'Article test', :profile_id => community.id, :notify_comments => true) | ||
| 90 | + create_comment_and_notify(:source => @article, :author => @author, :title => 'comment title', :body => 'comment body') | ||
| 91 | + sent = ActionMailer::Base.deliveries.first | ||
| 92 | + assert_not_includes sent.to, @author.email | ||
| 93 | + end | ||
| 94 | + | ||
| 83 | private | 95 | private |
| 84 | 96 | ||
| 85 | def create_comment_and_notify(args) | 97 | def create_comment_and_notify(args) |