From 0c827980e6689df333143a43641924f7f8e38d02 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 5 Mar 2024 20:26:35 +0100 Subject: [PATCH] FIX: do not show threads with no replies (#26033) Prior to this fix if a user had started to reply to a message without actually sending a message, the thread would still be created and we would end up listing it in the threads list of a channel. This commit also improves adds thread and thread_replies_count to the 4th parameter of the chat_message_created event. --- .../chat/app/serializers/chat/thread_serializer.rb | 3 ++- plugins/chat/app/services/chat/create_message.rb | 11 +++++++++-- .../discourse/components/chat-thread-list.gjs | 6 +++++- .../javascripts/discourse/models/chat-thread.js | 3 +++ .../discourse/services/chat-subscriptions-manager.js | 2 ++ .../chat/spec/services/chat/create_message_spec.rb | 11 +++++++++-- .../chat/spec/system/thread_list/full_page_spec.rb | 9 +++++++++ 7 files changed, 39 insertions(+), 6 deletions(-) diff --git a/plugins/chat/app/serializers/chat/thread_serializer.rb b/plugins/chat/app/serializers/chat/thread_serializer.rb index dcbf93e02ef..77bae3375c3 100644 --- a/plugins/chat/app/serializers/chat/thread_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_serializer.rb @@ -12,7 +12,8 @@ module Chat :meta, :reply_count, :current_user_membership, - :preview + :preview, + :last_message_id def initialize(object, opts) super(object, opts) diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index 962cafb84f5..a0e06cd0437 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -190,7 +190,7 @@ module Chat Chat::Publisher.publish_thread_created!(channel, reply, thread.id) end - def process(channel:, message_instance:, contract:, **) + def process(channel:, message_instance:, contract:, thread:, **) ::Chat::Publisher.publish_new!(channel, message_instance, contract.staged_id) DiscourseEvent.trigger( @@ -198,7 +198,14 @@ module Chat message_instance, channel, message_instance.user, - { context: { post_ids: contract.context_post_ids, topic_id: contract.context_topic_id } }, + { + thread: thread, + thread_replies_count: thread&.replies_count_cache || 0, + context: { + post_ids: contract.context_post_ids, + topic_id: contract.context_topic_id, + }, + }, ) if contract.process_inline diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread-list.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-thread-list.gjs index 22cb8ed8b78..056fb2722bb 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread-list.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread-list.gjs @@ -78,7 +78,11 @@ export default class ChatThreadList extends Component { @cached get sortedThreads() { return this.threadsManager.threads - .filter((thread) => !thread.originalMessage.deletedAt) + .filter( + (thread) => + !thread.originalMessage.deletedAt && + thread.originalMessage?.id !== thread.lastMessageId + ) .sort((threadA, threadB) => { // If both are unread we just want to sort by last reply date + time descending. if (threadA.tracking.unreadCount && threadB.tracking.unreadCount) { diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js index 9df1277db9d..99be405c604 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js @@ -26,6 +26,7 @@ export default class ChatThread { @tracked staged; @tracked channel; @tracked originalMessage; + @tracked lastMessageId; @tracked threadMessageBusLastId; @tracked replyCount; @tracked tracking; @@ -49,6 +50,8 @@ export default class ChatThread { this.originalMessage.thread = this; } + this.lastMessageId = args.last_message_id; + this.title = args.title; if (args.current_user_membership) { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js index 1ebbc068415..eff4aa14a98 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -233,6 +233,8 @@ export default class ChatSubscriptionsManager extends Service { channel.threadsManager .find(busData.channel_id, busData.thread_id) .then((thread) => { + thread.lastMessageId = busData.message.id; + if (busData.message.user.id === this.currentUser.id) { // Thread should no longer be considered unread. if (thread.currentUserMembership) { diff --git a/plugins/chat/spec/services/chat/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb index bb6829d54c9..a84ffec17bf 100644 --- a/plugins/chat/spec/services/chat/create_message_spec.rb +++ b/plugins/chat/spec/services/chat/create_message_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Chat::CreateMessage do instance_of(Chat::Message), channel, user, - anything, + has_entries(thread: anything, thread_replies_count: anything, context: anything), ) result @@ -117,7 +117,14 @@ RSpec.describe Chat::CreateMessage do instance_of(Chat::Message), channel, user, - { context: { post_ids: context_post_ids, topic_id: context_topic_id } }, + has_entries( + thread: anything, + thread_replies_count: anything, + context: { + post_ids: context_post_ids, + topic_id: context_topic_id, + }, + ), ) result diff --git a/plugins/chat/spec/system/thread_list/full_page_spec.rb b/plugins/chat/spec/system/thread_list/full_page_spec.rb index 6ed0bc2e1c2..4a719666bd7 100644 --- a/plugins/chat/spec/system/thread_list/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_list/full_page_spec.rb @@ -61,6 +61,15 @@ describe "Thread list in side panel | full page", type: :system do end end + it "doesn’t list threads with no replies" do + thread = Fabricate(:chat_thread, channel: channel, use_service: true) + + chat_page.visit_channel(channel) + channel_page.open_thread_list + + expect(thread_list_page).to have_no_thread(thread) + end + context "when there are threads that the user is participating in" do fab!(:thread_1) do chat_thread_chain_bootstrap(channel: channel, users: [current_user, other_user])