#352

backend/app/controllers/materials_controller.rb

@@ -89,13 +89,15 @@ class MaterialsController < ApplicationController
 
     begin
       Material.transaction do
-        tag = resolve_material_tag!(tag_name_raw)
+        tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw)
+        tag = tag_name.tag
+        tag = Tag.create!(tag_name:, category: :material) unless tag
+
         material = Material.new(tag:, url:,
                                 created_by_user: current_user,
                                 updated_by_user: current_user)
         material.file.attach(uploaded_blob) if uploaded_blob
         material.save!
-        TagVersioning.record_tag_snapshot!(tag, created_by_user: current_user)
         upsert_export_paths!(material)
         MaterialVersionRecorder.record!(material:, event_type: :create,
                                         created_by_user: current_user)
@@ -137,7 +139,10 @@ class MaterialsController < ApplicationController
     begin
       Material.transaction do
         MaterialVersionRecorder.ensure_snapshot!(material, created_by_user: current_user)
-        tag = resolve_material_tag!(tag_name_raw)
+        tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw)
+        tag = tag_name.tag
+        tag = Tag.create!(tag_name:, category: :material) unless tag
+
         material.assign_attributes(tag:, url:, updated_by_user: current_user)
         if uploaded_blob
           material.file.attach(uploaded_blob)
@@ -145,7 +150,6 @@ class MaterialsController < ApplicationController
           material.file.detach
         end
         material.save!
-        TagVersioning.record_tag_snapshot!(tag, created_by_user: current_user)
         upsert_export_paths!(material)
         MaterialVersionRecorder.record!(material:, event_type: :update,
                                         created_by_user: current_user)
@@ -236,12 +240,6 @@ class MaterialsController < ApplicationController
     nil
   end
 
-  def resolve_material_tag! tag_name_raw
-    tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw)
-    tag = tag_name.tag
-    tag || Tag.create!(tag_name:, category: :material)
-  end
-
   def material_index_needs_tag_name? filters
     filters[:q].present? || filters[:sort] == 'tag_name'
   end

backend/app/controllers/tags_controller.rb

@@ -245,7 +245,10 @@ class TagsController < ApplicationController
     return render_unprocessable_entity('カテゴリは必須です．', field: :category) if category.blank?
     return render_unprocessable_entity '廃止状態は必須です．', field: :deprecated unless params.key?(:deprecated)
 
-    return unless validate_tag_rename(tag, name)
+    if (name != tag.name &&
+        tag.in?([Tag.tagme, Tag.bot, Tag.no_deerjikist, Tag.video, Tag.niconico]))
+      return render_unprocessable_entity 'システム・タグの名称は変更できません．', field: :name
+    end
 
     alias_names = params[:aliases].to_s.split.uniq
     parent_names = params[:parent_tags].to_s.split.uniq
@@ -271,7 +274,7 @@ class TagsController < ApplicationController
       else
         tag.update!(category:, deprecated_at: deprecated ? Time.current : nil)
       end
-      rename_tag_name!(tag, name) if name_changed
+      tag.tag_name.update!(name:)
 
       alias_names << old_name if name_changed
       alias_names.delete(name)
@@ -307,8 +310,6 @@ class TagsController < ApplicationController
       return render_unprocessable_entity 'ニコタグは廃止できません．', field: :deprecated
     end
 
-    return unless validate_tag_rename(tag, name)
-
     if tag.nico? || (category.present? && category == 'nico')
       return render_unprocessable_entity 'ニコタグは変更できません．', field: :category
     end
@@ -320,7 +321,7 @@ class TagsController < ApplicationController
       name_changed = name.present? && name != old_name
       wiki_page = tag.tag_name.wiki_page if name_changed
 
-      rename_tag_name!(tag, name) if name_changed
+      tag.tag_name.update!(name:) if name.present?
       tag.update!(category:) if category.present?
       if deprecated_given && tag.deprecated? != deprecated
         tag.update!(deprecated_at: deprecated ? Time.current : nil)
@@ -534,67 +535,6 @@ class TagsController < ApplicationController
         created_by_user:)
   end
 
