From 4b6e706e5e50f2da0ed4acfed91ea5bfb0f61f24 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 28 Jun 2021 06:04:14 -0500 Subject: [PATCH] Fix #4603: Total Upload Limit Being Reduced After A Failed Appeal --- app/logical/upload_limit.rb | 23 +++++++++----- app/models/post.rb | 2 +- app/models/post_approval.rb | 3 +- script/fixes/062_initialize_upload_points.rb | 2 +- test/unit/upload_limit_test.rb | 32 ++++++++++++++++++++ 5 files changed, 51 insertions(+), 11 deletions(-) diff --git a/app/logical/upload_limit.rb b/app/logical/upload_limit.rb index 4728ebb8e..845f69e94 100644 --- a/app/logical/upload_limit.rb +++ b/app/logical/upload_limit.rb @@ -82,19 +82,26 @@ class UploadLimit end # Update the uploader's upload points when a post is approved or deleted. - # @param post [Post] The post that was approved or deleted. - # @param incremental [Boolean] True if the post was status:pending and we can - # simply increment/decrement the upload points. False if the post was an - # approval or undeletion of an old post, and we have to replay the user's - # entire upload history to recalculate their points. - def update_limit!(post, incremental: true) + # This must be called *after* the post is approved or deleted. + # + # @param is_pending [Boolean] true if the post is pending, false if the post is + # active, flagged, appealed, or deleted. + # @param is_approval [Boolean] true if the post is being approved or + # undeleted, false if the post is being deleted. + def update_limit!(is_pending, is_approval) return if user.can_upload_free? user.with_lock do - if incremental - user.upload_points += UploadLimit.upload_value(user.upload_points, post.is_deleted) + # If we're approving or deleting a pending post, we can simply increment + # or decrement the upload points. + if is_pending + user.upload_points += UploadLimit.upload_value(user.upload_points, !is_approval) user.upload_points = user.upload_points.clamp(0, MAXIMUM_POINTS) user.save! + + # If we're undeleting a deleted or appealed post, or deleting a flagged + # or active post, then we have to replay the user's entire upload + # history to recalculate their upload points. else user.update!(upload_points: UploadLimit.points_for_user(user)) end diff --git a/app/models/post.rb b/app/models/post.rb index 0c50382e1..5dde03746 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1003,7 +1003,7 @@ class Post < ApplicationRecord # XXX This must happen *after* the `is_deleted` flag is set to true (issue #3419). give_favorites_to_parent if move_favorites - uploader.upload_limit.update_limit!(self, incremental: automated) + uploader.upload_limit.update_limit!(is_pending?, false) unless automated ModAction.log("deleted post ##{id}, reason: #{reason}", :post_delete) diff --git a/app/models/post_approval.rb b/app/models/post_approval.rb index 09a4f1043..7f317b8d8 100644 --- a/app/models/post_approval.rb +++ b/app/models/post_approval.rb @@ -26,6 +26,7 @@ class PostApproval < ApplicationRecord end def approve_post + is_pending = post.is_pending is_undeletion = post.is_deleted post.flags.pending.update!(status: :rejected) @@ -34,7 +35,7 @@ class PostApproval < ApplicationRecord post.update(approver: user, is_flagged: false, is_pending: false, is_deleted: false) ModAction.log("undeleted post ##{post_id}", :post_undelete) if is_undeletion - post.uploader.upload_limit.update_limit!(post, incremental: !is_undeletion) + post.uploader.upload_limit.update_limit!(is_pending, true) end def self.search(params) diff --git a/script/fixes/062_initialize_upload_points.rb b/script/fixes/062_initialize_upload_points.rb index 78431d523..ab23b7d5c 100755 --- a/script/fixes/062_initialize_upload_points.rb +++ b/script/fixes/062_initialize_upload_points.rb @@ -6,7 +6,7 @@ uploaders = User.where(id: Post.select(:uploader_id)).bit_prefs_match(:can_uploa warn "uploaders=#{uploaders.count}" uploaders.find_each.with_index do |uploader, n| - uploader.upload_limit.update_limit!(nil, incremental: false) + uploader.update!(upload_points: UploadLimit.points_for_user(user)) warn "n=#{n} id=#{uploader.id} name=#{uploader.name} points=#{uploader.upload_points}" end diff --git a/test/unit/upload_limit_test.rb b/test/unit/upload_limit_test.rb index 7f6d7a052..218d852c9 100644 --- a/test/unit/upload_limit_test.rb +++ b/test/unit/upload_limit_test.rb @@ -68,5 +68,37 @@ class UploadLimitTest < ActiveSupport::TestCase assert_equal(1010, @user.reload.upload_points) end end + + context "an appealed post that is undeleted" do + should "increase the uploader's upload points" do + @post = create(:post, uploader: @user) + + as(@approver) { @post.delete!("bad") } + assert_equal(967, @user.reload.upload_points) + + @appeal = create(:post_appeal, post: @post) + @post.approve!(@approver) + + assert_equal(true, @appeal.reload.succeeded?) + assert_equal(false, @post.reload.is_deleted?) + assert_equal(1010, @user.reload.upload_points) + end + end + + context "an appealed post that is rejected" do + should "not decrease the uploader's upload points" do + @post = create(:post, uploader: @user) + + as(@approver) { @post.delete!("bad") } + assert_equal(967, @user.reload.upload_points) + + @appeal = create(:post_appeal, post: @post) + travel(4.days) { PostPruner.prune! } + + assert_equal(true, @appeal.reload.rejected?) + assert_equal(true, @post.reload.is_deleted?) + assert_equal(967, @user.reload.upload_points) + end + end end end