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
This commit is contained in:
Martin Brennan 2022-12-15 13:01:44 +10:00 committed by GitHub
parent 2b4009c6bc
commit ec9ec1e04e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 289 additions and 134 deletions

View File

@ -34,13 +34,28 @@ class CategoryHashtagDataSource
.map { |category| category_to_hashtag_item(category) } .map { |category| category_to_hashtag_item(category) }
end end
def self.search(guardian, term, limit) def self.search(
Category guardian,
.secured(guardian) term,
.includes(:parent_category) limit,
.where("LOWER(name) LIKE :term OR LOWER(slug) LIKE :term", term: "%#{term}%") condition = HashtagAutocompleteService.search_conditions[:contains]
.take(limit) )
.map { |category| category_to_hashtag_item(category) } 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 end
def self.search_sort(search_results, term) def self.search_sort(search_results, term)

View File

@ -4,6 +4,10 @@ class HashtagAutocompleteService
HASHTAGS_PER_REQUEST = 20 HASHTAGS_PER_REQUEST = 20
SEARCH_MAX_LIMIT = 50 SEARCH_MAX_LIMIT = 50
def self.search_conditions
@search_conditions ||= Enum.new(contains: 0, starts_with: 1)
end
attr_reader :guardian attr_reader :guardian
cattr_reader :data_sources, :contexts cattr_reader :data_sources, :contexts
@ -69,6 +73,16 @@ class HashtagAutocompleteService
# item, used for the cooked hashtags, e.g. /c/2/staff # item, used for the cooked hashtags, e.g. /c/2/staff
attr_accessor :relative_url 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 def to_h
{ {
relative_url: self.relative_url, relative_url: self.relative_url,
@ -204,27 +218,37 @@ class HashtagAutocompleteService
break if limited_results.length >= limit break if limited_results.length >= limit
end 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, # Search the data source for each type, validate and sort results,
# and break off from searching more data sources if we reach our limit # and break off from searching more data sources if we reach our limit
types_in_priority_order.each do |type| if limited_results.length < limit
search_results = search_for_type(type, guardian, term, limit - limited_results.length) types_in_priority_order.each do |type|
next if search_results.empty? limited_results =
search_using_condition(
next if !all_data_items_valid?(search_results) limited_results,
term,
search_results = type,
@@data_sources[type].search_sort( limit,
search_results.reject do |item| HashtagAutocompleteService.search_conditions[:contains],
limited_results.any? { |exact| exact.type == type && exact.slug === item.slug } )
end, top_ranked_type = type if top_ranked_type.nil?
term, break if limited_results.length >= limit
) end
top_ranked_type = type if top_ranked_type.nil?
limited_results.concat(search_results)
break if limited_results.length >= limit
end end
# Any items that are _not_ the top-ranked type (which could possibly not be # 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 # 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 # with the slug #general, then the tag will have its ref changed to #general::tag
limited_results.each do |hashtag_item| append_types_to_conflicts(limited_results, top_ranked_type, limit)
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
# TODO (martin) Remove this once plugins are not relying on the old lookup # TODO (martin) Remove this once plugins are not relying on the old lookup
@ -297,14 +312,30 @@ class HashtagAutocompleteService
private 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) def search_without_term(types_in_priority_order, limit)
split_limit = (limit.to_f / types_in_priority_order.length.to_f).ceil split_limit = (limit.to_f / types_in_priority_order.length.to_f).ceil
limited_results = [] limited_results = []
types_in_priority_order.each do |type| 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 search_results.empty?
next if !all_data_items_valid?(search_results)
# This is purposefully unsorted as search_without_term should sort # This is purposefully unsorted as search_without_term should sort
# in its own way. # in its own way.
@ -326,17 +357,24 @@ class HashtagAutocompleteService
hashtag_items.each { |item| item.type = type } hashtag_items.each { |item| item.type = type }
end end
def all_data_items_valid?(items) def filter_valid_data_items(items)
items.all? { |item| item.kind_of?(HashtagItem) && item.slug.present? && item.text.present? } items.select { |item| item.kind_of?(HashtagItem) && item.slug.present? && item.text.present? }
end end
def search_for_type(type, guardian, term, limit) def search_for_type(
set_types(set_refs(@@data_sources[type].search(guardian, term, limit)), 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 end
def execute_lookup!(lookup_results, type, guardian, slugs) def execute_lookup!(lookup_results, type, guardian, slugs)
found_from_slugs = lookup_for_type(type, guardian, slugs) found_from_slugs = filter_valid_data_items(lookup_for_type(type, guardian, slugs))
return if !all_data_items_valid?(found_from_slugs)
found_from_slugs.sort_by! { |item| item.text.downcase } found_from_slugs.sort_by! { |item| item.text.downcase }
if lookup_results.present? if lookup_results.present?
@ -349,4 +387,17 @@ class HashtagAutocompleteService
def lookup_for_type(type, guardian, slugs) def lookup_for_type(type, guardian, slugs)
set_types(set_refs(@@data_sources[type].lookup(guardian, slugs)), type) set_types(set_refs(@@data_sources[type].lookup(guardian, slugs)), type)
end 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 end

View File

@ -33,13 +33,26 @@ class TagHashtagDataSource
.map { |tag| tag_to_hashtag_item(tag) } .map { |tag| tag_to_hashtag_item(tag) }
end end
def self.search(guardian, term, limit) def self.search(
guardian,
term,
limit,
condition = HashtagAutocompleteService.search_conditions[:contains]
)
return [] if !SiteSetting.tagging_enabled return [] if !SiteSetting.tagging_enabled
tags_with_counts, _ = tags_with_counts, _ =
DiscourseTagging.filter_allowed_tags( DiscourseTagging.filter_allowed_tags(
guardian, guardian,
term: term, 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, with_context: true,
limit: limit, limit: limit,
for_input: true, for_input: true,
@ -53,7 +66,7 @@ class TagHashtagDataSource
end end
def self.search_sort(search_results, _) def self.search_sort(search_results, _)
search_results.sort_by { |result| result.text.downcase } search_results.sort_by { |item| item.text.downcase }
end end
def self.search_without_term(guardian, limit) def self.search_without_term(guardian, limit)

View File

@ -13,6 +13,10 @@ module DiscourseTagging
ON tgm.tag_group_id = tg.id ON tgm.tag_group_id = tg.id
SQL 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) def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false)
if guardian.can_tag?(topic) if guardian.can_tag?(topic)
tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || [] tag_names = DiscourseTagging.tags_for_saving(tag_names_arg, guardian) || []
@ -262,6 +266,7 @@ module DiscourseTagging
# Options: # Options:
# term: a search term to filter tags by name # 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 # limit: max number of results
# category: a Category to which the object being tagged belongs # category: a Category to which the object being tagged belongs
# for_input: result is for an input field, so only show permitted tags # for_input: result is for an input field, so only show permitted tags
@ -355,9 +360,15 @@ module DiscourseTagging
term = opts[:term] term = opts[:term]
if term.present? if term.present?
term = term.gsub("_", "\\_").downcase term = term.gsub("_", "\\_").downcase
builder.where("LOWER(name) LIKE :term")
builder_params[:cleaned_term] = 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") sql.gsub!("/*and_name_like*/", "AND LOWER(t.name) LIKE :term")
else else
sql.gsub!("/*and_name_like*/", "") sql.gsub!("/*and_name_like*/", "")

View File

@ -99,19 +99,19 @@ module Chat::ChatChannelFetcher
channels = channels.where(status: options[:status]) if options[:status].present? channels = channels.where(status: options[:status]) if options[:status].present?
if options[:filter].present? if options[:filter].present?
category_filter = \ category_filter =
if options[:filter_on_category_name] (options[:filter_on_category_name] ? "OR categories.name ILIKE :filter" : "")
"OR categories.name ILIKE :filter"
else
""
end
sql = sql =
"chat_channels.name ILIKE :filter OR chat_channels.slug ILIKE :filter #{category_filter}" "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 =
channels.where(sql, filter: "%#{options[:filter].downcase}%").order( channels.where(sql, filter: filter_sql).order("chat_channels.name ASC, categories.name ASC")
"chat_channels.name ASC, categories.name ASC",
)
end end
if options.key?(:slugs) if options.key?(:slugs)

View File

@ -27,7 +27,12 @@ class Chat::ChatChannelHashtagDataSource
end end
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 if SiteSetting.enable_experimental_hashtag_autocomplete
return [] if !guardian.can_chat? return [] if !guardian.can_chat?
Chat::ChatChannelFetcher Chat::ChatChannelFetcher
@ -36,6 +41,8 @@ class Chat::ChatChannelHashtagDataSource
filter: term, filter: term,
limit: limit, limit: limit,
exclude_dm_channels: true, exclude_dm_channels: true,
match_filter_on_starts_with:
condition == HashtagAutocompleteService.search_conditions[:starts_with],
) )
.map { |channel| channel_to_hashtag_item(guardian, channel) } .map { |channel| channel_to_hashtag_item(guardian, channel) }
else else

View File

@ -183,4 +183,22 @@ RSpec.describe Chat::ChatChannelHashtagDataSource do
expect(described_class.search_without_term(Guardian.new(user), 10)).to be_empty expect(described_class.search_without_term(Guardian.new(user), 10)).to be_empty
end end
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 end

View File

@ -71,7 +71,7 @@ RSpec.describe CategoryHashtagDataSource do
describe "#search_without_term" do describe "#search_without_term" do
it "returns distinct categories ordered by topic_count" do it "returns distinct categories ordered by topic_count" do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq( expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq(
["books", "movies", "casual", "random", "fun"], %w[books movies casual random fun],
) )
end end
@ -92,4 +92,19 @@ RSpec.describe CategoryHashtagDataSource do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("random") expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("random")
end end
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 end

View File

@ -2,7 +2,7 @@
RSpec.describe HashtagAutocompleteService do RSpec.describe HashtagAutocompleteService do
fab!(:user) { Fabricate(:user) } 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!(:tag1) { Fabricate(:tag, name: "great-books", topic_count: 22) }
fab!(:topic1) { Fabricate(:topic) } fab!(:topic1) { Fabricate(:topic) }
let(:guardian) { Guardian.new(user) } let(:guardian) { Guardian.new(user) }
@ -30,11 +30,21 @@ RSpec.describe HashtagAutocompleteService do
end end
end end
def self.search(guardian_scoped, term, limit) def self.search(
guardian_scoped guardian_scoped,
.user term,
.bookmarks limit,
.where("name ILIKE ?", "%#{term}%") 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) .limit(limit)
.map do |bm| .map do |bm|
HashtagAutocompleteService::HashtagItem.new.tap do |item| HashtagAutocompleteService::HashtagItem.new.tap do |item|
@ -72,13 +82,13 @@ RSpec.describe HashtagAutocompleteService do
describe "#search" do describe "#search" do
it "returns search results for tags and categories by default" do it "returns search results for tags and categories by default" do
expect(subject.search("book", %w[category tag]).map(&:text)).to eq( 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 end
it "respects the types_in_priority_order param" do it "respects the types_in_priority_order param" do
expect(subject.search("book", %w[tag category]).map(&:text)).to eq( 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 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 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 stub_const(HashtagAutocompleteService, "SEARCH_MAX_LIMIT", 1) do
expect(subject.search("book", %w[category tag], limit: 1000).map(&:text)).to eq( expect(subject.search("book", %w[category tag], limit: 1000).map(&:text)).to eq(
["Book Club"], ["The Book Club"],
) )
end end
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 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 # only expected once to try get the exact matches first
DiscourseTagging.expects(:filter_allowed_tags).never DiscourseTagging.expects(:filter_allowed_tags).never
subject.search("book", %w[category tag], limit: 1) subject.search("the-book", %w[category tag], limit: 1)
end end
it "includes the tag count" do it "includes the tag count" do
tag1.update!(topic_count: 78) tag1.update!(topic_count: 78)
expect(subject.search("book", %w[tag category]).map(&:text)).to eq( 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 end
it "does case-insensitive search" do it "does case-insensitive search" do
expect(subject.search("book", %w[category tag]).map(&:text)).to eq( 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( 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 end
it "can search categories by name or slug" do 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("the-book-club", %w[category]).map(&:text)).to eq(["The Book Club"])
expect(subject.search("Book C", %w[category]).map(&:text)).to eq(["Book Club"]) expect(subject.search("Book C", %w[category]).map(&:text)).to eq(["The Book Club"])
end end
it "does not include categories the user cannot access" do it "does not include categories the user cannot access" do
@ -141,7 +151,7 @@ RSpec.describe HashtagAutocompleteService do
HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource)
expect(subject.search("book", %w[category tag bookmark]).map(&:text)).to eq( 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 end
@ -149,15 +159,15 @@ RSpec.describe HashtagAutocompleteService do
parent = Fabricate(:category, name: "Hobbies", slug: "hobbies") parent = Fabricate(:category, name: "Hobbies", slug: "hobbies")
category1.update!(parent_category: parent) category1.update!(parent_category: parent)
expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( 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) category1.update!(parent_category: nil)
end end
it "appends type suffixes for the ref on conflicting slugs on items that are not the top priority type" do 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( 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") Fabricate(:bookmark, user: user, name: "book club")
@ -166,37 +176,50 @@ RSpec.describe HashtagAutocompleteService do
HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource)
expect(subject.search("book", %w[category tag bookmark]).map(&:ref)).to eq( 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 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") category2 = Fabricate(:category, name: "Book Library", slug: "book-library")
Fabricate(:category, name: "Horror", slug: "book", parent_category: category2) Fabricate(:category, name: "Horror", slug: "book", parent_category: category2)
Fabricate(:category, name: "Romance", slug: "romance-books") Fabricate(:category, name: "Romance", slug: "romance-books")
Fabricate(:category, name: "Abstract Philosophy", slug: "abstract-philosophy-books") Fabricate(:category, name: "Abstract Philosophy", slug: "abstract-philosophy-books")
category6 = Fabricate(:category, name: "Book Reviews", slug: "book-reviews") category6 = Fabricate(:category, name: "Book Reviews", slug: "book-reviews")
Fabricate(:category, name: "Good Books", slug: "book", parent_category: category6) Fabricate(:category, name: "Good Books", slug: "book", parent_category: category6)
expect(subject.search("book", %w[category]).map(&:ref)).to eq(
%w[ Fabricate(:tag, name: "bookmania", topic_count: 15)
book-reviews:book Fabricate(:tag, name: "awful-books", topic_count: 56)
book-library:book
abstract-philosophy-books expect(subject.search("book", %w[category tag]).map(&:ref)).to eq(
book-club [
book-library "book-reviews:book", # category exact match on slug, name sorted
book-reviews "book-library:book",
romance-books "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", "Good Books",
"Horror", "Horror",
"Abstract Philosophy",
"Book Club",
"Book Library", "Book Library",
"Book Reviews", "Book Reviews",
"bookmania x 15",
"Abstract Philosophy",
"Romance", "Romance",
"The Book Club",
"awful-books x 56",
"great-books x 22",
], ],
) )
end end
@ -211,19 +234,19 @@ RSpec.describe HashtagAutocompleteService do
it "orders them by name within their type order" do it "orders them by name within their type order" do
expect(subject.search("book", %w[category tag], limit: 10).map(&:ref)).to eq( 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
it "orders correctly with lower limits" do it "orders correctly with lower limits" do
expect(subject.search("book", %w[category tag], limit: 5).map(&:ref)).to eq( 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 end
it "prioritises exact matches to the top of the list" do it "prioritises exact matches to the top of the list" do
expect(subject.search("book", %w[category tag], limit: 10).map(&:ref)).to eq( 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
end end
@ -232,7 +255,7 @@ RSpec.describe HashtagAutocompleteService do
before { SiteSetting.tagging_enabled = false } before { SiteSetting.tagging_enabled = false }
it "does not return any tags" do 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
end end
@ -273,8 +296,8 @@ RSpec.describe HashtagAutocompleteService do
fab!(:tag2) { Fabricate(:tag, name: "fiction-books") } fab!(:tag2) { Fabricate(:tag, name: "fiction-books") }
it "returns categories and tags in a hash format with the slug and url" do 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]) result = subject.lookup_old(%w[the-book-club great-books fiction-books])
expect(result[:categories]).to eq({ "book-club" => "/c/book-club/#{category1.id}" }) expect(result[:categories]).to eq({ "the-book-club" => "/c/the-book-club/#{category1.id}" })
expect(result[:tags]).to eq( expect(result[:tags]).to eq(
{ {
"fiction-books" => "http://test.localhost/tag/fiction-books", "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 it "does not include categories the user cannot access" do
category1.update!(read_restricted: true) 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({}) expect(result[:categories]).to eq({})
end end
it "does not include tags the user cannot access" do it "does not include tags the user cannot access" do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["great-books"]) 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" }) expect(result[:tags]).to eq({ "fiction-books" => "http://test.localhost/tag/fiction-books" })
end end
it "handles tags which have the ::tag suffix" do 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( expect(result[:tags]).to eq(
{ {
"fiction-books" => "http://test.localhost/tag/fiction-books", "fiction-books" => "http://test.localhost/tag/fiction-books",
@ -309,8 +332,8 @@ RSpec.describe HashtagAutocompleteService do
before { SiteSetting.tagging_enabled = false } before { SiteSetting.tagging_enabled = false }
it "does not return tags" do it "does not return tags" do
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({ "book-club" => "/c/book-club/#{category1.id}" }) expect(result[:categories]).to eq({ "the-book-club" => "/c/the-book-club/#{category1.id}" })
expect(result[:tags]).to eq({}) expect(result[:tags]).to eq({})
end end
end end
@ -320,31 +343,31 @@ RSpec.describe HashtagAutocompleteService do
fab!(:tag2) { Fabricate(:tag, name: "fiction-books") } fab!(:tag2) { Fabricate(:tag, name: "fiction-books") }
it "returns category and tag in a hash format with the slug and url" do 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]) result = subject.lookup(%w[the-book-club great-books fiction-books], %w[category tag])
expect(result[:category].map(&:slug)).to eq(["book-club"]) expect(result[:category].map(&:slug)).to eq(["the-book-club"])
expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) 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(&:slug)).to eq(%w[fiction-books great-books])
expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books])
end end
it "does not include category the user cannot access" do it "does not include category the user cannot access" do
category1.update!(read_restricted: true) 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([]) expect(result[:category]).to eq([])
end end
it "does not include tag the user cannot access" do it "does not include tag the user cannot access" do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["great-books"]) 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(&:slug)).to eq(%w[fiction-books])
expect(result[:tag].map(&:relative_url)).to eq(["/tag/fiction-books"]) expect(result[:tag].map(&:relative_url)).to eq(["/tag/fiction-books"])
end end
it "handles type suffixes for slugs" do it "handles type suffixes for slugs" do
result = result =
subject.lookup(%w[book-club::category great-books::tag fiction-books], %w[category tag]) subject.lookup(%w[the-book-club::category great-books::tag fiction-books], %w[category tag])
expect(result[:category].map(&:slug)).to eq(["book-club"]) expect(result[:category].map(&:slug)).to eq(["the-book-club"])
expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) 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(&:slug)).to eq(%w[fiction-books great-books])
expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books])
end end
@ -352,49 +375,51 @@ RSpec.describe HashtagAutocompleteService do
it "handles parent:child category lookups" do it "handles parent:child category lookups" do
parent_category = Fabricate(:category, name: "Media", slug: "media") parent_category = Fabricate(:category, name: "Media", slug: "media")
category1.update!(parent_category: parent_category) category1.update!(parent_category: parent_category)
result = subject.lookup(%w[media:book-club], %w[category tag]) result = subject.lookup(%w[media:the-book-club], %w[category tag])
expect(result[:category].map(&:slug)).to eq(["book-club"]) expect(result[:category].map(&:slug)).to eq(["the-book-club"])
expect(result[:category].map(&:ref)).to eq(["media:book-club"]) expect(result[:category].map(&:ref)).to eq(["media:the-book-club"])
expect(result[:category].map(&:relative_url)).to eq(["/c/media/book-club/#{category1.id}"]) expect(result[:category].map(&:relative_url)).to eq(
["/c/media/the-book-club/#{category1.id}"],
)
category1.update!(parent_category: nil) category1.update!(parent_category: nil)
end end
it "does not return the category if the parent does not match the child" do it "does not return the category if the parent does not match the child" do
parent_category = Fabricate(:category, name: "Media", slug: "media") parent_category = Fabricate(:category, name: "Media", slug: "media")
category1.update!(parent_category: parent_category) 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 expect(result[:category]).to be_empty
end end
it "for slugs without a type suffix it falls back in type order until a result is found or types are exhausted" do 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]) result = subject.lookup(%w[the-book-club great-books fiction-books], %w[category tag])
expect(result[:category].map(&:slug)).to eq(["book-club"]) expect(result[:category].map(&:slug)).to eq(["the-book-club"])
expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) 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(&:slug)).to eq(%w[fiction-books great-books])
expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/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") category2 = Fabricate(:category, name: "Great Books", slug: "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[:category].map(&:slug)).to eq(%w[book-club great-books]) expect(result[:category].map(&:slug)).to eq(%w[great-books the-book-club])
expect(result[:category].map(&:relative_url)).to eq( 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(&:slug)).to eq(%w[fiction-books])
expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books]) expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books])
category1.destroy! category1.destroy!
Fabricate(:tag, name: "book-club") Fabricate(:tag, name: "the-book-club")
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].map(&:slug)).to eq(["great-books"]) expect(result[:category].map(&:slug)).to eq(["great-books"])
expect(result[:category].map(&:relative_url)).to eq(["/c/great-books/#{category2.id}"]) 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(&:slug)).to eq(%w[fiction-books the-book-club])
expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/book-club /tag/fiction-books]) 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[: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( 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 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 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) category1.update!(read_restricted: true)
Fabricate(:tag, name: "book-club") Fabricate(:tag, name: "the-book-club")
result = subject.lookup(%w[book-club::tag book-club], %w[category tag]) result = subject.lookup(%w[the-book-club::tag the-book-club], %w[category tag])
expect(result[:category].map(&:ref)).to eq([]) 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 end
context "when not tagging_enabled" do context "when not tagging_enabled" do
before { SiteSetting.tagging_enabled = false } before { SiteSetting.tagging_enabled = false }
it "does not return tag" do it "does not return tag" do
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].map(&:slug)).to eq(["book-club"]) expect(result[:category].map(&:slug)).to eq(["the-book-club"])
expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) expect(result[:category].map(&:relative_url)).to eq(["/c/the-book-club/#{category1.id}"])
expect(result[:tag]).to eq([]) expect(result[:tag]).to eq([])
end end
end end