tags の論理削除対応(#287) (#288)
#287 #287 Co-authored-by: miteruzo <miteruzo@naver.com> Reviewed-on: #288
This commit was merged in pull request #288.
This commit is contained in:
+16
-14
@@ -1,8 +1,12 @@
|
|||||||
class Tag < ApplicationRecord
|
class Tag < ApplicationRecord
|
||||||
|
include Discard::Model
|
||||||
|
|
||||||
class NicoTagNormalisationError < ArgumentError
|
class NicoTagNormalisationError < ArgumentError
|
||||||
;
|
;
|
||||||
end
|
end
|
||||||
|
|
||||||
|
default_scope -> { kept }
|
||||||
|
|
||||||
has_many :post_tags, inverse_of: :tag
|
has_many :post_tags, inverse_of: :tag
|
||||||
has_many :active_post_tags, -> { kept }, class_name: 'PostTag', 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'
|
has_many :post_tags_with_discarded, -> { with_discarded }, class_name: 'PostTag'
|
||||||
@@ -144,25 +148,23 @@ class Tag < ApplicationRecord
|
|||||||
target_tag => Tag
|
target_tag => Tag
|
||||||
|
|
||||||
Tag.transaction do
|
Tag.transaction do
|
||||||
Array(source_tags).compact.uniq.each do |st|
|
Array(source_tags).compact.uniq.each do |source_tag|
|
||||||
st => Tag
|
source_tag => Tag
|
||||||
|
|
||||||
next if st == target_tag
|
next if source_tag == target_tag
|
||||||
|
|
||||||
st.post_tags.find_each do |pt|
|
source_tag.post_tags.kept.find_each do |source_pt|
|
||||||
if PostTag.kept.exists?(post_id: pt.post_id, tag_id: target_tag.id)
|
post_id = source_pt.post_id
|
||||||
pt.discard_by!(nil)
|
source_pt.discard_by!(nil)
|
||||||
# discard 後の update! は禁止なので DB を直に更新
|
unless PostTag.kept.exists?(post_id:, tag: target_tag)
|
||||||
pt.update_columns(tag_id: target_tag.id, updated_at: Time.current)
|
PostTag.create!(post_id:, tag: target_tag)
|
||||||
else
|
|
||||||
pt.update!(tag: target_tag)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
tag_name = st.tag_name
|
source_tag_name = source_tag.tag_name
|
||||||
st.destroy!
|
source_tag.discard!
|
||||||
tag_name.reload
|
source_tag_name.reload
|
||||||
tag_name.update!(canonical: target_tag.tag_name)
|
source_tag_name.update!(canonical: target_tag.tag_name)
|
||||||
end
|
end
|
||||||
|
|
||||||
# 投稿件数を再集計
|
# 投稿件数を再集計
|
||||||
|
|||||||
@@ -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
|
||||||
Generated
+13
-1
@@ -10,7 +10,7 @@
|
|||||||
#
|
#
|
||||||
# It's strongly recommended that you check this file into your version control system.
|
# 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|
|
create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
|
||||||
t.string "name", null: false
|
t.string "name", null: false
|
||||||
t.string "record_type", 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"
|
t.index ["tag_id"], name: "index_tag_implications_on_tag_id"
|
||||||
end
|
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|
|
create_table "tag_names", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
|
||||||
t.string "name", null: false
|
t.string "name", null: false
|
||||||
t.bigint "canonical_id"
|
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 "created_at", null: false
|
||||||
t.datetime "updated_at", null: false
|
t.datetime "updated_at", null: false
|
||||||
t.integer "post_count", default: 0, 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
|
t.index ["tag_name_id"], name: "index_tags_on_tag_name_id", unique: true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -10,11 +10,18 @@ RSpec.describe Tag, type: :model do
|
|||||||
context 'when merging a simple source tag' do
|
context 'when merging a simple source tag' do
|
||||||
let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) }
|
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])
|
described_class.merge_tags!(target_tag, [source_tag])
|
||||||
|
|
||||||
expect(source_post_tag.reload.tag_id).to eq(target_tag.id)
|
source_pt = PostTag.with_discarded.find(source_post_tag.id)
|
||||||
expect(Tag.exists?(source_tag.id)).to be(false)
|
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)
|
expect(source_tag.tag_name.reload.canonical_id).to eq(target_tag.tag_name_id)
|
||||||
end
|
end
|
||||||
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!(:target_post_tag) { PostTag.create!(post: post_record, tag: target_tag) }
|
||||||
let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_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])
|
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)
|
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(active.count).to eq(1)
|
||||||
expect(discarded_source.discarded_at).to be_present
|
expect(active.first.id).to eq(target_post_tag.id)
|
||||||
expect(Tag.exists?(source_tag.id)).to be(false)
|
|
||||||
|
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)
|
expect(source_tag.tag_name.reload.canonical_id).to eq(target_tag.tag_name_id)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@@ -39,12 +50,17 @@ RSpec.describe Tag, type: :model do
|
|||||||
context 'when source_tags includes the target itself' do
|
context 'when source_tags includes the target itself' do
|
||||||
let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) }
|
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])
|
described_class.merge_tags!(target_tag, [source_tag, target_tag])
|
||||||
|
|
||||||
expect(Tag.exists?(target_tag.id)).to be(true)
|
source_pt = PostTag.with_discarded.find(source_post_tag.id)
|
||||||
expect(Tag.exists?(source_tag.id)).to be(false)
|
active_target = PostTag.kept.find_by(post_id: post_record.id, tag_id: target_tag.id)
|
||||||
expect(source_post_tag.reload.tag_id).to eq(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
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -54,7 +70,8 @@ RSpec.describe Tag, type: :model do
|
|||||||
WikiPage.create!(
|
WikiPage.create!(
|
||||||
tag_name: source_tag.tag_name,
|
tag_name: source_tag.tag_name,
|
||||||
created_user: create_admin_user!,
|
created_user: create_admin_user!,
|
||||||
updated_user: create_admin_user!)
|
updated_user: create_admin_user!
|
||||||
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'rolls back the transaction' do
|
it 'rolls back the transaction' do
|
||||||
@@ -62,8 +79,8 @@ RSpec.describe Tag, type: :model do
|
|||||||
described_class.merge_tags!(target_tag, [source_tag])
|
described_class.merge_tags!(target_tag, [source_tag])
|
||||||
}.to raise_error(ActiveRecord::RecordInvalid)
|
}.to raise_error(ActiveRecord::RecordInvalid)
|
||||||
|
|
||||||
expect(Tag.exists?(source_tag.id)).to be(true)
|
expect(Tag.with_discarded.find(source_tag.id)).not_to be_discarded
|
||||||
expect(source_post_tag.reload.tag_id).to eq(source_tag.id)
|
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
|
expect(source_tag.tag_name.reload.canonical_id).to be_nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user