From 74c5242828cae1efa4d05133289b1c72fc265e8e Mon Sep 17 00:00:00 2001 From: miteruzo Date: Sun, 1 Nov 2026 00:20:32 +0900 Subject: [PATCH] #434 --- .../app/controllers/application_controller.rb | 3 + backend/app/controllers/posts_controller.rb | 84 ++++++++----------- backend/app/services/counts_condition.rb | 19 ----- backend/app/services/counts_population.rb | 22 ++--- backend/app/services/counts_table_query.rb | 4 +- backend/app/services/post_tag_search.rb | 64 +++++--------- backend/app/services/query_state.rb | 5 +- backend/spec/requests/counts_table_spec.rb | 65 +++++++++++--- .../post_locale_filter_identity_spec.rb | 46 +++++----- .../services/counts_pivot_security_spec.rb | 1 - frontend/src/App.tsx | 6 +- frontend/src/components/TagSidebar.tsx | 8 +- .../components/counts/CountsControls.test.tsx | 13 +-- .../src/components/counts/CountsControls.tsx | 26 +----- frontend/src/lib/posts.test.ts | 18 +++- frontend/src/lib/posts.ts | 22 +---- frontend/src/lib/prefetchers.ts | 3 - frontend/src/lib/queryState.ts | 7 +- .../src/pages/CountsResourcePage.test.tsx | 13 +-- frontend/src/pages/posts/PostListPage.tsx | 2 - frontend/src/pages/posts/PostSearchPage.tsx | 2 - frontend/src/types.ts | 1 - 22 files changed, 202 insertions(+), 232 deletions(-) diff --git a/backend/app/controllers/application_controller.rb b/backend/app/controllers/application_controller.rb index b7a7b8a..3e9305a 100644 --- a/backend/app/controllers/application_controller.rb +++ b/backend/app/controllers/application_controller.rb @@ -27,6 +27,9 @@ class ApplicationController < ActionController::API private def enforce_query_contract! + if request.query_parameters.keys.any? { _1 == 'tag_refs' || _1.start_with?('tag_refs.') } + return head :bad_request + end pairs = URI.decode_www_form(request.query_string) return head :bad_request if pairs.length > 1000 || pairs.map(&:first).uniq.length != pairs.length return head :bad_request if request.query_parameters.values.any? { !_1.is_a?(String) } diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index 3ee9232..5665fa7 100644 --- a/backend/app/controllers/posts_controller.rb +++ b/backend/app/controllers/posts_controller.rb @@ -409,52 +409,40 @@ class PostsController < ApplicationController target = Locale.find_by(code:, deprecated_at: nil) return head :not_found unless target - literals = params[:tags].to_s.split - references = tag_filter_references(literals) - names = literals.each_with_index.map do |literal, index| - reference = references&.[](index) - (reference && reference['negative'] == false) ? literal : literal.sub(/\Anot:/i, '') - end - internal = references ? {} : TagName.where( - language_code: source.language_code, name: names).pluck(:name, :tag_id).to_h - canonical = references ? {} : TagName.where( - language_code: source.language_code, primary_flg: true, - tag_id: internal.values).pluck(:tag_id, :name).to_h - external_names = names.map { canonical[internal[_1]] || _1 } - external = references ? {} : ExternalTag.where( - "CONCAT(platform, ':', name) IN (?)", external_names).pluck( - Arel.sql("CONCAT(platform, ':', name)"), :id).to_h - ids = references ? references.filter_map { _1['tag_id'] } : internal.values - translated = TagName.where(language_code: target.language_code, - primary_flg: true, tag_id: ids).pluck(:tag_id, :name).to_h + tokens = params[:tags].to_s.split + literals = PostTagSearch.literals(source, tokens.join(' ')) + source_ids = PostTagSearch.references_for(source, literals) + ids = source_ids.values.flat_map(&:first).uniq + names = TagName.where(language_code: target.language_code, + primary_flg: true, tag_id: ids).pluck(:tag_id, :name).to_h + filters = literals.map do |literal| + internal, = source_ids.fetch(literal[:name]) + name = internal.empty? ? literal[:name] : names[internal.first] + return head :unprocessable_entity unless name && internal.length <= 1 - filters = literals.each_with_index.map do |literal, index| - name = names[index] - reference = if references - references[index].except('name') - else - matches = {} - matches['tag_id'] = internal[name] if internal[name] - external_id = external[external_names[index]] - matches['external_tag_id'] = external_id if external_id - matches.empty? ? { 'missing' => true } : matches - end - name = translated[reference['tag_id']] || name - negative = reference.fetch('negative', literal.downcase.start_with?('not:')) - prefix = negative ? literal[/\Anot:/i] || 'not:' : '' - reference.merge('name' => "#{ prefix }#{ name }", 'negative' => negative) + literal[:negative] ? "not:#{ name }" : name end - render json: { references: filters } + translated = PostTagSearch.literals(target, filters.join(' ')) + target_ids = PostTagSearch.references_for(target, translated) + literals.zip(translated).each do |literal, translation| + unless (source_ids.fetch(literal[:name]).map(&:sort) == + target_ids.fetch(translation[:name]).map(&:sort)) + return head :unprocessable_entity + end + end + index = 0 + filters = tokens.map do |token| + next token if token.downcase == 'type:or' + + value = filters[index] + index += 1 + value + end + render json: { tags: filters.join(' ') } end private - def tag_filter_references literals - PostTagSearch.parse_references(params.to_unsafe_h, literals) - rescue ArgumentError, JSON::ParserError, TypeError - raise ActionController::BadRequest - end - def filtered_posts locale tag_names = params[:tags].to_s.split match_type = params[:match] @@ -466,19 +454,17 @@ class PostsController < ApplicationController end def filter_posts_by_tags locale, tag_names, match_type - references = tag_filter_references(tag_names) - literals = PostTagSearch.literals(locale, tag_names.join(' '), references:) + literals = PostTagSearch.literals(locale, tag_names.join(' ')) return Post.all if literals.empty? if match_type == 'any' literals.reduce(Post.none) do |posts, literal| - posts.or(tag_literal_relation(locale, literal[:name], negative: literal[:negative], - reference: literal[:reference])) + posts.or(tag_literal_relation(locale, literal[:name], negative: literal[:negative])) end else literals.reduce(Post.all) do |posts, literal| - ids = tagged_post_ids_for(locale, literal[:name], reference: literal[:reference]) + ids = tagged_post_ids_for(locale, literal[:name]) if literal[:negative] posts.where.not(id: ids) else @@ -488,8 +474,8 @@ class PostsController < ApplicationController end end - def tag_literal_relation locale, name, negative:, reference: nil - ids = tagged_post_ids_for(locale, name, reference:) + def tag_literal_relation locale, name, negative: + ids = tagged_post_ids_for(locale, name) if negative Post.where.not(id: ids) else @@ -497,8 +483,8 @@ class PostsController < ApplicationController end end - def tagged_post_ids_for locale, name, reference: nil - internal, external = PostTagSearch.references(locale, { name:, reference: }) + def tagged_post_ids_for locale, name + internal, external = PostTagSearch.references(locale, { name: }) Post.where(id: PostTag.where(tag_id: Array(internal)).select(:post_id)) .or(Post.where(id: PostExternalTag.where(external_tag_id: Array(external)) .select(:post_id))).select(:id) diff --git a/backend/app/services/counts_condition.rb b/backend/app/services/counts_condition.rb index 0df9646..22931bf 100644 --- a/backend/app/services/counts_condition.rb +++ b/backend/app/services/counts_condition.rb @@ -161,25 +161,6 @@ class CountsCondition scope.where(predicate, *binds) end - def tag_name_match(value) - source = post_tag_source - bound = @population.history ? 'v.created_at' : @connection.quote(point) - source += ' LEFT JOIN nico_tag_versions external_snapshot ON external_snapshot.tag_id = ' + - scalar('tag_snapshot.item', 'external_tag_id', :integer) + - " AND external_snapshot.created_at <= #{ bound } AND " + - @population.access_sql(NicoTagVersion, 'external_snapshot') + - ' AND NOT EXISTS (SELECT 1 FROM nico_tag_versions newer_external ' + - 'WHERE newer_external.tag_id = external_snapshot.tag_id ' + - "AND newer_external.created_at <= #{ bound } " + - 'AND newer_external.version_no > external_snapshot.version_no)' - document = "JSON_MERGE_PRESERVE(JSON_ARRAY(#{ scalar('tag_snapshot.item', 'name') }, " + - "referenced_tag.name, CONCAT('nico:', external_snapshot.name)), " + - "COALESCE(JSON_EXTRACT(referenced_tag.names, '$**.name'), JSON_ARRAY()))" - source += ' JOIN ' + json_table(document, '$[*]', 'search_name') - @definition.model.sanitize_sql_array([ - "EXISTS (SELECT 1 FROM #{ source } WHERE JSON_UNQUOTE(search_name.item) = ?)", value]) - end - private def set_condition(scope, entry, field) diff --git a/backend/app/services/counts_population.rb b/backend/app/services/counts_population.rb index feaa257..cea8d78 100644 --- a/backend/app/services/counts_population.rb +++ b/backend/app/services/counts_population.rb @@ -210,23 +210,19 @@ class CountsPopulation raw = params['tags'] tokens = raw.split.reject { _1.downcase == 'type:or' } - refs = PostTagSearch.parse_references(params, tokens, partial: true) locale = Locale.find_by(code: params.fetch('locale', 'ja')) raise ArgumentError unless locale - literals = PostTagSearch.literals(locale, tokens.join(' '), references: refs) + literals = PostTagSearch.literals(locale, tokens.join(' ')) + identities = PostTagSearch.references_for(locale, literals) predicates = literals.map do |literal| - reference = literal[:reference] - expression = if reference - parts = { 'internal' => reference['tag_id'], - 'external' => reference['external_tag_id'] }.filter_map do |kind, id| - snapshot_membership("#{ kind }:#{ id }") if id - end - parts.empty? ? 'FALSE' : "(#{ parts.join(' OR ') })" - else - @condition ||= CountsCondition.new(self) - @condition.tag_name_match(literal[:name]) - end + # Resolve current names/aliases exactly as normal post search does. Only + # membership comes from history; snapshot names never identify a tag. + internal, external = identities.fetch(literal[:name]) + parts = { 'internal' => internal, 'external' => external }.flat_map do |kind, ids| + Array(ids).map { |id| snapshot_membership("#{ kind }:#{ id }") } + end + expression = parts.empty? ? 'FALSE' : "(#{ parts.join(' OR ') })" literal[:negative] ? "NOT #{ expression }" : expression end return if predicates.empty? diff --git a/backend/app/services/counts_table_query.rb b/backend/app/services/counts_table_query.rb index 51e646b..f2c52f6 100644 --- a/backend/app/services/counts_table_query.rb +++ b/backend/app/services/counts_table_query.rb @@ -4,7 +4,7 @@ class CountsTableQuery class TooManyColumns < ArgumentError; end KEYS = ['resource', 'definition', 'mode', 'at', 'execution_at', 'from', 'through', - 'timezone', 'rows', 'cols', 'conditions', 'drill', 'tags', 'tag_refs', 'match', + 'timezone', 'rows', 'cols', 'conditions', 'drill', 'tags', 'match', 'tag', 'kind', 'resource_id', 'user_id', 'page', 'limit', 'sort', 'locale', 'measures', 'column_order', 'condition_values', 'name_scope'].freeze attr_reader :population, :axes, :page, :limit, :mode @@ -224,7 +224,7 @@ class CountsTableQuery fields = QueryState.records(@params, 'conditions') + QueryState.records(@params, 'drill') private_fields = fields.any? { !CountsAxes::PUBLIC_FIELDS.include?(_1['field']) } - keys = ['tags', 'tag_refs', 'tag', 'kind', 'resource_id', 'user_id'] + keys = ['tags', 'tag', 'kind', 'resource_id', 'user_id'] private_parameters = keys.any? do |key| @params.any? { |name, value| (name == key || name.start_with?("#{ key }.")) && value != '' } end diff --git a/backend/app/services/post_tag_search.rb b/backend/app/services/post_tag_search.rb index ce3ae88..461400d 100644 --- a/backend/app/services/post_tag_search.rb +++ b/backend/app/services/post_tag_search.rb @@ -1,52 +1,30 @@ # Shared literal parsing and reference resolution for current and snapshot post search. class PostTagSearch - def self.parse_references(raw, literals, partial: false) - return nil unless raw.key?('tag_refs') - - references = QueryState.records(raw, 'tag_refs') - valid = references.is_a?(Array) && references.all? do |reference| - next false unless reference.is_a?(Hash) && reference['name'].is_a?(String) - - ids = [reference['tag_id'], reference['external_tag_id']].compact - ((ids.any? || reference['missing'] == true) && - ids.all? { _1.is_a?(Integer) && _1.positive? } && - (!reference.key?('missing') || (reference['missing'] == true && ids.empty?)) && - (!reference.key?('negative') || [true, false].include?(reference['negative']))) - end - raise ArgumentError unless valid - - if partial - remaining = references.dup - result = literals.map do |literal| - index = remaining.index { _1['name'] == literal } - index ? remaining.delete_at(index) : nil - end - raise ArgumentError if remaining.any? - - return result - end - references.map { _1['name'] } == literals ? references : nil - end - - def self.literals(locale, raw, references: nil) - raw.split.each_with_index.filter_map do |token, index| - next if token.downcase == 'type:or' - - reference = references&.[](index) - name = token.sub(/\Anot:/i, '') - name = TagName.canonicalise(locale, name).first unless reference || locale.nil? - negative = reference&.key?('negative') ? reference['negative'] : - token.downcase.start_with?('not:') - { name:, negative:, reference: } + def self.literals(locale, raw) + tokens = raw.split.reject { _1.downcase == 'type:or' } + names = tokens.map { _1.sub(/\Anot:/i, '') } + registered = TagName.where(language_code: locale.language_code, name: names) + .pluck(:name, :tag_id).to_h + primary = TagName.where(language_code: locale.language_code, + tag_id: registered.values, primary_flg: true).pluck(:tag_id, :name).to_h + tokens.zip(names).map do |token, name| + { name: primary[registered[name]] || name, + negative: token.downcase.start_with?('not:') } end end def self.references(locale, literal) - reference = literal[:reference] - return [reference['tag_id'], reference['external_tag_id']] if reference + references_for(locale, [literal]).fetch(literal[:name]) + end - [TagName.where(language_code: locale.language_code, name: literal[:name]) - .distinct.pluck(:tag_id), - ExternalTag.where("CONCAT(platform, ':', name) = ?", literal[:name]).pluck(:id)] + def self.references_for(locale, literals) + names = literals.map { _1[:name] }.uniq + internal = TagName.where(language_code: locale.language_code, name: names) + .pluck(:name, :tag_id).group_by(&:first) + external = names.empty? ? {} : ExternalTag.where( + "CONCAT(platform, ':', name) IN (?)", names) + .pluck(Arel.sql("CONCAT(platform, ':', name)"), :id).group_by(&:first) + names.to_h { |name| [name, [internal.fetch(name, []).map(&:last).uniq, + external.fetch(name, []).map(&:last).uniq]] } end end diff --git a/backend/app/services/query_state.rb b/backend/app/services/query_state.rb index 3d4a4f6..ce9b3de 100644 --- a/backend/app/services/query_state.rb +++ b/backend/app/services/query_state.rb @@ -4,7 +4,6 @@ class QueryState 'conditions' => ['field', 'operator', 'value', 'end', 'blank', 'input', 'values', 'grain', 'value_type'], 'condition_values' => ['condition', 'value', 'value_type'], 'drill' => ['field', 'grain', 'value', 'value_type'], - 'tag_refs' => ['name', 'tag_id', 'external_tag_id', 'negative', 'missing'], 'column_order' => ['value', 'value_type'], 'measures' => ['key', 'axis', 'value', 'value_type'] }.freeze @@ -41,11 +40,11 @@ class QueryState _root, index, field = name.split('.') record = records[index.to_i] ||= {} - if ['blank', 'negative', 'missing', 'values'].include?(field) + if ['blank', 'values'].include?(field) raise ArgumentError unless ['0', '1'].include?(value) value = value == '1' - elsif ['tag_id', 'external_tag_id', 'axis', 'condition'].include?(field) + elsif ['axis', 'condition'].include?(field) raise ArgumentError unless value.match?(/\A[0-9]{1,18}\z/) value = value.to_i diff --git a/backend/spec/requests/counts_table_spec.rb b/backend/spec/requests/counts_table_spec.rb index b847cfd..d45d67e 100644 --- a/backend/spec/requests/counts_table_spec.rb +++ b/backend/spec/requests/counts_table_spec.rb @@ -120,17 +120,61 @@ RSpec.describe 'Counts table API', type: :request do expect(versions(query(empty.except('conditions.0.values')))).to eq([1, 2, 3]) end - it 'canonicalises unreferenced tags while preserving partial stable references' do - reference = { tag_refs: '', 'tag_refs.0.name' => 'alpha', - 'tag_refs.0.tag_id' => tag.id.to_s } + it 'resolves current tag names for AND, OR and negative searches' do expect(versions(query(tags: 'alpha'))).to eq([1, 2]) - expect(versions(query(reference.merge(tags: 'alpha nico:beta')))).to eq([1]) - expect(versions(query(reference.merge(tags: 'nico:beta alpha')))).to eq([1]) - expect(versions(query(reference.merge(tags: 'alpha not:nico:beta')))).to eq([2]) - expect(versions(query(reference.merge(tags: 'alpha nico:beta', match: 'any')))).to eq([1, 2]) + expect(versions(query(tags: 'alpha nico:beta'))).to eq([1]) + expect(versions(query(tags: 'nico:beta alpha'))).to eq([1]) + expect(versions(query(tags: 'alpha not:nico:beta'))).to eq([2]) + expect(versions(query(tags: 'alpha nico:beta', match: 'any'))).to eq([1, 2]) expect(versions(query(tags: 'not:alpha'))).to eq([3]) - get '/counts/query', params: base.merge(reference).merge(tags: 'different') - expect(response).to have_http_status(:bad_request) + end + + context 'when current tag names differ from recorded snapshot names' do + before do + tag.tag_names.find_by!(language_code: 'ja', primary_flg: true) + .update!(name: 'renamed-alpha') + create(:tag_name, :alias, tag:, name: 'alpha-alias') + TagVersion.create!(tag:, version_no: 2, event_type: 'update', name: 'renamed-alpha', + category: 'general', aliases: 'alpha-alias', parent_tag_ids: '', + names: { ja: { name: 'renamed-alpha', script_code: 'Jpan', auto_generated: false, + aliases: [{ name: 'alpha-alias', script_code: 'Jpan' }] } }, + created_at: Time.utc(2024, 2, 1)) + PostVersion.create!(post: record, version_no: 4, event_type: 'update', + url: record.url, title: 'after rename', tags: '', parent_post_ids: '', + tags_json: [{ tag_id: tag.id, version_no: 2, name: 'renamed-alpha', + category: 'general', sections: [] }], + created_by_user: actor, created_at: Time.utc(2024, 2, 2)) + end + + it 'resolves current names and aliases to stable IDs in both definitions' do + { 'history' => [1, 2, 4], 'latest' => [4] }.each do |definition, expected| + ['renamed-alpha', 'alpha-alias'].each do |name| + expect(versions(query(tags: name, definition:))).to eq(expected) + end + end + result = query(tags: 'renamed-alpha') + expect(result['versions'].first['tags'].first['name']).to eq('alpha') + expect(versions(query(tags: 'renamed-alpha', definition: 'latest', + at: '2024-01-31T16:00:00Z', current: '0'))).to eq([2]) + expect(versions(query(tags: 'not:alpha-alias'))).to eq([3]) + expect(versions(query(tags: 'renamed-alpha nico:beta'))).to eq([1]) + expect(versions(query(tags: 'renamed-alpha nico:beta', match: 'any'))).to eq([1, 2, 4]) + end + + it 'does not match a historical name reassigned to another tag ID' do + create(:tag, primary_name: 'alpha') + expect(versions(query(tags: 'alpha'))).to eq([]) + expect(versions(query(tags: 'unregistered-name'))).to eq([]) + expect(versions(query(tags: 'not:unregistered-name'))).to eq([1, 2, 3, 4]) + expect(versions(query(tags: 'renamed-alpha'))).to eq([1, 2, 4]) + end + + it 'resolves external names from current registrations too' do + external.update!(name: 'renamed-beta') + expect(versions(query(tags: 'nico:renamed-beta'))).to eq([1]) + expect(versions(query(tags: 'nico:beta'))).to eq([]) + expect(versions(query(tags: 'not:nico:renamed-beta'))).to eq([2, 3, 4]) + end end it 'rejects malformed structured conditions and axes without accepting arbitrary SQL' do @@ -149,7 +193,8 @@ RSpec.describe 'Counts table API', type: :request do end it 'rejects duplicate URL keys and JSON query payloads at the HTTP boundary' do - ['resource=posts&limit=20&limit=50', 'resource=posts&headers=%5B%22Id.%22%5D', + ['resource=posts&tag_refs=', 'resource=posts&tag_refs.0.tag_id=1', + 'resource=posts&limit=20&limit=50', 'resource=posts&headers=%5B%22Id.%22%5D', 'resource=posts&limit%5B%5D=20'].each do |query_string| get "/counts/query?#{ query_string }" expect(response).to have_http_status(:bad_request) diff --git a/backend/spec/requests/post_locale_filter_identity_spec.rb b/backend/spec/requests/post_locale_filter_identity_spec.rb index 50dc16a..1b55e1e 100644 --- a/backend/spec/requests/post_locale_filter_identity_spec.rb +++ b/backend/spec/requests/post_locale_filter_identity_spec.rb @@ -11,43 +11,49 @@ RSpec.describe 'Post filter identity across locales', type: :request do let!(:other_post) { create(:post) } before do - # Legacy data can predate the current prohibition on internal nico: names. - primary_tag_name_for(tag, 'ja').update_columns(name: 'nico:shared') primary_tag_name_for(tag, 'ain').update!(name: 'シロシ', auto_generated: false) create(:tag_name, :alias, tag:, name: 'union_alias', language_code: 'ja') PostTag.create!(post: internal_post, tag:) PostExternalTag.create!(post: external_post, external_tag: external) end - ['nico:shared', 'union_alias', 'not:nico:shared'].each do |literal| + literals = ['legacy_union_source', 'union_alias', 'not:legacy_union_source', 'nico:shared'] + literals.each do |literal| it "preserves the complete match set for #{ literal }" do get '/posts', params: { locale: 'ja', tags: literal } expect(response).to have_http_status(:ok) original_ids = json.fetch('posts').map { _1.fetch('id') } - expected = literal.start_with?('not:') ? [other_post.id] : - [internal_post.id, external_post.id] + expected = if literal.start_with?('not:') + [external_post.id, other_post.id] + elsif literal == 'nico:shared' + [external_post.id] + else + [internal_post.id] + end expect(original_ids).to match_array(expected) get '/posts/tag_filters', params: { locale: 'ja', target_locale: 'ain-Kana', tags: literal } expect(response).to have_http_status(:ok) - references = json.fetch('references') - expect(references.first).to include('tag_id' => tag.id, 'external_tag_id' => external.id) - - reference_params = { tag_refs: '' } - references.each_with_index do |reference, index| - ['name', 'tag_id', 'external_tag_id', 'negative', 'missing'].each do |field| - value = reference[field] - next if value.nil? || (field == 'missing' && value == false) - - reference_params["tag_refs.#{ index }.#{ field }"] = - [true, false].include?(value) ? (value ? '1' : '0') : value.to_s - end - end - get '/posts', params: { locale: 'ain-Kana', - tags: references.map { _1.fetch('name') }.join(' ') }.merge(reference_params) + translated = json.fetch('tags') + expect(translated).to eq(literal == 'nico:shared' ? literal : + (literal.start_with?('not:') ? 'not:シロシ' : 'シロシ')) + get '/posts', params: { locale: 'ain-Kana', tags: translated } expect(response).to have_http_status(:ok) expect(json.fetch('posts').map { _1.fetch('id') }).to match_array(original_ids) end end + + it 'rejects a locale conversion that cannot preserve a legacy union' do + primary_tag_name_for(tag, 'ja').update_columns(name: 'nico:shared') + get '/posts/tag_filters', params: { + locale: 'ja', target_locale: 'ain-Kana', tags: 'nico:shared' } + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'rejects removed reference parameters instead of overriding the name' do + get '/posts', params: { locale: 'ja', tags: 'legacy_union_source', + tag_refs: '', 'tag_refs.0.tag_id' => external.id.to_s } + expect(response).to have_http_status(:bad_request) + end end diff --git a/backend/spec/services/counts_pivot_security_spec.rb b/backend/spec/services/counts_pivot_security_spec.rb index 6548e36..6c3bc74 100644 --- a/backend/spec/services/counts_pivot_security_spec.rb +++ b/backend/spec/services/counts_pivot_security_spec.rb @@ -133,7 +133,6 @@ RSpec.describe 'Counts masked pivot contracts' do it 'rejects private predicates and supplied measure coordinates before SQL labels escape' do probes = [{ 'resource_id' => PostVersion.first.post_id.to_s }, { 'tags' => 'A' }, - { 'tag_refs' => '', 'tag_refs.0.name' => 'A', 'tag_refs.0.tag_id' => tags.first.id.to_s }, { 'conditions' => '', 'conditions.0.field' => 'title', 'conditions.0.operator' => 'eq', 'conditions.0.value' => 'hidden-title-0' }, { 'drill' => '', 'drill.0.field' => 'tags', 'drill.0.value_type' => 'string', diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 4f86c30..044ffb0 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -200,9 +200,11 @@ export const pageRoutes = ( handle: { locale: 'ja', sitemap: ['static'], preload: locale => locale === 'ja' ? GekanatorPage.preload () : GekanatorUnavailablePage.preload () } }, { path: 'counts', element: , - handle: { preload: CountsResourcePage.preload, prefetch: false, robots: 'noindex' } }, + handle: { preload: CountsResourcePage.preload, prefetch: false, robots: 'noindex', + switchLocale: switchPostFilterLocale } }, { path: 'counts/posts', element: , - handle: { preload: CountsResourcePage.preload, prefetch: false, robots: 'noindex' } }, + handle: { preload: CountsResourcePage.preload, prefetch: false, robots: 'noindex', + switchLocale: switchPostFilterLocale } }, { path: 'counts/tags', element: , handle: { preload: CountsResourcePage.preload, prefetch: false, robots: 'noindex' } }, { path: 'counts/nico-tags', element: , diff --git a/frontend/src/components/TagSidebar.tsx b/frontend/src/components/TagSidebar.tsx index a061854..26f21ab 100644 --- a/frontend/src/components/TagSidebar.tsx +++ b/frontend/src/components/TagSidebar.tsx @@ -43,6 +43,12 @@ const TagSidebar: FC = ({ posts, onClick }) => { const query = new URLSearchParams (location.search) const tagsQuery = query.get ('tags') ?? '' const anyFlg = query.get ('match') === 'any' + const countsQuery = new URLSearchParams ({ tags: tagsQuery }) + for (const [key, value] of query) + { + if (key === 'match') + countsQuery.append (key, value) + } useEffect (() => { const tagsTmp: TagByCategory = { } @@ -120,7 +126,7 @@ const TagSidebar: FC = ({ posts, onClick }) => {
  • - + {label ('actions.count')}
    • diff --git a/frontend/src/components/counts/CountsControls.test.tsx b/frontend/src/components/counts/CountsControls.test.tsx index 25932d8..f2b525a 100644 --- a/frontend/src/components/counts/CountsControls.test.tsx +++ b/frontend/src/components/counts/CountsControls.test.tsx @@ -68,22 +68,17 @@ describe ('Counts filter draft', () => { expect (screen.queryByText ('pattern.input')).not.toBeInTheDocument () }) - it ('retains stable references only for surviving search tokens', () => { - const params = appliedParams ({ tags: 'alpha beta' }) - setCountState (params, 'tag_refs', encodeCountState ('tag_refs', [ - { name: 'alpha', tag_id: 7 }, { name: 'beta', external_tag_id: 8 }])) - const { execute } = renderControls (params) + it ('submits only the edited tag names', () => { + const { execute } = renderControls (appliedParams ({ tags: 'alpha beta' })) const tags = screen.getByDisplayValue ('alpha beta') fireEvent.change (tags, { target: { value: 'not:beta gamma alpha' } }) submit () - expect (readCountState (execute.mock.lastCall![0], 'tag_refs', [])).toEqual ([ - { name: 'alpha', tag_id: 7 }, { name: 'not:beta', external_tag_id: 8, negative: true }]) + expect (execute.mock.lastCall![0].get ('tags')).toBe ('not:beta gamma alpha') fireEvent.change (tags, { target: { value: 'gamma' } }) submit () - expect (readCountState (execute.mock.lastCall![0], 'tag_refs', [])).toEqual ([]) expect (execute.mock.lastCall![0].get ('tags')).toBe ('gamma') - expect (screen.queryByText (/internal:7|external:8/)).not.toBeInTheDocument () }) + }) diff --git a/frontend/src/components/counts/CountsControls.tsx b/frontend/src/components/counts/CountsControls.tsx index 02f375f..3ca3fe9 100644 --- a/frontend/src/components/counts/CountsControls.tsx +++ b/frontend/src/components/counts/CountsControls.tsx @@ -17,7 +17,7 @@ import { useClientMotion } from '@/lib/useClientMotion' import { inputClass } from '@/lib/utils' import { useUITranslation } from '@/lib/useUITranslation' -import { readCountState, setCountState, encodeCountState } from '@/lib/queryState' +import { readCountState, setCountState } from '@/lib/queryState' import type { ReactNode } from 'react' @@ -97,27 +97,6 @@ const CountsControls = ( const update = (key: string, value: string) => setDraft (previous => { const next = new URLSearchParams (previous) setCountState (next, key, value) - if (key === 'tags') - { - const tokens = value.split (/\s+/).filter (token => token - && token.toLowerCase () !== 'type:or') - const remaining = [...tokens] - const references = readCountState[]> ( - previous, 'tag_refs', []).flatMap (reference => { - const name = String (reference.name) - const exact = remaining.indexOf (name) - const index = exact >= 0 ? exact : remaining.findIndex (token => - token.replace (/^not:/i, '') === name.replace (/^not:/i, '')) - if (index < 0) return [] - const [token] = remaining.splice (index, 1) - return [token === name ? reference : { ...reference, name: token, - negative: token.toLowerCase ().startsWith ('not:') }] - }) - for (const name of [...next.keys ()]) - if (name === 'tag_refs' || name.startsWith ('tag_refs.')) next.delete (name) - if (references.length > 0) - setCountState (next, 'tag_refs', encodeCountState ('tag_refs', references)) - } if (key === 'limit') next.delete ('page') if (next.toString () === previous.toString ()) return previous if (['rows', 'cols'].includes (key) && next.get ('sort') !== '') next.delete ('sort') @@ -278,8 +257,7 @@ const CountsControls = ( aria-label={message ('remove').text} onClick={() => setDraft (previous => { const next = new URLSearchParams (previous) for (const key of [...next.keys ()]) - if (['tags', 'tag', 'kind', 'match', 'tag_refs', 'page'].includes (key) - || key.startsWith ('tag_refs.')) next.delete (key) + if (['tags', 'tag', 'kind', 'match', 'page'].includes (key)) next.delete (key) return next })}>× } diff --git a/frontend/src/lib/posts.test.ts b/frontend/src/lib/posts.test.ts index 2e84da0..32b2937 100644 --- a/frontend/src/lib/posts.test.ts +++ b/frontend/src/lib/posts.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { fetchPostChanges, fetchPosts, toggleViewedFlg, updatePost } from '@/lib/posts' +import { fetchPostChanges, fetchPosts, switchPostFilterLocale, toggleViewedFlg, updatePost } + from '@/lib/posts' import type { FetchPostsParams } from '@/types' @@ -34,6 +35,21 @@ describe ('posts API functions', () => { vi.clearAllMocks () }) + it ('switches current tag names without adding identity state to the URL', async () => { + api.apiGet.mockResolvedValueOnce ({ tags: 'シロシ not:nico:shared' }) + const path = await switchPostFilterLocale ({ + location: { pathname: '/ja/counts/posts', + search: '?tags=source+not%3Anico%3Ashared&match=any', hash: '#table' }, + params: {}, locale: 'ja', targetLocale: 'ain-Kana' }) + expect (api.apiGet).toHaveBeenCalledWith ('/posts/tag_filters', { + locale: 'ja', params: { tags: 'source not:nico:shared', target_locale: 'ain-Kana' } }) + const url = new URL (path, 'https://example.test') + expect (url.pathname).toBe ('/ain-Kana/counts/posts') + expect (Object.fromEntries (url.searchParams)).toEqual ({ + tags: 'シロシ not:nico:shared', match: 'any' }) + expect (url.hash).toBe ('#table') + }) + it ('maps post search parameters to backend snake_case names', async () => { api.apiGet.mockResolvedValueOnce ({ posts: [], count: 0 }) diff --git a/frontend/src/lib/posts.ts b/frontend/src/lib/posts.ts index abffd3d..1c24987 100644 --- a/frontend/src/lib/posts.ts +++ b/frontend/src/lib/posts.ts @@ -1,5 +1,4 @@ import { apiDelete, apiGet, apiPost, apiPut } from '@/lib/api' -import { queryRecordParams, setQueryRecords } from '@/lib/queryState' import { switchLocalePath } from '@/lib/locales' import type { LocaleCode } from '@/lib/locales' @@ -8,7 +7,7 @@ import type { FetchPostsParams, Post, PostVersion } from '@/types' export const fetchPosts = async ( - { url, title, tags, tagReferences, match, createdFrom, createdTo, updatedFrom, updatedTo, + { url, title, tags, match, createdFrom, createdTo, updatedFrom, updatedTo, originalCreatedFrom, originalCreatedTo, page, limit, order }: FetchPostsParams, locale: LocaleCode = 'ja', ): Promise<{ @@ -18,7 +17,6 @@ export const fetchPosts = async ( ...(url && { url }), ...(title && { title }), ...(tags && { tags }), - ...(tagReferences && queryRecordParams (new URLSearchParams (tagReferences), 'tag_refs')), ...(match && { match }), ...(createdFrom && { created_from: createdFrom }), ...(createdTo && { created_to: createdTo }), @@ -39,21 +37,9 @@ export const switchPostFilterLocale = async ( if (tags == null || tags.trim () === '') return switchLocalePath (location, targetLocale) - const { references } = await apiGet<{ references: { - name: string - tagId?: number - externalTagId?: number - negative?: boolean - missing?: boolean }[] }> ('/posts/tag_filters', { - locale, params: { tags, target_locale: targetLocale, - ...queryRecordParams (query, 'tag_refs') } }) - query.set ('tags', references.map (reference => reference.name).join (' ')) - setQueryRecords (query, 'tag_refs', references.map (reference => ({ - name: reference.name, - negative: reference.negative, - ...(reference.tagId != null && { tag_id: reference.tagId }), - ...(reference.externalTagId != null && { external_tag_id: reference.externalTagId }), - ...(reference.missing && { missing: true }) }))) + const result = await apiGet<{ tags: string }> ('/posts/tag_filters', { + locale, params: { tags, target_locale: targetLocale } }) + query.set ('tags', result.tags) return switchLocalePath ({ ...location, search: `?${query}` }, targetLocale) } diff --git a/frontend/src/lib/prefetchers.ts b/frontend/src/lib/prefetchers.ts index db146ac..65dd142 100644 --- a/frontend/src/lib/prefetchers.ts +++ b/frontend/src/lib/prefetchers.ts @@ -9,7 +9,6 @@ import { getHelpArticle, import { localeFromPath, localeLanguage, unlocalisedPath } from '@/lib/locales' import { fetchPost, fetchPosts, fetchPostChanges } from '@/lib/posts' import { helpKeys, postsKeys, tagsKeys, wikiKeys } from '@/lib/queryKeys' -import { queryRecordParams } from '@/lib/queryState' import { isCountsPath } from '@/lib/routeLocales' import { fetchNicoTags, fetchTag, fetchTagChanges, fetchTags } from '@/lib/tags' import { fetchWikiAutolinkCandidates, @@ -178,8 +177,6 @@ const prefetchPostsIndex: Prefetcher = async (qc, url, locale) => { const keys = { tags, match: m, page, limit, url: qURL, title, - ...(url.searchParams.has ('tag_refs') && { - tagReferences: new URLSearchParams (queryRecordParams (url.searchParams, 'tag_refs')).toString () }), originalCreatedFrom, originalCreatedTo, createdFrom, createdTo, updatedFrom, updatedTo, order } diff --git a/frontend/src/lib/queryState.ts b/frontend/src/lib/queryState.ts index b8b5878..55e95ec 100644 --- a/frontend/src/lib/queryState.ts +++ b/frontend/src/lib/queryState.ts @@ -4,7 +4,6 @@ const schemas: Record = { conditions: ['field', 'operator', 'value', 'end', 'blank', 'input', 'values', 'grain', 'value_type'], condition_values: ['condition', 'value', 'value_type'], drill: ['field', 'grain', 'value', 'value_type'], - tag_refs: ['name', 'tag_id', 'external_tag_id', 'negative', 'missing'], column_order: ['value', 'value_type'], measures: ['key', 'axis', 'value', 'value_type'] } @@ -24,8 +23,8 @@ export const queryRecords = (params: URLSearchParams, key: string): QueryRecord[ const index = Number (parts[0]) const record = indexed.get (index) ?? { } const field = parts[1] - record[field] = ['blank', 'negative', 'missing', 'values'].includes (field) - ? value === '1' : (['tag_id', 'external_tag_id', 'axis', 'condition'].includes (field) + record[field] = ['blank', 'values'].includes (field) + ? value === '1' : (['axis', 'condition'].includes (field) ? Number (value) : value) indexed.set (index, record) } @@ -59,7 +58,7 @@ export const setQueryRecords = ( params.set (`${key}.${index}.value_type`, value == null ? 'null' : typeof value) if (value == null) continue params.set (name, typeof value === 'boolean' - ? (['blank', 'negative', 'missing', 'values'].includes (field) + ? (['blank', 'values'].includes (field) ? (value ? '1' : '0') : String (value)) : String (value)) } diff --git a/frontend/src/pages/CountsResourcePage.test.tsx b/frontend/src/pages/CountsResourcePage.test.tsx index 93824b1..e79856d 100644 --- a/frontend/src/pages/CountsResourcePage.test.tsx +++ b/frontend/src/pages/CountsResourcePage.test.tsx @@ -1,4 +1,4 @@ -import { act, fireEvent, screen, waitFor } from '@testing-library/react' +import { act, fireEvent, screen, waitFor, within } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { hydrateRoot } from 'react-dom/client' import { renderToString } from 'react-dom/server' @@ -36,6 +36,9 @@ const execute = () => fireEvent.submit ( screen.getByText ('集計実行').closest ('form')!) const openConditions = () => fireEvent.click ( screen.getByText ('集計条件').closest ('button')!) +const resourceInput = () => within ( + screen.getByText ('抽出条件', { selector: 'legend' }).closest ('fieldset')!, +).getByRole ('textbox', { name: 'Id.' }) const renderPage = (extra = '') => renderWithProviders (<> , { route: `/ja/counts/posts?mode=details&timezone=UTC${extra}` }) @@ -149,7 +152,7 @@ describe ('Counts execution and URL state', () => { await waitForResult (queryClient) await waitFor (() => expect (currentParams ().get ('execution_at')).toBe (executionAt)) openConditions () - const input = screen.getByRole ('textbox', { name: 'Id.' }) + const input = resourceInput () fireEvent.change (input, { target: { value: '7' } }) fireEvent.change (screen.getByLabelText ('表示件数', { selector: 'select' }), { target: { value: '20' } }) @@ -167,7 +170,7 @@ describe ('Counts execution and URL state', () => { await waitFor (() => expect (fetchCounts).toHaveBeenCalledTimes (2)) expect (fetchCounts.mock.calls[1][1].get ('resource_id')).toBe ('7') expect (currentParams ().toString ()).toBe (oldURL) - expect (screen.getByRole ('textbox', { name: 'Id.' })).toBe (input) + expect (resourceInput ()).toBe (input) await act (async () => complete ({ ...detailResult, executionAt: '2024-03-02T00:00:00.000000Z', limit: 20 })) await waitFor (() => expect (currentParams ().get ('resource_id')).toBe ('7')) @@ -182,14 +185,14 @@ describe ('Counts execution and URL state', () => { await waitForResult (queryClient) await waitFor (() => expect (currentParams ().get ('execution_at')).toBe (executionAt)) openConditions () - fireEvent.change (screen.getByRole ('textbox', { name: 'Id.' }), { + fireEvent.change (resourceInput (), { target: { value: '99' } }) fetchCounts.mockRejectedValueOnce (new Error ('offline')) execute () await screen.findByRole ('alert') expect (fetchCounts).toHaveBeenCalledTimes (2) expect (fetchCounts.mock.calls[1][1].get ('resource_id')).toBe ('99') - expect (screen.getByRole ('textbox', { name: 'Id.' })).toHaveValue ('99') + expect (resourceInput ()).toHaveValue ('99') expect (currentParams ().has ('resource_id')).toBe (false) }) diff --git a/frontend/src/pages/posts/PostListPage.tsx b/frontend/src/pages/posts/PostListPage.tsx index a0ee1ab..3b10b11 100644 --- a/frontend/src/pages/posts/PostListPage.tsx +++ b/frontend/src/pages/posts/PostListPage.tsx @@ -1,4 +1,3 @@ -import { queryRecordParams } from '@/lib/queryState' import { useQuery } from '@tanstack/react-query' import { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { useLocation } from 'react-router-dom' @@ -62,7 +61,6 @@ const PostListPage: FC = () => { const keys = { tags: tagsKey, match, page, limit, - ...(query.has ('tag_refs') && { tagReferences: new URLSearchParams (queryRecordParams (query, 'tag_refs')).toString () }), url: '', title: '', originalCreatedFrom: '', originalCreatedTo: '', createdFrom: '', createdTo: '', updatedFrom: '', updatedTo: '', order } as const diff --git a/frontend/src/pages/posts/PostSearchPage.tsx b/frontend/src/pages/posts/PostSearchPage.tsx index 89582fe..f2b1c78 100644 --- a/frontend/src/pages/posts/PostSearchPage.tsx +++ b/frontend/src/pages/posts/PostSearchPage.tsx @@ -1,4 +1,3 @@ -import { queryRecordParams } from '@/lib/queryState' import { useQuery } from '@tanstack/react-query' import { motion } from 'framer-motion' import { useEffect, useMemo, useState } from 'react' @@ -127,7 +126,6 @@ const PostSearchPage: FC = () => { const keys: FetchPostsParams = { tags: qTags, match: qMatch, page, limit, - ...(query.has ('tag_refs') && { tagReferences: new URLSearchParams (queryRecordParams (query, 'tag_refs')).toString () }), url: qURL, title: qTitle, originalCreatedFrom: qOriginalCreatedFrom, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 86a3f8c..69cfa6f 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -20,7 +20,6 @@ export type FetchPostsParams = { url: string title: string tags: string - tagReferences?: string match: 'all' | 'any' originalCreatedFrom: string originalCreatedTo: string