diff --git a/backend/app/controllers/materials_controller.rb b/backend/app/controllers/materials_controller.rb index 17cd4d0..e4ada01 100644 --- a/backend/app/controllers/materials_controller.rb +++ b/backend/app/controllers/materials_controller.rb @@ -237,7 +237,7 @@ class MaterialsController < ApplicationController end def resolve_material_tag! tag_name_raw - tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw) + tag_name = TagName.find_or_create_by!(name: tag_name_raw) tag = tag_name.tag tag || Tag.create!(tag_name:, category: :material) end diff --git a/backend/app/controllers/tags_controller.rb b/backend/app/controllers/tags_controller.rb index a940f0b..f152d19 100644 --- a/backend/app/controllers/tags_controller.rb +++ b/backend/app/controllers/tags_controller.rb @@ -573,7 +573,7 @@ class TagsController < ApplicationController return false end - target_tag_name = TagName.with_discarded.find_by(name:) + target_tag_name = TagName.find_by(name:) return true if target_tag_name.nil? return true if target_tag_name.canonical_id? @@ -585,17 +585,14 @@ class TagsController < ApplicationController return if name == tag.name current_tag_name = tag.tag_name - target_tag_name = TagName.with_discarded.find_by(name:) + target_tag_name = TagName.find_by(name:) if target_tag_name.nil? current_tag_name.update!(name:) return end - promote_tag_alias!( - tag, - current_tag_name:, - promoted_tag_name: target_tag_name) + promote_tag_alias!(tag, current_tag_name:, promoted_tag_name: target_tag_name) end def promote_tag_alias! tag, current_tag_name:, promoted_tag_name: @@ -605,11 +602,9 @@ class TagsController < ApplicationController TagVersioning.ensure_snapshot!(old_owner_tag, created_by_user: current_user) end - promoted_tag_name.undiscard! if promoted_tag_name.discarded? promoted_tag_name.update!(canonical: nil) - TagName.with_discarded - .where(canonical_id: current_tag_name.id) + TagName.where(canonical_id: current_tag_name.id) .where.not(id: promoted_tag_name.id) .find_each do |alias_tag_name| alias_tag_name.update!(canonical: promoted_tag_name) @@ -640,7 +635,7 @@ class TagsController < ApplicationController end alias_names.each do |alias_name| - alias_tag_name = TagName.find_undiscard_or_create_by!(name: alias_name) + alias_tag_name = TagName.find_or_create_by!(name: alias_name) affected_tags << alias_tag_name.canonical&.tag end @@ -655,7 +650,7 @@ class TagsController < ApplicationController end alias_names.each do |alias_name| - alias_tag_name = TagName.find_undiscard_or_create_by!(name: alias_name) + alias_tag_name = TagName.find_or_create_by!(name: alias_name) alias_tag_name.update!(canonical: tag.tag_name) end diff --git a/backend/app/controllers/wiki_pages_controller.rb b/backend/app/controllers/wiki_pages_controller.rb index f3ece93..9b38b0e 100644 --- a/backend/app/controllers/wiki_pages_controller.rb +++ b/backend/app/controllers/wiki_pages_controller.rb @@ -94,7 +94,7 @@ class WikiPagesController < ApplicationController return render_unprocessable_entity('タイトルは必須です．', field: :title) if title.blank? return render_unprocessable_entity('本文は必須です．', field: :body) if body.blank? - tag_name = TagName.find_undiscard_or_create_by!(name: title) + tag_name = TagName.find_or_create_by!(name: title) page = Wiki::Commit.create_content!( diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index 4d7a851..e4fd44c 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -63,16 +63,19 @@ class Post < ApplicationRecord has_many :user_post_views, dependent: :delete_all has_many :post_similarities, dependent: :delete_all has_many :post_versions + has_many :gekanator_guessed_games, class_name: 'GekanatorGame', foreign_key: :guessed_post_id, dependent: :delete_all, inverse_of: :guessed_post + has_many :gekanator_correct_games, class_name: 'GekanatorGame', foreign_key: :correct_post_id, dependent: :delete_all, inverse_of: :correct_post + has_many :gekanator_question_examples, dependent: :delete_all has_many :parent_post_implications, diff --git a/backend/app/models/tag.rb b/backend/app/models/tag.rb index 4a4598f..8a09ef4 100644 --- a/backend/app/models/tag.rb +++ b/backend/app/models/tag.rb @@ -2,8 +2,6 @@ require 'set' class Tag < ApplicationRecord - include MyDiscard - class NicoTagNormalisationError < ArgumentError ; end @@ -232,10 +230,10 @@ class Tag < ApplicationRecord end def self.find_or_create_by_tag_name! name, category: - tn = TagName.find_undiscard_or_create_by!(name: name.to_s.strip) + tn = TagName.find_or_create_by!(name: name.to_s.strip) tn = tn.canonical if tn.canonical_id? - Tag.find_undiscard_or_create_by!(tag_name_id: tn.id) do |t| + Tag.find_or_create_by!(tag_name_id: tn.id) do |t| t.category = category end rescue ActiveRecord::RecordNotUnique @@ -273,10 +271,10 @@ class Tag < ApplicationRecord end TagVersioning.record!(source_tag, event_type: :discard, created_by_user:) - source_tag.discard! + source_tag.destroy! if source_tag.nico? - source_tag_name.discard! + source_tag_name.destroy! else source_tag_name.update_columns(canonical_id: target_tag.tag_name_id, updated_at: Time.current) @@ -297,7 +295,7 @@ class Tag < ApplicationRecord target_tag.reload end - def snapshot_aliases = tag_name.aliases.kept.order(:name).pluck(:name) + def snapshot_aliases = tag_name.aliases.order(:name).pluck(:name) def snapshot_parent_tag_ids = parents.order(:id).pluck(:id) diff --git a/backend/app/models/tag_name.rb b/backend/app/models/tag_name.rb index de79d10..fe12a06 100644 --- a/backend/app/models/tag_name.rb +++ b/backend/app/models/tag_name.rb @@ -1,6 +1,4 @@ class TagName < ApplicationRecord - include MyDiscard - has_one :tag has_one :wiki_page diff --git a/backend/app/models/tag_name_sanitisation_rule.rb b/backend/app/models/tag_name_sanitisation_rule.rb index e37738a..bb13a53 100644 --- a/backend/app/models/tag_name_sanitisation_rule.rb +++ b/backend/app/models/tag_name_sanitisation_rule.rb @@ -32,7 +32,7 @@ class TagNameSanitisationRule < ApplicationRecord elsif source_tag source_tag.update_columns(tag_name_id: existing_tn.id, updated_at: Time.current) end - tn.discard! + tn.destroy! next end diff --git a/backend/db/migrate/20260921020000_delete_discarded_records_from_tags.rb b/backend/db/migrate/20260921020000_delete_discarded_records_from_tags.rb new file mode 100644 index 0000000..a9af657 --- /dev/null +++ b/backend/db/migrate/20260921020000_delete_discarded_records_from_tags.rb @@ -0,0 +1,46 @@ +class DeleteDiscardedRecordsFromTags < ActiveRecord::Migration[8.0] + def up + remove_foreign_key :tag_versions, :tags, column: :tag_id + remove_foreign_key :nico_tag_versions, :tags, column: :tag_id + remove_foreign_key :material_versions, :tags, column: :tag_id + + execute <<~SQL + DELETE + ntr + FROM + nico_tag_relations ntr + INNER JOIN + tags t + ON + t.discarded_at IS NOT NULL + AND t.id IN (ntr.tag_id, ntr.nico_tag_id) + SQL + + execute <<~SQL + DELETE + ti + FROM + tag_implications ti + INNER JOIN + tags t + ON + t.discarded_at IS NOT NULL + AND t.id IN (ti.tag_id, ti.parent_tag_id) + SQL + + execute <<~SQL + DELETE + FROM + tags + WHERE + discarded_at IS NOT NULL + SQL + + remove_index :tags, :discarded_at + remove_column :tags, :discarded_at + end + + def down + raise ActiveRecord::IrreversibleMigration, '戻せません．' + end +end diff --git a/backend/db/migrate/20260921030000_delete_discarded_records_from_tag_names.rb b/backend/db/migrate/20260921030000_delete_discarded_records_from_tag_names.rb new file mode 100644 index 0000000..fe1d5ca --- /dev/null +++ b/backend/db/migrate/20260921030000_delete_discarded_records_from_tag_names.rb @@ -0,0 +1,18 @@ +class DeleteDiscardedRecordsFromTagNames < ActiveRecord::Migration[8.0] + def up + execute <<~SQL + DELETE + FROM + tag_names + WHERE + discarded_at IS NOT NULL + SQL + + remove_index :tag_names, :discarded_at + remove_column :tag_names, :discarded_at + end + + def down + raise ActiveRecord::IrreversibleMigration, '戻せません．' + end +end diff --git a/backend/db/schema.rb b/backend/db/schema.rb index f4ca18c..1cb6423 100644 --- a/backend/db/schema.rb +++ b/backend/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_09_21_010000) do +ActiveRecord::Schema[8.0].define(version: 2026_09_21_030000) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -409,9 +409,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_09_21_010000) do t.bigint "canonical_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.datetime "discarded_at" t.index ["canonical_id"], name: "index_tag_names_on_canonical_id" - t.index ["discarded_at"], name: "index_tag_names_on_discarded_at" t.index ["name"], name: "index_tag_names_on_name", unique: true end @@ -448,10 +446,8 @@ ActiveRecord::Schema[8.0].define(version: 2026_09_21_010000) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "post_count", default: 0, null: false - t.datetime "discarded_at" t.integer "version_no", null: false t.index ["deprecated_at"], name: "index_tags_on_deprecated_at" - t.index ["discarded_at"], name: "index_tags_on_discarded_at" t.index ["tag_name_id"], name: "index_tags_on_tag_name_id", unique: true t.check_constraint "(`deprecated_at` is null) or (`category` <> _utf8mb4'nico')", name: "chk_tags_deprecated_at_not_nico" t.check_constraint "`version_no` > 0", name: "chk_tags_version_no_positive" @@ -698,7 +694,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_09_21_010000) do add_foreign_key "material_sync_suppressions", "users", column: "created_by_user_id" add_foreign_key "material_versions", "materials" add_foreign_key "material_versions", "materials", column: "parent_id" - add_foreign_key "material_versions", "tags" add_foreign_key "material_versions", "users", column: "created_by_user_id" add_foreign_key "material_versions", "users", column: "updated_by_user_id" add_foreign_key "materials", "materials", column: "parent_id" @@ -707,7 +702,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_09_21_010000) do add_foreign_key "materials", "users", column: "updated_by_user_id" add_foreign_key "nico_tag_relations", "tags" add_foreign_key "nico_tag_relations", "tags", column: "nico_tag_id" - add_foreign_key "nico_tag_versions", "tags" add_foreign_key "nico_tag_versions", "users", column: "created_by_user_id" add_foreign_key "post_implications", "posts" add_foreign_key "post_implications", "posts", column: "parent_post_id" @@ -726,7 +720,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_09_21_010000) do add_foreign_key "tag_names", "tag_names", column: "canonical_id" add_foreign_key "tag_similarities", "tags" add_foreign_key "tag_similarities", "tags", column: "target_tag_id" - add_foreign_key "tag_versions", "tags" add_foreign_key "tag_versions", "users", column: "created_by_user_id" add_foreign_key "tags", "tag_names" add_foreign_key "theatre_comments", "theatres" diff --git a/backend/spec/db/delete_discarded_tags_spec.rb b/backend/spec/db/delete_discarded_tags_spec.rb new file mode 100644 index 0000000..3882e0d --- /dev/null +++ b/backend/spec/db/delete_discarded_tags_spec.rb @@ -0,0 +1,123 @@ +require 'rails_helper' +require_relative '../../db/migrate/20260921020000_delete_discarded_records_from_tags' +require_relative '../../db/migrate/20260921030000_delete_discarded_records_from_tag_names' + +RSpec.describe 'discarded tag cleanup migrations' do + [DeleteDiscardedRecordsFromTags, DeleteDiscardedRecordsFromTagNames].each do |migration_class| + it "rejects rollback of #{ migration_class.name }" do + expect { migration_class.new.down } + .to raise_error(ActiveRecord::IrreversibleMigration) + end + end + + context 'with legacy records' do + self.use_transactional_tests = false + + before do + record_class = Class.new(ActiveRecord::Base) do + self.abstract_class = true + end + stub_const('TagCleanupMigrationRecord', record_class) + config = ActiveRecord::Base.connection_db_config.configuration_hash + @database = "btrc_hub_test_tag_cleanup_#{ Process.pid }_#{ SecureRandom.hex(4) }" + record_class.establish_connection(config.merge(database: nil)) + @connection = record_class.lease_connection + @connection.create_database(@database) + @database_created = true + @connection.execute("USE #{ @connection.quote_table_name(@database) }") + end + + after do + @connection.drop_database(@database) if @database_created + ensure + TagCleanupMigrationRecord.remove_connection + end + + before do + @connection.create_table(:tag_names) do |t| + t.string :name, null: false, index: { unique: true } + t.bigint :canonical_id + t.datetime :discarded_at, index: true + end + @connection.add_foreign_key(:tag_names, :tag_names, column: :canonical_id) + @connection.create_table(:tags) do |t| + t.references :tag_name, null: false, foreign_key: true, index: { unique: true } + t.datetime :discarded_at, index: true + end + @connection.create_table(:nico_tag_relations) do |t| + t.references :tag, null: false, foreign_key: true + t.references :nico_tag, null: false, foreign_key: { to_table: :tags } + end + @connection.create_table(:tag_implications) do |t| + t.references :tag, null: false, foreign_key: true + t.references :parent_tag, null: false, foreign_key: { to_table: :tags } + end + [:tag_versions, :nico_tag_versions, :material_versions].each do |table| + @connection.create_table(table) do |t| + t.references :tag, null: false, foreign_key: true + end + end + + @connection.execute(<<~SQL) + INSERT INTO tag_names (id, name, canonical_id, discarded_at) VALUES + (1, 'kept', NULL, NULL), + (2, 'merged_alias', 1, NULL), + (3, 'nico:deleted', NULL, '2026-09-20'), + (4, 'nico:kept', NULL, NULL), + (5, 'deleted_name', NULL, '2026-09-20') + SQL + @connection.execute(<<~SQL) + INSERT INTO tags (id, tag_name_id, discarded_at) VALUES + (1, 1, NULL), (2, 2, '2026-09-20'), + (3, 3, '2026-09-20'), (4, 4, NULL) + SQL + @connection.execute(<<~SQL) + INSERT INTO nico_tag_relations (id, tag_id, nico_tag_id) VALUES + (1, 1, 4), (2, 2, 4), (3, 1, 3), (4, 2, 3) + SQL + @connection.execute(<<~SQL) + INSERT INTO tag_implications (id, tag_id, parent_tag_id) VALUES + (1, 1, 4), (2, 2, 1), (3, 1, 2), (4, 2, 3) + SQL + @connection.execute('INSERT INTO tag_versions (tag_id) VALUES (1), (2)') + @connection.execute('INSERT INTO nico_tag_versions (tag_id) VALUES (3), (4)') + @connection.execute('INSERT INTO material_versions (tag_id) VALUES (1), (2)') + end + + it 'removes discarded records and their links while retaining aliases and history' do + [DeleteDiscardedRecordsFromTags, DeleteDiscardedRecordsFromTagNames].each do |klass| + migration = klass.new + allow(migration).to receive(:connection).and_return(@connection) + migration.suppress_messages { migration.up } + end + + expect(@connection.select_values('SELECT id FROM tags ORDER BY id')).to eq([1, 4]) + expect(@connection.select_rows('SELECT id, canonical_id FROM tag_names ORDER BY id')) + .to eq([[1, nil], [2, 1], [4, nil]]) + expect(@connection.select_values('SELECT id FROM nico_tag_relations')).to eq([1]) + expect(@connection.select_values('SELECT id FROM tag_implications')).to eq([1]) + expect(@connection.select_values('SELECT tag_id FROM tag_versions ORDER BY tag_id')) + .to eq([1, 2]) + expect(@connection.select_values('SELECT tag_id FROM nico_tag_versions ORDER BY tag_id')) + .to eq([3, 4]) + expect(@connection.select_values('SELECT tag_id FROM material_versions ORDER BY tag_id')) + .to eq([1, 2]) + + [:tags, :tag_names].each do |table| + expect(@connection.column_exists?(table, :discarded_at)).to be(false) + expect(@connection.index_exists?(table, :discarded_at)).to be(false) + end + [:tag_versions, :nico_tag_versions, :material_versions].each do |table| + expect(@connection.foreign_key_exists?(table, :tags, column: :tag_id)).to be(false) + end + expect(@connection.foreign_key_exists?(:tags, :tag_names)).to be(true) + expect(@connection.foreign_key_exists?(:tag_names, :tag_names, column: :canonical_id)) + .to be(true) + expect(@connection.index_exists?(:tag_names, :name, unique: true)).to be(true) + expect(@connection.index_exists?(:tags, :tag_name_id, unique: true)).to be(true) + [:nico_tag_relations, :tag_implications].each do |table| + expect(@connection.foreign_keys(table).map(&:to_table)).to eq(['tags', 'tags']) + end + end + end +end diff --git a/backend/spec/models/tag_name_sanitisation_rule_spec.rb b/backend/spec/models/tag_name_sanitisation_rule_spec.rb index ea14461..b82f9fa 100644 --- a/backend/spec/models/tag_name_sanitisation_rule_spec.rb +++ b/backend/spec/models/tag_name_sanitisation_rule_spec.rb @@ -57,7 +57,7 @@ RSpec.describe TagNameSanitisationRule, type: :model do it 'deletes the source tag_name' do described_class.apply! - expect(TagName.exists?(source.id)).to be(false) + expect(TagName.unscoped.exists?(source.id)).to be(false) expect(existing.reload.name).to eq('foobar') end end @@ -75,7 +75,27 @@ RSpec.describe TagNameSanitisationRule, type: :model do described_class.apply! expected_tag_name_id = existing.canonical_id || existing.id expect(source_tag.reload.tag_name_id).to eq(expected_tag_name_id) - expect(TagName.exists?(source_tag_name_id)).to be(false) + expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false) + end + end + + context 'when the sanitised name is an alias of an existing tag' do + let!(:existing_tag) { create(:tag) } + let!(:alias_name) do + TagName.create!(name: 'foobar', canonical: existing_tag.tag_name) + end + let!(:source) do + TagName.create!(name: 'tmp').tap do |tn| + tn.update_columns(name: 'foo_bar', updated_at: Time.current) + end + end + + it 'deletes only the source and preserves the alias and its canonical tag' do + described_class.apply! + + expect(TagName.unscoped.exists?(source.id)).to be(false) + expect(alias_name.reload.canonical).to eq(existing_tag.tag_name) + expect(Tag.find(existing_tag.id)).to eq(existing_tag) end end @@ -92,13 +112,15 @@ RSpec.describe TagNameSanitisationRule, type: :model do end it 'merges the source tag into the existing tag and deletes the source tag_name' do - expect(TagName.find_by(name: 'foobar')&.tag&.id).to eq(existing_tag.id) - expect(TagName.find_by(name: 'foo_bar')&.tag&.id).to eq(source_tag.id) + post = create(:post) + PostTag.create!(post:, tag: source_tag) described_class.apply! - expect(Tag.exists?(source_tag.id)).to be(false) - expect(TagName.exists?(source_tag.tag_name_id)).to be(false) + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false) + expect(post.reload.tags).to contain_exactly(existing_tag) + expect(existing_tag.reload.name).to eq('foobar') end end end diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index 79b0ac0..e532e25 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -173,6 +173,47 @@ RSpec.describe Tag, type: :model do end end + describe '.find_or_create_by_tag_name!' do + it 'creates a tag and name with the requested category after stripping whitespace' do + tag = nil + + expect { + tag = described_class.find_or_create_by_tag_name!( + ' lookup_new ', category: :character) + }.to change(Tag, :count).by(1).and change(TagName, :count).by(1) + + expect(tag.name).to eq('lookup_new') + expect(tag.category).to eq('character') + end + + it 'reuses the canonical tag for an alias without changing its category' do + tag = create(:tag, category: :character) + alias_name = TagName.create!(name: 'lookup_alias', canonical: tag.tag_name) + found = nil + + expect { + found = described_class.find_or_create_by_tag_name!( + alias_name.name, category: :general) + }.to change(Tag, :count).by(0).and change(TagName, :count).by(0) + + expect(found).to eq(tag) + expect(found.category).to eq('character') + end + + it 'creates a tag for an existing canonical name reached through an alias' do + canonical = create(:tag_name) + alias_name = TagName.create!(name: 'lookup_alias', canonical:) + tag = nil + + expect { + tag = described_class.find_or_create_by_tag_name!( + alias_name.name, category: :general) + }.to change(Tag, :count).by(1).and change(TagName, :count).by(0) + + expect(tag.tag_name).to eq(canonical) + end + end + describe '.merge_tags!' do let!(:target_tag) { create(:tag, category: :general) } let!(:source_tag) { create(:tag, category: :general) } @@ -185,17 +226,14 @@ RSpec.describe Tag, type: :model do context 'when merging a simple source tag' do let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) } - it 'deletes the source link, links the target, and aliases the discarded source tag' do + it 'deletes the source tag, moves its post link, and keeps its name as an alias' do described_class.merge_tags!(target_tag, [source_tag]) target_link = PostTag.find_by(post: post_record, tag: target_tag) expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) expect(target_link).to be_present - expect(source_tag.reload.post_count).to eq(0) - - expect(Tag.with_discarded.find(source_tag.id)).to be_discarded - expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end @@ -218,17 +256,76 @@ RSpec.describe Tag, type: :model do expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) expect(target_links).to contain_exactly(target_post_tag) - expect(source_tag.reload.post_count).to eq(0) expect(PostTagSection.where(post: post_record, tag: source_tag)).to be_empty expect(target_post_tag.reload.sections).to contain_exactly(target_section) - expect(Tag.with_discarded.find(source_tag.id)).to be_discarded - expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end end + it 'keeps source history and records the new target alias after deleting the source' do + user = create_member_user! + source_name = source_tag.name + source_alias = TagName.create!(name: 'merge_alias', canonical: source_tag_name) + TagVersioning.ensure_snapshot!(source_tag, created_by_user: user) + original_version = source_tag.tag_versions.first + + described_class.merge_tags!(target_tag, [source_tag], created_by_user: user) + + versions = TagVersion.where(tag_id: source_tag.id).order(:version_no) + expect(versions.pluck(:version_no, :event_type)) + .to eq([[1, 'create'], [2, 'discard']]) + expect(versions.first).to eq(original_version) + expect(versions.last).to have_attributes( + name: source_name, aliases: source_alias.name, created_by_user: user) + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + + target_versions = target_tag.tag_versions.order(:version_no) + expect(target_versions.pluck(:event_type)).to eq(['create', 'update']) + expect(target_versions.last.aliases.split).to eq([source_name]) + end + + it 'deletes source relationships while preserving unrelated relationships' do + parent = create(:tag) + child = create(:tag) + nico_tag = create(:tag, :nico) + TagImplication.create!(tag: source_tag, parent_tag: parent) + TagImplication.create!(tag: child, parent_tag: source_tag) + kept_implication = TagImplication.create!(tag: target_tag, parent_tag: parent) + NicoTagRelation.create!(tag: source_tag, nico_tag:) + kept_relation = NicoTagRelation.create!(tag: target_tag, nico_tag:) + TagSimilarity.create!(tag: source_tag, target_tag:, cos: 0.5) + TagSimilarity.create!(tag: target_tag, target_tag: source_tag, cos: 0.5) + kept_similarity = TagSimilarity.create!(tag: target_tag, target_tag: parent, + cos: 0.5) + + described_class.merge_tags!(target_tag, [source_tag]) + + expect(TagImplication.all).to contain_exactly(kept_implication) + expect(NicoTagRelation.all).to contain_exactly(kept_relation) + expect(TagSimilarity.all).to contain_exactly(kept_similarity) + expect(TagVersion.where(tag_id: source_tag.id).order(:version_no).last.parent_tag_ids) + .to eq(parent.id.to_s) + end + + it 'preserves material history referencing the deleted source tag' do + source_tag.update!(category: :material) + target_tag.update!(category: :material) + material = Material.create!(tag: source_tag, url: 'https://example.com/material') + version = MaterialVersionRecorder.record!( + material:, event_type: :create, created_by_user: nil) + material.update!(tag: target_tag) + + described_class.merge_tags!(target_tag, [source_tag]) + + expect(version.reload).to have_attributes( + tag_id: source_tag.id, tag_name: source_tag_name.name, tag_category: 'material') + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(material.reload.tag).to eq(target_tag) + end + context 'when source_tags includes the target itself' do let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) } @@ -238,7 +335,7 @@ RSpec.describe Tag, type: :model do target_link = PostTag.find_by(post: post_record, tag: target_tag) expect(Tag.find(target_tag.id)).to be_present - expect(Tag.with_discarded.find(source_tag.id)).to be_discarded + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) expect(target_link).to be_present expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) @@ -263,7 +360,7 @@ RSpec.describe Tag, type: :model do ) end - it 'still merges, but discards the source tag_name instead of aliasing it' do + it 'still merges and keeps the source name as an alias without validating it' do described_class.merge_tags!(target_tag, [source_tag]) target_link = PostTag.find_by(post: post_record, tag: target_tag) @@ -271,7 +368,8 @@ RSpec.describe Tag, type: :model do expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) expect(target_link).to be_present - expect(Tag.with_discarded.find(source_tag.id)).to be_discarded + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end end @@ -288,17 +386,23 @@ RSpec.describe Tag, type: :model do message: 'init') end - it 'rolls back the transaction' do + it 'rolls back earlier deletions, links, and history when a later source has a wiki' do + earlier_source = create(:tag) + earlier_name = earlier_source.tag_name source_section = create(:post_tag_section, post: post_record, tag: source_tag, begin_ms: 1000, end_ms: 2000) expect { - described_class.merge_tags!(target_tag, [source_tag]) + described_class.merge_tags!(target_tag, [earlier_source, source_tag]) }.to raise_error(ActiveRecord::RecordInvalid) - expect(Tag.with_discarded.find(source_tag.id)).not_to be_discarded - expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded + expect(Tag.unscoped.exists?(earlier_source.id)).to be(true) + expect(earlier_name.reload.canonical_id).to be_nil + expect(TagVersion.where(tag_id: [earlier_source.id, source_tag.id, target_tag.id])) + .to be_empty + expect(Tag.unscoped.exists?(source_tag.id)).to be(true) + expect(TagName.unscoped.exists?(source_tag_name.id)).to be(true) expect(source_post_tag.reload.tag_id).to eq(source_tag.id) expect(source_post_tag.sections).to contain_exactly(source_section) expect(PostTag.find_by(post: post_record, tag: target_tag)).to be_nil @@ -309,21 +413,45 @@ RSpec.describe Tag, type: :model do end context 'when merging a nico source tag' do - let!(:target_tag) { create(:tag, category: :nico, name: 'nico:foo') } - let!(:source_tag) { create(:tag, category: :nico, name: 'nico:bar') } + let!(:target_tag) do + create(:tag, category: :nico, tag_name: create(:tag_name, name: 'nico:foo')) + end + let!(:source_tag) do + create(:tag, category: :nico, tag_name: create(:tag_name, name: 'nico:bar')) + end let!(:source_tag_name_id) { source_tag.tag_name_id } - it 'discards the source tag_name instead of aliasing it' do + it 'deletes the source tag and name instead of keeping an alias' do described_class.merge_tags!(target_tag, [source_tag]) - discarded_source_tag = Tag.with_discarded.find(source_tag.id) - discarded_source_tag_name = TagName.with_discarded.find(source_tag_name_id) - - expect(discarded_source_tag).to be_discarded - expect(discarded_source_tag_name).to be_discarded - expect(discarded_source_tag_name.canonical_id).to be_nil + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false) expect(target_tag.reload.post_count).to eq(0) end + + it 'keeps nico history while deleting source links and allows recreating the name' do + linked_tag = create(:tag) + NicoTagRelation.create!(nico_tag: source_tag, tag: linked_tag) + kept_relation = NicoTagRelation.create!(nico_tag: target_tag, tag: linked_tag) + user = create_member_user! + source_name = source_tag.name + + described_class.merge_tags!(target_tag, [source_tag], created_by_user: user) + + expect(NicoTagRelation.all).to contain_exactly(kept_relation) + versions = NicoTagVersion.where(tag_id: source_tag.id).order(:version_no) + expect(versions.pluck(:version_no, :event_type)) + .to eq([[1, 'create'], [2, 'discard']]) + expect(versions.last).to have_attributes( + name: source_name, linked_tags: linked_tag.name, created_by_user: user) + + recreated = described_class.find_or_create_by_tag_name!(source_name, category: :nico) + + expect(recreated.id).not_to eq(source_tag.id) + expect(recreated.tag_name_id).not_to eq(source_tag_name_id) + expect(recreated.nico_tag_versions).to be_empty + expect(versions.reload.size).to eq(2) + end end def snapshot_tags(post) diff --git a/backend/spec/requests/posts_spec.rb b/backend/spec/requests/posts_spec.rb index 350b5e7..fb08d21 100644 --- a/backend/spec/requests/posts_spec.rb +++ b/backend/spec/requests/posts_spec.rb @@ -1252,9 +1252,7 @@ RSpec.describe 'Posts API', type: :request do context 'when nico tag already exists in tags' do before do - Tag.find_undiscard_or_create_by!( - tag_name: TagName.find_undiscard_or_create_by!(name: 'nico:nico_tag'), - category: :nico) + Tag.find_or_create_by_tag_name!('nico:nico_tag', category: :nico) end it 'returns 422 with tag field errors' do @@ -1580,9 +1578,7 @@ RSpec.describe 'Posts API', type: :request do context 'when nico tag already exists in tags' do before do - Tag.find_undiscard_or_create_by!( - tag_name: TagName.find_undiscard_or_create_by!(name: 'nico:nico_tag'), - category: :nico) + Tag.find_or_create_by_tag_name!('nico:nico_tag', category: :nico) end it 'returns 422 with tag field errors' do diff --git a/backend/spec/tasks/nico_sync_spec.rb b/backend/spec/tasks/nico_sync_spec.rb index f708608..1ec5b08 100644 --- a/backend/spec/tasks/nico_sync_spec.rb +++ b/backend/spec/tasks/nico_sync_spec.rb @@ -8,8 +8,7 @@ RSpec.describe 'nico:sync' do end def create_tag!(name, category:) - tn = TagName.find_undiscard_or_create_by!(name: name.to_s.strip) - Tag.find_undiscard_or_create_by!(tag_name_id: tn.id) { |t| t.category = category } + Tag.find_or_create_by_tag_name!(name, category:) end def link_nico_to_tag!(nico_tag, tag)