jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #2739 from evazion/fix-tag-alias

Browse Source 
Fix "conflicting wikis" message; fix alias/implication approvers in BURs (#2715)
This commit is contained in:
Albert Yi
2016-10-27 10:44:51 -07:00
committed by GitHub
parent af49c9c46a 0dcd7e82be
commit 8e87d41a3c
14 changed files with 119 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/bulk_update_requests_controller.rb
View File

@@ -31,7 +31,7 @@ class BulkUpdateRequestsController < ApplicationController
end
def approve
@bulk_update_request.approve!(CurrentUser.user.id)
@bulk_update_request.approve!(CurrentUser.user)
respond_with(@bulk_update_request, :location => bulk_update_requests_path)
end

2
app/controllers/tag_aliases_controller.rb
View File

@@ -43,7 +43,7 @@ class TagAliasesController < ApplicationController
def approve
@tag_alias = TagAlias.find(params[:id])
@tag_alias.approve!(CurrentUser.user.id)
@tag_alias.approve!(CurrentUser.user)
respond_with(@tag_alias, :location => tag_alias_path(@tag_alias))
end

2
app/controllers/tag_implications_controller.rb
View File

@@ -48,7 +48,7 @@ class TagImplicationsController < ApplicationController
def approve
@tag_implication = TagImplication.find(params[:id])
@tag_implication.approve!(CurrentUser.user.id)
@tag_implication.approve!(CurrentUser.user)
respond_with(@tag_implication, :location => tag_implication_path(@tag_implication))
end

10
app/logical/alias_and_implication_importer.rb
View File

@@ -8,9 +8,9 @@ class AliasAndImplicationImporter
@skip_secondary_validations = skip_secondary_validations
end
def process!
def process!(approver = CurrentUser.user)
tokens = AliasAndImplicationImporter.tokenize(text)
parse(tokens)
parse(tokens, approver)
end
def validate!
@@ -77,7 +77,7 @@ class AliasAndImplicationImporter
private
def parse(tokens)
def parse(tokens, approver)
ActiveRecord::Base.transaction do
tokens.map do |token|
case token[0]
@@ -87,14 +87,14 @@ private
raise "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
end
tag_alias.rename_wiki_and_artist if rename_aliased_pages?
tag_alias.delay(:queue => "default").process!(false)
tag_alias.approve!(approver, update_topic: false)
when :create_implication
tag_implication = TagImplication.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
unless tag_implication.valid?
raise "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
end
tag_implication.delay(:queue => "default").process!(false)
tag_implication.approve!(approver)
when :remove_alias
tag_alias = TagAlias.where("antecedent_name = ?", token[1]).first

2
app/models/artist.rb
View File

@@ -318,7 +318,7 @@ class Artist < ActiveRecord::Base
# potential race condition but unlikely
unless TagImplication.where(:antecedent_name => name, :consequent_name => "banned_artist").exists?
tag_implication = TagImplication.create!(:antecedent_name => name, :consequent_name => "banned_artist", :skip_secondary_validations => true, :status => "pending")
tag_implication.delay(:queue => "default").process!
tag_implication.approve!(CurrentUser.user)
end
update_column(:is_banned, true)

10
app/models/bulk_update_request.rb
View File

@@ -33,12 +33,10 @@ class BulkUpdateRequest < ActiveRecord::Base
extend SearchMethods
def approve!(approver_id)
self.approver_id = approver_id
AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process!
self.status = "approved"
self.skip_secondary_validations = true
save
def approve!(approver)
AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process!(approver)
update({ :status => "approved", :approver_id => approver.id, :skip_secondary_validations => true }, :as => approver.role)
update_forum_topic_for_approve
rescue Exception => x

72
app/models/tag_alias.rb
View File

@@ -21,7 +21,7 @@ class TagAlias < ActiveRecord::Base
belongs_to :approver, :class_name => "User"
belongs_to :forum_topic
attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :skip_secondary_validations
attr_accessible :status, :as => [:admin]
attr_accessible :status, :approver_id, :as => [:admin]
module SearchMethods
def name_matches(name)
@@ -88,13 +88,9 @@ class TagAlias < ActiveRecord::Base
end.uniq
end
def approve!(approver_id)
self.status = "queued"
self.approver_id = approver_id
save
rename_wiki_and_artist
delay(:queue => "default").process!(true)
def approve!(approver = CurrentUser.user, update_topic: true)
update({ :status => "queued", :approver_id => approver.id }, :as => approver.role)
delay(:queue => "default").process!(update_topic)
end
def process!(update_topic=true)
@@ -103,18 +99,19 @@ class TagAlias < ActiveRecord::Base
end
tries = 0
forum_message = []
begin
admin = CurrentUser.user || approver || User.admins.first
CurrentUser.scoped(admin, "127.0.0.1") do
update({ :status => "processing" }, :as => CurrentUser.role)
CurrentUser.scoped(approver, CurrentUser.ip_addr) do
update({ :status => "processing" }, :as => approver.role)
move_aliases_and_implications
move_saved_searches
clear_all_cache
ensure_category_consistency
update_posts
update_forum_topic_for_approve if update_topic
update({ :status => "active", :post_count => consequent_tag.post_count }, :as => CurrentUser.role)
forum_message << "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been approved."
forum_message << rename_wiki_and_artist
update({ :status => "active", :post_count => consequent_tag.post_count }, :as => approver.role)
end
rescue Exception => e
if tries < 5
@@ -123,9 +120,18 @@ class TagAlias < ActiveRecord::Base
retry
end
update_forum_topic_for_error(e)
update({ :status => "error: #{e}" }, :as => CurrentUser.role)
NewRelic::Agent.notice_error(e, :custom_params => {:tag_alias_id => id, :antecedent_name => antecedent_name, :consequent_name => consequent_name})
forum_message << "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) failed during processing. Reason: #{e}"
update({ :status => "error: #{e}" }, :as => approver.role)
if Rails.env.production?
NewRelic::Agent.notice_error(e, :custom_params => {:tag_alias_id => id, :antecedent_name => antecedent_name, :consequent_name => consequent_name})
end
ensure
if update_topic && forum_topic.present?
CurrentUser.scoped(approver, CurrentUser.ip_addr) do
forum_topic.posts.create(:body => forum_message.join("\n\n"))
end
end
end
end
@@ -241,6 +247,8 @@ class TagAlias < ActiveRecord::Base
end
def rename_wiki_and_artist
message = ""
antecedent_wiki = WikiPage.titled(antecedent_name).first
if antecedent_wiki.present?
if WikiPage.titled(consequent_name).blank?
@@ -250,7 +258,7 @@ class TagAlias < ActiveRecord::Base
)
end
else
update_forum_topic_for_wiki_conflict
message = "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary."
end
end
@@ -264,6 +272,8 @@ class TagAlias < ActiveRecord::Base
end
end
end
message
end
def deletable_by?(user)
@@ -277,40 +287,14 @@ class TagAlias < ActiveRecord::Base
deletable_by?(user)
end
def update_forum_topic_for_approve
if forum_topic
forum_topic.posts.create(
:body => "The tag alias #{antecedent_name} -> #{consequent_name} has been approved."
)
end
end
def update_forum_topic_for_error(e)
if forum_topic
forum_topic.posts.create(
:body => "The tag alias #{antecedent_name} -> #{consequent_name} failed during processing. Reason: #{e}"
)
end
end
def update_forum_topic_for_reject
if forum_topic
forum_topic.posts.create(
:body => "The tag alias #{antecedent_name} -> #{consequent_name} has been rejected."
:body => "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been rejected."
)
end
end
def update_forum_topic_for_wiki_conflict
if forum_topic
CurrentUser.scoped(User.admins.first, "127.0.0.1") do
forum_topic.posts.create(
:body => "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] has conflicting wiki pages. [[#{consequent_name}]] should be updated to include information from [[#{antecedent_name}]] if necessary."
)
end
end
end
def reject!
update({ :status => "deleted", }, :as => CurrentUser.role)
clear_all_cache

