add approver field to aliases/implications

app/controllers/tag_aliases_controller.rb

@@ -43,9 +43,7 @@ class TagAliasesController < ApplicationController
 
   def approve
     @tag_alias = TagAlias.find(params[:id])
-    @tag_alias.update_column(:status, "queued")
-    @tag_alias.rename_wiki_and_artist
-    @tag_alias.delay(:queue => "default").process!(true, CurrentUser.id)
+    @tag_alias.approve!(CurrentUser.user.id)
     respond_with(@tag_alias, :location => tag_alias_path(@tag_alias))
   end
 end

app/controllers/tag_implications_controller.rb

@@ -48,8 +48,7 @@ class TagImplicationsController < ApplicationController
 
   def approve
     @tag_implication = TagImplication.find(params[:id])
-    @tag_implication.update_column(:status, "queued")
-    @tag_implication.delay(:queue => "default").process!(true, CurrentUser.id)
+    @tag_implication.approve!(CurrentUser.user.id)
     respond_with(@tag_implication, :location => tag_implication_path(@tag_implication))
   end
 end

app/models/bulk_update_request.rb

@@ -3,6 +3,7 @@ class BulkUpdateRequest < ActiveRecord::Base
 
   belongs_to :user
   belongs_to :forum_topic
+  belongs_to :approver, :class_name => "User"
 
   validates_presence_of :user
   validates_presence_of :script
@@ -12,7 +13,7 @@ class BulkUpdateRequest < ActiveRecord::Base
   validate :forum_topic_id_not_invalid
   validate :validate_script
   attr_accessible :user_id, :forum_topic_id, :script, :title, :reason, :skip_secondary_validations
-  attr_accessible :status, :as => [:admin]
+  attr_accessible :status, :approver_id, :as => [:admin]
   before_validation :initialize_attributes, :on => :create
   before_validation :normalize_text
   after_create :create_forum_topic
@@ -34,16 +35,15 @@ class BulkUpdateRequest < ActiveRecord::Base
 
   def approve!
     AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process!
+    update_columns(:status => "approved", :approver_id => CurrentUser.user.id)
     update_forum_topic_for_approve
-    update_attribute(:status, "approved")
 
   rescue Exception => x
-    message_admin_on_failure(x)
+    message_approver_on_failure(x)
     update_topic_on_failure(x)
   end
 
-  def message_admin_on_failure(x)
-    admin = User.admins.first
+  def message_approver_on_failure(x)
     msg = <<-EOS
       Bulk Update Request ##{id} failed\n
       Exception: #{x.class}\n
@@ -56,13 +56,13 @@ class BulkUpdateRequest < ActiveRecord::Base
     end
 
     dmail = Dmail.new(
-      :from_id => admin.id,
-      :to_id => admin.id,
-      :owner_id => admin.id,
+      :from_id => approver.id,
+      :to_id => approver.id,
+      :owner_id => approver.id,
       :title => "Bulk update request approval failed",
       :body => msg
     )
-    dmail.owner_id = admin.id
+    dmail.owner_id = approver.id
     dmail.save
   end
 

app/models/tag_alias.rb

@@ -13,6 +13,7 @@ class TagAlias < ActiveRecord::Base
   validate :consequent_has_wiki_page, :on => :create
   validate :mininum_antecedent_count, :on => :create
   belongs_to :creator, :class_name => "User"
+  belongs_to :approver, :class_name => "User"
   belongs_to :forum_topic
   attr_accessible :antecedent_name, :consequent_name, :forum_topic_id, :status, :skip_secondary_validations
 
@@ -81,7 +82,16 @@ class TagAlias < ActiveRecord::Base
     end.uniq
   end
 
-  def process!(update_topic=true, approver_id = nil)
+  def approve!(approver_id)
+    self.status = "queued"
+    self.approver_id = approver_id
+    save
+
+    rename_wiki_and_artist
+    delay(:queue => "default").process!(true)
+  end
+
+  def process!(update_topic=true)
     unless valid?
       raise errors.full_messages.join("; ")
     end
@@ -89,7 +99,7 @@ class TagAlias < ActiveRecord::Base
     tries = 0
 
     begin
-      admin = CurrentUser.user || User.where(id: approver_id).first || User.admins.first
+      admin = CurrentUser.user || approver || User.admins.first
       CurrentUser.scoped(admin, "127.0.0.1") do
         update_column(:status, "processing")
         move_aliases_and_implications

app/models/tag_implication.rb

@@ -5,6 +5,7 @@ class TagImplication < ActiveRecord::Base
   after_save :update_descendant_names_for_parents
   after_destroy :update_descendant_names_for_parents
   belongs_to :creator, :class_name => "User"
