diff --git a/.rubocop.yml b/.rubocop.yml index 1abae8bb1..8b41dd5fd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,7 +17,6 @@ Metrics/ClassLength: - 'app/builders/messages/facebook/message_builder.rb' - 'app/controllers/api/v1/accounts/contacts_controller.rb' - 'app/listeners/action_cable_listener.rb' - - 'app/models/conversation.rb' RSpec/ExampleLength: Max: 25 Style/Documentation: @@ -188,4 +187,3 @@ AllCops: - db/migrate/20200927135222_add_last_activity_at_to_conversation.rb - db/migrate/20210306170117_add_last_activity_at_to_contacts.rb - db/migrate/20220809104508_revert_cascading_indexes.rb - diff --git a/app/controllers/api/v1/accounts/conversations/participants_controller.rb b/app/controllers/api/v1/accounts/conversations/participants_controller.rb new file mode 100644 index 000000000..ebd02380f --- /dev/null +++ b/app/controllers/api/v1/accounts/conversations/participants_controller.rb @@ -0,0 +1,41 @@ +class Api::V1::Accounts::Conversations::ParticipantsController < Api::V1::Accounts::Conversations::BaseController + def show + @participants = @conversation.conversation_participants + end + + def create + ActiveRecord::Base.transaction do + @participants = participants_to_be_added_ids.map { |user_id| @conversation.conversation_participants.find_or_create_by(user_id: user_id) } + end + end + + def update + ActiveRecord::Base.transaction do + participants_to_be_added_ids.each { |user_id| @conversation.conversation_participants.find_or_create_by(user_id: user_id) } + participants_to_be_removed_ids.each { |user_id| @conversation.conversation_participants.find_by(user_id: user_id)&.destroy } + end + @participants = @conversation.conversation_participants + render action: 'show' + end + + def destroy + ActiveRecord::Base.transaction do + params[:user_ids].map { |user_id| @conversation.conversation_participants.find_by(user_id: user_id)&.destroy } + end + head :ok + end + + private + + def participants_to_be_added_ids + params[:user_ids] - current_participant_ids + end + + def participants_to_be_removed_ids + current_participant_ids - params[:user_ids] + end + + def current_participant_ids + @current_participant_ids ||= @conversation.conversation_participants.pluck(:user_id) + end +end diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb index b337e8458..9472dc623 100644 --- a/app/finders/conversation_finder.rb +++ b/app/finders/conversation_finder.rb @@ -97,6 +97,8 @@ class ConversationFinder when 'mention' conversation_ids = current_account.mentions.where(user: current_user).pluck(:conversation_id) @conversations = @conversations.where(id: conversation_ids) + when 'participating' + @conversations = current_user.participating_conversations.where(account_id: current_account.id) when 'unattended' @conversations = @conversations.where(first_reply_created_at: nil) end diff --git a/app/mailers/agent_notifications/conversation_notifications_mailer.rb b/app/mailers/agent_notifications/conversation_notifications_mailer.rb index c0f1f8630..95ed39b8d 100644 --- a/app/mailers/agent_notifications/conversation_notifications_mailer.rb +++ b/app/mailers/agent_notifications/conversation_notifications_mailer.rb @@ -42,6 +42,18 @@ class AgentNotifications::ConversationNotificationsMailer < ApplicationMailer send_mail_with_liquid(to: @agent.email, subject: subject) and return end + def participating_conversation_new_message(message, agent) + return unless smtp_config_set_or_development? + # Don't spam with email notifications if agent is online + return if ::OnlineStatusTracker.get_presence(message.account_id, 'User', agent.id) + + @agent = agent + @conversation = message.conversation + subject = "#{@agent.available_name}, New message in your participating conversation [ID - #{@conversation.display_id}]." + @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) + send_mail_with_liquid(to: @agent.email, subject: subject) and return + end + private def liquid_droppables diff --git a/app/models/concerns/assignment_handler.rb b/app/models/concerns/assignment_handler.rb index dde962ba1..cf4ef123e 100644 --- a/app/models/concerns/assignment_handler.rb +++ b/app/models/concerns/assignment_handler.rb @@ -4,7 +4,7 @@ module AssignmentHandler included do before_save :ensure_assignee_is_from_team - after_commit :notify_assignment_change, :process_assignment_activities + after_commit :notify_assignment_change, :process_assignment_changes end private @@ -36,6 +36,11 @@ module AssignmentHandler end end + def process_assignment_changes + process_assignment_activities + process_participant_assignment + end + def process_assignment_activities user_name = Current.user.name if Current.user.present? if saved_change_to_team_id? @@ -44,4 +49,10 @@ module AssignmentHandler create_assignee_change_activity(user_name) end end + + def process_participant_assignment + return unless saved_change_to_assignee_id? && assignee_id.present? + + conversation_participants.find_or_create_by!(user_id: assignee_id) + end end diff --git a/app/models/concerns/user_attribute_helpers.rb b/app/models/concerns/user_attribute_helpers.rb new file mode 100644 index 000000000..32ff026ac --- /dev/null +++ b/app/models/concerns/user_attribute_helpers.rb @@ -0,0 +1,53 @@ +module UserAttributeHelpers + extend ActiveSupport::Concern + + def available_name + self[:display_name].presence || name + end + + def availability_status + current_account_user&.availability_status + end + + def auto_offline + current_account_user&.auto_offline + end + + def inviter + current_account_user&.inviter + end + + def active_account_user + account_users.order(active_at: :desc)&.first + end + + def current_account_user + # We want to avoid subsequent queries in case where the association is preloaded. + # using where here will trigger n+1 queries. + account_users.find { |ac_usr| ac_usr.account_id == Current.account.id } if Current.account + end + + def account + current_account_user&.account + end + + def administrator? + current_account_user&.administrator? + end + + def agent? + current_account_user&.agent? + end + + def role + current_account_user&.role + end + + # Used internally for Chatwoot in Chatwoot + def hmac_identifier + hmac_key = GlobalConfig.get('CHATWOOT_INBOX_HMAC_KEY')['CHATWOOT_INBOX_HMAC_KEY'] + return OpenSSL::HMAC.hexdigest('sha256', hmac_key, email) if hmac_key.present? + + '' + end +end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index e2c1d43a4..5d40ab513 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -86,6 +86,7 @@ class Conversation < ApplicationRecord has_many :mentions, dependent: :destroy_async has_many :messages, dependent: :destroy_async, autosave: true has_one :csat_survey_response, dependent: :destroy_async + has_many :conversation_participants, dependent: :destroy_async has_many :notifications, as: :primary_actor, dependent: :destroy_async before_save :ensure_snooze_until_reset diff --git a/app/models/conversation_participant.rb b/app/models/conversation_participant.rb new file mode 100644 index 000000000..830eb7baa --- /dev/null +++ b/app/models/conversation_participant.rb @@ -0,0 +1,41 @@ +# == Schema Information +# +# Table name: conversation_participants +# +# id :bigint not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# account_id :bigint not null +# conversation_id :bigint not null +# user_id :bigint not null +# +# Indexes +# +# index_conversation_participants_on_account_id (account_id) +# index_conversation_participants_on_conversation_id (conversation_id) +# index_conversation_participants_on_user_id (user_id) +# index_conversation_participants_on_user_id_and_conversation_id (user_id,conversation_id) UNIQUE +# +class ConversationParticipant < ApplicationRecord + validates :account_id, presence: true + validates :conversation_id, presence: true + validates :user_id, presence: true + validates :user_id, uniqueness: { scope: [:conversation_id] } + validate :ensure_inbox_access + + belongs_to :account + belongs_to :conversation + belongs_to :user + + before_validation :ensure_account_id + + private + + def ensure_account_id + self.account_id = conversation&.account_id + end + + def ensure_inbox_access + errors.add(:user, 'must have inbox access') if conversation && conversation.inbox.assignable_agents.exclude?(user) + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 78591b96a..681203710 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -34,7 +34,8 @@ class Notification < ApplicationRecord conversation_creation: 1, conversation_assignment: 2, assigned_conversation_new_message: 3, - conversation_mention: 4 + conversation_mention: 4, + participating_conversation_new_message: 5 }.freeze enum notification_type: NOTIFICATION_TYPES @@ -94,7 +95,7 @@ class Notification < ApplicationRecord I18n.t('notifications.notification_title.conversation_creation', display_id: primary_actor.display_id, inbox_name: primary_actor.inbox.name) when 'conversation_assignment' I18n.t('notifications.notification_title.conversation_assignment', display_id: primary_actor.display_id) - when 'assigned_conversation_new_message' + when 'assigned_conversation_new_message', 'participating_conversation_new_message' I18n.t( 'notifications.notification_title.assigned_conversation_new_message', display_id: conversation.display_id, @@ -109,7 +110,11 @@ class Notification < ApplicationRecord # rubocop:enable Metrics/CyclomaticComplexity def conversation - return primary_actor.conversation if %w[assigned_conversation_new_message conversation_mention].include? notification_type + return primary_actor.conversation if %w[ + assigned_conversation_new_message + participating_conversation_new_message + conversation_mention + ].include? notification_type primary_actor end diff --git a/app/models/user.rb b/app/models/user.rb index 3094990d5..a908bd1f8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,6 +48,7 @@ class User < ApplicationRecord include Rails.application.routes.url_helpers include Reportable include SsoAuthenticatable + include UserAttributeHelpers devise :database_authenticatable, :registerable, @@ -76,6 +77,8 @@ class User < ApplicationRecord has_many :assigned_conversations, foreign_key: 'assignee_id', class_name: 'Conversation', dependent: :nullify alias_attribute :conversations, :assigned_conversations has_many :csat_survey_responses, foreign_key: 'assigned_agent_id', dependent: :nullify + has_many :conversation_participants, dependent: :destroy_async + has_many :participating_conversations, through: :conversation_participants, source: :conversation has_many :inbox_members, dependent: :destroy_async has_many :inboxes, through: :inbox_members, source: :inbox @@ -110,60 +113,10 @@ class User < ApplicationRecord self.uid = email end - def active_account_user - account_users.order(active_at: :desc)&.first - end - - def current_account_user - # We want to avoid subsequent queries in case where the association is preloaded. - # using where here will trigger n+1 queries. - account_users.find { |ac_usr| ac_usr.account_id == Current.account.id } if Current.account - end - - def available_name - self[:display_name].presence || name - end - - # Used internally for Chatwoot in Chatwoot - def hmac_identifier - hmac_key = GlobalConfig.get('CHATWOOT_INBOX_HMAC_KEY')['CHATWOOT_INBOX_HMAC_KEY'] - return OpenSSL::HMAC.hexdigest('sha256', hmac_key, email) if hmac_key.present? - - '' - end - - def account - current_account_user&.account - end - def assigned_inboxes administrator? ? Current.account.inboxes : inboxes.where(account_id: Current.account.id) end - def administrator? - current_account_user&.administrator? - end - - def agent? - current_account_user&.agent? - end - - def role - current_account_user&.role - end - - def availability_status - current_account_user&.availability_status - end - - def auto_offline - current_account_user&.auto_offline - end - - def inviter - current_account_user&.inviter - end - def serializable_hash(options = nil) super(options).merge(confirmed: confirmed?) end diff --git a/app/services/messages/mention_service.rb b/app/services/messages/mention_service.rb index c7b81213b..aa8cbfce2 100644 --- a/app/services/messages/mention_service.rb +++ b/app/services/messages/mention_service.rb @@ -9,6 +9,7 @@ class Messages::MentionService Conversations::UserMentionJob.perform_later(validated_mentioned_ids, message.conversation.id, message.account.id) generate_notifications_for_mentions(validated_mentioned_ids) + add_mentioned_users_as_participants(validated_mentioned_ids) end private @@ -38,4 +39,10 @@ class Messages::MentionService ).perform end end + + def add_mentioned_users_as_participants(validated_mentioned_ids) + validated_mentioned_ids.each do |user_id| + message.conversation.conversation_participants.find_or_create_by!(user_id: user_id) + end + end end diff --git a/app/services/messages/new_message_notification_service.rb b/app/services/messages/new_message_notification_service.rb index 2a500fbac..f292ef19a 100644 --- a/app/services/messages/new_message_notification_service.rb +++ b/app/services/messages/new_message_notification_service.rb @@ -4,6 +4,7 @@ class Messages::NewMessageNotificationService def perform return unless message.notifiable? + notify_participating_users notify_conversation_assignee end @@ -11,8 +12,23 @@ class Messages::NewMessageNotificationService delegate :conversation, :sender, :account, to: :message + def notify_participating_users + participating_users = conversation.conversation_participants.map(&:user) + participating_users -= [sender] if sender.is_a?(User) + + participating_users.uniq.each do |participant| + NotificationBuilder.new( + notification_type: 'participating_conversation_new_message', + user: participant, + account: account, + primary_actor: message + ).perform + end + end + def notify_conversation_assignee return if conversation.assignee.blank? + return if assignee_already_notified_via_participation? return if conversation.assignee == sender NotificationBuilder.new( @@ -22,4 +38,13 @@ class Messages::NewMessageNotificationService primary_actor: message ).perform end + + def assignee_already_notified_via_participation? + return unless conversation.conversation_participants.map(&:user).include?(conversation.assignee) + + # check whether participation notifcation is disabled for assignee + notification_setting = conversation.assignee.notification_settings.find_by(account_id: account.id) + notification_setting.public_send(:email_participating_conversation_new_message?) || notification_setting + .public_send(:push_participating_conversation_new_message?) + end end diff --git a/app/views/api/v1/accounts/conversations/participants/create.json.jbuilder b/app/views/api/v1/accounts/conversations/participants/create.json.jbuilder new file mode 100644 index 000000000..5de95847d --- /dev/null +++ b/app/views/api/v1/accounts/conversations/participants/create.json.jbuilder @@ -0,0 +1,3 @@ +json.array! @participants do |participant| + json.partial! 'api/v1/models/agent', format: :json, resource: participant.user +end diff --git a/app/views/api/v1/accounts/conversations/participants/show.json.jbuilder b/app/views/api/v1/accounts/conversations/participants/show.json.jbuilder new file mode 100644 index 000000000..5de95847d --- /dev/null +++ b/app/views/api/v1/accounts/conversations/participants/show.json.jbuilder @@ -0,0 +1,3 @@ +json.array! @participants do |participant| + json.partial! 'api/v1/models/agent', format: :json, resource: participant.user +end diff --git a/app/views/api/v1/accounts/notifications/index.json.jbuilder b/app/views/api/v1/accounts/notifications/index.json.jbuilder index e715d5ada..4e881fe1a 100644 --- a/app/views/api/v1/accounts/notifications/index.json.jbuilder +++ b/app/views/api/v1/accounts/notifications/index.json.jbuilder @@ -11,7 +11,11 @@ json.data do json.notification_type notification.notification_type json.push_message_title notification.push_message_title # TODO: front end assumes primary actor to be conversation. should fix in future - if %w[assigned_conversation_new_message conversation_mention].include? notification.notification_type + if %w[ + assigned_conversation_new_message + participating_conversation_new_message + conversation_mention + ].include? notification.notification_type json.primary_actor_type 'Conversation' json.primary_actor_id notification.conversation.id json.primary_actor notification.conversation.push_event_data diff --git a/app/views/mailers/agent_notifications/conversation_notifications_mailer/participating_conversation_new_message.liquid b/app/views/mailers/agent_notifications/conversation_notifications_mailer/participating_conversation_new_message.liquid new file mode 100644 index 000000000..977b15b6a --- /dev/null +++ b/app/views/mailers/agent_notifications/conversation_notifications_mailer/participating_conversation_new_message.liquid @@ -0,0 +1,5 @@ +
Hi {{user.available_name}},
+ +You have received a new message in a conversation you are participating.
+ +Click here to get cracking.
diff --git a/config/routes.rb b/config/routes.rb index 141e9a0dd..02f1e4991 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -77,6 +77,7 @@ Rails.application.routes.draw do resources :messages, only: [:index, :create, :destroy] resources :assignments, only: [:create] resources :labels, only: [:create, :index] + resource :participants, only: [:show, :create, :update, :destroy] resource :direct_uploads, only: [:create] end member do diff --git a/db/migrate/20220808193420_add_conversation_participants.rb b/db/migrate/20220808193420_add_conversation_participants.rb new file mode 100644 index 000000000..a130a5e01 --- /dev/null +++ b/db/migrate/20220808193420_add_conversation_participants.rb @@ -0,0 +1,11 @@ +class AddConversationParticipants < ActiveRecord::Migration[6.1] + def change + create_table 'conversation_participants', force: :cascade do |t| + t.references :account, null: false + t.references :user, null: false + t.references :conversation, null: false + t.datetime 'created_at', precision: 6, null: false + t.datetime 'updated_at', precision: 6, null: false + end + end +end diff --git a/db/migrate/20230118155353_add_unique_index_to_conversation_participants.rb b/db/migrate/20230118155353_add_unique_index_to_conversation_participants.rb new file mode 100644 index 000000000..cf47c6ea8 --- /dev/null +++ b/db/migrate/20230118155353_add_unique_index_to_conversation_participants.rb @@ -0,0 +1,5 @@ +class AddUniqueIndexToConversationParticipants < ActiveRecord::Migration[6.1] + def change + add_index :conversation_participants, [:user_id, :conversation_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c85a598cd..68284421c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -388,6 +388,18 @@ ActiveRecord::Schema.define(version: 2023_02_09_033203) do t.index ["phone_number", "account_id"], name: "index_contacts_on_phone_number_and_account_id" end + create_table "conversation_participants", force: :cascade do |t| + t.bigint "account_id", null: false + t.bigint "user_id", null: false + t.bigint "conversation_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["account_id"], name: "index_conversation_participants_on_account_id" + t.index ["conversation_id"], name: "index_conversation_participants_on_conversation_id" + t.index ["user_id", "conversation_id"], name: "index_conversation_participants_on_user_id_and_conversation_id", unique: true + t.index ["user_id"], name: "index_conversation_participants_on_user_id" + end + create_table "conversations", id: :serial, force: :cascade do |t| t.integer "account_id", null: false t.integer "inbox_id", null: false diff --git a/spec/controllers/api/v1/accounts/bulk_actions_controller_spec.rb b/spec/controllers/api/v1/accounts/bulk_actions_controller_spec.rb index e6128ca82..8306ec539 100644 --- a/spec/controllers/api/v1/accounts/bulk_actions_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/bulk_actions_controller_spec.rb @@ -12,6 +12,10 @@ RSpec.describe 'Api::V1::Accounts::BulkActionsController', type: :request do create(:conversation, account_id: account.id, status: :open, team_id: team_1.id) create(:conversation, account_id: account.id, status: :open) create(:conversation, account_id: account.id, status: :open) + Conversation.all.each do |conversation| + create(:inbox_member, inbox: conversation.inbox, user: agent_1) + create(:inbox_member, inbox: conversation.inbox, user: agent_2) + end end describe 'POST /api/v1/accounts/{account.id}/bulk_action' do diff --git a/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb index dbe19690a..a4e9d8cb0 100644 --- a/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb @@ -56,8 +56,8 @@ RSpec.describe 'Conversation Assignment API', type: :request do let(:agent) { create(:user, account: account, role: :agent) } before do - conversation.update!(assignee: agent) create(:inbox_member, inbox: conversation.inbox, user: agent) + conversation.update!(assignee: agent) end it 'unassigns the assignee from the conversation' do diff --git a/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb new file mode 100644 index 000000000..6238314ab --- /dev/null +++ b/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb @@ -0,0 +1,142 @@ +require 'rails_helper' + +RSpec.describe 'Conversation Participants API', type: :request do + let(:account) { create(:account) } + let(:conversation) { create(:conversation, account: account) } + let(:agent) { create(:user, account: account, role: :agent) } + + before do + create(:inbox_member, inbox: conversation.inbox, user: agent) + end + + describe 'GET /api/v1/accounts/{account.id}/conversations/