diff --git a/backend/app/controllers/nico_tags_controller.rb b/backend/app/controllers/nico_tags_controller.rb index 68c35f5..ca9a5b1 100644 --- a/backend/app/controllers/nico_tags_controller.rb +++ b/backend/app/controllers/nico_tags_controller.rb @@ -37,7 +37,7 @@ class NicoTagsController < ApplicationController if link_status.in?(['linked', 'unlinked']) exists_sql = 'EXISTS (SELECT 1 FROM nico_tag_relations ' \ - 'WHERE nico_tag_relations.nico_tag_id = tags.id)' + 'WHERE nico_tag_relations.nico_tag_id = external_tags.id)' q = link_status == 'linked' ? q.where(exists_sql) : q.where("NOT #{ exists_sql }") end @@ -45,21 +45,21 @@ class NicoTagsController < ApplicationController sort_sql = case order[0] when 'name' - 'tag_names.name' + 'external_tags.name' when 'updated_at' 'post_tag_max.max_created_at' else - "tags.#{ order[0] }" + "external_tags.#{ order[0] }" end - tags = q.reselect('tags.*', + tags = q.reselect('external_tags.*', Arel.sql('post_tag_max.max_created_at AS recent_post_tag_created_at')) - .order(Arel.sql("#{ sort_sql } #{ order[1] }, tags.id #{ order[1] }")) + .order(Arel.sql("#{ sort_sql } #{ order[1] }, external_tags.id #{ order[1] }")) .limit(limit) .offset((page - 1) * limit) .to_a render json: { tags: tags.map { |tag| - TagRepr.base(tag).merge( + external_tag_json(tag).merge( recent_post_tag_created_at: tag.recent_post_tag_created_at, linked_tags: tag.linked_tags.map { |lt| TagRepr.base(lt) }) }, count: } @@ -85,7 +85,9 @@ class NicoTagsController < ApplicationController tag.linked_tags = linked_tags tag.save! - NicoTagVersionRecorder.record!(tag:, event_type: :update, created_by_user: current_user) + NicoTagVersionRecorder.record!(external_tag: tag, + event_type: :update, + created_by_user: current_user) end render json: tag.linked_tags.map { |t| TagRepr.base(t) }, status: :ok @@ -97,6 +99,18 @@ class NicoTagsController < ApplicationController private + def external_tag_json tag + { id: tag.id, + name: "#{ tag.platform }:#{ tag.name }", + category: 'nico', + post_count: tag.post_count, + created_at: tag.created_at, + updated_at: tag.created_at, + deprecated_at: nil, + aliases: [], + parents: [] } + end + def render_nico_tag_form_record_invalid record if record.is_a?(TagName) || record.is_a?(Tag) render_validation_error fields: { tags: record.errors.full_messages.map { |message| diff --git a/backend/app/controllers/post_versions_controller.rb b/backend/app/controllers/post_versions_controller.rb index d337fe1..5d8d657 100644 --- a/backend/app/controllers/post_versions_controller.rb +++ b/backend/app/controllers/post_versions_controller.rb @@ -43,50 +43,19 @@ class PostVersionsController < ApplicationController private def serialise_versions rows + rows = rows.to_a user_ids = rows.map(&:created_by_user_id).compact.uniq users_by_id = User.where(id: user_ids).pluck(:id, :name).to_h + snapshots = rows.flat_map { |row| + [normalise_json(row.tags_json), + normalise_json(row.attributes['prev_tags_json']) || []] + } + external_tag_names = external_tag_names_for(snapshots) rows.map do |row| - cur_tags = - normalise_json(row.tags_json) - .sort_by { [(case _1.fetch('category') - when 'deerjikist' - 0 - when 'meme' - 1 - when 'character' - 2 - when 'general' - 3 - when 'material' - 4 - when 'meta' - 5 - else - 6 - end), - _1.fetch('name').downcase] } - .map { Post.tag_snapshot_literal(_1) } - prev_tags = - (normalise_json(row.attributes['prev_tags_json']) || []) - .sort_by { [(case _1.fetch('category') - when 'deerjikist' - 0 - when 'meme' - 1 - when 'character' - 2 - when 'general' - 3 - when 'material' - 4 - when 'meta' - 5 - else - 6 - end), - _1.fetch('name').downcase] } - .map { Post.tag_snapshot_literal(_1) } + cur_tags = snapshot_tag_literals(normalise_json(row.tags_json), external_tag_names) + prev_tags = snapshot_tag_literals( + normalise_json(row.attributes['prev_tags_json']) || [], external_tag_names) { post_id: row.post_id, version_no: row.version_no, @@ -113,6 +82,40 @@ class PostVersionsController < ApplicationController end end + def external_tag_names_for snapshots + ids = snapshots.flatten.filter_map { _1['external_tag_id'] }.uniq + names = ExternalTag.where(id: ids).pluck(:id, :platform, :name).to_h { |id, platform, name| + [id, "#{ platform }:#{ name }"] + } + missing_ids = ids - names.keys + + NicoTagVersion + .where(tag_id: missing_ids) + .order(:tag_id, version_no: :desc) + .pluck(:tag_id, :name) + .each { |id, name| names[id] ||= name } + + names + end + + def snapshot_tag_literals snapshots, external_tag_names + snapshots + .filter_map { |snapshot| + if snapshot.key?('tag_id') + [tag_category_order(snapshot['category']), + Post.tag_snapshot_literal(snapshot)] + elsif external_tag_names[snapshot['external_tag_id']] + [6, external_tag_names[snapshot['external_tag_id']]] + end + } + .sort_by { |order, name| [order, name.downcase] } + .map(&:second) + end + + def tag_category_order category + ['deerjikist', 'meme', 'character', 'general', 'material', 'meta'].index(category) || 6 + end + def build_version_tags cur_tags, prev_tags (cur_tags | prev_tags).map do |name| type = diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index 0239644..daa68b2 100644 --- a/backend/app/controllers/posts_controller.rb +++ b/backend/app/controllers/posts_controller.rb @@ -653,7 +653,7 @@ class PostsController < ApplicationController def editable_tag_names_from_version version version.tags_json - .reject { _1.fetch('category') == 'nico' } + .select { _1.key?('tag_id') } .map { Post.tag_snapshot_literal(_1) } .sort end diff --git a/backend/app/models/nico_tag_relation.rb b/backend/app/models/nico_tag_relation.rb index ccaa4d2..443937f 100644 --- a/backend/app/models/nico_tag_relation.rb +++ b/backend/app/models/nico_tag_relation.rb @@ -7,21 +7,4 @@ class NicoTagRelation < ApplicationRecord validates :nico_tag_id, presence: true validates :tag_id, presence: true - - validate :nico_tag_must_be_nico - validate :tag_mustnt_be_nico - - private - - def nico_tag_must_be_nico - if nico_tag && nico_tag.category != 'nico' - errors.add :nico_tag_id, 'タグのカテゴリがニコニコである必要があります.' - end - end - - def tag_mustnt_be_nico - if tag && tag.category == 'nico' - errors.add :tag_id, '連携先タグのカテゴリはニコニコであってはなりません.' - end - end end diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index f10eb07..805e91d 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -92,7 +92,7 @@ class Post < ApplicationRecord inverse_of: :parent_post has_many :children, through: :child_post_implications, source: :post - has_many :post_external_tags, dependent: :delete_all + has_many :post_external_tags, dependent: :destroy has_many :external_tags, through: :post_external_tags has_one_attached :thumbnail diff --git a/backend/app/models/post_external_tag.rb b/backend/app/models/post_external_tag.rb index 38cb528..1aa4d7e 100644 --- a/backend/app/models/post_external_tag.rb +++ b/backend/app/models/post_external_tag.rb @@ -1,4 +1,4 @@ class PostExternalTag < ApplicationRecord belongs_to :post - belongs_to :external_tag + belongs_to :external_tag, counter_cache: :post_count end diff --git a/backend/app/models/tag.rb b/backend/app/models/tag.rb index eabe5ba..02346d4 100644 --- a/backend/app/models/tag.rb +++ b/backend/app/models/tag.rb @@ -30,7 +30,7 @@ class Tag < ApplicationRecord has_many :reversed_nico_tag_relations, class_name: 'NicoTagRelation', - foreign_key: :nico_tag_id, + foreign_key: :tag_id, dependent: :destroy has_many :linked_nico_tags, through: :reversed_nico_tag_relations, source: :nico_tag @@ -71,8 +71,6 @@ class Tag < ApplicationRecord validate :tag_name_must_be_canonical validate :category_must_be_deerjikist_with_deerjikists - def self.nico_tags = ExternalTag.where(platform: :nico) - CATEGORY_PREFIXES = { 'general:' => :general, 'gen:' => :general, diff --git a/backend/spec/models/nico_tag_relation_spec.rb b/backend/spec/models/nico_tag_relation_spec.rb new file mode 100644 index 0000000..91012f8 --- /dev/null +++ b/backend/spec/models/nico_tag_relation_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +RSpec.describe NicoTagRelation, type: :model do + it 'accepts an ExternalTag regardless of its platform' do + id = ExternalTag.maximum(:id).to_i + 10_000 + + ExternalTag.insert_all!([ + { id:, + platform: 'spec_external', + name: 'external_name', + post_count: 0, + created_at: Time.current }]) + + external_tag = ExternalTag.find(id) + tag = create(:tag) + + expect(external_tag.platform_before_type_cast).to eq('spec_external') + expect { + described_class.create!(nico_tag: external_tag, tag:) + }.to change(described_class, :count).by(1) + expect(tag.linked_nico_tags).to contain_exactly(external_tag) + end + + it 'rejects an internal Tag through the external association type' do + expect { + described_class.new(nico_tag: create(:tag), tag: create(:tag)) + }.to raise_error(ActiveRecord::AssociationTypeMismatch) + end + + it 'rejects an ExternalTag through the internal association type' do + expect { + described_class.new(nico_tag: create(:external_tag), tag: create(:external_tag)) + }.to raise_error(ActiveRecord::AssociationTypeMismatch) + end +end diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index 163873e..d63be45 100644 --- a/backend/spec/models/tag_spec.rb +++ b/backend/spec/models/tag_spec.rb @@ -14,13 +14,6 @@ RSpec.describe Tag, type: :model do end end - it 'returns external records from the legacy nico_tags entrypoint' do - external = create(:external_tag) - create(:tag) - - expect(described_class.nico_tags.where(id: external.id)).to contain_exactly(external) - end - it 'finds external links by the internal tag id even when ids differ' do tag = create(:tag) external = create(:external_tag, id: tag.id + 10_000) diff --git a/backend/spec/requests/nico_tags_spec.rb b/backend/spec/requests/nico_tags_spec.rb index 3a70909..46faad5 100644 --- a/backend/spec/requests/nico_tags_spec.rb +++ b/backend/spec/requests/nico_tags_spec.rb @@ -3,6 +3,47 @@ require 'rails_helper' RSpec.describe 'NicoTags', type: :request do describe 'GET /tags/nico' do + it 'returns the legacy Tag-compatible external fields' do + external = create(:external_tag, name: 'legacy_external', post_count: 3) + + get '/tags/nico', params: { name: 'legacy_external' } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(1) + expect(json.fetch('tags')).to contain_exactly( + a_hash_including( + 'id' => external.id, + 'name' => 'nico:legacy_external', + 'category' => 'nico', + 'post_count' => 3, + 'created_at' => external.created_at.as_json, + 'updated_at' => external.created_at.as_json, + 'deprecated_at' => nil, + 'aliases' => [], + 'parents' => [], + 'has_wiki' => false, + 'material_id' => nil, + 'has_deerjikists' => false, + 'linked_tags' => [])) + end + + it 'lists ExternalTag records from every platform through the legacy URI' do + external = create(:external_tag, name: 'platform_contract_nico') + other_id = ExternalTag.maximum(:id) + 10_000 + # A second platform is not registered in the enum yet. + ExternalTag.insert_all!([ + { id: other_id, platform: 'spec_external', name: 'platform_contract_other', + post_count: 0, created_at: Time.current }]) + + get '/tags/nico', params: { name: 'platform_contract_' } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(2) + expect(json.fetch('tags')).to contain_exactly( + a_hash_including('id' => external.id, 'category' => 'nico'), + a_hash_including('id' => other_id, 'category' => 'nico')) + end + it 'returns paginated tags and total count' do 3.times { |i| create(:external_tag, name: "pagination_#{ i }") } diff --git a/backend/spec/requests/posts_spec.rb b/backend/spec/requests/posts_spec.rb index 71afb9a..5200bfb 100644 --- a/backend/spec/requests/posts_spec.rb +++ b/backend/spec/requests/posts_spec.rb @@ -93,6 +93,26 @@ RSpec.describe 'Posts API', type: :request do count end + def expect_external_tag_json tag_json, external_tag + external_tag.reload + + expect(tag_json).to include( + 'id' => external_tag.id, + 'name' => "#{ external_tag.platform }:#{ external_tag.name }", + 'category' => 'nico', + 'created_at' => external_tag.created_at.as_json, + 'updated_at' => external_tag.created_at.as_json, + 'deprecated_at' => nil, + 'aliases' => [], + 'parents' => [], + 'post_count' => external_tag.post_count, + 'has_wiki' => false, + 'material_id' => nil, + 'has_deerjikists' => false, + 'children' => [], + 'sections' => []) + end + let!(:tag_name) { TagName.create!(name: 'spec_tag') } let!(:tag) { Tag.create!(tag_name: tag_name, category: :general) } @@ -612,6 +632,30 @@ RSpec.describe 'Posts API', type: :request do expect(json.fetch('count')).to eq(2) end end + + it 'returns internal and external tags with colliding ids in the legacy tags array' do + external_tag = create(:external_tag, id: tag.id, name: 'post_index_external') + PostExternalTag.create!(post: hit_post, external_tag:) + + get '/posts' + + expect(response).to have_http_status(:ok) + + post_json = + json + .fetch('posts') + .find { _1.fetch('id') == hit_post.id } + + external_json = + post_json + .fetch('tags') + .find { _1['name'] == 'nico:post_index_external' } + + expect(post_json.fetch('tags')).to include( + a_hash_including('id' => tag.id, 'name' => tag.name, 'category' => 'general')) + expect(external_json).not_to be_nil + expect_external_tag_json(external_json, external_tag) + end end describe 'GET /posts/:id' do @@ -769,6 +813,25 @@ RSpec.describe 'Posts API', type: :request do expect(response).to have_http_status(:ok) expect(query_count).to be <= 45 end + + it 'returns external tags as root nodes in the legacy tag tree' do + external_tag = create(:external_tag, id: tag.id, name: 'post_detail_external') + PostExternalTag.create!(post: post_record, external_tag:) + + request + + expect(response).to have_http_status(:ok) + + expect(json.fetch('tags')).to include( + a_hash_including('id' => tag.id, 'name' => tag.name, 'category' => 'general')) + external_json = + json + .fetch('tags') + .find { _1['name'] == 'nico:post_detail_external' } + + expect(external_json).not_to be_nil + expect_external_tag_json(external_json, external_tag) + end end context 'when post does not exist' do @@ -1938,6 +2001,14 @@ RSpec.describe 'Posts API', type: :request do expect(names).to include('spec_tag') expect(names).to include(Tag.no_deerjikist.name) expect(post_record.external_tags).to contain_exactly(nico_tag) + + external_json = + json + .fetch('tags') + .find { _1['name'] == "nico:#{ nico_tag.name }" } + + expect(external_json).not_to be_nil + expect_external_tag_json(external_json, nico_tag) end it 'allows non-nico tags linked from nico tags to be removed by normal post update' do @@ -2180,18 +2251,71 @@ RSpec.describe 'Posts API', type: :request do expect(first.fetch('created_at')).to eq(t_v1.iso8601) end - it 'does not treat an external id as the internal tag filter' do - external_post = create(:post) - external = create(:external_tag, id: tag.id) - PostExternalTag.create!(post: external_post, external_tag: external) - PostVersionRecorder.record!(post: external_post, - event_type: :create, created_by_user: member) + context 'with external tag history' do + let(:external_id) { tag.id } + let(:external) { create(:external_tag, id: external_id) } + let(:external_post) { create(:post) } - get '/posts/versions', params: { post: external_post.id, tag: tag.id } + before do + PostExternalTag.create!(post: external_post, external_tag: external) + PostVersionRecorder.record!( + post: external_post, event_type: :create, created_by_user: member) - expect(response).to have_http_status(:ok) - expect(json.fetch('versions')).to be_empty - expect(json.fetch('count')).to eq(0) + unrelated_post = create(:post) + PostExternalTag.create!(post: unrelated_post, external_tag: create(:external_tag)) + PostVersionRecorder.record!( + post: unrelated_post, event_type: :create, created_by_user: member) + end + + it 'prefers Tag over ExternalTag for the legacy tag parameter' do + get '/posts/versions', params: { tag: tag.id } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(3) + expect(json.fetch('versions').map { [_1.fetch('post_id'), _1.fetch('version_no')] }) + .to contain_exactly( + [post_record.id, 1], [post_record.id, 2], [other_post_version.post_id, 1]) + end + + it 'explicitly filters ExternalTag even when its id collides with Tag' do + get '/posts/versions', params: { external_tag: external.id } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(1) + expect(json.fetch('versions')).to contain_exactly( + a_hash_including('post_id' => external_post.id, 'version_no' => 1)) + end + + context 'without an internal Tag with the external id' do + let(:external_id) { Tag.maximum(:id).to_i + 10_000 } + + it 'falls back to ExternalTag for the legacy tag parameter' do + expect(Tag.exists?(external.id)).to be(false) + + get '/posts/versions', params: { tag: external.id } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(1) + expect(json.fetch('versions')).to contain_exactly( + a_hash_including('post_id' => external_post.id, 'version_no' => 1)) + end + + [:tag, :external_tag].each do |parameter| + it "includes external tag removal history through #{ parameter }" do + external_post.post_external_tags.destroy_all + PostVersionRecorder.record!( + post: external_post.reload, event_type: :update, created_by_user: member) + + get '/posts/versions', params: { parameter => external.id } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(2) + expect(json.fetch('versions')).to contain_exactly( + a_hash_including('post_id' => external_post.id, 'version_no' => 1), + a_hash_including('post_id' => external_post.id, 'version_no' => 2)) + end + end + end end it 'can render history containing external identifiers' do diff --git a/backend/spec/requests/tags_spec.rb b/backend/spec/requests/tags_spec.rb index 4d48d60..e4f167f 100644 --- a/backend/spec/requests/tags_spec.rb +++ b/backend/spec/requests/tags_spec.rb @@ -30,6 +30,48 @@ RSpec.describe 'Tags API', type: :request do end describe 'GET /tags' do + it 'includes legacy external JSON alongside an internal tag with the same id' do + external = create(:external_tag, id: tag.id, name: 'spec_external', post_count: 3) + + get '/tags', params: { name: 'spec_' } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(2) + expect(response_tags).to contain_exactly( + a_hash_including('id' => tag.id, 'name' => tag.name, 'category' => 'general'), + a_hash_including( + 'id' => external.id, + 'name' => 'nico:spec_external', + 'category' => 'nico', + 'post_count' => 3, + 'created_at' => external.created_at.as_json, + 'updated_at' => external.created_at.as_json, + 'deprecated_at' => nil, + 'aliases' => [], + 'parents' => [], + 'has_wiki' => false, + 'material_id' => nil, + 'has_deerjikists' => false)) + end + + it 'uses category nico to select ExternalTag records regardless of platform' do + external = create(:external_tag, name: 'platform_contract_nico') + create(:tag, name: 'platform_contract_internal') + other_id = ExternalTag.maximum(:id) + 10_000 + # A second platform is not registered in the enum yet. + ExternalTag.insert_all!([ + { id: other_id, platform: 'spec_external', name: 'platform_contract_other', + post_count: 0, created_at: Time.current }]) + + get '/tags', params: { category: 'nico', name: 'platform_contract_' } + + expect(response).to have_http_status(:ok) + expect(json.fetch('count')).to eq(2) + expect(response_tags).to contain_exactly( + a_hash_including('id' => external.id, 'category' => 'nico'), + a_hash_including('id' => other_id, 'category' => 'nico')) + end + it 'returns tags with count and metadata' do get '/tags' @@ -171,7 +213,8 @@ RSpec.describe 'Tags API', type: :request do expect(response).to have_http_status(:ok) expect(response_names).to eq([ 'cat_deerjikist', 'cat_meme', 'cat_character', - 'cat_general', 'cat_material', 'cat_meta']) + 'cat_general', 'cat_material', 'cat_meta', 'nico:cat_nico']) + expect(json.fetch('count')).to eq(7) end it 'paginates and keeps total count' do @@ -305,8 +348,11 @@ RSpec.describe 'Tags API', type: :request do .to eq(['nico:mixed_external', 'mixed_internal']) expect(json.first).to include( 'id' => external.id, 'category' => 'nico', 'post_count' => 3, - 'deprecated_at' => nil, 'matched_alias' => nil) - expect(json.first.fetch('updated_at')).to eq(json.first.fetch('created_at')) + 'created_at' => external.created_at.as_json, + 'updated_at' => external.created_at.as_json, + 'deprecated_at' => nil, 'matched_alias' => nil, + 'aliases' => [], 'parents' => [], 'has_wiki' => false, + 'material_id' => nil, 'has_deerjikists' => false) end it 'matches an external tag by its platform prefix' do @@ -341,7 +387,7 @@ RSpec.describe 'Tags API', type: :request do expect(json.map { |row| row.fetch('name') }).to eq(expected.first(20)) end - it 'excludes external and alias-only matches when nico is false' do + it 'excludes external tags but preserves internal alias matches when nico is false' do internal = Tag.create!(category: :general, name: 'switch_internal', post_count: 1) alias_target = Tag.create!(category: :general, name: 'alias_target', post_count: 1) TagName.create!(name: 'switch_alias', canonical: alias_target.tag_name) @@ -349,7 +395,11 @@ RSpec.describe 'Tags API', type: :request do get '/tags/autocomplete', params: { q: 'switch', nico: '0' } - expect(json.map { |row| row.fetch('id') }).to eq([internal.id]) + expect(response).to have_http_status(:ok) + expect(json).to contain_exactly( + a_hash_including('id' => internal.id, 'name' => internal.name), + a_hash_including('id' => alias_target.id, 'name' => alias_target.name, + 'matched_alias' => 'switch_alias')) end ['%', '_'].each do |wildcard|