FEATURE: Chat thread inline oneboxes (#30834)

Previously if you linked to a chat thread inline, our oneboxer
did not have special handling for this, so the link would end
up with the text "Chat #channel-name" which is not ideal for a
thread.

This commit makes it so the thread onebox is in the format
"Thread title - #channel-name" if the thread title exists,
otherwise we show "Thread in #channel-name"
This commit is contained in:
Martin Brennan 2025-01-20 10:08:38 +10:00 committed by GitHub
parent f4857d3b1c
commit dcb1e2a341
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 227 additions and 57 deletions

View File

@ -184,6 +184,8 @@ en:
inline_to_message: "Message #%{message_id} by %{username} – #%{chat_channel}"
inline_to_channel: "Chat #%{chat_channel}"
inline_to_topic_channel: "Chat for Topic %{topic_title}"
inline_to_thread: "%{thread_title} - #%{chat_channel}"
inline_to_thread_no_title: "Thread in #%{chat_channel}"
thread_title_connector: "in"
x_members:

View File

@ -0,0 +1,52 @@
# frozen_string_literal: true
module Chat
class InlineOneboxHandler
def self.handle(url, route)
if route[:message_id].present?
message = Chat::Message.find_by(id: route[:message_id])
return if !message
chat_channel = message.chat_channel
user = message.user
return if !chat_channel || !user
title =
I18n.t(
"chat.onebox.inline_to_message",
message_id: message.id,
chat_channel: chat_channel.name,
username: user.username,
)
else
chat_channel = Chat::Channel.find_by(id: route[:channel_id])
return if !chat_channel
if route[:thread_id].present?
thread = Chat::Thread.find_by(id: route[:thread_id])
return if !thread
title =
if thread.title.present?
I18n.t(
"chat.onebox.inline_to_thread",
chat_channel: chat_channel.name,
thread_title: thread.title,
)
else
I18n.t("chat.onebox.inline_to_thread_no_title", chat_channel: chat_channel.name)
end
else
title =
if chat_channel.name.present?
I18n.t("chat.onebox.inline_to_channel", chat_channel: chat_channel.name)
end
end
end
return if !Guardian.new.can_preview_chat_channel?(chat_channel)
{ url: url, title: title }
end
end
end

View File

@ -89,34 +89,7 @@ after_initialize do
if InlineOneboxer.respond_to?(:register_local_handler)
InlineOneboxer.register_local_handler("chat/chat") do |url, route|
if route[:message_id].present?
message = Chat::Message.find_by(id: route[:message_id])
next if !message
chat_channel = message.chat_channel
user = message.user
next if !chat_channel || !user
title =
I18n.t(
"chat.onebox.inline_to_message",
message_id: message.id,
chat_channel: chat_channel.name,
username: user.username,
)
else
chat_channel = Chat::Channel.find_by(id: route[:channel_id])
next if !chat_channel
title =
if chat_channel.name.present?
I18n.t("chat.onebox.inline_to_channel", chat_channel: chat_channel.name)
end
end
next if !Guardian.new.can_preview_chat_channel?(chat_channel)
{ url: url, title: title }
Chat::InlineOneboxHandler.handle(url, route)
end
end

View File

@ -0,0 +1,172 @@
# frozen_string_literal: true
describe Chat::InlineOneboxHandler do
fab!(:private_category_group) { Fabricate(:group) }
fab!(:private_category) { Fabricate(:private_category, group: private_category_group) }
fab!(:private_channel) { Fabricate(:category_channel, chatable: private_category) }
fab!(:public_channel) { Fabricate(:category_channel) }
fab!(:user)
fab!(:user_2) { Fabricate(:user, active: false) }
fab!(:user_3) { Fabricate(:user, staged: true) }
fab!(:user_4) { Fabricate(:user, suspended_till: 3.weeks.from_now) }
let(:public_chat_url) { "#{Discourse.base_url}/chat/c/-/#{public_channel.id}" }
let(:private_chat_url) { "#{Discourse.base_url}/chat/c/-/#{private_channel.id}" }
let(:invalid_chat_url) { "#{Discourse.base_url}/chat/c/-/999" }
context "when the link is to a public channel" do
describe "channel" do
it "renders an inline onebox for the channel" do
expect(
Chat::InlineOneboxHandler.handle(public_chat_url, { channel_id: public_channel.id }),
).to eq(
{
url: public_chat_url,
title: I18n.t("chat.onebox.inline_to_channel", chat_channel: public_channel.name),
},
)
end
it "does not render an inline onebox for a channel which does not exist" do
public_channel.trash!
expect(
Chat::InlineOneboxHandler.handle(public_chat_url, { channel_id: public_channel.id }),
).to be_nil
end
end
describe "message" do
fab!(:message) { Fabricate(:chat_message, chat_channel: public_channel) }
let(:public_chat_message_url) do
"#{Discourse.base_url}/chat/c/-/#{public_channel.id}/#{message.id}"
end
it "renders an inline onebox for a message" do
expect(
Chat::InlineOneboxHandler.handle(
public_chat_message_url,
{ channel_id: public_channel.id, message_id: message.id },
),
).to eq(
{
url: public_chat_message_url,
title:
I18n.t(
"chat.onebox.inline_to_message",
chat_channel: public_channel.name,
message_id: message.id,
username: message.user.username,
),
},
)
end
it "does not render an inline onebox for a message which does not exist" do
message.trash!
expect(
Chat::InlineOneboxHandler.handle(
public_chat_message_url,
{ channel_id: public_channel.id, message_id: message.id },
),
).to be_nil
end
end
describe "thread" do
fab!(:thread) do
Fabricate(:chat_thread, channel: public_channel, title: "Let's talk about some games")
end
let(:public_chat_thread_url) do
"#{Discourse.base_url}/chat/c/-/#{public_channel.id}/t/#{thread.id}"
end
it "renders an inline onebox for a thread" do
expect(
Chat::InlineOneboxHandler.handle(
public_chat_thread_url,
{ channel_id: public_channel.id, thread_id: thread.id },
),
).to eq(
{
url: public_chat_thread_url,
title:
I18n.t(
"chat.onebox.inline_to_thread",
chat_channel: public_channel.name,
thread_id: thread.id,
thread_title: thread.title,
),
},
)
end
it "renders an inline onebox for a thread with no title" do
thread.update!(title: nil)
expect(
Chat::InlineOneboxHandler.handle(
public_chat_thread_url,
{ channel_id: public_channel.id, thread_id: thread.id },
),
).to eq(
{
url: public_chat_thread_url,
title:
I18n.t(
"chat.onebox.inline_to_thread_no_title",
chat_channel: public_channel.name,
thread_id: thread.id,
thread_title: thread.title,
),
},
)
end
it "does not render an inline onebox for a thread which does not exist" do
thread.destroy!
expect(
Chat::InlineOneboxHandler.handle(
public_chat_thread_url,
{ channel_id: public_channel.id, thread_id: thread.id },
),
).to be_nil
end
end
end
context "when the link is to a private channel" do
fab!(:message) { Fabricate(:chat_message, chat_channel: private_channel) }
fab!(:thread) do
Fabricate(:chat_thread, channel: private_channel, title: "Let's talk about some games")
end
let(:private_chat_thread_url) do
"#{Discourse.base_url}/chat/c/-/#{private_channel.id}/t/#{thread.id}"
end
let(:private_chat_message_url) do
"#{Discourse.base_url}/chat/c/-/#{private_channel.id}/#{message.id}"
end
it "does not render an inline onebox for the channel for any users" do
expect(
Chat::InlineOneboxHandler.handle(private_chat_url, { channel_id: private_channel.id }),
).to be_nil
end
it "does not render an inline onebox for the channel message for any users" do
expect(
Chat::InlineOneboxHandler.handle(
private_chat_message_url,
{ channel_id: private_channel.id, message_id: message.id },
),
).to be_nil
end
it "does not render an inline onebox for the channel thread for any users" do
expect(
Chat::InlineOneboxHandler.handle(
private_chat_thread_url,
{ channel_id: private_channel.id, thread_id: thread.id },
),
).to be_nil
end
end
end

View File

@ -152,35 +152,6 @@ describe Chat do
end
end
describe "chat oneboxes" do
fab!(:chat_channel) { Fabricate(:category_channel) }
fab!(:user)
fab!(:chat_message) do
Fabricate(:chat_message, chat_channel: chat_channel, user: user, message: "Hello world!")
end
let(:chat_url) { "#{Discourse.base_url}/chat/c/-/#{chat_channel.id}" }
context "when inline" do
it "renders channel" do
results = InlineOneboxer.new([chat_url], skip_cache: true).process
expect(results).to be_present
expect(results[0][:url]).to eq(chat_url)
expect(results[0][:title]).to eq("Chat ##{chat_channel.name}")
end
it "renders messages" do
results = InlineOneboxer.new(["#{chat_url}/#{chat_message.id}"], skip_cache: true).process
expect(results).to be_present
expect(results[0][:url]).to eq("#{chat_url}/#{chat_message.id}")
expect(results[0][:title]).to eq(
"Message ##{chat_message.id} by #{chat_message.user.username}##{chat_channel.name}",
)
end
end
end
describe "auto-joining users to a channel" do
fab!(:chatters_group) { Fabricate(:group) }
fab!(:user) { Fabricate(:user, last_seen_at: 15.minutes.ago, trust_level: 1) }