FEATURE: new jump to channel menu (#22383)

This commit replaces two existing screens:
- draft
- channel selection modal

Main features compared to existing solutions
- features are now combined, meaning you can for example create multi users DM
- it will show users with chat disabled
- it shows unread state
- hopefully a better look/feel
- lots of small details and fixes...

Other noticeable fixes
- starting a DM with a user, even from the user card and clicking <kbd>Chat</kbd> will not show a green dot for the target user (or even the channel) until a message is actually sent
- it should almost never do a full page reload anymore

---------

Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Jordan Vidrine <30537603+jordanvidrine@users.noreply.github.com>
Co-authored-by: chapoi <101828855+chapoi@users.noreply.github.com>
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>

plugins/chat/spec/lib/chat/channel_fetcher_spec.rb

@@ -197,9 +197,7 @@ describe Chat::ChannelFetcher do
 
     it "does not include DM channels" do
       expect(
-        described_class.secured_public_channels(guardian, memberships, following: following).map(
-          &:id
-        ),
+        described_class.secured_public_channels(guardian, following: following).map(&:id),
       ).to match_array([category_channel.id])
     end
 
@@ -207,7 +205,6 @@ describe Chat::ChannelFetcher do
       expect(
         described_class.secured_public_channels(
           guardian,
-          memberships,
           following: following,
           filter: "support",
         ).map(&:id),
@@ -218,7 +215,6 @@ describe Chat::ChannelFetcher do
       expect(
         described_class.secured_public_channels(
           guardian,
-          memberships,
           following: following,
           filter: "cool stuff",
         ).map(&:id),
@@ -227,33 +223,29 @@ describe Chat::ChannelFetcher do
 
     it "can filter by an array of slugs" do
       expect(
-        described_class.secured_public_channels(guardian, memberships, slugs: ["support"]).map(
-          &:id
-        ),
+        described_class.secured_public_channels(guardian, slugs: ["support"]).map(&:id),
       ).to match_array([category_channel.id])
     end
 
     it "returns nothing if the array of slugs is empty" do
-      expect(
-        described_class.secured_public_channels(guardian, memberships, slugs: []).map(&:id),
-      ).to eq([])
+      expect(described_class.secured_public_channels(guardian, slugs: []).map(&:id)).to eq([])
     end
 
     it "can filter by status" do
       expect(
-        described_class.secured_public_channels(guardian, memberships, status: "closed").map(&:id),
+        described_class.secured_public_channels(guardian, status: "closed").map(&:id),
       ).to match_array([])
 
       category_channel.closed!(Discourse.system_user)
 
       expect(
-        described_class.secured_public_channels(guardian, memberships, status: "closed").map(&:id),
+        described_class.secured_public_channels(guardian, status: "closed").map(&:id),
       ).to match_array([category_channel.id])
     end
 
     it "can filter by following" do
       expect(
-        described_class.secured_public_channels(guardian, memberships, following: true).map(&:id),
+        described_class.secured_public_channels(guardian, following: true).map(&:id),
       ).to be_blank
     end
 
@@ -262,21 +254,19 @@ describe Chat::ChannelFetcher do
       another_channel = Fabricate(:category_channel)
 
       expect(
-        described_class.secured_public_channels(guardian, memberships, following: false).map(&:id),
+        described_class.secured_public_channels(guardian, following: false).map(&:id),
       ).to match_array([category_channel.id, another_channel.id])
     end
 
     it "ensures offset is >= 0" do
       expect(
-        described_class.secured_public_channels(guardian, memberships, offset: -235).map(&:id),
+        described_class.secured_public_channels(guardian, offset: -235).map(&:id),
       ).to match_array([category_channel.id])
     end
 
     it "ensures limit is > 0" do
       expect(
-        described_class.secured_public_channels(guardian, memberships, limit: -1, offset: 0).map(
-          &:id
-        ),
+        described_class.secured_public_channels(guardian, limit: -1, offset: 0).map(&:id),
       ).to match_array([category_channel.id])
     end
 
@@ -284,17 +274,15 @@ describe Chat::ChannelFetcher do
       over_limit = Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS + 1
       over_limit.times { Fabricate(:category_channel) }
 
-      expect(
-        described_class.secured_public_channels(guardian, memberships, limit: over_limit).length,
-      ).to eq(Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS)
+      expect(described_class.secured_public_channels(guardian, limit: over_limit).length).to eq(
+        Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS,
+      )
     end
 
     it "does not show the user category channels they cannot access" do
       category_channel.update!(chatable: private_category)
       expect(
-        described_class.secured_public_channels(guardian, memberships, following: following).map(
-          &:id
-        ),
+        described_class.secured_public_channels(guardian, following: following).map(&:id),
       ).to be_empty
     end
 
@@ -303,9 +291,7 @@ describe Chat::ChannelFetcher do
 
       it "only returns channels where the user is a member and is following the channel" do
         expect(
-          described_class.secured_public_channels(guardian, memberships, following: following).map(
-            &:id
-          ),
+          described_class.secured_public_channels(guardian, following: following).map(&:id),
         ).to be_empty
 
         Chat::UserChatChannelMembership.create!(
@@ -315,9 +301,7 @@ describe Chat::ChannelFetcher do
         )
 
         expect(
-          described_class.secured_public_channels(guardian, memberships, following: following).map(
-            &:id
-          ),
+          described_class.secured_public_channels(guardian, following: following).map(&:id),
         ).to match_array([category_channel.id])
       end
 
@@ -369,9 +353,9 @@ describe Chat::ChannelFetcher do
       direct_message_channel1.update!(last_message_sent_at: 1.day.ago)
       direct_message_channel2.update!(last_message_sent_at: 1.hour.ago)
 
-      expect(
-        described_class.secured_direct_message_channels(user1.id, memberships, guardian).map(&:id),
-      ).to eq([direct_message_channel2.id, direct_message_channel1.id])
+      expect(described_class.secured_direct_message_channels(user1.id, guardian).map(&:id)).to eq(
+        [direct_message_channel2.id, direct_message_channel1.id],
+      )
     end
 
     it "does not include direct message channels where the user is a member but not a direct_message_user" do
@@ -384,7 +368,7 @@ describe Chat::ChannelFetcher do
       Chat::DirectMessageUser.create!(direct_message: dm_channel1, user: user2)
 
       expect(
-        described_class.secured_direct_message_channels(user1.id, memberships, guardian).map(&:id),
+        described_class.secured_direct_message_channels(user1.id, guardian).map(&:id),
       ).not_to include(direct_message_channel1.id)
     end
 

plugins/chat/spec/requests/chat/api/chatables_controller_spec.rb

@@ -8,197 +8,35 @@ RSpec.describe Chat::Api::ChatablesController do
     SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
   end
 
-  describe "#index" do
-    fab!(:user) { Fabricate(:user, username: "johndoe", name: "John Doe") }
+  fab!(:current_user) { Fabricate(:user) }
 
+  describe "#index" do
     describe "without chat permissions" do
       it "errors errors for anon" do
-        get "/chat/api/chatables", params: { filter: "so" }
+        get "/chat/api/chatables"
         expect(response.status).to eq(403)
       end
 
       it "errors when user cannot chat" do
         SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff]
-        sign_in(user)
-        get "/chat/api/chatables", params: { filter: "so" }
+        sign_in(current_user)
+        get "/chat/api/chatables"
         expect(response.status).to eq(403)
       end
     end
 
     describe "with chat permissions" do
-      fab!(:other_user) { Fabricate(:user, username: "janemay", name: "Jane May") }
-      fab!(:admin) { Fabricate(:admin, username: "andyjones", name: "Andy Jones") }
-      fab!(:category) { Fabricate(:category) }
-      fab!(:chat_channel) { Fabricate(:category_channel, chatable: category) }
-      fab!(:dm_chat_channel) { Fabricate(:direct_message_channel, users: [user, admin]) }
+      fab!(:channel_1) { Fabricate(:chat_channel) }
 
-      before do
-        chat_channel.update(name: "something")
-        sign_in(user)
-      end
+      before { sign_in(current_user) }
+
+      it "returns results" do
+        get "/chat/api/chatables", params: { term: channel_1.name }
 
-      it "returns the correct channels with filter 'so'" do
-        get "/chat/api/chatables", params: { filter: "so" }
         expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"][0]["id"]).to eq(chat_channel.id)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-        expect(response.parsed_body["users"].count).to eq(0)
-      end
-
-      it "returns the correct channels with filter 'something'" do
-        get "/chat/api/chatables", params: { filter: "something" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"][0]["id"]).to eq(chat_channel.id)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-        expect(response.parsed_body["users"].count).to eq(0)
-      end
-
-      it "returns the correct channels with filter 'andyjones'" do
-        get "/chat/api/chatables", params: { filter: "andyjones" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-        expect(response.parsed_body["users"].count).to eq(0)
-      end
-
-      it "returns the current user inside the users array if their username matches the filter too" do
-        user.update!(username: "andysmith")
-        get "/chat/api/chatables", params: { filter: "andy" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-        expect(response.parsed_body["users"].map { |u| u["id"] }).to match_array([user.id])
-      end
-
-      it "returns no channels with a whacky filter" do
-        get "/chat/api/chatables", params: { filter: "hello good sir" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-        expect(response.parsed_body["users"].count).to eq(0)
-      end
-
-      it "only returns open channels" do
-        chat_channel.update(status: Chat::Channel.statuses[:closed])
-        get "/chat/api/chatables", params: { filter: "so" }
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-
-        chat_channel.update(status: Chat::Channel.statuses[:read_only])
-        get "/chat/api/chatables", params: { filter: "so" }
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-
-        chat_channel.update(status: Chat::Channel.statuses[:archived])
-        get "/chat/api/chatables", params: { filter: "so" }
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-
-        # Now set status to open and the channel is there!
-        chat_channel.update(status: Chat::Channel.statuses[:open])
-        get "/chat/api/chatables", params: { filter: "so" }
-        expect(response.parsed_body["public_channels"][0]["id"]).to eq(chat_channel.id)
-      end
-
-      it "only finds users by username_lower if not enable_names" do
-        SiteSetting.enable_names = false
-        get "/chat/api/chatables", params: { filter: "Andy J" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-
-        get "/chat/api/chatables", params: { filter: "andyjones" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-      end
-
-      it "only finds users by username if prioritize_username_in_ux" do
-        SiteSetting.prioritize_username_in_ux = true
-        get "/chat/api/chatables", params: { filter: "Andy J" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-
-        get "/chat/api/chatables", params: { filter: "andyjones" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-      end
-
-      it "can find users by name or username if not prioritize_username_in_ux and enable_names" do
-        SiteSetting.prioritize_username_in_ux = false
-        SiteSetting.enable_names = true
-        get "/chat/api/chatables", params: { filter: "Andy J" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-
-        get "/chat/api/chatables", params: { filter: "andyjones" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"].count).to eq(0)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-      end
-
-      it "does not return DM channels for users who do not have chat enabled" do
-        admin.user_option.update!(chat_enabled: false)
-        get "/chat/api/chatables", params: { filter: "andyjones" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-      end
-
-      xit "does not return DM channels for users who are not in the chat allowed group" do
-        group = Fabricate(:group, name: "chatpeeps")
-        SiteSetting.chat_allowed_groups = group.id
-        GroupUser.create(user: user, group: group)
-        dm_chat_channel_2 = Fabricate(:direct_message_channel, users: [user, other_user])
-
-        get "/chat/api/chatables", params: { filter: "janemay" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["direct_message_channels"].count).to eq(0)
-
-        GroupUser.create(user: other_user, group: group)
-        get "/chat/api/chatables", params: { filter: "janemay" }
-        if response.status == 500
-          puts "ERROR in ChatablesController spec:\n"
-          puts response.body
-        end
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel_2.id)
-      end
-
-      it "returns DM channels for staff users even if they are not in chat_allowed_groups" do
-        group = Fabricate(:group, name: "chatpeeps")
-        SiteSetting.chat_allowed_groups = group.id
-        GroupUser.create(user: user, group: group)
-
-        get "/chat/api/chatables", params: { filter: "andyjones" }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_chat_channel.id)
-      end
-
-      it "returns followed channels" do
-        Fabricate(
-          :user_chat_channel_membership,
-          user: user,
-          chat_channel: chat_channel,
-          following: true,
+        expect(response.parsed_body["category_channels"][0]["identifier"]).to eq(
+          "c-#{channel_1.id}",
         )
-
-        get "/chat/api/chatables", params: { filter: chat_channel.name }
-
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"][0]["id"]).to eq(chat_channel.id)
-      end
-
-      it "returns not followed channels" do
-        Fabricate(
-          :user_chat_channel_membership,
-          user: user,
-          chat_channel: chat_channel,
-          following: false,
-        )
-
-        get "/chat/api/chatables", params: { filter: chat_channel.name }
-
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["public_channels"][0]["id"]).to eq(chat_channel.id)
       end
     end
   end

plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb

@@ -49,7 +49,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do
         )
         result.channel.user_chat_channel_memberships.each do |membership|
           expect(membership).to have_attributes(
-            following: true,
+            following: false,
             muted: false,
             desktop_notification_level: "always",
             mobile_notification_level: "always",
@@ -57,12 +57,6 @@ RSpec.describe Chat::CreateDirectMessageChannel do
         end
       end
 
-      it "publishes the new channel" do
-        messages =
-          MessageBus.track_publish(Chat::Publisher::NEW_CHANNEL_MESSAGE_BUS_CHANNEL) { result }
-        expect(messages.first.data[:channel][:title]).to eq("@elaine, @lechuck")
-      end
-
       context "when there is an existing direct message channel for the target users" do
         before { described_class.call(params) }
 

plugins/chat/spec/services/chat/search_chatable_spec.rb

@@ -0,0 +1,139 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::SearchChatable do
+  describe ".call" do
+    subject(:result) { described_class.call(params) }
+
+    fab!(:current_user) { Fabricate(:user, username: "bob-user") }
+    fab!(:sam) { Fabricate(:user, username: "sam-user") }
+    fab!(:charlie) { Fabricate(:user, username: "charlie-user") }
+    fab!(:channel_1) { Fabricate(:chat_channel, name: "bob-channel") }
+    fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, sam]) }
+    fab!(:channel_3) { Fabricate(:direct_message_channel, users: [current_user, sam, charlie]) }
+    fab!(:channel_4) { Fabricate(:direct_message_channel, users: [sam, charlie]) }
+
+    let(:guardian) { Guardian.new(current_user) }
+    let(:params) { { guardian: guardian, term: term } }
+    let(:term) { "" }
+
+    before do
+      SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone]
+      # simpler user search without having to worry about user search data
+      SiteSetting.enable_names = false
+      return unless guardian.can_create_direct_message?
+      channel_1.add(current_user)
+    end
+
+    context "when all steps pass" do
+      it "sets the service result as successful" do
+        expect(result).to be_a_success
+      end
+
+      it "returns chatables" do
+        expect(result.memberships).to contain_exactly(
+          channel_1.membership_for(current_user),
+          channel_2.membership_for(current_user),
+          channel_3.membership_for(current_user),
+        )
+        expect(result.category_channels).to contain_exactly(channel_1)
+        expect(result.direct_message_channels).to contain_exactly(channel_2, channel_3)
+        expect(result.users).to include(current_user, sam)
+      end
+
+      it "doesn’t return direct message of other users" do
+        expect(result.direct_message_channels).to_not include(channel_4)
+      end
+
+      context "with private channel" do
+        fab!(:private_channel_1) { Fabricate(:private_category_channel, name: "private") }
+        let(:term) { "#private" }
+
+        it "doesn’t return category channels you can't access" do
+          expect(result.category_channels).to_not include(private_channel_1)
+        end
+      end
+    end
+
+    context "when term is prefixed with #" do
+      let(:term) { "#" }
+
+      it "doesn’t return users" do
+        expect(result.users).to be_blank
+        expect(result.category_channels).to contain_exactly(channel_1)
+        expect(result.direct_message_channels).to contain_exactly(channel_2, channel_3)
+      end
+    end
+
+    context "when term is prefixed with @" do
+      let(:term) { "@" }
+
+      it "doesn’t return channels" do
+        expect(result.users).to include(current_user, sam)
+        expect(result.category_channels).to be_blank
+        expect(result.direct_message_channels).to be_blank
+      end
+    end
+
+    context "when filtering" do
+      context "with full match" do
+        let(:term) { "bob" }
+
+        it "returns matching channels" do
+          expect(result.users).to contain_exactly(current_user)
+          expect(result.category_channels).to contain_exactly(channel_1)
+          expect(result.direct_message_channels).to contain_exactly(channel_2, channel_3)
+        end
+      end
+
+      context "with partial match" do
+        let(:term) { "cha" }
+
+        it "returns matching channels" do
+          expect(result.users).to contain_exactly(charlie)
+          expect(result.category_channels).to contain_exactly(channel_1)
+          expect(result.direct_message_channels).to contain_exactly(channel_3)
+        end
+      end
+    end
+
+    context "when filtering with non existing term" do
+      let(:term) { "xxxxxxxxxx" }
+
+      it "returns matching channels" do
+        expect(result.users).to be_blank
+        expect(result.category_channels).to be_blank
+        expect(result.direct_message_channels).to be_blank
+      end
+    end
+
+    context "when filtering with @prefix" do
+      let(:term) { "@bob" }
+
+      it "returns matching channels" do
+        expect(result.users).to contain_exactly(current_user)
+        expect(result.category_channels).to be_blank
+        expect(result.direct_message_channels).to be_blank
+      end
+    end
+
+    context "when filtering with #prefix" do
+      let(:term) { "#bob" }
+
+      it "returns matching channels" do
+        expect(result.users).to be_blank
+        expect(result.category_channels).to contain_exactly(channel_1)
+        expect(result.direct_message_channels).to contain_exactly(channel_2, channel_3)
+      end
+    end
+
+    context "when current user can't created direct messages" do
+      let(:term) { "@bob" }
+
+      before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:staff] }
+
+      it "doesn’t return users" do
+        expect(result.users).to be_blank
+      end
+    end
+  end
+end

plugins/chat/spec/support/api/schemas/user_chat_channel_membership.json

@@ -6,7 +6,8 @@
     "muted",
     "desktop_notification_level",
     "mobile_notification_level",
-    "following"
+    "following",
+    "last_viewed_at"
   ],
   "properties": {
     "chat_channel_id": { "type": "number" },
@@ -14,6 +15,7 @@
     "muted": { "type": "boolean" },
     "desktop_notification_level": { "type": "string" },
     "mobile_notification_level": { "type": "string" },
+    "last_viewed_at": { "type": "string" },
     "following": { "type": "boolean" },
     "user": {
       "type": ["object", "null"],

plugins/chat/spec/system/channel_selector_modal_spec.rb

@@ -1,80 +0,0 @@
-# frozen_string_literal: true
-
-RSpec.describe "Channel selector modal", type: :system do
-  fab!(:current_user) { Fabricate(:user) }
-
-  let(:chat_page) { PageObjects::Pages::Chat.new }
-  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
-  let(:key_modifier) { RUBY_PLATFORM =~ /darwin/i ? :meta : :control }
-
-  before do
-    chat_system_bootstrap
-    sign_in(current_user)
-    visit("/")
-  end
-
-  context "when used with public channel" do
-    fab!(:channel_1) { Fabricate(:category_channel) }
-
-    it "works" do
-      find("body").send_keys([key_modifier, "k"])
-      find("#chat-channel-selector-input").fill_in(with: channel_1.title)
-      find(".chat-channel-selection-row[data-id='#{channel_1.id}']").click
-
-      channel_page.send_message("Hello world")
-
-      expect(channel_page).to have_message(text: "Hello world")
-    end
-  end
-
-  context "when used with user" do
-    fab!(:user_1) { Fabricate(:user) }
-
-    it "works" do
-      find("body").send_keys([key_modifier, "k"])
-      find("#chat-channel-selector-input").fill_in(with: user_1.username)
-      find(".chat-channel-selection-row[data-id='#{user_1.id}']").click
-
-      channel_page.send_message("Hello world")
-
-      expect(channel_page).to have_message(text: "Hello world")
-    end
-  end
-
-  context "when used with dm channel" do
-    fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [current_user]) }
-
-    it "works" do
-      find("body").send_keys([key_modifier, "k"])
-      find("#chat-channel-selector-input").fill_in(with: current_user.username)
-      find(".chat-channel-selection-row[data-id='#{dm_channel_1.id}']").click
-      channel_page.send_message("Hello world")
-
-      expect(channel_page).to have_message(text: "Hello world")
-    end
-  end
-
-  context "when on a channel" do
-    fab!(:channel_1) { Fabricate(:category_channel) }
-
-    it "it doesn’t include current channel" do
-      chat_page.visit_channel(channel_1)
-      find("body").send_keys([key_modifier, "k"])
-      find("#chat-channel-selector-input").click
-
-      expect(page).to have_no_css(".chat-channel-selection-row[data-id='#{channel_1.id}']")
-    end
-  end
-
-  context "with limited access channels" do
-    fab!(:group_1) { Fabricate(:group) }
-    fab!(:channel_1) { Fabricate(:private_category_channel, group: group_1) }
-
-    it "it doesn’t include limited access channel" do
-      find("body").send_keys([key_modifier, "k"])
-      find("#chat-channel-selector-input").fill_in(with: channel_1.title)
-
-      expect(page).to have_no_css(".chat-channel-selection-row[data-id='#{channel_1.id}']")
-    end
-  end
-end

plugins/chat/spec/system/draft_message_spec.rb

@@ -1,26 +0,0 @@
-# frozen_string_literal: true
-
-RSpec.describe "Draft message", type: :system do
-  fab!(:current_user) { Fabricate(:admin) }
-  let(:chat_page) { PageObjects::Pages::Chat.new }
-  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
-  let(:drawer) { PageObjects::Pages::ChatDrawer.new }
-
-  before do
-    chat_system_bootstrap
-    sign_in(current_user)
-  end
-
-  context "when current user never interacted with other user" do
-    fab!(:user) { Fabricate(:user) }
-
-    it "opens channel info page" do
-      visit("/chat/draft-channel")
-      expect(page).to have_selector(".results")
-
-      find(".results .user:nth-child(1)").click
-
-      expect(channel_page).to have_no_loading_skeleton
-    end
-  end
-end

plugins/chat/spec/system/list_channels/mobile_spec.rb

@@ -121,8 +121,8 @@ RSpec.describe "List channels | mobile", type: :system, mobile: true do
 
   it "has a new dm channel button" do
     visit("/chat")
-    find(".open-draft-channel-page-btn").click
+    find(".open-new-message-btn").click
 
-    expect(page).to have_current_path("/chat/draft-channel")
+    expect(chat.message_creator).to be_opened
   end
 end

plugins/chat/spec/system/navigation_spec.rb

@@ -223,37 +223,15 @@ RSpec.describe "Navigation", type: :system do
       end
     end
 
-    context "when starting draft from sidebar with drawer preferred" do
-      it "opens draft in drawer" do
-        visit("/")
-        sidebar_page.open_draft_channel
-
-        expect(page).to have_current_path("/")
-        expect(page).to have_css(".chat-drawer.is-expanded .direct-message-creator")
-      end
-    end
-
-    context "when starting draft from drawer with drawer preferred" do
-      it "opens draft in drawer" do
-        visit("/")
-        chat_page.open_from_header
-        chat_drawer_page.open_draft_channel
-
-        expect(page).to have_current_path("/")
-        expect(page).to have_css(".chat-drawer.is-expanded .direct-message-creator")
-      end
-    end
-
     context "when starting draft from sidebar with full page preferred" do
       it "opens draft in full page" do
         visit("/")
         chat_page.open_from_header
         chat_drawer_page.maximize
         visit("/")
-        sidebar_page.open_draft_channel
+        chat_page.open_new_message
 
-        expect(page).to have_current_path("/chat/draft-channel")
-        expect(page).not_to have_css(".chat-drawer.is-expanded")
+        expect(chat_page.message_creator).to be_opened
       end
     end
 

plugins/chat/spec/system/new_message_spec.rb

@@ -0,0 +1,375 @@
+# frozen_string_literal: true
+
+RSpec.describe "New message", type: :system do
+  fab!(:current_user) { Fabricate(:admin) }
+
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+
+  before do
+    # simpler user search without having to worry about user search data
+    SiteSetting.enable_names = false
+
+    chat_system_bootstrap
+    sign_in(current_user)
+  end
+
+  it "cmd + k opens new message" do
+    visit("/")
+    chat_page.open_new_message
+
+    expect(chat_page.message_creator).to be_opened
+  end
+
+  context "when the the content is not filtered" do
+    fab!(:channel_1) { Fabricate(:chat_channel) }
+    fab!(:channel_2) { Fabricate(:chat_channel) }
+    fab!(:user_1) { Fabricate(:user) }
+    fab!(:user_2) { Fabricate(:user) }
+    fab!(:direct_message_channel_1) do
+      Fabricate(:direct_message_channel, users: [current_user, user_1])
+    end
+    fab!(:direct_message_channel_2) { Fabricate(:direct_message_channel, users: [user_1, user_2]) }
+
+    before { channel_1.add(current_user) }
+
+    it "lists channels the user is following" do
+      visit("/")
+      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_listing(user_1)
+      expect(chat_page.message_creator).to be_not_listing(user_2)
+    end
+  end
+
+  context "with no selection" do
+    context "when clicking a row" do
+      context "when the row is a channel" do
+        fab!(:channel_1) { Fabricate(:chat_channel) }
+
+        before { channel_1.add(current_user) }
+
+        it "opens the channel" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.click_row(channel_1)
+
+          expect(chat_page).to have_drawer(channel_id: channel_1.id)
+        end
+      end
+
+      context "when the row is a user" do
+        fab!(:user_1) { Fabricate(:user) }
+        fab!(:channel_1) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+
+        it "opens the channel" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.click_row(user_1)
+
+          expect(chat_page).to have_drawer(channel_id: channel_1.id)
+        end
+      end
+    end
+
+    context "when shift clicking a row" do
+      context "when the row is a channel" do
+        fab!(:channel_1) { Fabricate(:chat_channel) }
+
+        before { channel_1.add(current_user) }
+
+        it "opens the channel" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.shift_click_row(channel_1)
+
+          expect(chat_page).to have_drawer(channel_id: channel_1.id)
+        end
+      end
+
+      context "when the row is a user" do
+        fab!(:user_1) { Fabricate(:user) }
+        fab!(:channel_1) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+
+        it "adds the user" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.shift_click_row(user_1)
+
+          expect(chat_page.message_creator).to be_selecting(user_1)
+        end
+      end
+    end
+
+    context "when pressing enter" do
+      context "when the row is a channel" do
+        fab!(:channel_1) { Fabricate(:chat_channel) }
+
+        before { channel_1.add(current_user) }
+
+        it "opens the channel" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.click_row(channel_1)
+
+          expect(chat_page).to have_drawer(channel_id: channel_1.id)
+        end
+      end
+
+      context "when the row is a user" do
+        fab!(:user_1) { Fabricate(:user) }
+        fab!(:channel_1) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+
+        it "opens the channel" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.click_row(user_1)
+
+          expect(chat_page).to have_drawer(channel_id: channel_1.id)
+        end
+      end
+    end
+
+    context "when pressing shift+enter" do
+      context "when the row is a channel" do
+        fab!(:channel_1) { Fabricate(:chat_channel) }
+
+        before { channel_1.add(current_user) }
+
+        it "opens the channel" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.shift_enter_shortcut
+
+          expect(chat_page).to have_drawer(channel_id: channel_1.id)
+        end
+      end
+
+      context "when the row is a user" do
+        fab!(:user_1) { Fabricate(:user) }
+        fab!(:channel_1) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+
+        it "adds the user" do
+          visit("/")
+          chat_page.open_new_message
+          chat_page.message_creator.shift_enter_shortcut
+
+          expect(chat_page.message_creator).to be_selecting(user_1)
+        end
+      end
+    end
+
+    context "when navigating content with arrows" do
+      fab!(:channel_1) { Fabricate(:chat_channel, name: "channela") }
+      fab!(:channel_2) { Fabricate(:chat_channel, name: "channelb") }
+
+      before do
+        channel_1.add(current_user)
+        channel_2.add(current_user)
+      end
+
+      it "changes active content" do
+        visit("/")
+        chat_page.open_new_message
+
+        expect(chat_page.message_creator).to be_listing(channel_1, active: true)
+
+        chat_page.message_creator.arrow_down_shortcut
+
+        expect(chat_page.message_creator).to be_listing(channel_2, active: true)
+
+        chat_page.message_creator.arrow_down_shortcut
+
+        expect(chat_page.message_creator).to be_listing(channel_1, active: true)
+
+        chat_page.message_creator.arrow_up_shortcut
+
+        expect(chat_page.message_creator).to be_listing(channel_2, active: true)
+      end
+    end
+
+    context "with disabled content" do
+      fab!(:user_1) { Fabricate(:user) }
+      fab!(:channel_1) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+
+      before { user_1.user_option.update!(chat_enabled: false) }
+
+      it "doesn’t make the content active" do
+        visit("/")
+        chat_page.open_new_message
+
+        expect(chat_page.message_creator).to be_listing(user_1, inactive: true, disabled: true)
+      end
+    end
+  end
+
+  context "when filtering" do
+    fab!(:channel_1) { Fabricate(:chat_channel, name: "bob-channel") }
+    fab!(:user_1) { Fabricate(:user, username: "bob-user") }
+    fab!(:user_2) { Fabricate(:user) }
+    fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+    fab!(:channel_3) { Fabricate(:direct_message_channel, users: [current_user, user_1, user_2]) }
+
+    before { channel_1.add(current_user) }
+
+    context "with no prefix" do
+      it "lists all matching content" do
+        visit("/")
+        chat_page.open_new_message
+
+        chat_page.message_creator.filter("bob")
+
+        expect(chat_page.message_creator).to be_listing(channel_1)
+        expect(chat_page.message_creator).to be_not_listing(channel_2)
+        expect(chat_page.message_creator).to be_listing(channel_3)
+        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 channel prefix" do
+      it "lists matching channel" do
+        visit("/")
+        chat_page.open_new_message
+
+        chat_page.message_creator.filter("#bob")
+
+        expect(chat_page.message_creator).to be_listing(channel_1)
+        expect(chat_page.message_creator).to be_not_listing(channel_2)
+        expect(chat_page.message_creator).to be_listing(channel_3)
+        expect(chat_page.message_creator).to be_not_listing(user_1)
+        expect(chat_page.message_creator).to be_not_listing(user_2)
+      end
+    end
+
+    context "with user prefix" do
+      it "lists matching users" do
+        visit("/")
+        chat_page.open_new_message
+
+        chat_page.message_creator.filter("@bob")
+
+        expect(chat_page.message_creator).to be_not_listing(channel_1)
+        expect(chat_page.message_creator).to be_not_listing(channel_2)
+        expect(chat_page.message_creator).to be_not_listing(channel_3)
+        expect(chat_page.message_creator).to be_listing(user_1)
+        expect(chat_page.message_creator).to be_not_listing(user_2)
+      end
+    end
+  end
+
+  context "with selection" do
+    fab!(:channel_1) { Fabricate(:chat_channel, name: "bob-channel") }
+    fab!(:user_1) { Fabricate(:user, username: "bob-user") }
+    fab!(:user_2) { Fabricate(:user, username: "bobby-user") }
+    fab!(:user_3) { Fabricate(:user, username: "sam-user") }
+    fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
+    fab!(:channel_3) { Fabricate(:direct_message_channel, users: [current_user, user_2]) }
+
+    before do
+      channel_1.add(current_user)
+      visit("/")
+      chat_page.open_new_message
+      chat_page.message_creator.shift_click_row(user_1)
+    end
+
+    context "when pressing enter" do
+      it "opens the channel" do
+        chat_page.message_creator.enter_shortcut
+
+        expect(chat_page).to have_drawer(channel_id: channel_2.id)
+      end
+    end
+
+    context "when clicking cta" do
+      it "opens the channel" do
+        chat_page.message_creator.click_cta
+
+        expect(chat_page).to have_drawer(channel_id: channel_2.id)
+      end
+    end
+
+    context "when filtering" do
+      it "shows only matching users regarless of prefix" do
+        chat_page.message_creator.filter("#bob")
+
+        expect(chat_page.message_creator).to be_listing(user_1)
+        expect(chat_page.message_creator).to be_listing(user_2)
+        expect(chat_page.message_creator).to be_not_listing(user_3)
+        expect(chat_page.message_creator).to be_not_listing(channel_1)
+        expect(chat_page.message_creator).to be_not_listing(channel_2)
+        expect(chat_page.message_creator).to be_not_listing(channel_3)
+      end
+
+      it "shows selected user as selected in content" do
+        chat_page.message_creator.filter("@bob")
+
+        expect(chat_page.message_creator).to be_listing(user_1, selected: true)
+        expect(chat_page.message_creator).to be_listing(user_2, selected: false)
+      end
+    end
+
+    context "when clicking another user" do
+      it "adds it to the selection" do
+        chat_page.message_creator.filter("@bob")
+        chat_page.message_creator.click_row(user_2)
+
+        expect(chat_page.message_creator).to be_selecting(user_1)
+        expect(chat_page.message_creator).to be_selecting(user_2)
+      end
+    end
+
+    context "when pressing backspace" do
+      it "removes it" do
+        chat_page.message_creator.backspace_shortcut
+
+        expect(chat_page.message_creator).to be_selecting(user_1, active: true)
+
+        chat_page.message_creator.backspace_shortcut
+
+        expect(chat_page.message_creator).to be_not_selecting(user_1)
+      end
+    end
+
+    context "when navigating selection with arrow left/right" do
+      it "changes active item" do
+        chat_page.message_creator.filter("@bob")
+        chat_page.message_creator.click_row(user_2)
+
+        chat_page.message_creator.arrow_left_shortcut
+
+        expect(chat_page.message_creator).to be_selecting(user_2, active: true)
+
+        chat_page.message_creator.arrow_left_shortcut
+
+        expect(chat_page.message_creator).to be_selecting(user_1, active: true)
+
+        chat_page.message_creator.arrow_left_shortcut
+
+        expect(chat_page.message_creator).to be_selecting(user_2, active: true)
+
+        chat_page.message_creator.arrow_right_shortcut
+
+        expect(chat_page.message_creator).to be_selecting(user_1, active: true)
+      end
+    end
+
+    context "when clicking selection" do
+      it "removes it" do
+        chat_page.message_creator.click_item(user_1)
+
+        expect(chat_page.message_creator).to be_not_selecting(user_1)
+      end
+    end
+  end
+end

plugins/chat/spec/system/page_objects/chat/chat.rb

@@ -3,6 +3,12 @@
 module PageObjects
   module Pages
     class Chat < PageObjects::Pages::Base
+      MODIFIER = RUBY_PLATFORM =~ /darwin/i ? :meta : :control
+
+      def message_creator
+        @message_creator ||= PageObjects::Components::Chat::MessageCreator.new
+      end
+
       def prefers_full_page
         page.execute_script(
           "window.localStorage.setItem('discourse_chat_preferred_mode', '\"FULL_PAGE_CHAT\"');",
@@ -17,6 +23,10 @@ module PageObjects
         visit("/chat")
       end
 
+      def open_new_message
+        send_keys([MODIFIER, "k"])
+      end
+
       def has_drawer?(channel_id: nil, expanded: true)
         drawer?(expectation: true, channel_id: channel_id, expanded: expanded)
       end

plugins/chat/spec/system/page_objects/chat/components/composer.rb

@@ -18,6 +18,10 @@ module PageObjects
           input.value.blank?
         end
 
+        def enabled?
+          component.has_css?(".chat-composer.is-enabled")
+        end
+
         def has_saved_draft?
           component.has_css?(".chat-composer.is-draft-saved")
         end

plugins/chat/spec/system/page_objects/chat/components/message_creator.rb

@@ -0,0 +1,131 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Components
+    module Chat
+      class MessageCreator < PageObjects::Components::Base
+        attr_reader :context
+
+        SELECTOR = ".chat-new-message-modal"
+
+        def component
+          find(SELECTOR)
+        end
+
+        def input
+          component.find(".chat-message-creator__input")
+        end
+
+        def filter(query = "")
+          input.fill_in(with: query)
+        end
+
+        def opened?
+          page.has_css?(SELECTOR)
+        end
+
+        def enter_shortcut
+          input.send_keys(:enter)
+        end
+
+        def backspace_shortcut
+          input.send_keys(:backspace)
+        end
+
+        def shift_enter_shortcut
+          input.send_keys(:shift, :enter)
+        end
+
+        def click_cta
+          component.find(".chat-message-creator__open-dm-btn").click
+        end
+
+        def arrow_left_shortcut
+          input.send_keys(:arrow_left)
+        end
+
+        def arrow_right_shortcut
+          input.send_keys(:arrow_right)
+        end
+
+        def arrow_down_shortcut
+          input.send_keys(:arrow_down)
+        end
+
+        def arrow_up_shortcut
+          input.send_keys(:arrow_up)
+        end
+
+        def listing?(chatable, **args)
+          component.has_css?(build_row_selector(chatable, **args))
+        end
+
+        def not_listing?(chatable, **args)
+          component.has_no_css?(build_row_selector(chatable, **args))
+        end
+
+        def selecting?(chatable, **args)
+          component.has_css?(build_item_selector(chatable, **args))
+        end
+
+        def not_selecting?(chatable, **args)
+          component.has_no_css?(build_item_selector(chatable, **args))
+        end
+
+        def click_item(chatable, **args)
+          component.find(build_item_selector(chatable, **args)).click
+        end
+
+        def click_row(chatable, **args)
+          component.find(build_row_selector(chatable, **args)).click
+        end
+
+        def shift_click_row(chatable, **args)
+          component.find(build_row_selector(chatable, **args)).click(:shift)
+        end
+
+        def build_item_selector(chatable, **args)
+          selector = ".chat-message-creator__selection-item"
+          selector += content_selector(**args)
+          selector += chatable_selector(chatable)
+          selector
+        end
+
+        def build_row_selector(chatable, **args)
+          selector = ".chat-message-creator__row"
+          selector += content_selector(**args)
+          selector += chatable_selector(chatable)
+          selector
+        end
+
+        def content_selector(**args)
+          selector = ""
+          selector = ".-disabled" if args[:disabled]
+          selector = ".-selected" if args[:selected]
+          selector = ":not(.-disabled)" if args[:enabled]
+          if args[:active]
+            selector += ".-active"
+          elsif args[:inactive]
+            selector += ":not(.-active)"
+          end
+          selector
+        end
+
+        def chatable_selector(chatable)
+          selector = ""
+          if chatable.try(:category_channel?)
+            selector += ".-channel"
+            selector += "[data-id='c-#{chatable.id}']"
+          elsif chatable.try(:direct_message_channel?)
+            selector += ".-channel"
+            selector += "[data-id='c-#{chatable.id}']"
+          else
+            selector += ".-user"
+            selector += "[data-id='u-#{chatable.id}']"
+          end
+          selector
+        end
+      end
+    end
+  end
+end

plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb

@@ -8,10 +8,6 @@ module PageObjects
         find("#{VISIBLE_DRAWER} .open-browse-page-btn").click
       end
 
-      def open_draft_channel
-        find("#{VISIBLE_DRAWER} .open-draft-channel-page-btn").click
-      end
-
       def close
         find("#{VISIBLE_DRAWER} .chat-drawer-header__close-btn").click
       end

plugins/chat/spec/system/page_objects/sidebar/sidebar.rb

@@ -11,13 +11,6 @@ module PageObjects
         find(".sidebar-section[data-section-name='chat-dms']")
       end
 
-      def open_draft_channel
-        find(
-          ".sidebar-section[data-section-name='chat-dms'] .sidebar-section-header-button",
-          visible: false,
-        ).click
-      end
-
       def open_browse
         find(
           ".sidebar-section[data-section-name='chat-channels'] .sidebar-section-header-button",

plugins/chat/spec/system/visit_channel_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe "Visit channel", type: :system do
   fab!(:inaccessible_dm_channel_1) { Fabricate(:direct_message_channel) }
 
   let(:chat) { PageObjects::Pages::Chat.new }
+  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
 
   before { chat_system_bootstrap }
 
@@ -143,13 +144,7 @@ RSpec.describe "Visit channel", type: :system do
           it "allows to join it" do
             chat.visit_channel(dm_channel_1)
 
-            expect(page).to have_content(I18n.t("js.chat.channel_settings.join_channel"))
-          end
-
-          it "shows a preview of the channel" do
-            chat.visit_channel(dm_channel_1)
-
-            expect(chat).to have_message(message_1)
+            expect(channel_page.composer).to be_enabled
           end
         end
       end