refactor forum notifications for tag changes

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)
+    @tag_alias.approve!(approver: CurrentUser.user)
     respond_with(@tag_alias, :location => tag_alias_path(@tag_alias))
   end
 

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)
+    @tag_implication.approve!(approver: CurrentUser.user)
     respond_with(@tag_implication, :location => tag_implication_path(@tag_implication))
   end
 

app/logical/alias_and_implication_importer.rb

@@ -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.approve!(approver, update_topic: false)
+          tag_alias.approve!(approver: 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.approve!(approver, update_topic: false)
+          tag_implication.approve!(approver: approver, update_topic: false)
 
         when :remove_alias
           tag_alias = TagAlias.where("antecedent_name = ?", token[1]).first

app/logical/forum_updater.rb

@@ -0,0 +1,38 @@
+class ForumUpdater
+  attr_reader :forum_topic, :forum_post, :expected_title
+
+  def initialize(forum_topic, options = {})
+    @forum_topic = forum_topic
+    @forum_post = options[:forum_post]
+    @expected_title = options[:expected_title]
+  end
+
+  def update(message, title_tag = nil)
+    return if forum_topic.nil?
+    
+    CurrentUser.scoped(User.system) do
+      create_response(message)
+      update_title(title_tag) if title_tag
+
+      if forum_post
+        update_post(message)
+      end
+    end
+  end
+
+  def create_response(body)
+    forum_topic.posts.create(
+      :body => body
+    )
+  end
+
+  def update_title(title_tag)
+    if forum_topic.title == expected_title
+      forum_topic.update(:title => "[#{title_tag}] #{forum_topic.title}")
+    end
+  end
+
+  def update_post(body)
+    forum_post.update(:body => "#{forum_post.body}\n\n#{body}")
+  end
+end

app/logical/tag_alias_request.rb

@@ -6,6 +6,14 @@ class TagAliasRequest
   validate :validate_tag_alias
   validate :validate_forum_topic
 
+  def self.topic_title(antecedent_name, consequent_name)
+    "Tag alias: #{antecedent_name} -> #{consequent_name}"
+  end
+
+  def self.command_string(antecedent_name, consequent_name)
+    "create alias [[#{antecedent_name}]] -> [[#{consequent_name}]]"
+  end
+
   def initialize(attributes)
     @antecedent_name = attributes[:antecedent_name].strip.tr(" ", "_")
     @consequent_name = attributes[:consequent_name].strip.tr(" ", "_")
@@ -23,7 +31,7 @@ class TagAliasRequest
       @forum_topic = build_forum_topic(@tag_alias.id)
       @forum_topic.save
 
-      @tag_alias.update_attribute(:forum_topic_id, @forum_topic.id)
+      @tag_alias.update_attributes(forum_topic_id: @forum_topic.id, forum_post_id: @forum_topic.posts.first.id)
     end
   end
 
@@ -39,9 +47,9 @@ class TagAliasRequest
 
   def build_forum_topic(tag_alias_id)
     ForumTopic.new(
-      :title => "Tag alias: #{antecedent_name} -> #{consequent_name}",
+      :title => TagAliasRequest.topic_title(antecedent_name, consequent_name),
       :original_post_attributes => {
-        :body => "create alias [[#{antecedent_name}]] -> [[#{consequent_name}]]\n\n\"Link to alias\":/tag_aliases?search[id]=#{tag_alias_id}\n\n#{reason}"
+        :body => TagAliasRequest.command_string(antecedent_name, consequent_name) + "\n\n\"Link to alias\":/tag_aliases?search[id]=#{tag_alias_id}\n\n#{reason}"
       },
       :category_id => 1
     )

app/logical/tag_implication_request.rb

@@ -6,6 +6,14 @@ class TagImplicationRequest
   validate :validate_tag_implication
   validate :validate_forum_topic
 
+  def self.topic_title(antecedent_name, consequent_name)
+    "Tag implication: #{antecedent_name} -> #{consequent_name}"
+  end
+
+  def self.command_string(antecedent_name, consequent_name)
+    "create implication [[#{antecedent_name}]] -> [[#{consequent_name}]]"
+  end
+
   def initialize(attributes)
     @antecedent_name = attributes[:antecedent_name].strip.tr(" ", "_")
     @consequent_name = attributes[:consequent_name].strip.tr(" ", "_")
@@ -23,7 +31,7 @@ class TagImplicationRequest
       @forum_topic = build_forum_topic(@tag_implication.id)
       @forum_topic.save
 
-      @tag_implication.update_attribute(:forum_topic_id, @forum_topic.id)
+      @tag_implication.update_attributes(:forum_topic_id => @forum_topic.id, :forum_post_id => @forum_topic.posts.first.id)
     end
   end
 
@@ -39,9 +47,9 @@ class TagImplicationRequest
 
   def build_forum_topic(tag_implication_id)
     ForumTopic.new(
-      :title => "Tag implication: #{antecedent_name} -> #{consequent_name}",
+      :title => TagImplicationRequest.topic_title(antecedent_name, consequent_name),
       :original_post_attributes => {
-        :body => "create implication [[#{antecedent_name}]] -> [[#{consequent_name}]]\n\n\"Link to implication\":/tag_implications?search[id]=#{tag_implication_id}\n\n#{reason}"
+        :body => TagImplicationRequest.command_string(antecedent_name, consequent_name) + "\n\n\"Link to implication\":/tag_implications?search[id]=#{tag_implication_id}\n\n#{reason}"
       },
       :category_id => 1
     )

app/models/artist.rb

@@ -323,7 +323,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)
-            tag_implication.approve!(CurrentUser.user)
+            tag_implication.approve!(approver: CurrentUser.user)
           end
 
           update_column(:is_banned, true)

