diff --git a/app/jobs/process_tag_alias_job.rb b/app/jobs/process_tag_alias_job.rb deleted file mode 100644 index 120171928..000000000 --- a/app/jobs/process_tag_alias_job.rb +++ /dev/null @@ -1,7 +0,0 @@ -class ProcessTagAliasJob < ApplicationJob - queue_as :bulk_update - - def perform(tag_alias, approver) - tag_alias.process!(approver) - end -end diff --git a/app/jobs/process_tag_implication_job.rb b/app/jobs/process_tag_implication_job.rb deleted file mode 100644 index 457f12386..000000000 --- a/app/jobs/process_tag_implication_job.rb +++ /dev/null @@ -1,7 +0,0 @@ -class ProcessTagImplicationJob < ApplicationJob - queue_as :bulk_update - - def perform(tag_implication, approver) - tag_implication.process!(approver) - end -end diff --git a/app/jobs/process_tag_relationship_job.rb b/app/jobs/process_tag_relationship_job.rb new file mode 100644 index 000000000..01b22a233 --- /dev/null +++ b/app/jobs/process_tag_relationship_job.rb @@ -0,0 +1,9 @@ +class ProcessTagRelationshipJob < ApplicationJob + queue_as :bulk_update + + def perform(class_name:, approver:, antecedent_name:, consequent_name:, forum_topic: nil) + relation_class = Kernel.const_get(class_name) + tag_relationship = relation_class.create!(creator: approver, approver: approver, antecedent_name: antecedent_name, consequent_name: consequent_name, forum_topic: forum_topic) + tag_relationship.process! + end +end diff --git a/app/logical/bulk_update_request_processor.rb b/app/logical/bulk_update_request_processor.rb index f9604f1f0..0b6d96b18 100644 --- a/app/logical/bulk_update_request_processor.rb +++ b/app/logical/bulk_update_request_processor.rb @@ -4,7 +4,7 @@ class BulkUpdateRequestProcessor class Error < StandardError; end attr_reader :bulk_update_request - delegate :script, :forum_topic_id, to: :bulk_update_request + delegate :script, :forum_topic, to: :bulk_update_request validate :validate_script def initialize(bulk_update_request) @@ -105,12 +105,10 @@ class BulkUpdateRequestProcessor commands.map do |command, *args| case command when :create_alias - tag_alias = TagAlias.create!(creator: approver, forum_topic_id: forum_topic_id, status: "pending", antecedent_name: args[0], consequent_name: args[1]) - tag_alias.approve!(approver) + TagAlias.approve!(antecedent_name: args[0], consequent_name: args[1], approver: approver, forum_topic: forum_topic) when :create_implication - tag_implication = TagImplication.create!(creator: approver, forum_topic_id: forum_topic_id, status: "pending", antecedent_name: args[0], consequent_name: args[1]) - tag_implication.approve!(approver) + TagImplication.approve!(antecedent_name: args[0], consequent_name: args[1], approver: approver, forum_topic: forum_topic) when :remove_alias tag_alias = TagAlias.active.find_by!(antecedent_name: args[0], consequent_name: args[1]) diff --git a/app/models/artist.rb b/app/models/artist.rb index d059a8367..ab533b37b 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -192,8 +192,7 @@ class Artist < ApplicationRecord # 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", creator: banner) - tag_implication.approve!(banner) + TagImplication.approve!(antecedent_name: name, consequent_name: "banned_artist", approver: banner) end update!(is_banned: true) diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 9b3b0f42d..72fee2ccb 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -3,10 +3,6 @@ class TagAlias < TagRelationship validates_uniqueness_of :antecedent_name, scope: :status, conditions: -> { active } validate :absence_of_transitive_relation - def approve!(approver) - ProcessTagAliasJob.perform_later(self, approver) - end - def self.to_aliased(names) names = Array(names).map(&:to_s) return [] if names.empty? @@ -14,8 +10,7 @@ class TagAlias < TagRelationship names.map { |name| aliases[name] || name } end - def process!(approver) - update!(approver: approver, status: "active") + def process! TagMover.new(antecedent_name, consequent_name, user: User.system).move! rescue Exception => e update!(status: "error: #{e}") diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index c5be0ac91..052dac101 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -109,13 +109,8 @@ class TagImplication < TagRelationship end module ApprovalMethods - def process!(approver) - unless valid? - raise errors.full_messages.join("; ") - end - + def process! CurrentUser.scoped(User.system) do - update!(approver: approver, status: "active") update_posts end rescue Exception => e @@ -123,10 +118,6 @@ class TagImplication < TagRelationship DanbooruLogger.log(e, tag_implication_id: id, antecedent_name: antecedent_name, consequent_name: consequent_name) end - def approve!(approver) - ProcessTagImplicationJob.perform_later(self, approver) - end - def create_mod_action implication = %("tag implication ##{id}":[#{Rails.application.routes.url_helpers.tag_implication_path(self)}]: [[#{antecedent_name}]] -> [[#{consequent_name}]]) diff --git a/app/models/tag_relationship.rb b/app/models/tag_relationship.rb index 7eb6c020c..a1cab6891 100644 --- a/app/models/tag_relationship.rb +++ b/app/models/tag_relationship.rb @@ -17,11 +17,10 @@ class TagRelationship < ApplicationRecord scope :deleted, -> {where(status: "deleted")} scope :expired, -> {where("created_at < ?", EXPIRY.days.ago)} scope :old, -> {where("created_at >= ? and created_at < ?", EXPIRY.days.ago, EXPIRY_WARNING.days.ago)} - scope :pending, -> {where(status: "pending")} scope :retired, -> {where(status: "retired")} before_validation :normalize_names - validates_format_of :status, :with => /\A(active|deleted|pending|retired|error: .*)\Z/ + validates_format_of :status, :with => /\A(active|deleted|retired|error: .*)\Z/ validates_presence_of :antecedent_name, :consequent_name validates :approver, presence: { message: "must exist" }, if: -> { approver_id.present? } validates :forum_topic, presence: { message: "must exist" }, if: -> { forum_topic_id.present? } @@ -44,10 +43,6 @@ class TagRelationship < ApplicationRecord status == "deleted" end - def is_pending? - status == "pending" - end - def is_active? status == "active" end @@ -74,11 +69,6 @@ class TagRelationship < ApplicationRecord where(field => Tag.search(params).reorder(nil).select(:name)) end - def pending_first - # unknown statuses return null and are sorted first - order(Arel.sql("array_position(array['pending', 'active', 'deleted', 'retired'], status::text) NULLS FIRST, id DESC")) - end - def search(params) q = super q = q.search_attributes(params, :antecedent_name, :consequent_name) @@ -107,8 +97,6 @@ class TagRelationship < ApplicationRecord q = q.order("antecedent_name asc, consequent_name asc") when "tag_count" q = q.joins(:consequent_tag).order("tags.post_count desc, antecedent_name asc, consequent_name asc") - when "status" - q = q.pending_first else q = q.apply_default_order(params) end @@ -144,6 +132,10 @@ class TagRelationship < ApplicationRecord end end + def self.approve!(antecedent_name:, consequent_name:, approver:, forum_topic: nil) + ProcessTagRelationshipJob.perform_later(class_name: self.name, approver: approver, antecedent_name: antecedent_name, consequent_name: consequent_name, forum_topic: forum_topic) + end + def self.model_restriction(table) super.where(table[:status].eq("active")) end diff --git a/app/views/tag_relationships/_search.html.erb b/app/views/tag_relationships/_search.html.erb index 719c921e3..21f2bb4cc 100644 --- a/app/views/tag_relationships/_search.html.erb +++ b/app/views/tag_relationships/_search.html.erb @@ -9,7 +9,7 @@ <%= f.simple_fields_for :consequent_tag do |fa| %> <%= fa.input :category, label: "To Category", collection: TagCategory.canonical_mapping.to_a, include_blank: true, selected: params.dig(:search, :consequent_tag, :category) %> <% end %> - <%= f.input :status, label: "Status", collection: %w[Active Pending Deleted Retired], include_blank: true, selected: params[:search][:status] %> - <%= f.input :order, label: "Order", collection: [%w[Created created_at], %w[Updated updated_at], %w[Name name], %w[Tag\ count tag_count], %w[Status status]], include_blank: true, selected: params[:search][:order] %> + <%= f.input :status, label: "Status", collection: %w[Active Deleted Retired], include_blank: true, selected: params[:search][:status] %> + <%= f.input :order, label: "Order", collection: [%w[Created created_at], %w[Updated updated_at], %w[Name name], %w[Tag\ count tag_count]], include_blank: true, selected: params[:search][:order] %> <%= f.submit "Search" %> <% end %> diff --git a/db/migrate/20201201211748_change_default_status_on_tag_relationships.rb b/db/migrate/20201201211748_change_default_status_on_tag_relationships.rb new file mode 100644 index 000000000..549f4ea87 --- /dev/null +++ b/db/migrate/20201201211748_change_default_status_on_tag_relationships.rb @@ -0,0 +1,6 @@ +class ChangeDefaultStatusOnTagRelationships < ActiveRecord::Migration[6.0] + def change + change_column_default(:tag_aliases, :status, from: "pending", to: "active") + change_column_default(:tag_implications, :status, from: "pending", to: "active") + end +end diff --git a/db/structure.sql b/db/structure.sql index c5813449e..dd4227258 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2857,7 +2857,7 @@ CREATE TABLE public.tag_aliases ( consequent_name character varying NOT NULL, creator_id integer NOT NULL, forum_topic_id integer, - status text DEFAULT 'pending'::text NOT NULL, + status text DEFAULT 'active'::text NOT NULL, created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL, approver_id integer, @@ -2894,7 +2894,7 @@ CREATE TABLE public.tag_implications ( consequent_name character varying NOT NULL, creator_id integer NOT NULL, forum_topic_id integer, - status text DEFAULT 'pending'::text NOT NULL, + status text DEFAULT 'active'::text NOT NULL, created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL, approver_id integer, @@ -7419,6 +7419,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200427190519'), ('20200520060951'), ('20200803022359'), -('20200816175151'); +('20200816175151'), +('20201201211748'); diff --git a/test/functional/artists_controller_test.rb b/test/functional/artists_controller_test.rb index 11ebae670..352d34a78 100644 --- a/test/functional/artists_controller_test.rb +++ b/test/functional/artists_controller_test.rb @@ -107,10 +107,13 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest context "ban action" do should "ban an artist" do - put_auth ban_artist_path(@artist.id), @admin + perform_enqueued_jobs do + put_auth ban_artist_path(@artist.id), @admin + end + assert_redirected_to(@artist) assert_equal(true, @artist.reload.is_banned?) - assert_equal(true, TagImplication.exists?(antecedent_name: @artist.name, consequent_name: "banned_artist")) + assert_equal(true, TagImplication.exists?(antecedent_name: @artist.name, consequent_name: "banned_artist", status: "active")) end should "not allow non-admins to ban artists" do diff --git a/test/functional/bulk_update_requests_controller_test.rb b/test/functional/bulk_update_requests_controller_test.rb index f4c47f79d..b72db21bc 100644 --- a/test/functional/bulk_update_requests_controller_test.rb +++ b/test/functional/bulk_update_requests_controller_test.rb @@ -156,12 +156,14 @@ class BulkUpdateRequestsControllerTest < ActionDispatch::IntegrationTest create(:tag, name: "artist2a", category: Tag.categories.artist, post_count: 20) @bulk_update_request = create(:bulk_update_request, script: "mass update artist1a -> artist1b\ncreate alias artist2a -> artist2b") - post_auth approve_bulk_update_request_path(@bulk_update_request), @builder + perform_enqueued_jobs do + post_auth approve_bulk_update_request_path(@bulk_update_request), @builder + end assert_redirected_to(bulk_update_requests_path) assert_equal("approved", @bulk_update_request.reload.status) assert_equal(@builder, @bulk_update_request.approver) - assert_equal(true, TagAlias.where(antecedent_name: "artist2a", consequent_name: "artist2b").exists?) + assert_equal(true, TagAlias.exists?(antecedent_name: "artist2a", consequent_name: "artist2b", status: "active")) end end diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index d0f36c3f3..18ad1ba37 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -161,6 +161,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest end should "show a notice for a single tag search with a pending BUR" do + create(:tag, name: "foo") create(:bulk_update_request, script: "create alias foo -> bar") get_auth posts_path(tags: "foo"), @user assert_select ".tag-change-notice" diff --git a/test/functional/tag_aliases_controller_test.rb b/test/functional/tag_aliases_controller_test.rb index 4e6111d83..2db17a743 100644 --- a/test/functional/tag_aliases_controller_test.rb +++ b/test/functional/tag_aliases_controller_test.rb @@ -18,7 +18,7 @@ class TagAliasesControllerTest < ActionDispatch::IntegrationTest @antecedent_wiki = create(:wiki_page, title: "touhou", body: "zun project") @consequent_wiki = create(:wiki_page, title: "touhou_project") - @other_alias = create(:tag_alias, antecedent_name: "touhou", consequent_name: "touhou_project", creator: @user, status: "pending", forum_topic: @forum_topic, forum_post: @forum_post) + @other_alias = create(:tag_alias, antecedent_name: "touhou", consequent_name: "touhou_project", creator: @user, status: "deleted", forum_topic: @forum_topic, forum_post: @forum_post) @unrelated_alias = create(:tag_alias) end @@ -30,7 +30,7 @@ class TagAliasesControllerTest < ActionDispatch::IntegrationTest should respond_to_search({}).with { [@unrelated_alias, @other_alias, @tag_alias] } should respond_to_search(antecedent_name: "aaa").with { @tag_alias } should respond_to_search(consequent_name: "bbb").with { @tag_alias } - should respond_to_search(status: "pending").with { @other_alias } + should respond_to_search(status: "deleted").with { @other_alias } context "using includes" do should respond_to_search(antecedent_tag: {post_count: 1000}).with { @other_alias } diff --git a/test/functional/tag_implications_controller_test.rb b/test/functional/tag_implications_controller_test.rb index f7f9b4187..e7355b422 100644 --- a/test/functional/tag_implications_controller_test.rb +++ b/test/functional/tag_implications_controller_test.rb @@ -18,7 +18,7 @@ class TagImplicationsControllerTest < ActionDispatch::IntegrationTest @antecedent_wiki = create(:wiki_page, title: "cannon", body: "made of fun") @consequent_wiki = create(:wiki_page, title: "weapon") - @other_implication = create(:tag_implication, antecedent_name: "cannon", consequent_name: "weapon", creator: @user, status: "pending", forum_topic: @forum_topic, forum_post: @forum_post) + @other_implication = create(:tag_implication, antecedent_name: "cannon", consequent_name: "weapon", creator: @user, status: "deleted", forum_topic: @forum_topic, forum_post: @forum_post) @unrelated_implication = create(:tag_implication) end @@ -30,7 +30,7 @@ class TagImplicationsControllerTest < ActionDispatch::IntegrationTest should respond_to_search({}).with { [@unrelated_implication, @other_implication, @tag_implication] } should respond_to_search(antecedent_name: "aaa").with { @tag_implication } should respond_to_search(consequent_name: "bbb").with { @tag_implication } - should respond_to_search(status: "pending").with { @other_implication } + should respond_to_search(status: "deleted").with { @other_implication } context "using includes" do should respond_to_search(antecedent_tag: {post_count: 10}).with { @other_implication } diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb index f63613241..7fcc08e9d 100644 --- a/test/unit/artist_test.rb +++ b/test/unit/artist_test.rb @@ -94,7 +94,6 @@ class ArtistTest < ActiveSupport::TestCase end should "create a new tag implication" do - perform_enqueued_jobs assert_equal(1, TagImplication.where(:antecedent_name => "aaa", :consequent_name => "banned_artist").count) assert_equal("aaa banned_artist", @post.reload.tag_string) end diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index 604ce57fa..87dfe6228 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -26,7 +26,6 @@ class TagAliasTest < ActiveSupport::TestCase should allow_value('active').for(:status) should allow_value('deleted').for(:status) - should allow_value('pending').for(:status) should allow_value('error: derp').for(:status) should_not allow_value('ACTIVE').for(:status) @@ -47,18 +46,17 @@ class TagAliasTest < ActiveSupport::TestCase ta2 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "retired") ta3 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") ta4 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") - ta5 = FactoryBot.create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb", status: "pending") - [ta1, ta2, ta3, ta4, ta5].each { |ta| assert(ta.valid?) } + [ta1, ta2, ta3, ta4].each { |ta| assert(ta.valid?) } - ta5.update(status: "active") - assert_includes(ta5.errors[:antecedent_name], "has already been taken") + ta4.update(status: "active") + assert_includes(ta4.errors[:antecedent_name], "has already been taken") end end context "#reject!" do should "not be blocked by validations" do ta1 = create(:tag_alias, antecedent_name: "kitty", consequent_name: "kitten", status: "active") - ta2 = build(:tag_alias, antecedent_name: "cat", consequent_name: "kitty", status: "pending") + ta2 = build(:tag_alias, antecedent_name: "cat", consequent_name: "kitty", status: "active") ta2.reject! assert_equal("deleted", ta2.reload.status) @@ -89,9 +87,8 @@ class TagAliasTest < ActiveSupport::TestCase @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!(@admin) + TagAlias.approve!(antecedent_name: "...", consequent_name: "bbb", approver: @admin) perform_enqueued_jobs assert_equal("123 bbb 456", @ss1.reload.query) @@ -111,9 +108,8 @@ class TagAliasTest < ActiveSupport::TestCase @u5 = create(:user, blacklisted_tags: "111 ...") @u6 = create(:user, blacklisted_tags: "111 222\n\n... 333\n") @u7 = create(:user, blacklisted_tags: "111 ...\r\n222 333\n") - @ta = create(:tag_alias, antecedent_name: "...", consequent_name: "aaa") - @ta.approve!(@admin) + TagAlias.approve!(antecedent_name: "...", consequent_name: "aaa", approver: @admin) perform_enqueued_jobs assert_equal("111 aaa 222", @u1.reload.blacklisted_tags) @@ -130,8 +126,7 @@ class TagAliasTest < ActiveSupport::TestCase post1 = FactoryBot.create(:post, :tag_string => "aaa bbb") post2 = FactoryBot.create(:post, :tag_string => "ccc ddd") - ta = FactoryBot.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "ccc") - ta.approve!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "ccc", approver: @admin) perform_enqueued_jobs assert_equal("bbb ccc", post1.reload.tag_string) @@ -151,11 +146,9 @@ class TagAliasTest < ActiveSupport::TestCase 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!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", approver: @admin) perform_enqueued_jobs - assert_equal("active", @ta.reload.status) assert_equal("bbb", @wiki.reload.title) end @@ -163,11 +156,9 @@ class TagAliasTest < ActiveSupport::TestCase 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!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", approver: @admin) perform_enqueued_jobs - assert_equal("active", @ta.reload.status) assert_equal(true, @wiki1.reload.is_deleted) assert_equal([], @wiki1.other_names) @@ -181,11 +172,9 @@ class TagAliasTest < ActiveSupport::TestCase 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!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", 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) @@ -198,11 +187,9 @@ class TagAliasTest < ActiveSupport::TestCase should "rewrite links in other wikis to use the new tag" do @wiki = create(:wiki_page, body: "foo [[aaa]] bar") - @ta = create(:tag_alias, antecedent_name: "aaa", consequent_name: "bbb") - @ta.approve!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", approver: @admin) perform_enqueued_jobs - assert_equal("active", @ta.reload.status) assert_equal("foo [[bbb]] bar", @wiki.reload.body) end @@ -211,11 +198,9 @@ class TagAliasTest < ActiveSupport::TestCase 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!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", approver: @admin) perform_enqueued_jobs - assert_equal("active", @ta.reload.status) assert_equal("bbb", @artist.reload.name) end @@ -224,11 +209,9 @@ class TagAliasTest < ActiveSupport::TestCase @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!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", 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) @@ -244,11 +227,9 @@ class TagAliasTest < ActiveSupport::TestCase 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!(@admin) + TagAlias.approve!(antecedent_name: "aaa", consequent_name: "bbb", 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) @@ -265,9 +246,8 @@ class TagAliasTest < ActiveSupport::TestCase should "push the consequent's category to the antecedent if the antecedent is general" do tag1 = create(:tag, name: "general", category: 0) tag2 = create(:tag, name: "artist", category: 1) - ta = create(:tag_alias, antecedent_name: "general", consequent_name: "artist") - ta.approve!(@admin) + TagAlias.approve!(antecedent_name: "general", consequent_name: "artist", approver: @admin) perform_enqueued_jobs assert_equal(1, tag1.reload.category) @@ -277,9 +257,8 @@ class TagAliasTest < ActiveSupport::TestCase should "push the antecedent's category to the consequent if the consequent is general" do tag1 = create(:tag, name: "artist", category: 1) tag2 = create(:tag, name: "general", category: 0) - ta = create(:tag_alias, antecedent_name: "artist", consequent_name: "general") - ta.approve!(@admin) + TagAlias.approve!(antecedent_name: "artist", consequent_name: "general", approver: @admin) perform_enqueued_jobs assert_equal(1, tag1.reload.category) @@ -289,9 +268,8 @@ class TagAliasTest < ActiveSupport::TestCase should "not change either tag category when neither the antecedent or consequent are general" do tag1 = create(:tag, name: "character", category: 4) tag2 = create(:tag, name: "copyright", category: 3) - ta = create(:tag_alias, antecedent_name: "character", consequent_name: "copyright") - ta.approve!(@admin) + TagAlias.approve!(antecedent_name: "character", consequent_name: "copyright", approver: @admin) perform_enqueued_jobs assert_equal(4, tag1.reload.category) diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index a71852814..f284d489b 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -22,7 +22,6 @@ class TagImplicationTest < ActiveSupport::TestCase should allow_value('active').for(:status) should allow_value('deleted').for(:status) - should allow_value('pending').for(:status) should allow_value('error: derp').for(:status) should_not allow_value('ACTIVE').for(:status) @@ -43,17 +42,16 @@ class TagImplicationTest < ActiveSupport::TestCase ti2 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "retired") ti3 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") ti4 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "deleted") - ti5 = FactoryBot.create(:tag_implication, antecedent_name: "aaa", consequent_name: "bbb", status: "pending") - [ti1, ti2, ti3, ti4, ti5].each { |ti| assert(ti.valid?) } + [ti1, ti2, ti3, ti4].each { |ti| assert(ti.valid?) } - ti5.update(status: "active") - assert_includes(ti5.errors[:antecedent_name], "Implication already exists") + ti4.update(status: "active") + assert_includes(ti4.errors[:antecedent_name], "Implication already exists") end end context "#reject!" do should "not be blocked by alias validations" do - ti = create(:tag_implication, antecedent_name: "cat", consequent_name: "animal", status: "pending") + ti = create(:tag_implication, antecedent_name: "cat", consequent_name: "animal", status: "active") ta = create(:tag_alias, antecedent_name: "cat", consequent_name: "kitty", status: "active") ti.reject! @@ -120,11 +118,9 @@ class TagImplicationTest < ActiveSupport::TestCase should "update any affected post upon save" do p1 = FactoryBot.create(:post, :tag_string => "aaa bbb ccc") - ti1 = FactoryBot.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "xxx") - ti2 = FactoryBot.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "yyy") - ti1.approve!(@admin) - ti2.approve!(@admin) + TagImplication.approve!(antecedent_name: "aaa", consequent_name: "xxx", approver: @admin) + TagImplication.approve!(antecedent_name: "aaa", consequent_name: "yyy", approver: @admin) perform_enqueued_jobs assert_equal("aaa bbb ccc xxx yyy", p1.reload.tag_string) @@ -149,7 +145,7 @@ class TagImplicationTest < ActiveSupport::TestCase should "not include inactive implications" do create(:tag_implication, antecedent_name: "a", consequent_name: "b", status: "active") - create(:tag_implication, antecedent_name: "b", consequent_name: "c", status: "pending") + create(:tag_implication, antecedent_name: "b", consequent_name: "c", status: "deleted") create(:tag_implication, antecedent_name: "c", consequent_name: "d", status: "active") assert_equal(["b"], TagImplication.tags_implied_by("a").map(&:name))