From 3210da1a237a60b35b82dd62a15324fc9d8cc820 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 8 Aug 2020 12:17:16 -0500 Subject: [PATCH] flags: raise flag limits. The old flag limits were: * 1 flag per day for regular members. * 10 flags per day for Gold users. * Unlimited flags for approvers. The new flag limits are: * 10 flags in the modqueue at once for regular users. * Unlimited flags for approvers. * Unlimited flags for users with a high enough flag success rate. If you have at least 30 flags in the last 3 months, and you have at least a 70% flag success rate, then you get unlimited flags. 10 flags at once means you can have up to 10 flagged posts in the modqueue at the same time. Because flags stay in the modqueue for 3 days, this means you can flag on average 10 posts every 3 days, or just over 3 posts per day. --- app/models/post_flag.rb | 25 ++------- app/models/user.rb | 15 +++++ test/unit/post_approval_test.rb | 58 +++++-------------- test/unit/post_flag_test.rb | 98 +++++++++++++++------------------ 4 files changed, 79 insertions(+), 117 deletions(-) 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