FEATURE: Allow showing hashtag autocomplete results without term (#19219)

This commit allows us to type # in the UI and present autocomplete
results immediately with the following logic for the topic composer,
and reversed for the chat composer:

* Categories the user can access and has not muted sorted by `topic_count`
* Tags the user can access and has not muted sorted by `topic_count`
* Chat channels the user is a member of sorted by `messages_count`

So in effect, we allow searching for hashtags without a search term.
To do this we add a new `search_without_term` to each data source so
each one can define how it wants to handle this logic.
This commit is contained in:
Martin Brennan 2022-12-08 13:47:59 +10:00 committed by GitHub
parent fde9e6bc25
commit 3fdb8ffb57
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 292 additions and 36 deletions

View File

@ -167,6 +167,9 @@ function _updateSearchCache(term, results) {
return results;
}
// Note that the search term is _not_ required here, and we follow special
// logic similar to @mentions when there is no search term, to show some
// useful default categories, tags, etc.
function _searchGeneric(term, siteSettings, contextualHashtagConfiguration) {
if (currentSearch) {
currentSearch.abort();
@ -187,7 +190,7 @@ function _searchGeneric(term, siteSettings, contextualHashtagConfiguration) {
resolve(CANCELLED_STATUS);
}, 5000);
if (term === "") {
if (!siteSettings.enable_experimental_hashtag_autocomplete && term === "") {
return resolve(CANCELLED_STATUS);
}

View File

@ -12,7 +12,6 @@ class HashtagsController < ApplicationController
end
def search
params.require(:term)
params.require(:order)
results = HashtagAutocompleteService.new(guardian).search(params[:term], params[:order])

View File

@ -8,7 +8,7 @@ class CategoryHashtagDataSource
"folder"
end
def self.category_to_hashtag_item(guardian_categories, category)
def self.category_to_hashtag_item(parent_category, category)
category = Category.new(category.slice(:id, :slug, :name, :parent_category_id, :description))
HashtagAutocompleteService::HashtagItem.new.tap do |item|
@ -22,8 +22,6 @@ class CategoryHashtagDataSource
# categories here.
item.ref =
if category.parent_category_id
parent_category =
guardian_categories.find { |cat| cat[:id] === category.parent_category_id }
!parent_category ? category.slug : "#{parent_category[:slug]}:#{category.slug}"
else
category.slug
@ -37,7 +35,11 @@ class CategoryHashtagDataSource
guardian_categories = Site.new(guardian).categories
Category
.query_from_cached_categories(slugs, guardian_categories)
.map { |category| category_to_hashtag_item(guardian_categories, category) }
.map do |category|
parent_category =
guardian_categories.find { |cat| cat[:id] == category[:parent_category_id] }
category_to_hashtag_item(parent_category, category)
end
end
def self.search(guardian, term, limit)
@ -48,15 +50,35 @@ class CategoryHashtagDataSource
category[:name].downcase.include?(term) || category[:slug].downcase.include?(term)
end
.take(limit)
.map { |category| category_to_hashtag_item(guardian_categories, category) }
.map do |category|
parent_category =
guardian_categories.find { |cat| cat[:id] == category[:parent_category_id] }
category_to_hashtag_item(parent_category, category)
end
end
def self.search_sort(search_results, term)
search_results
.select { |item| item.slug == term }
.sort_by { |item| item.text.downcase }
.concat(
search_results.select { |item| item.slug != term }.sort_by { |item| item.text.downcase },
if term.present?
search_results.sort_by { |item| [item.slug == term ? 0 : 1, item.text.downcase] }
else
search_results.sort_by { |item| item.text.downcase }
end
end
def self.search_without_term(guardian, limit)
Category
.includes(:parent_category)
.secured(guardian)
.joins(
"LEFT JOIN category_users ON category_users.user_id = #{guardian.user.id}
AND category_users.category_id = categories.id",
)
.where(
"category_users.notification_level IS NULL OR category_users.notification_level != ?",
CategoryUser.notification_levels[:muted],
)
.order(topic_count: :desc)
.take(limit)
.map { |category| category_to_hashtag_item(category.parent_category, category) }
end
end

View File

@ -188,6 +188,8 @@ class HashtagAutocompleteService
raise Discourse::InvalidParameters.new(:order) if !types_in_priority_order.is_a?(Array)
limit = [limit, SEARCH_MAX_LIMIT].min
return search_without_term(types_in_priority_order, limit) if term.blank?
limited_results = []
top_ranked_type = nil
term = term.downcase
@ -295,6 +297,23 @@ class HashtagAutocompleteService
private
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)
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.
limited_results.concat(set_types(set_refs(search_results), type))
end
limited_results.take(limit)
end
# Sometimes a specific ref is required, e.g. for categories that have
# a parent their ref will be parent_slug:child_slug, though most of the
# time it will be the same as the slug. The ref can then be used for
@ -303,12 +322,16 @@ class HashtagAutocompleteService
hashtag_items.each { |item| item.ref ||= item.slug }
end
def set_types(hashtag_items, type)
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? }
end
def search_for_type(type, guardian, term, limit)
set_refs(@@data_sources[type].search(guardian, term, limit)).each { |item| item.type = type }
set_types(set_refs(@@data_sources[type].search(guardian, term, limit)), type)
end
def execute_lookup!(lookup_results, type, guardian, slugs)
@ -324,6 +347,6 @@ class HashtagAutocompleteService
end
def lookup_for_type(type, guardian, slugs)
set_refs(@@data_sources[type].lookup(guardian, slugs)).each { |item| item.type = type }
set_types(set_refs(@@data_sources[type].lookup(guardian, slugs)), type)
end
end

View File

@ -55,4 +55,23 @@ class TagHashtagDataSource
def self.search_sort(search_results, _)
search_results.sort_by { |result| result.text.downcase }
end
def self.search_without_term(guardian, limit)
return [] if !SiteSetting.tagging_enabled
tags_with_counts, _ =
DiscourseTagging.filter_allowed_tags(
guardian,
with_context: true,
limit: limit,
for_input: true,
order_popularity: true,
excluded_tag_names: DiscourseTagging.muted_tags(guardian.user),
)
TagsController
.tag_counts_json(tags_with_counts)
.take(limit)
.map { |tag| tag_to_hashtag_item(tag, include_count: true) }
end
end

View File

@ -271,6 +271,7 @@ module DiscourseTagging
# exclude_synonyms: exclude synonyms from results
# order_search_results: result should be ordered for name search results
# order_popularity: order result by topic_count
# excluded_tag_names: an array of tag names not to include in the results
def self.filter_allowed_tags(guardian, opts = {})
selected_tag_ids = opts[:selected_tags] ? Tag.where_name(opts[:selected_tags]).pluck(:id) : []
category = opts[:category]
@ -427,6 +428,10 @@ module DiscourseTagging
builder.where("id NOT IN (SELECT target_tag_id FROM tags WHERE target_tag_id IS NOT NULL)")
end
if opts[:excluded_tag_names]&.any?
builder.where("name NOT IN (?)", opts[:excluded_tag_names])
end
if opts[:limit]
if required_tag_ids && term.blank?
# override limit so all required tags are shown by default

View File

@ -44,4 +44,27 @@ class Chat::ChatChannelHashtagDataSource
def self.search_sort(search_results, _)
search_results.sort_by { |result| result.text.downcase }
end
def self.search_without_term(guardian, limit)
if SiteSetting.enable_experimental_hashtag_autocomplete
allowed_channel_ids_sql =
Chat::ChatChannelFetcher.generate_allowed_channel_ids_sql(
guardian,
exclude_dm_channels: true,
)
ChatChannel
.joins(
"INNER JOIN user_chat_channel_memberships
ON user_chat_channel_memberships.chat_channel_id = chat_channels.id
AND user_chat_channel_memberships.user_id = #{guardian.user.id}
AND user_chat_channel_memberships.following = true",
)
.where("chat_channels.id IN (#{allowed_channel_ids_sql})")
.order(messages_count: :desc)
.limit(limit)
.map { |channel| channel_to_hashtag_item(guardian, channel) }
else
[]
end
end
end

View File

@ -5,9 +5,24 @@ RSpec.describe Chat::ChatChannelHashtagDataSource do
fab!(:category) { Fabricate(:category) }
fab!(:group) { Fabricate(:group) }
fab!(:private_category) { Fabricate(:private_category, group: group) }
fab!(:channel1) { Fabricate(:chat_channel, slug: "random", name: "Zany Things", chatable: category, description: "Just weird stuff") }
fab!(:channel1) do
Fabricate(
:chat_channel,
slug: "random",
name: "Zany Things",
chatable: category,
description: "Just weird stuff",
messages_count: 245,
)
end
fab!(:channel2) do
Fabricate(:chat_channel, slug: "secret", name: "Secret Stuff", chatable: private_category)
Fabricate(
:chat_channel,
slug: "secret",
name: "Secret Stuff",
chatable: private_category,
messages_count: 78,
)
end
let!(:guardian) { Guardian.new(user) }
@ -109,4 +124,41 @@ RSpec.describe Chat::ChatChannelHashtagDataSource do
)
end
end
describe "#search_without_term" do
fab!(:channel3) { Fabricate(:chat_channel, slug: "general", messages_count: 24) }
fab!(:channel4) { Fabricate(:chat_channel, slug: "chat", messages_count: 435) }
fab!(:channel5) { Fabricate(:chat_channel, slug: "code-review", messages_count: 334) }
fab!(:membership2) do
Fabricate(:user_chat_channel_membership, user: user, chat_channel: channel1)
end
fab!(:membership2) do
Fabricate(:user_chat_channel_membership, user: user, chat_channel: channel2)
end
fab!(:membership3) do
Fabricate(:user_chat_channel_membership, user: user, chat_channel: channel3)
end
fab!(:membership4) do
Fabricate(:user_chat_channel_membership, user: user, chat_channel: channel4)
end
fab!(:membership5) do
Fabricate(:user_chat_channel_membership, user: user, chat_channel: channel5)
end
it "returns distinct channels for messages that have been recently created in the past 2 weeks" do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq(
%w[chat code-review random general],
)
end
it "does not return channels the user does not have permission to view" do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("secret")
end
it "does not return channels where the user is not following the channel via user_chat_channel_memberships" do
membership5.destroy
membership3.update!(following: false)
expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq(%w[chat random])
end
end
end

View File

@ -0,0 +1,38 @@
# frozen_string_literal: true
RSpec.describe CategoryHashtagDataSource do
fab!(:category1) { Fabricate(:category, slug: "random", topic_count: 12) }
fab!(:category2) { Fabricate(:category, slug: "books", topic_count: 566) }
fab!(:category3) { Fabricate(:category, slug: "movies", topic_count: 245) }
fab!(:group) { Fabricate(:group) }
fab!(:category4) { Fabricate(:private_category, slug: "secret", group: group, topic_count: 40) }
fab!(:category5) { Fabricate(:category, slug: "casual", topic_count: 99) }
fab!(:user) { Fabricate(:user) }
let(:guardian) { Guardian.new(user) }
let(:uncategorized_category) { Category.find(SiteSetting.uncategorized_category_id) }
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", "#{uncategorized_category.slug}"],
)
end
it "does not return categories the user does not have permission to view" do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("secret")
group.add(user)
expect(described_class.search_without_term(Guardian.new(user), 5).map(&:slug)).to include(
"secret",
)
end
it "does not return categories the user has muted" do
CategoryUser.create!(
user: user,
category: category1,
notification_level: CategoryUser.notification_levels[:muted],
)
expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("random")
end
end
end

