FEATURE: Generic hashtag autocomplete sorting (#18718)

Adds sorting for the HashtagAutocompleteService to
sort the results by case-insensitive text _within_
the type sort order specified by the params. This
should fix some flaky specs as well.

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)

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 }