DEV: Refactor chat oneboxes (#23031)

- moves the onebox logic away from `plugin.rb` to a new `onebox_handler` lib
- splits the `discourse_chat_message` template into two: one for channels, and one for messages
- refactors the logic code slightly to send only the necessary arguments to each template

This commit shouldn't change end-user behavior.
This commit is contained in:
Jan Cernik 2023-09-04 11:55:02 -03:00 committed by GitHub
parent 3ee77c29a5
commit aaf47c02bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 262 additions and 156 deletions

View File

@ -13,10 +13,18 @@ module ::Chat
SiteSetting.chat_allowed_groups_map
end
def self.onebox_template
@onebox_template ||=
def self.message_onebox_template
@message_onebox_template ||=
begin
path = "#{Rails.root}/plugins/chat/lib/onebox/templates/discourse_chat.mustache"
path = "#{Rails.root}/plugins/chat/lib/onebox/templates/discourse_chat_message.mustache"
File.read(path)
end
end
def self.channel_onebox_template
@channel_onebox_template ||=
begin
path = "#{Rails.root}/plugins/chat/lib/onebox/templates/discourse_chat_channel.mustache"
File.read(path)
end
end

View File

@ -0,0 +1,86 @@
# frozen_string_literal: true
module Chat
class OneboxHandler
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
else
chat_channel = Chat::Channel.find_by(id: route[:channel_id])
return if !chat_channel
end
return if !Guardian.new.can_preview_chat_channel?(chat_channel)
args = build_args(url, chat_channel)
if message.present?
render_message_onebox(args, message)
else
render_channel_onebox(args, chat_channel)
end
end
private
def self.build_args(url, chat_channel)
args = {
url: url,
channel_id: chat_channel.id,
channel_name: chat_channel.name,
is_category: chat_channel.category_channel?,
color: chat_channel.category_channel? ? chat_channel.chatable.color : nil,
}
end
def self.render_message_onebox(args, message)
args.merge!(
message_id: message.id,
username: message.user.username,
avatar_url: message.user.avatar_template_url.gsub("{size}", "20"),
cooked: message.cooked,
created_at: message.created_at,
created_at_str: message.created_at.iso8601,
)
Mustache.render(Chat.message_onebox_template, args)
end
def self.render_channel_onebox(args, chat_channel)
users = build_users_list(chat_channel)
remaining_user_count_str = build_remaining_user_count_str(chat_channel, users)
args.merge!(
users: users,
user_count_str: I18n.t("chat.onebox.x_members", count: chat_channel.user_count),
remaining_user_count_str: remaining_user_count_str,
description: chat_channel.description,
)
Mustache.render(Chat.channel_onebox_template, args)
end
def self.build_users_list(chat_channel)
Chat::ChannelMembershipsQuery
.call(channel: chat_channel, limit: 10)
.map do |membership|
{
username: membership.user.username,
avatar_url: membership.user.avatar_template_url.gsub("{size}", "60"),
}
end
end
def self.build_remaining_user_count_str(chat_channel, users)
if chat_channel.user_count > users.size
I18n.t("chat.onebox.and_x_others", count: chat_channel.user_count - users.size)
end
end
end
end

View File

@ -0,0 +1,26 @@
<aside class="onebox chat-onebox">
<article class="onebox-body chat-onebox-body">
<h3 class="chat-onebox-title">
<a href="{{url}}">
{{#is_category}}
<span class="category-chat-badge" style="color: #{{color}}">
<svg class="fa d-icon d-icon-d-chat svg-icon svg-string" xmlns="http://www.w3.org/2000/svg"><use href="#d-chat"></use></svg>
</span>
{{/is_category}}
<span class="clear-badge">{{{channel_name}}}</span>
</a>
</h3>
{{#description}}
<div class="chat-onebox-description">{{description}}</div>
{{/description}}
<div class="chat-onebox-members-count">{{user_count_str}}</div>
<div class="chat-onebox-members">
{{#users}}
<a class="trigger-user-card" data-user-card="{{username}}" aria-hidden="true" tabindex="-1">
<img loading="lazy" alt="{{username}}" width="30" height="30" src="{{avatar_url}}" class="avatar">
</a>
{{/users}}
{{remaining_user_count_str}}
</div>
</article>
</aside>

View File

@ -1,33 +1,3 @@
{{^cooked}}
<aside class="onebox chat-onebox">
<article class="onebox-body chat-onebox-body">
<h3 class="chat-onebox-title">
<a href="{{url}}">
{{#is_category}}
<span class="category-chat-badge" style="color: #{{color}}">
<svg class="fa d-icon d-icon-d-chat svg-icon svg-string" xmlns="http://www.w3.org/2000/svg"><use href="#d-chat"></use></svg>
</span>
{{/is_category}}
<span class="clear-badge">{{{channel_name}}}</span>
</a>
</h3>
{{#description}}
<div class="chat-onebox-description">{{description}}</div>
{{/description}}
<div class="chat-onebox-members-count">{{user_count_str}}</div>
<div class="chat-onebox-members">
{{#users}}
<a class="trigger-user-card" data-user-card="{{username}}" aria-hidden="true" tabindex="-1">
<img loading="lazy" alt="{{username}}" width="30" height="30" src="{{avatar_url}}" class="avatar">
</a>
{{/users}}
{{remaining_user_count_str}}
</div>
</article>
</aside>
{{/cooked}}
{{#cooked}}
<div class="chat-transcript" data-message-id="{{message_id}}" data-username="{{username}}" data-datetime="{{created_at_str}}" data-channel-name="{{channel_name}}" data-channel-id="{{channel_id}}">
<div class="chat-transcript-user">
<div class="chat-transcript-user-avatar">
@ -55,4 +25,3 @@
</div>
<div class="chat-transcript-messages">{{{cooked}}}</div>
</div>
{{/cooked}}

View File

@ -71,62 +71,7 @@ after_initialize do
if Oneboxer.respond_to?(:register_local_handler)
Oneboxer.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
else
chat_channel = Chat::Channel.find_by(id: route[:channel_id])
next if !chat_channel
end
next if !Guardian.new.can_preview_chat_channel?(chat_channel)
name = (chat_channel.name if chat_channel.name.present?)
users =
chat_channel
.user_chat_channel_memberships
.includes(:user)
.where(user: User.activated.not_suspended.not_staged)
.limit(10)
.map do |membership|
{
username: membership.user.username,
avatar_url: membership.user.avatar_template_url.gsub("{size}", "60"),
}
end
remaining_user_count_str =
if chat_channel.user_count > users.size
I18n.t("chat.onebox.and_x_others", count: chat_channel.user_count - users.size)
end
args = {
url: url,
channel_id: chat_channel.id,
channel_name: name,
description: chat_channel.description,
user_count_str: I18n.t("chat.onebox.x_members", count: chat_channel.user_count),
users: users,
remaining_user_count_str: remaining_user_count_str,
is_category: chat_channel.chatable_type == "Category",
color: chat_channel.chatable_type == "Category" ? chat_channel.chatable.color : nil,
}
if message.present?
args[:message_id] = message.id
args[:username] = message.user.username
args[:avatar_url] = message.user.avatar_template_url.gsub("{size}", "20")
args[:cooked] = message.cooked
args[:created_at] = message.created_at
args[:created_at_str] = message.created_at.iso8601
end
Mustache.render(Chat.onebox_template, args)
Chat::OneboxHandler.handle(url, route)
end
end

View File

@ -0,0 +1,136 @@
# frozen_string_literal: true
require "rails_helper"
describe Chat::OneboxHandler do
fab!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) }
fab!(:private_channel) { Fabricate(:category_channel, chatable: private_category) }
fab!(:public_channel) { Fabricate(:category_channel) }
fab!(:user) { Fabricate(: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" }
describe "chat channel" do
context "when valid" do
it "renders channel onebox, excluding inactive, staged, and suspended users" do
public_channel.add(user)
public_channel.add(user_2)
public_channel.add(user_3)
public_channel.add(user_4)
Chat::Channel.ensure_consistency!
onebox_html = Chat::OneboxHandler.handle(public_chat_url, { channel_id: public_channel.id })
expect(onebox_html).to match_html <<~HTML
<aside class="onebox chat-onebox">
<article class="onebox-body chat-onebox-body">
<h3 class="chat-onebox-title">
<a href="#{public_chat_url}">
<span class="category-chat-badge" style="color: ##{public_channel.chatable.color}">
<svg class="fa d-icon d-icon-d-chat svg-icon svg-string" xmlns="http://www.w3.org/2000/svg"><use href="#d-chat"></use></svg>
</span>
<span class="clear-badge">#{public_channel.name}</span>
</a>
</h3>
<div class="chat-onebox-members-count">1 member</div>
<div class="chat-onebox-members">
<a class="trigger-user-card" data-user-card="#{user.username}" aria-hidden="true" tabindex="-1">
<img loading="lazy" alt="#{user.username}" width="30" height="30" src="#{user.avatar_template_url.gsub("{size}", "60")}" class="avatar">
</a>
</div>
</article>
</aside>
HTML
end
end
context "when channel is private" do
it "does not create a onebox" do
onebox_html =
Chat::OneboxHandler.handle(private_chat_url, { channel_id: private_channel.id })
expect(onebox_html).to be_nil
end
end
context "when channel does not exists" do
it "does not raise an error" do
onebox_html = Chat::OneboxHandler.handle(invalid_chat_url, { channel_id: 999 })
expect(onebox_html).to be_nil
end
end
end
describe "chat message" do
fab!(:public_message) do
Fabricate(:chat_message, chat_channel: public_channel, user: user, message: "Hello world!")
end
fab!(:private_message) do
Fabricate(:chat_message, chat_channel: private_channel, user: user, message: "Hello world!")
end
context "when valid" do
it "renders message onebox" do
onebox_html =
Chat::OneboxHandler.handle(
"#{public_chat_url}/#{public_message.id}",
{ channel_id: public_channel.id, message_id: public_message.id },
)
expect(onebox_html).to match_html <<~HTML
<div class="chat-transcript" data-message-id="#{public_message.id}" data-username="#{user.username}" data-datetime="#{public_message.created_at.iso8601}" data-channel-name="#{public_channel.name}" data-channel-id="#{public_channel.id}">
<div class="chat-transcript-user">
<div class="chat-transcript-user-avatar">
<a class="trigger-user-card" data-user-card="#{user.username}" aria-hidden="true" tabindex="-1">
<img loading="lazy" alt="#{user.username}" width="20" height="20" src="#{user.avatar_template_url.gsub("{size}", "20")}" class="avatar">
</a>
</div>
<div class="chat-transcript-username">#{user.username}</div>
<div class="chat-transcript-datetime">
<a href="#{public_chat_url}/#{public_message.id}" title="#{public_message.created_at}">#{public_message.created_at}</a>
</div>
<a class="chat-transcript-channel" href="/chat/c/-/#{public_channel.id}">
<span class="category-chat-badge" style="color: ##{public_channel.chatable.color}">
<svg class="fa d-icon d-icon-d-chat svg-icon svg-string" xmlns="http://www.w3.org/2000/svg"><use href="#d-chat"></use></svg>
</span>
#{public_channel.name}
</a>
</div>
<div class="chat-transcript-messages"><p>Hello world!</p></div>
</div>
HTML
end
end
context "when channel is private" do
it "does not create a onebox" do
onebox_html =
Chat::OneboxHandler.handle(
"#{private_chat_url}/#{private_message.id}",
{ channel_id: private_channel.id, message_id: private_message.id },
)
expect(onebox_html).to be_nil
end
end
context "when message does not exists" do
it "does not raise an error" do
onebox_html =
Chat::OneboxHandler.handle(
"#{public_chat_url}/999",
{ channel_id: public_channel.id, message_id: 999 },
)
expect(onebox_html).to be_nil
end
end
end
end

View File

@ -127,22 +127,14 @@ describe Chat do
describe "chat oneboxes" do
fab!(:chat_channel) { Fabricate(:category_channel) }
fab!(:user) { Fabricate(:user, active: true) }
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) }
fab!(:user) { Fabricate(:user) }
let!(:chat_message) do
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}" }
before do
chat_channel.update!(last_message: chat_message)
chat_channel.add(user)
end
context "when inline" do
it "renders channel" do
results = InlineOneboxer.new([chat_url], skip_cache: true).process
@ -160,62 +152,6 @@ describe Chat do
)
end
end
context "when regular" do
it "renders channel, excluding inactive, staged, and suspended users" do
user_2.user_chat_channel_memberships.create!(chat_channel: chat_channel, following: true)
user_3.user_chat_channel_memberships.create!(chat_channel: chat_channel, following: true)
user_4.user_chat_channel_memberships.create!(chat_channel: chat_channel, following: true)
Chat::Channel.ensure_consistency!
expect(Oneboxer.preview(chat_url)).to match_html <<~HTML
<aside class="onebox chat-onebox">
<article class="onebox-body chat-onebox-body">
<h3 class="chat-onebox-title">
<a href="#{chat_url}">
<span class="category-chat-badge" style="color: ##{chat_channel.chatable.color}">
<svg class="fa d-icon d-icon-d-chat svg-icon svg-string" xmlns="http://www.w3.org/2000/svg"><use href="#d-chat"></use></svg>
</span>
<span class="clear-badge">#{chat_channel.name}</span>
</a>
</h3>
<div class="chat-onebox-members-count">1 member</div>
<div class="chat-onebox-members">
<a class="trigger-user-card" data-user-card="#{user.username}" aria-hidden="true" tabindex="-1">
<img loading="lazy" alt="#{user.username}" width="30" height="30" src="#{user.avatar_template_url.gsub("{size}", "60")}" class="avatar">
</a>
</div>
</article>
</aside>
HTML
end
it "renders messages" do
expect(Oneboxer.preview("#{chat_url}/#{chat_message.id}")).to match_html <<~HTML
<div class="chat-transcript" data-message-id="#{chat_message.id}" data-username="#{user.username}" data-datetime="#{chat_message.created_at.iso8601}" data-channel-name="#{chat_channel.name}" data-channel-id="#{chat_channel.id}">
<div class="chat-transcript-user">
<div class="chat-transcript-user-avatar">
<a class="trigger-user-card" data-user-card="#{user.username}" aria-hidden="true" tabindex="-1">
<img loading="lazy" alt="#{user.username}" width="20" height="20" src="#{user.avatar_template_url.gsub("{size}", "20")}" class="avatar">
</a>
</div>
<div class="chat-transcript-username">#{user.username}</div>
<div class="chat-transcript-datetime">
<a href="#{chat_url}/#{chat_message.id}" title="#{chat_message.created_at}">#{chat_message.created_at}</a>
</div>
<a class="chat-transcript-channel" href="/chat/c/-/#{chat_channel.id}">
<span class="category-chat-badge" style="color: ##{chat_channel.chatable.color}">
<svg class="fa d-icon d-icon-d-chat svg-icon svg-string" xmlns="http://www.w3.org/2000/svg"><use href="#d-chat"></use></svg>
</span>
#{chat_channel.name}
</a>
</div>
<div class="chat-transcript-messages"><p>Hello world!</p></div>
</div>
HTML
end
end
end
describe "auto-joining users to a channel" do