From e74e3af7e3e7c8c39f80e0e67ca1bd2891f04c10 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Sat, 16 Aug 2025 10:39:27 -0400 Subject: [PATCH] Simplify tag search_for method --- app/models/admin/tag_filter.rb | 2 +- app/models/tag.rb | 19 ++++++++++++------- spec/models/tag_spec.rb | 28 ++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/app/models/admin/tag_filter.rb b/app/models/admin/tag_filter.rb index 5e75757b237..a35c77bcad1 100644 --- a/app/models/admin/tag_filter.rb +++ b/app/models/admin/tag_filter.rb @@ -32,7 +32,7 @@ class Admin::TagFilter when :status status_scope(value) when :name - Tag.search_for(value.to_s.strip, params[:limit], params[:offset], exclude_unlistable: false) + Tag.search_for(value, params[:limit], params[:offset], exclude_unlistable: false) when :order order_scope(value) else diff --git a/app/models/tag.rb b/app/models/tag.rb index dff10111123..f9aac54e863 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -67,6 +67,8 @@ class Tag < ApplicationRecord } 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 :by_name_length, -> { order(Arel.sql('LENGTH(name)').asc, name: :asc) } + update_index('tags', :self) def to_param @@ -122,16 +124,19 @@ class Tag < ApplicationRecord end def search_for(term, limit = 5, offset = 0, options = {}) - stripped_term = term.strip options.reverse_merge!({ exclude_unlistable: true, exclude_unreviewed: false }) - query = Tag.matches_name(stripped_term) - query = query.merge(Tag.listable) if options[:exclude_unlistable] - query = query.merge(matching_name(stripped_term).or(reviewed)) if options[:exclude_unreviewed] + search_query(term.to_s.strip, options) + .limit(limit) + .offset(offset) + .by_name_length + end - query.order(Arel.sql('LENGTH(name)').asc, name: :asc) - .limit(limit) - .offset(offset) + def search_query(term, options) + matches_name(term).tap do |query| + query.merge!(listable) if options[:exclude_unlistable] + query.merge!(reviewed) if options[:exclude_unreviewed] + end end def find_normalized(name) diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 18378c000d2..fcea8f875fd 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -280,6 +280,23 @@ RSpec.describe Tag do expect(results).to eq [tag] end + it 'finds tag records from padded term queries' do + tag = Fabricate(:tag, name: 'MATCH') + _miss_tag = Fabricate(:tag, name: 'miss') + + results = described_class.search_for(' match ') + + expect(results) + .to contain_exactly(tag) + end + + it 'handles nil query' do + results = described_class.search_for(nil) + + expect(results) + .to be_empty + end + it 'finds the exact matching tag as the first item' do similar_tag = Fabricate(:tag, name: 'matchlater', reviewed_at: Time.now.utc) tag = Fabricate(:tag, name: 'match', reviewed_at: Time.now.utc) @@ -306,5 +323,16 @@ RSpec.describe Tag do expect(results).to eq [tag, unlisted_tag] end + + it 'excludes non reviewed tags via option' do + tag = Fabricate(:tag, name: 'match', reviewed_at: 5.days.ago) + unreviewed_tag = Fabricate(:tag, name: 'matchreviewed', reviewed_at: nil) + + results = described_class.search_for('match', 5, 0, exclude_unreviewed: true) + + expect(results) + .to include(tag) + .and not_include(unreviewed_tag) + end end end