From 176519b929f7b7ae9e0d4d389f9beeec49d36237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=BF=E3=81=A6=E3=82=8B=E3=81=9E?= Date: Wed, 11 Mar 2026 22:55:57 +0900 Subject: [PATCH] =?UTF-8?q?tags=20=E3=81=AE=E8=AB=96=E7=90=86=E5=89=8A?= =?UTF-8?q?=E9=99=A4=E5=AF=BE=E5=BF=9C=EF=BC=88#287=EF=BC=89=20(#288)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #287 #287 Co-authored-by: miteruzo Reviewed-on: https://git.miteruzo.com/miteruzo/btrc-hub/pulls/288 --- backend/app/models/tag.rb | 34 +++++++------ ...20260311123100_add_discarded_at_to_tags.rb | 6 +++ backend/db/schema.rb | 14 +++++- backend/spec/models/tag_spec.rb | 49 +++++++++++++------ 4 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 backend/db/migrate/20260311123100_add_discarded_at_to_tags.rb diff --git a/backend/app/models/tag.rb b/backend/app/models/tag.rb index eaf51da..21e20b4 100644 --- a/backend/app/models/tag.rb +++ b/backend/app/models/tag.rb @@ -1,8 +1,12 @@ class Tag < ApplicationRecord + include Discard::Model + class NicoTagNormalisationError < ArgumentError ; end + default_scope -> { kept } + has_many :post_tags, inverse_of: :tag has_many :active_post_tags, -> { kept }, class_name: 'PostTag', inverse_of: :tag has_many :post_tags_with_discarded, -> { with_discarded }, class_name: 'PostTag' @@ -144,25 +148,23 @@ class Tag < ApplicationRecord target_tag => Tag Tag.transaction do - Array(source_tags).compact.uniq.each do |st| - st => Tag - - next if st == target_tag - - st.post_tags.find_each do |pt| - if PostTag.kept.exists?(post_id: pt.post_id, tag_id: target_tag.id) - pt.discard_by!(nil) - # discard 後の update! は禁止なので DB を直に更新 - pt.update_columns(tag_id: target_tag.id, updated_at: Time.current) - else - pt.update!(tag: target_tag) + Array(source_tags).compact.uniq.each do |source_tag| + source_tag => Tag + + next if source_tag == target_tag + + source_tag.post_tags.kept.find_each do |source_pt| + post_id = source_pt.post_id + source_pt.discard_by!(nil) + unless PostTag.kept.exists?(post_id:, tag: target_tag) + PostTag.create!(post_id:, tag: target_tag) end end - tag_name = st.tag_name - st.destroy! - tag_name.reload - tag_name.update!(canonical: target_tag.tag_name) + source_tag_name = source_tag.tag_name + source_tag.discard! + source_tag_name.reload + source_tag_name.update!(canonical: target_tag.tag_name) end # 投稿件数を再集計 diff --git a/backend/db/migrate/20260311123100_add_discarded_at_to_tags.rb b/backend/db/migrate/20260311123100_add_discarded_at_to_tags.rb new file mode 100644 index 0000000..4bd5c5d --- /dev/null +++ b/backend/db/migrate/20260311123100_add_discarded_at_to_tags.rb @@ -0,0 +1,6 @@ +class AddDiscardedAtToTags < ActiveRecord::Migration[8.0] + def change + add_column :tags, :discarded_at, :datetime + add_index :tags, :discarded_at + end +end diff --git a/backend/db/schema.rb b/backend/db/schema.rb index 8077fac..b6ffbd3 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_03_03_122700) do +ActiveRecord::Schema[8.0].define(version: 2026_03_11_123100) 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 @@ -127,6 +127,16 @@ ActiveRecord::Schema[8.0].define(version: 2026_03_03_122700) do t.index ["tag_id"], name: "index_tag_implications_on_tag_id" end + create_table "tag_name_sanitisation_rules", primary_key: "priority", id: :integer, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.string "source_pattern", null: false + t.string "replacement", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.datetime "discarded_at" + t.index ["discarded_at"], name: "index_tag_name_sanitisation_rules_on_discarded_at" + t.index ["source_pattern"], name: "index_tag_name_sanitisation_rules_on_source_pattern", unique: true + end + create_table "tag_names", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "name", null: false t.bigint "canonical_id" @@ -150,6 +160,8 @@ ActiveRecord::Schema[8.0].define(version: 2026_03_03_122700) 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.index ["discarded_at"], name: "index_tags_on_discarded_at" t.index ["tag_name_id"], name: "index_tags_on_tag_name_id", unique: true end diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index 389f1a1..c2947fa 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -10,11 +10,18 @@ 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 'moves the post_tag, deletes the source tag, and aliases the source tag_name' do + it 'discards the source post_tag, creates an active target post_tag, ' + + 'discards the source tag, and aliases the source tag_name' do described_class.merge_tags!(target_tag, [source_tag]) - expect(source_post_tag.reload.tag_id).to eq(target_tag.id) - expect(Tag.exists?(source_tag.id)).to be(false) + source_pt = PostTag.with_discarded.find(source_post_tag.id) + active_target = PostTag.kept.find_by(post_id: post_record.id, tag_id: target_tag.id) + + expect(source_pt.discarded_at).to be_present + expect(source_pt.tag_id).to eq(source_tag.id) + expect(active_target).to be_present + + expect(Tag.with_discarded.find(source_tag.id)).to be_discarded expect(source_tag.tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) end end @@ -23,15 +30,19 @@ RSpec.describe Tag, type: :model do let!(:target_post_tag) { PostTag.create!(post: post_record, tag: target_tag) } let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) } - it 'discards the duplicate source post_tag and keeps one active target post_tag' do + it 'discards the source post_tag, keeps one active target post_tag, ' + + 'and aliases the source tag_name' do described_class.merge_tags!(target_tag, [source_tag]) + source_pt = PostTag.with_discarded.find(source_post_tag.id) active = PostTag.kept.where(post_id: post_record.id, tag_id: target_tag.id) - discarded_source = PostTag.with_discarded.find(source_post_tag.id) + expect(source_pt.discarded_at).to be_present + expect(source_pt.tag_id).to eq(source_tag.id) expect(active.count).to eq(1) - expect(discarded_source.discarded_at).to be_present - expect(Tag.exists?(source_tag.id)).to be(false) + expect(active.first.id).to eq(target_post_tag.id) + + expect(Tag.with_discarded.find(source_tag.id)).to be_discarded expect(source_tag.tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) end end @@ -39,12 +50,17 @@ RSpec.describe Tag, type: :model do context 'when source_tags includes the target itself' do let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) } - it 'ignores the target tag in source_tags' do + it 'ignores the target in source_tags while still merging the source tag' do described_class.merge_tags!(target_tag, [source_tag, target_tag]) - expect(Tag.exists?(target_tag.id)).to be(true) - expect(Tag.exists?(source_tag.id)).to be(false) - expect(source_post_tag.reload.tag_id).to eq(target_tag.id) + source_pt = PostTag.with_discarded.find(source_post_tag.id) + active_target = PostTag.kept.find_by(post_id: post_record.id, tag_id: target_tag.id) + + expect(Tag.find(target_tag.id)).to be_present + expect(Tag.with_discarded.find(source_tag.id)).to be_discarded + expect(source_pt.discarded_at).to be_present + expect(source_pt.tag_id).to eq(source_tag.id) + expect(active_target).to be_present end end @@ -52,9 +68,10 @@ RSpec.describe Tag, type: :model do let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) } let!(:wiki_page) do WikiPage.create!( - tag_name: source_tag.tag_name, - created_user: create_admin_user!, - updated_user: create_admin_user!) + tag_name: source_tag.tag_name, + created_user: create_admin_user!, + updated_user: create_admin_user! + ) end it 'rolls back the transaction' do @@ -62,8 +79,8 @@ RSpec.describe Tag, type: :model do described_class.merge_tags!(target_tag, [source_tag]) }.to raise_error(ActiveRecord::RecordInvalid) - expect(Tag.exists?(source_tag.id)).to be(true) - expect(source_post_tag.reload.tag_id).to eq(source_tag.id) + expect(Tag.with_discarded.find(source_tag.id)).not_to be_discarded + expect(PostTag.kept.find(source_post_tag.id).tag_id).to eq(source_tag.id) expect(source_tag.tag_name.reload.canonical_id).to be_nil end end