From 9762b4a75cf7e8d48c04390f5515ed76223751bc Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 27 Feb 2026 10:56:08 -0500 Subject: [PATCH] Use `normalizes` for tag name attribute (#37119) --- app/controllers/api/v1/tags_controller.rb | 2 +- app/models/featured_tag.rb | 2 +- app/models/tag.rb | 19 ++++------ app/services/tag_search_service.rb | 2 +- spec/models/tag_spec.rb | 46 +++++++++++++++-------- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/app/controllers/api/v1/tags_controller.rb b/app/controllers/api/v1/tags_controller.rb index 67a4d8ef492..36822d831b1 100644 --- a/app/controllers/api/v1/tags_controller.rb +++ b/app/controllers/api/v1/tags_controller.rb @@ -39,6 +39,6 @@ class Api::V1::TagsController < Api::BaseController def set_or_create_tag return not_found unless Tag::HASHTAG_NAME_RE.match?(params[:id]) - @tag = Tag.find_normalized(params[:id]) || Tag.new(name: Tag.normalize(params[:id]), display_name: params[:id]) + @tag = Tag.find_normalized(params[:id]) || Tag.new(name: params[:id], display_name: params[:id]) end end diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index 9a91ab3bed7..a0938d6c0a4 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -26,7 +26,7 @@ class FeaturedTag < ApplicationRecord normalizes :name, with: ->(name) { name.strip.delete_prefix('#') } - scope :by_name, ->(name) { joins(:tag).where(tag: { name: HashtagNormalizer.new.normalize(name) }) } + scope :by_name, ->(name) { joins(:tag).where(tag: { name: }) } before_validation :set_tag diff --git a/app/models/tag.rb b/app/models/tag.rb index b87fbc42463..97edda879e5 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -65,8 +65,9 @@ class Tag < ApplicationRecord .where(statuses: { id: account.statuses.select(:id).limit(RECENT_STATUS_LIMIT) }) .group(:id).order(Arel.star.count.desc) } - scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } # Search with case-sensitive to use B-tree index + scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(normalize_value_for(:name, term))}%"), nil, true)) } # Search with case-sensitive to use B-tree index + normalizes :name, with: ->(value) { HashtagNormalizer.new.normalize(value) } normalizes :display_name, with: ->(value) { value.gsub(HASHTAG_INVALID_CHARS_RE, '') } update_index('tags', :self) @@ -111,13 +112,13 @@ class Tag < ApplicationRecord class << self def find_or_create_by_names(name_or_names) - names = Array(name_or_names).map { |str| [normalize(str), str] }.uniq(&:first) + names = Array(name_or_names).map { |str| [normalize_value_for(:name, str), str] }.uniq(&:first) - names.map do |(normalized_name, display_name)| + names.map do |name, display_name| tag = begin - matching_name(normalized_name).first || create!(name: normalized_name, display_name:) + matching_name(name).first || create!(name:, display_name:) rescue ActiveRecord::RecordNotUnique - find_normalized(normalized_name) + find_normalized(name) end yield tag if block_given? @@ -148,7 +149,7 @@ class Tag < ApplicationRecord end def matching_name(name_or_names) - names = Array(name_or_names).map { |name| arel_table.lower(normalize(name)) } + names = Array(name_or_names).map { |name| arel_table.lower(normalize_value_for(:name, name)) } if names.size == 1 where(arel_table[:name].lower.eq(names.first)) @@ -156,10 +157,6 @@ class Tag < ApplicationRecord where(arel_table[:name].lower.in(names)) end end - - def normalize(str) - HashtagNormalizer.new.normalize(str) - end end private @@ -173,6 +170,6 @@ class Tag < ApplicationRecord end def display_name_matches_name? - HashtagNormalizer.new.normalize(display_name).casecmp(name).zero? + self.class.normalize_value_for(:name, display_name).casecmp(name).zero? end end diff --git a/app/services/tag_search_service.rb b/app/services/tag_search_service.rb index 6a4af5c9a0a..1557c07bf0b 100644 --- a/app/services/tag_search_service.rb +++ b/app/services/tag_search_service.rb @@ -39,7 +39,7 @@ class TagSearchService < BaseService def ensure_exact_match(results) return results unless @offset.nil? || @offset.zero? - normalized_query = Tag.normalize(@query) + normalized_query = Tag.normalize_value_for(:name, @query) exact_match = results.find { |tag| tag.name.downcase == normalized_query } exact_match ||= Tag.find_normalized(normalized_query) unless exact_match.nil? diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index a6406a69a72..9fe723b3ba6 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -40,26 +40,40 @@ RSpec.describe Tag do I18n.t('tags.does_not_match_previous_name') end - it 'invalid with #' do - expect(described_class.new(name: '#hello_world')).to_not be_valid + describe 'when skipping normalizations' do + subject { described_class.new } + + before { subject.attributes[:name] = name } + + context 'with a # in string' do + let(:name) { '#hello_world' } + + it { is_expected.to_not be_valid } + end + + context 'with a . in string' do + let(:name) { '.abcdef123' } + + it { is_expected.to_not be_valid } + end + + context 'with a space in string' do + let(:name) { 'hello world' } + + it { is_expected.to_not be_valid } + end end - it 'invalid with .' do - expect(described_class.new(name: '.abcdef123')).to_not be_valid - end - - it 'invalid with spaces' do - expect(described_class.new(name: 'hello world')).to_not be_valid - end - - it 'valid with ａｅｓｔｈｅｔｉｃ' do - expect(described_class.new(name: 'ａｅｓｔｈｅｔｉｃ')).to be_valid - end + it { is_expected.to allow_value('ａｅｓｔｈｅｔｉｃ').for(:name) } end describe 'Normalizations' do it { is_expected.to normalize(:display_name).from('#HelloWorld').to('HelloWorld') } it { is_expected.to normalize(:display_name).from('Hello❤️World').to('HelloWorld') } + + it { is_expected.to normalize(:name).from('#hello_world').to('hello_world') } + it { is_expected.to normalize(:name).from('hello world').to('helloworld') } + it { is_expected.to normalize(:name).from('.abcdef123').to('abcdef123') } end describe 'HASHTAG_RE' do @@ -210,7 +224,7 @@ RSpec.describe Tag do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' downcase_string = 'abcabcａｂｃａｂｃやゆよ' - tag = Fabricate(:tag, name: HashtagNormalizer.new.normalize(downcase_string)) + tag = Fabricate(:tag, name: downcase_string) expect(described_class.find_normalized(upcase_string)).to eq tag end end @@ -239,7 +253,7 @@ RSpec.describe Tag do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' downcase_string = 'abcabcａｂｃａｂｃやゆよ' - tag = Fabricate(:tag, name: HashtagNormalizer.new.normalize(downcase_string)) + tag = Fabricate(:tag, name: downcase_string) expect(described_class.matches_name(upcase_string)).to eq [tag] end @@ -254,7 +268,7 @@ RSpec.describe Tag do upcase_string = 'abcABCａｂｃＡＢＣやゆよ' downcase_string = 'abcabcａｂｃａｂｃやゆよ' - tag = Fabricate(:tag, name: HashtagNormalizer.new.normalize(downcase_string)) + tag = Fabricate(:tag, name: downcase_string) expect(described_class.matching_name(upcase_string)).to eq [tag] end end