app/models/bulk_update_request.rb

@@ -3,6 +3,7 @@ class BulkUpdateRequest < ActiveRecord::Base
 
   belongs_to :user
   belongs_to :forum_topic
+  belongs_to :forum_post
   belongs_to :approver, :class_name => "User"
 
   validates_presence_of :user
@@ -31,63 +32,90 @@ class BulkUpdateRequest < ActiveRecord::Base
     end
   end
 
+  module ApprovalMethods
+    def forum_updater
+      @forum_updater ||= begin
+        post = if forum_topic
+          forum_post || forum_topic.posts.first
+        else
+          nil
+        end
+        ForumUpdater.new(
+          forum_topic, 
+          forum_post: post, 
+          expected_title: title
+        )
+      end
+    end
+
+    def approve!(approver)
+      CurrentUser.scoped(approver) do
+        AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process!
+        update({ :status => "approved", :approver_id => CurrentUser.id, :skip_secondary_validations => true }, :as => CurrentUser.role)
+        forum_updater.update("[i]UPDATE #{date_timestamp}[/i]: The \"bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has been approved.", "APPROVED")
+      end
+
+    rescue Exception => x
+      self.approver = approver
+      CurrentUser.scoped(approver) do
+        forum_updater.update("[i]UPDATE #{date_timestamp}[/i]: \"Bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} failed: #{x.to_s}", "FAILED")
+      end
+    end
+
+    def date_timestamp
+      Time.now.strftime("%Y-%m-%d")
+    end
+
+    def create_forum_topic
+      if forum_topic_id
+        update_attributes(:forum_post_id => forum_post.id)
+      else
+        forum_topic = ForumTopic.create(:title => title, :category_id => 1, :original_post_attributes => {:body => reason_with_link})
+        update_attributes(:forum_topic_id => forum_topic.id, :forum_post_id => forum_topic.posts.first.id)
+      end
+    end
+
+    def reject!
+      forum_updater.update("[i]UPDATE #{date_timestamp}[/i]: The \"bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has been rejected.", "REJECTED")
+      update_attribute(:status, "rejected")
+    end
+  end
+
+  module ValidationMethods
+    def script_formatted_correctly
+      AliasAndImplicationImporter.tokenize(script)
+      return true
+    rescue StandardError => e
+      errors.add(:base, e.message)
+      return false
+    end
+
+    def forum_topic_id_not_invalid
+      if forum_topic_id && !forum_topic
+        errors.add(:base, "Forum topic ID is invalid")
+      end
+    end
+
+    def validate_script
+      begin
+        AliasAndImplicationImporter.new(script, forum_topic_id, "1", skip_secondary_validations).validate!
+      rescue RuntimeError => e
+        self.errors[:base] = e.message
+        return false
+      end
+
+      errors.empty?
+    end
+  end
+
   extend SearchMethods
