aliases: refactor tag moving code.

* Factor out the code for moving tags from tag aliases to a separate
  TagMover class.

* When aliasing two tags that have conflicting wikis, merge the old wiki
  into the new one instead of failing with an error. Merge the other names
  fields, replace the old wiki body with a message linking to the new
  wiki, and mark the old wiki as deleted.

* When aliasing two tags that have conflicting artist entries, merge the
  old artist into the new one instead of silently ignore the conflict.
  Merge the group name, other names, and urls fields, and mark the old
  artist as deleted.

* When two tags have conflicting wikis or artist entries, but the old
  wiki or artist entry is deleted, then just ignore the old wiki or
  artist and don't try to merge it.

* Fix it so that when saved searches are rewritten, we rewrite negated
  searches too.

app/logical/concerns/searchable.rb

@@ -57,12 +57,12 @@ module Searchable
 
   # https://www.postgresql.org/docs/current/static/functions-matching.html#FUNCTIONS-POSIX-REGEXP
   # "(?e)" means force use of ERE syntax; see sections 9.7.3.1 and 9.7.3.4.
-  def where_regex(attr, value)
+  def where_regex(attr, value, flags: "e")
-    where("#{qualified_column_for(attr)} ~ ?", "(?e)" + value)
+    where("#{qualified_column_for(attr)} ~ ?", "(?#{flags})" + value)
   end
 
-  def where_not_regex(attr, value)
+  def where_not_regex(attr, value, flags: "e")
-    where.not("#{qualified_column_for(attr)} ~ ?", "(?e)" + value)
+    where.not("#{qualified_column_for(attr)} ~ ?", "(?#{flags})" + value)
   end
 
   def where_inet_matches(attr, value)

app/logical/tag_mover.rb

@@ -0,0 +1,108 @@
+class TagMover
+  attr_reader :old_tag, :new_tag, :user
+
+  def initialize(old_name, new_name, user: User.system)
+    @old_tag = Tag.find_or_create_by_name(old_name)
+    @new_tag = Tag.find_or_create_by_name(new_name)
+    @user = user
+  end
+
+  def move!
+    CurrentUser.scoped(user) do
+      move_tag_category!
+      move_artist!
+      move_wiki!
+      move_saved_searches!
+      move_posts!
+    end
+  end
+
+  def move_tag_category!
+    if old_tag.general? && !new_tag.general?
+      old_tag.update!(category: new_tag.category)
+    elsif new_tag.general? && !old_tag.general?
+      new_tag.update!(category: old_tag.category)
+    end
+  end
+
+  def move_artist!
+    return unless old_tag.artist? && old_artist.present? && !old_artist.is_deleted?
+
+    if new_artist.nil?
+      old_artist.update!(name: new_tag.name)
+    else
+      merge_artists!
+    end
+  end
+
+  def move_wiki!
+    return unless old_wiki.present? && !old_wiki.is_deleted?
+
+    if new_wiki.nil?
+      old_wiki.update!(title: new_tag.name)
+    else
+      merge_wikis!
+    end
+  end
+
+  def move_posts!
+    Post.raw_tag_match(old_tag.name).find_each do |post|
+      post.lock!
+      post.remove_tag(old_tag.name)
+      post.add_tag(new_tag.name)
+      post.save!
+    end
+  end
+
+  def move_saved_searches!
+    SavedSearch.rewrite_queries!(old_tag.name, new_tag.name)
+  end
+
+  def merge_artists!
+    old_artist.lock!
+    new_artist.lock!
+
+    new_artist.other_names += old_artist.other_names
+    new_artist.other_names += [old_artist.name]
+    new_artist.group_name = old_artist.group_name unless new_artist.group_name.present?
+    new_artist.url_string += "\n" + old_artist.url_string
+    new_artist.is_deleted = false
+    new_artist.save!
+
+    old_artist.other_names = [new_artist.name]
+    old_artist.group_name = ""
+    old_artist.url_string = ""
+    old_artist.is_deleted = true
+    old_artist.save!
+  end
+
+  def merge_wikis!
+    old_wiki.lock!
+    new_wiki.lock!
+
+    new_wiki.other_names += old_wiki.other_names
+    new_wiki.is_deleted = false
+    new_wiki.save!
+
+    old_wiki.body = "This tag has been moved to [[#{new_wiki.title}]]."
+    old_wiki.other_names = []
+    old_wiki.is_deleted = true
+    old_wiki.save!
+  end
+
+  def old_wiki
+    old_tag.wiki_page
+  end
+
+  def new_wiki
+    new_tag.wiki_page
+  end
+
+  def old_artist
+    old_tag.artist
+  end
+
+  def new_artist
+    new_tag.artist
+  end
+end