-  def validate_tag_rename tag, name
-    return true if name.blank? || name == tag.name
-
-    if tag.in?([Tag.tagme, Tag.bot, Tag.no_deerjikist, Tag.video, Tag.niconico])
-      render_unprocessable_entity 'システム・タグの名称は変更できません．', field: :name
-      return false
-    end
-
-    target_tag_name = TagName.with_discarded.find_by(name:)
-    return true if target_tag_name.nil?
-    return true if target_tag_name.canonical_id?
-
-    render_unprocessable_entity 'その名前は既に使はれてゐます．', field: :name
-    false
-  end
-
-  def rename_tag_name! tag, name
-    return if name == tag.name
-
-    current_tag_name = tag.tag_name
-    target_tag_name = TagName.with_discarded.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)
-  end
-
-  def promote_tag_alias! tag, current_tag_name:, promoted_tag_name:
-    old_owner_tag = promoted_tag_name.canonical&.tag
-
-    if old_owner_tag && old_owner_tag != tag
-      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)
-           .where.not(id: promoted_tag_name.id)
-           .find_each do |alias_tag_name|
-      alias_tag_name.update!(canonical: promoted_tag_name)
-    end
-
-    current_tag_name.wiki_page&.update!(tag_name: promoted_tag_name)
-    tag.update!(tag_name: promoted_tag_name)
-    current_tag_name.association(:wiki_page).reset
-    current_tag_name.association(:tag).reset
-    current_tag_name.reload.update!(canonical: promoted_tag_name)
-
-    return unless old_owner_tag && old_owner_tag != tag
-
-    record_tag_version!(old_owner_tag.reload, event_type: :update, created_by_user: current_user)
-  end
-
   def update_aliases! tag, alias_names
     alias_names = alias_names.uniq
 

backend/app/models/tag_implication.rb

@@ -6,7 +6,6 @@ class TagImplication < ApplicationRecord
   validates :parent_tag_id, presence: true
 
   validate :parent_tag_mustnt_be_itself
-  validate :parent_tag_mustnt_create_cycle
 
   private
 
@@ -15,27 +14,4 @@ class TagImplication < ApplicationRecord
       errors.add :parent_tag_id, '親タグは子タグと同一であってはなりません．'
     end
   end
-
-  def parent_tag_mustnt_create_cycle
-    return if tag_id.blank? || parent_tag_id.blank?
-    return if errors[:parent_tag_id].present?
-
-    seen = { }
-    stack = [parent_tag_id]
-
-    until stack.empty?
-      current_id = stack.pop
-      next if seen[current_id]
-
-      seen[current_id] = true
-
-      if current_id == tag_id
-        errors.add :parent_tag_id, '親タグに子孫タグを指定すると循環します．'
-        errors.add :base, 'タグの親子関係が循環します．'
-        return
-      end
-
-      stack.concat(TagImplication.where(tag_id: current_id).pluck(:parent_tag_id))
-    end
-  end
 end

backend/spec/models/tag_implication_spec.rb

@@ -1,36 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe TagImplication, type: :model do
-  it 'rejects a parent tag that would create a cycle' do
-    child = create(:tag, name: 'tag_implication_cycle_child')
-    parent = create(:tag, name: 'tag_implication_cycle_parent')
-
-    described_class.create!(tag: child, parent_tag: parent)
-
-    implication = described_class.new(tag: parent, parent_tag: child)
-
-    expect(implication).not_to be_valid
-    expect(implication.errors[:parent_tag_id]).to include(
-      '親タグに子孫タグを指定すると循環します．'
-    )
-    expect(implication.errors[:base]).to be_present
-  end
-
-  it 'terminates even when existing data already contains a cycle' do
-    child = create(:tag, name: 'tag_implication_existing_cycle_child')
-    parent = create(:tag, name: 'tag_implication_existing_cycle_parent')
-    ancestor = create(:tag, name: 'tag_implication_existing_cycle_ancestor')
-
-    described_class.create!(tag: parent, parent_tag: ancestor)
-    described_class.insert_all!(
-      [
-        { tag_id: ancestor.id, parent_tag_id: parent.id,
-          created_at: Time.current, updated_at: Time.current }
-      ]
-    )
-
-    implication = described_class.new(tag: child, parent_tag: parent)
-
-    expect(implication).to be_valid
-  end
-end

backend/spec/models/tag_spec.rb

@@ -54,17 +54,7 @@ RSpec.describe Tag, type: :model do
       first = create(:tag, name: 'expand_cycle_first')
       second = create(:tag, name: 'expand_cycle_second')
       TagImplication.create!(tag: first, parent_tag: second)
