外部タグを分離 (#419) (#420)

**本番 DB のバックアップをかならずすること**

Reviewed-on: #420
Co-authored-by: miteruzo <miteruzo@naver.com>

backend/spec/models/external_tag_spec.rb

@@ -0,0 +1,38 @@
+require 'rails_helper'
+
+RSpec.describe ExternalTag, type: :model do
+  it 'preserves names without internal tag sanitisation or TagName records' do
+    external = nil
+
+    expect {
+      external = described_class.create!(platform: :nico, name: 'raw tag[]')
+    }.not_to change(TagName, :count)
+
+    expect(external.reload.name).to eq('raw tag[]')
+  end
+
+  it 'deletes post associations without deleting posts or version history' do
+    external = create(:external_tag)
+    post = create(:post)
+    PostExternalTag.create!(post:, external_tag: external)
+    version = NicoTagVersionRecorder.record!(
+        external_tag: external, event_type: :create, created_by_user: nil)
+
+    external.destroy!
+
+    expect(PostExternalTag.where(external_tag_id: external.id)).to be_empty
+    expect(post.reload.external_tags).to be_empty
+    expect(version.reload.tag_id).to eq(external.id)
+  end
+
+  it 'deletes external links without deleting linked internal tags' do
+    external = create(:external_tag)
+    tag = create(:tag)
+    NicoTagRelation.create!(nico_tag: external, tag:)
+
+    external.destroy!
+
+    expect(NicoTagRelation.where(nico_tag_id: external.id)).to be_empty
+    expect(Tag.exists?(tag.id)).to be(true)
+  end
+end

backend/spec/models/nico_tag_relation_spec.rb

@@ -0,0 +1,27 @@
+require 'rails_helper'
+
+RSpec.describe NicoTagRelation, type: :model do
+  it 'does not constrain the ExternalTag association to the nico platform' do
+    external_tag = create(:external_tag)
+    tag = create(:tag)
+    allow(external_tag).to receive(:platform).and_return('registered_external')
+    allow(external_tag).to receive(:nico?).and_return(false)
+
+    expect {
+      described_class.create!(nico_tag: external_tag, tag:)
+    }.to change(described_class, :count).by(1)
+    expect(tag.linked_nico_tags).to contain_exactly(external_tag)
+  end
+
+  it 'rejects an internal Tag through the external association type' do
+    expect {
+      described_class.new(nico_tag: create(:tag), tag: create(:tag))
+    }.to raise_error(ActiveRecord::AssociationTypeMismatch)
+  end
+
+  it 'rejects an ExternalTag through the internal association type' do
+    expect {
+      described_class.new(nico_tag: create(:external_tag), tag: create(:external_tag))
+    }.to raise_error(ActiveRecord::AssociationTypeMismatch)
+  end
+end

backend/spec/models/post_external_tag_spec.rb

@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+RSpec.describe PostExternalTag, type: :model do
+  it 'exposes both sides and enforces a unique post and external tag pair' do
+    post = create(:post)
+    external_tag = create(:external_tag)
+    described_class.create!(post:, external_tag:)
+
+    expect(post.external_tags).to contain_exactly(external_tag)
+    expect(external_tag.posts).to contain_exactly(post)
+    expect { described_class.create!(post:, external_tag:) }
+        .to raise_error(ActiveRecord::RecordNotUnique)
+  end
+
+  it 'removes associations when the post is deleted while retaining the external tag' do
+    post = create(:post)
+    external_tag = create(:external_tag)
+    described_class.create!(post:, external_tag:)
+
+    post.destroy!
+
+    expect(described_class.where(post_id: post.id)).to be_empty
+    expect(external_tag.reload.posts).to be_empty
+  end
+end

backend/spec/models/post_spec.rb

@@ -6,6 +6,25 @@ RSpec.describe Post, type: :model do
     PostUrlSanitisationRule.unscoped.delete_all
   end
 
+  describe '#snapshot_tags_json' do
+    it 'keeps internal snapshots and external identifiers distinct, even with the same id' do
+      post = create(:post)
+      tag = create(:tag)
+      external = create(:external_tag, id: tag.id)
+      create(:post_tag, post:, tag:)
+      create(:post_tag_section, post:, tag:, begin_ms: 2000, end_ms: nil)
+      PostExternalTag.create!(post:, external_tag: external)
+
+      expect(post.snapshot_tags_json).to eq([
+          { 'tag_id' => tag.id,
+            'version_no' => tag.version_no,
+            'name' => tag.name,
+            'category' => tag.category,
+            'sections' => [{ 'begin_ms' => 2000, 'end_ms' => nil }] },
+          { 'external_tag_id' => external.id }])
+    end
+  end
+
   describe 'URL normalisation' do
     it 'normalises the HTTP URL before applying sanitisation rules' do
       PostUrlSanitisationRule.create!(

backend/spec/models/tag_spec.rb

@@ -1,6 +1,29 @@
 require 'rails_helper'
 
 RSpec.describe Tag, type: :model do
+  describe 'external tag separation' do
+    ['nico:reserved', 'NiCo:reserved'].each do |name|
+      it "rejects the reserved prefix #{ name } for internal tags" do
+        tag = build(:tag, name:)
+
+        expect(tag).to be_invalid
+        expect(tag.errors[:name]).to be_present
+        expect {
+          described_class.normalise_tags!([name])
+        }.to raise_error(Tag::NicoTagNormalisationError)
+      end
+    end
+
+    it 'finds external links by the internal tag id even when ids differ' do
+      tag = create(:tag)
+      external = create(:external_tag, id: tag.id + 10_000)
+      # The migration retains existing links without running model validation.
+      NicoTagRelation.insert_all!([{ tag_id: tag.id, nico_tag_id: external.id }])
+
+      expect(tag.linked_nico_tags).to contain_exactly(external)
+    end
+  end
+
   describe '.normalise_tags!' do
     it 'rejects deprecated tags when deny_deprecated is enabled' do
       tag_name = TagName.create!(name: 'normalise deprecated tag')
@@ -159,20 +182,6 @@ RSpec.describe Tag, type: :model do
     end
   end
 
-  describe 'deprecated validation' do
-    it 'rejects deprecated nico tags' do
-      tag = build(
-        :tag,
-        name: 'nico:deprecated_validation',
-        category: :nico,
-        deprecated_at: Time.current
-      )
-
-      expect(tag).not_to be_valid
-      expect(tag.errors[:deprecated_at]).to include('ニコタグは廃止できません．')
-    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
@@ -290,7 +299,7 @@ RSpec.describe Tag, type: :model do
     it 'deletes source relationships while preserving unrelated relationships' do
       parent = create(:tag)
       child = create(:tag)
-      nico_tag = create(:tag, :nico)
+      nico_tag = create(:external_tag)
       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)
@@ -412,48 +421,6 @@ RSpec.describe Tag, type: :model do
       end
     end
 
-    context 'when merging a nico source tag' do
-      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 'deletes the source tag and name instead of keeping an alias' do
-        described_class.merge_tags!(target_tag, [source_tag])
-
-        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)
       post.snapshot_tag_names.join(' ')
     end