app/models/saved_search.rb

@@ -137,6 +137,14 @@ class SavedSearch < ApplicationRecord
         queries = searches.map(&:normalized_query)
         queries.sort.uniq
       end
+
+      def rewrite_queries!(old_name, new_name)
+        has_tag(old_name).find_each do |ss|
+          ss.lock!
+          ss.rewrite_query(old_name, new_name)
+          ss.save!
+        end
+      end
     end
 
     def normalized_query
@@ -146,6 +154,11 @@ class SavedSearch < ApplicationRecord
     def normalize_query
       self.query = PostQueryBuilder.new(query).normalized_query(sort: false).to_s
     end
+
+    def rewrite_query(old_name, new_name)
+      self.query.gsub!(/(?:\A| )([-~])?#{Regexp.escape(old_name)}(?: |\z)/i) { " #{$1}#{new_name} " }
+      self.query.strip!
+    end
   end
 
   attr_reader :disable_labels
@@ -155,6 +168,7 @@ class SavedSearch < ApplicationRecord
   before_validation :normalize_query
   before_validation :normalize_labels
   scope :labeled, ->(label) { where_array_includes_any_lower(:labels, [normalize_label(label)]) }
+  scope :has_tag, ->(name) { where_regex(:query, "(^| )[~-]?#{Regexp.escape(name)}( |$)", flags: "i") }
 
   def validate_count
     if user.saved_searches.count + 1 > user.max_saved_searches

app/models/tag_alias.rb

@@ -2,7 +2,6 @@ class TagAlias < TagRelationship
   after_save :create_mod_action
   validates_uniqueness_of :antecedent_name, scope: :status, conditions: -> { active }
   validate :absence_of_transitive_relation
-  validate :wiki_pages_present, on: :create, unless: :skip_secondary_validations
 
   module ApprovalMethods
     def approve!(approver: CurrentUser.user)
@@ -20,25 +19,13 @@ class TagAlias < TagRelationship
     names.map { |name| aliases[name] || name }
   end
 
-  def process!
+  def process!(user: User.system)
-    unless valid?
+    update!(status: "processing")
-      raise errors.full_messages.join("; ")
+    move_aliases_and_implications
-    end
+    TagMover.new(antecedent_name, consequent_name, user: user).move!
-
+    update!(status: "active")
-    CurrentUser.scoped(User.system) do
-      update!(status: "processing")
-      move_aliases_and_implications
-      move_saved_searches
-      ensure_category_consistency
-      update_posts
-      rename_wiki_and_artist
-      update!(status: "active")
-    end
   rescue Exception => e
-    CurrentUser.scoped(approver) do
+    update!(status: "error: #{e}")
-      update(status: "error: #{e}")
-    end
-
     DanbooruLogger.log(e, tag_alias_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name)
   end
 
@@ -52,15 +39,6 @@ class TagAlias < TagRelationship
     end
   end
 
-  def move_saved_searches
-    escaped = Regexp.escape(antecedent_name)
-
-    SavedSearch.where("query like ?", "%#{antecedent_name}%").find_each do |ss|
-      ss.query = ss.query.sub(/(?:^| )#{escaped}(?:$| )/, " #{consequent_name} ").strip.gsub(/  /, " ")
-      ss.save
-    end
-  end
-
   def move_aliases_and_implications
     aliases = TagAlias.where(["consequent_name = ?", antecedent_name])
     aliases.each do |ta|
@@ -90,35 +68,6 @@ class TagAlias < TagRelationship
     end
   end
 
-  def ensure_category_consistency
-    if antecedent_tag.general? && !consequent_tag.general?
-      antecedent_tag.update!(category: consequent_tag.category)
-    elsif consequent_tag.general? && !antecedent_tag.general?
-      consequent_tag.update!(category: antecedent_tag.category)
-    end
-  end
-
-  def rename_wiki_and_artist
-    antecedent_wiki = WikiPage.titled(antecedent_name).first
-    if antecedent_wiki.present?
-      if WikiPage.titled(consequent_name).blank?
-        antecedent_wiki.update!(title: consequent_name)
-      end
-    end
-
-    if antecedent_tag.artist?
-      if antecedent_tag.artist.present? && consequent_tag.artist.blank?
-        antecedent_tag.artist.update!(name: consequent_name)
-      end
-    end
-  end
-
-  def wiki_pages_present
-    if antecedent_wiki.present? && consequent_wiki.present?
-      errors[:base] << "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
-
   def create_mod_action
     alias_desc = %("tag alias ##{id}":[#{Rails.application.routes.url_helpers.tag_alias_path(self)}]: [[#{antecedent_name}]] -> [[#{consequent_name}]])
 

test/unit/tag_alias_test.rb

@@ -67,17 +67,6 @@ class TagAliasTest < ActiveSupport::TestCase
       end
     end
 
-    context "on secondary validation" do
-      should "warn about conflicting wiki pages" do
-        FactoryBot.create(:wiki_page, title: "aaa", body: "aaa")
-        FactoryBot.create(:wiki_page, title: "bbb", body: "bbb")
-        ti = FactoryBot.build(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", skip_secondary_validations: false)
-
-        assert(ti.invalid?)
-        assert_includes(ti.errors[:base], "The tag alias [[aaa]] -> [[bbb]] has conflicting wiki pages. [[bbb]] should be updated to include information from [[aaa]] if necessary.")
-      end
-    end
-
     should "populate the creator information" do
       ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", creator: CurrentUser.user)
       assert_equal(CurrentUser.user.id, ta.creator_id)
@@ -97,15 +86,21 @@ class TagAliasTest < ActiveSupport::TestCase
 
     context "saved searches" do
       should "move saved searches" do
-        tag1 = FactoryBot.create(:tag, :name => "...")
+        @ss1 = create(:saved_search, query: "123 ... 456", user: CurrentUser.user)
-        tag2 = FactoryBot.create(:tag, :name => "bbb")
+        @ss2 = create(:saved_search, query: "123 -... 456", user: CurrentUser.user)
-        ss = FactoryBot.create(:saved_search, :query => "123 ... 456", :user => CurrentUser.user)
+        @ss3 = create(:saved_search, query: "123 ~... 456", user: CurrentUser.user)
-        ta = FactoryBot.create(:tag_alias, :antecedent_name => "...", :consequent_name => "bbb")
+        @ss4 = create(:saved_search, query: "... 456", user: CurrentUser.user)
+        @ss5 = create(:saved_search, query: "123 ...", user: CurrentUser.user)
+        @ta = create(:tag_alias, antecedent_name: "...", consequent_name: "bbb")
 
-        ta.approve!(approver: @admin)
+        @ta.approve!(approver: @admin)
         perform_enqueued_jobs
 
-        assert_equal(%w(123 456 bbb), ss.reload.query.split.sort)
+        assert_equal("123 bbb 456", @ss1.reload.query)
+        assert_equal("123 -bbb 456", @ss2.reload.query)
+        assert_equal("123 ~bbb 456", @ss3.reload.query)
+        assert_equal("bbb 456", @ss4.reload.query)
+        assert_equal("123 bbb", @ss5.reload.query)
       end
     end
 
@@ -131,16 +126,110 @@ class TagAliasTest < ActiveSupport::TestCase
       end
     end
 
-    should "move existing wikis" do
+    context "when the tags have wikis" do
-      wiki = create(:wiki_page, title: "aaa")
+      should "rename the old wiki if there is no conflict" do
-      ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+        @wiki = create(:wiki_page, title: "aaa")
+        @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
 
-      ta.approve!(approver: @admin)
+        @ta.approve!(approver: @admin)
-      perform_enqueued_jobs
+        perform_enqueued_jobs
+        assert_equal("active", @ta.reload.status)
 
-      assert_equal("bbb", wiki.reload.title)
+        assert_equal("bbb", @wiki.reload.title)
+      end
+
+      should "merge existing wikis if there is a conflict" do
+        @wiki1 = create(:wiki_page, title: "aaa", other_names: "111 222", body: "first")
+        @wiki2 = create(:wiki_page, title: "bbb", other_names: "111 333", body: "second")
+        @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+
+        @ta.approve!(approver: @admin)
+        perform_enqueued_jobs
+        assert_equal("active", @ta.reload.status)
+
+        assert_equal(true, @wiki1.reload.is_deleted)
+        assert_equal([], @wiki1.other_names)
+        assert_equal("This tag has been moved to [[#{@wiki2.title}]].", @wiki1.body)
+
+        assert_equal(false, @wiki2.reload.is_deleted)
+        assert_equal(%w[111 333 222], @wiki2.other_names)
+        assert_equal("second", @wiki2.body)
+      end
+
+      should "ignore the old wiki if it has been deleted" do
+        @wiki1 = create(:wiki_page, title: "aaa", other_names: "111 222", body: "first", is_deleted: true)
+        @wiki2 = create(:wiki_page, title: "bbb", other_names: "111 333", body: "second")
+        @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+
+        @ta.approve!(approver: @admin)
+        perform_enqueued_jobs
+        assert_equal("active", @ta.reload.status)
+
+        assert_equal(true, @wiki1.reload.is_deleted)
+        assert_equal(%w[111 222], @wiki1.other_names)
+        assert_equal("first", @wiki1.body)
+
+        assert_equal(false, @wiki2.reload.is_deleted)
+        assert_equal(%w[111 333], @wiki2.other_names)
+        assert_equal("second", @wiki2.body)
+      end
     end
 
+    context "when the tags have artist entries" do
+      should "rename the old artist entry if there is no conflict" do
+        @artist = create(:artist, name: "aaa")
+        @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+
+        @ta.approve!(approver: @admin)
+        perform_enqueued_jobs
+        assert_equal("active", @ta.reload.status)
+
+        assert_equal("bbb", @artist.reload.name)
+      end
+
+      should "merge existing artists if there is a conflict" do
+        @tag = create(:tag, name: "aaa", category: Tag.categories.artist)
+        @artist1 = create(:artist, name: "aaa", group_name: "g_aaa", other_names: "111 222", url_string: "https://twitter.com/111\n-https://twitter.com/222")
+        @artist2 = create(:artist, name: "bbb", other_names: "111 333", url_string: "https://twitter.com/111\n-https://twitter.com/333\nhttps://twitter.com/444")
+        @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+
+        @ta.approve!(approver: @admin)
+        perform_enqueued_jobs
+        assert_equal("active", @ta.reload.status)
+
+        assert_equal(true, @artist1.reload.is_deleted)
+        assert_equal([@artist2.name], @artist1.other_names)
+        assert_equal("", @artist1.group_name)
+        assert_equal([], @artist1.url_array)
+
+        assert_equal(false, @artist2.reload.is_deleted)
+        assert_equal(%w[111 333 222 aaa], @artist2.other_names)
+        assert_equal("g_aaa", @artist2.group_name)
+        assert_equal(%w[-https://twitter.com/222 -https://twitter.com/333 https://twitter.com/111 https://twitter.com/444], @artist2.url_array)
+      end
+
+      should "ignore the old artist if it has been deleted" do
+        @artist1 = create(:artist, name: "aaa", group_name: "g_aaa", other_names: "111 222", url_string: "https://twitter.com/111\n-https://twitter.com/222", is_deleted: true)
+        @artist2 = create(:artist, name: "bbb", other_names: "111 333", url_string: "https://twitter.com/111\n-https://twitter.com/333\nhttps://twitter.com/444")
+        @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending")
+
+        @ta.approve!(approver: @admin)
+        perform_enqueued_jobs
+        assert_equal("active", @ta.reload.status)
+
+        assert_equal(true, @artist1.reload.is_deleted)
+        assert_equal(%w[111 222], @artist1.other_names)
+        assert_equal("g_aaa", @artist1.group_name)
+        assert_equal(%w[-https://twitter.com/222 https://twitter.com/111], @artist1.url_array)
+
+        assert_equal(false, @artist2.reload.is_deleted)
+        assert_equal(%w[111 333], @artist2.other_names)
+        assert_equal("", @artist2.group_name)
+        assert_equal(%w[-https://twitter.com/333 https://twitter.com/111 https://twitter.com/444], @artist2.url_array)
+      end
+    end
+
+
     should "move existing aliases" do
       ta1 = FactoryBot.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb", :status => "pending")
       ta2 = FactoryBot.create(:tag_alias, :antecedent_name => "bbb", :consequent_name => "ccc", :status => "pending")