FEATURE: Thread list initial UI (#21412)

This commit adds an initial thread list UI. There are several limitations
with this that will be addressed in future PRs:

* There is no MessageBus reactivity, so e.g. if someone edits the original
   message of the thread it will not be reflected in the list. However if
   the thread title is updated the original message indicator will be updated.
* There is no unread functionality for threads in the list, if new messages
   come into the thread there is no indicator in the UI.
* There is no unread indicator on the actual button to open the thread list.
* No pagination.

In saying that, this is the functionality so far:

* We show a list of the 50 threads that the user has most recently participated
   in (i.e. sent a message) for the channel in descending order.
* Each thread we show a rich excerpt, the title, and the user who is the OM creator.
* The title is editable by staff and by the OM creator.
* Thread indicators show a title. We also replace emojis in the titles.
* Thread list works in the drawer/mobile.
This commit is contained in:
Joffrey JAFFEUX
2023-05-10 11:42:32 +02:00
committed by GitHub
parent 7a84fc3d9d
commit c6b43ce68b
74 changed files with 1512 additions and 64 deletions

View File

@ -142,9 +142,14 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do
end
transient :channel
transient :original_message_user
original_message do |attrs|
Fabricate(:chat_message, chat_channel: attrs[:channel] || Fabricate(:chat_channel))
Fabricate(
:chat_message,
chat_channel: attrs[:channel] || Fabricate(:chat_channel),
user: attrs[:original_message_user] || Fabricate(:user),
)
end
after_create { |thread| thread.original_message.update!(thread_id: thread.id) }

View File

@ -49,6 +49,7 @@ RSpec.describe Jobs::Chat::UpdateThreadReplyCount do
"original_message_id" => thread.original_message_id,
"replies_count" => 2,
"type" => "update_thread_original_message",
"title" => thread.title,
},
)
end

View File

@ -27,7 +27,7 @@ module ChatSystemHelpers
Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user)
end
def chat_thread_chain_bootstrap(channel:, users:, messages_count: 4)
def chat_thread_chain_bootstrap(channel:, users:, messages_count: 4, thread_attrs: {})
last_user = nil
last_message = nil
@ -50,6 +50,7 @@ module ChatSystemHelpers
end
last_message.thread.set_replies_count_cache(messages_count - 1, update_db: true)
last_message.thread.update!(thread_attrs) if thread_attrs.any?
last_message.thread
end
end

View File