-
+  include ApprovalMethods
-  def approve!(approver)
+  include ValidationMethods
-    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
-    self.approver = approver
-    message_approver_on_failure(x)
-    update_topic_on_failure(x)
-  end
-
-  def message_approver_on_failure(x)
-    msg = <<-EOS
-      Bulk Update Request ##{id} failed\n
-      Exception: #{x.class}\n
-      Message: #{x.to_s}\n
-      Stack trace:\n
-    EOS
-
-    x.backtrace.each do |line|
-      msg += "#{line}\n"
-    end
-
-    dmail = Dmail.new(
-      :from_id => approver.id,
-      :to_id => approver.id,
-      :owner_id => approver.id,
-      :title => "Bulk update request approval failed",
-      :body => msg
-    )
-    dmail.owner_id = approver.id
-    dmail.save
-  end
-
-  def update_topic_on_failure(x)
-    if forum_topic_id
-      body = "\"Bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} failed: #{x.to_s}"
-      ForumPost.create(:body => body, :topic_id => forum_topic_id)
-    end
-  end
 
   def editable?(user)
     user_id == user.id || user.is_builder?
   end
 
-  def create_forum_topic
-    if forum_topic_id
-      ForumPost.create(:body => reason_with_link, :topic_id => forum_topic_id)
-    else
-      forum_topic = ForumTopic.create(:title => "[bulk] #{title}", :category_id => 1, :original_post_attributes => {:body => reason_with_link})
-      update_attribute(:forum_topic_id, forum_topic.id)
-    end
-  end
-
   def reason_with_link
     "#{script_with_links}\n\n\"Link to request\":/bulk_update_requests?search[id]=#{id}\n\n#{reason}"
   end
@@ -109,46 +137,11 @@ class BulkUpdateRequest < ActiveRecord::Base
     lines.join("\n")
   end
 
-  def reject!
-    update_forum_topic_for_reject
-    update_attribute(:status, "rejected")
-  end
-
   def initialize_attributes
     self.user_id = CurrentUser.user.id unless self.user_id
     self.status = "pending"
   end
 
-  def script_formatted_correctly
-    AliasAndImplicationImporter.tokenize(script)
-    return true
-  rescue StandardError => e
-    errors.add(:base, e.message)
-    return false
-  end
-
-  def forum_topic_id_not_invalid
-    if forum_topic_id && !forum_topic
-      errors.add(:base, "Forum topic ID is invalid")
-    end
-  end
-
-  def update_forum_topic_for_approve
-    if forum_topic
-      forum_topic.posts.create(
-        :body => "The \"bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has been approved."
-      )
-    end
-  end
-
-  def update_forum_topic_for_reject
-    if forum_topic
-      forum_topic.posts.create(
-        :body => "The \"bulk update request ##{id}\":/bulk_update_requests?search%5Bid%5D=#{id} has been rejected."
-      )
-    end
-  end
-
   def normalize_text
     self.script = script.downcase
   end
@@ -160,15 +153,4 @@ class BulkUpdateRequest < ActiveRecord::Base
       @skip_secondary_validations = false
     end
   end
-
-  def validate_script
-    begin
-      AliasAndImplicationImporter.new(script, forum_topic_id, "1", skip_secondary_validations).validate!
-    rescue RuntimeError => e
-      self.errors[:base] = e.message
-      return false
-    end
-
-    errors.empty?
-  end
 end

app/models/tag_alias.rb

@@ -20,6 +20,7 @@ class TagAlias < ActiveRecord::Base
   belongs_to :creator, :class_name => "User"
   belongs_to :approver, :class_name => "User"
   belongs_to :forum_topic
+  belongs_to :forum_post
   attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :skip_secondary_validations
   attr_accessible :status, :approver_id, :as => [:admin]
 
@@ -77,8 +78,56 @@ class TagAlias < ActiveRecord::Base
     end
   end
 
+  module ApprovalMethods
+    def approve!(update_topic: true, approver: CurrentUser.user)
+      CurrentUser.scoped(approver) do
+        update({ :status => "queued", :approver_id => approver.id }, :as => CurrentUser.role)
+        delay(:queue => "default").process!(update_topic: update_topic)
+      end
+    end
+
+    def approval_message
+      "[i]UPDATE #{date_timestamp}[/i]: The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been approved."
+    end
+
+    def failure_message(e = nil)
+      "[i]UPDATE #{date_timestamp}[/i]: The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) failed during processing. Reason: #{e}"
+    end
+
+    def reject_message
+      "[i]UPDATE #{date_timestamp}[/i]: The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been rejected."
+    end
+
+    def conflict_message
+      "[i]UPDATE #{date_timestamp}[/i]: 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
+
+    def date_timestamp
+      Time.now.strftime("%Y-%m-%d")
+    end
+  end
+
+  module ForumMethods
+    def forum_updater
+      @forum_updater ||= begin
+        post = if forum_topic
+          forum_post || forum_topic.posts.where("body like ?", TagAliasRequest.command_string(antecedent_name, consequent_name) + "%").last
+        else
+          nil
+        end
+        ForumUpdater.new(
+          forum_topic, 
+          forum_post: post,
+          expected_title: TagAliasRequest.topic_title(antecedent_name, consequent_name)
+        )
+      end
+    end
+  end
+
   extend SearchMethods
   include CacheMethods
