DEV: Introduce enabled? API to hashtag data sources (#22632)

We need a nice way to only return some hashtag data
sources based on various site settings. This commit
adds an enabled? method that every hashtag data source
must implement. If this returns false the data source
will not be used at all for hashtag lookups or search.

app/services/category_hashtag_data_source.rb

@@ -4,6 +4,10 @@
 # results when looking up a category slug via markdown or searching for
 # categories via the # autocomplete character.
 class CategoryHashtagDataSource
+  def self.enabled?
+    SiteSetting.enable_experimental_hashtag_autocomplete
+  end
+
   def self.icon
     "folder"
   end

app/services/hashtag_autocomplete_service.rb

@@ -15,6 +15,8 @@ class HashtagAutocompleteService
 
   attr_reader :guardian
 
+  # NOTE: This is not meant to be called directly; use `enabled_data_sources`
+  # or the individual data_source_X methods instead.
   def self.data_sources
     # Category and Tag data sources are in core and always should be
     # included for searches and lookups.
@@ -30,16 +32,20 @@ class HashtagAutocompleteService
     )
   end
 
+  def self.enabled_data_sources
+    self.data_sources.filter(&:enabled?)
+  end
+
   def self.data_source_types
-    data_sources.map(&:type)
+    self.enabled_data_sources.map(&:type)
   end
 
   def self.data_source_icon_map
-    data_sources.map { |ds| [ds.type, ds.icon] }.to_h
+    self.enabled_data_sources.map { |ds| [ds.type, ds.icon] }.to_h
   end
 
   def self.data_source_from_type(type)
-    data_sources.find { |ds| ds.type == type }
+    self.enabled_data_sources.find { |ds| ds.type == type }
   end
 
   def self.find_priorities_for_context(context)

app/services/tag_hashtag_data_source.rb

@@ -4,6 +4,10 @@
 # results when looking up a tag slug via markdown or searching for
 # tags via the # autocomplete character.
 class TagHashtagDataSource
+  def self.enabled?
+    SiteSetting.enable_experimental_hashtag_autocomplete && SiteSetting.tagging_enabled
+  end
+
   def self.icon
     "tag"
   end
@@ -33,7 +37,6 @@ class TagHashtagDataSource
   private_class_method :tag_to_hashtag_item
 
   def self.lookup(guardian, slugs)
-    return [] if !SiteSetting.tagging_enabled
     DiscourseTagging
       .filter_visible(Tag.where_name(slugs), guardian)
       .map { |tag| tag_to_hashtag_item(tag, guardian) }
@@ -45,8 +48,6 @@ class TagHashtagDataSource
     limit,
     condition = HashtagAutocompleteService.search_conditions[:contains]
   )