+  belongs_to :approver, :class_name => "User"
   belongs_to :forum_topic
   before_validation :initialize_creator, :on => :create
   before_validation :normalize_names
@@ -123,7 +124,7 @@ class TagImplication < ActiveRecord::Base
     self.creator_ip_addr = CurrentUser.ip_addr
   end
 
-  def process!(update_topic=true, approver_id=nil)
+  def process!(update_topic=true)
     unless valid?
       raise errors.full_messages.join("; ")
     end
@@ -131,7 +132,7 @@ class TagImplication < ActiveRecord::Base
     tries = 0
 
     begin
-      admin = CurrentUser.user || User.where(id: approver_id).first || User.admins.first
+      admin = CurrentUser.user || approver || User.admins.first
       CurrentUser.scoped(admin, "127.0.0.1") do
         update_column(:status, "processing")
         update_posts
@@ -273,6 +274,13 @@ class TagImplication < ActiveRecord::Base
     end
   end
 
+  def approve!(approver_id)
+    self.status = "queued"
+    self.approver_id = approver_id
+    save
+    delay(:queue => "default").process!(true)
+  end
+
   def reject!
     update_forum_topic_for_reject
     destroy

app/views/bulk_update_requests/_listing.html.erb

@@ -2,6 +2,7 @@
   <thead>
     <tr>
       <th>Creator</th>
+      <th>Approver</th>
       <th>Forum</th>
       <th>Script</th>
       <th>Status</th>
@@ -12,6 +13,7 @@
     <% bulk_update_requests.each do |request| %>
       <tr id="request-<%= request.id %>">
         <td><%= link_to_user(request.user) %></td>
+        <td><% if request.approver %><%= link_to_user(request.approver) %><% end %></td>
         <td><%= link_to(request.forum_topic_id, forum_topic_path(request.forum_topic_id)) %></td>
         <td><%= script_with_line_breaks(request.script) %></td>
         <td id="request-status-for-<%= request.id %>"><%= request.status %></td>

app/views/tag_aliases/_listing.html.erb

@@ -1,11 +1,12 @@
 <table width="100%" class="striped">
   <thead>
     <tr>
-      <th width="30%">From</th>
-      <th width="30%">To</th>
+      <th width="25%">From</th>
+      <th width="25%">To</th>
       <th width="10%">Reference</th>
-      <th width="10%">Status</th>
-      <th width="20%"></th>
+      <th width="15%">Approver</th>
+      <th width="5%">Status</th>
+      <th width="15%"></th>
     </tr>
   </thead>
   <tbody>
@@ -18,6 +19,7 @@
             <%= link_to tag_alias.forum_topic_id, forum_topic_path(tag_alias.forum_topic_id) %>
           <% end %>
         </td>
+        <td><%= link_to_user(tag_alias.approver) if tag_alias.approver %></td>
         <td id="tag-alias-status-for-<%= tag_alias.id %>">
           <%= tag_alias.status %>
         </td>

app/views/tag_implications/_listing.html.erb

@@ -1,10 +1,11 @@
 <table width="100%" class="striped">
   <thead>
     <tr>
-      <th width="30%">From</th>
-      <th width="30%">To</th>
+      <th width="25%">From</th>
+      <th width="25%">To</th>
       <th width="10%">Reference</th>
-      <th width="10%">Status</th>
+      <th width="15%">Approver</th>
+      <th width="5%">Status</th>
       <th width="15%"></th>
     </tr>
   </thead>
@@ -18,6 +19,7 @@
             <%= link_to tag_implication.forum_topic_id, forum_topic_path(tag_implication.forum_topic_id) %>
           <% end %>
         </td>
+        <td><%= link_to_user(tag_implication.approver) if tag_implication.approver %></td>
         <td id="tag-implication-status-for-<%= tag_implication.id %>"><%= tag_implication.status %></td>
         <td>
           <%= link_to "Show", tag_implication_path(tag_implication) %>

db/migrate/20160526174848_add_approver_id.rb

@@ -0,0 +1,7 @@
+class AddApproverId < ActiveRecord::Migration
+  def change
+    add_column :bulk_update_requests, :approver_id, :integer
+    add_column :tag_aliases, :approver_id, :integer
+    add_column :tag_implications, :approver_id, :integer
+  end
+end

test/unit/bulk_update_request_test.rb

@@ -14,7 +14,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
 
     should "create a forum topic" do
       assert_difference("ForumTopic.count", 1) do
-        BulkUpdateRequest.create(:title => "abc", :reason => "zzz", :script => "create alias aaa -> bbb")
+        BulkUpdateRequest.create(:title => "abc", :reason => "zzz", :script => "create alias aaa -> bbb", :skip_secondary_validations => true)
       end
     end