+  include ApprovalMethods
+  include ForumMethods
 
   def self.to_aliased(names)
     Cache.get_multi(Array(names), "ta") do |tag|
@@ -86,30 +135,25 @@ class TagAlias < ActiveRecord::Base
     end.values
   end
 
-  def approve!(approver = CurrentUser.user, update_topic: true)
+  def process!(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)
     unless valid?
       raise errors.full_messages.join("; ")
     end
 
     tries = 0
-    forum_message = []
+    messages = []
 
     begin
       CurrentUser.scoped(approver, CurrentUser.ip_addr) do
-        update({ :status => "processing" }, :as => approver.role)
+        update({ :status => "processing" }, :as => CurrentUser.role)
         move_aliases_and_implications
         move_saved_searches
         clear_all_cache
         ensure_category_consistency
         update_posts
-        forum_message << "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been approved."
+        forum_updater.update(approval_message, "APPROVED") if update_topic
-        forum_message << rename_wiki_and_artist
+        rename_wiki_and_artist
-        update({ :status => "active", :post_count => consequent_tag.post_count }, :as => approver.role)
+        update({ :status => "active", :post_count => consequent_tag.post_count }, :as => CurrentUser.role)
       end
     rescue Exception => e
       if tries < 5
@@ -118,18 +162,14 @@ class TagAlias < ActiveRecord::Base
         retry
       end
 
-      forum_message << "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) failed during processing. Reason: #{e}"
+      CurrentUser.scoped(approver, CurrentUser.ip_addr) do
-      update({ :status => "error: #{e}" }, :as => approver.role)
+        forum_updater.update(failure_message(e), "FAILED") if update_topic
+        update({ :status => "error: #{e}" }, :as => CurrentUser.role)
+      end
 
       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
 
@@ -248,8 +288,6 @@ 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?
@@ -257,7 +295,7 @@ class TagAlias < ActiveRecord::Base
           antecedent_wiki.update(title: consequent_name, skip_secondary_validations: true)
         end
       else
-        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."
+        forum_updater.update(conflict_message)
       end
     end
 
@@ -271,8 +309,6 @@ class TagAlias < ActiveRecord::Base
         end
       end
     end
-
-    message
   end
 
   def deletable_by?(user)
@@ -286,18 +322,10 @@ class TagAlias < ActiveRecord::Base
     deletable_by?(user)
   end
 
-  def update_forum_topic_for_reject
-    if forum_topic
-      forum_topic.posts.create(
-        :body => "The tag alias [[#{antecedent_name}]] -> [[#{consequent_name}]] (alias ##{id}) has been rejected."
-      )
-    end
-  end
-
   def reject!
-    update({ :status => "deleted", }, :as => CurrentUser.role)
+    update({ :status => "deleted" }, :as => CurrentUser.role)
     clear_all_cache
-    update_forum_topic_for_reject
+    forum_updater.update(reject_message, "REJECTED")
     destroy
   end
 

app/models/tag_implication.rb

@@ -8,6 +8,7 @@ class TagImplication < ActiveRecord::Base
   belongs_to :creator, :class_name => "User"
   belongs_to :approver, :class_name => "User"
   belongs_to :forum_topic
+  belongs_to :forum_post
   before_validation :initialize_creator, :on => :create
   before_validation :normalize_names
   validates_format_of :status, :with => /\A(active|deleted|pending|processing|queued|error: .*)\Z/
@@ -127,91 +128,161 @@ class TagImplication < ActiveRecord::Base
     end
   end
 
