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