View File

@ -3,7 +3,8 @@
RSpec.describe HashtagAutocompleteService do
fab!(:user) { Fabricate(:user) }
fab!(:category1) { Fabricate(:category, name: "Book Club", slug: "book-club") }
fab!(:tag1) { Fabricate(:tag, name: "great-books") }
fab!(:tag1) { Fabricate(:tag, name: "great-books", topic_count: 22) }
fab!(:topic1) { Fabricate(:topic) }
let(:guardian) { Guardian.new(user) }
subject { described_class.new(guardian) }
@ -71,19 +72,19 @@ 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 0"],
["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 0", "Book Club"],
["great-books x 22", "Book Club"],
)
end
it "respects the limit param" do
expect(subject.search("book", %w[tag category], limit: 1).map(&:text)).to eq(
["great-books x 0"],
["great-books x 22"],
)
end
@ -111,10 +112,10 @@ RSpec.describe HashtagAutocompleteService do
it "does case-insensitive search" do
expect(subject.search("book", %w[category tag]).map(&:text)).to eq(
["Book Club", "great-books x 0"],
["Book Club", "great-books x 22"],
)
expect(subject.search("bOOk", %w[category tag]).map(&:text)).to eq(
["Book Club", "great-books x 0"],
["Book Club", "great-books x 22"],
)
end
@ -125,7 +126,7 @@ RSpec.describe HashtagAutocompleteService do
it "does not include categories the user cannot access" do
category1.update!(read_restricted: true)
expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books x 0"])
expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books x 22"])
end
it "does not include tags the user cannot access" do
@ -141,7 +142,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 0", "read review of this fantasy book"],
["Book Club", "great-books x 22", "read review of this fantasy book"],
)
end
@ -235,6 +236,38 @@ RSpec.describe HashtagAutocompleteService do
expect(subject.search("book", %w[category tag]).map(&:text)).to eq(["Book Club"])
end
end
context "when no term is provided (default results) triggered by a # with no characters in the UI" do
fab!(:category2) do
Fabricate(:category, name: "Book Zone", slug: "book-zone", topic_count: 546)
end
fab!(:category3) do
Fabricate(:category, name: "Book Dome", slug: "book-dome", topic_count: 987)
end
fab!(:category4) { Fabricate(:category, name: "Bookworld", slug: "book", topic_count: 56) }
fab!(:category5) { Fabricate(:category, name: "Media", slug: "media", topic_count: 446) }
fab!(:tag2) { Fabricate(:tag, name: "mid-books", topic_count: 33) }
fab!(:tag3) { Fabricate(:tag, name: "terrible-books", topic_count: 2) }
fab!(:tag4) { Fabricate(:tag, name: "book", topic_count: 1) }
it "returns the 'most polular' categories and tags (based on topic_count) that the user can access" do
category1.update!(read_restricted: true)
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["terrible-books"])
expect(subject.search(nil, %w[category tag]).map(&:text)).to eq(
[
"Book Dome",
"Book Zone",
"Media",
"Bookworld",
Category.find(SiteSetting.uncategorized_category_id).name,
"mid-books x 33",
"great-books x 22",
"book x 1",
],
)
end
end
end
describe "#lookup_old" do

