DEV: check if user can_chat inside the can_join_chat_channel guardian (#21812)

Someone who cannot chat is also not able to join chat channels, 
so we may not check all the time user.can_chat? && user.can_join_chat_channel? 
and just call user.can_join_chat_channel? instead.

plugins/chat/app/controllers/chat/chat_controller.rb

@@ -212,8 +212,7 @@ module Chat
           .not_suspended
           .where(id: params[:user_ids])
       users.each do |user|
-        guardian = Guardian.new(user)
-        if guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
+        if user.guardian.can_join_chat_channel?(@chat_channel)
           data = {
             message: "chat.invitation_notification",
             chat_channel_id: @chat_channel.id,

plugins/chat/app/jobs/regular/chat/notify_watching.rb

@@ -45,8 +45,7 @@ module Jobs
 
       def send_notifications(membership)
         user = membership.user
-        guardian = ::Guardian.new(user)
-        return unless guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
+        return unless user.guardian.can_join_chat_channel?(@chat_channel)
         return if ::Chat::Notifier.user_has_seen_message?(membership, @chat_message.id)
         return if online_user_ids.include?(user.id)
 

plugins/chat/lib/chat/guardian_extensions.rb

@@ -103,6 +103,7 @@ module Chat
 
     def can_join_chat_channel?(chat_channel)
       return false if anonymous?
+      return false unless can_chat?
       can_preview_chat_channel?(chat_channel) &&
         (chat_channel.direct_message_channel? || can_post_in_category?(chat_channel.chatable))
     end

plugins/chat/lib/chat/notifier.rb

@@ -159,10 +159,7 @@ module Chat
 
     def group_users_to_notify(users)
       potential_participants, unreachable =
-        users.partition do |user|
-          guardian = Guardian.new(user)
-          guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
-        end
+        users.partition { |user| user.guardian.can_join_chat_channel?(@chat_channel) }
 
       participants, welcome_to_join =
         potential_participants.partition do |participant|

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

@@ -8,7 +8,8 @@ describe Chat::ChannelFetcher do
   fab!(:dm_channel2) { Fabricate(:direct_message) }
   fab!(:direct_message_channel1) { Fabricate(:direct_message_channel, chatable: dm_channel1) }
   fab!(:direct_message_channel2) { Fabricate(:direct_message_channel, chatable: dm_channel2) }
-  fab!(:user1) { Fabricate(:user) }
+  fab!(:chatters) { Fabricate(:group) }
+  fab!(:user1) { Fabricate(:user, group_ids: [chatters.id]) }
   fab!(:user2) { Fabricate(:user) }
 
   def guardian
@@ -19,6 +20,8 @@ describe Chat::ChannelFetcher do
     Chat::UserChatChannelMembership.where(user: user1)
   end
 
+  before { SiteSetting.chat_allowed_groups = [chatters] }
+
   describe ".structured" do
     it "returns open channel only" do
       category_channel.user_chat_channel_memberships.create!(user: user1, following: true)

plugins/chat/spec/lib/chat/guardian_extensions_spec.rb

@@ -3,7 +3,8 @@
 require "rails_helper"
 
 RSpec.describe Chat::GuardianExtensions do
-  fab!(:user) { Fabricate(:user) }
+  fab!(:chatters) { Fabricate(:group) }
+  fab!(:user) { Fabricate(:user, group_ids: [chatters.id]) }
   fab!(:staff) { Fabricate(:user, admin: true) }
   fab!(:chat_group) { Fabricate(:group) }
   fab!(:channel) { Fabricate(:category_channel) }
@@ -11,6 +12,8 @@ RSpec.describe Chat::GuardianExtensions do
   let(:guardian) { Guardian.new(user) }
   let(:staff_guardian) { Guardian.new(staff) }
 
+  before { SiteSetting.chat_allowed_groups = [chatters] }
+
   it "cannot chat if the user is not in the Chat.allowed_group_ids" do
     SiteSetting.chat_allowed_groups = ""
     expect(guardian.can_chat?).to eq(false)

plugins/chat/spec/lib/chat/message_bookmarkable_spec.rb

@@ -5,7 +5,8 @@ require "rails_helper"
 describe Chat::MessageBookmarkable do
   subject(:registered_bookmarkable) { RegisteredBookmarkable.new(described_class) }
 
-  fab!(:user) { Fabricate(:user) }
+  fab!(:chatters) { Fabricate(:group) }
+  fab!(:user) { Fabricate(:user, group_ids: [chatters.id]) }
   fab!(:guardian) { Guardian.new(user) }
   fab!(:other_category) { Fabricate(:private_category, group: Fabricate(:group)) }
   fab!(:category_channel) { Fabricate(:category_channel, chatable: other_category) }
@@ -15,6 +16,7 @@ describe Chat::MessageBookmarkable do
   before do
     register_test_bookmarkable(described_class)
     Chat::UserChatChannelMembership.create(chat_channel: channel, user: user, following: true)
+    SiteSetting.chat_allowed_groups = [chatters]
   end
 
   after { DiscoursePluginRegistry.reset_register!(:bookmarkables) }

plugins/chat/spec/lib/chat/message_reactor_spec.rb

@@ -10,6 +10,8 @@ describe Chat::MessageReactor do
   fab!(:reactor) { described_class.new(reacting_user, channel) }
   fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel, user: reacting_user) }
 
