FIX: an existing member of a channel is allowed to join (#26884)

There's no point checking if a user can join a channel if they are already part of it. This case was frequent when using `enforce_membership: true` for custom bots for example.

plugins/chat/app/services/chat/create_message.rb

@@ -92,7 +92,7 @@ module Chat
     end
 
     def allowed_to_join_channel(guardian:, channel:)
-      guardian.can_join_chat_channel?(channel)
+      channel.membership_for(guardian.user) || guardian.can_join_chat_channel?(channel)
     end
 
     def fetch_channel_membership(guardian:, channel:)

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

@@ -394,7 +394,9 @@ RSpec.describe Chat::Api::ChannelMessagesController do
       end
 
       it "errors when the user is not part of the direct message channel" do
+        Chat::UserChatChannelMembership.where(user_id: user1.id).destroy_all
         Chat::DirectMessageUser.find_by(user: user1, direct_message: chatable).destroy!
+
         sign_in(user1)
         post "/chat/#{direct_message_channel.id}.json", params: { message: message }
         expect(response.status).to eq(403)

plugins/chat/spec/services/chat/create_message_spec.rb

@@ -227,9 +227,15 @@ RSpec.describe Chat::CreateMessage do
 
         context "when channel model is found" do
           context "when user can't join channel" do
-            let(:guardian) { Guardian.new }
+            let(:guardian) { other_user.guardian }
 
             it { is_expected.to fail_a_policy(:allowed_to_join_channel) }
+
+            context "when the user is already member of the channel" do
+              before { channel.add(guardian.user) }
+
+              it { is_expected.to be_a_success }
+            end
           end
 
           context "when user is system" do