Commit dff781c579f22c8ebf56a988b4ddcfcdcdfce262
1 parent
df46012f
Exists in
master
and in
28 other branches
Pluggable SPAM handling system
ActionItem2306
Showing
4 changed files
with
92 additions
and
4 deletions
Show diff stats
app/models/comment.rb
| @@ -94,6 +94,18 @@ class Comment < ActiveRecord::Base | @@ -94,6 +94,18 @@ class Comment < ActiveRecord::Base | ||
| 94 | Delayed::Job.enqueue CommentHandler.new(self.id) | 94 | Delayed::Job.enqueue CommentHandler.new(self.id) |
| 95 | end | 95 | end |
| 96 | 96 | ||
| 97 | + delegate :environment, :to => :profile | ||
| 98 | + delegate :profile, :to => :source | ||
| 99 | + | ||
| 100 | + include Noosfero::Plugin::HotSpot | ||
| 101 | + | ||
| 102 | + def verify_and_notify | ||
| 103 | + plugins.dispatch(:check_comment_for_spam, self) | ||
| 104 | + unless spam? | ||
| 105 | + notify_by_mail | ||
| 106 | + end | ||
| 107 | + end | ||
| 108 | + | ||
| 97 | def notify_by_mail | 109 | def notify_by_mail |
| 98 | if source.kind_of?(Article) && article.notify_comments? | 110 | if source.kind_of?(Article) && article.notify_comments? |
| 99 | if !article.profile.notification_emails.empty? | 111 | if !article.profile.notification_emails.empty? |
| @@ -202,11 +214,15 @@ class Comment < ActiveRecord::Base | @@ -202,11 +214,15 @@ class Comment < ActiveRecord::Base | ||
| 202 | def spam! | 214 | def spam! |
| 203 | self.spam = true | 215 | self.spam = true |
| 204 | self.save! | 216 | self.save! |
| 217 | + plugins.dispatch(:comment_marked_as_spam, self) | ||
| 218 | + self | ||
| 205 | end | 219 | end |
| 206 | 220 | ||
| 207 | def ham! | 221 | def ham! |
| 208 | self.spam = false | 222 | self.spam = false |
| 209 | self.save! | 223 | self.save! |
| 224 | + plugins.dispatch(:comment_marked_as_ham, self) | ||
| 225 | + self | ||
| 210 | end | 226 | end |
| 211 | 227 | ||
| 212 | end | 228 | end |
app/models/comment_handler.rb
| @@ -2,7 +2,7 @@ class CommentHandler < Struct.new(:comment_id) | @@ -2,7 +2,7 @@ class CommentHandler < Struct.new(:comment_id) | ||
| 2 | 2 | ||
| 3 | def perform | 3 | def perform |
| 4 | comment = Comment.find(comment_id) | 4 | comment = Comment.find(comment_id) |
| 5 | - comment.notify_by_mail | 5 | + comment.verify_and_notify |
| 6 | rescue ActiveRecord::RecordNotFound | 6 | rescue ActiveRecord::RecordNotFound |
| 7 | # just ignore non-existing comments | 7 | # just ignore non-existing comments |
| 8 | end | 8 | end |
test/unit/comment_handler_test.rb
| @@ -16,7 +16,7 @@ class CommentHandlerTest < ActiveSupport::TestCase | @@ -16,7 +16,7 @@ class CommentHandlerTest < ActiveSupport::TestCase | ||
| 16 | handler = CommentHandler.new(-1) | 16 | handler = CommentHandler.new(-1) |
| 17 | comment = Comment.new | 17 | comment = Comment.new |
| 18 | Comment.stubs(:find).with(-1).returns(comment) | 18 | Comment.stubs(:find).with(-1).returns(comment) |
| 19 | - comment.expects(:notify_by_mail) | 19 | + comment.expects(:verify_and_notify) |
| 20 | 20 | ||
| 21 | handler.perform | 21 | handler.perform |
| 22 | end | 22 | end |
test/unit/comment_test.rb
| @@ -446,7 +446,7 @@ class CommentTest < ActiveSupport::TestCase | @@ -446,7 +446,7 @@ class CommentTest < ActiveSupport::TestCase | ||
| 446 | end | 446 | end |
| 447 | 447 | ||
| 448 | should 'be able to mark as spam atomically' do | 448 | should 'be able to mark as spam atomically' do |
| 449 | - c1 = fast_create(Comment, :name => 'foo', :email => 'foo@example.com') | 449 | + c1 = create_comment |
| 450 | c1.spam! | 450 | c1.spam! |
| 451 | c1.reload | 451 | c1.reload |
| 452 | assert c1.spam? | 452 | assert c1.spam? |
| @@ -461,10 +461,82 @@ class CommentTest < ActiveSupport::TestCase | @@ -461,10 +461,82 @@ class CommentTest < ActiveSupport::TestCase | ||
| 461 | end | 461 | end |
| 462 | 462 | ||
| 463 | should 'be able to mark as ham atomically' do | 463 | should 'be able to mark as ham atomically' do |
| 464 | - c1 = fast_create(Comment, :name => 'foo', :email => 'foo@example.com', :spam => true) | 464 | + c1 = create_comment |
| 465 | c1.ham! | 465 | c1.ham! |
| 466 | c1.reload | 466 | c1.reload |
| 467 | assert c1.ham? | 467 | assert c1.ham? |
| 468 | end | 468 | end |
| 469 | 469 | ||
| 470 | + should 'notify by email' do | ||
| 471 | + c1 = create_comment | ||
| 472 | + c1.expects(:notify_by_mail) | ||
| 473 | + c1.verify_and_notify | ||
| 474 | + end | ||
| 475 | + | ||
| 476 | + should 'not notify by email when comment is spam' do | ||
| 477 | + c1 = create_comment(:spam => true) | ||
| 478 | + c1.expects(:notify_by_mail).never | ||
| 479 | + c1.verify_and_notify | ||
| 480 | + end | ||
| 481 | + | ||
| 482 | + class EverythingIsSpam < Noosfero::Plugin | ||
| 483 | + def check_comment_for_spam(comment) | ||
| 484 | + comment.spam! | ||
| 485 | + end | ||
| 486 | + end | ||
| 487 | + | ||
| 488 | + | ||
| 489 | + should 'delegate spam detection to plugins' do | ||
| 490 | + Environment.default.enable_plugin(EverythingIsSpam) | ||
| 491 | + | ||
| 492 | + c1 = create_comment | ||
| 493 | + | ||
| 494 | + c1.expects(:notify_by_mail).never | ||
| 495 | + | ||
| 496 | + c1.verify_and_notify | ||
| 497 | + end | ||
| 498 | + | ||
| 499 | + class SpamNotification < Noosfero::Plugin | ||
| 500 | + class << self | ||
| 501 | + attr_accessor :marked_as_spam | ||
| 502 | + attr_accessor :marked_as_ham | ||
| 503 | + end | ||
| 504 | + | ||
| 505 | + def comment_marked_as_spam(c) | ||
| 506 | + self.class.marked_as_spam = c | ||
| 507 | + end | ||
| 508 | + | ||
| 509 | + def comment_marked_as_ham(c) | ||
| 510 | + self.class.marked_as_ham = c | ||
| 511 | + end | ||
| 512 | + end | ||
| 513 | + | ||
| 514 | + should 'notify plugins of comments being marked as spam' do | ||
| 515 | + Environment.default.enable_plugin(SpamNotification) | ||
| 516 | + | ||
| 517 | + c = create_comment | ||
| 518 | + | ||
| 519 | + c.spam! | ||
| 520 | + | ||
| 521 | + assert_equal c, SpamNotification.marked_as_spam | ||
| 522 | + end | ||
| 523 | + | ||
| 524 | + should 'notify plugins of comments being marked as ham' do | ||
| 525 | + Environment.default.enable_plugin(SpamNotification) | ||
| 526 | + | ||
| 527 | + c = create_comment | ||
| 528 | + | ||
| 529 | + c.ham! | ||
| 530 | + | ||
| 531 | + assert_equal c, SpamNotification.marked_as_ham | ||
| 532 | + end | ||
| 533 | + | ||
| 534 | + private | ||
| 535 | + | ||
| 536 | + def create_comment(args = {}) | ||
| 537 | + owner = create_user('testuser').person | ||
| 538 | + article = create(TextileArticle, :profile_id => owner.id) | ||
| 539 | + create(Comment, { :name => 'foo', :email => 'foo@example.com', :source => article }.merge(args)) | ||
| 540 | + end | ||
| 541 | + | ||
| 470 | end | 542 | end |