22
app/models/tag_implication.rb
View File

@@ -22,7 +22,7 @@ class TagImplication < ActiveRecord::Base
validate :antecedent_and_consequent_are_different
validate :wiki_pages_present, :on => :create
attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :skip_secondary_validations
attr_accessible :status, :as => [:admin]
attr_accessible :status, :approver_id, :as => [:admin]
module DescendantMethods
extend ActiveSupport::Concern
@@ -138,11 +138,10 @@ class TagImplication < ActiveRecord::Base
tries = 0
begin
admin = CurrentUser.user || approver || User.admins.first
CurrentUser.scoped(admin, "127.0.0.1") do
update({ :status => "processing" }, :as => CurrentUser.role)
CurrentUser.scoped(approver, CurrentUser.ip_addr) do
update({ :status => "processing" }, :as => approver.role)
update_posts
update({ :status => "active" }, :as => CurrentUser.role)
update({ :status => "active" }, :as => approver.role)
update_descendant_names_for_parents
update_forum_topic_for_approve if update_topic
end
@@ -155,7 +154,10 @@ class TagImplication < ActiveRecord::Base
update_forum_topic_for_error(e)
update({ :status => "error: #{e}" }, :as => CurrentUser.role)
NewRelic::Agent.notice_error(e, :custom_params => {:tag_implication_id => id, :antecedent_name => antecedent_name, :consequent_name => consequent_name})
if Rails.env.production?
NewRelic::Agent.notice_error(e, :custom_params => {:tag_implication_id => id, :antecedent_name => antecedent_name, :consequent_name => consequent_name})
end
end
end
@@ -280,11 +282,9 @@ class TagImplication < ActiveRecord::Base
end
end
def approve!(approver_id)
self.status = "queued"
self.approver_id = approver_id
save
delay(:queue => "default").process!(true)
def approve!(approver = CurrentUser.user, update_topic: true)
update({ :status => "queued", :approver_id => approver.id }, :as => approver.role)
delay(:queue => "default").process!(update_topic)
end
def reject!