-      now = Time.current
-      TagImplication.insert_all!(
-        [
-          {
-            tag_id: second.id,
-            parent_tag_id: first.id,
-            created_at: now,
-            updated_at: now
-          }
-        ]
-      )
+      TagImplication.create!(tag: second, parent_tag: first)
 
       expect(described_class.expand_parent_tags([first])).to contain_exactly(first, second)
     end

backend/spec/requests/materials_spec.rb

@@ -287,25 +287,6 @@ RSpec.describe 'Materials API', type: :request do
         expect(json.dig('export_paths', 'legacy_drive')).to eq('伊地知ニジカ/created.png')
       end
 
-      it 'creates a create tag_version for a newly created material tag' do
-        expect do
-          post '/materials', params: {
-            tag: 'material_create_versioned_tag',
-            file: dummy_upload(filename: 'created.png')
-          }
-        end.to change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:created)
-
-        tag = Tag.joins(:tag_name).find_by!(tag_names: { name: 'material_create_versioned_tag' })
-        version = tag.tag_versions.order(:version_no).last
-
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_create_versioned_tag')
-        expect(version.category).to eq('material')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
       it 'snapshots attached file metadata and sha256' do
         post '/materials', params: {
           tag: 'material_create_file_version',
@@ -485,73 +466,6 @@ RSpec.describe 'Materials API', type: :request do
         expect(json.dig('tag', 'name')).to eq('material_update_new')
       end
 
-      it 'creates a create tag_version when update creates a new tag' do
-        expect do
-          put "/materials/#{ material.id }", params: {
-            tag: 'material_update_versioned_tag',
-            file: dummy_upload(filename: 'updated.png')
-          }
-        end.to change(Tag, :count).by(1)
-          .and change(TagName, :count).by(1)
-          .and change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:ok)
-
-        tag = Tag.joins(:tag_name).find_by!(tag_names: { name: 'material_update_versioned_tag' })
-        version = tag.tag_versions.order(:version_no).last
-
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_update_versioned_tag')
-        expect(version.category).to eq('material')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
-      it 'backfills a create tag_version for an existing material tag without history' do
-        existing_tag =
-            Tag.create!(tag_name: TagName.create!(name: 'material_update_existing_no_history'),
-                        category: :material)
-
-        expect(existing_tag.tag_versions).to be_empty
-
-        expect do
-          put "/materials/#{ material.id }", params: {
-            tag: 'material_update_existing_no_history',
-            file: dummy_upload(filename: 'updated.png')
-          }
-        end.to change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:ok)
-
-        version = existing_tag.reload.tag_versions.order(:version_no).last
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_update_existing_no_history')
-        expect(version.category).to eq('material')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
-      it 'backfills a create tag_version for an existing character tag without history' do
-        existing_tag =
-            Tag.create!(tag_name: TagName.create!(name: 'material_update_character_no_history'),
-                        category: :character)
-
-        expect(existing_tag.tag_versions).to be_empty
-
-        expect do
-          put "/materials/#{ material.id }", params: {
-            tag: 'material_update_character_no_history',
-            file: dummy_upload(filename: 'updated.png')
-          }
-        end.to change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:ok)
-
-        version = existing_tag.reload.tag_versions.order(:version_no).last
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_update_character_no_history')
-        expect(version.category).to eq('character')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
       it 'detaches the existing file without purging blob when url replaces file' do
         old_blob_id = material.file.blob.id
 
@@ -580,7 +494,6 @@ RSpec.describe 'Materials API', type: :request do
       it 'does not increase version for the same snapshot update' do
         MaterialVersionRecorder.record!(material:, event_type: :create,
                                         created_by_user: member_user)
-        TagVersioning.ensure_snapshot!(tag, created_by_user: member_user)
 
         expect do
           put "/materials/#{ material.id }", params: {
@@ -588,8 +501,6 @@ RSpec.describe 'Materials API', type: :request do
           }
         end.not_to change(MaterialVersion, :count)
 
-        expect(tag.reload.tag_versions.count).to eq(1)
-
         expect(response).to have_http_status(:ok)
         expect(material.reload.version_no).to eq(1)
       end

backend/spec/requests/tag_children_spec.rb

@@ -56,17 +56,6 @@ RSpec.describe "TagChildren", type: :request do
 
         expect(response).to have_http_status(:no_content)
       end