@ -92,4 +92,137 @@ RSpec.describe Chat::Api::ChannelThreadsController do
end
end
end
describe "index" do
fab!(:thread_1) { Fabricate(:chat_thread, channel: public_channel) }
fab!(:thread_2) { Fabricate(:chat_thread, channel: public_channel) }
fab!(:thread_3) { Fabricate(:chat_thread, channel: public_channel) }
fab!(:message_1) do
Fabricate(
:chat_message,
user: current_user,
chat_channel: public_channel,
thread: thread_1,
created_at: 10.minutes.ago,
)
end
fab!(:message_2) do
Fabricate(
:chat_message,
user: current_user,
chat_channel: public_channel,
thread: thread_3,
created_at: 2.seconds.ago,
)
end
it "returns the threads the user has sent messages in for the channel" do
get "/chat/api/channels/#{public_channel.id}/threads"
expect(response.status).to eq(200)
expect(response.parsed_body["threads"].map { |thread| thread["id"] }).to eq(
[thread_3.id, thread_1.id],
)
end
context "when the channel is not accessible to the useer" do
before do
public_channel.update!(chatable: Fabricate(:private_category, group: Fabricate(:group)))
end
it "returns 404" do
get "/chat/api/channels/#{public_channel.id}/threads"
expect(response.status).to eq(403)
end
end
context "when channel does not have threading enabled" do
before { public_channel.update!(threading_enabled: false) }
it "returns 404" do
get "/chat/api/channels/#{public_channel.id}/threads"
expect(response.status).to eq(404)
end
end
context "when enable_experimental_chat_threaded_discussions is disabled" do
before { SiteSetting.enable_experimental_chat_threaded_discussions = false }
it "returns 404" do
get "/chat/api/channels/#{public_channel.id}/threads"
expect(response.status).to eq(404)
end
end
end
describe "update" do
let(:title) { "New title" }
let(:params) { { title: title } }
fab!(:thread) do
Fabricate(:chat_thread, channel: public_channel, original_message_user: current_user)
end
context "when thread does not exist" do
it "returns 404" do
thread.destroy!
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(404)
end
end
context "when thread exists" do
it "updates the title" do
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(200)
expect(thread.reload.title).to eq(title)
end
context "when user cannot view the channel" do
before { thread.update!(channel: Fabricate(:private_category_channel)) }
it "returns 403" do
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(403)
end
end
context "when the user is not the original message user" do
before { thread.update!(original_message_user: Fabricate(:user)) }
it "returns 403" do
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(403)
end
end
context "when the title is too long" do
let(:title) { "x" * Chat::Thread::MAX_TITLE_LENGTH + "x" }
it "returns 400" do
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(400)
expect(response.parsed_body["errors"]).to eq(
["Title is too long (maximum is #{Chat::Thread::MAX_TITLE_LENGTH} characters)"],
)
end
end
end
context "when channel does not have threading enabled" do
before { public_channel.update!(threading_enabled: false) }
it "returns 404" do
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(404)
end
end
context "when enable_experimental_chat_threaded_discussions is disabled" do
before { SiteSetting.enable_experimental_chat_threaded_discussions = false }
it "returns 404" do
put "/chat/api/channels/#{thread.channel_id}/threads/#{thread.id}", params: params
expect(response.status).to eq(404)
end
end
end
end

View File

@ -0,0 +1,114 @@
# frozen_string_literal: true
RSpec.describe Chat::LookupChannelThreads do
describe Chat::LookupChannelThreads::Contract, type: :model do
it { is_expected.to validate_presence_of :channel_id }
end
describe ".call" do
subject(:result) { described_class.call(params) }
fab!(:current_user) { Fabricate(:user) }
fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) }
fab!(:thread_1) { Fabricate(:chat_thread, channel: channel) }
fab!(:thread_2) { Fabricate(:chat_thread, channel: channel) }
fab!(:thread_3) { Fabricate(:chat_thread, channel: channel) }
let(:guardian) { Guardian.new(current_user) }
let(:params) { { guardian: guardian, channel_id: thread_1.channel_id } }
context "when enable_experimental_chat_threaded_discussions is disabled" do
before { SiteSetting.enable_experimental_chat_threaded_discussions = false }
it { is_expected.to fail_a_policy(:threaded_discussions_enabled) }
end
context "when enable_experimental_chat_threaded_discussions is enabled" do
before { SiteSetting.enable_experimental_chat_threaded_discussions = true }
context "when all steps pass" do
before do
Fabricate(
:chat_message,
user: current_user,
chat_channel: channel,
thread: thread_1,
created_at: 10.minutes.ago,
)
Fabricate(
:chat_message,
user: current_user,
chat_channel: channel,
thread: thread_2,
created_at: 1.day.ago,
)
Fabricate(
:chat_message,
user: current_user,
chat_channel: channel,
thread: thread_3,
created_at: 2.seconds.ago,
)
end
it "sets the service result as successful" do
expect(result).to be_a_success
end
it "returns the threads ordered by the last thread the current user posted in" do
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
end
it "does not return threads where the original message is deleted" do
thread_1.original_message.trash!
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_2.id])
end
it "does not count deleted messages for sort order" do
Chat::Message.find_by(user: current_user, thread: thread_3).trash!
expect(result.threads.map(&:id)).to eq([thread_1.id, thread_2.id])
end
it "does not return threads from the channel where the user has not sent a message" do
Fabricate(:chat_thread, channel: channel)
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
end
it "does not return threads from another channel" do
thread_4 = Fabricate(:chat_thread)
Fabricate(
:chat_message,
user: current_user,
thread: thread_4,
chat_channel: thread_4.channel,
created_at: 2.seconds.ago,
)
expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id])
end
end
context "when params are not valid" do
before { params.delete(:channel_id) }
it { is_expected.to fail_a_contract }
end
context "when user cannot see channel" do
fab!(:private_channel) { Fabricate(:private_category_channel, group: Fabricate(:group)) }
before do
thread_1.update!(channel: private_channel)
private_channel.update!(threading_enabled: true)
end
it { is_expected.to fail_a_policy(:can_view_channel) }
end
context "when threading is not enabled for the channel" do
before { channel.update!(threading_enabled: false) }
it { is_expected.to fail_a_policy(:threading_enabled_for_channel) }
end
end
end
end

