diff --git a/app/controllers/bulk_update_requests_controller.rb b/app/controllers/bulk_update_requests_controller.rb index 787893783..cdaadaedd 100644 --- a/app/controllers/bulk_update_requests_controller.rb +++ b/app/controllers/bulk_update_requests_controller.rb @@ -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 diff --git a/app/controllers/tag_aliases_controller.rb b/app/controllers/tag_aliases_controller.rb index 27957e3b3..01bcbea99 100644 --- a/app/controllers/tag_aliases_controller.rb +++ b/app/controllers/tag_aliases_controller.rb @@ -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 diff --git a/app/controllers/tag_implications_controller.rb b/app/controllers/tag_implications_controller.rb index a893b53db..9fc755e38 100644 --- a/app/controllers/tag_implications_controller.rb +++ b/app/controllers/tag_implications_controller.rb @@ -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 diff --git a/app/logical/alias_and_implication_importer.rb b/app/logical/alias_and_implication_importer.rb index 9b4b26022..33d8e1ee8 100644 --- a/app/logical/alias_and_implication_importer.rb +++ b/app/logical/alias_and_implication_importer.rb @@ -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 diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 3ae63ba75..fc0a17bfc 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -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 diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 217f24d8f..d83752450 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -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,9 +88,9 @@ class TagAlias < ActiveRecord::Base end.uniq end - def approve!(approver_id) - update_attributes(:status => "queued", :approver_id => approver_id) - 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) @@ -102,9 +102,8 @@ class TagAlias < ActiveRecord::Base 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 @@ -112,7 +111,7 @@ class TagAlias < ActiveRecord::Base update_posts 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 => CurrentUser.role) + update({ :status => "active", :post_count => consequent_tag.post_count }, :as => approver.role) end rescue Exception => e if tries < 5 diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index c0b906a2d..53c76cebc 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -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 @@ -283,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! diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index 3f55b7837..74a62f4bf 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -22,7 +22,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase ) @bur = FactoryGirl.create(:bulk_update_request, :script => @script) - @bur.approve!(@admin.id) + @bur.approve!(@admin) @ta = TagAlias.where(:antecedent_name => "foo", :consequent_name => "bar").first @ti = TagImplication.where(:antecedent_name => "bar", :consequent_name => "baz").first @@ -72,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) @@ -84,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