View File

@ -1,11 +1,11 @@
# frozen_string_literal: true
RSpec.describe TagHashtagDataSource do
fab!(:tag1) { Fabricate(:tag, name: "fact") }
fab!(:tag1) { Fabricate(:tag, name: "fact", topic_count: 0) }
fab!(:tag2) { Fabricate(:tag, name: "factor", topic_count: 5) }
fab!(:tag3) { Fabricate(:tag, name: "factory", topic_count: 1) }
fab!(:tag4) { Fabricate(:tag, name: "factorio") }
fab!(:tag5) { Fabricate(:tag, name: "factz") }
fab!(:tag3) { Fabricate(:tag, name: "factory", topic_count: 4) }
fab!(:tag4) { Fabricate(:tag, name: "factorio", topic_count: 3) }
fab!(:tag5) { Fabricate(:tag, name: "factz", topic_count: 1) }
fab!(:user) { Fabricate(:user) }
let(:guardian) { Guardian.new(user) }
@ -33,7 +33,7 @@ RSpec.describe TagHashtagDataSource do
it "includes the topic count for the text of the tag" do
expect(described_class.search(guardian, "fact", 5).map(&:text)).to eq(
["fact x 0", "factor x 5", "factory x 1", "factorio x 0", "factz x 0"],
["fact x 0", "factor x 5", "factory x 4", "factorio x 3", "factz x 1"],
)
end
@ -42,4 +42,22 @@ RSpec.describe TagHashtagDataSource do
expect(described_class.search(guardian, "fact", 5)).to be_empty
end
end
describe "#search_without_term" do
it "returns distinct tags sorted by topic_count" do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq(
%w[factor factory factorio factz fact],
)
end
it "does not return tags the user does not have permission to view" do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["factor"])
expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("factor")
end
it "does not return tags the user has muted" do
TagUser.create(user: user, tag: tag2, notification_level: TagUser.notification_levels[:muted])
expect(described_class.search_without_term(guardian, 5).map(&:slug)).not_to include("factor")
end
end
end