1
app/models/user.rb
View File

@@ -87,6 +87,7 @@ class User < ActiveRecord::Base
has_many :note_versions, :foreign_key => "updater_id"
has_many :dmails, lambda {order("dmails.id desc")}, :foreign_key => "owner_id"
has_many :saved_searches
has_many :forum_posts, lambda {order("forum_posts.created_at")}, :foreign_key => "creator_id"
belongs_to :inviter, :class_name => "User"
after_update :create_mod_action
accepts_nested_attributes_for :dmail_filter

3
test/factories/tag_alias.rb
View File

@@ -7,7 +7,8 @@ FactoryGirl.define do
after(:create) do |tag_alias|
unless tag_alias.status == "pending"
tag_alias.process!
approver = FactoryGirl.create(:admin_user) unless approver.present?
tag_alias.approve!(approver)
end
end
end

3
test/factories/tag_implication.rb
View File

@@ -7,7 +7,8 @@ FactoryGirl.define do
after(:create) do |tag_implication|
unless tag_implication.status == "pending"
tag_implication.process!
approver = FactoryGirl.create(:admin_user) unless approver.present?
tag_implication.approve!(approver)
end
end
end

8
test/unit/artist_test.rb
View File

@@ -62,7 +62,8 @@ class ArtistTest < ActiveSupport::TestCase
setup do
@post = FactoryGirl.create(:post, :tag_string => "aaa")
@artist = FactoryGirl.create(:artist, :name => "aaa")
@artist.ban!
@admin = FactoryGirl.create(:admin_user)
CurrentUser.scoped(@admin) { @artist.ban! }
@post.reload
end
@@ -89,6 +90,11 @@ class ArtistTest < ActiveSupport::TestCase
assert_equal(1, TagImplication.where(:antecedent_name => "aaa", :consequent_name => "banned_artist").count)
assert_equal("aaa banned_artist", @post.tag_string)
end
should "set the approver of the banned_artist implication" do
ta = TagImplication.where(:antecedent_name => "aaa", :consequent_name => "banned_artist").first
assert_equal(@admin.id, ta.approver.id)
end
end
should "create a new wiki page to store any note information" do

40
test/unit/bulk_update_request_test.rb
View File