+  before { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] }
+
   it "calls guardian ensure_can_join_chat_channel!" do
     Guardian.any_instance.expects(:ensure_can_join_chat_channel!).once
     message_reactor.react!(message_id: message_1.id, react_action: :add, emoji: ":+1:")

plugins/chat/spec/requests/core_ext/bookmarks_controller_spec.rb

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 
 RSpec.describe BookmarksController do
-  let(:current_user) { Fabricate(:user) }
+  fab!(:chatters) { Fabricate(:group) }
+  let(:current_user) { Fabricate(:user, group_ids: [chatters.id]) }
   let(:bookmark_message) { Fabricate(:chat_message) }
   let(:bookmark_user) { current_user }
 
   before do
     register_test_bookmarkable(Chat::MessageBookmarkable)
+    SiteSetting.chat_allowed_groups = [chatters]
     sign_in(current_user)
   end
 

plugins/chat/spec/services/chat/update_user_last_read_spec.rb

@@ -9,16 +9,19 @@ RSpec.describe Chat::UpdateUserLastRead do
   describe ".call" do
     subject(:result) { described_class.call(params) }
 
-    fab!(:current_user) { Fabricate(:user) }
+    fab!(:chatters) { Fabricate(:group) }
+    fab!(:current_user) { Fabricate(:user, group_ids: [chatters.id]) }
     fab!(:channel) { Fabricate(:chat_channel) }
-    fab!(:membership) do
+    let(:membership) do
       Fabricate(:user_chat_channel_membership, user: current_user, chat_channel: channel)
     end
-    fab!(:message_1) { Fabricate(:chat_message, chat_channel: membership.chat_channel) }
+    let(:message_1) { Fabricate(:chat_message, chat_channel: membership.chat_channel) }
 
     let(:guardian) { Guardian.new(current_user) }
     let(:params) { { guardian: guardian, channel_id: channel.id, message_id: message_1.id } }
 
+    before { SiteSetting.chat_allowed_groups = [chatters] }
+
     context "when params are not valid" do
       before { params.delete(:message_id) }
 

plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb

@@ -9,7 +9,8 @@ RSpec.describe Chat::UpdateUserThreadLastRead do
   describe ".call" do
     subject(:result) { described_class.call(params) }
 
-    fab!(:current_user) { Fabricate(:user) }
+    fab!(:chatters) { Fabricate(:group) }
+    fab!(:current_user) { Fabricate(:user, group_ids: [chatters.id]) }
     fab!(:channel) { Fabricate(:chat_channel) }
     fab!(:thread) { Fabricate(:chat_thread, channel: channel, old_om: true) }
     fab!(:thread_reply_1) { Fabricate(:chat_message, chat_channel: channel, thread: thread) }
@@ -18,6 +19,8 @@ RSpec.describe Chat::UpdateUserThreadLastRead do
     let(:guardian) { Guardian.new(current_user) }
     let(:params) { { guardian: guardian, channel_id: channel.id, thread_id: thread.id } }
 
+    before { SiteSetting.chat_allowed_groups = [chatters] }
+
     context "when params are not valid" do
       before { params.delete(:thread_id) }
 

plugins/chat/spec/system/react_to_message_spec.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 RSpec.describe "React to message", type: :system do
-  fab!(:current_user) { Fabricate(:user) }
-  fab!(:other_user) { Fabricate(:user) }
+  fab!(:current_user) { Fabricate(:user, group_ids: [Group::AUTO_GROUPS[:trust_level_1]]) }
+  fab!(:other_user) { Fabricate(:user, group_ids: [Group::AUTO_GROUPS[:trust_level_1]]) }
   fab!(:category_channel_1) { Fabricate(:category_channel) }
   fab!(:message_1) { Fabricate(:chat_message, chat_channel: category_channel_1) }
 
@@ -137,7 +137,7 @@ RSpec.describe "React to message", type: :system do
   end
 
   context "when current user and another have reacted" do
-    fab!(:other_user) { Fabricate(:user) }
+    fab!(:other_user) { Fabricate(:user, group_ids: [Group::AUTO_GROUPS[:trust_level_1]]) }
 
     fab!(:reaction_1) do
       Chat::MessageReactor.new(current_user, category_channel_1).react!(
@@ -205,7 +205,7 @@ RSpec.describe "React to message", type: :system do
     end
 
     context "when receiving a duplicate reaction event" do
-      fab!(:user_1) { Fabricate(:user) }
+      fab!(:user_1) { Fabricate(:user, group_ids: [Group::AUTO_GROUPS[:trust_level_1]]) }
 
       fab!(:reaction_2) do
         Chat::MessageReactor.new(user_1, category_channel_1).react!(