View File

@ -0,0 +1,106 @@
# frozen_string_literal: true
RSpec.describe Chat::UpdateThread do
describe Chat::UpdateThread::Contract, type: :model do
it { is_expected.to validate_presence_of :channel_id }
it { is_expected.to validate_presence_of :thread_id }
end
describe ".call" do
subject(:result) { described_class.call(params) }
fab!(:current_user) { Fabricate(:user) }
fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) }
fab!(:private_channel) { Fabricate(:private_category_channel, group: Fabricate(:group)) }
fab!(:thread) { Fabricate(:chat_thread, channel: channel, original_message_user: current_user) }
fab!(:other_thread) { Fabricate(:chat_thread) }
let(:guardian) { Guardian.new(current_user) }
let(:title) { "some new title :D" }
let(:params) do
{ guardian: guardian, thread_id: thread.id, channel_id: thread.channel_id, title: title }
end
context "when enable_experimental_chat_threaded_discussions is disabled" do
before { SiteSetting.enable_experimental_chat_threaded_discussions = false }
it { is_expected.to fail_a_policy(:threaded_discussions_enabled) }
end
context "when enable_experimental_chat_threaded_discussions is enabled" do
before { SiteSetting.enable_experimental_chat_threaded_discussions = true }
context "when all steps pass" do
it "sets the service result as successful" do
expect(result).to be_a_success
end
it "updates the title of the thread" do
result
expect(thread.reload.title).to eq(title)
end
it "publishes a MessageBus message" do
message =
MessageBus
.track_publish(Chat::Publisher.root_message_bus_channel(thread.channel_id)) { result }
.first
expect(message.data["type"]).to eq("update_thread_original_message")
expect(message.data["title"]).to eq(title)
end
end
context "when params are not valid" do
before { params.delete(:thread_id) }
it { is_expected.to fail_a_contract }
end
context "when title is too long" do
let(:title) { "a" * Chat::Thread::MAX_TITLE_LENGTH + "a" }
it { is_expected.to fail_a_contract }
end
context "when thread is not found because the channel ID differs" do
before { params[:thread_id] = other_thread.id }
it { is_expected.to fail_to_find_a_model(:thread) }
end
context "when thread is not found" do
before { thread.destroy! }
it { is_expected.to fail_to_find_a_model(:thread) }
end
context "when user cannot see channel" do
before { thread.update!(channel: private_channel) }
it { is_expected.to fail_a_policy(:can_view_channel) }
end
context "when user is not the thread original message creator" do
before { thread.update!(original_message_user: Fabricate(:user)) }
it { is_expected.to fail_a_policy(:can_edit_thread) }
end
context "when user is not the thread original message creator but they are staff" do
before do
thread.original_message.update!(user: Fabricate(:user))
current_user.update!(admin: true)
end
it { is_expected.not_to fail_a_policy(:can_edit_thread) }
end
context "when threading is not enabled for the channel" do
before { channel.update!(threading_enabled: false) }
it { is_expected.to fail_a_policy(:threading_enabled_for_channel) }
end
end
end
end

View File

@ -120,6 +120,10 @@ module Chat
FailWithInvalidModel.new(name)
end
def fail_a_step(name = "model")
FailStep.new(name)
end
def inspect_steps(result)
inspector = Chat::StepsInspector.new(result)
puts "Steps:"