+  module ValidationMethods
+    def absence_of_circular_relation
+      # We don't want a -> b && b -> a chains
+      if self.class.active.exists?(["antecedent_name = ? and consequent_name = ?", consequent_name, antecedent_name])
+        self.errors[:base] << "Tag implication can not create a circular relation with another tag implication"
+        false
+      end
+    end
+
+    def antecedent_is_not_aliased
+      # We don't want to implicate a -> b if a is already aliased to c
+      if TagAlias.active.exists?(["antecedent_name = ?", antecedent_name])
+        self.errors[:base] << "Antecedent tag must not be aliased to another tag"
+        false
+      end
+    end
+
+    def consequent_is_not_aliased
+      # We don't want to implicate a -> b if b is already aliased to c
+      if TagAlias.active.exists?(["antecedent_name = ?", consequent_name])
+        self.errors[:base] << "Consequent tag must not be aliased to another tag"
+        false
+      end
+    end
+
+    def antecedent_and_consequent_are_different
+      normalize_names
+      if antecedent_name == consequent_name
+        self.errors[:base] << "Cannot implicate a tag to itself"
+        false
+      end
+    end
+
+    def wiki_pages_present
+      return if skip_secondary_validations
+
+      unless WikiPage.titled(consequent_name).exists?
+        self.errors[:base] = "The #{consequent_name} tag needs a corresponding wiki page"
+        return false
+      end
+
+      unless WikiPage.titled(antecedent_name).exists?
+        self.errors[:base] = "The #{antecedent_name} tag needs a corresponding wiki page"
+        return false
+      end
+    end
+  end
+
+  module ApprovalMethods
+    def process!(update_topic: true)
+      unless valid?
+        raise errors.full_messages.join("; ")
+      end
+
+      tries = 0
+
+      begin
+        CurrentUser.scoped(approver, CurrentUser.ip_addr) do
+          update({ :status => "processing" }, :as => CurrentUser.role)
+          update_posts
+          update({ :status => "active" }, :as => CurrentUser.role)
+          update_descendant_names_for_parents
+          forum_updater.update("[i]UPDATE #{date_timestamp}[/i]: The tag implication #{antecedent_name} -> #{consequent_name} has been approved.", "APPROVED") if update_topic
+        end
+      rescue Exception => e
+        if tries < 5
+          tries += 1
+          sleep 2 ** tries
+          retry
+        end
+
+        forum_updater.update("[i]UPDATE #{date_timestamp}[/i]: The tag implication #{antecedent_name} -> #{consequent_name} failed during processing. Reason: #{e}", "FAILED") if update_topic
+        update({ :status => "error: #{e}" }, :as => CurrentUser.role)
+
+        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
+
+    def update_posts
+      Post.without_timeout do
+        Post.raw_tag_match(antecedent_name).where("true /* TagImplication#update_posts */").find_each do |post|
+          fixed_tags = "#{post.tag_string} #{descendant_names}".strip
+          CurrentUser.scoped(creator, creator_ip_addr) do
+            post.update_attributes(
+              :tag_string => fixed_tags
+            )
+          end
+        end
+      end
+    end
+
+    def approve!(approver: CurrentUser.user, update_topic: true)
+      update({ :status => "queued", :approver_id => approver.id }, :as => approver.role)
+      delay(:queue => "default").process!(update_topic: update_topic)
+    end
+
+    def reject!
+      update({ :status => "deleted", }, :as => CurrentUser.role)
+      forum_updater.update("[i]UPDATE #{date_timestamp}[/i]: The tag implication #{antecedent_name} -> #{consequent_name} has been rejected.", "REJECTED")
+      destroy
+    end
+
+    def create_mod_action
+      implication = %Q("tag implication ##{id}":[#{Rails.application.routes.url_helpers.tag_implication_path(self)}]: [[#{antecedent_name}]] -> [[#{consequent_name}]])
+
+      if id_changed?
+        ModAction.log("created #{status} #{implication}")
+      else
+        # format the changes hash more nicely.
+        change_desc = changes.except(:updated_at).map do |attribute, values|
+          old, new = values[0], values[1]
+          if old.nil?
+            %Q(set #{attribute} to "#{new}")
+          else
+            %Q(changed #{attribute} from "#{old}" to "#{new}")
+          end
+        end.join(", ")
+
+        ModAction.log("updated #{implication}\n#{change_desc}")
+      end
+    end
+
+    def date_timestamp
+      Time.now.strftime("%Y-%m-%d")
+    end
+
+    def forum_updater
+      @forum_updater ||= begin
+        post = if forum_topic
+          forum_post || forum_topic.posts.where("body like ?", TagImplicationRequest.command_string(antecedent_name, consequent_name) + "%").last
+        else
+          nil
+        end
+        ForumUpdater.new(
+          forum_topic, 
+          forum_post: post, 
+          expected_title: TagImplicationRequest.topic_title(antecedent_name, consequent_name)
+        )
+      end
+    end
+  end
+
   include DescendantMethods
   include ParentMethods
   extend SearchMethods
+  include ValidationMethods
+  include ApprovalMethods
 
   def initialize_creator
     self.creator_id = CurrentUser.user.id
     self.creator_ip_addr = CurrentUser.ip_addr
   end
 
-  def process!(update_topic=true)
-    unless valid?
-      raise errors.full_messages.join("; ")
-    end
-
-    tries = 0
-
-    begin
-      CurrentUser.scoped(approver, CurrentUser.ip_addr) do
-        update({ :status => "processing" }, :as => approver.role)
-        update_posts
-        update({ :status => "active" }, :as => approver.role)
-        update_descendant_names_for_parents
-        update_forum_topic_for_approve if update_topic
-      end
-    rescue Exception => e
-      if tries < 5
-        tries += 1
-        sleep 2 ** tries
-        retry
-      end
-
-      update_forum_topic_for_error(e)
-      update({ :status => "error: #{e}" }, :as => CurrentUser.role)
-
-      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
-
-  def absence_of_circular_relation
-    # We don't want a -> b && b -> a chains
-    if self.class.active.exists?(["antecedent_name = ? and consequent_name = ?", consequent_name, antecedent_name])
-      self.errors[:base] << "Tag implication can not create a circular relation with another tag implication"
-      false
-    end
-  end
-
-  def antecedent_is_not_aliased
-    # We don't want to implicate a -> b if a is already aliased to c
-    if TagAlias.active.exists?(["antecedent_name = ?", antecedent_name])
-      self.errors[:base] << "Antecedent tag must not be aliased to another tag"
-      false
-    end
-  end
-
-  def consequent_is_not_aliased
-    # We don't want to implicate a -> b if b is already aliased to c
-    if TagAlias.active.exists?(["antecedent_name = ?", consequent_name])
-      self.errors[:base] << "Consequent tag must not be aliased to another tag"
-      false
-    end
-  end
-
-  def antecedent_and_consequent_are_different
-    normalize_names
-    if antecedent_name == consequent_name
-      self.errors[:base] << "Cannot implicate a tag to itself"
-      false
-    end
-  end
-
-  def update_posts
-    Post.without_timeout do
-      Post.raw_tag_match(antecedent_name).where("true /* TagImplication#update_posts */").find_each do |post|
-        fixed_tags = "#{post.tag_string} #{descendant_names}".strip
-        CurrentUser.scoped(creator, creator_ip_addr) do
-          post.update_attributes(
-            :tag_string => fixed_tags
-          )
-        end
-      end
-    end
-  end
-  
   def normalize_names
     self.antecedent_name = antecedent_name.downcase.tr(" ", "_")
     self.consequent_name = consequent_name.downcase.tr(" ", "_")
@@ -249,73 +320,4 @@ class TagImplication < ActiveRecord::Base
   def editable_by?(user)
     deletable_by?(user)
   end
-
-  def update_forum_topic_for_approve
-    if forum_topic
-      forum_topic.posts.create(
-        :body => "The tag implication #{antecedent_name} -> #{consequent_name} has been approved."
-      )
-    end
-  end
-
-  def update_forum_topic_for_reject
-    if forum_topic
-      forum_topic.posts.create(
-        :body => "The tag implication #{antecedent_name} -> #{consequent_name} has been rejected."
-      )
-    end
-  end
-
-  def update_forum_topic_for_error(e)
-    if forum_topic
-      forum_topic.posts.create(
-        :body => "The tag implication #{antecedent_name} -> #{consequent_name} failed during processing. Reason: #{e}"
-      )
-    end
-  end
-
-  def wiki_pages_present
-    return if skip_secondary_validations
-
-    unless WikiPage.titled(consequent_name).exists?
-      self.errors[:base] = "The #{consequent_name} tag needs a corresponding wiki page"
-      return false
-    end
-
-    unless WikiPage.titled(antecedent_name).exists?
-      self.errors[:base] = "The #{antecedent_name} tag needs a corresponding wiki page"
-      return false
-    end
-  end
-
-  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!
-    update({ :status => "deleted", }, :as => CurrentUser.role)
-    update_forum_topic_for_reject
-    destroy
-  end
-
-  def create_mod_action
-    implication = %Q("tag implication ##{id}":[#{Rails.application.routes.url_helpers.tag_implication_path(self)}]: [[#{antecedent_name}]] -> [[#{consequent_name}]])
-
-    if id_changed?
-      ModAction.log("created #{status} #{implication}")
-    else
-      # format the changes hash more nicely.
-      change_desc = changes.except(:updated_at).map do |attribute, values|
-        old, new = values[0], values[1]
-        if old.nil?
-          %Q(set #{attribute} to "#{new}")
-        else
-          %Q(changed #{attribute} from "#{old}" to "#{new}")
-        end
-      end.join(", ")
-
-      ModAction.log("updated #{implication}\n#{change_desc}")
-    end
-  end
 end

db/migrate/20170330230231_add_forum_post_id_to_tag_requests.rb

@@ -0,0 +1,9 @@
+class AddForumPostIdToTagRequests < ActiveRecord::Migration
+  def change
+  	ActiveRecord::Base.without_timeout do
+  		add_column :tag_aliases, :forum_post_id, :integer
+  		add_column :tag_implications, :forum_post_id, :integer
+  		add_column :bulk_update_requests, :forum_post_id, :integer
+  	end
+  end
+end

db/structure.sql

@@ -891,7 +891,8 @@ CREATE TABLE bulk_update_requests (
     status character varying DEFAULT 'pending'::character varying NOT NULL,
     created_at timestamp without time zone,
     updated_at timestamp without time zone,
-    approver_id integer
+    approver_id integer,
+    forum_post_id integer
 );
 
 
@@ -2952,7 +2953,8 @@ CREATE TABLE tag_aliases (
     created_at timestamp without time zone,
     updated_at timestamp without time zone,
     post_count integer DEFAULT 0 NOT NULL,
-    approver_id integer
+    approver_id integer,
+    forum_post_id integer
 );
 
 
@@ -2990,7 +2992,8 @@ CREATE TABLE tag_implications (
     status text DEFAULT 'pending'::text NOT NULL,
     created_at timestamp without time zone,
     updated_at timestamp without time zone,
-    approver_id integer
+    approver_id integer,
+    forum_post_id integer
 );
 
 
@@ -7217,13 +7220,6 @@ CREATE UNIQUE INDEX index_wiki_pages_on_title ON wiki_pages USING btree (title);
 CREATE INDEX index_wiki_pages_on_title_pattern ON wiki_pages USING btree (title text_pattern_ops);
 
 
---
--- Name: index_wiki_pages_on_updated_at; Type: INDEX; Schema: public; Owner: -
---
-
-CREATE INDEX index_wiki_pages_on_updated_at ON wiki_pages USING btree (updated_at);
-
-
 --
 -- Name: unique_schema_migrations; Type: INDEX; Schema: public; Owner: -
 --
@@ -7569,5 +7565,5 @@ INSERT INTO schema_migrations (version) VALUES ('20170316224630');
 
 INSERT INTO schema_migrations (version) VALUES ('20170319000519');
 
-INSERT INTO schema_migrations (version) VALUES ('20170329185605');
+INSERT INTO schema_migrations (version) VALUES ('20170330230231');
 

test/factories/tag_alias.rb

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

test/factories/tag_implication.rb

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

test/unit/bulk_update_request_test.rb

@@ -64,17 +64,21 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
 
     context "with an associated forum topic" do
       setup do
-        @topic = FactoryGirl.create(:forum_topic)
+        @topic = FactoryGirl.create(:forum_topic, :title => "[bulk] hoge")
-        @req = FactoryGirl.create(:bulk_update_request, :script => "create alias AAA -> BBB", :forum_topic => @topic)
+        @post = FactoryGirl.create(:forum_post, :topic_id => @topic.id)
+        @req = FactoryGirl.create(:bulk_update_request, :script => "create alias AAA -> BBB", :forum_topic_id => @topic.id, :forum_post_id => @post.id, :title => "[bulk] hoge")
       end
 
       should "handle errors gracefully" do
-        @req.stubs(:update_forum_topic_for_approve).raises(RuntimeError.new("blah"))
+        @req.stubs(:update).raises(RuntimeError.new("blah"))
-        assert_difference("Dmail.count", 1) do
+        assert_difference("ForumPost.count", 1) do
           @req.approve!(@admin)
         end
-        assert_match(/Exception: RuntimeError/, Dmail.last.body)
+
-        assert_match(/Message: blah/, Dmail.last.body)
+        @topic.reload
+        @post.reload
+        assert_match(/\[FAILED\]/, @topic.title)
+        assert_match(/UPDATE/, @post.body)
       end
 
       should "downcase the text" do
@@ -85,6 +89,11 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
         assert_difference("ForumPost.count") do
           @req.approve!(@admin)
         end
+
+        @topic.reload
+        @post.reload
+        assert_match(/\[APPROVED\]/, @topic.title)
+        assert_match(/UPDATE/, @post.body)
       end
 
       should "update the topic when rejected" do
@@ -93,6 +102,11 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
         assert_difference("ForumPost.count") do
           @req.reject!
         end
+
+        @topic.reload
+        @post.reload
+        assert_match(/\[REJECTED\]/, @topic.title)
+        assert_match(/UPDATE/, @post.body)
       end
     end
   end

test/unit/tag_alias_test.rb

@@ -74,7 +74,7 @@ class TagAliasTest < ActiveSupport::TestCase
       assert_nil(Cache.get("ta:aaa"))
     end
 
-    should "zzz move saved searches" do
+    should "move saved searches" do
       tag1 = FactoryGirl.create(:tag, :name => "...")
       tag2 = FactoryGirl.create(:tag, :name => "bbb")
       ss = FactoryGirl.create(:saved_search, :query => "123 ... 456", :user => CurrentUser.user)
@@ -139,7 +139,8 @@ class TagAliasTest < ActiveSupport::TestCase
     context "with an associated forum topic" do
       setup do
         @admin = FactoryGirl.create(:admin_user)
-        @topic = FactoryGirl.create(:forum_topic)
+        @topic = FactoryGirl.create(:forum_topic, :title => TagAliasRequest.topic_title("aaa", "bbb"))
+        @post = FactoryGirl.create(:forum_post, :topic_id => @topic.id, :body => TagAliasRequest.command_string("aaa", "bbb"))
         @alias = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic, :status => "pending")
       end
 
@@ -147,26 +148,34 @@ class TagAliasTest < ActiveSupport::TestCase
         setup do
           @wiki1 = FactoryGirl.create(:wiki_page, :title => "aaa")
           @wiki2 = FactoryGirl.create(:wiki_page, :title => "bbb")
-          @alias.approve!(@admin)
+          @alias.approve!(approver: @admin)
           @admin.reload # reload to get the forum post the approval created.
+          @topic.reload
         end
 
         should "update the forum topic when approved" do
-          assert(@topic.posts.last, @admin.forum_posts.last)
+          assert_equal("[APPROVED] Tag alias: aaa -> bbb", @topic.title)
-          assert_match(/The tag alias .* been approved/, @admin.forum_posts.last.body)
+          assert_match(/The tag alias .* been approved/m, @admin.forum_posts[-2].body)
         end
 
         should "warn about conflicting wiki pages when approved" do
-          assert_match(/has conflicting wiki pages/, @admin.forum_posts.last.body)
+          assert_match(/has conflicting wiki pages/m, @admin.forum_posts[-1].body)
         end
       end
 
       should "update the topic when processed" do
         assert_difference("ForumPost.count") do
-          @alias.approve!(@admin)
+          @alias.approve!(approver: @admin)
         end
       end
 
+      should "update the parent post" do
+        previous = @post.body
+        @alias.approve!(approver: @admin)
+        @post.reload
+        assert_not_equal(previous, @post.body)
+      end
+
       should "update the topic when rejected" do
         assert_difference("ForumPost.count") do
           @alias.reject!
@@ -176,8 +185,10 @@ class TagAliasTest < ActiveSupport::TestCase
       should "update the topic when failed" do
         @alias.stubs(:sleep).returns(true)
         @alias.stubs(:update_posts).raises(Exception, "oh no")
-        @alias.approve!(@admin)
+        @alias.approve!(approver: @admin)
+        @topic.reload
 
+        assert_equal("[FAILED] Tag alias: aaa -> bbb", @topic.title)
         assert_match(/error: oh no/, @alias.status)
         assert_match(/The tag alias .* failed during processing/, @admin.forum_posts.last.body)
       end

test/unit/tag_implication_test.rb

@@ -186,20 +186,29 @@ class TagImplicationTest < ActiveSupport::TestCase
     context "with an associated forum topic" do
       setup do
         @admin = FactoryGirl.create(:admin_user)
-        @topic = FactoryGirl.create(:forum_topic)
+        @topic = FactoryGirl.create(:forum_topic, :title => TagImplicationRequest.topic_title("aaa", "bbb"))
-        @implication = FactoryGirl.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic)
+        @post = FactoryGirl.create(:forum_post, topic_id: @topic.id, :body => TagImplicationRequest.command_string("aaa", "bbb"))
+        @implication = FactoryGirl.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb", :forum_topic => @topic, :status => "pending")
       end
 
       should "update the topic when processed" do
         assert_difference("ForumPost.count") do
-          @implication.process!
+          @implication.approve!
         end
+        @post.reload
+        @topic.reload
+        assert_match(/The tag implication .* has been approved/, @post.body)
+        assert_equal("[APPROVED] Tag implication: aaa -> bbb", @topic.title)
       end
 
       should "update the topic when rejected" do
         assert_difference("ForumPost.count") do
           @implication.reject!
         end
+        @post.reload
+        @topic.reload
+        assert_match(/The tag implication .* has been rejected/, @post.body)
+        assert_equal("[REJECTED] Tag implication: aaa -> bbb", @topic.title)
       end
     end
   end