From 1425332c0b4cfc3e898ce60166e06308ee7b10e1 Mon Sep 17 00:00:00 2001 From: Tarrgon <61888458+DontTalkToMeThx@users.noreply.github.com> Date: Thu, 25 Jul 2024 13:26:05 -0400 Subject: [PATCH] [Replacements] Allow janitor+ to auto approve submitted replacements (#650) --- .../post_replacements_controller.rb | 15 +++++- .../src/javascripts/replacement_uploader.vue | 10 ++++ app/models/post_replacement.rb | 6 ++- .../post_replacements_controller_test.rb | 52 +++++++++++++++++-- 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/app/controllers/post_replacements_controller.rb b/app/controllers/post_replacements_controller.rb index c0c602459..02b72b065 100644 --- a/app/controllers/post_replacements_controller.rb +++ b/app/controllers/post_replacements_controller.rb @@ -27,6 +27,19 @@ class PostReplacementsController < ApplicationController if @post_replacement.errors.none? flash[:notice] = "Post replacement submitted" end + + if CurrentUser.can_approve_posts? && !@post_replacement.upload_as_pending? + if @post_replacement.errors.any? + respond_to do |format| + format.json do + return render json: { success: false, message: @post_replacement.errors.full_messages.join("; ") }, status: 412 + end + end + end + + @post_replacement.approve!(penalize_current_uploader: CurrentUser.id != @post.uploader_id) + end + respond_to do |format| format.json do return render json: { success: false, message: @post_replacement.errors.full_messages.join("; ") }, status: 412 if @post_replacement.errors.any? @@ -92,7 +105,7 @@ class PostReplacementsController < ApplicationController end def create_params - params.require(:post_replacement).permit(:replacement_url, :replacement_file, :reason, :source) + params.require(:post_replacement).permit(:replacement_url, :replacement_file, :reason, :source, :as_pending) end def ensure_uploads_enabled diff --git a/app/javascript/src/javascripts/replacement_uploader.vue b/app/javascript/src/javascripts/replacement_uploader.vue index 039a1e7d1..0980ad65f 100644 --- a/app/javascript/src/javascripts/replacement_uploader.vue +++ b/app/javascript/src/javascripts/replacement_uploader.vue @@ -19,6 +19,12 @@ Tell us why this file should replace the original. +
+ +
+
{{ errorMessage }}
@@ -35,6 +41,7 @@ import autocompletableInput from "./autocompletable_input.vue"; import filePreview from "./uploader/file_preview.vue"; import fileInput from "./uploader/file_input.vue"; import sources from "./uploader/sources.vue"; +import Utility from "./utility"; export default { components: { @@ -57,6 +64,8 @@ export default { sourceWarning: false, submitting: false, submittedReason: undefined, + canApprove: Utility.meta("current-user-can-approve-posts") === "true", + uploadAsPending: false, }; }, mounted() { @@ -87,6 +96,7 @@ export default { } formData.append("post_replacement[source]", this.sources[0]); formData.append("post_replacement[reason]", this.reason); + formData.append("post_replacement[as_pending]", this.uploadAsPending); this.submittedReason = this.reason; diff --git a/app/models/post_replacement.rb b/app/models/post_replacement.rb index 2a32be1ef..aec76b0e5 100644 --- a/app/models/post_replacement.rb +++ b/app/models/post_replacement.rb @@ -6,7 +6,7 @@ class PostReplacement < ApplicationRecord belongs_to :creator, class_name: "User" belongs_to :approver, class_name: "User", optional: true belongs_to :uploader_on_approve, class_name: "User", foreign_key: :uploader_id_on_approve, optional: true - attr_accessor :replacement_file, :replacement_url, :tags, :is_backup, :is_destroyed_reupload + attr_accessor :replacement_file, :replacement_url, :tags, :is_backup, :as_pending, :is_destroyed_reupload validate :user_is_not_limited, on: :create validate :post_is_valid, on: :create @@ -314,6 +314,10 @@ class PostReplacement < ApplicationRecord user.is_janitor? end + def upload_as_pending? + as_pending.to_s.truthy? + end + include ApiMethods include StorageMethods include FileMethods diff --git a/test/functional/post_replacements_controller_test.rb b/test/functional/post_replacements_controller_test.rb index a7e38a769..259a1c5c9 100644 --- a/test/functional/post_replacements_controller_test.rb +++ b/test/functional/post_replacements_controller_test.rb @@ -6,6 +6,7 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest context "The post replacements controller" do setup do @user = create(:moderator_user, can_approve_posts: true, created_at: 1.month.ago) + @regular_user = create(:member_user, replacements_beta: true, created_at: 1.month.ago) as(@user) do @upload = UploadService.new(attributes_for(:jpg_upload).merge({ uploader: @user })).start! @post = @upload.post @@ -21,8 +22,9 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest post_id: @post.id, post_replacement: { replacement_file: file, - reason: 'test replacement' - } + reason: "test replacement", + as_pending: true, + }, } assert_difference(-> { @post.replacements.size }) do @@ -33,6 +35,48 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest assert_equal @response.parsed_body["location"], post_path(@post) end + context "with as_pending false" do + should "immediately approve a replacement" do + file = fixture_file_upload("alpha.png") + params = { + format: :json, + post_id: @post.id, + post_replacement: { + replacement_file: file, + reason: "test replacement", + as_pending: false, + }, + } + + post_auth post_replacements_path, @user, params: params + @post.reload + + # 200be2be97a465ecd2054a51522f65b5 is the md5 of alpha.png + assert_equal "200be2be97a465ecd2054a51522f65b5", @post.md5 + assert_equal @response.parsed_body["location"], post_path(@post) + end + + should "always upload as pending if user can't approve posts" do + file = fixture_file_upload("test.gif") + params = { + format: :json, + post_id: @post.id, + post_replacement: { + replacement_file: file, + reason: "test replacement", + as_pending: false, + }, + } + + post_auth post_replacements_path, @regular_user, params: params + @post.reload + + # 1e2edf6bdbd971d8c3cc4da0f98f38ab is the md5 of test.gif + assert_not_equal "1e2edf6bdbd971d8c3cc4da0f98f38ab", @post.md5 + assert_equal @response.parsed_body["location"], post_path(@post) + end + end + context "with a previously destroyed post" do setup do @admin = create(:admin_user) @@ -104,7 +148,7 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest put_auth toggle_penalize_post_replacement_path(@replacement), @user assert_redirected_to post_replacement_path(@replacement) @replacement.reload - assert !@replacement.penalize_uploader_on_approve + assert_not @replacement.penalize_uploader_on_approve end end @@ -117,7 +161,7 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest context "new action" do should "render" do - get_auth new_post_replacement_path, @user, params: {post_id: @post.id} + get_auth new_post_replacement_path, @user, params: { post_id: @post.id } assert_response :success end end