From ec9ec1e04eca36ce711a6e6a25a2c04c3f75e61d Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 15 Dec 2022 13:01:44 +1000 Subject: [PATCH] FEATURE: Sort hashtags starting with term higher priority (#19463) This introduces another "section" of queries to the hashtag autocomplete search, which returns results for each type that start with the search term. So now results will be in this order, and within these sections ordered by the types in priority order: 1. Exact matches sorted by type 2. "starts with" sorted by type 3. Everything else sorted by type then name within type --- app/services/category_hashtag_data_source.rb | 29 ++- app/services/hashtag_autocomplete_service.rb | 123 ++++++++---- app/services/tag_hashtag_data_source.rb | 17 +- lib/discourse_tagging.rb | 15 +- plugins/chat/lib/chat_channel_fetcher.rb | 18 +- .../lib/chat_channel_hashtag_data_source.rb | 9 +- .../chat_channel_hashtag_data_source_spec.rb | 18 ++ .../category_hashtag_data_source_spec.rb | 17 +- .../hashtag_autocomplete_service_spec.rb | 177 ++++++++++-------- 9 files changed, 289 insertions(+), 134 deletions(-) diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb index ab912bf1c4f..4ae93a0e47f 100644 --- a/app/services/category_hashtag_data_source.rb +++ b/app/services/category_hashtag_data_source.rb @@ -34,13 +34,28 @@ class CategoryHashtagDataSource .map { |category| category_to_hashtag_item(category) } end - def self.search(guardian, term, limit) - Category - .secured(guardian) - .includes(:parent_category) - .where("LOWER(name) LIKE :term OR LOWER(slug) LIKE :term", term: "%#{term}%") - .take(limit) - .map { |category| category_to_hashtag_item(category) } + def self.search( + guardian, + term, + limit, + condition = HashtagAutocompleteService.search_conditions[:contains] + ) + base_search = + Category + .secured(guardian) + .select(:id, :parent_category_id, :slug, :name, :description) + .includes(:parent_category) + + if condition == HashtagAutocompleteService.search_conditions[:starts_with] + base_search = base_search.where("LOWER(slug) LIKE :term", term: "#{term}%") + elsif condition == HashtagAutocompleteService.search_conditions[:contains] + base_search = + base_search.where("LOWER(name) LIKE :term OR LOWER(slug) LIKE :term", term: "%#{term}%") + else + raise Discourse::InvalidParameters.new("Unknown search condition: #{condition}") + end + + base_search.take(limit).map { |category| category_to_hashtag_item(category) } end def self.search_sort(search_results, term) diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index 31ff7d1e091..b41176bdaeb 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -4,6 +4,10 @@ class HashtagAutocompleteService HASHTAGS_PER_REQUEST = 20 SEARCH_MAX_LIMIT = 50 + def self.search_conditions + @search_conditions ||= Enum.new(contains: 0, starts_with: 1) + end + attr_reader :guardian cattr_reader :data_sources, :contexts @@ -69,6 +73,16 @@ class HashtagAutocompleteService # item, used for the cooked hashtags, e.g. /c/2/staff attr_accessor :relative_url + def initialize(params = {}) + @relative_url = params[:relative_url] + @text = params[:text] + @description = params[:description] + @icon = params[:icon] + @type = params[:type] + @ref = params[:ref] + @slug = params[:slug] + end + def to_h { relative_url: self.relative_url, @@ -204,27 +218,37 @@ class HashtagAutocompleteService break if limited_results.length >= limit end - return limited_results if limited_results.length >= limit + # Next priority are slugs which start with the search term. + if limited_results.length < limit + types_in_priority_order.each do |type| + limited_results = + search_using_condition( + limited_results, + term, + type, + limit, + HashtagAutocompleteService.search_conditions[:starts_with], + ) + top_ranked_type = type if top_ranked_type.nil? + break if limited_results.length >= limit + end + end # Search the data source for each type, validate and sort results, # and break off from searching more data sources if we reach our limit - types_in_priority_order.each do |type| - search_results = search_for_type(type, guardian, term, limit - limited_results.length) - next if search_results.empty? - - next if !all_data_items_valid?(search_results) - - search_results = - @@data_sources[type].search_sort( - search_results.reject do |item| - limited_results.any? { |exact| exact.type == type && exact.slug === item.slug } - end, - term, - ) - - top_ranked_type = type if top_ranked_type.nil? - limited_results.concat(search_results) - break if limited_results.length >= limit + if limited_results.length < limit + types_in_priority_order.each do |type| + limited_results = + search_using_condition( + limited_results, + term, + type, + limit, + HashtagAutocompleteService.search_conditions[:contains], + ) + top_ranked_type = type if top_ranked_type.nil? + break if limited_results.length >= limit + end end # Any items that are _not_ the top-ranked type (which could possibly not be @@ -238,16 +262,7 @@ class HashtagAutocompleteService # # For example, if there is a category with the slug #general and a tag # with the slug #general, then the tag will have its ref changed to #general::tag - limited_results.each do |hashtag_item| - next if hashtag_item.type == top_ranked_type - - other_slugs = limited_results.reject { |r| r.type === hashtag_item.type }.map(&:slug) - if other_slugs.include?(hashtag_item.slug) - hashtag_item.ref = "#{hashtag_item.slug}::#{hashtag_item.type}" - end - end - - limited_results.take(limit) + append_types_to_conflicts(limited_results, top_ranked_type, limit) end # TODO (martin) Remove this once plugins are not relying on the old lookup @@ -297,14 +312,30 @@ class HashtagAutocompleteService private + def search_using_condition(limited_results, term, type, limit, condition) + search_results = + search_for_type(type, guardian, term, limit - limited_results.length, condition) + return limited_results if search_results.empty? + + search_results = + @@data_sources[type].search_sort( + search_results.reject do |item| + limited_results.any? { |exact| exact.type == type && exact.slug === item.slug } + end, + term, + ) + + limited_results.concat(search_results) + end + def search_without_term(types_in_priority_order, limit) split_limit = (limit.to_f / types_in_priority_order.length.to_f).ceil limited_results = [] types_in_priority_order.each do |type| - search_results = @@data_sources[type].search_without_term(guardian, split_limit) + search_results = + filter_valid_data_items(@@data_sources[type].search_without_term(guardian, split_limit)) next if search_results.empty? - next if !all_data_items_valid?(search_results) # This is purposefully unsorted as search_without_term should sort # in its own way. @@ -326,17 +357,24 @@ class HashtagAutocompleteService hashtag_items.each { |item| item.type = type } end - def all_data_items_valid?(items) - items.all? { |item| item.kind_of?(HashtagItem) && item.slug.present? && item.text.present? } + def filter_valid_data_items(items) + items.select { |item| item.kind_of?(HashtagItem) && item.slug.present? && item.text.present? } end - def search_for_type(type, guardian, term, limit) - set_types(set_refs(@@data_sources[type].search(guardian, term, limit)), type) + def search_for_type( + type, + guardian, + term, + limit, + condition = HashtagAutocompleteService.search_conditions[:contains] + ) + filter_valid_data_items( + set_types(set_refs(@@data_sources[type].search(guardian, term, limit, condition)), type), + ) end def execute_lookup!(lookup_results, type, guardian, slugs) - found_from_slugs = lookup_for_type(type, guardian, slugs) - return if !all_data_items_valid?(found_from_slugs) + found_from_slugs = filter_valid_data_items(lookup_for_type(type, guardian, slugs)) found_from_slugs.sort_by! { |item| item.text.downcase } if lookup_results.present? @@ -349,4 +387,17 @@ class HashtagAutocompleteService def lookup_for_type(type, guardian, slugs) set_types(set_refs(@@data_sources[type].lookup(guardian, slugs)), type) end + + def append_types_to_conflicts(limited_results, top_ranked_type, limit) + limited_results.each do |hashtag_item| + next if hashtag_item.type == top_ranked_type + + other_slugs = limited_results.reject { |r| r.type === hashtag_item.type }.map(&:slug) + if other_slugs.include?(hashtag_item.slug) + hashtag_item.ref = "#{hashtag_item.slug}::#{hashtag_item.type}" + end + end + + limited_results.take(limit) + end end diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb index 093e22c2c61..a7057e95fd6 100644 --- a/app/services/tag_hashtag_data_source.rb +++ b/app/services/tag_hashtag_data_source.rb @@ -33,13 +33,26 @@ class TagHashtagDataSource .map { |tag| tag_to_hashtag_item(tag) } end - def self.search(guardian, term, limit) + def self.search( + guardian, + term, + limit, + condition = HashtagAutocompleteService.search_conditions[:contains] + ) return [] if !SiteSetting.tagging_enabled tags_with_counts, _ = DiscourseTagging.filter_allowed_tags( guardian, term: term, + term_type: + ( + if condition == HashtagAutocompleteService.search_conditions[:starts_with] + DiscourseTagging.term_types[:starts_with] + else + DiscourseTagging.term_types[:contains] + end + ), with_context: true, limit: limit, for_input: true, @@ -53,7 +66,7 @@ class TagHashtagDataSource end def self.search_sort(search_results, _) - search_results.sort_by { |result| result.text.downcase } + search_results.sort_by { |item| item.text.downcase } end def self.search_without_term(guardian, limit) diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index 4d964e8fbb9..e68b9032cdf 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -13,6 +13,10 @@ module DiscourseTagging ON tgm.tag_group_id = tg.id SQL + def self.term_types + @term_types ||= Enum.new(contains: 0, starts_with: 1) + end + def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false) if guardian.can_tag?(topic) tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || [] @@ -262,6 +266,7 @@ module DiscourseTagging # Options: # term: a search term to filter tags by name + # term_type: whether to search by "starts_with" or "contains" with the term # limit: max number of results # category: a Category to which the object being tagged belongs # for_input: result is for an input field, so only show permitted tags @@ -355,9 +360,15 @@ module DiscourseTagging term = opts[:term] if term.present? term = term.gsub("_", "\\_").downcase - builder.where("LOWER(name) LIKE :term") builder_params[:cleaned_term] = term - builder_params[:term] = "%#{term}%" + + if opts[:term_type] == DiscourseTagging.term_types[:starts_with] + builder_params[:term] = "#{term}%" + else + builder_params[:term] = "%#{term}%" + end + + builder.where("LOWER(name) LIKE :term") sql.gsub!("/*and_name_like*/", "AND LOWER(t.name) LIKE :term") else sql.gsub!("/*and_name_like*/", "") diff --git a/plugins/chat/lib/chat_channel_fetcher.rb b/plugins/chat/lib/chat_channel_fetcher.rb index 55a2341cb23..2e42ed76177 100644 --- a/plugins/chat/lib/chat_channel_fetcher.rb +++ b/plugins/chat/lib/chat_channel_fetcher.rb @@ -99,19 +99,19 @@ module Chat::ChatChannelFetcher channels = channels.where(status: options[:status]) if options[:status].present? if options[:filter].present? - category_filter = \ - if options[:filter_on_category_name] - "OR categories.name ILIKE :filter" - else - "" - end + category_filter = + (options[:filter_on_category_name] ? "OR categories.name ILIKE :filter" : "") sql = "chat_channels.name ILIKE :filter OR chat_channels.slug ILIKE :filter #{category_filter}" + if options[:match_filter_on_starts_with] + filter_sql = "#{options[:filter].downcase}%" + else + filter_sql = "%#{options[:filter].downcase}%" + end + channels = - channels.where(sql, filter: "%#{options[:filter].downcase}%").order( - "chat_channels.name ASC, categories.name ASC", - ) + channels.where(sql, filter: filter_sql).order("chat_channels.name ASC, categories.name ASC") end if options.key?(:slugs) diff --git a/plugins/chat/lib/chat_channel_hashtag_data_source.rb b/plugins/chat/lib/chat_channel_hashtag_data_source.rb index 7fed9fc8b68..49cc18280ce 100644 --- a/plugins/chat/lib/chat_channel_hashtag_data_source.rb +++ b/plugins/chat/lib/chat_channel_hashtag_data_source.rb @@ -27,7 +27,12 @@ class Chat::ChatChannelHashtagDataSource end end - def self.search(guardian, term, limit) + def self.search( + guardian, + term, + limit, + condition = HashtagAutocompleteService.search_conditions[:contains] + ) if SiteSetting.enable_experimental_hashtag_autocomplete return [] if !guardian.can_chat? Chat::ChatChannelFetcher @@ -36,6 +41,8 @@ class Chat::ChatChannelHashtagDataSource 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 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 30358123622..cf7de252b76 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 @@ -183,4 +183,22 @@ RSpec.describe Chat::ChatChannelHashtagDataSource do expect(described_class.search_without_term(Guardian.new(user), 10)).to be_empty end end + + describe "#search_sort" do + it "orders by exact slug match, starts with, then text" do + results_to_sort = [ + HashtagAutocompleteService::HashtagItem.new( + text: "System Tests", + slug: "system-test-development", + ), + HashtagAutocompleteService::HashtagItem.new(text: "Ruby Dev", slug: "ruby-dev"), + HashtagAutocompleteService::HashtagItem.new(text: "Dev", slug: "dev"), + HashtagAutocompleteService::HashtagItem.new(text: "Dev Tools", slug: "dev-tools"), + HashtagAutocompleteService::HashtagItem.new(text: "Dev Lore", slug: "dev-lore"), + ] + expect(described_class.search_sort(results_to_sort, "dev").map(&:slug)).to eq( + %w[dev dev-lore dev-tools ruby-dev system-test-development], + ) + end + end end diff --git a/spec/services/category_hashtag_data_source_spec.rb b/spec/services/category_hashtag_data_source_spec.rb index 2084bb0ae65..f6f62b45682 100644 --- a/spec/services/category_hashtag_data_source_spec.rb +++ b/spec/services/category_hashtag_data_source_spec.rb @@ -71,7 +71,7 @@ RSpec.describe CategoryHashtagDataSource do describe "#search_without_term" do it "returns distinct categories ordered by topic_count" do expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq( - ["books", "movies", "casual", "random", "fun"], + %w[books movies casual random fun], ) end @@ -92,4 +92,19 @@ RSpec.describe CategoryHashtagDataSource do expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("random") end end + + describe "#search_sort" do + it "orders by exact slug match then text" do + results_to_sort = [ + HashtagAutocompleteService::HashtagItem.new(text: "System Tests", slug: "system-test-development"), + HashtagAutocompleteService::HashtagItem.new(text: "Ruby Dev", slug: "ruby-dev"), + HashtagAutocompleteService::HashtagItem.new(text: "Dev", slug: "dev"), + HashtagAutocompleteService::HashtagItem.new(text: "Dev Tools", slug: "dev-tools"), + HashtagAutocompleteService::HashtagItem.new(text: "Dev Lore", slug: "dev-lore"), + ] + expect(described_class.search_sort(results_to_sort, "dev").map(&:slug)).to eq( + %w[dev dev-lore dev-tools ruby-dev system-test-development], + ) + end + end end diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index 45a625e0c26..59a9b70807e 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -2,7 +2,7 @@ RSpec.describe HashtagAutocompleteService do fab!(:user) { Fabricate(:user) } - fab!(:category1) { Fabricate(:category, name: "Book Club", slug: "book-club") } + fab!(:category1) { Fabricate(:category, name: "The Book Club", slug: "the-book-club") } fab!(:tag1) { Fabricate(:tag, name: "great-books", topic_count: 22) } fab!(:topic1) { Fabricate(:topic) } let(:guardian) { Guardian.new(user) } @@ -30,11 +30,21 @@ RSpec.describe HashtagAutocompleteService do end end - def self.search(guardian_scoped, term, limit) - guardian_scoped - .user - .bookmarks - .where("name ILIKE ?", "%#{term}%") + 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| @@ -72,13 +82,13 @@ RSpec.describe HashtagAutocompleteService do describe "#search" do it "returns search results for tags and categories by default" do expect(subject.search("book", %w[category tag]).map(&:text)).to eq( - ["Book Club", "great-books x 22"], + ["The Book Club", "great-books x 22"], ) end it "respects the types_in_priority_order param" do expect(subject.search("book", %w[tag category]).map(&:text)).to eq( - ["great-books x 22", "Book Club"], + ["great-books x 22", "The Book Club"], ) end @@ -91,7 +101,7 @@ RSpec.describe HashtagAutocompleteService do 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"], + ["The Book Club"], ) end end @@ -99,28 +109,28 @@ RSpec.describe HashtagAutocompleteService do it "does not search other data sources if the limit is reached by earlier type data sources" do # only expected once to try get the exact matches first DiscourseTagging.expects(:filter_allowed_tags).never - subject.search("book", %w[category tag], limit: 1) + subject.search("the-book", %w[category tag], limit: 1) end it "includes the tag count" do tag1.update!(topic_count: 78) expect(subject.search("book", %w[tag category]).map(&:text)).to eq( - ["great-books x 78", "Book Club"], + ["great-books x 78", "The Book Club"], ) end it "does case-insensitive search" do expect(subject.search("book", %w[category tag]).map(&:text)).to eq( - ["Book Club", "great-books x 22"], + ["The Book Club", "great-books x 22"], ) expect(subject.search("bOOk", %w[category tag]).map(&:text)).to eq( - ["Book Club", "great-books x 22"], + ["The Book Club", "great-books x 22"], ) end it "can search categories by name or slug" do - expect(subject.search("book-club", %w[category]).map(&:text)).to eq(["Book Club"]) - expect(subject.search("Book C", %w[category]).map(&:text)).to eq(["Book Club"]) + expect(subject.search("the-book-club", %w[category]).map(&:text)).to eq(["The Book Club"]) + expect(subject.search("Book C", %w[category]).map(&:text)).to eq(["The Book Club"]) end it "does not include categories the user cannot access" do @@ -141,7 +151,7 @@ RSpec.describe HashtagAutocompleteService do HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) expect(subject.search("book", %w[category tag bookmark]).map(&:text)).to eq( - ["Book Club", "great-books x 22", "read review of this fantasy book"], + ["The Book Club", "great-books x 22", "read review of this fantasy book"], ) end @@ -149,15 +159,15 @@ RSpec.describe HashtagAutocompleteService do parent = Fabricate(:category, name: "Hobbies", slug: "hobbies") category1.update!(parent_category: parent) expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( - %w[hobbies:book-club great-books], + %w[hobbies:the-book-club great-books], ) category1.update!(parent_category: nil) end 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") + Fabricate(:tag, name: "the-book-club") expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( - %w[book-club book-club::tag great-books], + %w[the-book-club great-books the-book-club::tag], ) Fabricate(:bookmark, user: user, name: "book club") @@ -166,37 +176,50 @@ RSpec.describe HashtagAutocompleteService do HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) expect(subject.search("book", %w[category tag bookmark]).map(&:ref)).to eq( - %w[book-club book-club::tag great-books book-club::bookmark], + %w[book-club the-book-club great-books the-book-club::tag], ) end - it "orders categories by exact match on slug (ignoring parent/child distinction) then name, and then name for everything else" do + it "orders results by (with type ordering within each section): + 1. exact match on slug (ignoring parent/child distinction for categories) + 2. slugs that start with the term + 3. then name for everything else" do category2 = Fabricate(:category, name: "Book Library", slug: "book-library") Fabricate(:category, name: "Horror", slug: "book", parent_category: category2) Fabricate(:category, name: "Romance", slug: "romance-books") Fabricate(:category, name: "Abstract Philosophy", slug: "abstract-philosophy-books") category6 = Fabricate(:category, name: "Book Reviews", slug: "book-reviews") Fabricate(:category, name: "Good Books", slug: "book", parent_category: category6) - expect(subject.search("book", %w[category]).map(&:ref)).to eq( - %w[ - book-reviews:book - book-library:book - abstract-philosophy-books - book-club - book-library - book-reviews - romance-books + + Fabricate(:tag, name: "bookmania", topic_count: 15) + Fabricate(:tag, name: "awful-books", topic_count: 56) + + expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( + [ + "book-reviews:book", # category exact match on slug, name sorted + "book-library:book", + "book-library", # category starts with match on slug, name sorted + "book-reviews", + "bookmania", # tag starts with match on slug, name sorted + "abstract-philosophy-books", # category partial match on slug, name sorted + "romance-books", + "the-book-club", + "awful-books", # tag partial match on slug, name sorted + "great-books", ], ) - expect(subject.search("book", %w[category]).map(&:text)).to eq( + expect(subject.search("book", %w[category tag]).map(&:text)).to eq( [ "Good Books", "Horror", - "Abstract Philosophy", - "Book Club", "Book Library", "Book Reviews", + "bookmania x 15", + "Abstract Philosophy", "Romance", + "The Book Club", + "awful-books x 56", + "great-books x 22", ], ) end @@ -211,19 +234,19 @@ RSpec.describe HashtagAutocompleteService do 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 book::tag book-club book-dome book-zone great-books mid-books terrible-books], + %w[book book::tag book-dome book-zone the-book-club 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 book::tag book-club book-dome book-zone], + %w[book book::tag book-dome book-zone the-book-club], ) end it "prioritises exact matches to the top of the list" do expect(subject.search("book", %w[category tag], limit: 10).map(&:ref)).to eq( - %w[book book::tag book-club book-dome book-zone great-books mid-books terrible-books], + %w[book book::tag book-dome book-zone the-book-club great-books mid-books terrible-books], ) end end @@ -232,7 +255,7 @@ RSpec.describe HashtagAutocompleteService do before { SiteSetting.tagging_enabled = false } it "does not return any tags" do - expect(subject.search("book", %w[category tag]).map(&:text)).to eq(["Book Club"]) + expect(subject.search("book", %w[category tag]).map(&:text)).to eq(["The Book Club"]) end end @@ -273,8 +296,8 @@ RSpec.describe HashtagAutocompleteService do fab!(:tag2) { Fabricate(:tag, name: "fiction-books") } it "returns categories and tags in a hash format with the slug and url" do - result = subject.lookup_old(%w[book-club great-books fiction-books]) - expect(result[:categories]).to eq({ "book-club" => "/c/book-club/#{category1.id}" }) + result = subject.lookup_old(%w[the-book-club great-books fiction-books]) + expect(result[:categories]).to eq({ "the-book-club" => "/c/the-book-club/#{category1.id}" }) expect(result[:tags]).to eq( { "fiction-books" => "http://test.localhost/tag/fiction-books", @@ -285,18 +308,18 @@ RSpec.describe HashtagAutocompleteService do it "does not include categories the user cannot access" do category1.update!(read_restricted: true) - result = subject.lookup_old(%w[book-club great-books fiction-books]) + result = subject.lookup_old(%w[the-book-club great-books fiction-books]) expect(result[:categories]).to eq({}) end it "does not include tags the user cannot access" do Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["great-books"]) - result = subject.lookup_old(%w[book-club great-books fiction-books]) + result = subject.lookup_old(%w[the-book-club great-books fiction-books]) expect(result[:tags]).to eq({ "fiction-books" => "http://test.localhost/tag/fiction-books" }) end it "handles tags which have the ::tag suffix" do - result = subject.lookup_old(%w[book-club great-books::tag fiction-books]) + result = subject.lookup_old(%w[the-book-club great-books::tag fiction-books]) expect(result[:tags]).to eq( { "fiction-books" => "http://test.localhost/tag/fiction-books", @@ -309,8 +332,8 @@ RSpec.describe HashtagAutocompleteService do before { SiteSetting.tagging_enabled = false } it "does not return tags" do - result = subject.lookup_old(%w[book-club great-books fiction-books]) - expect(result[:categories]).to eq({ "book-club" => "/c/book-club/#{category1.id}" }) + result = subject.lookup_old(%w[the-book-club great-books fiction-books]) + expect(result[:categories]).to eq({ "the-book-club" => "/c/the-book-club/#{category1.id}" }) expect(result[:tags]).to eq({}) end end @@ -320,31 +343,31 @@ RSpec.describe HashtagAutocompleteService do fab!(:tag2) { Fabricate(:tag, name: "fiction-books") } it "returns category and tag in a hash format with the slug and url" do - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) - expect(result[:category].map(&:slug)).to eq(["book-club"]) - expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + result = subject.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].map(&:slug)).to eq(%w[fiction-books great-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) end it "does not include category the user cannot access" do category1.update!(read_restricted: true) - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + result = subject.lookup(%w[the-book-club great-books fiction-books], %w[category tag]) expect(result[:category]).to eq([]) end it "does not include tag the user cannot access" do Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["great-books"]) - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + result = subject.lookup(%w[the-book-club great-books fiction-books], %w[category tag]) expect(result[:tag].map(&:slug)).to eq(%w[fiction-books]) expect(result[:tag].map(&:relative_url)).to eq(["/tag/fiction-books"]) end it "handles type suffixes for slugs" do result = - subject.lookup(%w[book-club::category great-books::tag fiction-books], %w[category tag]) - expect(result[:category].map(&:slug)).to eq(["book-club"]) - expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + subject.lookup(%w[the-book-club::category great-books::tag 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].map(&:slug)).to eq(%w[fiction-books great-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) end @@ -352,49 +375,51 @@ RSpec.describe HashtagAutocompleteService do it "handles parent:child category lookups" do parent_category = Fabricate(:category, name: "Media", slug: "media") category1.update!(parent_category: parent_category) - result = subject.lookup(%w[media:book-club], %w[category tag]) - expect(result[:category].map(&:slug)).to eq(["book-club"]) - expect(result[:category].map(&:ref)).to eq(["media:book-club"]) - expect(result[:category].map(&:relative_url)).to eq(["/c/media/book-club/#{category1.id}"]) + result = subject.lookup(%w[media:the-book-club], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["the-book-club"]) + expect(result[:category].map(&:ref)).to eq(["media:the-book-club"]) + expect(result[:category].map(&:relative_url)).to eq( + ["/c/media/the-book-club/#{category1.id}"], + ) category1.update!(parent_category: nil) end it "does not return the category if the parent does not match the child" do parent_category = Fabricate(:category, name: "Media", slug: "media") category1.update!(parent_category: parent_category) - result = subject.lookup(%w[bad-parent:book-club], %w[category tag]) + result = subject.lookup(%w[bad-parent:the-book-club], %w[category tag]) expect(result[:category]).to be_empty end it "for slugs without a type suffix it falls back in type order until a result is found or types are exhausted" do - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) - expect(result[:category].map(&:slug)).to eq(["book-club"]) - expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + result = subject.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].map(&:slug)).to eq(%w[fiction-books great-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) category2 = Fabricate(:category, name: "Great Books", slug: "great-books") - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) - expect(result[:category].map(&:slug)).to eq(%w[book-club great-books]) + result = subject.lookup(%w[the-book-club great-books fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(%w[great-books the-book-club]) expect(result[:category].map(&:relative_url)).to eq( - ["/c/book-club/#{category1.id}", "/c/great-books/#{category2.id}"], + ["/c/great-books/#{category2.id}", "/c/the-book-club/#{category1.id}"], ) expect(result[:tag].map(&:slug)).to eq(%w[fiction-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books]) category1.destroy! - Fabricate(:tag, name: "book-club") - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + Fabricate(:tag, name: "the-book-club") + result = subject.lookup(%w[the-book-club great-books fiction-books], %w[category tag]) expect(result[:category].map(&:slug)).to eq(["great-books"]) expect(result[:category].map(&:relative_url)).to eq(["/c/great-books/#{category2.id}"]) - expect(result[:tag].map(&:slug)).to eq(%w[book-club fiction-books]) - expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/book-club /tag/fiction-books]) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books the-book-club]) + expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/the-book-club]) - result = subject.lookup(%w[book-club great-books fiction-books], %w[tag category]) + result = subject.lookup(%w[the-book-club great-books fiction-books], %w[tag category]) expect(result[:category]).to eq([]) - expect(result[:tag].map(&:slug)).to eq(%w[book-club fiction-books great-books]) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books great-books the-book-club]) expect(result[:tag].map(&:relative_url)).to eq( - %w[/tag/book-club /tag/fiction-books /tag/great-books], + %w[/tag/fiction-books /tag/great-books /tag/the-book-club], ) end @@ -411,19 +436,19 @@ RSpec.describe HashtagAutocompleteService do it "handles type suffix lookups where there is another type with a conflicting slug that the user cannot access" do category1.update!(read_restricted: true) - Fabricate(:tag, name: "book-club") - result = subject.lookup(%w[book-club::tag book-club], %w[category tag]) + Fabricate(:tag, name: "the-book-club") + result = subject.lookup(%w[the-book-club::tag the-book-club], %w[category tag]) expect(result[:category].map(&:ref)).to eq([]) - expect(result[:tag].map(&:ref)).to eq(["book-club::tag"]) + expect(result[:tag].map(&:ref)).to eq(["the-book-club::tag"]) end context "when not tagging_enabled" do before { SiteSetting.tagging_enabled = false } it "does not return tag" do - result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) - expect(result[:category].map(&:slug)).to eq(["book-club"]) - expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + result = subject.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([]) end end