-
-      it 'returns 422 and does not create relation when the new link makes a cycle' do
-        TagImplication.create!(tag: parent, parent_tag: child)
-
-        expect {
-          do_request
-        }.not_to change(TagImplication, :count)
-
-        expect(response).to have_http_status(:unprocessable_entity)
-        expect(TagImplication.where(tag: child, parent_tag: parent)).not_to exist
-      end
     end
 
     context "when Tag.find raises (invalid ids)" do

backend/spec/requests/tags_spec.rb

@@ -581,58 +581,6 @@ RSpec.describe 'Tags API', type: :request do
         expect(response).to have_http_status(:ok)
         expect(wiki_page.reload.wiki_versions.count).to eq(before_wiki_version_count)
       end
-
-      it 'full update で旧名 alias が残った tag を PATCH で旧名へ戻せる' do
-        put "/tags/#{ tag.id }", params: {
-          name: 'patch_roundtrip_target',
-          category: 'general',
-          aliases: 'unko',
-          parent_tags: '',
-          deprecated: '0',
-        }
-
-        expect(response).to have_http_status(:ok)
-        expect(TagName.find_by!(name: 'spec_tag').canonical).to eq(tag.reload.tag_name)
-
-        patch "/tags/#{ tag.id }", params: { name: 'spec_tag' }
-
-        expect(response).to have_http_status(:ok)
-
-        tag.reload
-
-        expect(tag.name).to eq('spec_tag')
-        expect(tag.tag_name.canonical_id).to be_nil
-        expect(TagName.find_by!(name: 'patch_roundtrip_target').canonical).to eq(tag.tag_name)
-      end
-
-      it '別 tag の正規名には変更できない' do
-        wiki_page =
-            Wiki::Commit.create_content!(
-                tag_name: tag.tag_name,
-                body: 'patch collision wiki',
-                created_by_user: member_user,
-                message: 'init')
-
-        patch "/tags/#{ tag.id }", params: { name: 'unknown' }
-
-        expect(response).to have_http_status(:unprocessable_entity)
-        expect(json.fetch('errors')).to include(
-          'name' => ['その名前は既に使はれてゐます．']
-        )
-
-        expect(tag.reload.name).to eq('spec_tag')
-        expect(tag.tag_name.aliases.map(&:name)).to contain_exactly('unko')
-        expect(wiki_page.reload.tag_name).to eq(tag.tag_name)
-      end
-
-      it 'system tag の name は変更できない' do
-        system_tag = Tag.bot
-
-        patch "/tags/#{ system_tag.id }", params: { name: 'patch_system_tag_renamed' }
-
-        expect(response).to have_http_status(:unprocessable_entity)
-        expect(system_tag.reload.name).to eq('bot操作')
-      end
     end
   end
 
@@ -797,17 +745,7 @@ RSpec.describe 'Tags API', type: :request do
       )
       TagImplication.create!(tag: first, parent_tag: root_material)
       TagImplication.create!(tag: second, parent_tag: first)
-      now = Time.current
-      TagImplication.insert_all!(
-        [
-          {
-            tag_id: first.id,
-            parent_tag_id: second.id,
-            created_at: now,
-            updated_at: now
-          }
-        ]
-      )
+      TagImplication.create!(tag: first, parent_tag: second)
 
       get '/tags/with-depth', params: { parent: root_material.id }
 
@@ -1091,42 +1029,6 @@ RSpec.describe 'Tags API', type: :request do
         expect(versions.second.created_by_user_id).to eq(member_user.id)
       end
 
-      it '同じ tag の旧 alias へ戻しても rename できる' do
-        put "/tags/#{ tag.id }", params: {
-          name: 'put_roundtrip_b',
-          category: 'general',
-          aliases: 'unko',
-          parent_tags: '',
-          deprecated: '0',
-        }
-
-        expect(response).to have_http_status(:ok)
-
-        put "/tags/#{ tag.id }", params: {
-          name: 'spec_tag',
-          category: 'general',
-          aliases: 'unko',
-          parent_tags: '',
-          deprecated: '0',
-        }
-
-        expect(response).to have_http_status(:ok)
-
-        tag.reload
-
-        expect(tag.name).to eq('spec_tag')
-        expect(TagName.find_by!(name: 'put_roundtrip_b').canonical).to eq(tag.tag_name)
-        expect(tag.tag_name.aliases.map(&:name)).to contain_exactly('put_roundtrip_b', 'unko')
-        expect(tag.tag_name.aliases.map(&:name)).not_to include('spec_tag')
-        expect(alias_tn.reload.canonical).to eq(tag.tag_name)
-
-        version = tag.tag_versions.order(:version_no).last
-
-        expect(version.event_type).to eq('update')
-        expect(version.name).to eq('spec_tag')
-        expect(version.aliases.split).to contain_exactly('put_roundtrip_b', 'unko')
-      end
-
       it 'parent tag の snapshot も作成する' do
         old_parent = Tag.create!(
           tag_name: TagName.create!(name: 'put_snapshot_old_parent'),
@@ -1251,48 +1153,6 @@ RSpec.describe 'Tags API', type: :request do
         )
       end
 