-    return [] if !SiteSetting.tagging_enabled
-
     tags_with_counts, _ =
       DiscourseTagging.filter_allowed_tags(
         guardian,
@@ -79,8 +80,6 @@ class TagHashtagDataSource
   end
 
   def self.search_without_term(guardian, limit)
-    return [] if !SiteSetting.tagging_enabled
-
     tags_with_counts, _ =
       DiscourseTagging.filter_allowed_tags(
         guardian,

plugins/chat/lib/chat/channel_hashtag_data_source.rb

@@ -2,6 +2,10 @@
 
 module Chat
   class ChannelHashtagDataSource
+    def self.enabled?
+      SiteSetting.enable_experimental_hashtag_autocomplete && SiteSetting.enable_public_channels
+    end
+
     def self.icon
       "comment"
     end
@@ -23,14 +27,10 @@ module Chat
     end
 
     def self.lookup(guardian, slugs)
-      if SiteSetting.enable_experimental_hashtag_autocomplete && SiteSetting.enable_public_channels
-        return [] if !guardian.can_chat?
-        Chat::ChannelFetcher
-          .secured_public_channel_slug_lookup(guardian, slugs)
-          .map { |channel| channel_to_hashtag_item(guardian, channel) }
-      else
-        []
-      end
+      return [] if !guardian.can_chat?
+      Chat::ChannelFetcher
+        .secured_public_channel_slug_lookup(guardian, slugs)
+        .map { |channel| channel_to_hashtag_item(guardian, channel) }
     end
 
     def self.search(
@@ -39,21 +39,17 @@ module Chat
       limit,
       condition = HashtagAutocompleteService.search_conditions[:contains]
     )
-      if SiteSetting.enable_experimental_hashtag_autocomplete && SiteSetting.enable_public_channels
-        return [] if !guardian.can_chat?
-        Chat::ChannelFetcher
-          .secured_public_channel_search(
-            guardian,
-            filter: term,
-            limit: limit,
-            exclude_dm_channels: true,
-            match_filter_on_starts_with:
-              condition == HashtagAutocompleteService.search_conditions[:starts_with],
-          )
-          .map { |channel| channel_to_hashtag_item(guardian, channel) }
-      else
-        []
-      end
+      return [] if !guardian.can_chat?
+      Chat::ChannelFetcher
+        .secured_public_channel_search(
+          guardian,
+          filter: term,
+          limit: limit,
+          exclude_dm_channels: true,
+          match_filter_on_starts_with:
+            condition == HashtagAutocompleteService.search_conditions[:starts_with],
+        )
+        .map { |channel| channel_to_hashtag_item(guardian, channel) }
     end
 
     def self.search_sort(search_results, _)
@@ -61,24 +57,20 @@ module Chat
     end
 
     def self.search_without_term(guardian, limit)
-      if SiteSetting.enable_experimental_hashtag_autocomplete && SiteSetting.enable_public_channels
-        return [] if !guardian.can_chat?
-        allowed_channel_ids_sql =
-          Chat::ChannelFetcher.generate_allowed_channel_ids_sql(guardian, exclude_dm_channels: true)
-        Chat::Channel
-          .joins(
-            "INNER JOIN user_chat_channel_memberships
+      return [] if !guardian.can_chat?
+      allowed_channel_ids_sql =
+        Chat::ChannelFetcher.generate_allowed_channel_ids_sql(guardian, exclude_dm_channels: true)
+      Chat::Channel
+        .joins(
+          "INNER JOIN user_chat_channel_memberships
             ON user_chat_channel_memberships.chat_channel_id = chat_channels.id
             AND user_chat_channel_memberships.user_id = #{guardian.user.id}
             AND user_chat_channel_memberships.following = true",
-          )
-          .where("chat_channels.id IN (#{allowed_channel_ids_sql})")
-          .order(messages_count: :desc)
-          .limit(limit)
-          .map { |channel| channel_to_hashtag_item(guardian, channel) }
-      else
-        []
-      end
+        )
+        .where("chat_channels.id IN (#{allowed_channel_ids_sql})")
+        .order(messages_count: :desc)
+        .limit(limit)
+        .map { |channel| channel_to_hashtag_item(guardian, channel) }
     end
   end
 end

plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb

@@ -32,6 +32,18 @@ RSpec.describe Chat::ChannelHashtagDataSource do
     Group.refresh_automatic_groups!
   end
 
+  describe "#enabled?" do
+    it "returns false if public channels are disabled" do
+      SiteSetting.enable_public_channels = false
+      expect(described_class.enabled?).to eq(false)
+    end
+
+    it "returns true if public channels are disabled" do
+      SiteSetting.enable_public_channels = true
+      expect(described_class.enabled?).to eq(true)
+    end
+  end
+
   describe "#lookup" do
     it "finds a channel by a slug" do
       result = described_class.lookup(guardian, ["random"]).first
@@ -79,11 +91,6 @@ RSpec.describe Chat::ChannelHashtagDataSource do
       Group.refresh_automatic_groups!
       expect(described_class.lookup(Guardian.new(user), ["random"])).to be_empty
     end
-
-    it "returns an empty array if public channels are disabled" do
-      SiteSetting.enable_public_channels = false
-      expect(described_class.lookup(guardian, ["random"])).to eq([])
-    end
   end
 
   describe "#search" do
@@ -149,11 +156,6 @@ RSpec.describe Chat::ChannelHashtagDataSource do
       Group.refresh_automatic_groups!
       expect(described_class.search(Guardian.new(user), "rand", 10)).to be_empty
     end
-
-    it "returns an empty array if public channels are disabled" do
-      SiteSetting.enable_public_channels = false
-      expect(described_class.search(guardian, "rand", 10)).to eq([])
-    end
   end
 
   describe "#search_without_term" do
@@ -182,11 +184,6 @@ RSpec.describe Chat::ChannelHashtagDataSource do
       )
     end
 
-    it "returns an empty array if public channels are disabled" do
-      SiteSetting.enable_public_channels = false
-      expect(described_class.search_without_term(guardian, 5)).to eq([])
-    end
-
     it "does not return channels the user does not have permission to view" do
       expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("secret")
     end

spec/services/hashtag_autocomplete_service_spec.rb

@@ -14,6 +14,14 @@ RSpec.describe HashtagAutocompleteService do
 
   after { DiscoursePluginRegistry.reset! }
 
+  describe ".enabled_data_sources" do
+    it "only returns data sources that are enabled" do
+      expect(HashtagAutocompleteService.enabled_data_sources).to eq(
+        HashtagAutocompleteService::DEFAULT_DATA_SOURCES,
+      )
+    end
+  end
+
   describe ".contexts_with_ordered_types" do
     it "returns a hash of all the registered search contexts and their types in the defined priority order" do
       expect(HashtagAutocompleteService.contexts_with_ordered_types).to eq(
@@ -463,7 +471,7 @@ RSpec.describe HashtagAutocompleteService do
         result = service.lookup(%w[the-book-club great-books fiction-books], %w[category tag])
         expect(result[:category].map(&:slug)).to eq(["the-book-club"])
         expect(result[:category].map(&:relative_url)).to eq(["/c/the-book-club/#{category1.id}"])
-        expect(result[:tag]).to eq([])
+        expect(result[:tag]).to eq(nil)
       end
     end
   end

spec/services/tag_hashtag_data_source_spec.rb

@@ -9,6 +9,18 @@ RSpec.describe TagHashtagDataSource do
   fab!(:user) { Fabricate(:user) }
   let(:guardian) { Guardian.new(user) }
 
+  describe "#enabled?" do
+    it "returns false if tagging is disabled" do
+      SiteSetting.tagging_enabled = false
+      expect(described_class.enabled?).to eq(false)
+    end
+
+    it "returns true if tagging is enabled" do
+      SiteSetting.tagging_enabled = true
+      expect(described_class.enabled?).to eq(true)
+    end
+  end
+
   describe "#search" do
     it "orders tag results by exact search match, then public topic count, then name" do
       expect(described_class.search(guardian, "fact", 5).map(&:slug)).to eq(
@@ -37,11 +49,6 @@ RSpec.describe TagHashtagDataSource do
       )
     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
-
     it "returns tags that are children of a TagGroup" do
       parent_tag = Fabricate(:tag, name: "sidebar")
       child_tag = Fabricate(:tag, name: "sidebar-v1")

spec/support/fake_bookmark_hashtag_data_source.rb

@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 class FakeBookmarkHashtagDataSource
+  def self.enabled?
+    true
+  end
+
   def self.icon
     "bookmark"
   end