From 9f6485771939519190d1971bfa02a2bb1aca6746 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 4 May 2017 14:58:03 -0500 Subject: [PATCH 1/4] flags: move status locked check to post_flag.rb. --- app/models/post.rb | 4 ---- app/models/post_flag.rb | 9 ++++----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 9906baff8..0fd17a293 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -296,10 +296,6 @@ class Post < ActiveRecord::Base end def flag!(reason, options = {}) - if is_status_locked? - raise PostFlag::Error.new("Post is locked and cannot be flagged") - end - flag = flags.create(:reason => reason, :is_resolved => false, :is_deletion => options[:is_deletion]) if flag.errors.any? diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 1bd137f1e..17a625653 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -11,7 +11,7 @@ class PostFlag < ActiveRecord::Base belongs_to :post validates_presence_of :reason, :creator_id, :creator_ip_addr validate :validate_creator_is_not_limited - validate :validate_post_is_active + validate :validate_post before_validation :initialize_creator, :on => :create validates_uniqueness_of :creator_id, :scope => :post_id, :on => :create, :unless => :is_deletion, :message => "have already flagged this post" before_save :update_post @@ -146,10 +146,9 @@ class PostFlag < ActiveRecord::Base end end - def validate_post_is_active - if post.is_deleted? - errors[:post] << "is deleted" - end + def validate_post + errors[:post] << "is locked and cannot be flagged" if post.is_status_locked? + errors[:post] << "is deleted" if post.is_deleted? end def initialize_creator From 8638b7527bad7d8c13cdc0a8146f51cd67494490 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 4 May 2017 15:05:02 -0500 Subject: [PATCH 2/4] flags: don't set is_flagged twice. PostFlag#update_post already sets is_flagged when the flag is created. No need to set it again. --- app/models/post.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 0fd17a293..a093080e3 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -301,8 +301,6 @@ class Post < ActiveRecord::Base if flag.errors.any? raise PostFlag::Error.new(flag.errors.full_messages.join("; ")) end - - update_column(:is_flagged, true) unless is_flagged? end def appeal!(reason) From 3b37bb6142b536e9ec2e99f2f0f39ea459da0b27 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 4 May 2017 16:40:09 -0500 Subject: [PATCH 3/4] flags: add 3 day flagging cooldown period. --- app/models/post_flag.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 17a625653..a4f0ff31a 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -7,6 +7,8 @@ class PostFlag < ActiveRecord::Base BANNED = "Artist requested removal" end + COOLDOWN_PERIOD = 3.days + belongs_to :creator, :class_name => "User" belongs_to :post validates_presence_of :reason, :creator_id, :creator_ip_addr @@ -18,6 +20,10 @@ class PostFlag < ActiveRecord::Base attr_accessible :post, :post_id, :reason, :is_resolved, :is_deletion attr_accessor :is_deletion + scope :by_users, lambda { where.not(creator: User.system) } + scope :by_system, lambda { where(creator: User.system) } + scope :in_cooldown, lambda { by_users.where("created_at >= ?", COOLDOWN_PERIOD.ago) } + module SearchMethods def reason_matches(query) if query =~ /\*/ @@ -149,6 +155,11 @@ class PostFlag < ActiveRecord::Base def validate_post errors[:post] << "is locked and cannot be flagged" if post.is_status_locked? errors[:post] << "is deleted" if post.is_deleted? + + flag = post.flags.in_cooldown.last + if flag.present? + errors[:post] << "cannot be flagged more than once every #{COOLDOWN_PERIOD.inspect} (last flagged: #{flag.created_at.to_s(:long)})" + end end def initialize_creator From 31a38ea39a58d46293b6eda2306dc4ee560e9fb4 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 4 May 2017 16:39:47 -0500 Subject: [PATCH 4/4] flags: add flag cooldown test. --- app/models/post_flag.rb | 4 ++-- test/unit/post_approval_test.rb | 4 +++- test/unit/post_flag_test.rb | 19 ++++++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index a4f0ff31a..6c6643eca 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -163,8 +163,8 @@ class PostFlag < ActiveRecord::Base end def initialize_creator - self.creator_id = CurrentUser.id - self.creator_ip_addr = CurrentUser.ip_addr + self.creator_id ||= CurrentUser.id + self.creator_ip_addr ||= CurrentUser.ip_addr end def resolve! diff --git a/test/unit/post_approval_test.rb b/test/unit/post_approval_test.rb index c718feb37..dbb7343a9 100644 --- a/test/unit/post_approval_test.rb +++ b/test/unit/post_approval_test.rb @@ -49,7 +49,9 @@ class PostApprovalTest < ActiveSupport::TestCase @post.approve!(@approver2) assert_not_equal(@approver.id, @post.approver_id) CurrentUser.user = @user3 - @post.flag!("blah blah") + travel_to(PostFlag::COOLDOWN_PERIOD.from_now + 1.minute) do + @post.flag!("blah blah") + end approval = @post.approve!(@approver) assert_includes(approval.errors.full_messages, "You have previously approved this post and cannot approve it again") diff --git a/test/unit/post_flag_test.rb b/test/unit/post_flag_test.rb index 2e11ee37c..91d92d095 100644 --- a/test/unit/post_flag_test.rb +++ b/test/unit/post_flag_test.rb @@ -44,7 +44,7 @@ class PostFlagTest < ActiveSupport::TestCase @post_flag = PostFlag.create(:post => @post, :reason => "aaa", :is_resolved => false) end - assert_equal(["You have already flagged this post"], @post_flag.errors.full_messages) + assert_equal(["have already flagged this post"], @post_flag.errors[:creator_id]) end should "not be able to flag more than 10 posts in 24 hours" do @@ -64,6 +64,23 @@ class PostFlagTest < ActiveSupport::TestCase assert_equal(["Post is deleted"], @post_flag.errors.full_messages) end + should "not be able to flag a post in the cooldown period" do + users = FactoryGirl.create_list(:user, 2, created_at: 2.weeks.ago) + flag1 = FactoryGirl.create(:post_flag, post: @post, creator: users.first) + @post.approve! + + travel_to(PostFlag::COOLDOWN_PERIOD.from_now - 1.minute) do + flag2 = FactoryGirl.build(:post_flag, post: @post, creator: users.second) + assert(flag2.invalid?) + assert_match(/cannot be flagged more than once/, flag2.errors[:post].join) + end + + travel_to(PostFlag::COOLDOWN_PERIOD.from_now + 1.minute) do + flag3 = FactoryGirl.build(:post_flag, post: @post, creator: users.second) + assert(flag3.valid?) + end + end + should "initialize its creator" do @post_flag = PostFlag.create(:post => @post, :reason => "aaa", :is_resolved => false) assert_equal(@alice.id, @post_flag.creator_id)