-      it 'wiki を持つ tag を旧 alias へ戻しても wiki を新 canonical へ移す' do
-        wiki_page =
-            Wiki::Commit.create_content!(
-                tag_name: tag.tag_name,
-                body: 'wiki body before',
-                created_by_user: member_user,
-                message: 'init')
-
-        expect {
-          put "/tags/#{ tag.id }", params: {
-            name: 'put_wiki_roundtrip_b',
-            category: 'general',
-            aliases: 'unko',
-            parent_tags: '',
-            deprecated: '0',
-          }
-
-          put "/tags/#{ tag.id }", params: {
-            name: 'spec_tag',
-            category: 'general',
-            aliases: 'unko',
-            parent_tags: '',
-            deprecated: '0',
-          }
-        }
-            .to change(TagVersion, :count).by(3)
-            .and change(WikiVersion, :count).by(2)
-
-        expect(response).to have_http_status(:ok)
-
-        tag.reload
-
-        expect(wiki_page.reload.tag_name).to eq(tag.tag_name)
-        expect(TagName.find_by!(name: 'put_wiki_roundtrip_b').wiki_page).to be_nil
-        expect(TagName.find_by!(name: 'put_wiki_roundtrip_b').canonical).to eq(tag.tag_name)
-
-        versions = wiki_page.wiki_versions.order(:version_no).last(2)
-
-        expect(versions.map(&:event_type)).to eq(['update', 'update'])
-        expect(versions.map(&:title)).to eq(['put_wiki_roundtrip_b', 'spec_tag'])
-      end
-
       it '別名を他 tag から奪った場合、奪はれた側の tag version も作成する' do
         old_owner = Tag.create!(
           tag_name: TagName.create!(name: 'put_alias_old_owner'),
@@ -1331,49 +1191,9 @@ RSpec.describe 'Tags API', type: :request do
         expect(old_owner_versions.second.aliases.split).not_to include('put_stolen_alias')
       end
 
-      it '別 tag の alias 名を rename で奪へる' do
-        old_owner = Tag.create!(
-          tag_name: TagName.create!(name: 'put_alias_collision_owner'),
-          category: :general
-        )
-        stolen_alias = TagName.create!(
-          name: 'put_alias_collision_name',
-          canonical: old_owner.tag_name
-        )
-        wiki_page =
-            Wiki::Commit.create_content!(
-                tag_name: tag.tag_name,
-                body: 'put collision wiki',
-                created_by_user: member_user,
-                message: 'init')
-
-        put "/tags/#{ tag.id }", params: {
-          name: 'put_alias_collision_name',
-          category: 'general',
-          aliases: 'unko',
-          parent_tags: '',
-          deprecated: '0',
-        }
-
-        expect(response).to have_http_status(:ok)
-
-        tag.reload
-        old_owner.reload
-        stolen_alias.reload
-
-        expect(tag.name).to eq('put_alias_collision_name')
-        expect(stolen_alias.canonical_id).to be_nil
-        expect(TagName.find_by!(name: 'spec_tag').canonical).to eq(tag.tag_name)
-        expect(old_owner.tag_name.aliases.map(&:name)).not_to include('put_alias_collision_name')
-
-        old_owner_versions = old_owner.tag_versions.order(:version_no)
-
-        expect(old_owner_versions.last.event_type).to eq('update')
-        expect(old_owner_versions.last.aliases.split).not_to include('put_alias_collision_name')
-        expect(wiki_page.reload.tag_name).to eq(tag.tag_name)
-      end
-
       it 'parent_tags に指定すると循環する tag は 422 にする' do
+        pending '#332 で対応予定'
+
         child = Tag.create!(
           tag_name: TagName.create!(name: 'put_cycle_child'),
           category: :general