@@ -1,10 +1,12 @@
require 'test_helper'
class BulkUpdateRequestTest < ActiveSupport::TestCase
context "creation" do
context "a bulk update request" do
setup do
CurrentUser.user = FactoryGirl.create(:user)
@admin = FactoryGirl.create(:admin_user)
CurrentUser.user = @admin
CurrentUser.ip_addr = "127.0.0.1"
Delayed::Worker.delay_jobs = false
end
teardown do
@@ -12,6 +14,35 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
CurrentUser.ip_addr = nil
end
context "on approval" do
setup do
@script = %q(
create alias foo -> bar
create implication bar -> baz
)
@bur = FactoryGirl.create(:bulk_update_request, :script => @script)
@bur.approve!(@admin)
@ta = TagAlias.where(:antecedent_name => "foo", :consequent_name => "bar").first
@ti = TagImplication.where(:antecedent_name => "bar", :consequent_name => "baz").first
end
should "set the BUR approver" do
assert_equal(@admin.id, @bur.approver.id)
end
should "create aliases/implications" do
assert_equal("active", @ta.status)
assert_equal("active", @ti.status)
end
should "set the alias/implication approvers" do
assert_equal(@admin.id, @ta.approver.id)
assert_equal(@admin.id, @ti.approver.id)
end
end
should "create a forum topic" do
assert_difference("ForumTopic.count", 1) do
BulkUpdateRequest.create(:title => "abc", :reason => "zzz", :script => "create alias aaa -> bbb", :skip_secondary_validations => true)
@@ -34,7 +65,6 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
context "with an associated forum topic" do
setup do
@admin = FactoryGirl.create(:admin_user)
@topic = FactoryGirl.create(:forum_topic)
@req = FactoryGirl.create(:bulk_update_request, :script => "create alias AAA -> BBB", :forum_topic => @topic)
end
@@ -42,7 +72,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
should "handle errors gracefully" do
@req.stubs(:update_forum_topic_for_approve).raises(RuntimeError.new("blah"))
assert_difference("Dmail.count", 1) do
@req.approve!(@admin.id)
@req.approve!(@admin)
end
assert_match(/Exception: RuntimeError/, Dmail.last.body)
assert_match(/Message: blah/, Dmail.last.body)
@@ -54,7 +84,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
should "update the topic when processed" do
assert_difference("ForumPost.count") do
@req.approve!(@admin.id)
@req.approve!(@admin)
end
end

26
test/unit/tag_alias_test.rb
View File

@@ -137,25 +137,30 @@ class TagAliasTest < ActiveSupport::TestCase
setup do
@admin = FactoryGirl.create(:admin_user)
@topic = FactoryGirl.create(:forum_topic)
@alias = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic)
@alias = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic, :status => "pending")
end
context "and conflicting wiki pages" do
setup do
@wiki1 = FactoryGirl.create(:wiki_page, :title => "aaa")
@wiki2 = FactoryGirl.create(:wiki_page, :title => "bbb")
@alias.approve!(@admin)
@admin.reload # reload to get the forum post the approval created.
end
should "update the topic when processed" do
assert_difference("ForumPost.count") do
@alias.rename_wiki_and_artist
end
should "update the forum topic when approved" do
assert(@topic.posts.last, @admin.forum_posts.last)
assert_match(/The tag alias .* been approved/, @admin.forum_posts.last.body)
end
should "warn about conflicting wiki pages when approved" do
assert_match(/has conflicting wiki pages/, @admin.forum_posts.last.body)
end
end
should "update the topic when processed" do
assert_difference("ForumPost.count") do
@alias.process!
@alias.approve!(@admin)
end
end
@@ -164,6 +169,15 @@ class TagAliasTest < ActiveSupport::TestCase
@alias.reject!
end
end
should "update the topic when failed" do
@alias.stubs(:sleep).returns(true)
@alias.stubs(:update_posts).raises(Exception, "oh no")
@alias.approve!(@admin)
assert_match(/error: oh no/, @alias.status)
assert_match(/The tag alias .* failed during processing/, @admin.forum_posts.last.body)
end
end
end
end