Commit eb86a96443d743f02dbbd4393bee48269c4eda79
1 parent
13bd4e38
Exists in
master
and in
22 other branches
spammable: encapsulate the spam logic to a module
Showing
5 changed files
with
54 additions
and
49 deletions
Show diff stats
app/models/comment.rb
| @@ -16,9 +16,7 @@ class Comment < ActiveRecord::Base | @@ -16,9 +16,7 @@ class Comment < ActiveRecord::Base | ||
| 16 | has_many :children, :class_name => 'Comment', :foreign_key => 'reply_of_id', :dependent => :destroy | 16 | has_many :children, :class_name => 'Comment', :foreign_key => 'reply_of_id', :dependent => :destroy |
| 17 | belongs_to :reply_of, :class_name => 'Comment', :foreign_key => 'reply_of_id' | 17 | belongs_to :reply_of, :class_name => 'Comment', :foreign_key => 'reply_of_id' |
| 18 | 18 | ||
| 19 | - named_scope :without_spam, :conditions => ['spam IS NULL OR spam = ?', false] | ||
| 20 | named_scope :without_reply, :conditions => ['reply_of_id IS NULL'] | 19 | named_scope :without_reply, :conditions => ['reply_of_id IS NULL'] |
| 21 | - named_scope :spam, :conditions => ['spam = ?', true] | ||
| 22 | 20 | ||
| 23 | # unauthenticated authors: | 21 | # unauthenticated authors: |
| 24 | validates_presence_of :name, :if => (lambda { |record| !record.email.blank? }) | 22 | validates_presence_of :name, :if => (lambda { |record| !record.email.blank? }) |
| @@ -205,27 +203,15 @@ class Comment < ActiveRecord::Base | @@ -205,27 +203,15 @@ class Comment < ActiveRecord::Base | ||
| 205 | @rejected = true | 203 | @rejected = true |
| 206 | end | 204 | end |
| 207 | 205 | ||
| 208 | - def spam? | ||
| 209 | - !spam.nil? && spam | ||
| 210 | - end | ||
| 211 | - | ||
| 212 | - def ham? | ||
| 213 | - !spam.nil? && !spam | ||
| 214 | - end | 206 | + include Spammable |
| 215 | 207 | ||
| 216 | - def spam! | ||
| 217 | - self.spam = true | ||
| 218 | - self.save! | 208 | + def after_spam! |
| 219 | SpammerLogger.log(ip_address, self) | 209 | SpammerLogger.log(ip_address, self) |
| 220 | Delayed::Job.enqueue(CommentHandler.new(self.id, :marked_as_spam)) | 210 | Delayed::Job.enqueue(CommentHandler.new(self.id, :marked_as_spam)) |
| 221 | - self | ||
| 222 | end | 211 | end |
| 223 | 212 | ||
| 224 | - def ham! | ||
| 225 | - self.spam = false | ||
| 226 | - self.save! | 213 | + def after_ham! |
| 227 | Delayed::Job.enqueue(CommentHandler.new(self.id, :marked_as_ham)) | 214 | Delayed::Job.enqueue(CommentHandler.new(self.id, :marked_as_ham)) |
| 228 | - self | ||
| 229 | end | 215 | end |
| 230 | 216 | ||
| 231 | def marked_as_spam | 217 | def marked_as_spam |
app/models/suggest_article.rb
| @@ -76,17 +76,13 @@ class SuggestArticle < Task | @@ -76,17 +76,13 @@ class SuggestArticle < Task | ||
| 76 | _('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name } | 76 | _('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name } |
| 77 | end | 77 | end |
| 78 | 78 | ||
| 79 | - def spam! | ||
| 80 | - super | 79 | + def after_spam! |
| 81 | SpammerLogger.log(ip_address, self) | 80 | SpammerLogger.log(ip_address, self) |
| 82 | self.delay.marked_as_spam | 81 | self.delay.marked_as_spam |
| 83 | - self | ||
| 84 | end | 82 | end |
| 85 | 83 | ||
| 86 | - def ham! | ||
| 87 | - super | 84 | + def after_ham! |
| 88 | self.delay.marked_as_ham | 85 | self.delay.marked_as_ham |
| 89 | - self | ||
| 90 | end | 86 | end |
| 91 | 87 | ||
| 92 | def marked_as_spam | 88 | def marked_as_spam |
app/models/task.rb
| @@ -235,25 +235,7 @@ class Task < ActiveRecord::Base | @@ -235,25 +235,7 @@ class Task < ActiveRecord::Base | ||
| 235 | end | 235 | end |
| 236 | end | 236 | end |
| 237 | 237 | ||
| 238 | - def spam? | ||
| 239 | - !spam.nil? && spam | ||
| 240 | - end | ||
| 241 | - | ||
| 242 | - def ham? | ||
| 243 | - !spam.nil? && !spam | ||
| 244 | - end | ||
| 245 | - | ||
| 246 | - def spam! | ||
| 247 | - self.spam = true | ||
| 248 | - self.save! | ||
| 249 | - self | ||
| 250 | - end | ||
| 251 | - | ||
| 252 | - def ham! | ||
| 253 | - self.spam = false | ||
| 254 | - self.save! | ||
| 255 | - self | ||
| 256 | - end | 238 | + include Spammable |
| 257 | 239 | ||
| 258 | protected | 240 | protected |
| 259 | 241 | ||
| @@ -294,8 +276,6 @@ class Task < ActiveRecord::Base | @@ -294,8 +276,6 @@ class Task < ActiveRecord::Base | ||
| 294 | named_scope :opened, :conditions => { :status => [Task::Status::ACTIVE, Task::Status::HIDDEN] } | 276 | named_scope :opened, :conditions => { :status => [Task::Status::ACTIVE, Task::Status::HIDDEN] } |
| 295 | named_scope :of, lambda { |type| conditions = type ? "type LIKE '#{type}'" : "1=1"; {:conditions => [conditions]} } | 277 | named_scope :of, lambda { |type| conditions = type ? "type LIKE '#{type}'" : "1=1"; {:conditions => [conditions]} } |
| 296 | named_scope :order_by, lambda { |attribute, ord| {:order => "#{attribute} #{ord}"} } | 278 | named_scope :order_by, lambda { |attribute, ord| {:order => "#{attribute} #{ord}"} } |
| 297 | - named_scope :without_spam, :conditions => ['spam IS NULL OR spam = ?', false] | ||
| 298 | - named_scope :spam, :conditions => ['spam = ?', true] | ||
| 299 | 279 | ||
| 300 | 280 | ||
| 301 | named_scope :to, lambda { |profile| | 281 | named_scope :to, lambda { |profile| |
| @@ -0,0 +1,44 @@ | @@ -0,0 +1,44 @@ | ||
| 1 | +module Spammable | ||
| 2 | + def self.included(recipient) | ||
| 3 | + recipient.extend(ClassMethods) | ||
| 4 | + end | ||
| 5 | + | ||
| 6 | + module ClassMethods | ||
| 7 | + def self.extended (base) | ||
| 8 | + base.class_eval do | ||
| 9 | + named_scope :without_spam, :conditions => ['spam IS NULL OR spam = ?', false] | ||
| 10 | + named_scope :spam, :conditions => ['spam = ?', true] | ||
| 11 | + end | ||
| 12 | + end | ||
| 13 | + end | ||
| 14 | + | ||
| 15 | + def spam? | ||
| 16 | + !spam.nil? && spam | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | + def ham? | ||
| 20 | + !spam.nil? && !spam | ||
| 21 | + end | ||
| 22 | + | ||
| 23 | + def spam! | ||
| 24 | + before_spam! | ||
| 25 | + self.spam = true | ||
| 26 | + self.save! | ||
| 27 | + after_spam! | ||
| 28 | + self | ||
| 29 | + end | ||
| 30 | + | ||
| 31 | + def ham! | ||
| 32 | + before_ham! | ||
| 33 | + self.spam = false | ||
| 34 | + self.save! | ||
| 35 | + after_ham! | ||
| 36 | + self | ||
| 37 | + end | ||
| 38 | + | ||
| 39 | + def after_spam!; end | ||
| 40 | + def before_spam!; end | ||
| 41 | + | ||
| 42 | + def after_ham!; end | ||
| 43 | + def before_ham!; end | ||
| 44 | +end |
plugins/anti_spam/test/unit/anti_spam_plugin_test.rb
| @@ -2,24 +2,23 @@ require 'test_helper' | @@ -2,24 +2,23 @@ require 'test_helper' | ||
| 2 | 2 | ||
| 3 | class AntiSpamPluginTest < ActiveSupport::TestCase | 3 | class AntiSpamPluginTest < ActiveSupport::TestCase |
| 4 | 4 | ||
| 5 | - class Spammable | 5 | + class SpammableContent |
| 6 | attr_accessor :spam | 6 | attr_accessor :spam |
| 7 | + include Spammable | ||
| 7 | 8 | ||
| 8 | def save!; end | 9 | def save!; end |
| 9 | - def spam!; end | ||
| 10 | - def ham!; end | ||
| 11 | - def spam?; true; end | ||
| 12 | def environment; Environment.default; end | 10 | def environment; Environment.default; end |
| 13 | end | 11 | end |
| 14 | 12 | ||
| 15 | def setup | 13 | def setup |
| 16 | - @spammable = Spammable.new | 14 | + @spammable = SpammableContent.new |
| 17 | @plugin = AntiSpamPlugin.new | 15 | @plugin = AntiSpamPlugin.new |
| 18 | end | 16 | end |
| 19 | 17 | ||
| 20 | attr_accessor :spammable | 18 | attr_accessor :spammable |
| 21 | 19 | ||
| 22 | should 'check for spam and mark as spam if server says it is spam' do | 20 | should 'check for spam and mark as spam if server says it is spam' do |
| 21 | + spammable.expects(:spam?).returns(true) | ||
| 23 | spammable.expects(:save!) | 22 | spammable.expects(:save!) |
| 24 | 23 | ||
| 25 | @plugin.check_for_spam(spammable) | 24 | @plugin.check_for_spam(spammable) |