diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index 3d1efe2227..1280f267df 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -2,6 +2,7 @@ class HashtagAutocompleteService HASHTAGS_PER_REQUEST = 20 + SEARCH_MAX_LIMIT = 20 attr_reader :guardian cattr_reader :data_sources @@ -137,8 +138,9 @@ class HashtagAutocompleteService { categories: categories_hashtags, tags: tag_hashtags } end - def search(term, types_in_priority_order, limit = 5) + def search(term, types_in_priority_order, limit: 5) raise Discourse::InvalidParameters.new(:order) if !types_in_priority_order.is_a?(Array) + limit = [limit, SEARCH_MAX_LIMIT].min results = [] slugs_by_type = {} @@ -159,6 +161,7 @@ class HashtagAutocompleteService item.type = type item.ref = item.ref || item.slug end + data.sort_by! { |item| item.text.downcase } slugs_by_type[type] = data.map(&:slug) results.concat(data) diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index c098dd3f5e..94281c4c66 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -41,12 +41,25 @@ RSpec.describe HashtagAutocompleteService do end it "respects the limit param" do - expect(subject.search("book", %w[tag category], 1).map(&:text)).to eq(["great-books x 0"]) + expect(subject.search("book", %w[tag category], limit: 1).map(&:text)).to eq(["great-books x 0"]) + end + + it "does not allow more than SEARCH_MAX_LIMIT results to be specified by the limit param" do + stub_const(HashtagAutocompleteService, "SEARCH_MAX_LIMIT", 1) do + expect(subject.search("book", %w[category tag], limit: 1000).map(&:text)).to eq( + ["Book Club"], + ) + end + end + + it "does not search other data sources if the limit is reached by earlier type data sources" do + Site.any_instance.expects(:categories).never + subject.search("book", %w[tag category], limit: 1) end it "includes the tag count" do tag1.update!(topic_count: 78) - expect(subject.search("book", %w[tag category], 1).map(&:text)).to eq(["great-books x 78"]) + expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books x 78", "Book Club"]) end it "does case-insensitive search" do @@ -68,11 +81,6 @@ RSpec.describe HashtagAutocompleteService do expect(subject.search("book", %w[tag]).map(&:text)).to be_empty end - it "does not search other data sources if the limit is reached by earlier type data sources" do - Site.any_instance.expects(:categories).never - subject.search("book", %w[tag category], 1) - end - it "includes other data sources" do Fabricate(:bookmark, user: user, name: "read review of this fantasy book") Fabricate(:bookmark, user: user, name: "cool rock song") @@ -109,7 +117,7 @@ RSpec.describe HashtagAutocompleteService do it "appends type suffixes for the ref on conflicting slugs on items that are not the top priority type" do Fabricate(:tag, name: "book-club") expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( - %w[book-club great-books book-club::tag], + %w[book-club book-club::tag great-books], ) Fabricate(:bookmark, user: user, name: "book club") @@ -118,10 +126,23 @@ RSpec.describe HashtagAutocompleteService do register_bookmark_data_source expect(subject.search("book", %w[category tag bookmark]).map(&:ref)).to eq( - %w[book-club great-books book-club::tag book-club::bookmark], + %w[book-club book-club::tag great-books book-club::bookmark], ) end + context "when multiple tags and categories are returned" do + fab!(:category2) { Fabricate(:category, name: "Book Zone", slug: "book-zone") } + fab!(:category3) { Fabricate(:category, name: "Book Dome", slug: "book-dome") } + fab!(:tag2) { Fabricate(:tag, name: "mid-books") } + fab!(:tag3) { Fabricate(:tag, name: "terrible-books") } + + it "orders them by name within their type order" do + expect(subject.search("book", %w[category tag], limit: 10).map(&:ref)).to eq( + %w[book-club book-dome book-zone great-books mid-books terrible-books], + ) + end + end + context "when not tagging_enabled" do before { SiteSetting.tagging_enabled = false }