Fix #4603: Total Upload Limit Being Reduced After A Failed Appeal

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.
+  # This must be called *after* the post is 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
+  # @param is_pending [Boolean] true if the post is pending, false if the post is
-  #   approval or undeletion of an old post, and we have to replay the user's
+  #   active, flagged, appealed, or deleted.
-  #   entire upload history to recalculate their points.
+  # @param is_approval [Boolean] true if the post is being approved or
-  def update_limit!(post, incremental: true)
+  #   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
+      # If we're approving or deleting a pending post, we can simply increment
-        user.upload_points += UploadLimit.upload_value(user.upload_points, post.is_deleted)
+      # 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

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)

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)

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
 

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