diff --git a/plugins/chat/app/queries/chat/thread_unreads_query.rb b/plugins/chat/app/queries/chat/thread_unreads_query.rb index 8132da091e5..fc724ff440e 100644 --- a/plugins/chat/app/queries/chat/thread_unreads_query.rb +++ b/plugins/chat/app/queries/chat/thread_unreads_query.rb @@ -44,6 +44,7 @@ module Chat INNER JOIN chat_channels ON chat_channels.id = chat_messages.chat_channel_id INNER JOIN chat_threads ON chat_threads.id = chat_messages.thread_id AND chat_threads.channel_id = chat_messages.chat_channel_id INNER JOIN user_chat_thread_memberships ON user_chat_thread_memberships.thread_id = chat_threads.id + INNER JOIN chat_messages AS original_message ON original_message.id = chat_threads.original_message_id AND chat_messages.thread_id = memberships.thread_id AND chat_messages.user_id != :user_id AND user_chat_thread_memberships.user_id = :user_id @@ -53,6 +54,7 @@ module Chat AND chat_messages.id != chat_threads.original_message_id AND chat_channels.threading_enabled AND user_chat_thread_memberships.notification_level NOT IN (:quiet_notification_levels) + AND original_message.deleted_at IS NULL ) AS unread_count, 0 AS mention_count, chat_threads.channel_id, diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 352498b4a49..15948a5aba2 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -146,6 +146,10 @@ module Chat ON tracked_threads_subquery.thread_id = chat_threads.id", ) .joins(:user_chat_thread_memberships) + .joins( + "LEFT JOIN chat_messages original_messages ON chat_threads.original_message_id = original_messages.id", + ) + .where("original_messages.deleted_at IS NULL") .where(user_chat_thread_memberships_chat_threads: { user_id: guardian.user.id }) end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs index 56442be547a..ef81a25b158 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs @@ -1,8 +1,10 @@ {{#if this.shouldRender}}
{{#if @includeHeader}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js index 9dd999a816d..b2e5f877b3f 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js @@ -1,10 +1,12 @@ import Component from "@glimmer/component"; +import { bind } from "discourse-common/utils/decorators"; import { tracked } from "@glimmer/tracking"; import { action } from "@ember/object"; import { inject as service } from "@ember/service"; export default class ChatThreadList extends Component { @service chat; + @service messageBus; @tracked loading = true; @@ -16,9 +18,30 @@ export default class ChatThreadList extends Component { return []; } - return this.args.channel.threadsManager.threads.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) { + return this.args.channel.threadsManager.threads + .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) { + if ( + threadA.preview.lastReplyCreatedAt > + threadB.preview.lastReplyCreatedAt + ) { + return -1; + } else { + return 1; + } + } + + // If one is unread and the other is not, we want to sort the unread one first. + if (threadA.tracking.unreadCount) { + return -1; + } + + if (threadB.tracking.unreadCount) { + return 1; + } + + // If both are read, we want to sort by last reply date + time descending. if ( threadA.preview.lastReplyCreatedAt > threadB.preview.lastReplyCreatedAt @@ -27,32 +50,65 @@ export default class ChatThreadList extends Component { } else { return 1; } - } - - // If one is unread and the other is not, we want to sort the unread one first. - if (threadA.tracking.unreadCount) { - return -1; - } - - if (threadB.tracking.unreadCount) { - return 1; - } - - // If both are read, we want to sort by last reply date + time descending. - if ( - threadA.preview.lastReplyCreatedAt > threadB.preview.lastReplyCreatedAt - ) { - return -1; - } else { - return 1; - } - }); + }) + .filter((thread) => !thread.originalMessage.deletedAt); } get shouldRender() { return !!this.args.channel; } + @action + subscribe() { + this.#unsubscribe(); + + this.messageBus.subscribe( + `/chat/${this.args.channel.id}`, + this.onMessageBus, + this.args.channel.messageBusLastId + ); + } + + @bind + onMessageBus(busData) { + switch (busData.type) { + case "delete": + this.handleDeleteMessage(busData); + break; + case "restore": + this.handleRestoreMessage(busData); + break; + } + } + + handleDeleteMessage(data) { + const deletedOriginalMessageThread = + this.args.channel.threadsManager.threads.findBy( + "originalMessage.id", + data.deleted_id + ); + + if (!deletedOriginalMessageThread) { + return; + } + + deletedOriginalMessageThread.originalMessage.deletedAt = new Date(); + } + + handleRestoreMessage(data) { + const restoredOriginalMessageThread = + this.args.channel.threadsManager.threads.findBy( + "originalMessage.id", + data.chat_message.id + ); + + if (!restoredOriginalMessageThread) { + return; + } + + restoredOriginalMessageThread.originalMessage.deletedAt = null; + } + @action loadThreads() { this.loading = true; @@ -64,5 +120,13 @@ export default class ChatThreadList extends Component { @action teardown() { this.loading = true; + this.#unsubscribe(); + } + + #unsubscribe() { + this.messageBus.unsubscribe( + `/chat/${this.args.channel.id}`, + this.onMessageBus + ); } } diff --git a/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb b/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb index 2999eef5108..1d0cf52b63c 100644 --- a/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb +++ b/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb @@ -93,6 +93,13 @@ describe Chat::ThreadUnreadsQuery do ) end + it "does not count the thread as unread if the original message is deleted" do + thread_1.original_message.destroy + expect(query.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + end + context "when include_read is false" do let(:include_read) { false } diff --git a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb index a80457dc6a7..89c50c062ba 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -58,6 +58,10 @@ module PageObjects has_css?(".chat-selection-management") end + def expand_deleted_message(message) + message_by_id(message.id).find(".chat-message-expand").click + end + def expand_message_actions(message) hover_message(message) click_more_button diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb index a16e73373dc..e190ee6207f 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -132,10 +132,29 @@ module PageObjects end end + def expand_deleted_message(message) + message_by_id(message.id).find(".chat-message-expand").click + end + def copy_link(message) + expand_message_actions(message) + find("[data-value='copyLink']").click + end + + def delete_message(message) + expand_message_actions(message) + find("[data-value='delete']").click + end + + def restore_message(message) + expand_deleted_message(message) + expand_message_actions(message) + find("[data-value='restore']").click + end + + def expand_message_actions(message) hover_message(message) click_more_button - find("[data-value='copyLink']").click end def click_more_button diff --git a/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb b/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb index 706a543e1d5..92956410099 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb @@ -23,6 +23,10 @@ module PageObjects item_by_id(thread.id) end + def has_no_thread?(thread) + component.has_no_css?(item_by_id_selector(thread.id)) + end + def item_by_id(id) component.find(item_by_id_selector(id)) end 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 3118fcb0a7b..6851672cbc6 100644 --- a/plugins/chat/spec/system/thread_list/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_list/full_page_spec.rb @@ -5,6 +5,7 @@ describe "Thread list in side panel | full page", type: :system do fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } fab!(:other_user) { Fabricate(:user) } + let(:side_panel_page) { PageObjects::Pages::ChatSidePanel.new } let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:side_panel) { PageObjects::Pages::ChatSidePanel.new } @@ -88,6 +89,53 @@ describe "Thread list in side panel | full page", type: :system do expect(side_panel).to have_open_thread(thread_1) end + describe "deleting and restoring the original message of the thread" do + before do + thread_1.update!(original_message_user: other_user) + thread_1.original_message.update!(user: other_user) + end + + it "hides the thread in the list when another user deletes the original message" do + chat_page.visit_channel(channel) + channel_page.open_thread_list + expect(thread_list_page).to have_thread(thread_1) + + using_session(:tab_2) do |session| + sign_in(other_user) + chat_page.visit_thread(thread_1) + expect(side_panel_page).to have_open_thread(thread_1) + thread_page.delete_message(thread_1.original_message) + session.quit + end + + expect(thread_list_page).to have_no_thread(thread_1) + end + + it "shows the thread in the list when another user restores the original message" do + # This is necessary because normal users can't see deleted messages + other_user.update!(admin: true) + current_user.update!(admin: true) + + thread_1.original_message.trash! + chat_page.visit_channel(channel) + channel_page.open_thread_list + expect(thread_list_page).to have_no_thread(thread_1) + + using_session(:tab_2) do |session| + sign_in(other_user) + chat_page.visit_channel(channel) + expect(channel_page).to have_no_loading_skeleton + channel_page.expand_deleted_message(thread_1.original_message) + channel_page.message_thread_indicator(thread_1.original_message).click + expect(side_panel_page).to have_open_thread(thread_1) + thread_page.restore_message(thread_1.original_message) + session.quit + end + + expect(thread_list_page).to have_thread(thread_1) + end + end + describe "updating the title of the thread" do let(:new_title) { "wow new title" } diff --git a/plugins/chat/spec/system/thread_tracking/full_page_spec.rb b/plugins/chat/spec/system/thread_tracking/full_page_spec.rb index 4becabe4641..7c091fbb8d0 100644 --- a/plugins/chat/spec/system/thread_tracking/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_tracking/full_page_spec.rb @@ -29,6 +29,12 @@ describe "Thread tracking state | full page", type: :system do expect(channel_page).to have_unread_thread_indicator(count: 1) end + it "does not include threads with deleted original messages in the count of threads with unread messages" do + thread.original_message.trash! + chat_page.visit_channel(channel) + expect(thread_page).to have_no_unread_list_indicator + end + it "shows an indicator on the unread thread in the list" do chat_page.visit_channel(channel) channel_page.open_thread_list