diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb index 4ae93a0e47f..0987b6eb0a6 100644 --- a/app/services/category_hashtag_data_source.rb +++ b/app/services/category_hashtag_data_source.rb @@ -8,6 +8,10 @@ class CategoryHashtagDataSource "folder" end + def self.type + "category" + end + def self.category_to_hashtag_item(category) HashtagAutocompleteService::HashtagItem.new.tap do |item| item.text = category.name diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index b41176bdaeb..0eceac75ce9 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -3,49 +3,61 @@ class HashtagAutocompleteService HASHTAGS_PER_REQUEST = 20 SEARCH_MAX_LIMIT = 50 + DEFAULT_DATA_SOURCES = [CategoryHashtagDataSource, TagHashtagDataSource] + DEFAULT_CONTEXTUAL_TYPE_PRIORITIES = [ + { type: "category", context: "topic-composer", priority: 100 }, + { type: "tag", context: "topic-composer", priority: 50 }, + ] def self.search_conditions @search_conditions ||= Enum.new(contains: 0, starts_with: 1) end attr_reader :guardian - cattr_reader :data_sources, :contexts - def self.register_data_source(type, klass) - @@data_sources[type] = klass + def self.data_sources + # Category and Tag data sources are in core and always should be + # included for searches and lookups. + Set.new(DEFAULT_DATA_SOURCES | DiscoursePluginRegistry.hashtag_autocomplete_data_sources) end - def self.clear_registered - @@data_sources = {} - @@contexts = {} - - register_data_source("category", CategoryHashtagDataSource) - register_data_source("tag", TagHashtagDataSource) - - register_type_in_context("category", "topic-composer", 100) - register_type_in_context("tag", "topic-composer", 50) + def self.contextual_type_priorities + # Category and Tag type priorities for the composer are default and + # always are included. + Set.new( + DEFAULT_CONTEXTUAL_TYPE_PRIORITIES | + DiscoursePluginRegistry.hashtag_autocomplete_contextual_type_priorities, + ) end - def self.register_type_in_context(type, context, priority) - @@contexts[context] = @@contexts[context] || {} - @@contexts[context][type] = priority + def self.data_source_types + data_sources.map(&:type) end def self.data_source_icons - @@data_sources.values.map(&:icon) + data_sources.map(&:icon) + end + + def self.data_source_from_type(type) + data_sources.find { |ds| ds.type == type } + end + + def self.find_priorities_for_context(context) + contextual_type_priorities.select { |ctp| ctp[:context] == context } + end + + def self.unique_contexts + contextual_type_priorities.map { |ctp| ctp[:context] }.uniq end def self.ordered_types_for_context(context) - return [] if @@contexts[context].blank? - @@contexts[context].sort_by { |param, priority| priority }.reverse.map(&:first) + find_priorities_for_context(context).sort_by { |ctp| -ctp[:priority] }.map { |ctp| ctp[:type] } end def self.contexts_with_ordered_types - Hash[@@contexts.keys.map { |context| [context, ordered_types_for_context(context)] }] + Hash[unique_contexts.map { |context| [context, ordered_types_for_context(context)] }] end - clear_registered - class HashtagItem # The text to display in the UI autocomplete menu for the item. attr_accessor :text @@ -120,13 +132,15 @@ class HashtagAutocompleteService raise Discourse::InvalidParameters.new(:order) if !types_in_priority_order.is_a?(Array) types_in_priority_order = - types_in_priority_order.select { |type| @@data_sources.keys.include?(type) } + types_in_priority_order.select do |type| + HashtagAutocompleteService.data_source_types.include?(type) + end lookup_results = Hash[types_in_priority_order.collect { |type| [type.to_sym, []] }] limited_slugs = slugs[0..HashtagAutocompleteService::HASHTAGS_PER_REQUEST] slugs_without_suffixes = limited_slugs.reject do |slug| - @@data_sources.keys.any? { |type| slug.ends_with?("::#{type}") } + HashtagAutocompleteService.data_source_types.any? { |type| slug.ends_with?("::#{type}") } end slugs_with_suffixes = (limited_slugs - slugs_without_suffixes) @@ -208,7 +222,9 @@ class HashtagAutocompleteService top_ranked_type = nil term = term.downcase types_in_priority_order = - types_in_priority_order.select { |type| @@data_sources.keys.include?(type) } + types_in_priority_order.select do |type| + HashtagAutocompleteService.data_source_types.include?(type) + end # Float exact matches by slug to the top of the list, any of these will be excluded # from further results. @@ -318,7 +334,7 @@ class HashtagAutocompleteService return limited_results if search_results.empty? search_results = - @@data_sources[type].search_sort( + HashtagAutocompleteService.data_source_from_type(type).search_sort( search_results.reject do |item| limited_results.any? { |exact| exact.type == type && exact.slug === item.slug } end, @@ -334,7 +350,12 @@ class HashtagAutocompleteService types_in_priority_order.each do |type| search_results = - filter_valid_data_items(@@data_sources[type].search_without_term(guardian, split_limit)) + filter_valid_data_items( + HashtagAutocompleteService.data_source_from_type(type).search_without_term( + guardian, + split_limit, + ), + ) next if search_results.empty? # This is purposefully unsorted as search_without_term should sort @@ -369,7 +390,17 @@ class HashtagAutocompleteService condition = HashtagAutocompleteService.search_conditions[:contains] ) filter_valid_data_items( - set_types(set_refs(@@data_sources[type].search(guardian, term, limit, condition)), type), + set_types( + set_refs( + HashtagAutocompleteService.data_source_from_type(type).search( + guardian, + term, + limit, + condition, + ), + ), + type, + ), ) end @@ -385,7 +416,10 @@ class HashtagAutocompleteService end def lookup_for_type(type, guardian, slugs) - set_types(set_refs(@@data_sources[type].lookup(guardian, slugs)), type) + set_types( + set_refs(HashtagAutocompleteService.data_source_from_type(type).lookup(guardian, slugs)), + type, + ) end def append_types_to_conflicts(limited_results, top_ranked_type, limit) diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb index a7057e95fd6..dd5c6a0af96 100644 --- a/app/services/tag_hashtag_data_source.rb +++ b/app/services/tag_hashtag_data_source.rb @@ -8,6 +8,10 @@ class TagHashtagDataSource "tag" end + def self.type + "tag" + end + def self.tag_to_hashtag_item(tag, include_count: false) tag = Tag.new(tag.slice(:id, :name, :description).merge(topic_count: tag[:count])) if tag.is_a?( Hash, diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb index 7272b4f0b76..b6bcbb98211 100644 --- a/lib/discourse_plugin_registry.rb +++ b/lib/discourse_plugin_registry.rb @@ -5,11 +5,16 @@ # class DiscoursePluginRegistry + # Plugins often need to be able to register additional handlers, data, or + # classes that will be used by core classes. This should be used if you + # need to control which type the registry is, and if it doesn't need to + # be removed if the plugin is disabled. + # # Shortcut to create new register in the plugin registry # - Register is created in a class variable using the specified name/type # - Defines singleton method to access the register # - Defines instance method as a shortcut to the singleton method - # - Automatically deletes the register on ::clear! + # - Automatically deletes the register on registry.reset! def self.define_register(register_name, type) @@register_names ||= Set.new @@register_names << register_name @@ -24,11 +29,15 @@ class DiscoursePluginRegistry end end + # Plugins often need to add values to a list, and we need to filter those + # lists at runtime to ignore values from disabled plugins. Unlike define_register, + # the type of the register cannot be defined, and is always Array. + # # Create a new register (see `define_register`) with some additions: # - Register is created in a class variable using the specified name/type # - Defines singleton method to access the register # - Defines instance method as a shortcut to the singleton method - # - Automatically deletes the register on ::clear! + # - Automatically deletes the register on registry.reset! def self.define_filtered_register(register_name) define_register(register_name, Array) @@ -100,6 +109,9 @@ class DiscoursePluginRegistry define_filtered_register :user_destroyer_on_content_deletion_callbacks + define_filtered_register :hashtag_autocomplete_data_sources + define_filtered_register :hashtag_autocomplete_contextual_type_priorities + def self.register_auth_provider(auth_provider) self.auth_providers << auth_provider end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 579c9ffd894..c0619adc791 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -1097,13 +1097,21 @@ class Plugin::Instance # Used to register data sources for HashtagAutocompleteService to look # up results based on a #hashtag string. # - # @param {String} type - Roughly corresponding to a model, this is used as a unique - # key for the datasource and is also used when allowing different - # contexts to search for and lookup these types. The `category` - # and `tag` types are registered by default. # @param {Class} klass - Must be a class that implements methods with the following # signatures: # + # Roughly corresponding to a model, this is used as a unique + # key for the datasource and is also used when allowing different + # contexts to search for and lookup these types. The `category` + # and `tag` types are registered by default. + # def self.type + # end + # + # The FontAwesome icon to use for the data source in the search results + # and cooked markdown. + # def self.icon + # end + # # @param {Guardian} guardian - Current user's guardian, used for permission-based filtering # @param {Array} slugs - An array of strings that represent slugs to search this type for, # e.g. category slugs. @@ -1117,8 +1125,20 @@ class Plugin::Instance # @returns {Array} An Array of HashtagAutocompleteService::HashtagItem # def self.search(guardian, term, limit) # end - def register_hashtag_data_source(type, klass) - HashtagAutocompleteService.register_data_source(type, klass) + # + # @param {Array} search_results - An array of HashtagAutocompleteService::HashtagItem to sort + # @param {String} term - The search term which was used, which may help with sorting. + # @returns {Array} An Array of HashtagAutocompleteService::HashtagItem + # def self.search_sort(search_results, term) + # end + # + # @param {Guardian} guardian - Current user's guardian, used for permission-based filtering + # @param {Integer} limit - The number of search results that should be returned by the query + # @returns {Array} An Array of HashtagAutocompleteService::HashtagItem + # def self.search_without_term(guardian, limit) + # end + def register_hashtag_data_source(klass) + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source(klass, self) end ## @@ -1134,8 +1154,10 @@ class Plugin::Instance # for certain types of hashtag result. # @param {Integer} priority - A number value for ordering type results when hashtag searches # or lookups occur. Priority is ordered by DESCENDING order. - def register_hashtag_type_in_context(type, context, priority) - HashtagAutocompleteService.register_type_in_context(type, context, priority) + def register_hashtag_type_priority_for_context(type, context, priority) + DiscoursePluginRegistry.register_hashtag_autocomplete_contextual_type_priority( + { type: type, context: context, priority: priority }, self + ) end ## diff --git a/plugins/chat/lib/chat_channel_hashtag_data_source.rb b/plugins/chat/lib/chat_channel_hashtag_data_source.rb index 49cc18280ce..5c4e31cd867 100644 --- a/plugins/chat/lib/chat_channel_hashtag_data_source.rb +++ b/plugins/chat/lib/chat_channel_hashtag_data_source.rb @@ -5,6 +5,10 @@ class Chat::ChatChannelHashtagDataSource "comment" end + def self.type + "channel" + end + def self.channel_to_hashtag_item(guardian, channel) HashtagAutocompleteService::HashtagItem.new.tap do |item| item.text = channel.title diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 1d3ff4bf24d..028ebcf4a84 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -730,11 +730,11 @@ after_initialize do register_about_stat_group("chat_users") { Chat::Statistics.about_users } # Make sure to update spec/system/hashtag_autocomplete_spec.rb when changing this. - register_hashtag_data_source("channel", Chat::ChatChannelHashtagDataSource) - register_hashtag_type_in_context("channel", "chat-composer", 200) - register_hashtag_type_in_context("category", "chat-composer", 100) - register_hashtag_type_in_context("tag", "chat-composer", 50) - register_hashtag_type_in_context("channel", "topic-composer", 10) + register_hashtag_data_source(Chat::ChatChannelHashtagDataSource) + register_hashtag_type_priority_for_context("channel", "chat-composer", 200) + register_hashtag_type_priority_for_context("category", "chat-composer", 100) + register_hashtag_type_priority_for_context("tag", "chat-composer", 50) + register_hashtag_type_priority_for_context("channel", "topic-composer", 10) Site.markdown_additional_options["chat"] = { limited_pretty_text_features: ChatMessage::MARKDOWN_FEATURES, diff --git a/plugins/chat/spec/models/chat_message_spec.rb b/plugins/chat/spec/models/chat_message_spec.rb index 3982f83800c..89d1920eac2 100644 --- a/plugins/chat/spec/models/chat_message_spec.rb +++ b/plugins/chat/spec/models/chat_message_spec.rb @@ -5,18 +5,6 @@ require "rails_helper" describe ChatMessage do fab!(:message) { Fabricate(:chat_message, message: "hey friend, what's up?!") } - # TODO (martin) Remove this after https://github.com/discourse/discourse/pull/19491 is merged - def register_hashtag_contexts - # This is annoying, but we need to reset the hashtag data sources inbetween - # tests, and since this is normally done in plugin.rb with the plugin API - # there is not an easier way to do this. - HashtagAutocompleteService.register_data_source("channel", Chat::ChatChannelHashtagDataSource) - HashtagAutocompleteService.register_type_in_context("channel", "chat-composer", 200) - HashtagAutocompleteService.register_type_in_context("category", "chat-composer", 100) - HashtagAutocompleteService.register_type_in_context("tag", "chat-composer", 50) - HashtagAutocompleteService.register_type_in_context("channel", "topic-composer", 10) - end - describe ".cook" do it "does not support HTML tags" do cooked = ChatMessage.cook("

