diff --git a/app/logical/bulk_update_request_processor.rb b/app/logical/bulk_update_request_processor.rb index 951495048..96212a72e 100644 --- a/app/logical/bulk_update_request_processor.rb +++ b/app/logical/bulk_update_request_processor.rb @@ -117,6 +117,22 @@ class BulkUpdateRequestProcessor [] end + def is_tag_move_allowed? + tokens.all? do |type, *args| + case type + when :create_alias + BulkUpdateRequestProcessor.is_tag_move_allowed?(args[0], args[1]) + when :mass_update + lhs = PostQueryBuilder.new(args[0]) + rhs = PostQueryBuilder.new(args[1]) + + lhs.is_simple_tag? && rhs.is_simple_tag? && BulkUpdateRequestProcessor.is_tag_move_allowed?(args[0], args[1]) + else + false + end + end + end + def to_dtext tokens.map do |token| case token[0] @@ -132,5 +148,15 @@ class BulkUpdateRequestProcessor end.join("\n") end + private + + def self.is_tag_move_allowed?(antecedent_name, consequent_name) + antecedent_tag = Tag.find_by_name(Tag.normalize_name(antecedent_name)) + consequent_tag = Tag.find_by_name(Tag.normalize_name(consequent_name)) + + (antecedent_tag.blank? || antecedent_tag.empty? || (antecedent_tag.artist? && antecedent_tag.post_count <= 100)) && + (consequent_tag.blank? || consequent_tag.empty? || (consequent_tag.artist? && consequent_tag.post_count <= 100)) + end + memoize :tokens end diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 96b095a74..6909728a6 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -136,6 +136,10 @@ class BulkUpdateRequest < ApplicationRecord @processor ||= BulkUpdateRequestProcessor.new(script, forum_topic_id: forum_topic_id, skip_secondary_validations: skip_secondary_validations) end + def is_tag_move_allowed? + processor.is_tag_move_allowed? + end + def is_pending? status == "pending" end diff --git a/app/policies/bulk_update_request_policy.rb b/app/policies/bulk_update_request_policy.rb index 5c9201d93..7f0700636 100644 --- a/app/policies/bulk_update_request_policy.rb +++ b/app/policies/bulk_update_request_policy.rb @@ -8,7 +8,7 @@ class BulkUpdateRequestPolicy < ApplicationPolicy end def approve? - user.is_admin? && !record.is_approved? + unbanned? && !record.is_approved? && (user.is_admin? || (user.is_builder? && record.is_tag_move_allowed?)) end def destroy? diff --git a/app/views/bulk_update_requests/_bur_edit_links.html.erb b/app/views/bulk_update_requests/_bur_edit_links.html.erb index 644938b27..def972e05 100644 --- a/app/views/bulk_update_requests/_bur_edit_links.html.erb +++ b/app/views/bulk_update_requests/_bur_edit_links.html.erb @@ -1,8 +1,6 @@ <%# bur %> -<% if policy(bur).approve? %> - <%= link_to "Approve", approve_bulk_update_request_path(bur), remote: true, method: :post, "data-confirm": "Are you sure you want to approve this bulk update request?" %> | -<% end %> +<%= link_to_if policy(bur).approve?, "Approve", approve_bulk_update_request_path(bur), remote: true, method: :post, "data-confirm": "Are you sure you want to approve this bulk update request?" %> | <% if policy(bur).destroy? %> <%= link_to "Reject", bur, remote: true, method: :delete, "data-confirm": "Are you sure you want to reject this bulk update request?" %> | <% end %> diff --git a/test/functional/bulk_update_requests_controller_test.rb b/test/functional/bulk_update_requests_controller_test.rb index f6b551448..b91a8e04c 100644 --- a/test/functional/bulk_update_requests_controller_test.rb +++ b/test/functional/bulk_update_requests_controller_test.rb @@ -4,6 +4,7 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest context "BulkUpdateRequestsController" do setup do @user = create(:user) + @builder = create(:builder_user) @admin = create(:admin_user) @bulk_update_request = create(:bulk_update_request, user: @user) end @@ -121,6 +122,33 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest end end + context "for a builder" do + should "fail for a large artist move" do + create(:tag, name: "artist1", category: Tag.categories.artist, post_count: 1000) + @bulk_update_request = create(:bulk_update_request, script: "create alias artist1 -> artist2") + + post_auth approve_bulk_update_request_path(@bulk_update_request), @builder + + assert_response 403 + assert_equal("pending", @bulk_update_request.reload.status) + assert_equal(false, TagAlias.where(antecedent_name: "artist1", consequent_name: "artist2").exists?) + end + + should "succeed for a small artist move" do + create(:tag, name: "artist1a", category: Tag.categories.artist, post_count: 10) + create(:tag, name: "artist1b", category: Tag.categories.general, post_count: 0) + create(:tag, name: "artist2a", category: Tag.categories.artist, post_count: 20) + @bulk_update_request = create(:bulk_update_request, script: "mass update artist1a -> artist1b\ncreate alias artist2a -> artist2b") + + post_auth approve_bulk_update_request_path(@bulk_update_request), @builder + + assert_redirected_to(bulk_update_requests_path) + assert_equal("approved", @bulk_update_request.reload.status) + assert_equal(@builder, @bulk_update_request.approver) + assert_equal(true, TagAlias.where(antecedent_name: "artist2a", consequent_name: "artist2b").exists?) + end + end + context "for an admin" do should "succeed" do post_auth approve_bulk_update_request_path(@bulk_update_request), @admin