diff --git a/plugins/chat/app/controllers/chat/chat_controller.rb b/plugins/chat/app/controllers/chat/chat_controller.rb index e0d9d3a9a79..66fa311a390 100644 --- a/plugins/chat/app/controllers/chat/chat_controller.rb +++ b/plugins/chat/app/controllers/chat/chat_controller.rb @@ -118,22 +118,21 @@ module Chat # If any of the channel users is ignoring, muting, or preventing DMs from # the current user then we should not auto-follow the channel once again or # publish the new channel. - user_ids_allowing_communication = + allowed_user_ids = UserCommScreener.new( acting_user: current_user, target_user_ids: @chat_channel.user_chat_channel_memberships.where(following: false).pluck(:user_id), ).allowing_actor_communication - if user_ids_allowing_communication.any? - Chat::Publisher.publish_new_channel( - @chat_channel, - User.where(id: user_ids_allowing_communication), - ) + allowed_user_ids << current_user.id if !@user_chat_channel_membership.following + + if allowed_user_ids.any? + Chat::Publisher.publish_new_channel(@chat_channel, User.where(id: allowed_user_ids)) @chat_channel .user_chat_channel_memberships - .where(user_id: user_ids_allowing_communication) + .where(user_id: allowed_user_ids) .update_all(following: true) end end diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 5f3c8b58b72..33da0ba24ed 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -372,7 +372,7 @@ module Chat serialized_channel = Chat::ChannelSerializer.new( chat_channel, - scope: Guardian.new(membership.user), # We need a guardian here for direct messages + scope: membership.user.guardian, # We need a guardian here for direct messages root: :channel, membership: membership, ).as_json diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js index ccbefd36d57..04c9f32de3c 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js @@ -113,6 +113,13 @@ export default class ChatLivePane extends Component { return; } + if ( + this.args.channel.isDirectMessageChannel && + !this.args.channel.isFollowing + ) { + this.chatChannelsManager.follow(this.args.channel); + } + // Technically we could keep messages to avoid re-fetching them, but // it's not worth the complexity for now this.args.channel.clearMessages(); diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 135291c449b..e1e2a51dc71 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -56,7 +56,6 @@ export default class ChatChannel { return new ChatChannel(args); } - @tracked currentUserMembership = null; @tracked title; @tracked slug; @tracked description; @@ -82,6 +81,7 @@ export default class ChatChannel { messagesManager = new ChatMessagesManager(getOwner(this)); @tracked _unreadThreadIds = new TrackedSet(); + @tracked _currentUserMembership; constructor(args = {}) { this.id = args.id; @@ -109,9 +109,7 @@ export default class ChatChannel { users: args.chatable?.users, }) : Category.create(args.chatable); - this.currentUserMembership = UserChatChannelMembership.create( - args.current_user_membership - ); + this.currentUserMembership = args.current_user_membership; if (args.archive_completed || args.archive_failed) { this.archive = ChatChannelArchive.create(args); @@ -301,15 +299,17 @@ export default class ChatChannel { return !READONLY_STATUSES.includes(this.status); } - updateMembership(membership) { - this.currentUserMembership.following = membership.following; - this.currentUserMembership.lastReadMessage_id = - membership.last_read_message_id; - this.currentUserMembership.desktopNotificationLevel = - membership.desktop_notification_level; - this.currentUserMembership.mobileNotificationLevel = - membership.mobile_notification_level; - this.currentUserMembership.muted = membership.muted; + get currentUserMembership() { + return this._currentUserMembership; + } + + set currentUserMembership(membership) { + if (membership instanceof UserChatChannelMembership) { + this._currentUserMembership = membership; + } else { + this._currentUserMembership = + UserChatChannelMembership.create(membership); + } } clearSelectedMessages() { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js index 03aa219bb91..f42ad19f551 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js @@ -67,16 +67,11 @@ export default class ChatChannelsManager extends Service { if (!model.currentUserMembership.following) { return this.chatApi.followChannel(model.id).then((membership) => { - model.currentUserMembership.following = membership.following; - model.currentUserMembership.muted = membership.muted; - model.currentUserMembership.desktopNotificationLevel = - membership.desktopNotificationLevel; - model.currentUserMembership.mobileNotificationLevel = - membership.mobileNotificationLevel; + model.currentUserMembership = membership; return model; }); } else { - return Promise.resolve(model); + return model; } } 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 ee172ac0eca..e65a97b1775 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -354,7 +354,7 @@ export default class ChatSubscriptionsManager extends Service { this.chatChannelsManager.find(data.channel.id).then((channel) => { // we need to refresh here to have correct last message ids channel.meta = data.channel.meta; - channel.updateMembership(data.channel.current_user_membership); + channel.currentUserMembership = data.channel.current_user_membership; if ( channel.isDirectMessageChannel && diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index 1b12c8580d2..decadb20f61 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -143,7 +143,7 @@ export default class Chat extends Service { channel.tracking.unreadCount = state.unread_count; channel.tracking.mentionCount = state.mention_count; - channel.updateMembership(channelObject.current_user_membership); + channel.updateMembership = channelObject.current_user_membership; this.chatSubscriptionsManager.startChannelSubscription(channel); }); diff --git a/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb b/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb index f10aa455ef9..656d16ab3f2 100644 --- a/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb +++ b/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb @@ -238,45 +238,6 @@ RSpec.describe "Message notifications - with sidebar", type: :system do end end end - - context "with dm and public channel" do - fab!(:current_user) { Fabricate(:admin) } - fab!(:user_1) { Fabricate(:user) } - fab!(:channel_1) { Fabricate(:category_channel) } - fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [current_user, user_1]) } - - before do - channel_1.add(user_1) - channel_1.add(current_user) - end - - context "when messages are created" do - xit "correctly renders notifications" do - using_session(:current_user) { visit("/") } - - using_session(:user_1) { create_message(channel: channel_1, creator: user_1) } - - using_session(:current_user) do - expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "") - expect(page).to have_css(".sidebar-row.channel-#{channel_1.id} .unread") - end - - using_session(:user_1) do |session| - create_message(channel: dm_channel_1, creator: user_1) - session.quit - end - - using_session(:current_user) do |session| - expect(page).to have_css(".sidebar-row.channel-#{dm_channel_1.id} .icon.urgent") - expect(page).to have_css( - ".chat-header-icon .chat-channel-unread-indicator", - text: "1", - ) - session.quit - end - end - end - end end end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 590b2f2f2d8..a79d717b4a7 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -7,6 +7,10 @@ module PageObjects @message_creator ||= PageObjects::Components::Chat::MessageCreator.new end + def sidebar + @sidebar ||= PageObjects::Components::Chat::Sidebar.new + end + def prefers_full_page page.execute_script( "window.localStorage.setItem('discourse_chat_preferred_mode', '\"FULL_PAGE_CHAT\"');", diff --git a/plugins/chat/spec/system/page_objects/chat/components/composer.rb b/plugins/chat/spec/system/page_objects/chat/components/composer.rb index cdb69716222..07695148542 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/composer.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/composer.rb @@ -49,26 +49,32 @@ module PageObjects end def reply_to_last_message_shortcut + input.click input.send_keys(%i[shift arrow_up]) end def edit_last_message_shortcut + input.click input.send_keys(%i[arrow_up]) end def emphasized_text_shortcut + input.click input.send_keys([PLATFORM_KEY_MODIFIER, "i"]) end def cancel_shortcut + input.click input.send_keys(:escape) end def indented_text_shortcut + input.click input.send_keys([PLATFORM_KEY_MODIFIER, "e"]) end def bold_text_shortcut + input.click input.send_keys([PLATFORM_KEY_MODIFIER, "b"]) end diff --git a/plugins/chat/spec/system/page_objects/chat/components/sidebar.rb b/plugins/chat/spec/system/page_objects/chat/components/sidebar.rb new file mode 100644 index 00000000000..95984bfbbe4 --- /dev/null +++ b/plugins/chat/spec/system/page_objects/chat/components/sidebar.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module PageObjects + module Components + module Chat + class Sidebar < PageObjects::Components::Base + attr_reader :context + + SELECTOR = "#d-sidebar" + + def component + page.find(SELECTOR) + end + + def has_direct_message_channel?(channel, **args) + channel_selector = direct_message_channel_selector(channel, **args) + predicate = component.has_css?(channel_selector) + + if args[:unread] + predicate && + component.has_css?("#{channel_selector} .sidebar-section-link-suffix.icon.unread") + elsif args[:mention] + predicate && + component.has_css?("#{channel_selector} .sidebar-section-link-suffix.icon.urgent") + else + predicate && + component.has_no_css?("#{channel_selector} .sidebar-section-link-suffix.icon") + end + end + + def has_no_direct_message_channel?(channel, **args) + component.has_no_css?(direct_message_channel_selector(channel, **args)) + end + + def direct_message_channel_selector(channel, **args) + selector = "#sidebar-section-content-chat-dms" + selector += " .sidebar-section-link.channel-#{channel.id}" + selector + end + end + end + end +end diff --git a/plugins/chat/spec/system/send_message_spec.rb b/plugins/chat/spec/system/send_message_spec.rb new file mode 100644 index 00000000000..05e49104346 --- /dev/null +++ b/plugins/chat/spec/system/send_message_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +RSpec.describe "Send message", type: :system do + fab!(:user_1) { Fabricate(:admin) } + fab!(:user_2) { Fabricate(:admin) } + + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + + before do + # simpler user search without having to worry about user search data + SiteSetting.enable_names = false + + chat_system_bootstrap + end + + context "with direct message channels" do + context "when users are not following the channel" do + fab!(:channel_1) { Fabricate(:direct_message_channel, users: [user_1, user_2]) } + + before do + channel_1.remove(user_1) + channel_1.remove(user_2) + end + + it "shows correct state" do + using_session(:user_1) do + sign_in(user_1) + visit("/") + + expect(chat_page.sidebar).to have_no_direct_message_channel(channel_1) + end + + using_session(:user_2) do + sign_in(user_2) + visit("/") + + expect(chat_page.sidebar).to have_no_direct_message_channel(channel_1) + end + + using_session(:user_1) do + chat_page.open_new_message + chat_page.message_creator.filter(user_2.username) + chat_page.message_creator.click_row(user_2) + + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + end + + using_session(:user_2) do + expect(chat_page.sidebar).to have_no_direct_message_channel(channel_1) + end + + using_session(:user_1) do |session| + channel_page.send_message + + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + + session.quit + end + + using_session(:user_2) do |session| + expect(chat_page.sidebar).to have_direct_message_channel(channel_1, mention: true) + + session.quit + end + end + end + + context "when users are following the channel" do + fab!(:channel_1) { Fabricate(:direct_message_channel, users: [user_1, user_2]) } + + it "shows correct state" do + using_session(:user_1) do + sign_in(user_1) + visit("/") + + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + end + + using_session(:user_2) do + sign_in(user_2) + visit("/") + + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + end + + using_session(:user_1) do + chat_page.open_new_message + chat_page.message_creator.filter(user_2.username) + chat_page.message_creator.click_row(user_2) + + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + end + + using_session(:user_2) do + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + end + + using_session(:user_1) do |session| + channel_page.send_message + + expect(chat_page.sidebar).to have_direct_message_channel(channel_1) + + session.quit + end + + using_session(:user_2) do |session| + expect(chat_page.sidebar).to have_direct_message_channel(channel_1, mention: true) + + session.quit + end + end + end + end +end