diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb index 98b02e152..151ad5f86 100644 --- a/app/logical/concerns/searchable.rb +++ b/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) - where("#{qualified_column_for(attr)} ~ ?", "(?e)" + value) + def where_regex(attr, value, flags: "e") + where("#{qualified_column_for(attr)} ~ ?", "(?#{flags})" + value) end - def where_not_regex(attr, value) - where.not("#{qualified_column_for(attr)} ~ ?", "(?e)" + value) + def where_not_regex(attr, value, flags: "e") + where.not("#{qualified_column_for(attr)} ~ ?", "(?#{flags})" + value) end def where_inet_matches(attr, value) diff --git a/app/logical/tag_mover.rb b/app/logical/tag_mover.rb new file mode 100644 index 000000000..83e9fa12d --- /dev/null +++ b/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 diff --git a/app/models/saved_search.rb b/app/models/saved_search.rb index d992bf11d..53b664163 100644 --- a/app/models/saved_search.rb +++ b/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 diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 87b2260b9..40a0c7759 100644 --- a/app/models/tag_alias.rb +++ b/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! - unless valid? - raise errors.full_messages.join("; ") - end - - 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 + def process!(user: User.system) + update!(status: "processing") + move_aliases_and_implications + TagMover.new(antecedent_name, consequent_name, user: user).move! + update!(status: "active") rescue Exception => e - CurrentUser.scoped(approver) do - update(status: "error: #{e}") - end - + update!(status: "error: #{e}") 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}]]) diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 1cb8f71dd..8eafa1e8e 100644 --- a/test/unit/tag_alias_test.rb +++ b/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 => "...") - tag2 = FactoryBot.create(:tag, :name => "bbb") - ss = FactoryBot.create(:saved_search, :query => "123 ... 456", :user => CurrentUser.user) - ta = FactoryBot.create(:tag_alias, :antecedent_name => "...", :consequent_name => "bbb") + @ss1 = create(:saved_search, query: "123 ... 456", user: CurrentUser.user) + @ss2 = create(:saved_search, query: "123 -... 456", user: CurrentUser.user) + @ss3 = create(:saved_search, query: "123 ~... 456", user: CurrentUser.user) + @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 - wiki = create(:wiki_page, title: "aaa") - ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending") + context "when the tags have wikis" do + should "rename the old wiki if there is no conflict" do + @wiki = create(:wiki_page, title: "aaa") + @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending") - ta.approve!(approver: @admin) - perform_enqueued_jobs + @ta.approve!(approver: @admin) + 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")