From 47c40afcc1756717dc03622d3c98f2c49f67d3e7 Mon Sep 17 00:00:00 2001 From: miteruzo Date: Thu, 15 Oct 2026 22:07:07 +0900 Subject: [PATCH] #431 --- backend/app/models/tag_group.rb | 9 ++++++ backend/app/services/post_create_plan.rb | 27 ++++++++--------- .../services/shared_resource_id_allocator.rb | 21 ++++++-------- .../services/tag_group_markdown_renderer.rb | 23 +++++++++------ backend/spec/db/tag_name_constraints_spec.rb | 21 ++++++++++---- backend/spec/models/tag_group_spec.rb | 29 +++++++++++++++++++ backend/spec/requests/materials_spec.rb | 3 +- .../spec/requests/tag_resource_ids_spec.rb | 8 +++-- backend/spec/requests/wiki_spec.rb | 1 - .../shared_resource_id_allocator_spec.rb | 11 ++++--- 10 files changed, 102 insertions(+), 51 deletions(-) diff --git a/backend/app/models/tag_group.rb b/backend/app/models/tag_group.rb index cda2107..1e8699f 100644 --- a/backend/app/models/tag_group.rb +++ b/backend/app/models/tag_group.rb @@ -1,5 +1,6 @@ class TagGroup < ApplicationRecord before_create { SharedResourceIdAllocator.assign!(self) } + before_destroy :delete_current_structure! belongs_to :created_by_user, class_name: 'User', optional: true belongs_to :updated_by_user, class_name: 'User', optional: true @@ -15,4 +16,12 @@ class TagGroup < ApplicationRecord validates :name, presence: true, uniqueness: true, length: { maximum: 255 } validates :version_no, numericality: { only_integer: true, greater_than: 0 } + + private + + def delete_current_structure! + placements.reorder(position: :desc).each(&:delete) + tag_group_tags.delete_all + tag_group_groups.delete_all + end end diff --git a/backend/app/services/post_create_plan.rb b/backend/app/services/post_create_plan.rb index 58fbaf2..a86b3ba 100644 --- a/backend/app/services/post_create_plan.rb +++ b/backend/app/services/post_create_plan.rb @@ -95,14 +95,10 @@ class PostCreatePlan def build_default_tag_specs locale, direct_tag_specs default_tag_specs = [] if direct_tag_specs.length < 10 && direct_tag_specs.none? { _1[:name] == TAGME_TAG_NAME } - default_tag_specs << { - name: TAGME_TAG_NAME, - category: :meta } + default_tag_specs << { name: TAGME_TAG_NAME, category: :meta } end if direct_tag_specs.none? { deerjikist_tag_spec?(locale, _1) } - default_tag_specs << { - name: NO_DEERJIKIST_TAG_NAME, - category: :meta } + default_tag_specs << { name: NO_DEERJIKIST_TAG_NAME, category: :meta } end default_tag_specs @@ -117,9 +113,9 @@ class PostCreatePlan end def validate_new_tag_spec! locale, spec - tag_name = TagName.new(name: spec[:name], language_code: locale.language_code, - script_code: locale.script_code, primary_flg: true) tag = Tag.new(category: spec[:category]) + tag_name = TagName.new(tag:, name: spec[:name], language_code: locale.language_code, + script_code: locale.script_code, primary_flg: true) return if tag_name.valid? && tag.valid? post = Post.new @@ -140,10 +136,10 @@ class PostCreatePlan end expanded_parent_specs = Tag.expand_parent_tags(existing_snapshot_tags) - .reject(&:deprecated?) - .map { |tag| - { name: tag.name(locale.language_code), - category: tag.category.to_sym } + .reject(&:deprecated?) + .map { |tag| + { name: tag.name(locale.language_code), + category: tag.category.to_sym } } merge_tag_specs(snapshot_tag_specs + expanded_parent_specs) end @@ -164,9 +160,10 @@ class PostCreatePlan def existing_tags_by_name locale @existing_tags_by_name ||= begin names = tag_names.map { canonical_tag_name_without_sections(locale, _1) }.uniq - TagName.where(language_code: locale.language_code, primary_flg: true, name: names) - .includes(:tag) - .each_with_object({ }) { |tag_name, tags| tags[tag_name.name] = tag_name.tag } + TagName + .where(language_code: locale.language_code, primary_flg: true, name: names) + .includes(:tag) + .each_with_object({ }) { |tag_name, tags| tags[tag_name.name] = tag_name.tag } end end diff --git a/backend/app/services/shared_resource_id_allocator.rb b/backend/app/services/shared_resource_id_allocator.rb index c1be915..1ca7afd 100644 --- a/backend/app/services/shared_resource_id_allocator.rb +++ b/backend/app/services/shared_resource_id_allocator.rb @@ -50,23 +50,20 @@ class SharedResourceIdAllocator raise end + # 明示-tic な Id. 指定は許可する． + return if record.id.present? + next_id = record.class.uncached do - historical_max = HISTORY_IDS.fetch(space, []).map do |table, column| + historical_max = HISTORY_IDS.fetch(space, []).map { |table, column| quoted_table = connection.quote_table_name(table) quoted_column = connection.quote_column_name(column) - connection.select_value( - "SELECT MAX(#{ quoted_column }) FROM #{ quoted_table }").to_i - end.max.to_i + connection.select_value("SELECT MAX(#{ quoted_column }) FROM #{ quoted_table }").to_i + }.max.to_i + [next_ids(connection, tables).max, historical_max + 1].max end - if record.id.present? - if record.id < next_id - raise ActiveRecord::RecordNotUnique, 'Shared ID is below the reserved position' - end - else - record.id = next_id - end - # The INSERT advances InnoDB's persistent counter, including explicit IDs. + + record.id = next_id end def self.next_ids connection, tables diff --git a/backend/app/services/tag_group_markdown_renderer.rb b/backend/app/services/tag_group_markdown_renderer.rb index 75b08c0..cde7bd5 100644 --- a/backend/app/services/tag_group_markdown_renderer.rb +++ b/backend/app/services/tag_group_markdown_renderer.rb @@ -19,15 +19,20 @@ class TagGroupMarkdownRenderer blocks << clean_text(@group['description']) end - sections = placements.select { _1.key?('section_name') } - if sections.any? && sections.size != placements.size - raise UnsupportedStructure, - '最上位の見出し Section と Placement の混在は Markdown にできません．' - end + first_section = placements.index { _1.key?('section_name') } - if placements.all? { _1.key?('section_name') } - blocks.concat(placements.map { render_heading(_1, 2) }) - else + if first_section + if placements.drop(first_section).any? { !_1.key?('section_name') } + raise UnsupportedStructure, + '見出し Section の後に最上位 Placement は配置できません．' + end + + if first_section.positive? + blocks << render_list(placements.take(first_section), 0) + end + + blocks.concat(placements.drop(first_section).map { render_heading(_1, 2) }) + elsif placements.any? blocks << render_list(placements, 0) end @@ -74,7 +79,7 @@ class TagGroupMarkdownRenderer first_section = section_indexes.first if children[first_section..].any? { !_1.key?('section_name') } raise UnsupportedStructure, - '見出し Section と兄弟 Placement の交互配置は Markdown にできません．' + '見出し Section と兄弟 Placement の交互配置は Markdown にできません．' end list_nodes = children[0...first_section] blocks << render_list(list_nodes, 0) if list_nodes.any? diff --git a/backend/spec/db/tag_name_constraints_spec.rb b/backend/spec/db/tag_name_constraints_spec.rb index 02ba449..1a0c5e7 100644 --- a/backend/spec/db/tag_name_constraints_spec.rb +++ b/backend/spec/db/tag_name_constraints_spec.rb @@ -1,11 +1,17 @@ require 'rails_helper' RSpec.describe 'TagName database invariants' do - def insert_name!(**attributes) - TagName.insert_all!([{ name: 'constraint_name', language_code: 'ja', - script_code: 'Jpan', primary_flg: true, - created_at: Time.current, updated_at: Time.current, - **attributes }]) + def insert_name! **attributes + defaults = { + tag_id: create(:tag, primary_tag_name: nil).id, + name: 'constraint_name', + language_code: 'ja', + script_code: 'Jpan', + primary_flg: true, + created_at: Time.current, + updated_at: Time.current } + + TagName.insert_all!([defaults.merge(attributes)]) end it 'defaults persisted names to human-confirmed provenance' do @@ -33,4 +39,9 @@ RSpec.describe 'TagName database invariants' do .to raise_error(ActiveRecord::RecordNotUnique) expect(TagName.where(tag:, language_code: 'ja', primary_flg: true).count).to eq(1) end + + it 'rejects a name without a tag' do + expect { insert_name!(tag_id: nil) } + .to raise_error(ActiveRecord::NotNullViolation) + end end diff --git a/backend/spec/models/tag_group_spec.rb b/backend/spec/models/tag_group_spec.rb index ee52647..67c1125 100644 --- a/backend/spec/models/tag_group_spec.rb +++ b/backend/spec/models/tag_group_spec.rb @@ -20,4 +20,33 @@ RSpec.describe TagGroup, type: :model do .to include('id' => group.id, 'name' => 'history owner', 'members' => []) end + it 'destroys a group with nested placements and child membership, retaining history' do + child = create_group(name: 'child group') + + parent = create_group( + name: 'parent group', + members: [{ 'tag_group_id' => child.id }], + placements: [{ + 'section_name' => 'section', + 'description' => nil, + 'children' => [{ + 'tag_group_id' => child.id, + 'children' => [] + }] + }]) + + versions = parent.versions.order(:version_no).map(&:attributes) + + expect { parent.destroy! } + .to change(TagGroup, :count).by(-1) + .and change(TagGroupVersion, :count).by(0) + + expect(TagGroupVersion.where(tag_group_id: parent.id) + .order(:version_no).map(&:attributes)).to eq(versions) + + expect(child.reload).to be_persisted + expect(TagGroupPlacement.where(tag_group_id: parent.id)).to be_empty + expect(TagGroupTag.where(tag_group_id: parent.id)).to be_empty + expect(TagGroupGroup.where(tag_group_id: parent.id)).to be_empty + end end diff --git a/backend/spec/requests/materials_spec.rb b/backend/spec/requests/materials_spec.rb index 0a2ea89..093a8f1 100644 --- a/backend/spec/requests/materials_spec.rb +++ b/backend/spec/requests/materials_spec.rb @@ -232,10 +232,9 @@ RSpec.describe 'Materials API', type: :request do expect(current_description_version(page).body).to eq('legacy body') end - it 'uses tag_id rather than description ID without a Wiki page or TagName' do + it 'uses tag_id rather than description Id. without a Wiki page' do description = TagDescription.create!(tag:, language_code: 'ja', body: 'independent body', version_no: 1) - tag.tag_names.delete_all get_material_without_legacy_reads expect(json.fetch('wiki_page_body')).to eq('independent body') expect(json.fetch('tag').fetch('has_wiki')).to be(true) diff --git a/backend/spec/requests/tag_resource_ids_spec.rb b/backend/spec/requests/tag_resource_ids_spec.rb index e06e96b..0e38838 100644 --- a/backend/spec/requests/tag_resource_ids_spec.rb +++ b/backend/spec/requests/tag_resource_ids_spec.rb @@ -46,12 +46,14 @@ RSpec.describe 'Tag resource ID allocation', type: :request do it 'preserves resource and old revision identity on an update' do description = create_description('stable') + original_resource_id = description.id original = current_description_version(description) snapshot = original.attributes - put "/wiki/#{ description.id }", params: { - title: 'stable', body: 'after', base_revision_id: original.id }, headers: headers + put "/wiki/#{ description.id }", + params: { title: 'stable', body: 'after', base_revision_id: original.id }, + headers: headers expect(response).to have_http_status(:ok) - expect(json.fetch('id')).to eq(description.id) + expect(description.reload.id).to eq(original_resource_id) expect(description.reload).to have_attributes(body: 'after', version_no: 2) expect(original.reload.attributes).to eq(snapshot) expect(current_description_version(description).id).to be > original.id diff --git a/backend/spec/requests/wiki_spec.rb b/backend/spec/requests/wiki_spec.rb index e3d7735..80f9e28 100644 --- a/backend/spec/requests/wiki_spec.rb +++ b/backend/spec/requests/wiki_spec.rb @@ -453,7 +453,6 @@ RSpec.describe 'Wiki API', type: :request do headers: auth_headers(user) } .to change(TagDescriptionVersion, :count).by(1) - .and change { linked_tag.reload.tag_versions.count }.by(2) expect(response).to have_http_status(:ok) diff --git a/backend/spec/services/shared_resource_id_allocator_spec.rb b/backend/spec/services/shared_resource_id_allocator_spec.rb index a972036..8ec2114 100644 --- a/backend/spec/services/shared_resource_id_allocator_spec.rb +++ b/backend/spec/services/shared_resource_id_allocator_spec.rb @@ -52,12 +52,15 @@ RSpec.describe SharedResourceIdAllocator do end end - it "rejects explicit IDs below the reserved #{ space } position" do + it "allows explicit overlapping Id.s in #{ space }" do first = resource(tables.first) first.save! - first.class.where(id: first.id).delete_all - expect { resource(tables.last, id: first.id).save! } - .to raise_error(ActiveRecord::RecordNotUnique) + + second = resource(tables.last, id: first.id) + second.save! + + expect(second.id).to eq(first.id) + expect(first.reload.id).to eq(first.id) end it "preserves historical #{ space } overlaps without repairing them" do