diff --git a/plugins/chat/app/services/update_user_last_read.rb b/plugins/chat/app/services/update_user_last_read.rb index 45b0a33050c..ad9e90dc6ca 100644 --- a/plugins/chat/app/services/update_user_last_read.rb +++ b/plugins/chat/app/services/update_user_last_read.rb @@ -17,9 +17,9 @@ module Chat # @param [Guardian] guardian # @return [Chat::Service::Base::Context] + contract model :membership, :fetch_active_membership policy :invalid_access - contract policy :ensure_message_id_recency policy :ensure_message_exists step :update_last_read_message_id @@ -31,6 +31,8 @@ module Chat attribute :message_id, :integer attribute :user_id, :integer attribute :channel_id, :integer + + validates :message_id, :user_id, :channel_id, presence: true end private diff --git a/plugins/chat/spec/services/update_user_last_read_spec.rb b/plugins/chat/spec/services/update_user_last_read_spec.rb index 4926e824f6c..40a5b105605 100644 --- a/plugins/chat/spec/services/update_user_last_read_spec.rb +++ b/plugins/chat/spec/services/update_user_last_read_spec.rb @@ -1,118 +1,119 @@ # frozen_string_literal: true -RSpec.describe(Chat::Service::UpdateUserLastRead) do - subject(:result) { described_class.call(params) } - - fab!(:current_user) { Fabricate(:user) } - fab!(:channel) { Fabricate(:chat_channel) } - fab!(: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(:guardian) { Guardian.new(current_user) } - let(:params) do - { - guardian: guardian, - user_id: current_user.id, - channel_id: channel.id, - message_id: message_1.id, - } +RSpec.describe Chat::Service::UpdateUserLastRead do + describe Chat::Service::UpdateUserLastRead::Contract, type: :model do + it { is_expected.to validate_presence_of :user_id } + it { is_expected.to validate_presence_of :channel_id } + it { is_expected.to validate_presence_of :message_id } end - context "when channel_id is not provided" do - before { params.delete(:channel_id) } + describe ".call" do + subject(:result) { described_class.call(params) } - it { is_expected.to fail_to_find_a_model(:membership) } - end - - context "when user_id is not provided" do - before { params.delete(:user_id) } - - it { is_expected.to fail_to_find_a_model(:membership) } - end - - context "when user has no membership" do - before { membership.destroy! } - - it { is_expected.to fail_to_find_a_model(:membership) } - end - - context "when user can’t access the channel" do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel) { Fabricate(:chat_channel) } fab!(:membership) do - Fabricate( - :user_chat_channel_membership, - user: current_user, - chat_channel: Fabricate(:private_category_channel), - ) + Fabricate(:user_chat_channel_membership, user: current_user, chat_channel: channel) + end + fab!(:message_1) { Fabricate(:chat_message, chat_channel: membership.chat_channel) } + + let(:guardian) { Guardian.new(current_user) } + let(:params) do + { + guardian: guardian, + user_id: current_user.id, + channel_id: channel.id, + message_id: message_1.id, + } end - before { params[:channel_id] = membership.chat_channel.id } + context "when params are not valid" do + before { params.delete(:user_id) } - it { is_expected.to fail_a_policy(:invalid_access) } - end - - context "when message_id is older than membership's last_read_message_id" do - before do - params[:message_id] = -2 - membership.update!(last_read_message_id: -1) + it { is_expected.to fail_a_contract } end - it { is_expected.to fail_a_policy(:ensure_message_id_recency) } - end + context "when params are valid" do + context "when user has no membership" do + before { membership.destroy! } - context "when message doesn’t exist" do - before do - params[:message_id] = 2 - membership.update!(last_read_message_id: 1) - end + it { is_expected.to fail_to_find_a_model(:membership) } + end - it { is_expected.to fail_a_policy(:ensure_message_exists) } - end + context "when user can’t access the channel" do + fab!(:membership) do + Fabricate( + :user_chat_channel_membership, + user: current_user, + chat_channel: Fabricate(:private_category_channel), + ) + end - context "when params are valid" do - before { Jobs.run_immediately! } + before { params[:channel_id] = membership.chat_channel.id } - it "sets the service result as successful" do - expect(result).to be_a_success - end + it { is_expected.to fail_a_policy(:invalid_access) } + end - it "updates the last_read message id" do - expect { result }.to change { membership.reload.last_read_message_id }.to(message_1.id) - end + context "when message_id is older than membership's last_read_message_id" do + before do + params[:message_id] = -2 + membership.update!(last_read_message_id: -1) + end - it "marks existing notifications related to the message as read" do - expect { - notification = + it { is_expected.to fail_a_policy(:ensure_message_id_recency) } + end + + context "when message doesn’t exist" do + before do + params[:message_id] = 2 + membership.update!(last_read_message_id: 1) + end + + it { is_expected.to fail_a_policy(:ensure_message_exists) } + end + + context "when everything is fine" do + fab!(:notification) do Fabricate( :notification, notification_type: Notification.types[:chat_mention], user: current_user, ) + end - # FIXME: we need a better way to create proper chat mention - ChatMention.create!(notification: notification, user: current_user, chat_message: message_1) - }.to change { - Notification.where( - notification_type: Notification.types[:chat_mention], - user: current_user, - read: false, - ).count - }.by(1) + let(:messages) { MessageBus.track_publish { result } } - expect { result }.to change { - Notification.where( - notification_type: Notification.types[:chat_mention], - user: current_user, - read: false, - ).count - }.by(-1) - end + before do + Jobs.run_immediately! + ChatMention.create!( + notification: notification, + user: current_user, + chat_message: message_1, + ) + end - it "publishes new last read to clients" do - messages = MessageBus.track_publish { result } + it "sets the service result as successful" do + expect(result).to be_a_success + end - expect(messages.map(&:channel)).to include("/chat/user-tracking-state/#{current_user.id}") + it "updates the last_read message id" do + expect { result }.to change { membership.reload.last_read_message_id }.to(message_1.id) + end + + it "marks existing notifications related to the message as read" do + expect { result }.to change { + Notification.where( + notification_type: Notification.types[:chat_mention], + user: current_user, + read: false, + ).count + }.by(-1) + end + + it "publishes new last read to clients" do + expect(messages.map(&:channel)).to include("/chat/user-tracking-state/#{current_user.id}") + end + end end end end