@@ -499,8 +466,8 @@ RSpec.describe Tag, type: :model do
         expect(latest.event_type).to eq('update')
         expect(latest.created_by_user).to be_nil
         expect(latest.tags).to eq(snapshot_tags(post_record.reload))
-        expect(latest.tags_json.map { |item| item.fetch('id') }).to eq([target_tag.id])
-        expect(affected_versions.first.tags_json.map { |item| item.fetch('id') })
+        expect(latest.tags_json.map { |item| item.fetch('tag_id') }).to eq([target_tag.id])
+        expect(affected_versions.first.tags_json.map { |item| item.fetch('tag_id') })
             .to eq([source_tag.id])
 
         expect(unaffected_post.reload.post_versions.count).to eq(1)

backend/spec/models/version_record_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 RSpec.describe VersionRecord, type: :model do
   let!(:tag) { create(:tag, name: 'version_record_tag') }
-  let!(:nico_tag) { create(:tag, :nico, name: 'nico:version_record_tag') }
+  let!(:nico_tag) { create(:external_tag, name: 'version_record_tag') }
 
   it 'makes TagVersion read only after create' do
     version = TagVersion.create!(
@@ -42,10 +42,10 @@ RSpec.describe VersionRecord, type: :model do
 
   it 'makes NicoTagVersion read only after create' do
     version = NicoTagVersion.create!(
-      tag: nico_tag,
+      external_tag: nico_tag,
       version_no: 1,
       event_type: 'create',
-      name: nico_tag.name,
+      name: "nico:#{ nico_tag.name }",
       linked_tags: '',
       created_at: Time.current,
       created_by_user: nil
@@ -58,10 +58,10 @@ RSpec.describe VersionRecord, type: :model do
 
   it 'prevents NicoTagVersion destroy' do
     version = NicoTagVersion.create!(
-      tag: nico_tag,
+      external_tag: nico_tag,
       version_no: 1,
       event_type: 'create',
-      name: nico_tag.name,
+      name: "nico:#{ nico_tag.name }",
       linked_tags: '',
       created_at: Time.current,
       created_by_user: nil