diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb index 0e47ec76d5e..960c383ad89 100644 --- a/app/services/category_hashtag_data_source.rb +++ b/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 diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index be60056adda..bd40cb53213 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/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) diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb index 7ec1b20ba7e..caaf8d3c4b4 100644 --- a/app/services/tag_hashtag_data_source.rb +++ b/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, diff --git a/plugins/chat/lib/chat/channel_hashtag_data_source.rb b/plugins/chat/lib/chat/channel_hashtag_data_source.rb index 538e5566f50..c5ccf8eff49 100644 --- a/plugins/chat/lib/chat/channel_hashtag_data_source.rb +++ b/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 diff --git a/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb b/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb index f6f9a46e87d..87c44e90e1c 100644 --- a/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb +++ b/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 diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index 4b8912a83bc..02789fb264e 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/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 diff --git a/spec/services/tag_hashtag_data_source_spec.rb b/spec/services/tag_hashtag_data_source_spec.rb index f1475973ac8..a4a30081fda 100644 --- a/spec/services/tag_hashtag_data_source_spec.rb +++ b/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") diff --git a/spec/support/fake_bookmark_hashtag_data_source.rb b/spec/support/fake_bookmark_hashtag_data_source.rb index c6dc9fd3996..1b5d75c2df1 100644 --- a/spec/support/fake_bookmark_hashtag_data_source.rb +++ b/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