diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index 3f5a92c..edbc392 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 @@ -384,50 +381,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 @@ -502,13 +455,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) @@ -717,7 +670,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/models/post.rb b/backend/app/models/post.rb index 08f8fec..4d7a851 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -55,11 +55,10 @@ 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 @@ -123,7 +122,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 +148,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 202610b..4a4598f 100644 --- a/backend/app/models/tag.rb +++ b/backend/app/models/tag.rb @@ -28,9 +28,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 @@ -259,11 +257,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 @@ -293,7 +291,7 @@ 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 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/schema.rb b/backend/db/schema.rb index 5ebbcc5..f4ca18c 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_010000) 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 @@ -311,23 +311,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 @@ -395,19 +384,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do t.index ["user_id"], name: "index_settings_on_user_id", unique: true 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 "tag_implications", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "tag_id", null: false t.bigint "parent_tag_id", null: false @@ -616,6 +592,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 @@ -724,12 +713,10 @@ ActiveRecord::Schema[8.0].define(version: 2026_07_27_123600) do 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" 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/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_spec.rb b/backend/spec/models/tag_spec.rb index ccff0be..79b0ac0 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -185,15 +185,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 link, links the target, and aliases the discarded source tag' 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(PostTag.exists?(post: post_record, tag: source_tag)).to be(false) + expect(target_link).to be_present + expect(source_tag.reload.post_count).to eq(0) expect(Tag.with_discarded.find(source_tag.id)).to be_discarded expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded @@ -206,16 +205,22 @@ 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(source_tag.reload.post_count).to eq(0) + 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 @@ -230,14 +235,12 @@ RSpec.describe Tag, type: :model 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]) - 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(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 @@ -263,13 +266,10 @@ RSpec.describe Tag, type: :model do it 'still merges, but discards the source tag_name instead of aliasing 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(target_tag.reload.post_count).to eq(1) @@ -289,14 +289,20 @@ RSpec.describe Tag, type: :model do end it 'rolls back the transaction' do + 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]) }.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(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 @@ -365,12 +371,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..350b5e7 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 > ?` @@ -1419,9 +1507,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 +1524,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 @@ -1879,123 +2001,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 +2075,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..f708608 100644 --- a/backend/spec/tasks/nico_sync_spec.rb +++ b/backend/spec/tasks/nico_sync_spec.rb @@ -108,7 +108,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 +117,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 +132,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