fixes #1382

app/models/tag_alias.rb

@@ -72,6 +72,7 @@ class TagAlias < ActiveRecord::Base
 
   def process!
     update_column(:status, "processing")
+    move_aliases_and_implications
     clear_all_cache
     ensure_category_consistency
     update_posts
@@ -107,13 +108,28 @@ class TagAlias < ActiveRecord::Base
   end
 
   def absence_of_transitive_relation
-    # We don't want a -> b && b -> c chains
-    if self.class.exists?(["antecedent_name = ?", consequent_name]) || self.class.exists?(["consequent_name = ?", antecedent_name])
+    # We don't want a -> b && b -> c chains if the b -> c alias was created first.
+    # If the a -> b alias was created first, the new one will be allowed and the old one will be moved automatically instead.
+    if self.class.exists?(["antecedent_name = ?", consequent_name])
       self.errors[:base] << "Tag alias can not create a transitive relation with another tag alias"
       false
     end
   end
 
+  def move_aliases_and_implications
+    aliases = TagAlias.where(["consequent_name = ?", antecedent_name])
+    aliases.each do |ta|
+      ta.consequent_name = self.consequent_name
+      ta.save
+    end
+
+    implications = TagImplication.where(["consequent_name = ?", antecedent_name])
+    implications.each do |ti|
+      ti.consequent_name = self.consequent_name
+      ti.save
+    end
+  end
+
   def ensure_tags_exist
     Tag.find_or_create_by_name(antecedent_name)
     Tag.find_or_create_by_name(consequent_name)

app/models/tag_implication.rb

@@ -8,6 +8,7 @@ class TagImplication < ActiveRecord::Base
   validates_presence_of :creator_id, :antecedent_name, :consequent_name
   validates_uniqueness_of :antecedent_name, :scope => :consequent_name
   validate :absence_of_circular_relation
+  validate :consequent_is_not_aliased
 
   module DescendantMethods
     extend ActiveSupport::Concern
@@ -125,6 +126,14 @@ class TagImplication < ActiveRecord::Base
     end
   end
 
+  def consequent_is_not_aliased
+    # We don't want to implicate a -> b if b is already aliased to c
+    if TagAlias.exists?(["antecedent_name = ?", consequent_name])
+      self.errors[:base] << "Consequent tag must not be aliased to another tag"
+      false
+    end
+  end
+
   def update_posts
     Post.without_timeout do
       Post.raw_tag_match(antecedent_name).find_each do |post|

db/migrate/20131006193238_update_aliased_implication_consequents.rb

@@ -0,0 +1,13 @@
+class UpdateAliasedImplicationConsequents < ActiveRecord::Migration
+  def change
+    execute "set statement_timeout = 0"
+    TagImplication.find_each do |ti|
+      ta = TagAlias.where("antecedent_name = ? AND status != ?", ti.consequent_name, "pending").first
+      if ta
+        ti.consequent_name = ta.consequent_name
+        ti.save
+        ta.update_posts
+      end
+    end
+  end
+end

test/factories/tag_alias.rb

@@ -2,9 +2,12 @@ FactoryGirl.define do
   factory :tag_alias do
     antecedent_name "aaa"
     consequent_name "bbb"
+    status "active"
 
     after(:create) do |tag_alias|
-      tag_alias.process!
+      unless tag_alias.status == "pending"
+        tag_alias.process!
+      end
     end
   end
 end

test/factories/tag_implication.rb

@@ -2,9 +2,12 @@ FactoryGirl.define do
   factory :tag_implication do
     antecedent_name "aaa"
     consequent_name "bbb"
+    status "active"
 
     after(:create) do |tag_implication|
-      tag_implication.process!
+      unless tag_implication.status == "pending"
+        tag_implication.process!
+      end
     end
   end
 end

test/unit/tag_alias_test.rb

@@ -53,15 +53,29 @@ class TagAliasTest < ActiveSupport::TestCase
     end
 
     should "not validate for transitive relations" do
-      ta1 = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb")
+      ta1 = FactoryGirl.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc")
       assert_difference("TagAlias.count", 0) do
-        ta3 = FactoryGirl.build(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ddd")
-        ta3.save
-        assert(ta3.errors.any?, "Tag alias should be invalid")
-        assert_equal("Tag alias can not create a transitive relation with another tag alias", ta3.errors.full_messages.join)
+        ta2 = FactoryGirl.build(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb")
+        ta2.save
+        assert(ta2.errors.any?, "Tag alias should be invalid")
+        assert_equal("Tag alias can not create a transitive relation with another tag alias", ta2.errors.full_messages.join)
       end
     end
 
+    should "move existing aliases" do
+      ta1 = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :status => "active")
+      ta2 = FactoryGirl.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc", :status => "active")
+      ta1.reload
+      assert_equal("ccc", ta1.consequent_name)
+    end
+
+    should "move existing implications" do
+      ti = FactoryGirl.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb", :status => "active")
+      ta = FactoryGirl.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc", :status => "active")
+      ti.reload
+      assert_equal("ccc", ti.consequent_name)
+    end
+
     should "not push the antecedent's category to the consequent if the antecedent is general" do
       tag1 = FactoryGirl.create(:tag, :name => "aaa")
       tag2 = FactoryGirl.create(:tag, :name => "bbb", :category => 1)

test/unit/tag_implication_test.rb

@@ -17,7 +17,7 @@ class TagImplicationTest < ActiveSupport::TestCase
     end
 
     should "ignore pending implications when building descendant names" do
-      ti2 = FactoryGirl.build(:tag_implication, :antecedent_name => "b", :consequent_name => "c")
+      ti2 = FactoryGirl.build(:tag_implication, :antecedent_name => "b", :consequent_name => "c", :status => "pending")
       ti2.save
       ti1 = FactoryGirl.create(:tag_implication, :antecedent_name => "a", :consequent_name => "b")
       assert_equal("b", ti1.descendant_names)
@@ -44,6 +44,14 @@ class TagImplicationTest < ActiveSupport::TestCase
       assert_equal("Antecedent name has already been taken", ti2.errors.full_messages.join(""))
     end
 
+    should "not validate if its consequent is aliased to another tag" do
+      ta = FactoryGirl.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc")
+      ti = FactoryGirl.build(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb")
+      ti.save
+      assert(ti.errors.any?, "Tag implication should not have validated.")
+      assert_equal("Consequent tag must not be aliased to another tag", ti.errors.full_messages.join(""))
+    end
+
     should "calculate all its descendants" do
       ti1 = FactoryGirl.create(:tag_implication, :antecedent_name => "bbb", :consequent_name => "ccc")
       assert_equal("ccc", ti1.descendant_names)