test

") @@ -258,7 +246,7 @@ describe ChatMessage do end it "supports hashtag-autocomplete plugin" do - register_hashtag_contexts + SiteSetting.chat_enabled = true SiteSetting.enable_experimental_hashtag_autocomplete = true category = Fabricate(:category) @@ -521,7 +509,7 @@ describe ChatMessage do fab!(:secure_category) { Fabricate(:private_category, group: group) } before do - register_hashtag_contexts + SiteSetting.chat_enabled = true SiteSetting.enable_experimental_hashtag_autocomplete = true SiteSetting.suppress_secured_categories_from_admin = true end diff --git a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb index 318e23bfb92..522a4e10009 100644 --- a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb +++ b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb @@ -19,15 +19,6 @@ describe "Using #hashtag autocompletion to search for and lookup channels", before do SiteSetting.enable_experimental_hashtag_autocomplete = true - # This is annoying, but we need to reset the hashtag data sources inbetween - # tests, and since this is normally done in plugin.rb with the plugin API - # there is not an easier way to do this. - HashtagAutocompleteService.register_data_source("channel", Chat::ChatChannelHashtagDataSource) - HashtagAutocompleteService.register_type_in_context("channel", "chat-composer", 200) - HashtagAutocompleteService.register_type_in_context("category", "chat-composer", 100) - HashtagAutocompleteService.register_type_in_context("tag", "chat-composer", 50) - HashtagAutocompleteService.register_type_in_context("channel", "topic-composer", 10) - chat_system_bootstrap(user, [channel1, channel2]) sign_in(user) end diff --git a/spec/multisite/hashtag_autocomplete_service_spec.rb b/spec/multisite/hashtag_autocomplete_service_spec.rb new file mode 100644 index 00000000000..4352cf7f3ea --- /dev/null +++ b/spec/multisite/hashtag_autocomplete_service_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +describe "HashtagAutocompleteService multisite registry", type: :multisite do + class MockPlugin + def initialize(setting_provider) + @setting_provider = setting_provider + end + + def enabled? + @setting_provider.find("bookmark_hashtag_enabled")&.value == "true" + end + end + + it "does not include the data source if one of the multisites has the plugin disabled" do + setting_provider = SiteSettings::LocalProcessProvider.new + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source( + FakeBookmarkHashtagDataSource, + MockPlugin.new(setting_provider), + ) + + test_multisite_connection("default") do + setting_provider.save("bookmark_hashtag_enabled", true, 5) + expect(HashtagAutocompleteService.data_source_types).to eq(%w[category tag bookmark]) + end + + test_multisite_connection("second") do + expect(HashtagAutocompleteService.data_source_types).to eq(%w[category tag]) + end + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index da6d184dedd..89fda6a3c69 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -154,9 +154,6 @@ module TestSetup # Make sure the default Post and Topic bookmarkables are registered Bookmark.reset_bookmarkables - # Make sure only the default category and tag hashtag data sources are registered. - HashtagAutocompleteService.clear_registered - OmniAuth.config.test_mode = false end end diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index 59a9b70807e..d1f591b3419 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -9,64 +9,21 @@ RSpec.describe HashtagAutocompleteService do subject { described_class.new(guardian) } - before { Site.clear_cache } - - class BookmarkDataSource - def self.icon - "bookmark" - end - - def self.lookup(guardian_scoped, slugs) - guardian_scoped - .user - .bookmarks - .where("LOWER(name) IN (:slugs)", slugs: slugs) - .map do |bm| - HashtagAutocompleteService::HashtagItem.new.tap do |item| - item.text = bm.name - item.slug = bm.name.gsub(" ", "-") - item.icon = icon - end - end - end - - def self.search( - guardian_scoped, - term, - limit, - condition = HashtagAutocompleteService.search_conditions[:starts_with] - ) - query = guardian_scoped.user.bookmarks - - if condition == HashtagAutocompleteService.search_conditions[:starts_with] - query = query.where("name ILIKE ?", "#{term}%") - else - query = query.where("name ILIKE ?", "%#{term}%") - end - - query - .limit(limit) - .map do |bm| - HashtagAutocompleteService::HashtagItem.new.tap do |item| - item.text = bm.name - item.slug = bm.name.gsub(" ", "-") - item.icon = icon - end - end - end - - def self.search_sort(search_results, _) - search_results.sort_by { |item| item.text.downcase } - end - end + after { DiscoursePluginRegistry.reset! } describe ".contexts_with_ordered_types" do it "returns a hash of all the registrered search contexts and their types in the defined priority order" do expect(HashtagAutocompleteService.contexts_with_ordered_types).to eq( { "topic-composer" => %w[category tag] }, ) - HashtagAutocompleteService.register_type_in_context("category", "awesome-composer", 50) - HashtagAutocompleteService.register_type_in_context("tag", "awesome-composer", 100) + DiscoursePluginRegistry.register_hashtag_autocomplete_contextual_type_priority( + { type: "category", context: "awesome-composer", priority: 50 }, + stub(enabled?: true), + ) + DiscoursePluginRegistry.register_hashtag_autocomplete_contextual_type_priority( + { type: "tag", context: "awesome-composer", priority: 100 }, + stub(enabled?: true), + ) expect(HashtagAutocompleteService.contexts_with_ordered_types).to eq( { "topic-composer" => %w[category tag], "awesome-composer" => %w[tag category] }, ) @@ -148,7 +105,10 @@ RSpec.describe HashtagAutocompleteService do Fabricate(:bookmark, user: user, name: "cool rock song") guardian.user.reload - HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source( + FakeBookmarkHashtagDataSource, + stub(enabled?: true), + ) expect(subject.search("book", %w[category tag bookmark]).map(&:text)).to eq( ["The Book Club", "great-books x 22", "read review of this fantasy book"], @@ -173,7 +133,10 @@ RSpec.describe HashtagAutocompleteService do Fabricate(:bookmark, user: user, name: "book club") guardian.user.reload - HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source( + FakeBookmarkHashtagDataSource, + stub(enabled?: true), + ) expect(subject.search("book", %w[category tag bookmark]).map(&:ref)).to eq( %w[book-club the-book-club great-books the-book-club::tag], @@ -428,7 +391,10 @@ RSpec.describe HashtagAutocompleteService do Fabricate(:bookmark, user: user, name: "coolrock") guardian.user.reload - HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source( + FakeBookmarkHashtagDataSource, + stub(enabled?: true), + ) result = subject.lookup(["coolrock"], %w[category tag bookmark]) expect(result[:bookmark].map(&:slug)).to eq(["coolrock"]) diff --git a/spec/support/fake_bookmark_hashtag_data_source.rb b/spec/support/fake_bookmark_hashtag_data_source.rb new file mode 100644 index 00000000000..c6dc9fd3996 --- /dev/null +++ b/spec/support/fake_bookmark_hashtag_data_source.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class FakeBookmarkHashtagDataSource + def self.icon + "bookmark" + end + + def self.type + "bookmark" + end + + def self.lookup(guardian_scoped, slugs) + guardian_scoped + .user + .bookmarks + .where("LOWER(name) IN (:slugs)", slugs: slugs) + .map do |bm| + HashtagAutocompleteService::HashtagItem.new.tap do |item| + item.text = bm.name + item.slug = bm.name.gsub(" ", "-") + item.icon = icon + end + end + end + + def self.search( + guardian_scoped, + term, + limit, + condition = HashtagAutocompleteService.search_conditions[:starts_with] + ) + query = guardian_scoped.user.bookmarks + + if condition == HashtagAutocompleteService.search_conditions[:starts_with] + query = query.where("name ILIKE ?", "#{term}%") + else + query = query.where("name ILIKE ?", "%#{term}%") + end + + query + .limit(limit) + .map do |bm| + HashtagAutocompleteService::HashtagItem.new.tap do |item| + item.text = bm.name + item.slug = bm.name.gsub(" ", "-") + item.icon = icon + end + end + end + + def self.search_sort(search_results, _) + search_results.sort_by { |item| item.text.downcase } + end +end