diff --git a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb index 102ffaf0970..6b02983982b 100644 --- a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb @@ -64,7 +64,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController on_failed_policy(:no_silenced_user) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise Discourse::NotFound } on_failed_policy(:allowed_to_join_channel) { raise Discourse::InvalidAccess } - on_model_not_found(:channel_membership) { raise Discourse::InvalidAccess } + on_model_not_found(:membership) { raise Discourse::NotFound } on_failed_policy(:ensure_reply_consistency) { raise Discourse::NotFound } on_failed_policy(:allowed_to_create_message_in_channel) do |policy| render_json_error(policy.reason) diff --git a/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb b/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb index fed94d60342..af2241f5a9f 100644 --- a/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_messages_streaming_controller.rb @@ -6,7 +6,7 @@ class Chat::Api::ChannelsMessagesStreamingController < Chat::Api::ChannelsContro on_success { render(json: success_json) } on_failure { render(json: failed_json, status: 422) } on_model_not_found(:message) { raise Discourse::NotFound } - on_failed_policy(:can_join_channel) { raise Discourse::InvalidAccess } + on_model_not_found(:membership) { raise Discourse::NotFound } on_failed_policy(:can_stop_streaming) { raise Discourse::InvalidAccess } on_failed_contract do |contract| render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index f0d7d172f29..20daafd3818 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -24,10 +24,9 @@ module Chat policy :no_silenced_user contract model :channel - step :enforce_system_membership - policy :allowed_to_join_channel + step :enforce_membership + model :membership policy :allowed_to_create_message_in_channel, class_name: Chat::Channel::MessageCreationPolicy - model :channel_membership model :reply, optional: true policy :ensure_reply_consistency model :thread, optional: true @@ -81,8 +80,8 @@ module Chat Chat::Channel.find_by_id_or_slug(contract.chat_channel_id) end - def enforce_system_membership(guardian:, channel:, contract:) - if guardian.user&.is_system_user? || contract.enforce_membership + def enforce_membership(guardian:, channel:, contract:) + if guardian.user.bot? || contract.enforce_membership channel.add(guardian.user) if channel.direct_message_channel? @@ -91,12 +90,8 @@ module Chat end end - def allowed_to_join_channel(guardian:, channel:) - channel.membership_for(guardian.user) || guardian.can_join_chat_channel?(channel) - end - - def fetch_channel_membership(guardian:, channel:) - Chat::ChannelMembershipManager.new(channel).find_for_user(guardian.user) + def fetch_membership(guardian:, channel:) + channel.membership_for(guardian.user) end def fetch_reply(contract:) @@ -187,15 +182,13 @@ module Chat channel.update!(last_message: message_instance) end - def update_membership_last_read(channel_membership:, message_instance:) + def update_membership_last_read(membership:, message_instance:) return if message_instance.in_thread? - channel_membership.update!(last_read_message: message_instance) + membership.update!(last_read_message: message_instance) end - def process_direct_message_channel(channel_membership:) - Chat::Action::PublishAndFollowDirectMessageChannel.call( - channel_membership: channel_membership, - ) + def process_direct_message_channel(membership:) + Chat::Action::PublishAndFollowDirectMessageChannel.call(channel_membership: membership) end def publish_new_thread(reply:, contract:, channel:, thread:) @@ -238,10 +231,10 @@ module Chat message_instance.excerpt = message_instance.build_excerpt end - def publish_user_tracking_state(message_instance:, channel:, channel_membership:, guardian:) + def publish_user_tracking_state(message_instance:, channel:, membership:, guardian:) message_to_publish = message_instance message_to_publish = - channel_membership.last_read_message || message_instance if message_instance.in_thread? + membership.last_read_message || message_instance if message_instance.in_thread? Chat::Publisher.publish_user_tracking_state!(guardian.user, channel, message_to_publish) end end diff --git a/plugins/chat/app/services/chat/stop_message_streaming.rb b/plugins/chat/app/services/chat/stop_message_streaming.rb index 661c2a5504a..730c93b749d 100644 --- a/plugins/chat/app/services/chat/stop_message_streaming.rb +++ b/plugins/chat/app/services/chat/stop_message_streaming.rb @@ -15,7 +15,8 @@ module Chat # @return [Service::Base::Context] contract model :message - policy :can_join_channel + step :enforce_membership + model :membership policy :can_stop_streaming step :stop_message_streaming step :publish_message_streaming_state @@ -33,12 +34,16 @@ module Chat ::Chat::Message.find_by(id: contract.message_id) end - def can_join_channel(guardian:, message:) - guardian.can_join_chat_channel?(message.chat_channel) + def enforce_membership(guardian:, message:) + message.chat_channel.add(guardian.user) if guardian.user.bot? + end + + def fetch_membership(guardian:, message:) + message.chat_channel.membership_for(guardian.user) end def can_stop_streaming(guardian:, message:) - guardian.is_admin? || message.user.id == guardian.user.id || + guardian.user.bot? || guardian.is_admin? || message.user.id == guardian.user.id || message.in_reply_to && message.in_reply_to.user_id == guardian.user.id end diff --git a/plugins/chat/app/services/chat/update_message.rb b/plugins/chat/app/services/chat/update_message.rb index 014994ecd6e..2bf2ecbb746 100644 --- a/plugins/chat/app/services/chat/update_message.rb +++ b/plugins/chat/app/services/chat/update_message.rb @@ -18,7 +18,8 @@ module Chat contract model :message model :uploads, optional: true - step :enforce_system_membership + step :enforce_membership + model :membership policy :can_modify_channel_message policy :can_modify_message step :clean_message @@ -49,8 +50,8 @@ module Chat private - def enforce_system_membership(guardian:, message:) - message.chat_channel.add(guardian.user) if guardian.user.is_system_user? + def enforce_membership(guardian:, message:) + message.chat_channel.add(guardian.user) if guardian.user.bot? end def fetch_message(contract:) @@ -71,6 +72,10 @@ module Chat ).find_by(id: contract.message_id) end + def fetch_membership(guardian:, message:) + message.chat_channel.membership_for(guardian.user) + end + def fetch_uploads(contract:, guardian:) return if !SiteSetting.chat_allow_uploads guardian.user.uploads.where(id: contract.upload_ids) diff --git a/plugins/chat/lib/chat_sdk/message.rb b/plugins/chat/lib/chat_sdk/message.rb index 21103cf63ec..0e4ba3c5d72 100644 --- a/plugins/chat/lib/chat_sdk/message.rb +++ b/plugins/chat/lib/chat_sdk/message.rb @@ -94,6 +94,9 @@ module ChatSDK with_service(Chat::StopMessageStreaming, message_id: message_id, guardian: guardian) do on_success { result.message } on_model_not_found(:message) { raise "Couldn't find message with id: `#{message_id}`" } + on_model_not_found(:membership) do + raise "Couldn't find membership for user with id: `#{guardian.user.id}`" + end on_failed_policy(:can_join_channel) do raise "User with id: `#{guardian.user.id}` can't join this channel" end @@ -132,8 +135,8 @@ module ChatSDK strip_whitespaces: strip_whitespaces, ) do on_model_not_found(:channel) { raise "Couldn't find channel with id: `#{channel_id}`" } - on_model_not_found(:channel_membership) do - raise "User with id: `#{guardian.user.id}` has no membership to this channel" + on_model_not_found(:membership) do + raise "Couldn't find membership for user with id: `#{guardian.user.id}`" end on_failed_policy(:ensure_valid_thread_for_channel) do raise "Couldn't find thread with id: `#{thread_id}`" diff --git a/plugins/chat/spec/lib/chat_sdk/message_spec.rb b/plugins/chat/spec/lib/chat_sdk/message_spec.rb index 07aacdac01a..045762edae7 100644 --- a/plugins/chat/spec/lib/chat_sdk/message_spec.rb +++ b/plugins/chat/spec/lib/chat_sdk/message_spec.rb @@ -48,7 +48,7 @@ describe ChatSDK::Message do params[:guardian] = Fabricate(:user).guardian expect { described_class.create(**params) }.to raise_error( - "User with id: `#{params[:guardian].user.id}` can't join this channel", + "Couldn't find membership for user with id: `#{params[:guardian].user.id}`", ) end end @@ -111,6 +111,7 @@ describe ChatSDK::Message do before do SiteSetting.chat_allowed_groups = [Group::AUTO_GROUPS[:everyone]] + message_1.chat_channel.add(message_1.user) message_1.update!(streaming: true) end @@ -131,7 +132,7 @@ describe ChatSDK::Message do end end - context "when user can't join channel" do + context "when user is not part of the channel" do fab!(:message_1) do Fabricate(:chat_message, chat_channel: Fabricate(:private_category_channel)) end @@ -141,7 +142,7 @@ describe ChatSDK::Message do expect { described_class.stop_stream(message_id: message_1.id, guardian: user.guardian) - }.to raise_error("User with id: `#{user.id}` can't join this channel") + }.to raise_error("Couldn't find membership for user with id: `#{user.id}`") end end end @@ -167,7 +168,11 @@ describe ChatSDK::Message do describe ".stream" do fab!(:message_1) { Fabricate(:chat_message, message: "first\n") } - before { message_1.update!(streaming: true) } + + before do + message_1.chat_channel.add(message_1.user) + message_1.update!(streaming: true) + end it "streams" do edit = diff --git a/plugins/chat/spec/models/chat/message_spec.rb b/plugins/chat/spec/models/chat/message_spec.rb index cb294315a36..264c3f8bfff 100644 --- a/plugins/chat/spec/models/chat/message_spec.rb +++ b/plugins/chat/spec/models/chat/message_spec.rb @@ -513,6 +513,8 @@ describe Chat::Message do it "keeps the same hashtags the user has permission to after rebake" do group.add(chat_message.user) + chat_message.chat_channel.add(chat_message.user) + update_message!( chat_message, user: chat_message.user, diff --git a/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb index c8493eb79f0..63c12100989 100644 --- a/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_messages_controller_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Chat::Api::ChannelMessagesController do sign_in(current_user) end - describe "index" do + describe "#index" do describe "success" do fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel) } fab!(:message_2) { Fabricate(:chat_message) } @@ -92,4 +92,50 @@ RSpec.describe Chat::Api::ChannelMessagesController do end end end + + describe "#update" do + context "when message is updated" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel, user: current_user) } + it "works" do + put "/chat/api/channels/#{channel.id}/messages/#{message_1.id}", + params: { + message: "abcdefg", + } + + expect(response.status).to eq(200) + expect(message_1.reload.message).to eq("abcdefg") + end + + context "when params are invalid" do + it "returns a 400" do + put "/chat/api/channels/#{channel.id}/messages/#{message_1.id}" + + expect(response.status).to eq(400) + end + end + + context "when user is not part of the channel" do + before { channel.membership_for(current_user).destroy! } + + it "returns a 404" do + put "/chat/api/channels/#{channel.id}/messages/#{message_1.id}" + + expect(response.status).to eq(400) + end + end + + context "when user is not the author" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel) } + + it "returns a 422" do + put "/chat/api/channels/#{channel.id}/messages/#{message_1.id}", + params: { + message: "abcdefg", + } + + expect(response.status).to eq(422) + end + end + end + end end diff --git a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb index 039a2c52ad7..84c4915bb10 100644 --- a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb @@ -105,6 +105,7 @@ RSpec.describe Chat::Api::ChannelThreadsController do end before do + public_channel.add(current_user) thread_1.add(current_user) thread_3.add(current_user) end @@ -118,6 +119,7 @@ RSpec.describe Chat::Api::ChannelThreadsController do end it "has preloaded chat mentions and users for the thread original message" do + public_channel.add(thread_1.original_message.user) update_message!( thread_1.original_message, user: thread_1.original_message.user, diff --git a/plugins/chat/spec/requests/chat/api/channels_messages_streaming_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channels_messages_streaming_controller_spec.rb index 6170a36abc4..247d9c12ff3 100644 --- a/plugins/chat/spec/requests/chat/api/channels_messages_streaming_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channels_messages_streaming_controller_spec.rb @@ -43,6 +43,8 @@ RSpec.describe Chat::Api::ChannelsMessagesStreamingController do context "when the user can’t stop" do fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + before { channel_1.add(current_user) } + it "returns a 403 error" do delete "/chat/api/channels/#{channel_1.id}/messages/#{message_1.id}/streaming" @@ -50,9 +52,21 @@ RSpec.describe Chat::Api::ChannelsMessagesStreamingController do end end + context "when the user is not a member" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, user: current_user) } + + it "returns a 404 error" do + delete "/chat/api/channels/#{channel_1.id}/messages/#{message_1.id}/streaming" + + expect(response.status).to eq(404) + end + end + context "when the user can stop" do fab!(:current_user) { Fabricate(:admin) } - fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, user: current_user) } + + before { channel_1.add(current_user) } it "returns a 200" do delete "/chat/api/channels/#{channel_1.id}/messages/#{message_1.id}/streaming" diff --git a/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb b/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb index cf38a20973f..a7e4dd44c99 100644 --- a/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb @@ -207,12 +207,13 @@ RSpec.describe Chat::Api::ChannelMessagesController do describe "for category" do fab!(:chat_channel) { Fabricate(:category_channel, chatable: category) } + before do + chat_channel.add(user) + sign_in(user) + end + context "when current user is silenced" do - before do - chat_channel.add(user) - sign_in(user) - UserSilencer.new(user).silence - end + before { UserSilencer.new(user).silence } it "raises invalid acces" do post "/chat/#{chat_channel.id}.json", params: { message: message } @@ -221,24 +222,20 @@ RSpec.describe Chat::Api::ChannelMessagesController do end it "errors for regular user when chat is staff-only" do - sign_in(user) SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff] post "/chat/#{chat_channel.id}.json", params: { message: message } expect(response.status).to eq(403) end - it "errors when the user isn't following the channel" do - sign_in(user) + it "errors when the user isn't member of the channel" do + chat_channel.membership_for(user).destroy! post "/chat/#{chat_channel.id}.json", params: { message: message } - expect(response.status).to eq(403) + expect(response.status).to eq(404) end it "errors when the user is not staff and the channel is not open" do - Fabricate(:user_chat_channel_membership, chat_channel: chat_channel, user: user) - sign_in(user) - chat_channel.update(status: :closed) post "/chat/#{chat_channel.id}.json", params: { message: message } expect(response.status).to eq(422) @@ -247,20 +244,21 @@ RSpec.describe Chat::Api::ChannelMessagesController do ) end - it "errors when the user is staff and the channel is not open or closed" do - Fabricate(:user_chat_channel_membership, chat_channel: chat_channel, user: admin) - sign_in(admin) + context "when the user is staff" do + fab!(:user) { Fabricate(:admin) } - chat_channel.update(status: :closed) - post "/chat/#{chat_channel.id}.json", params: { message: message } - expect(response.status).to eq(200) + it "errors when the channel is not open or closed" do + chat_channel.update(status: :closed) + post "/chat/#{chat_channel.id}.json", params: { message: message } + expect(response.status).to eq(200) - chat_channel.update(status: :read_only) - post "/chat/#{chat_channel.id}.json", params: { message: message } - expect(response.status).to eq(422) - expect(response.parsed_body["errors"]).to include( - I18n.t("chat.errors.channel_new_message_disallowed.read_only"), - ) + chat_channel.update(status: :read_only) + post "/chat/#{chat_channel.id}.json", params: { message: message } + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to include( + I18n.t("chat.errors.channel_new_message_disallowed.read_only"), + ) + end end context "when the regular user is following the channel" do @@ -275,8 +273,6 @@ RSpec.describe Chat::Api::ChannelMessagesController do end it "sends a message for regular user when staff-only is disabled and they are following channel" do - sign_in(user) - expect { post "/chat/#{chat_channel.id}.json", params: { message: message } }.to change { Chat::Message.count }.by(1) @@ -292,7 +288,6 @@ RSpec.describe Chat::Api::ChannelMessagesController do end it "publishes user tracking state using the new chat message as the last_read_message_id" do - sign_in(user) messages = MessageBus.track_publish( Chat::Publisher.user_tracking_state_message_bus_channel(user.id), @@ -306,8 +301,6 @@ RSpec.describe Chat::Api::ChannelMessagesController do Fabricate(:chat_thread, channel: chat_channel, original_message: message_1) end - before { sign_in(user) } - it "does not update the last_read_message_id for the user who sent the message" do post "/chat/#{chat_channel.id}.json", params: { message: message, thread_id: thread.id } expect(response.status).to eq(200) @@ -332,9 +325,7 @@ RSpec.describe Chat::Api::ChannelMessagesController do context "when thread is not part of the provided channel" do let!(:another_channel) { Fabricate(:category_channel) } - before do - Fabricate(:user_chat_channel_membership, chat_channel: another_channel, user: user) - end + before { another_channel.add(user) } it "returns an error" do post "/chat/#{another_channel.id}.json", @@ -399,7 +390,7 @@ RSpec.describe Chat::Api::ChannelMessagesController do sign_in(user1) post "/chat/#{direct_message_channel.id}.json", params: { message: message } - expect(response.status).to eq(403) + expect(response.status).to eq(404) Chat::UserChatChannelMembership.find_by(user_id: user2.id).update!(following: true) sign_in(user2) diff --git a/plugins/chat/spec/services/chat/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb index b4d8ae1242d..51b4172e4ef 100644 --- a/plugins/chat/spec/services/chat/create_message_spec.rb +++ b/plugins/chat/spec/services/chat/create_message_spec.rb @@ -47,6 +47,8 @@ RSpec.describe Chat::CreateMessage do end let(:message) { result[:message_instance].reload } + before { channel.add(guardian.user) } + shared_examples "creating a new message" do it "saves the message" do expect { result }.to change { Chat::Message.count }.by(1) @@ -226,19 +228,13 @@ RSpec.describe Chat::CreateMessage do end context "when channel model is found" do - context "when user can't join channel" do - let(:guardian) { other_user.guardian } + context "when user is not part of the channel" do + before { channel.membership_for(user).destroy! } - 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 + it { is_expected.to fail_to_find_a_model(:membership) } end - context "when user is system" do + context "when user is a bot" do fab!(:user) { Discourse.system_user } it { is_expected.to be_a_success } @@ -265,17 +261,13 @@ RSpec.describe Chat::CreateMessage do end context "when user can create a message in the channel" do - context "when user is not a member of the channel" do - it { is_expected.to fail_to_find_a_model(:channel_membership) } - end - context "when user is a member of the channel" do fab!(:existing_message) { Fabricate(:chat_message, chat_channel: channel) } - let(:membership) { Chat::UserChatChannelMembership.last } + let(:membership) { channel.membership_for(user) } before do - channel.add(user).update!(last_read_message: existing_message) + membership.update!(last_read_message: existing_message) DiscourseEvent.stubs(:trigger) end diff --git a/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb b/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb index d8a224dfc3f..b3d1e3a3378 100644 --- a/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb +++ b/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb @@ -4,19 +4,20 @@ RSpec.describe Chat::StopMessageStreaming do describe ".call" do subject(:result) { described_class.call(params) } - let(:params) { { guardian: guardian } } let(:guardian) { Guardian.new(current_user) } + let(:params) { { guardian: guardian, message_id: message_1.id } } fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, streaming: true) } - before { SiteSetting.chat_allowed_groups = [Group::AUTO_GROUPS[:everyone]] } + before do + channel_1.add(current_user) + SiteSetting.chat_allowed_groups = [Group::AUTO_GROUPS[:everyone]] + end context "with valid params" do fab!(:current_user) { Fabricate(:admin) } - fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, streaming: true) } - - let(:params) { { guardian: guardian, channel_id: channel_1.id, message_id: message_1.id } } it { is_expected.to be_a_success } @@ -33,25 +34,31 @@ RSpec.describe Chat::StopMessageStreaming do end end - context "when the channel_id is not provided" do - it { is_expected.to fail_a_contract } + context "when the user is not part of the channel" do + before { channel_1.membership_for(current_user).destroy! } + + it { is_expected.to fail_to_find_a_model(:membership) } + + context "when the user is a bot" do + fab!(:current_user) { Discourse.system_user } + + it { is_expected.to be_a_success } + end end context "when the message_id is not provided" do - let(:params) { { guardian: guardian, channel_id: channel_1.id } } + before { params[:message_id] = nil } it { is_expected.to fail_a_contract } end context "when the message doesnt exist" do - let(:params) { { guardian: guardian, channel_id: channel_1.id, message_id: -999 } } + before { params[:message_id] = -999 } it { is_expected.to fail_to_find_a_model(:message) } end context "when the message is a reply" do - let(:params) { { guardian: guardian, channel_id: channel_1.id, message_id: reply.id } } - context "when the OM is from current user" do fab!(:original_message) do Fabricate(:chat_message, chat_channel: channel_1, user: current_user) @@ -60,6 +67,8 @@ RSpec.describe Chat::StopMessageStreaming do Fabricate(:chat_message, chat_channel: channel_1, in_reply_to: original_message) end + before { params[:message_id] = reply.id } + it { is_expected.to be_a_success } end @@ -71,10 +80,18 @@ RSpec.describe Chat::StopMessageStreaming do Fabricate(:chat_message, chat_channel: channel_1, in_reply_to: original_message) end + before { params[:message_id] = reply.id } + context "when current user is a regular user" do it { is_expected.to fail_a_policy(:can_stop_streaming) } end + context "when current user is a bot" do + fab!(:current_user) { Discourse.system_user } + + it { is_expected.to be_a_success } + end + context "when current user is an admin" do fab!(:current_user) { Fabricate(:admin) } @@ -84,14 +101,20 @@ RSpec.describe Chat::StopMessageStreaming do end context "when the message is not a reply" do - let(:params) { { guardian: guardian, channel_id: channel_1.id, message_id: message.id } } - fab!(:message) { Fabricate(:chat_message, chat_channel: channel_1) } + before { params[:message_id] = message.id } + context "when current user is a regular user" do it { is_expected.to fail_a_policy(:can_stop_streaming) } end + context "when current user is a bot" do + fab!(:current_user) { Discourse.system_user } + + it { is_expected.to be_a_success } + end + context "when current user is an admin" do fab!(:current_user) { Fabricate(:admin) } diff --git a/plugins/chat/spec/services/chat/update_message_spec.rb b/plugins/chat/spec/services/chat/update_message_spec.rb index 5852eec4322..ff2f453e079 100644 --- a/plugins/chat/spec/services/chat/update_message_spec.rb +++ b/plugins/chat/spec/services/chat/update_message_spec.rb @@ -43,9 +43,7 @@ RSpec.describe Chat::UpdateMessage do SiteSetting.chat_duplicate_message_sensitivity = 0 Jobs.run_immediately! - [admin1, admin2, user1, user2, user3, user4].each do |user| - Fabricate(:user_chat_channel_membership, chat_channel: public_chat_channel, user: user) - end + [admin1, admin2, user1, user2, user3, user4].each { |user| public_chat_channel.add(user) } end def create_chat_message(user, message, channel, upload_ids: nil) @@ -452,11 +450,7 @@ RSpec.describe Chat::UpdateMessage do it "doesn't notify the second time users that has already been notified when creating the message" do group_user = Fabricate(:user) - Fabricate( - :user_chat_channel_membership, - chat_channel: public_chat_channel, - user: group_user, - ) + public_chat_channel.add(group_user) group = Fabricate( :public_group, @@ -483,7 +477,7 @@ RSpec.describe Chat::UpdateMessage do describe "with @here mentions" do it "doesn't notify the second time users that has already been notified when creating the message" do user = Fabricate(:user) - Fabricate(:user_chat_channel_membership, chat_channel: public_chat_channel, user: user) + public_chat_channel.add(user) user.update!(last_seen_at: 4.minutes.ago) chat_message = create_chat_message(user1, "Mentioning @here", public_chat_channel) @@ -504,7 +498,7 @@ RSpec.describe Chat::UpdateMessage do describe "with @all mentions" do it "doesn't notify the second time users that has already been notified when creating the message" do user = Fabricate(:user) - Fabricate(:user_chat_channel_membership, chat_channel: public_chat_channel, user: user) + public_chat_channel.add(user) chat_message = create_chat_message(user1, "Mentioning @all", public_chat_channel) notification_id = user.notifications.first.id @@ -880,6 +874,8 @@ RSpec.describe Chat::UpdateMessage do SiteSetting.chat_editing_grace_period = 10 SiteSetting.chat_editing_grace_period_max_diff_low_trust = 10 SiteSetting.chat_editing_grace_period_max_diff_high_trust = 40 + + channel_1.add(current_user) end context "when all steps pass" do @@ -920,6 +916,15 @@ RSpec.describe Chat::UpdateMessage do Jobs::Chat::ProcessMessage.any_instance.expects(:execute).once expect_not_enqueued_with(job: Jobs::Chat::ProcessMessage) { result } end + + context "when user is a bot" do + fab!(:bot) { Discourse.system_user } + let(:guardian) { Guardian.new(bot) } + + it "creates the membership" do + expect { result }.to change { channel_1.membership_for(bot) }.from(nil).to(be_present) + end + end end context "when params are not valid" do @@ -934,10 +939,10 @@ RSpec.describe Chat::UpdateMessage do it { is_expected.to fail_a_policy(:can_modify_channel_message) } end - context "when user can't modify this message" do + context "when user is not member of the channel" do let(:message_id) { Fabricate(:chat_message).id } - it { is_expected.to fail_a_policy(:can_modify_message) } + it { is_expected.to fail_to_find_a_model(:membership) } end context "when edit grace period" do