Fix #4359: Allow builders to move small (artist) tags manually.

Allow builders to approve artist alias BURs. The BUR must contain only
artist aliases or mass updates and each artist must have less than 100
posts.
This commit is contained in:
evazion
2020-05-11 00:20:21 -05:00
parent e3187e0bd0
commit d136a12a65
5 changed files with 60 additions and 4 deletions

View File

@@ -117,6 +117,22 @@ class BulkUpdateRequestProcessor
[] []
end 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 def to_dtext
tokens.map do |token| tokens.map do |token|
case token[0] case token[0]
@@ -132,5 +148,15 @@ class BulkUpdateRequestProcessor
end.join("\n") end.join("\n")
end 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 memoize :tokens
end end

View File

@@ -136,6 +136,10 @@ class BulkUpdateRequest < ApplicationRecord
@processor ||= BulkUpdateRequestProcessor.new(script, forum_topic_id: forum_topic_id, skip_secondary_validations: skip_secondary_validations) @processor ||= BulkUpdateRequestProcessor.new(script, forum_topic_id: forum_topic_id, skip_secondary_validations: skip_secondary_validations)
end end
def is_tag_move_allowed?
processor.is_tag_move_allowed?
end
def is_pending? def is_pending?
status == "pending" status == "pending"
end end

View File

@@ -8,7 +8,7 @@ class BulkUpdateRequestPolicy < ApplicationPolicy
end end
def approve? def approve?
user.is_admin? && !record.is_approved? unbanned? && !record.is_approved? && (user.is_admin? || (user.is_builder? && record.is_tag_move_allowed?))
end end
def destroy? def destroy?

View File

@@ -1,8 +1,6 @@
<%# bur %> <%# bur %>
<% if policy(bur).approve? %> <%= 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?" %> |
<%= 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 %>
<% if policy(bur).destroy? %> <% 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?" %> | <%= link_to "Reject", bur, remote: true, method: :delete, "data-confirm": "Are you sure you want to reject this bulk update request?" %> |
<% end %> <% end %>

View File

@@ -4,6 +4,7 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest
context "BulkUpdateRequestsController" do context "BulkUpdateRequestsController" do
setup do setup do
@user = create(:user) @user = create(:user)
@builder = create(:builder_user)
@admin = create(:admin_user) @admin = create(:admin_user)
@bulk_update_request = create(:bulk_update_request, user: @user) @bulk_update_request = create(:bulk_update_request, user: @user)
end end
@@ -121,6 +122,33 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest
end end
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 context "for an admin" do
should "succeed" do should "succeed" do
post_auth approve_bulk_update_request_path(@bulk_update_request), @admin post_auth approve_bulk_update_request_path(@bulk_update_request), @admin