From abbf834256551477fc988d41289e03b2948ec799 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Thu, 13 Oct 2016 10:56:11 +0000
Subject: [PATCH 1/6] Merge wiki conflict and tag alias approval forum posts
 (#2715).

* Attribute the "tag alias has conflicting wiki pages" message to the
  alias approver, not to the first admin.
* Merge the conflict message and alias approval message into one forum post.
* Fix an error with NewRelic gem not installed in test environment.
---
 app/models/tag_alias.rb       | 59 +++++++++++++----------------------
 app/models/tag_implication.rb |  5 ++-
 2 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb
index f8423395e..217f24d8f 100644
--- a/app/models/tag_alias.rb
+++ b/app/models/tag_alias.rb
@@ -89,11 +89,7 @@ class TagAlias < ActiveRecord::Base
   end
 
   def approve!(approver_id)
-    self.status = "queued"
-    self.approver_id = approver_id
-    save
-
-    rename_wiki_and_artist
+    update_attributes(:status => "queued", :approver_id => approver_id)
     delay(:queue => "default").process!(true)
   end
 
@@ -103,6 +99,7 @@ class TagAlias < ActiveRecord::Base
     end
 
     tries = 0
+    forum_message = []
 
     begin
       admin = CurrentUser.user || approver || User.admins.first
@@ -113,7 +110,8 @@ class TagAlias < ActiveRecord::Base
         clear_all_cache
         ensure_category_consistency
         update_posts
-        update_forum_topic_for_approve if update_topic
+        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)
       end
     rescue Exception => e
@@ -123,9 +121,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 +248,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 +259,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 +273,8 @@ class TagAlias < ActiveRecord::Base
         end
       end
     end
+
+    message
   end
 
   def deletable_by?(user)
@@ -277,40 +288,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
diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb
index 3fca48e97..c0b906a2d 100644
--- a/app/models/tag_implication.rb
+++ b/app/models/tag_implication.rb
@@ -155,7 +155,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
 

From 1e8a68a56b4b1ad5e5ecd111ac340df98e05202b Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Wed, 26 Oct 2016 18:23:32 -0500
Subject: [PATCH 2/6] Test that approving BUR sets approver of
 aliases/implications.

---
 test/factories/tag_alias.rb           |  3 ++-
 test/factories/tag_implication.rb     |  3 ++-
 test/unit/bulk_update_request_test.rb | 36 ++++++++++++++++++++++++---
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/test/factories/tag_alias.rb b/test/factories/tag_alias.rb
index 37453f307..ee9e7715a 100644
--- a/test/factories/tag_alias.rb
+++ b/test/factories/tag_alias.rb
@@ -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
diff --git a/test/factories/tag_implication.rb b/test/factories/tag_implication.rb
index b3cd8b8dc..6bb91806e 100644
--- a/test/factories/tag_implication.rb
+++ b/test/factories/tag_implication.rb
@@ -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
diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb
index dca027f3f..3f55b7837 100644
--- a/test/unit/bulk_update_request_test.rb
+++ b/test/unit/bulk_update_request_test.rb
@@ -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.id)
+
+        @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

From 6dd8ec909de44ec12028abffcf4fd2a61e76981a Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Wed, 26 Oct 2016 18:40:58 -0500
Subject: [PATCH 3/6] Set approver of aliases/implications in BURs.

Previously only the BUR's approver was set when a BUR was approved. Set
the approver for each alias/implication in the BUR as well.

Additionally:

* Refactor `approve!` to take a user instead of just a user id.
* Be mass-assignment permissions aware when setting approver_id.
---
 .../bulk_update_requests_controller.rb          |  2 +-
 app/controllers/tag_aliases_controller.rb       |  2 +-
 app/controllers/tag_implications_controller.rb  |  2 +-
 app/logical/alias_and_implication_importer.rb   | 10 +++++-----
 app/models/bulk_update_request.rb               | 10 ++++------
 app/models/tag_alias.rb                         | 15 +++++++--------
 app/models/tag_implication.rb                   | 17 +++++++----------
 test/unit/bulk_update_request_test.rb           |  6 +++---
 8 files changed, 29 insertions(+), 35 deletions(-)

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
 

From 1e9bcf75de2cc7975d929f34327b7d92de18b39f Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Wed, 26 Oct 2016 18:50:17 -0500
Subject: [PATCH 4/6] Test banning artist sets approver of banned_artist
 implication.

---
 test/unit/artist_test.rb | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb
index 4726cb0ab..d9f56a932 100644
--- a/test/unit/artist_test.rb
+++ b/test/unit/artist_test.rb
@@ -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

From e67194c19d01b8ebb216fe60c8844850237b2a2a Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Wed, 26 Oct 2016 18:50:49 -0500
Subject: [PATCH 5/6] Set approver when creating banned_artist implication.

---
 app/models/artist.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/artist.rb b/app/models/artist.rb
index 3727a2fee..911e1b88f 100644
--- a/app/models/artist.rb
+++ b/app/models/artist.rb
@@ -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)

From 0dcd7e82be7cc7e95471c233ec7435a63c129a2f Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sun, 16 Oct 2016 08:37:17 +0000
Subject: [PATCH 6/6] Test forum posts generated by tag alias approval.

---
 app/models/user.rb          |  1 +
 test/unit/tag_alias_test.rb | 26 ++++++++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index f3bddb175..15be1e2ad 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -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
diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb
index 16bce66c9..a8ff2abf4 100644
--- a/test/unit/tag_alias_test.rb
+++ b/test/unit/tag_alias_test.rb
@@ -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