FEATURE: Generic hashtag autocomplete lookup and markdown cooking (#18937)

This commit fleshes out and adds functionality for the new `#hashtag` search and
lookup system, still hidden behind the `enable_experimental_hashtag_autocomplete`
feature flag.

**Serverside**

We have two plugin API registration methods that are used to define data sources
(`register_hashtag_data_source`) and hashtag result type priorities depending on
the context (`register_hashtag_type_in_context`). Reading the comments in plugin.rb
should make it clear what these are doing. Reading the `HashtagAutocompleteService`
in full will likely help a lot as well.

Each data source is responsible for providing its own **lookup** and **search**
method that returns hashtag results based on the arguments provided. For example,
the category hashtag data source has to take into account parent categories and
how they relate, and each data source has to define their own icon to use for the
hashtag, and so on.

The `Site` serializer has two new attributes that source data from `HashtagAutocompleteService`.
There is `hashtag_icons` that is just a simple array of all the different icons that
can be used for allowlisting in our markdown pipeline, and there is `hashtag_context_configurations`
that is used to store the type priority orders for each registered context.

When sending emails, we cannot render the SVG icons for hashtags, so
we need to change the HTML hashtags to the normal `#hashtag` text.

**Markdown**

The `hashtag-autocomplete.js` file is where I have added the new `hashtag-autocomplete`
markdown rule, and like all of our rules this is used to cook the raw text on both the clientside
and on the serverside using MiniRacer. Only on the server side do we actually reach out to
the database with the `hashtagLookup` function, on the clientside we just render a plainer
version of the hashtag HTML. Only in the composer preview do we do further lookups based
on this.

This rule is the first one (that I can find) that uses the `currentUser` based on a passed
in `user_id` for guardian checks in markdown rendering code. This is the `last_editor_id`
for both the post and chat message. In some cases we need to cook without a user present,
so the `Discourse.system_user` is used in this case.

**Chat Channels**

This also contains the changes required for chat so that chat channels can be used
as a data source for hashtag searches and lookups. This data source will only be
used when `enable_experimental_hashtag_autocomplete` is `true`, so we don't have
to worry about channel results suddenly turning up.

------

**Known Rough Edges**

- Onebox excerpts will not render the icon svg/use tags, I plan to address that in a follow up PR
- Selecting a hashtag + pressing the Quote button will result in weird behaviour, I plan to address that in a follow up PR
- Mixed hashtag contexts for hashtags without a type suffix will not work correctly, e.g. #ux which is both a category and a channel slug will resolve to a category when used inside a post or within a [chat] transcript in that post. Users can get around this manually by adding the correct suffix, for example ::channel. We may get to this at some point in future
- Icons will not show for the hashtags in emails since SVG support is so terrible in email (this is not likely to be resolved, but still noting for posterity)
- Additional refinements and review fixes wil
This commit is contained in:
Martin Brennan
2022-11-21 08:37:06 +10:00
committed by GitHub
parent a597ef7131
commit d3f02a1270
56 changed files with 1682 additions and 299 deletions

View File

@ -76,11 +76,11 @@ class ChatChannel < ActiveRecord::Base
end
def url
"#{Discourse.base_url}#{relative_url}"
"#{Discourse.base_url}/chat/channel/#{self.id}/#{self.slug || "-"}"
end
def relative_url
"/chat/channel/#{self.id}/#{self.slug || "-"}"
"#{Discourse.base_path}/chat/channel/#{self.id}/#{self.slug || "-"}"
end
private

View File

@ -33,7 +33,7 @@ class ChatMessage < ActiveRecord::Base
scope :created_before, ->(date) { where("chat_messages.created_at < ?", date) }
before_save { self.last_editor_id ||= self.user_id }
before_save { ensure_last_editor_id }
def validate_message(has_uploads:)
WatchedWordsValidator.new(attributes: [:message]).validate(self)
@ -98,7 +98,15 @@ class ChatMessage < ActiveRecord::Base
end
def cook
self.cooked = self.class.cook(self.message)
ensure_last_editor_id
# A rule in our Markdown pipeline may have Guardian checks that require a
# user to be present. The last editing user of the message will be more
# generally up to date than the creating user. For example, we use
# this when cooking #hashtags to determine whether we should render
# the found hashtag based on whether the user can access the channel it
# is referencing.
self.cooked = self.class.cook(self.message, user_id: self.last_editor_id)
self.cooked_version = BAKED_VERSION
end
@ -130,6 +138,7 @@ class ChatMessage < ActiveRecord::Base
emojiShortcuts
inlineEmoji
html-img
hashtag-autocomplete
mentions
unicodeUsernames
onebox
@ -164,6 +173,8 @@ class ChatMessage < ActiveRecord::Base
features_override: MARKDOWN_FEATURES + DiscoursePluginRegistry.chat_markdown_features.to_a,
markdown_it_rules: MARKDOWN_IT_RULES,
force_quote_link: true,
user_id: opts[:user_id],
hashtag_context: "chat-composer"
)
result =
@ -193,6 +204,10 @@ class ChatMessage < ActiveRecord::Base
def message_too_short?
message.length < SiteSetting.chat_minimum_message_length
end
def ensure_last_editor_id
self.last_editor_id ||= self.user_id
end
end
# == Schema Information