View File

@ -52,6 +52,10 @@ module PageObjects
find(".open-drawer-btn").click
end
def open_thread_list
find(".open-thread-list-btn").click
end
def has_message?(message)
container = find(".chat-message-container[data-id=\"#{message.id}\"")
container.has_content?(message.message)

View File

@ -14,6 +14,10 @@ module PageObjects
def has_no_open_thread?
!has_css?(".chat-side-panel .chat-thread")
end
def has_open_thread_list?
has_css?(".chat-side-panel .chat-thread-list")
end
end
end
end

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
module PageObjects
module Pages
class ChatThreadList < PageObjects::Pages::Base
def item_by_id(id)
find(item_by_id_selector(id))
end
def item_by_id_selector(id)
".chat-thread-list__items .chat-thread-list-item[data-thread-id=\"#{id}\"]"
end
end
end
end

View File

@ -42,6 +42,10 @@ module PageObjects
def has_open_channel?(channel)
has_css?("#{VISIBLE_DRAWER} .chat-channel[data-id='#{channel.id}']")
end
def has_open_thread_list?
has_css?("#{VISIBLE_DRAWER} .chat-thread-list")
end
end
end
end

View File

@ -0,0 +1,82 @@
# frozen_string_literal: true
describe "Thread list in side panel | drawer", type: :system, js: true do
fab!(:current_user) { Fabricate(:admin) }
fab!(:channel) { Fabricate(:chat_channel) }
fab!(:other_user) { Fabricate(:user) }
let(:chat_page) { PageObjects::Pages::Chat.new }
let(:channel_page) { PageObjects::Pages::ChatChannel.new }
let(:side_panel) { PageObjects::Pages::ChatSidePanel.new }
let(:thread_page) { PageObjects::Pages::ChatThread.new }
let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new }
let(:drawer_page) { PageObjects::Pages::ChatDrawer.new }
before do
SiteSetting.enable_experimental_chat_threaded_discussions = true
chat_system_bootstrap(current_user, [channel])
sign_in(current_user)
end
context "when threading not enabled for the channel" do
before { channel.update!(threading_enabled: false) }
it "does not show the thread list button in drawer header" do
visit("/")
chat_page.open_from_header
drawer_page.open_channel(channel)
expect(find(".chat-drawer-header__right-actions")).not_to have_css(".open-thread-list-btn")
end
end
context "when threading is enabled for the channel" do
before { channel.update!(threading_enabled: true) }
fab!(:thread_1) do
chat_thread_chain_bootstrap(
channel: channel,
users: [current_user, other_user],
thread_attrs: {
title: "favourite album?",
},
)
end
fab!(:thread_2) do
chat_thread_chain_bootstrap(
channel: channel,
users: [current_user, other_user],
thread_attrs: {
title: "current event",
},
)
end
it "opens the thread list from the header button" do
visit("/")
chat_page.open_from_header
drawer_page.open_channel(channel)
find(".open-thread-list-btn").click
expect(drawer_page).to have_open_thread_list
end
it "shows the titles of the threads the user is participating in" do
visit("/")
chat_page.open_from_header
drawer_page.open_channel(channel)
find(".open-thread-list-btn").click
expect(drawer_page).to have_open_thread_list
expect(thread_list_page).to have_content(thread_1.title)
expect(thread_list_page).to have_content(thread_2.title)
end
it "opens a thread" do
visit("/")
chat_page.open_from_header
drawer_page.open_channel(channel)
find(".open-thread-list-btn").click
expect(drawer_page).to have_open_thread_list
thread_list_page.item_by_id(thread_1.id).click
expect(drawer_page).to have_open_thread(thread_1)
end
end
end

View File

