From 3dcf158b56695bd7d5d5c26f776ce752029fef6d Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Mon, 21 Nov 2022 16:40:15 +1000
Subject: [PATCH] FIX: Tag ordering adjustment for new hashtag autocompletion
 (#19120)

The tag ordering was inconsistent, because we were not
passing the correct order option to DiscourseTagging.filter_allowed_tags.
The order would change based on the limit provided. Now,
we can have a consistent order which is term exact match -> topic count ->
name.
---
 app/services/tag_hashtag_data_source.rb       |  1 +
 .../hashtag_autocomplete_service_spec.rb      |  9 +++-
 spec/services/tag_hashtag_data_source.rb      | 45 +++++++++++++++++++
 3 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 spec/services/tag_hashtag_data_source.rb

diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb
index 0323298d441..dde1ccbbdb4 100644
--- a/app/services/tag_hashtag_data_source.rb
+++ b/app/services/tag_hashtag_data_source.rb
@@ -41,6 +41,7 @@ class TagHashtagDataSource
         with_context: true,
         limit: limit,
         for_input: true,
+        order_search_results: true,
       )
     TagsController
       .tag_counts_json(tags_with_counts)
diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb
index 06efda53478..75fad31ac37 100644
--- a/spec/services/hashtag_autocomplete_service_spec.rb
+++ b/spec/services/hashtag_autocomplete_service_spec.rb
@@ -169,10 +169,17 @@ RSpec.describe HashtagAutocompleteService do
       fab!(:category3) { Fabricate(:category, name: "Book Dome", slug: "book-dome") }
       fab!(:tag2) { Fabricate(:tag, name: "mid-books") }
       fab!(:tag3) { Fabricate(:tag, name: "terrible-books") }
+      fab!(:tag4) { Fabricate(:tag, name: "book") }
 
       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],
+          %w[book-club book-dome book-zone book great-books mid-books terrible-books],
+        )
+      end
+
+      it "orders correctly with lower limits" do
+        expect(subject.search("book", %w[category tag], limit: 5).map(&:ref)).to eq(
+          %w[book-club book-dome book-zone book great-books],
         )
       end
     end
diff --git a/spec/services/tag_hashtag_data_source.rb b/spec/services/tag_hashtag_data_source.rb
new file mode 100644
index 00000000000..92cd232b82a
--- /dev/null
+++ b/spec/services/tag_hashtag_data_source.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+RSpec.describe TagHashtagDataSource do
+  fab!(:tag1) { Fabricate(:tag, name: "fact") }
+  fab!(:tag2) { Fabricate(:tag, name: "factor", topic_count: 5) }
+  fab!(:tag3) { Fabricate(:tag, name: "factory", topic_count: 1) }
+  fab!(:tag4) { Fabricate(:tag, name: "factorio") }
+  fab!(:tag5) { Fabricate(:tag, name: "factz") }
+  fab!(:user) { Fabricate(:user) }
+  let(:guardian) { Guardian.new(user) }
+
+  describe "#search" do
+    it "orders tag results by exact search match, then topic count, then name" do
+      expect(described_class.search(guardian, "fact", 5).map(&:slug)).to eq(
+        %w[fact factor factory factorio factz],
+      )
+    end
+
+    it "does not get more than the limit" do
+      expect(described_class.search(guardian, "fact", 1).map(&:slug)).to eq(%w[fact])
+    end
+
+    it "does not get tags that the user does not have permission to see" do
+      Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["fact"])
+      expect(described_class.search(guardian, "fact", 5).map(&:slug)).not_to include("fact")
+    end
+
+    it "returns an array of HashtagAutocompleteService::HashtagItem" do
+      expect(described_class.search(guardian, "fact", 1).first).to be_a(
+        HashtagAutocompleteService::HashtagItem,
+      )
+    end
+
+    it "includes the topic count for the text of the tag" do
+      expect(described_class.search(guardian, "fact", 5).map(&:text)).to eq(
+        ["fact x 0", "factor x 5", "factory x 1", "factorio x 0", "factz x 0"],
+      )
+    end
+
+    it "returns nothing if tagging is not enabled" do
+      SiteSetting.tagging_enabled = false
+      expect(described_class.search(guardian, "fact", 5)).to be_empty
+    end
+  end
+end