View File

@ -357,7 +357,7 @@ export default Component.extend(TextareaTextManipulation, {
_applyCategoryHashtagAutocomplete($textarea) {
setupHashtagAutocomplete(
"chat-composer",
this.site.hashtag_configurations["chat-composer"],
$textarea,
this.siteSettings,
(value) => {

View File

@ -43,11 +43,6 @@ export default {
});
}
if (this.siteSettings.enable_experimental_hashtag_autocomplete) {
api.registerHashtagSearchParam("category", "chat-composer", 100);
api.registerHashtagSearchParam("tag", "chat-composer", 50);
}
api.registerChatComposerButton({
label: "chat.emoji",
id: "emoji",

View File

@ -146,16 +146,19 @@ export default class Chat extends Service {
return Promise.resolve(this.cook);
}
const prettyTextFeatures = {
const markdownOptions = {
featuresOverride: Site.currentProp(
"markdown_additional_options.chat.limited_pretty_text_features"
),
markdownItRules: Site.currentProp(
"markdown_additional_options.chat.limited_pretty_text_markdown_rules"
),
hashtagTypesInPriorityOrder:
this.site.hashtag_configurations["chat-composer"],
hashtagIcons: this.site.hashtag_icons,
};
return generateCookFunction(prettyTextFeatures).then((cookFunction) => {
return generateCookFunction(markdownOptions).then((cookFunction) => {
return this.set("cook", (raw) => {
return simpleCategoryHashMentionTransform(
cookFunction(raw),

View File

@ -155,6 +155,7 @@ const chatTranscriptRule = {
// rendering chat message content with limited markdown rule subset
const token = state.push("html_raw", "", 1);
token.content = customMarkdownCookFn(content);
state.push("html_raw", "", -1);
@ -246,6 +247,10 @@ export function setup(helper) {
{
featuresOverride: chatAdditionalOpts.limited_pretty_text_features,
markdownItRules,
hashtagLookup: opts.discourse.hashtagLookup,
hashtagTypesInPriorityOrder:
chatAdditionalOpts.hashtag_configurations["chat-composer"],
hashtagIcons: opts.discourse.hashtagIcons,
},
(customCookFn) => {
customMarkdownCookFn = customCookFn;

View File

@ -76,13 +76,17 @@ module Chat::ChatChannelFetcher
channels = channels.where(status: options[:status]) if options[:status].present?
if options[:filter].present?
sql = "chat_channels.name ILIKE :filter OR categories.name ILIKE :filter"
sql = "chat_channels.name ILIKE :filter OR chat_channels.slug ILIKE :filter OR categories.name ILIKE :filter"
channels =
channels.where(sql, filter: "%#{options[:filter].downcase}%").order(
"chat_channels.name ASC, categories.name ASC",
)
end
if options.key?(:slugs)
channels = channels.where("chat_channels.slug IN (:slugs)", slugs: options[:slugs])
end
if options.key?(:following)
if options[:following]
channels =

View File

@ -0,0 +1,37 @@
# frozen_string_literal: true
class Chat::ChatChannelHashtagDataSource
def self.icon
"comment"
end
def self.channel_to_hashtag_item(guardian, channel)
HashtagAutocompleteService::HashtagItem.new.tap do |item|
item.text = channel.title(guardian.user)
item.slug = channel.slug
item.icon = icon
item.relative_url = channel.relative_url
item.type = "channel"
end
end
def self.lookup(guardian, slugs)
if SiteSetting.enable_experimental_hashtag_autocomplete
Chat::ChatChannelFetcher
.secured_public_channel_search(guardian, slugs: slugs)
.map { |channel| channel_to_hashtag_item(guardian, channel) }
else
[]
end
end
def self.search(guardian, term, limit)
if SiteSetting.enable_experimental_hashtag_autocomplete
Chat::ChatChannelFetcher
.secured_public_channel_search(guardian, filter: term, limit: limit)
.map { |channel| channel_to_hashtag_item(guardian, channel) }
else
[]
end
end
end

View File

@ -31,6 +31,7 @@ class Chat::ChatMessageCreator
ChatMessage.new(
chat_channel: @chat_channel,
user_id: @user.id,
last_editor_id: @user.id,
in_reply_to_id: @in_reply_to_id,
message: @content,
)

View File

@ -157,6 +157,7 @@ after_initialize do
load File.expand_path("../app/serializers/user_chat_message_bookmark_serializer.rb", __FILE__)
load File.expand_path("../app/serializers/reviewable_chat_message_serializer.rb", __FILE__)
load File.expand_path("../lib/chat_channel_fetcher.rb", __FILE__)
load File.expand_path("../lib/chat_channel_hashtag_data_source.rb", __FILE__)
load File.expand_path("../lib/chat_mailer.rb", __FILE__)
load File.expand_path("../lib/chat_message_creator.rb", __FILE__)
load File.expand_path("../lib/chat_message_processor.rb", __FILE__)
@ -228,10 +229,6 @@ after_initialize do
ReviewableScore.add_new_types([:needs_review])
Site.preloaded_category_custom_fields << Chat::HAS_CHAT_ENABLED
Site.markdown_additional_options["chat"] = {
limited_pretty_text_features: ChatMessage::MARKDOWN_FEATURES,
limited_pretty_text_markdown_rules: ChatMessage::MARKDOWN_IT_RULES,
}
Guardian.prepend Chat::GuardianExtensions
UserNotifications.prepend Chat::UserNotificationsExtension
@ -719,6 +716,19 @@ after_initialize do
register_about_stat_group("chat_channels") { Chat::Statistics.about_channels }
register_about_stat_group("chat_users") { Chat::Statistics.about_users }
# Make sure to update spec/system/hashtag_autocomplete_spec.rb when changing this.
register_hashtag_data_source("channel", Chat::ChatChannelHashtagDataSource)
register_hashtag_type_in_context("channel", "chat-composer", 200)
register_hashtag_type_in_context("category", "chat-composer", 100)
register_hashtag_type_in_context("tag", "chat-composer", 50)
register_hashtag_type_in_context("channel", "topic-composer", 10)
Site.markdown_additional_options["chat"] = {
limited_pretty_text_features: ChatMessage::MARKDOWN_FEATURES,
limited_pretty_text_markdown_rules: ChatMessage::MARKDOWN_IT_RULES,
hashtag_configurations: HashtagAutocompleteService.contexts_with_ordered_types,
}
end
if Rails.env == "test"

View File

@ -3,7 +3,7 @@
describe Chat::ChatChannelFetcher do
fab!(:category) { Fabricate(:category, name: "support") }
fab!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) }
fab!(:category_channel) { Fabricate(:category_channel, chatable: category) }
fab!(:category_channel) { Fabricate(:category_channel, chatable: category, slug: "support") }
fab!(:dm_channel1) { Fabricate(:direct_message) }
fab!(:dm_channel2) { Fabricate(:direct_message) }
fab!(:direct_message_channel1) { Fabricate(:direct_message_channel, chatable: dm_channel1) }
@ -170,6 +170,26 @@ describe Chat::ChatChannelFetcher do
).to match_array([category_channel.id])
end
it "can filter by an array of slugs" do
expect(
subject.secured_public_channels(
guardian,
memberships,
slugs: ["support"],
).map(&:id),
).to match_array([category_channel.id])
end
it "returns nothing if the array of slugs is empty" do
expect(
subject.secured_public_channels(
guardian,
memberships,
slugs: [],
).map(&:id),
).to eq([])
end
it "can filter by status" do
expect(
subject.secured_public_channels(guardian, memberships, status: "closed").map(&:id),

View File

@ -0,0 +1,116 @@
# frozen_string_literal: true
RSpec.describe Chat::ChatChannelHashtagDataSource do
fab!(:user) { Fabricate(:user) }
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) }
fab!(:channel2) do
Fabricate(:chat_channel, slug: "secret", name: "Secret Stuff", chatable: private_category)
end
let!(:guardian) { Guardian.new(user) }
before { SiteSetting.enable_experimental_hashtag_autocomplete = true }
describe "#lookup" do
it "finds a channel by a slug" do
result = described_class.lookup(guardian, ["random"]).first
expect(result.to_h).to eq(
{
relative_url: channel1.relative_url,
text: "Zany Things",
icon: "comment",
type: "channel",
ref: nil,
slug: "random",
},
)
end
it "does not return a channel that a user does not have permission to view" do
result = described_class.lookup(guardian, ["secret"]).first
expect(result).to eq(nil)
GroupUser.create(user: user, group: group)
result = described_class.lookup(Guardian.new(user), ["secret"]).first
expect(result.to_h).to eq(
{
relative_url: channel2.relative_url,
text: "Secret Stuff",
icon: "comment",
type: "channel",
ref: nil,
slug: "secret",
},
)
end
it "returns nothing if the slugs array is empty" do
result = described_class.lookup(guardian, []).first
expect(result).to eq(nil)
end
end
describe "#search" do
it "finds a channel by category name" do
category.update!(name: "Randomizer")
result = described_class.search(guardian, "randomiz", 10).first
expect(result.to_h).to eq(
{
relative_url: channel1.relative_url,
text: "Zany Things",
icon: "comment",
type: "channel",
ref: nil,
slug: "random",
},
)
end
it "finds a channel by slug" do
result = described_class.search(guardian, "rand", 10).first
expect(result.to_h).to eq(
{
relative_url: channel1.relative_url,
text: "Zany Things",
icon: "comment",
type: "channel",
ref: nil,
slug: "random",
},
)
end
it "finds a channel by channel name" do
result = described_class.search(guardian, "aNY t", 10).first
expect(result.to_h).to eq(
{
relative_url: channel1.relative_url,
text: "Zany Things",
icon: "comment",
type: "channel",
ref: nil,
slug: "random",
},
)
end
it "does not return channels the user does not have permission to view" do
result = described_class.search(guardian, "Sec", 10).first
expect(result).to eq(nil)
GroupUser.create(user: user, group: group)
result = described_class.search(Guardian.new(user), "Sec", 10).first
expect(result.to_h).to eq(
{
relative_url: channel2.relative_url,
text: "Secret Stuff",
icon: "comment",
type: "channel",
ref: nil,
slug: "secret",
},
)
end
end
end

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
module ChatSystemHelpers
def chat_system_bootstrap(user, channels_for_membership = [])
# ensures we have one valid registered admin/user
user.activate
SiteSetting.chat_enabled = true
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
channels_for_membership.each do |channel|
membership = channel.add(user)
if channel.chat_messages.any?
membership.update!(last_read_message_id: channel.chat_messages.last.id)
end
end
Group.refresh_automatic_groups!
end
end
RSpec.configure do |config|
config.include ChatSystemHelpers, type: :system
end

View File

@ -0,0 +1,89 @@
# frozen_string_literal: true
describe "Using #hashtag autocompletion to search for and lookup channels",
type: :system,
js: true do
fab!(:user) { Fabricate(:user) }
fab!(:channel1) { Fabricate(:chat_channel, name: "Music Lounge", slug: "music") }
fab!(:channel2) { Fabricate(:chat_channel, name: "Random", slug: "random") }
fab!(:category) { Fabricate(:category, name: "Raspberry", slug: "raspberry-beret") }
fab!(:tag) { Fabricate(:tag, name: "razed") }
fab!(:topic) { Fabricate(:topic) }
fab!(:post) { Fabricate(:post, topic: topic) }
fab!(:message1) { Fabricate(:chat_message, chat_channel: channel1) }
let(:chat_page) { PageObjects::Pages::Chat.new }
let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new }
let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new }
let(:topic_page) { PageObjects::Pages::Topic.new }
before do
SiteSetting.enable_experimental_hashtag_autocomplete = true
# This is annoying, but we need to reset the hashtag data sources inbetween
# tests, and since this is normally done in plugin.rb with the plugin API
# there is not an easier way to do this.
HashtagAutocompleteService.register_data_source("channel", Chat::ChatChannelHashtagDataSource)
HashtagAutocompleteService.register_type_in_context("channel", "chat-composer", 200)
HashtagAutocompleteService.register_type_in_context("category", "chat-composer", 100)
HashtagAutocompleteService.register_type_in_context("tag", "chat-composer", 50)
HashtagAutocompleteService.register_type_in_context("channel", "topic-composer", 10)
chat_system_bootstrap(user, [channel1, channel2])
sign_in(user)
end
it "searches for channels, categories, and tags with # and prioritises channels in the results" do
chat_page.visit_channel(channel1)
expect(chat_channel_page).to have_no_loading_skeleton
chat_channel_page.type_in_composer("this is #ra")
expect(page).to have_css(
".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link",
count: 3,
)
hashtag_results = page.all(".hashtag-autocomplete__link", count: 3)
expect(hashtag_results.map(&:text)).to eq(["Random", "Raspberry", "razed x 0"])
end
it "searches for channels as well with # in a topic composer and deprioritises them" do
topic_page.visit_topic_and_open_composer(topic)
expect(topic_page).to have_expanded_composer
topic_page.type_in_composer("something #ra")
expect(page).to have_css(
".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link",
count: 3,
)
hashtag_results = page.all(".hashtag-autocomplete__link", count: 3)
expect(hashtag_results.map(&:text)).to eq(["Raspberry", "razed x 0", "Random"])
end
# TODO (martin) Commenting this out for now, we need to add the MessageBus
# last_message_id to our chat subscriptions in JS for this to work, since it
# relies on a MessageBus "sent" event to be published to substitute the
# staged message ID for the real one.
xit "cooks the hashtags for channels, categories, and tags serverside when the chat message is saved to the database" do
chat_page.visit_channel(channel1)
expect(chat_channel_page).to have_no_loading_skeleton
chat_channel_page.type_in_composer("this is #random and this is #raspberry and this is #razed which is cool")
chat_channel_page.click_send_message
try_until_success do
expect(ChatMessage.exists?(user: user, message: "this is #random and this is #raspberry and this is #razed which is cool")).to eq(true)
end
message = ChatMessage.where(user: user).last
expect(chat_channel_page).to have_message(id: message.id)
within chat_channel_page.message_by_id(message.id) do
cooked_hashtags = page.all(".hashtag-cooked", count: 3)
expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp)
<a class=\"hashtag-cooked\" href=\"#{channel1.relative_url}\" data-type=\"channel\" data-slug=\"random\"><span><svg class=\"fa d-icon d-icon-comment svg-icon svg-node\"><use href=\"#comment\"></use></svg>Random</span></a>
HTML
expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp)
<a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"raspberry\"><span><svg class=\"fa d-icon d-icon-folder svg-icon svg-node\"><use href=\"#folder\"></use></svg>raspberry</span></a>
HTML
expect(cooked_hashtags[2]["outerHTML"]).to eq(<<~HTML.chomp)
<a class=\"hashtag-cooked\" href=\"#{tag.url}\" data-type=\"tag\" data-slug=\"razed\"><span><svg class=\"fa d-icon d-icon-tag svg-icon svg-node\"><use href=\"#tag\"></use></svg>razed</span></a>
HTML
end
end
end

View File

@ -13,14 +13,7 @@ RSpec.describe "Navigation", type: :system, js: true do
let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new }
before do
# ensures we have one valid registered admin
user.activate
SiteSetting.chat_enabled = true
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
category_channel.add(user)
category_channel_2.add(user)
chat_system_bootstrap(user, [category_channel, category_channel_2])
sign_in(user)
end

View File

@ -11,6 +11,10 @@ module PageObjects
visit("/chat")
end
def visit_channel(channel)
visit(channel.url)
end
def minimize_full_page
find(".open-drawer-btn").click
end

View File

@ -0,0 +1,35 @@
# frozen_string_literal: true
module PageObjects
module Pages
class ChatChannel < PageObjects::Pages::Base
def type_in_composer(input)
find(".chat-composer-input").send_keys(input)
end
def fill_composer(input)
find(".chat-composer-input").fill_in(with: input)
end
def click_send_message
find(".chat-composer .send-btn").click
end
def message_by_id(id)
find(".chat-message-container[data-id=\"#{id}\"]")
end
def has_no_loading_skeleton?
has_no_css?(".chat-skeleton")
end
def has_message?(text: nil, id: nil)
if text
has_css?(".chat-message-text", text: text)
elsif id
has_css?(".chat-message-container[data-id=\"#{id}\"]", wait: 10)
end
end
end
end
end

View File

@ -164,6 +164,9 @@ function buildAdditionalOptions() {
"blockquote",
"emphasis",
],
hashtag_configurations: {
"chat-composer": ["channel", "category", "tag"],
},
},
};
}