diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 4dcbd86df..5ea127be9 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -12,7 +12,7 @@ class PostFlag < ApplicationRecord belongs_to :post validates :reason, presence: true, length: { in: 1..140 } validate :validate_creator_is_not_limited, on: :create - validate :validate_post + validate :validate_post, 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 attr_accessor :is_deletion @@ -26,7 +26,6 @@ class PostFlag < ApplicationRecord scope :by_users, -> { where.not(creator: User.system) } scope :by_system, -> { where(creator: User.system) } scope :in_cooldown, -> { by_users.where("created_at >= ?", COOLDOWN_PERIOD.ago) } - scope :recent, -> { where("post_flags.created_at >= ?", 1.day.ago) } scope :expired, -> { pending.where("post_flags.created_at <= ?", 3.days.ago) } module SearchMethods @@ -97,30 +96,18 @@ class PostFlag < ApplicationRecord end def validate_creator_is_not_limited - return if is_deletion - - if creator.can_approve_posts? - # do nothing - elsif creator.is_gold? && flag_count_for_creator >= 10 - errors[:creator] << "can flag 10 posts a day" - elsif !creator.is_gold? && flag_count_for_creator >= 1 - errors[:creator] << "can flag 1 post a day" - end - - 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 + errors[:creator] << "have reached your flag limit" if creator.is_flag_limited? && !is_deletion end def validate_post errors[:post] << "is pending and cannot be flagged" if post.is_pending? && !is_deletion errors[:post] << "is deleted and cannot be flagged" if post.is_deleted? && !is_deletion errors[:post] << "is locked and cannot be flagged" if post.is_status_locked? - end - def flag_count_for_creator - creator.post_flags.recent.count + 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 uploader_id diff --git a/app/models/user.rb b/app/models/user.rb index c5cd85805..d73e89412 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -354,6 +354,21 @@ class User < ApplicationRecord upload_limit.free_upload_slots < UploadLimit::APPEAL_COST end + def is_flag_limited? + return false if has_unlimited_flags? + post_flags.pending.count >= 10 + end + + # Flags are unlimited if you're an approver or you have at least 30 flags + # in the last 3 months and have a 70% flag success rate. + def has_unlimited_flags? + return true if can_approve_posts? + + recent_flags = post_flags.where("created_at >= ?", 3.months.ago) + flag_ratio = recent_flags.succeeded.count / recent_flags.count.to_f + recent_flags.count >= 30 && flag_ratio >= 0.70 + end + def upload_limit @upload_limit ||= UploadLimit.new(self) end diff --git a/test/unit/post_approval_test.rb b/test/unit/post_approval_test.rb index b70b411e9..d484e995e 100644 --- a/test/unit/post_approval_test.rb +++ b/test/unit/post_approval_test.rb @@ -3,64 +3,36 @@ require 'test_helper' class PostApprovalTest < ActiveSupport::TestCase context "a pending post" do setup do - @user = FactoryBot.create(:user, created_at: 2.weeks.ago) - CurrentUser.user = @user - CurrentUser.ip_addr = "127.0.0.1" - - @post = FactoryBot.create(:post, uploader_id: @user.id, tag_string: "touhou", is_pending: true) - - @approver = FactoryBot.create(:user) - @approver.can_approve_posts = true - @approver.save - CurrentUser.user = @approver - - CurrentUser.stubs(:can_approve_posts?).returns(true) - end - - teardown do - CurrentUser.user = nil - CurrentUser.ip_addr = nil + @user = create(:user, created_at: 2.weeks.ago) + @post = create(:post, uploader: @user, is_pending: true) + @approver = create(:user, can_approve_posts: true) end context "That is approved" do should "create a postapproval record" do assert_difference("PostApproval.count") do - @post.approve! + @post.approve!(@approver) end end - context "that is then flagged" do - setup do - @user2 = create(:user, created_at: 2.weeks.ago) - @user3 = create(:user, created_at: 2.weeks.ago) - @approver2 = FactoryBot.create(:user) - @approver2.can_approve_posts = true - @approver2.save - end + should "prevent an approver from approving the same post twice" do + @approval1 = create(:post_approval, post: @post, user: @approver) + @approval2 = build(:post_approval, post: @post, user: @approver) - should "prevent the first approver from approving again" do - @post.approve!(@approver) - CurrentUser.user = @user2 - @post.flag!("blah") - @post.approve!(@approver2) - assert_not_equal(@approver.id, @post.approver_id) - CurrentUser.user = @user3 - travel(PostFlag::COOLDOWN_PERIOD + 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") - end + assert_equal(false, @approval2.valid?) + assert_equal(["You have previously approved this post and cannot approve it again"], @approval2.errors[:base]) end end context "#search method" do should "work" do - @approval = @post.approve!(@approver) - @approvals = PostApproval.search(user_name: @approver.name, post_tags_match: "touhou", post_id: @post.id) + CurrentUser.scoped(@approver) do + @post.update!(tag_string: "touhou") + @approval = @post.approve!(@approver) + @approvals = PostApproval.search(user_name: @approver.name, post_tags_match: "touhou", post_id: @post.id) - assert_equal([@approval.id], @approvals.map(&:id)) + assert_equal([@approval.id], @approvals.map(&:id)) + end end end end diff --git a/test/unit/post_flag_test.rb b/test/unit/post_flag_test.rb index ced0cbc36..f9a167ec1 100644 --- a/test/unit/post_flag_test.rb +++ b/test/unit/post_flag_test.rb @@ -1,82 +1,75 @@ require 'test_helper' class PostFlagTest < ActiveSupport::TestCase - context "In all cases" do - setup do - travel_to(2.weeks.ago) do - @alice = create(:gold_user) + context "PostFlag: " do + context "an approver" do + should "be able to flag an unlimited number of posts" do + @user = create(:user, can_approve_posts: true) + + assert_nothing_raised do + create_list(:post_flag, 6, creator: @user, status: :pending) + end end - as(@alice) do - @post = create(:post, tag_string: "aaa", uploader: @alice) + end + + context "a user with unlimited flags" do + should "be able to flag an unlimited number of posts" do + @user = create(:user) + create_list(:post_flag, 30, status: :succeeded, creator: @user) + + assert_equal(true, @user.has_unlimited_flags?) + assert_equal(false, @user.is_flag_limited?) + + assert_nothing_raised do + create_list(:post_flag, 11, creator: @user, status: :pending) + end end end context "a basic user" do - should "not be able to flag more than 1 post in 24 hours" do - @bob = create(:user, created_at: 2.weeks.ago) - @post_flag = build(:post_flag, creator: @bob) - @post_flag.expects(:flag_count_for_creator).returns(1) + should "be able to flag up to 10 posts" do + @user = create(:user) + @flags = create_list(:post_flag, 10, creator: @user, status: :pending) + @flag = build(:post_flag, creator: @user, status: :pending) - assert_equal(false, @post_flag.valid?) - assert_equal(["You can flag 1 post a day"], @post_flag.errors.full_messages) + assert_equal(false, @flag.valid?) + assert_equal(["have reached your flag limit"], @flag.errors[:creator]) end end - context "a gold user" do - setup do - @bob = create(:gold_user, created_at: 1.month.ago) - end - - should "not be able to flag a post more than twice" do - @post_flag = create(:post_flag, post: @post, creator: @bob) - @post_flag = build(:post_flag, post: @post, creator: @bob) + context "a user" do + should "not be able to flag a post more than once" do + @user = create(:user) + @post = create(:post) + @post_flag = create(:post_flag, post: @post, creator: @user) + @post_flag = build(:post_flag, post: @post, creator: @user) assert_equal(false, @post_flag.valid?) 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 - @post_flag = build(:post_flag, post: @post, creator: @bob) - @post_flag.expects(:flag_count_for_creator).returns(10) - - assert_difference(-> { PostFlag.count }, 0) do - @post_flag.save - end - - assert_equal(["You can flag 10 posts a day"], @post_flag.errors.full_messages) - end - should "not be able to flag a deleted post" do - as(@alice) do - @post.update(is_deleted: true) - end + @post = create(:post, is_deleted: true) + @post_flag = build(:post_flag, post: @post) - @post_flag = build(:post_flag, post: @post, creator: @bob) - @post_flag.save + assert_equal(false, @post_flag.valid?) assert_equal(["Post is deleted and cannot be flagged"], @post_flag.errors.full_messages) end should "not be able to flag a pending post" do - as(@alice) do - @post.update(is_pending: true) - end - @flag = @post.flags.create(reason: "test", creator: @bob) + @post = create(:post, is_pending: true) + @flag = build(:post_flag, post: @post) + assert_equal(false, @flag.valid?) assert_equal(["Post is pending and cannot be flagged"], @flag.errors.full_messages) end should "not be able to flag a post in the cooldown period" do @mod = create(:moderator_user) - - travel_to(2.weeks.ago) do - @users = FactoryBot.create_list(:user, 2) - end - - @flag1 = create(:post_flag, post: @post, reason: "something", creator: @users.first) - - as(@mod) do - @post.approve! - end + @users = create_list(:user, 2) + @post = create(:post) + @flag1 = create(:post_flag, post: @post, creator: @users.first) + as(@mod) { @post.approve! } travel_to(PostFlag::COOLDOWN_PERIOD.from_now - 1.minute) do @flag2 = build(:post_flag, post: @post, reason: "something", creator: @users.second) @@ -89,11 +82,6 @@ class PostFlagTest < ActiveSupport::TestCase assert(@flag3.errors.empty?) end end - - should "initialize its creator" do - @post_flag = create(:post_flag, creator: @alice) - assert_equal(@alice.id, @post_flag.creator_id) - end end end end