View File

@ -4,10 +4,17 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
type: :system,
js: true do
fab!(:user) { Fabricate(:user) }
fab!(:topic) { Fabricate(:topic) }
fab!(:category) do
Fabricate(:category, name: "Cool Category", slug: "cool-cat", topic_count: 3234)
end
fab!(:category2) do
Fabricate(:category, name: "Other Category", slug: "other-cat", topic_count: 23)
end
fab!(:tag) { Fabricate(:tag, name: "cooltag", topic_count: 324) }
fab!(:tag2) { Fabricate(:tag, name: "othertag", topic_count: 66) }
fab!(:topic) { Fabricate(:topic, category: category, tags: [tag]) }
fab!(:post) { Fabricate(:post, topic: topic) }
fab!(:category) { Fabricate(:category, name: "Cool Category", slug: "cool-cat") }
fab!(:tag) { Fabricate(:tag, name: "cooltag") }
let(:uncategorized_category) { Category.find(SiteSetting.uncategorized_category_id) }
let(:topic_page) { PageObjects::Pages::Topic.new }
before do
@ -15,20 +22,34 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
sign_in user
end
def visit_topic_and_initiate_autocomplete
def visit_topic_and_initiate_autocomplete(initiation_text: "something #co", expected_count: 2)
topic_page.visit_topic_and_open_composer(topic)
expect(topic_page).to have_expanded_composer
topic_page.type_in_composer("something #co")
topic_page.type_in_composer(initiation_text)
expect(page).to have_css(
".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link",
count: 2,
count: expected_count,
)
end
xit "searches for categories and tags with # and prioritises categories in the results" do
visit_topic_and_initiate_autocomplete
hashtag_results = page.all(".hashtag-autocomplete__link", count: 2)
expect(hashtag_results.map(&:text)).to eq(["Cool Category", "cooltag x 0"])
expect(hashtag_results.map(&:text)).to eq(["Cool Category", "cooltag x 325"])
end
xit "begins showing results as soon as # is pressed based on categories and tags topic_count" do
visit_topic_and_initiate_autocomplete(initiation_text: "#", expected_count: 5)
hashtag_results = page.all(".hashtag-autocomplete__link")
expect(hashtag_results.map(&:text)).to eq(
[
"Cool Category",
"Other Category",
uncategorized_category.name,
"cooltag x 325",
"othertag x 66",
],
)
end
xit "cooks the selected hashtag clientside with the correct url and icon" do