@ -0,0 +1,120 @@
# frozen_string_literal: true
describe "Thread list in side panel | full page", type: :system, js: true do
fab!(:current_user) { Fabricate(:user) }
fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) }
fab!(:other_user) { Fabricate(:user) }
let(:chat_page) { PageObjects::Pages::Chat.new }
let(:channel_page) { PageObjects::Pages::ChatChannel.new }
let(:side_panel) { PageObjects::Pages::ChatSidePanel.new }
let(:thread_page) { PageObjects::Pages::ChatThread.new }
let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new }
before do
SiteSetting.enable_experimental_chat_threaded_discussions = true
chat_system_bootstrap(current_user, [channel])
sign_in(current_user)
end
context "when there are no threads that the user is participating in" do
it "shows a message" do
chat_page.visit_channel(channel)
chat_page.open_thread_list
expect(page).to have_content(I18n.t("js.chat.threads.none"))
end
end
context "when there are threads that the user is participating in" do
before { chat_system_user_bootstrap(user: other_user, channel: channel) }
fab!(:thread_1) do
chat_thread_chain_bootstrap(channel: channel, users: [current_user, other_user])
end
fab!(:thread_2) do
chat_thread_chain_bootstrap(channel: channel, users: [current_user, other_user])
end
it "shows a default title for threads without a title" do
chat_page.visit_channel(channel)
chat_page.open_thread_list
expect(page).to have_content(I18n.t("js.chat.thread.default_title", thread_id: thread_1.id))
end
it "shows the thread title with emoji" do
thread_1.update!(title: "What is for dinner? :hamburger:")
chat_page.visit_channel(channel)
chat_page.open_thread_list
expect(thread_list_page.item_by_id(thread_1.id)).to have_content("What is for dinner?")
expect(thread_list_page.item_by_id(thread_1.id)).to have_css("img.emoji[alt='hamburger']")
end
it "shows an excerpt of the original message of the thread" do
thread_1.original_message.update!(message: "This is a long message for the excerpt")
thread_1.original_message.rebake!
chat_page.visit_channel(channel)
chat_page.open_thread_list
expect(thread_list_page.item_by_id(thread_1.id)).to have_content(
"This is a long message for the excerpt",
)
end
it "shows the thread original message user username and avatar" do
chat_page.visit_channel(channel)
chat_page.open_thread_list
expect(thread_list_page.item_by_id(thread_1.id)).to have_css(
".chat-thread-original-message__avatar .chat-user-avatar .chat-user-avatar-container img",
)
expect(
thread_list_page.item_by_id(thread_1.id).find(".chat-thread-original-message__username"),
).to have_content(thread_1.original_message.user.username)
end
it "opens a thread" do
chat_page.visit_channel(channel)
chat_page.open_thread_list
thread_list_page.item_by_id(thread_1.id).click
expect(side_panel).to have_open_thread(thread_1)
end
describe "updating the title of the thread" do
let(:new_title) { "wow new title" }
def open_thread_list
chat_page.visit_channel(channel)
chat_page.open_thread_list
expect(side_panel).to have_open_thread_list
end
it "allows updating when user is admin" do
current_user.update!(admin: true)
open_thread_list
thread_list_page.item_by_id(thread_1.id).find(".chat-thread-list-item__settings").click
find(".thread-title-input").fill_in(with: new_title)
find(".modal-footer .btn-primary").click
expect(thread_list_page.item_by_id(thread_1.id)).to have_content(new_title)
end
it "allows updating when user is same as the chat original message user" do
thread_1.update!(original_message_user: current_user)
thread_1.original_message.update!(user: current_user)
open_thread_list
thread_list_page.item_by_id(thread_1.id).find(".chat-thread-list-item__settings").click
find(".thread-title-input").fill_in(with: new_title)
find(".modal-footer .btn-primary").click
expect(thread_list_page.item_by_id(thread_1.id)).to have_content(new_title)
end
it "does not allow updating if user is neither admin nor original message user" do
thread_1.update!(original_message_user: other_user)
thread_1.original_message.update!(user: other_user)
open_thread_list
expect(
thread_list_page.item_by_id(thread_1.id).find(".chat-thread-list-item__settings")[
:disabled
],
).to eq("true")
end
end
end
end