From cfdf5b95187d3dde9abccaf75b8a0f6095fe62e8 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 5 Jul 2023 21:01:23 +0200 Subject: [PATCH] FIX: correctly show unread and presence (#22441) - Presence needs to be explicitly set on the component now - We were not checking and testing correctly the presence of the unread indicator in the menu --- .../components/chat/message-creator.js | 9 ++++-- .../chat/message-creator/channel-row.hbs | 11 +------ .../chat/message-creator/channel-row.js | 8 +++++ .../chat/message-creator/user-row.hbs | 2 +- .../components/chat/message/avatar.hbs | 6 +++- plugins/chat/spec/system/new_message_spec.rb | 31 +++++++++++++------ .../spec/system/page_objects/chat/chat.rb | 1 + .../chat/components/message_creator.rb | 12 +++++-- plugins/chat/spec/system/user_presence.rb | 9 +++--- 9 files changed, 59 insertions(+), 30 deletions(-) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator.js b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator.js index 719f5c3ee71..87730b296a7 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator.js @@ -74,11 +74,14 @@ class Search { #loadExistingChannels() { return this.chatChannelsManager.allChannels .map((channel) => { + let chatable; if (channel.chatable?.users?.length === 1) { - return ChatChatable.createUser(channel.chatable.users[0]); + chatable = ChatChatable.createUser(channel.chatable.users[0]); + chatable.tracking = this.#injectTracking(chatable); + } else { + chatable = ChatChatable.createChannel(channel); + chatable.tracking = channel.tracking; } - const chatable = ChatChatable.createChannel(channel); - chatable.tracking = channel.tracking; return chatable; }) .filter(Boolean) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.hbs index 6420a67eaea..ac4f7767f7a 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.hbs @@ -2,16 +2,7 @@ {{#if (gt @content.tracking.unreadCount 0)}}
{{/if}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.js b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.js index 5763359c9f4..52869e4cf1e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/channel-row.js @@ -9,4 +9,12 @@ export default class ChatMessageCreatorChannelRow extends Component { get openChannelLabel() { return htmlSafe(I18n.t("chat.new_message_modal.open_channel")); } + + get isUrgent() { + return ( + this.args.content.model.isDirectMessageChannel || + (this.args.content.model.isCategoryChannel && + this.args.content.model.tracking.mentionCount > 0) + ); + } } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/user-row.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/user-row.hbs index 397bb059ffa..ef6e94c563e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/user-row.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message-creator/user-row.hbs @@ -2,7 +2,7 @@ {{#if (gt @content.tracking.unreadCount 0)}} -
+
{{/if}} {{user-status @content.model currentUser=this.currentUser}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message/avatar.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/message/avatar.hbs index 1d2e9676fbf..949271d7a20 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message/avatar.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message/avatar.hbs @@ -2,6 +2,10 @@ {{#if @message.chatWebhookEvent.emoji}} {{else}} - + {{/if}} \ No newline at end of file diff --git a/plugins/chat/spec/system/new_message_spec.rb b/plugins/chat/spec/system/new_message_spec.rb index 4da9830989a..d9a90010ff8 100644 --- a/plugins/chat/spec/system/new_message_spec.rb +++ b/plugins/chat/spec/system/new_message_spec.rb @@ -37,22 +37,35 @@ RSpec.describe "New message", type: :system do chat_page.open_new_message expect(chat_page.message_creator).to be_listing(channel_1) - # it lists user_1 instead of this channel as it's a 1:1 channel expect(chat_page.message_creator).to be_not_listing(channel_2) - expect(chat_page.message_creator).to be_not_listing( - direct_message_channel_1, - current_user: current_user, - ) - expect(chat_page.message_creator).to be_not_listing( - direct_message_channel_2, - current_user: current_user, - ) + expect(chat_page.message_creator).to be_not_listing(direct_message_channel_2) expect(chat_page.message_creator).to be_listing(user_1) expect(chat_page.message_creator).to be_not_listing(user_2) end end context "with no selection" do + context "with unread state" do + fab!(:user_1) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, user_1]) } + + before do + channel_1.add(user_1) + channel_1.add(current_user) + Fabricate(:chat_message, chat_channel: channel_1, user: user_1) + Fabricate(:chat_message, chat_channel: channel_2, user: user_1) + end + + it "shows the correct state" do + visit("/") + chat_page.open_new_message + + expect(chat_page.message_creator).to have_unread_row(channel_1, urgent: false) + expect(chat_page.message_creator).to have_unread_row(user_1, urgent: true) + end + end + context "when clicking a row" do context "when the row is a channel" do fab!(:channel_1) { Fabricate(:chat_channel) } diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 63e17676b44..9a415fe283b 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -25,6 +25,7 @@ module PageObjects def open_new_message send_keys([MODIFIER, "k"]) + find(".chat-new-message-modal") end def has_drawer?(channel_id: nil, expanded: true) diff --git a/plugins/chat/spec/system/page_objects/chat/components/message_creator.rb b/plugins/chat/spec/system/page_objects/chat/components/message_creator.rb index 59fc474c595..8a02518453e 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/message_creator.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/message_creator.rb @@ -84,6 +84,14 @@ module PageObjects component.find(build_row_selector(chatable, **args)).click(:shift) end + def has_unread_row?(chatable, **args) + unread_selector = build_row_selector(chatable, **args) + unread_selector += " .unread-indicator" + unread_selector += ".-urgent" if args[:urgent] + unread_selector += ":not(.-urgent)" unless args[:urgent] + component.has_css?(unread_selector) + end + def build_item_selector(chatable, **args) selector = ".chat-message-creator__selection-item" selector += content_selector(**args) @@ -94,7 +102,7 @@ module PageObjects def build_row_selector(chatable, **args) selector = ".chat-message-creator__row" selector += content_selector(**args) - selector += chatable_selector(chatable) + selector += chatable_selector(chatable, **args) selector end @@ -111,7 +119,7 @@ module PageObjects selector end - def chatable_selector(chatable) + def chatable_selector(chatable, **args) selector = "" if chatable.try(:category_channel?) selector += ".-channel" diff --git a/plugins/chat/spec/system/user_presence.rb b/plugins/chat/spec/system/user_presence.rb index 1ce317ceb7b..147a7c1e027 100644 --- a/plugins/chat/spec/system/user_presence.rb +++ b/plugins/chat/spec/system/user_presence.rb @@ -4,6 +4,7 @@ RSpec.describe "User presence", type: :system do fab!(:channel_1) { Fabricate(:chat_channel) } fab!(:current_user) { Fabricate(:user) } + let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel) { PageObjects::Pages::ChatChannel.new } before do @@ -13,20 +14,20 @@ RSpec.describe "User presence", type: :system do it "shows presence indicator" do sign_in(current_user) - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) channel.send_message("Am I present?") - expect(page).to have_selector(".chat-user-avatar.is-online") + expect(page).to have_css(".chat-message .chat-user-avatar.is-online") end context "when user hides presence" do it "hides the presence indicator" do current_user.user_option.update!(hide_profile_and_presence: true) sign_in(current_user) - chat.visit_channel(channel_1) + chat_page.visit_channel(channel_1) channel.send_message("Am I present?") - expect(page).to have_no_selector(".chat-user-avatar.is-online") + expect(page).to have_no_css(".chat-user-avatar.is-online") end end end