diff --git a/backend/app/controllers/materials_controller.rb b/backend/app/controllers/materials_controller.rb index 3383806..be1353a 100644 --- a/backend/app/controllers/materials_controller.rb +++ b/backend/app/controllers/materials_controller.rb @@ -237,8 +237,8 @@ class MaterialsController < ApplicationController end def resolve_material_tag! locale, tag_name_raw - tag_name = TagName.find_undiscard_or_create_by!(language_code: locale.language_code, - name: tag_name_raw) + tag_name = TagName.find_or_create_by!(language_code: locale.language_code, + name: tag_name_raw) tag_name.tag || Tag.create!(tag_name:, category: :material) end diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index d59a725..e600f1a 100644 --- a/backend/app/controllers/posts_controller.rb +++ b/backend/app/controllers/posts_controller.rb @@ -36,8 +36,8 @@ class PostsController < ApplicationController offset = (page - 1) * limit pt_max_sql = - PostTag - .select('post_id, MAX(updated_at) AS max_updated_at') + PostVersion + .select('post_id, MAX(created_at) AS max_updated_at') .group('post_id') .to_sql @@ -50,9 +50,8 @@ class PostsController < ApplicationController .joins("LEFT JOIN (#{ pt_max_sql }) pt_max ON pt_max.post_id = posts.id") .reselect('posts.*', Arel.sql("#{ updated_at_all_sql } AS updated_at_all")) .preload(:uploaded_user, :parents, :children, - active_post_tags: [:sections, - { tag: [:deerjikists, :materials, - { tag_name: :wiki_page }] }]) + post_tags: [:sections, { tag: [:deerjikists, :materials, + { tag_name: :wiki_page }] }]) .with_attached_thumbnail q = q.where('posts.url LIKE ?', "%#{ url }%") if url @@ -104,9 +103,8 @@ class PostsController < ApplicationController def random post = filtered_posts.preload(:uploaded_user, :parents, :children, - active_post_tags: [:sections, - { tag: [:deerjikists, :materials, - { tag_name: :wiki_page }] }]) + post_tags: [:sections, { tag: [:deerjikists, :materials, + { tag_name: :wiki_page }] }]) .with_attached_thumbnail .order('RAND()') .first @@ -190,9 +188,8 @@ class PostsController < ApplicationController post = Post .includes(:uploaded_user, :parents, :children, - active_post_tags: [:sections, - { tag: [:deerjikists, :materials, - { tag_name: :wiki_page }] }]) + post_tags: [:sections, { tag: [:deerjikists, :materials, + { tag_name: :wiki_page }] }]) .with_attached_thumbnail .find_by(id: params[:id]) return head :not_found unless post @@ -386,50 +383,6 @@ class PostsController < ApplicationController render_post_form_record_invalid e.record end - def changes - id = params[:id].presence - tag_id = params[:tag].presence - page = (params[:page].presence || 1).to_i - limit = (params[:limit].presence || 20).to_i - - page = 1 if page < 1 - limit = 1 if limit < 1 - - offset = (page - 1) * limit - - pts = PostTag.with_discarded - pts = pts.where(post_id: id) if id.present? - pts = pts.where(tag_id:) if tag_id.present? - pts = pts.includes(:post, :created_user, :deleted_user, - tag: [:deerjikists, :materials, { tag_name: :wiki_page }]) - - events = [] - pts.each do |pt| - tag = TagRepr.base(pt.tag) - post = pt.post - - events << Event.new( - post:, - tag:, - user: pt.created_user && { id: pt.created_user.id, name: pt.created_user.name }, - change_type: 'add', - timestamp: pt.created_at) - - if pt.discarded_at - events << Event.new( - post:, - tag:, - user: pt.deleted_user && { id: pt.deleted_user.id, name: pt.deleted_user.name }, - change_type: 'remove', - timestamp: pt.discarded_at) - end - end - events.sort_by!(&:timestamp) - events.reverse! - - render json: { changes: (events.slice(offset, limit) || []).as_json, count: events.size } - end - private def filtered_posts @@ -504,13 +457,13 @@ class PostsController < ApplicationController end end - PostTag.where(post_id: post.id, tag_id: to_remove.to_a).kept.find_each do |pt| - pt.discard_by!(current_user) + PostTag.where(post_id: post.id, tag_id: to_remove.to_a).find_each do |pt| + pt.destroy! end end def build_tag_tree_for post - post_tags = post.active_post_tags.reject { |post_tag| post_tag.tag.deprecated? } + post_tags = post.post_tags.reject { |post_tag| post_tag.tag.deprecated? } tags = post_tags.map(&:tag) tag_ids = tags.map(&:id) @@ -719,7 +672,6 @@ class PostsController < ApplicationController def editable_tag_names_from_post post post .post_tags - .kept .joins(tag: :tag_name) .merge(Tag.not_nico) .merge(Tag.where(deprecated_at: nil)) diff --git a/backend/app/controllers/tags_controller.rb b/backend/app/controllers/tags_controller.rb index a940f0b..f152d19 100644 --- a/backend/app/controllers/tags_controller.rb +++ b/backend/app/controllers/tags_controller.rb @@ -573,7 +573,7 @@ class TagsController < ApplicationController return false end - target_tag_name = TagName.with_discarded.find_by(name:) + target_tag_name = TagName.find_by(name:) return true if target_tag_name.nil? return true if target_tag_name.canonical_id? @@ -585,17 +585,14 @@ class TagsController < ApplicationController return if name == tag.name current_tag_name = tag.tag_name - target_tag_name = TagName.with_discarded.find_by(name:) + target_tag_name = TagName.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) + 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: @@ -605,11 +602,9 @@ class TagsController < ApplicationController 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) + TagName.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) @@ -640,7 +635,7 @@ class TagsController < ApplicationController end alias_names.each do |alias_name| - alias_tag_name = TagName.find_undiscard_or_create_by!(name: alias_name) + alias_tag_name = TagName.find_or_create_by!(name: alias_name) affected_tags << alias_tag_name.canonical&.tag end @@ -655,7 +650,7 @@ class TagsController < ApplicationController end alias_names.each do |alias_name| - alias_tag_name = TagName.find_undiscard_or_create_by!(name: alias_name) + alias_tag_name = TagName.find_or_create_by!(name: alias_name) alias_tag_name.update!(canonical: tag.tag_name) end diff --git a/backend/app/controllers/wiki_pages_controller.rb b/backend/app/controllers/wiki_pages_controller.rb index f3ece93..9b38b0e 100644 --- a/backend/app/controllers/wiki_pages_controller.rb +++ b/backend/app/controllers/wiki_pages_controller.rb @@ -94,7 +94,7 @@ class WikiPagesController < ApplicationController return render_unprocessable_entity('タイトルは必須です.', field: :title) if title.blank? return render_unprocessable_entity('本文は必須です.', field: :body) if body.blank? - tag_name = TagName.find_undiscard_or_create_by!(name: title) + tag_name = TagName.find_or_create_by!(name: title) page = Wiki::Commit.create_content!( diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index 08f8fec..e4fd44c 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -55,25 +55,27 @@ class Post < ApplicationRecord belongs_to :uploaded_user, class_name: 'User', optional: true has_many :post_tags, dependent: :destroy, inverse_of: :post - has_many :active_post_tags, -> { kept }, class_name: 'PostTag', inverse_of: :post - has_many :post_tags_with_discarded, -> { with_discarded }, class_name: 'PostTag' - has_many :tags, through: :active_post_tags + has_many :tags, through: :post_tags has_many :active_tags, -> { where(tags: { deprecated_at: nil }) }, - through: :active_post_tags, source: :tag + through: :post_tags, + source: :tag has_many :user_post_views, dependent: :delete_all has_many :post_similarities, dependent: :delete_all has_many :post_versions + has_many :gekanator_guessed_games, class_name: 'GekanatorGame', foreign_key: :guessed_post_id, dependent: :delete_all, inverse_of: :guessed_post + has_many :gekanator_correct_games, class_name: 'GekanatorGame', foreign_key: :correct_post_id, dependent: :delete_all, inverse_of: :correct_post + has_many :gekanator_question_examples, dependent: :delete_all has_many :parent_post_implications, @@ -123,7 +125,6 @@ class Post < ApplicationRecord def snapshot_tag_names post_tags - .kept .joins(tag: :tag_name) .includes(:sections, tag: :tag_name) .order('tag_names.name') @@ -150,7 +151,6 @@ class Post < ApplicationRecord def snapshot_tags_json post_tags - .kept .joins(tag: :tag_name) .includes(:sections, tag: :tag_name) .order('tags.id') diff --git a/backend/app/models/post_tag.rb b/backend/app/models/post_tag.rb index ac56c77..7ed85b0 100644 --- a/backend/app/models/post_tag.rb +++ b/backend/app/models/post_tag.rb @@ -1,14 +1,7 @@ class PostTag < ApplicationRecord - include Discard::Model - - before_destroy do - raise ActiveRecord::ReadOnlyRecord, '消さないでください.' - end - belongs_to :post belongs_to :tag, counter_cache: :post_count belongs_to :created_user, class_name: 'User', optional: true - belongs_to :deleted_user, class_name: 'User', optional: true has_many :sections, -> { order(:begin_ms) }, class_name: 'PostTagSection', foreign_key: [:post_id, :tag_id], @@ -18,18 +11,5 @@ class PostTag < ApplicationRecord validates :post_id, presence: true validates :tag_id, presence: true - validates :post_id, uniqueness: { - scope: :tag_id, - conditions: -> { where(discarded_at: nil) } } - - def discard_by! deleted_user - return self if discarded? - - transaction do - update!(discarded_at: Time.current, deleted_user:) - Tag.where(id: tag_id).update_all('post_count = GREATEST(post_count - 1, 0)') - end - - self - end + validates :post_id, uniqueness: { scope: :tag_id } end diff --git a/backend/app/models/post_tag_section.rb b/backend/app/models/post_tag_section.rb index 5c47e4c..bb4abff 100644 --- a/backend/app/models/post_tag_section.rb +++ b/backend/app/models/post_tag_section.rb @@ -4,10 +4,10 @@ class PostTagSection < ApplicationRecord belongs_to :post belongs_to :tag - belongs_to :post_tag, -> { kept }, foreign_key: [:post_id, :tag_id], - primary_key: [:post_id, :tag_id], - inverse_of: :sections, - optional: true + belongs_to :post_tag, foreign_key: [:post_id, :tag_id], + primary_key: [:post_id, :tag_id], + inverse_of: :sections, + optional: true validates :post_id, presence: true validates :tag_id, presence: true diff --git a/backend/app/models/tag.rb b/backend/app/models/tag.rb index 10d6bb5..9c80f71 100644 --- a/backend/app/models/tag.rb +++ b/backend/app/models/tag.rb @@ -2,8 +2,6 @@ require 'set' class Tag < ApplicationRecord - include MyDiscard - class NicoTagNormalisationError < ArgumentError ; end @@ -28,9 +26,7 @@ class Tag < ApplicationRecord end 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' - has_many :posts, through: :active_post_tags + has_many :posts, through: :post_tags has_many :nico_tag_relations, foreign_key: :nico_tag_id, dependent: :destroy has_many :linked_tags, through: :nico_tag_relations, source: :tag @@ -272,11 +268,11 @@ class Tag < ApplicationRecord TagVersioning.ensure_snapshot!(source_tag, created_by_user:) - source_tag.post_tags.kept.find_each do |source_pt| + source_tag.post_tags.find_each do |source_pt| post_id = source_pt.post_id affected_post_ids << post_id - source_pt.discard_by!(created_by_user) - unless PostTag.kept.exists?(post_id:, tag: target_tag) + source_pt.destroy! + unless PostTag.exists?(post_id:, tag: target_tag) PostTag.create!(post_id:, tag: target_tag) end end @@ -288,10 +284,10 @@ class Tag < ApplicationRecord end TagVersioning.record!(source_tag, event_type: :discard, created_by_user:) - source_tag.discard! + source_tag.destroy! if source_tag.nico? - source_tag_name.discard! + source_tag_name.destroy! else source_tag_name.update_columns(canonical_id: target_tag.tag_name_id, updated_at: Time.current) @@ -306,13 +302,13 @@ class Tag < ApplicationRecord end # 投稿件数を再集計 - target_tag.update_columns(post_count: PostTag.kept.where(tag: target_tag).count) + target_tag.update_columns(post_count: PostTag.where(tag: target_tag).count) end target_tag.reload end - def snapshot_aliases = tag_name.aliases.kept.order(:name).pluck(:name) + def snapshot_aliases = tag_name.aliases.order(:name).pluck(:name) def snapshot_parent_tag_ids = parents.order(:id).pluck(:id) diff --git a/backend/app/models/tag_name.rb b/backend/app/models/tag_name.rb index 12fea7a..a70aa4b 100644 --- a/backend/app/models/tag_name.rb +++ b/backend/app/models/tag_name.rb @@ -1,6 +1,4 @@ class TagName < ApplicationRecord - include MyDiscard - belongs_to :tag has_one :wiki_page diff --git a/backend/app/models/tag_name_sanitisation_rule.rb b/backend/app/models/tag_name_sanitisation_rule.rb index e37738a..bb13a53 100644 --- a/backend/app/models/tag_name_sanitisation_rule.rb +++ b/backend/app/models/tag_name_sanitisation_rule.rb @@ -32,7 +32,7 @@ class TagNameSanitisationRule < ApplicationRecord elsif source_tag source_tag.update_columns(tag_name_id: existing_tn.id, updated_at: Time.current) end - tn.discard! + tn.destroy! next end diff --git a/backend/app/representations/post_repr.rb b/backend/app/representations/post_repr.rb index 0dc8a9b..79a1b1d 100644 --- a/backend/app/representations/post_repr.rb +++ b/backend/app/representations/post_repr.rb @@ -88,14 +88,14 @@ module PostRepr def tag_json post post - .active_post_tags + .post_tags .reject { _1.tag.deprecated? } .sort_by { _1.tag.name } - .map { |post_tag| + .map do |post_tag| TagRepr.inline(post_tag.tag).merge( 'children' => [], 'sections' => post_tag.sections.as_json(only: [:begin_ms, :end_ms])) - } + end end def thumbnail_url post, host: nil diff --git a/backend/app/services/post_creator.rb b/backend/app/services/post_creator.rb index 8078991..788a671 100644 --- a/backend/app/services/post_creator.rb +++ b/backend/app/services/post_creator.rb @@ -121,9 +121,11 @@ class PostCreator def sync_post_tags! post, desired_tags, sections desired_ids = desired_tags.map(&:id).to_set current_ids = post.tags.pluck(:id).to_set + Tag.where(id: desired_ids - current_ids).find_each do |tag| PostTag.create_or_find_by!(post:, tag:, created_user: @actor) end + PostTagSection.where(post_id: post.id).destroy_all sections.each do |tag_id, ranges| ranges.each do |begin_ms, end_ms| @@ -133,10 +135,9 @@ class PostCreator end_ms:) end end + PostTag.where(post_id: post.id, - tag_id: (current_ids - desired_ids).to_a).kept.find_each do |post_tag| - post_tag.discard_by!(@actor) - end + tag_id: (current_ids - desired_ids).to_a).destroy_all end def sync_parent_posts! post, ids diff --git a/backend/app/services/youtube/sync.rb b/backend/app/services/youtube/sync.rb index 7fe46f0..e415c95 100644 --- a/backend/app/services/youtube/sync.rb +++ b/backend/app/services/youtube/sync.rb @@ -103,7 +103,7 @@ module Youtube end def sync_post_tags! post, desired_tag_ids, current_tag_ids: nil - current_tag_ids ||= PostTag.kept.where(post_id: post.id).pluck(:tag_id).to_set + current_tag_ids ||= PostTag.where(post_id: post.id).pluck(:tag_id).to_set desired_tag_ids = desired_tag_ids.compact.to_set to_add = desired_tag_ids - current_tag_ids @@ -117,8 +117,8 @@ module Youtube end end - PostTag.where(post_id: post.id, tag_id: to_remove.to_a).kept.find_each do |pt| - pt.discard_by!(nil) + PostTag.where(post_id: post.id, tag_id: to_remove.to_a).find_each do |pt| + pt.destroy! end end diff --git a/backend/config/routes.rb b/backend/config/routes.rb index 30cc5bd..00fc425 100644 --- a/backend/config/routes.rb +++ b/backend/config/routes.rb @@ -55,7 +55,6 @@ Rails.application.routes.draw do get :metadata post :bulk get :random - get :changes get :versions, to: 'post_versions#index' end diff --git a/backend/db/migrate/20260921000000_delete_inactive_records_from_post_tags.rb b/backend/db/migrate/20260921000000_delete_inactive_records_from_post_tags.rb new file mode 100644 index 0000000..a1a5d58 --- /dev/null +++ b/backend/db/migrate/20260921000000_delete_inactive_records_from_post_tags.rb @@ -0,0 +1,53 @@ +class DeleteInactiveRecordsFromPostTags < ActiveRecord::Migration[8.0] + def up + execute <<~SQL + DELETE + FROM + post_tags + WHERE + discarded_at IS NOT NULL + SQL + + remove_index :post_tags, [:tag_id, :discarded_at] + remove_index :post_tags, [:post_id, :discarded_at] + remove_index :post_tags, name: 'idx_post_tags_active_unique' + remove_index :post_tags, :discarded_at + + remove_foreign_key :post_tags, column: :deleted_user_id + remove_index :post_tags, :deleted_user_id + + remove_column :post_tags, :active_unique_key + remove_column :post_tags, :is_active + remove_column :post_tags, :discarded_at + remove_column :post_tags, :deleted_user_id + remove_column :post_tags, :updated_at + + execute <<~SQL + ALTER TABLE + post_tags + MODIFY COLUMN + id BIGINT NOT NULL + SQL + + execute <<~SQL + ALTER TABLE + post_tags + DROP PRIMARY KEY + SQL + + remove_column :post_tags, :id + + execute <<~SQL + ALTER TABLE + post_tags + ADD PRIMARY KEY + (post_id, tag_id) + SQL + + remove_index :post_tags, :post_id + end + + def down + raise ActiveRecord::IrreversibleMigration, '戻せません.' + end +end diff --git a/backend/db/migrate/20260921010000_add_foreign_key_on_post_id_and_tag_id_in_post_tag_sections.rb b/backend/db/migrate/20260921010000_add_foreign_key_on_post_id_and_tag_id_in_post_tag_sections.rb new file mode 100644 index 0000000..891a886 --- /dev/null +++ b/backend/db/migrate/20260921010000_add_foreign_key_on_post_id_and_tag_id_in_post_tag_sections.rb @@ -0,0 +1,11 @@ +class AddForeignKeyOnPostIdAndTagIdInPostTagSections < ActiveRecord::Migration[8.0] + def change + remove_foreign_key :post_tag_sections, :posts, column: :post_id + remove_foreign_key :post_tag_sections, :tags, column: :tag_id + + add_foreign_key :post_tag_sections, :post_tags, + column: [:post_id, :tag_id], + primary_key: [:post_id, :tag_id], + on_delete: :cascade + end +end diff --git a/backend/db/migrate/20260921020000_delete_discarded_records_from_tags.rb b/backend/db/migrate/20260921020000_delete_discarded_records_from_tags.rb new file mode 100644 index 0000000..a9af657 --- /dev/null +++ b/backend/db/migrate/20260921020000_delete_discarded_records_from_tags.rb @@ -0,0 +1,46 @@ +class DeleteDiscardedRecordsFromTags < ActiveRecord::Migration[8.0] + def up + remove_foreign_key :tag_versions, :tags, column: :tag_id + remove_foreign_key :nico_tag_versions, :tags, column: :tag_id + remove_foreign_key :material_versions, :tags, column: :tag_id + + execute <<~SQL + DELETE + ntr + FROM + nico_tag_relations ntr + INNER JOIN + tags t + ON + t.discarded_at IS NOT NULL + AND t.id IN (ntr.tag_id, ntr.nico_tag_id) + SQL + + execute <<~SQL + DELETE + ti + FROM + tag_implications ti + INNER JOIN + tags t + ON + t.discarded_at IS NOT NULL + AND t.id IN (ti.tag_id, ti.parent_tag_id) + SQL + + execute <<~SQL + DELETE + FROM + tags + WHERE + discarded_at IS NOT NULL + SQL + + remove_index :tags, :discarded_at + remove_column :tags, :discarded_at + end + + def down + raise ActiveRecord::IrreversibleMigration, '戻せません.' + end +end diff --git a/backend/db/migrate/20260921030000_delete_discarded_records_from_tag_names.rb b/backend/db/migrate/20260921030000_delete_discarded_records_from_tag_names.rb new file mode 100644 index 0000000..fe1d5ca --- /dev/null +++ b/backend/db/migrate/20260921030000_delete_discarded_records_from_tag_names.rb @@ -0,0 +1,18 @@ +class DeleteDiscardedRecordsFromTagNames < ActiveRecord::Migration[8.0] + def up + execute <<~SQL + DELETE + FROM + tag_names + WHERE + discarded_at IS NOT NULL + SQL + + remove_index :tag_names, :discarded_at + remove_column :tag_names, :discarded_at + end + + def down + raise ActiveRecord::IrreversibleMigration, '戻せません.' + end +end diff --git a/backend/db/schema.rb b/backend/db/schema.rb index 5690ffa..58c768b 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_07_27_123600) do +ActiveRecord::Schema[8.0].define(version: 2026_09_21_030000) 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 @@ -308,23 +308,12 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do t.check_constraint "`begin_ms` >= 0", name: "chk_post_tag_sections_begin_ms_natural" end - create_table "post_tags", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + create_table "post_tags", primary_key: ["post_id", "tag_id"], charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "post_id", null: false t.bigint "tag_id", null: false t.bigint "created_user_id" - t.bigint "deleted_user_id" t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.datetime "discarded_at" - t.virtual "is_active", type: :boolean, as: "(`discarded_at` is null)", stored: true - t.virtual "active_unique_key", type: :string, as: "(case when (`discarded_at` is null) then concat(`post_id`,_utf8mb4':',`tag_id`) else NULL end)", stored: true - t.index ["active_unique_key"], name: "idx_post_tags_active_unique", unique: true t.index ["created_user_id"], name: "index_post_tags_on_created_user_id" - t.index ["deleted_user_id"], name: "index_post_tags_on_deleted_user_id" - t.index ["discarded_at"], name: "index_post_tags_on_discarded_at" - t.index ["post_id", "discarded_at"], name: "index_post_tags_on_post_id_and_discarded_at" - t.index ["post_id"], name: "index_post_tags_on_post_id" - t.index ["tag_id", "discarded_at"], name: "index_post_tags_on_tag_id_and_discarded_at" t.index ["tag_id"], name: "index_post_tags_on_tag_id" end @@ -420,9 +409,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do t.bigint "canonical_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.datetime "discarded_at" t.index ["canonical_id"], name: "index_tag_names_on_canonical_id" - t.index ["discarded_at"], name: "index_tag_names_on_discarded_at" t.index ["name"], name: "index_tag_names_on_name", unique: true end @@ -458,10 +445,8 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) 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.integer "version_no", null: false t.index ["deprecated_at"], name: "index_tags_on_deprecated_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.check_constraint "(`deprecated_at` is null) or (`category` <> _utf8mb4'nico')", name: "chk_tags_deprecated_at_not_nico" end @@ -601,6 +586,19 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do t.index ["banned_at"], name: "index_users_on_banned_at" end + create_table "wiki_assets", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.bigint "wiki_page_id", null: false + t.integer "no", null: false + t.string "alt_text" + t.binary "sha256", limit: 32, null: false + t.bigint "created_by_user_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["created_by_user_id"], name: "index_wiki_assets_on_created_by_user_id" + t.index ["wiki_page_id", "no"], name: "index_wiki_assets_on_wiki_page_id_and_no", unique: true + t.index ["wiki_page_id", "sha256"], name: "index_wiki_assets_on_wiki_page_id_and_sha256", unique: true + end + create_table "wiki_lines", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "sha256", limit: 64, null: false t.text "body", null: false @@ -690,7 +688,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do add_foreign_key "material_sync_suppressions", "users", column: "created_by_user_id" add_foreign_key "material_versions", "materials" add_foreign_key "material_versions", "materials", column: "parent_id" - add_foreign_key "material_versions", "tags" add_foreign_key "material_versions", "users", column: "created_by_user_id" add_foreign_key "material_versions", "users", column: "updated_by_user_id" add_foreign_key "materials", "materials", column: "parent_id" @@ -699,18 +696,15 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do add_foreign_key "materials", "users", column: "updated_by_user_id" add_foreign_key "nico_tag_relations", "tags" add_foreign_key "nico_tag_relations", "tags", column: "nico_tag_id" - add_foreign_key "nico_tag_versions", "tags" add_foreign_key "nico_tag_versions", "users", column: "created_by_user_id" add_foreign_key "post_implications", "posts" add_foreign_key "post_implications", "posts", column: "parent_post_id" add_foreign_key "post_similarities", "posts" add_foreign_key "post_similarities", "posts", column: "target_post_id" - add_foreign_key "post_tag_sections", "posts" - add_foreign_key "post_tag_sections", "tags" + add_foreign_key "post_tag_sections", "post_tags", column: ["post_id", "tag_id"], primary_key: ["post_id", "tag_id"], on_delete: :cascade add_foreign_key "post_tags", "posts" add_foreign_key "post_tags", "tags" add_foreign_key "post_tags", "users", column: "created_user_id" - add_foreign_key "post_tags", "users", column: "deleted_user_id" add_foreign_key "post_versions", "posts" add_foreign_key "post_versions", "users", column: "created_by_user_id" add_foreign_key "posts", "users", column: "uploaded_user_id" @@ -720,7 +714,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do add_foreign_key "tag_names", "tag_names", column: "canonical_id" add_foreign_key "tag_similarities", "tags" add_foreign_key "tag_similarities", "tags", column: "target_tag_id" - add_foreign_key "tag_versions", "tags" add_foreign_key "tag_versions", "users", column: "created_by_user_id" add_foreign_key "tags", "tag_names" add_foreign_key "theatre_comments", "theatres" diff --git a/backend/lib/tasks/sync_nico.rake b/backend/lib/tasks/sync_nico.rake index 7aca540..f95fba1 100644 --- a/backend/lib/tasks/sync_nico.rake +++ b/backend/lib/tasks/sync_nico.rake @@ -16,7 +16,7 @@ namespace :nico do end def sync_post_tags! post, desired_tag_ids, current_tag_ids: nil - current_tag_ids ||= PostTag.kept.where(post_id: post.id).pluck(:tag_id).to_set + current_tag_ids ||= PostTag.where(post_id: post.id).pluck(:tag_id).to_set desired_tag_ids = desired_tag_ids.compact.to_set to_add = desired_tag_ids - current_tag_ids @@ -30,8 +30,8 @@ namespace :nico do end end - PostTag.where(post_id: post.id, tag_id: to_remove.to_a).kept.find_each do |pt| - pt.discard_by!(nil) + PostTag.where(post_id: post.id, tag_id: to_remove.to_a).find_each do |pt| + pt.destroy! end end diff --git a/backend/spec/db/delete_discarded_tags_spec.rb b/backend/spec/db/delete_discarded_tags_spec.rb new file mode 100644 index 0000000..3882e0d --- /dev/null +++ b/backend/spec/db/delete_discarded_tags_spec.rb @@ -0,0 +1,123 @@ +require 'rails_helper' +require_relative '../../db/migrate/20260921020000_delete_discarded_records_from_tags' +require_relative '../../db/migrate/20260921030000_delete_discarded_records_from_tag_names' + +RSpec.describe 'discarded tag cleanup migrations' do + [DeleteDiscardedRecordsFromTags, DeleteDiscardedRecordsFromTagNames].each do |migration_class| + it "rejects rollback of #{ migration_class.name }" do + expect { migration_class.new.down } + .to raise_error(ActiveRecord::IrreversibleMigration) + end + end + + context 'with legacy records' do + self.use_transactional_tests = false + + before do + record_class = Class.new(ActiveRecord::Base) do + self.abstract_class = true + end + stub_const('TagCleanupMigrationRecord', record_class) + config = ActiveRecord::Base.connection_db_config.configuration_hash + @database = "btrc_hub_test_tag_cleanup_#{ Process.pid }_#{ SecureRandom.hex(4) }" + record_class.establish_connection(config.merge(database: nil)) + @connection = record_class.lease_connection + @connection.create_database(@database) + @database_created = true + @connection.execute("USE #{ @connection.quote_table_name(@database) }") + end + + after do + @connection.drop_database(@database) if @database_created + ensure + TagCleanupMigrationRecord.remove_connection + end + + before do + @connection.create_table(:tag_names) do |t| + t.string :name, null: false, index: { unique: true } + t.bigint :canonical_id + t.datetime :discarded_at, index: true + end + @connection.add_foreign_key(:tag_names, :tag_names, column: :canonical_id) + @connection.create_table(:tags) do |t| + t.references :tag_name, null: false, foreign_key: true, index: { unique: true } + t.datetime :discarded_at, index: true + end + @connection.create_table(:nico_tag_relations) do |t| + t.references :tag, null: false, foreign_key: true + t.references :nico_tag, null: false, foreign_key: { to_table: :tags } + end + @connection.create_table(:tag_implications) do |t| + t.references :tag, null: false, foreign_key: true + t.references :parent_tag, null: false, foreign_key: { to_table: :tags } + end + [:tag_versions, :nico_tag_versions, :material_versions].each do |table| + @connection.create_table(table) do |t| + t.references :tag, null: false, foreign_key: true + end + end + + @connection.execute(<<~SQL) + INSERT INTO tag_names (id, name, canonical_id, discarded_at) VALUES + (1, 'kept', NULL, NULL), + (2, 'merged_alias', 1, NULL), + (3, 'nico:deleted', NULL, '2026-09-20'), + (4, 'nico:kept', NULL, NULL), + (5, 'deleted_name', NULL, '2026-09-20') + SQL + @connection.execute(<<~SQL) + INSERT INTO tags (id, tag_name_id, discarded_at) VALUES + (1, 1, NULL), (2, 2, '2026-09-20'), + (3, 3, '2026-09-20'), (4, 4, NULL) + SQL + @connection.execute(<<~SQL) + INSERT INTO nico_tag_relations (id, tag_id, nico_tag_id) VALUES + (1, 1, 4), (2, 2, 4), (3, 1, 3), (4, 2, 3) + SQL + @connection.execute(<<~SQL) + INSERT INTO tag_implications (id, tag_id, parent_tag_id) VALUES + (1, 1, 4), (2, 2, 1), (3, 1, 2), (4, 2, 3) + SQL + @connection.execute('INSERT INTO tag_versions (tag_id) VALUES (1), (2)') + @connection.execute('INSERT INTO nico_tag_versions (tag_id) VALUES (3), (4)') + @connection.execute('INSERT INTO material_versions (tag_id) VALUES (1), (2)') + end + + it 'removes discarded records and their links while retaining aliases and history' do + [DeleteDiscardedRecordsFromTags, DeleteDiscardedRecordsFromTagNames].each do |klass| + migration = klass.new + allow(migration).to receive(:connection).and_return(@connection) + migration.suppress_messages { migration.up } + end + + expect(@connection.select_values('SELECT id FROM tags ORDER BY id')).to eq([1, 4]) + expect(@connection.select_rows('SELECT id, canonical_id FROM tag_names ORDER BY id')) + .to eq([[1, nil], [2, 1], [4, nil]]) + expect(@connection.select_values('SELECT id FROM nico_tag_relations')).to eq([1]) + expect(@connection.select_values('SELECT id FROM tag_implications')).to eq([1]) + expect(@connection.select_values('SELECT tag_id FROM tag_versions ORDER BY tag_id')) + .to eq([1, 2]) + expect(@connection.select_values('SELECT tag_id FROM nico_tag_versions ORDER BY tag_id')) + .to eq([3, 4]) + expect(@connection.select_values('SELECT tag_id FROM material_versions ORDER BY tag_id')) + .to eq([1, 2]) + + [:tags, :tag_names].each do |table| + expect(@connection.column_exists?(table, :discarded_at)).to be(false) + expect(@connection.index_exists?(table, :discarded_at)).to be(false) + end + [:tag_versions, :nico_tag_versions, :material_versions].each do |table| + expect(@connection.foreign_key_exists?(table, :tags, column: :tag_id)).to be(false) + end + expect(@connection.foreign_key_exists?(:tags, :tag_names)).to be(true) + expect(@connection.foreign_key_exists?(:tag_names, :tag_names, column: :canonical_id)) + .to be(true) + expect(@connection.index_exists?(:tag_names, :name, unique: true)).to be(true) + expect(@connection.index_exists?(:tags, :tag_name_id, unique: true)).to be(true) + [:nico_tag_relations, :tag_implications].each do |table| + expect(@connection.foreign_keys(table).map(&:to_table)).to eq(['tags', 'tags']) + end + end + end +end diff --git a/backend/spec/models/post_tag_spec.rb b/backend/spec/models/post_tag_spec.rb index 6f89ac5..eb7d2db 100644 --- a/backend/spec/models/post_tag_spec.rb +++ b/backend/spec/models/post_tag_spec.rb @@ -1,5 +1,73 @@ +require 'rails_helper' + RSpec.describe PostTag, type: :model do + describe 'uniqueness' do + it 'rejects duplicate post and tag pairs but allows either to be reused' do + post_tag = create(:post_tag) + duplicate = build(:post_tag, post: post_tag.post, tag: post_tag.tag) + + expect(duplicate).not_to be_valid + expect(duplicate.errors.of_kind?(:post_id, :taken)).to be(true) + expect(build(:post_tag, post: post_tag.post, tag: create(:tag))).to be_valid + expect(build(:post_tag, post: create(:post), tag: post_tag.tag)).to be_valid + end + + it 'enforces uniqueness in the database when validation is bypassed' do + post_tag = create(:post_tag) + duplicate = build(:post_tag, post: post_tag.post, tag: post_tag.tag) + + expect { duplicate.save!(validate: false) } + .to raise_error(ActiveRecord::RecordNotUnique) + end + end + + describe '#destroy!' do + it 'deletes only the selected pair and its sections and updates the counter' do + post_tag = create(:post_tag) + same_post = create(:post_tag, post: post_tag.post) + same_tag = create(:post_tag, tag: post_tag.tag) + sections = [post_tag, same_post, same_tag].map do |link| + create(:post_tag_section, post: link.post, tag: link.tag, + begin_ms: 1000, end_ms: 2000) + end + + expect { post_tag.destroy! }.to change(described_class, :count).by(-1) + .and change(PostTagSection, :count).by(-1) + .and change { post_tag.tag.reload.post_count }.from(2).to(1) + + expect(described_class.exists?(post: post_tag.post, tag: post_tag.tag)).to be(false) + expect(same_post.reload).to be_persisted + expect(same_tag.reload).to be_persisted + expect(PostTagSection.all).to contain_exactly(*sections.drop(1)) + expect(post_tag.post.reload.tags).to contain_exactly(same_post.tag) + expect(post_tag.tag.reload.posts).to contain_exactly(same_tag.post) + end + + it 'allows a removed tag to be added again without restoring old sections' do + post_tag = create(:post_tag) + create(:post_tag_section, post: post_tag.post, tag: post_tag.tag, + begin_ms: 1000, end_ms: 2000) + post_tag.destroy! + + replacement = create(:post_tag, post: post_tag.post, tag: post_tag.tag) + + expect(replacement.reload.sections).to be_empty + expect(replacement.tag.reload.post_count).to eq(1) + end + end + describe '#sections' do + it 'loads the owning post_tag from a section using both keys' do + post_tag = create(:post_tag) + create(:post_tag, post: post_tag.post) + create(:post_tag, tag: post_tag.tag) + section = create(:post_tag_section, post: post_tag.post, + tag: post_tag.tag, + begin_ms: 1000, end_ms: 2000) + + expect(section.reload.post_tag).to eq(post_tag) + end + it 'loads sections by post_id and tag_id' do post_tag = create(:post_tag) section = create(:post_tag_section, @@ -12,18 +80,25 @@ RSpec.describe PostTag, type: :model do end it 'does not load sections for another tag on the same post' do - post = create(:post) - tag = create(:tag) + post_tag = create(:post_tag) + post = post_tag.post other_tag = create(:tag) - post_tag = create(:post_tag, post:, tag:) + own_section = create(:post_tag_section, + post:, + tag: post_tag.tag, + begin_ms: 1000, + end_ms: 2000) + + create(:post_tag, post:, tag: other_tag) + create(:post_tag_section, post:, tag: other_tag, begin_ms: 1000, end_ms: 2000) - expect(post_tag.sections).to be_empty + expect(post_tag.reload.sections).to contain_exactly(own_section) end it 'allows open-ended sections' do diff --git a/backend/spec/models/tag_name_sanitisation_rule_spec.rb b/backend/spec/models/tag_name_sanitisation_rule_spec.rb index ea14461..b82f9fa 100644 --- a/backend/spec/models/tag_name_sanitisation_rule_spec.rb +++ b/backend/spec/models/tag_name_sanitisation_rule_spec.rb @@ -57,7 +57,7 @@ RSpec.describe TagNameSanitisationRule, type: :model do it 'deletes the source tag_name' do described_class.apply! - expect(TagName.exists?(source.id)).to be(false) + expect(TagName.unscoped.exists?(source.id)).to be(false) expect(existing.reload.name).to eq('foobar') end end @@ -75,7 +75,27 @@ RSpec.describe TagNameSanitisationRule, type: :model do described_class.apply! expected_tag_name_id = existing.canonical_id || existing.id expect(source_tag.reload.tag_name_id).to eq(expected_tag_name_id) - expect(TagName.exists?(source_tag_name_id)).to be(false) + expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false) + end + end + + context 'when the sanitised name is an alias of an existing tag' do + let!(:existing_tag) { create(:tag) } + let!(:alias_name) do + TagName.create!(name: 'foobar', canonical: existing_tag.tag_name) + end + let!(:source) do + TagName.create!(name: 'tmp').tap do |tn| + tn.update_columns(name: 'foo_bar', updated_at: Time.current) + end + end + + it 'deletes only the source and preserves the alias and its canonical tag' do + described_class.apply! + + expect(TagName.unscoped.exists?(source.id)).to be(false) + expect(alias_name.reload.canonical).to eq(existing_tag.tag_name) + expect(Tag.find(existing_tag.id)).to eq(existing_tag) end end @@ -92,13 +112,15 @@ RSpec.describe TagNameSanitisationRule, type: :model do end it 'merges the source tag into the existing tag and deletes the source tag_name' do - expect(TagName.find_by(name: 'foobar')&.tag&.id).to eq(existing_tag.id) - expect(TagName.find_by(name: 'foo_bar')&.tag&.id).to eq(source_tag.id) + post = create(:post) + PostTag.create!(post:, tag: source_tag) described_class.apply! - expect(Tag.exists?(source_tag.id)).to be(false) - expect(TagName.exists?(source_tag.tag_name_id)).to be(false) + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false) + expect(post.reload.tags).to contain_exactly(existing_tag) + expect(existing_tag.reload.name).to eq('foobar') end end end diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index ccff0be..e532e25 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -173,6 +173,47 @@ RSpec.describe Tag, type: :model do 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 + + expect { + tag = described_class.find_or_create_by_tag_name!( + ' lookup_new ', category: :character) + }.to change(Tag, :count).by(1).and change(TagName, :count).by(1) + + expect(tag.name).to eq('lookup_new') + expect(tag.category).to eq('character') + end + + it 'reuses the canonical tag for an alias without changing its category' do + tag = create(:tag, category: :character) + alias_name = TagName.create!(name: 'lookup_alias', canonical: tag.tag_name) + found = nil + + expect { + found = described_class.find_or_create_by_tag_name!( + alias_name.name, category: :general) + }.to change(Tag, :count).by(0).and change(TagName, :count).by(0) + + expect(found).to eq(tag) + expect(found.category).to eq('character') + end + + it 'creates a tag for an existing canonical name reached through an alias' do + canonical = create(:tag_name) + alias_name = TagName.create!(name: 'lookup_alias', canonical:) + tag = nil + + expect { + tag = described_class.find_or_create_by_tag_name!( + alias_name.name, category: :general) + }.to change(Tag, :count).by(1).and change(TagName, :count).by(0) + + expect(tag.tag_name).to eq(canonical) + end + end + describe '.merge_tags!' do let!(:target_tag) { create(:tag, category: :general) } let!(:source_tag) { create(:tag, category: :general) } @@ -185,18 +226,14 @@ 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 'discards the source post_tag, creates an active target post_tag, discards the source tag, and aliases the source tag_name' do + it 'deletes the source tag, moves its post link, and keeps its name as an alias' do described_class.merge_tags!(target_tag, [source_tag]) - 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) + target_link = PostTag.find_by(post: post_record, tag: target_tag) - 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(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded + expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) + expect(target_link).to be_present + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end @@ -206,38 +243,101 @@ 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 source post_tag, keeps one active target post_tag, discards the source tag, and aliases the source tag_name' do + it 'deletes the source link and preserves the existing target link' do + create(:post_tag_section, post: post_record, tag: source_tag, + begin_ms: 1000, end_ms: 2000) + target_section = create(:post_tag_section, post: post_record, + tag: target_tag, + begin_ms: 3000, end_ms: nil) + 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) + target_links = PostTag.where(post: post_record, tag: target_tag) - 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.first.id).to eq(target_post_tag.id) + expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) + expect(target_links).to contain_exactly(target_post_tag) + expect(PostTagSection.where(post: post_record, tag: source_tag)).to be_empty + expect(target_post_tag.reload.sections).to contain_exactly(target_section) - expect(Tag.with_discarded.find(source_tag.id)).to be_discarded - expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end end + it 'keeps source history and records the new target alias after deleting the source' do + user = create_member_user! + source_name = source_tag.name + source_alias = TagName.create!(name: 'merge_alias', canonical: source_tag_name) + TagVersioning.ensure_snapshot!(source_tag, created_by_user: user) + original_version = source_tag.tag_versions.first + + described_class.merge_tags!(target_tag, [source_tag], created_by_user: user) + + versions = TagVersion.where(tag_id: source_tag.id).order(:version_no) + expect(versions.pluck(:version_no, :event_type)) + .to eq([[1, 'create'], [2, 'discard']]) + expect(versions.first).to eq(original_version) + expect(versions.last).to have_attributes( + name: source_name, aliases: source_alias.name, created_by_user: user) + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + + target_versions = target_tag.tag_versions.order(:version_no) + expect(target_versions.pluck(:event_type)).to eq(['create', 'update']) + expect(target_versions.last.aliases.split).to eq([source_name]) + end + + it 'deletes source relationships while preserving unrelated relationships' do + parent = create(:tag) + child = create(:tag) + nico_tag = create(:tag, :nico) + 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) + NicoTagRelation.create!(tag: source_tag, nico_tag:) + kept_relation = NicoTagRelation.create!(tag: target_tag, nico_tag:) + TagSimilarity.create!(tag: source_tag, target_tag:, cos: 0.5) + TagSimilarity.create!(tag: target_tag, target_tag: source_tag, cos: 0.5) + kept_similarity = TagSimilarity.create!(tag: target_tag, target_tag: parent, + cos: 0.5) + + described_class.merge_tags!(target_tag, [source_tag]) + + expect(TagImplication.all).to contain_exactly(kept_implication) + expect(NicoTagRelation.all).to contain_exactly(kept_relation) + expect(TagSimilarity.all).to contain_exactly(kept_similarity) + expect(TagVersion.where(tag_id: source_tag.id).order(:version_no).last.parent_tag_ids) + .to eq(parent.id.to_s) + end + + it 'preserves material history referencing the deleted source tag' do + source_tag.update!(category: :material) + target_tag.update!(category: :material) + material = Material.create!(tag: source_tag, url: 'https://example.com/material') + version = MaterialVersionRecorder.record!( + material:, event_type: :create, created_by_user: nil) + material.update!(tag: target_tag) + + described_class.merge_tags!(target_tag, [source_tag]) + + expect(version.reload).to have_attributes( + tag_id: source_tag.id, tag_name: source_tag_name.name, tag_category: 'material') + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(material.reload.tag).to eq(target_tag) + end + 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 in source_tags while still merging the source tag' do described_class.merge_tags!(target_tag, [source_tag, target_tag]) - 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) + target_link = PostTag.find_by(post: post_record, tag: target_tag) 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 + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) + expect(target_link).to be_present expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end @@ -260,18 +360,16 @@ RSpec.describe Tag, type: :model do ) end - it 'still merges, but discards the source tag_name instead of aliasing it' do + it 'still merges and keeps the source name as an alias without validating it' do described_class.merge_tags!(target_tag, [source_tag]) - 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) - discarded_source_tag_name = TagName.with_discarded.find(source_tag_name.id) + target_link = PostTag.find_by(post: post_record, tag: target_tag) - 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(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) + expect(target_link).to be_present - expect(Tag.with_discarded.find(source_tag.id)).to be_discarded + expect(Tag.unscoped.exists?(source_tag.id)).to be(false) + expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id) expect(target_tag.reload.post_count).to eq(1) end end @@ -288,36 +386,72 @@ RSpec.describe Tag, type: :model do message: 'init') end - it 'rolls back the transaction' do + it 'rolls back earlier deletions, links, and history when a later source has a wiki' do + earlier_source = create(:tag) + earlier_name = earlier_source.tag_name + source_section = create(:post_tag_section, post: post_record, + tag: source_tag, + begin_ms: 1000, end_ms: 2000) + expect { - described_class.merge_tags!(target_tag, [source_tag]) + described_class.merge_tags!(target_tag, [earlier_source, source_tag]) }.to raise_error(ActiveRecord::RecordInvalid) - expect(Tag.with_discarded.find(source_tag.id)).not_to be_discarded - expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded - expect(PostTag.kept.find(source_post_tag.id).tag_id).to eq(source_tag.id) - expect(PostTag.kept.find_by(post_id: post_record.id, tag_id: target_tag.id)).to be_nil + expect(Tag.unscoped.exists?(earlier_source.id)).to be(true) + expect(earlier_name.reload.canonical_id).to be_nil + expect(TagVersion.where(tag_id: [earlier_source.id, source_tag.id, target_tag.id])) + .to be_empty + expect(Tag.unscoped.exists?(source_tag.id)).to be(true) + expect(TagName.unscoped.exists?(source_tag_name.id)).to be(true) + expect(source_post_tag.reload.tag_id).to eq(source_tag.id) + expect(source_post_tag.sections).to contain_exactly(source_section) + expect(PostTag.find_by(post: post_record, tag: target_tag)).to be_nil + expect(source_tag.reload.post_count).to eq(1) expect(source_tag_name.reload.canonical_id).to be_nil expect(target_tag.reload.post_count).to eq(0) end end context 'when merging a nico source tag' do - let!(:target_tag) { create(:tag, category: :nico, name: 'nico:foo') } - let!(:source_tag) { create(:tag, category: :nico, name: 'nico:bar') } + 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 'discards the source tag_name instead of aliasing it' do + it 'deletes the source tag and name instead of keeping an alias' do described_class.merge_tags!(target_tag, [source_tag]) - discarded_source_tag = Tag.with_discarded.find(source_tag.id) - discarded_source_tag_name = TagName.with_discarded.find(source_tag_name_id) - - expect(discarded_source_tag).to be_discarded - expect(discarded_source_tag_name).to be_discarded - expect(discarded_source_tag_name.canonical_id).to be_nil + 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) @@ -365,12 +499,15 @@ 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') }) + .to eq([source_tag.id]) expect(unaffected_post.reload.post_versions.count).to eq(1) end end - context 'when the source tag has no active post_tags' do + context 'when the source tag has no post_tags' do let!(:another_post) do Post.create!(url: 'https://example.com/posts/3', title: 'another post') end diff --git a/backend/spec/requests/posts_spec.rb b/backend/spec/requests/posts_spec.rb index 26a7c2b..fb08d21 100644 --- a/backend/spec/requests/posts_spec.rb +++ b/backend/spec/requests/posts_spec.rb @@ -148,12 +148,15 @@ RSpec.describe 'Posts API', type: :request do it 'keeps children and sections keys in non-detail tag responses' do PostTagSection.create!(post: hit_post, tag:, begin_ms: 1_000, end_ms: nil) + deprecated_tag = create(:tag, deprecated_at: Time.current) + create(:post_tag, post: hit_post, tag: deprecated_tag) get '/posts' expect(response).to have_http_status(:ok) hit_json = json.fetch('posts').find { |post| post['id'] == hit_post.id } + expect(hit_json.fetch('tags').map { |item| item.fetch('id') }).to eq([tag.id]) tag_json = hit_json.fetch('tags').find { |item| item['name'] == 'spec_tag' } expect(tag_json.fetch('children')).to eq([]) @@ -162,6 +165,26 @@ RSpec.describe 'Posts API', type: :request do ]) end + it 'preloads tag details and sections as the number of posts grows' do + 5.times do + link = create(:post_tag, post: create(:post, uploaded_user: user)) + create(:post_tag_section, post: link.post, tag: link.tag, + begin_ms: 1000, end_ms: 2000) + end + get '/posts', params: { limit: 1 } + + one_post_queries = count_sql_queries do + get '/posts', params: { limit: 1 } + end + many_post_queries = count_sql_queries do + get '/posts', params: { limit: 20 } + end + + expect(response).to have_http_status(:ok) + expect(json.fetch('posts').size).to eq(8) + expect(many_post_queries).to be <= one_post_queries + end + context 'when q is provided' do it 'filters posts by q (hit case)' do get '/posts', params: { tags: 'spec_tag' } @@ -460,6 +483,71 @@ RSpec.describe 'Posts API', type: :request do end end + context 'when update times include version history' do + let(:t0) { Time.zone.parse('2020-01-01 12:00:00') } + let(:t1) { t0 + 1.day } + let(:t2) { t0 + 2.days } + let(:t3) { t0 + 3.days } + let!(:history_post) do + create(:post, url: 'https://example.com/version-time/history', + created_at: t0, updated_at: t0) + end + let!(:plain_post) do + create(:post, url: 'https://example.com/version-time/plain', + created_at: t1, updated_at: t1) + end + let!(:newer_post) do + create(:post, url: 'https://example.com/version-time/newer', + created_at: t0, updated_at: t3) + end + + before do + link = create(:post_tag, post: history_post, tag:) + travel_to(t0) do + PostVersionRecorder.record!(post: history_post, + event_type: :create, created_by_user: nil) + PostVersionRecorder.record!(post: newer_post, + event_type: :create, created_by_user: nil) + end + travel_to(t2) do + link.destroy! + PostVersionRecorder.record!(post: history_post, + event_type: :update, created_by_user: nil) + end + create(:post_tag, post: plain_post, tag:, created_at: t3) + end + + ['asc', 'desc'].each do |direction| + it "sorts by the later of post update and latest version time (#{ direction })" do + get '/posts', params: { url: '/version-time/', order: "updated_at:#{ direction }" } + + expect(response).to have_http_status(:ok) + expected_ids = [plain_post.id, history_post.id, newer_post.id] + expected_ids.reverse! if direction == 'desc' + expect(json.fetch('posts').map { |item| item.fetch('id') }).to eq(expected_ids) + expect(json.fetch('count')).to eq(3) + + times = json.fetch('posts').to_h do |item| + [item.fetch('id'), Time.zone.parse(item.fetch('updated_at'))] + end + expect(times).to eq({ plain_post.id => t1, + history_post.id => t2, + newer_post.id => t3 }) + expect(history_post.reload.updated_at).to eq(t0) + end + end + + it 'filters inclusively by the latest version time after a tag is deleted' do + get '/posts', params: { url: '/version-time/', + updated_from: t2.iso8601, + updated_to: t2.iso8601 } + + expect(response).to have_http_status(:ok) + expect(json.fetch('posts').map { |item| item.fetch('id') }).to eq([history_post.id]) + expect(json.fetch('count')).to eq(1) + end + end + context 'when original_created_from/original_created_to are provided' do # 注意: controller の現状ロジックに合わせてる # original_created_from は `original_created_before > ?` @@ -1164,9 +1252,7 @@ RSpec.describe 'Posts API', type: :request do context 'when nico tag already exists in tags' do before do - Tag.find_undiscard_or_create_by!( - tag_name: TagName.find_undiscard_or_create_by!(name: 'nico:nico_tag'), - category: :nico) + Tag.find_or_create_by_tag_name!('nico:nico_tag', category: :nico) end it 'returns 422 with tag field errors' do @@ -1419,9 +1505,11 @@ RSpec.describe 'Posts API', type: :request do it '200 and updates title + resync tags when member' do sign_in_as(member) + create(:post_tag_section, post: post_record, tag:, + begin_ms: 1000, end_ms: 2000) tn2 = TagName.create!(name: 'spec_tag_2') - Tag.create!(tag_name: tn2, category: :general) + replacement_tag = Tag.create!(tag_name: tn2, category: :general) put "/posts/#{post_record.id}", params: post_update_params( post_record, @@ -1434,6 +1522,38 @@ RSpec.describe 'Posts API', type: :request do names = json['tags'].map { |n| n['name'] } expect(names).to include('spec_tag_2') + expect(names).not_to include('spec_tag') + expect(PostTag.exists?(post: post_record, tag:)).to be(false) + expect(PostTagSection.exists?(post: post_record, tag:)).to be(false) + expect(tag.reload.post_count).to eq(0) + expect(replacement_tag.reload.post_count).to eq(1) + + versions = post_record.post_versions.order(:version_no) + expect(versions.first.tags_json).to include( + a_hash_including('id' => tag.id, + 'sections' => [{ 'begin_ms' => 1000, 'end_ms' => 2000 }])) + expect(versions.last.tags_json.map { |item| item.fetch('id') }) + .not_to include(tag.id) + end + + it 'can add a removed tag again and records both changes' do + sign_in_as(member) + + put "/posts/#{ post_record.id }", params: post_update_params(post_record, tags: '') + expect(response).to have_http_status(:ok) + expect(PostTag.exists?(post: post_record, tag:)).to be(false) + + put "/posts/#{ post_record.id }", params: post_update_params( + post_record, tags: 'spec_tag') + + expect(response).to have_http_status(:ok) + expect(PostTag.where(post: post_record, tag:).count).to eq(1) + expect(PostTag.find_by!(post: post_record, tag:).created_user).to eq(member) + expect(tag.reload.post_count).to eq(1) + snapshots = post_record.post_versions.order(:version_no).map do |version| + version.tags_json.map { |item| item.fetch('id') } + end + expect(snapshots.map { |ids| ids.include?(tag.id) }).to eq([true, false, true]) end it 'rejects a deprecated tag specified directly' do @@ -1458,9 +1578,7 @@ RSpec.describe 'Posts API', type: :request do context 'when nico tag already exists in tags' do before do - Tag.find_undiscard_or_create_by!( - tag_name: TagName.find_undiscard_or_create_by!(name: 'nico:nico_tag'), - category: :nico) + Tag.find_or_create_by_tag_name!('nico:nico_tag', category: :nico) end it 'returns 422 with tag field errors' do @@ -1879,123 +1997,29 @@ RSpec.describe 'Posts API', type: :request do expect(response).to have_http_status(:not_found) end - it '200 and returns viewed boolean' do + it 'returns viewed state and current tags with their sections' do + create(:post_tag_section, post: post_record, tag:, + begin_ms: 1000, end_ms: nil) + deprecated_tag = create(:tag, deprecated_at: Time.current) + create(:post_tag, post: post_record, tag: deprecated_tag) + get '/posts/random' + expect(response).to have_http_status(:ok) expect(json).to have_key('viewed') expect([true, false]).to include(json['viewed']) + expect(json.fetch('tags')).to contain_exactly( + a_hash_including('id' => tag.id, + 'children' => [], + 'sections' => [{ 'begin_ms' => 1000, 'end_ms' => nil }])) end end describe 'GET /posts/changes' do - let(:member) { create(:user, :member) } + it 'returns 404 for the retired history endpoint' do + get '/posts/changes' - it 'returns add/remove events (history) for a post' do - # add - tn2 = TagName.create!(name: 'spec_tag2') - tag2 = Tag.create!(tag_name: tn2, category: :general) - pt = PostTag.create!(post: post_record, tag: tag2, created_user: member) - - # remove (discard) - pt.discard_by!(member) - - get '/posts/changes', params: { id: post_record.id } - - expect(response).to have_http_status(:ok) - expect(json).to include('changes', 'count') - expect(json['changes']).to be_an(Array) - expect(json['count']).to be >= 2 - - types = json['changes'].map { |e| e['change_type'] }.uniq - expect(types).to include('add') - expect(types).to include('remove') - end - - it 'filters history by tag' do - tn2 = TagName.create!(name: 'history_tag_hit') - tag2 = Tag.create!(tag_name: tn2, category: :general) - - tn3 = TagName.create!(name: 'history_tag_miss') - tag3 = Tag.create!(tag_name: tn3, category: :general) - - other_post = Post.create!( - title: 'other post', - url: 'https://example.com/history-other' - ) - - # hit: add - PostTag.create!(post: post_record, tag: tag2, created_user: member) - - # hit: add + remove - pt2 = PostTag.create!(post: other_post, tag: tag2, created_user: member) - pt2.discard_by!(member) - - # miss: add + remove - pt3 = PostTag.create!(post: post_record, tag: tag3, created_user: member) - pt3.discard_by!(member) - - get '/posts/changes', params: { tag: tag2.id } - - expect(response).to have_http_status(:ok) - expect(json).to include('changes', 'count') - expect(json['count']).to eq(3) - - changes = json.fetch('changes') - - expect(changes.map { |e| e.dig('tag', 'id') }.uniq).to eq([tag2.id]) - expect(changes.map { |e| e['change_type'] }).to match_array(%w[add add remove]) - expect(changes.map { |e| e.dig('post', 'id') }).to match_array([ - post_record.id, - other_post.id, - other_post.id - ]) - end - - it 'filters history by post and tag together' do - tn2 = TagName.create!(name: 'history_tag_combo_hit') - tag2 = Tag.create!(tag_name: tn2, category: :general) - - tn3 = TagName.create!(name: 'history_tag_combo_miss') - tag3 = Tag.create!(tag_name: tn3, category: :general) - - other_post = Post.create!( - title: 'other combo post', - url: 'https://example.com/history-combo-other' - ) - - # hit - PostTag.create!(post: post_record, tag: tag2, created_user: member) - - # miss by post - pt2 = PostTag.create!(post: other_post, tag: tag2, created_user: member) - pt2.discard_by!(member) - - # miss by tag - pt3 = PostTag.create!(post: post_record, tag: tag3, created_user: member) - pt3.discard_by!(member) - - get '/posts/changes', params: { id: post_record.id, tag: tag2.id } - - expect(response).to have_http_status(:ok) - expect(json).to include('changes', 'count') - expect(json['count']).to eq(1) - - changes = json.fetch('changes') - expect(changes.size).to eq(1) - expect(changes[0]['change_type']).to eq('add') - expect(changes[0].dig('post', 'id')).to eq(post_record.id) - expect(changes[0].dig('tag', 'id')).to eq(tag2.id) - end - - it 'returns empty history when tag does not match' do - tn2 = TagName.create!(name: 'history_tag_no_hit') - tag2 = Tag.create!(tag_name: tn2, category: :general) - - get '/posts/changes', params: { tag: tag2.id } - - expect(response).to have_http_status(:ok) - expect(json.fetch('changes')).to eq([]) - expect(json.fetch('count')).to eq(0) + expect(response).to have_http_status(:not_found) end end @@ -2047,7 +2071,7 @@ RSpec.describe 'Posts API', type: :request do end let!(:v2) do - post_record.post_tags.kept.find_by!(tag: tag).discard_by!(member) + post_record.post_tags.find_by!(tag: tag).destroy! PostTag.create!(post: post_record, tag: tag2, created_user: member) post_record.update!( title: 'updated spec post', diff --git a/backend/spec/services/youtube/sync_spec.rb b/backend/spec/services/youtube/sync_spec.rb index 9e8f2c0..c24c6a1 100644 --- a/backend/spec/services/youtube/sync_spec.rb +++ b/backend/spec/services/youtube/sync_spec.rb @@ -268,6 +268,10 @@ RSpec.describe Youtube::Sync do expect(tag_ids).to include(deerjikist_tag.id) expect(tag_ids).not_to include(Tag.no_deerjikist.id) + expect(PostTag.exists?(post:, tag: Tag.no_deerjikist)).to be(false) + expect(Tag.no_deerjikist.reload.post_count).to eq(0) + expect(deerjikist_tag.reload.post_count).to eq(1) + expect(PostVersionRecorder).to have_received(:ensure_snapshot!).with( post, created_by_user: nil diff --git a/backend/spec/tasks/nico_sync_spec.rb b/backend/spec/tasks/nico_sync_spec.rb index 04833e5..1ec5b08 100644 --- a/backend/spec/tasks/nico_sync_spec.rb +++ b/backend/spec/tasks/nico_sync_spec.rb @@ -8,8 +8,7 @@ RSpec.describe 'nico:sync' do end def create_tag!(name, category:) - tn = TagName.find_undiscard_or_create_by!(name: name.to_s.strip) - Tag.find_undiscard_or_create_by!(tag_name_id: tn.id) { |t| t.category = category } + Tag.find_or_create_by_tag_name!(name, category:) end def link_nico_to_tag!(nico_tag, tag) @@ -108,7 +107,7 @@ RSpec.describe 'nico:sync' do expect(calls).to eq(2) end - it '既存 post にあった古い nico tag は active から外され、履歴として discard される' do + it '古い nico tag の関連を物理削除し、変更前後の履歴を version に残す' do post = Post.create!( title: 'old', url: 'https://www.nicovideo.jp/watch/sm9', @@ -117,8 +116,8 @@ RSpec.describe 'nico:sync' do # 旧nicoタグ(今回の同期結果に含まれない) old_nico = create_tag!('nico:OLD', category: 'nico') - old_pt = PostTag.create!(post: post, tag: old_nico) - expect(old_pt.discarded_at).to be_nil + PostTag.create!(post:, tag: old_nico) + create_post_version_for!(post) # 今回は NEW のみ欲しい new_nico = create_tag!('nico:NEW', category: 'nico') @@ -132,9 +131,17 @@ RSpec.describe 'nico:sync' do run_rake_task('nico:sync') - # OLD は active から外れる(discarded_at が入る) - old_pts = PostTag.where(post_id: post.id, tag_id: old_nico.id).order(:id).to_a - expect(old_pts.last.discarded_at).to be_present + expect(PostTag.exists?(post:, tag: old_nico)).to be(false) + expect(old_nico.reload.post_count).to eq(0) + expect(new_nico.reload.post_count).to eq(1) + + versions = post.post_versions.order(:version_no) + expect(versions.first.tags_json.map { |item| item.fetch('id') }) + .to include(old_nico.id) + expect(versions.last.tags_json.map { |item| item.fetch('id') }) + .to include(new_nico.id) + expect(versions.last.tags_json.map { |item| item.fetch('id') }) + .not_to include(old_nico.id) # NEW は active にいる post.reload