From 31c07771e844d4f41357da68ca2d8dcd69a5064c Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Thu, 10 Sep 2020 19:19:15 +0530 Subject: [PATCH] feat: Notification on new messages in conversation (#1204) fixes: #895 fixes: #1118 fixes: #1075 Co-authored-by: Pranav Raj S --- .rubocop.yml | 1 + Gemfile.lock | 104 +++++++++--------- .../api/v1/widget/conversations_controller.rb | 2 +- .../dashboard/i18n/locale/en/settings.json | 4 +- .../settings/profile/NotificationSettings.vue | 34 ++++++ .../specs/contactConversations/fixtures.js | 4 +- app/javascript/widget/api/conversation.js | 2 +- .../store/modules/conversationAttributes.js | 2 +- app/jobs/contact_avatar_job.rb | 4 +- .../notification/email_notification_job.rb | 3 + app/listeners/notification_listener.rb | 16 +++ .../conversation_notifications_mailer.rb | 12 ++ app/mailers/application_mailer.rb | 6 + app/mailers/conversation_reply_mailer.rb | 14 +++ app/models/conversation.rb | 4 +- app/models/message.rb | 28 +++-- app/models/notification.rb | 6 +- .../conversations/event_data_presenter.rb | 2 +- .../notification/push_notification_service.rb | 2 + .../partials/_conversation.json.jbuilder | 2 +- .../widget/conversations/index.json.jbuilder | 2 +- .../assigned_conversation_new_message.liquid | 7 ++ .../20200907094912_rename_user_last_seen.rb | 5 + db/schema.rb | 4 +- docs/channels/api-channel/callback-url.md | 2 +- lib/integrations/facebook/delivery_status.rb | 2 +- package.json | 2 +- .../widget/conversations_controller_spec.rb | 4 +- spec/mailboxes/conversation_mailbox_spec.rb | 5 +- .../conversation_notifications_mailer_spec.rb | 17 +++ .../mailers/conversation_reply_mailer_spec.rb | 18 ++- spec/models/conversation_spec.rb | 4 +- spec/models/message_spec.rb | 23 +++- .../event_data_presenter_spec.rb | 2 +- swagger/definitions/resource/conversation.yml | 2 +- swagger/swagger.json | 2 +- 36 files changed, 259 insertions(+), 94 deletions(-) create mode 100644 app/views/mailers/agent_notifications/conversation_notifications_mailer/assigned_conversation_new_message.liquid create mode 100644 db/migrate/20200907094912_rename_user_last_seen.rb diff --git a/.rubocop.yml b/.rubocop.yml index 3ee387090..c27b9651b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -75,6 +75,7 @@ Metrics/AbcSize: - 'db/migrate/20190819005836_add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb' - 'db/migrate/20161123131628_devise_token_auth_create_users.rb' Metrics/CyclomaticComplexity: + Max: 7 Exclude: - 'db/migrate/20190819005836_add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb' Rails/ReversibleMigration: diff --git a/Gemfile.lock b/Gemfile.lock index 3001534e6..0c9cda2ec 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -18,56 +18,56 @@ GEM specs: action-cable-testing (0.6.1) actioncable (>= 5.0) - actioncable (6.0.3.2) - actionpack (= 6.0.3.2) + actioncable (6.0.3.3) + actionpack (= 6.0.3.3) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.0.3.2) - actionpack (= 6.0.3.2) - activejob (= 6.0.3.2) - activerecord (= 6.0.3.2) - activestorage (= 6.0.3.2) - activesupport (= 6.0.3.2) + actionmailbox (6.0.3.3) + actionpack (= 6.0.3.3) + activejob (= 6.0.3.3) + activerecord (= 6.0.3.3) + activestorage (= 6.0.3.3) + activesupport (= 6.0.3.3) mail (>= 2.7.1) - actionmailer (6.0.3.2) - actionpack (= 6.0.3.2) - actionview (= 6.0.3.2) - activejob (= 6.0.3.2) + actionmailer (6.0.3.3) + actionpack (= 6.0.3.3) + actionview (= 6.0.3.3) + activejob (= 6.0.3.3) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.0.3.2) - actionview (= 6.0.3.2) - activesupport (= 6.0.3.2) + actionpack (6.0.3.3) + actionview (= 6.0.3.3) + activesupport (= 6.0.3.3) rack (~> 2.0, >= 2.0.8) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.0.3.2) - actionpack (= 6.0.3.2) - activerecord (= 6.0.3.2) - activestorage (= 6.0.3.2) - activesupport (= 6.0.3.2) + actiontext (6.0.3.3) + actionpack (= 6.0.3.3) + activerecord (= 6.0.3.3) + activestorage (= 6.0.3.3) + activesupport (= 6.0.3.3) nokogiri (>= 1.8.5) - actionview (6.0.3.2) - activesupport (= 6.0.3.2) + actionview (6.0.3.3) + activesupport (= 6.0.3.3) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.0.3.2) - activesupport (= 6.0.3.2) + activejob (6.0.3.3) + activesupport (= 6.0.3.3) globalid (>= 0.3.6) - activemodel (6.0.3.2) - activesupport (= 6.0.3.2) - activerecord (6.0.3.2) - activemodel (= 6.0.3.2) - activesupport (= 6.0.3.2) - activestorage (6.0.3.2) - actionpack (= 6.0.3.2) - activejob (= 6.0.3.2) - activerecord (= 6.0.3.2) + activemodel (6.0.3.3) + activesupport (= 6.0.3.3) + activerecord (6.0.3.3) + activemodel (= 6.0.3.3) + activesupport (= 6.0.3.3) + activestorage (6.0.3.3) + actionpack (= 6.0.3.3) + activejob (= 6.0.3.3) + activerecord (= 6.0.3.3) marcel (~> 0.3.1) - activesupport (6.0.3.2) + activesupport (6.0.3.3) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -299,7 +299,7 @@ GEM mini_magick (4.10.1) mini_mime (1.0.2) mini_portile2 (2.4.0) - minitest (5.14.1) + minitest (5.14.2) momentjs-rails (2.20.1) railties (>= 3.1) msgpack (1.3.3) @@ -307,7 +307,7 @@ GEM multi_xml (0.6.0) multipart-post (2.1.1) netrc (0.11.0) - nio4r (2.5.2) + nio4r (2.5.3) nokogiri (1.10.10) mini_portile2 (~> 2.4.0) oauth (0.5.4) @@ -336,29 +336,29 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) - rails (6.0.3.2) - actioncable (= 6.0.3.2) - actionmailbox (= 6.0.3.2) - actionmailer (= 6.0.3.2) - actionpack (= 6.0.3.2) - actiontext (= 6.0.3.2) - actionview (= 6.0.3.2) - activejob (= 6.0.3.2) - activemodel (= 6.0.3.2) - activerecord (= 6.0.3.2) - activestorage (= 6.0.3.2) - activesupport (= 6.0.3.2) + rails (6.0.3.3) + actioncable (= 6.0.3.3) + actionmailbox (= 6.0.3.3) + actionmailer (= 6.0.3.3) + actionpack (= 6.0.3.3) + actiontext (= 6.0.3.3) + actionview (= 6.0.3.3) + activejob (= 6.0.3.3) + activemodel (= 6.0.3.3) + activerecord (= 6.0.3.3) + activestorage (= 6.0.3.3) + activesupport (= 6.0.3.3) bundler (>= 1.3.0) - railties (= 6.0.3.2) + railties (= 6.0.3.3) sprockets-rails (>= 2.0.0) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) rails-html-sanitizer (1.3.0) loofah (~> 2.3) - railties (6.0.3.2) - actionpack (= 6.0.3.2) - activesupport (= 6.0.3.2) + railties (6.0.3.3) + actionpack (= 6.0.3.3) + activesupport (= 6.0.3.3) method_source rake (>= 0.8.7) thor (>= 0.20.3, < 2.0) diff --git a/app/controllers/api/v1/widget/conversations_controller.rb b/app/controllers/api/v1/widget/conversations_controller.rb index ac41c86c2..bafd008f9 100644 --- a/app/controllers/api/v1/widget/conversations_controller.rb +++ b/app/controllers/api/v1/widget/conversations_controller.rb @@ -8,7 +8,7 @@ class Api::V1::Widget::ConversationsController < Api::V1::Widget::BaseController def update_last_seen head :ok && return if conversation.nil? - conversation.user_last_seen_at = DateTime.now.utc + conversation.contact_last_seen_at = DateTime.now.utc conversation.save! head :ok end diff --git a/app/javascript/dashboard/i18n/locale/en/settings.json b/app/javascript/dashboard/i18n/locale/en/settings.json index f39997b6e..0ac7e8d61 100644 --- a/app/javascript/dashboard/i18n/locale/en/settings.json +++ b/app/javascript/dashboard/i18n/locale/en/settings.json @@ -26,7 +26,8 @@ "TITLE": "Email Notifications", "NOTE": "Update your email notification preferences here", "CONVERSATION_ASSIGNMENT": "Send email notifications when a conversation is assigned to me", - "CONVERSATION_CREATION": "Send email notifications when a new conversation is created" + "CONVERSATION_CREATION": "Send email notifications when a new conversation is created", + "ASSIGNED_CONVERSATION_NEW_MESSAGE": "Send email notifications when a new message is created in an assigned conversation" }, "API": { "UPDATE_SUCCESS": "Your notification preferences are updated successfully", @@ -37,6 +38,7 @@ "NOTE": "Update your push notification preferences here", "CONVERSATION_ASSIGNMENT": "Send push notifications when a conversation is assigned to me", "CONVERSATION_CREATION": "Send push notifications when a new conversation is created", + "ASSIGNED_CONVERSATION_NEW_MESSAGE": "Send push notifications when a new message is created in an assigned conversation", "HAS_ENABLED_PUSH": "You have enabled push for this browser.", "REQUEST_PUSH": "Enable push notifications" }, diff --git a/app/javascript/dashboard/routes/dashboard/settings/profile/NotificationSettings.vue b/app/javascript/dashboard/routes/dashboard/settings/profile/NotificationSettings.vue index 704978a00..0caf29589 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/profile/NotificationSettings.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/profile/NotificationSettings.vue @@ -43,6 +43,23 @@ }} + +
+ + +
@@ -105,6 +122,23 @@ }}
+ +
+ + +
diff --git a/app/javascript/dashboard/store/modules/specs/contactConversations/fixtures.js b/app/javascript/dashboard/store/modules/specs/contactConversations/fixtures.js index 99f195f52..a10f08595 100644 --- a/app/javascript/dashboard/store/modules/specs/contactConversations/fixtures.js +++ b/app/javascript/dashboard/store/modules/specs/contactConversations/fixtures.js @@ -35,7 +35,7 @@ export default [ inbox_id: 1, status: 0, timestamp: 1578555084, - user_last_seen_at: 0, + contact_last_seen_at: 0, agent_last_seen_at: 1578555084, unread_count: 0, }, @@ -75,7 +75,7 @@ export default [ inbox_id: 2, status: 0, timestamp: 1578555084, - user_last_seen_at: 0, + contact_last_seen_at: 0, agent_last_seen_at: 1578555084, unread_count: 0, }, diff --git a/app/javascript/widget/api/conversation.js b/app/javascript/widget/api/conversation.js index 9933e529a..0901fbcb7 100755 --- a/app/javascript/widget/api/conversation.js +++ b/app/javascript/widget/api/conversation.js @@ -33,7 +33,7 @@ const toggleTyping = async ({ typingStatus }) => { const setUserLastSeenAt = async ({ lastSeen }) => { return API.post( `/api/v1/widget/conversations/update_last_seen${window.location.search}`, - { user_last_seen_at: lastSeen } + { contact_last_seen_at: lastSeen } ); }; diff --git a/app/javascript/widget/store/modules/conversationAttributes.js b/app/javascript/widget/store/modules/conversationAttributes.js index b4cbdeec0..3391b2960 100644 --- a/app/javascript/widget/store/modules/conversationAttributes.js +++ b/app/javascript/widget/store/modules/conversationAttributes.js @@ -17,7 +17,7 @@ export const actions = { get: async ({ commit }) => { try { const { data } = await getConversationAPI(); - const { user_last_seen_at: lastSeen } = data; + const { contact_last_seen_at: lastSeen } = data; commit(SET_CONVERSATION_ATTRIBUTES, data); commit('conversation/setMetaUserLastSeenAt', lastSeen, { root: true }); } catch (error) { diff --git a/app/jobs/contact_avatar_job.rb b/app/jobs/contact_avatar_job.rb index b9b59a456..e1de371fd 100644 --- a/app/jobs/contact_avatar_job.rb +++ b/app/jobs/contact_avatar_job.rb @@ -4,7 +4,7 @@ class ContactAvatarJob < ApplicationJob def perform(contact, avatar_url) avatar_resource = LocalResource.new(avatar_url) contact.avatar.attach(io: avatar_resource.file, filename: avatar_resource.tmp_filename, content_type: avatar_resource.encoding) - rescue Errno::ETIMEDOUT, Errno::ECONNREFUSED, SocketError => e - Rails.logger.info "invalid url #{file_url} : #{e.message}" + rescue Errno::ETIMEDOUT, Errno::ECONNREFUSED, SocketError, NoMethodError => e + Rails.logger.info "invalid url #{avatar_url} : #{e.message}" end end diff --git a/app/jobs/notification/email_notification_job.rb b/app/jobs/notification/email_notification_job.rb index 80a361aaf..6a1e33c71 100644 --- a/app/jobs/notification/email_notification_job.rb +++ b/app/jobs/notification/email_notification_job.rb @@ -2,6 +2,9 @@ class Notification::EmailNotificationJob < ApplicationJob queue_as :default def perform(notification) + # no need to send email if notification has been read already + return if notification.read_at.present? + Notification::EmailNotificationService.new(notification: notification).perform end end diff --git a/app/listeners/notification_listener.rb b/app/listeners/notification_listener.rb index ab7e4cc3c..350c9d80a 100644 --- a/app/listeners/notification_listener.rb +++ b/app/listeners/notification_listener.rb @@ -26,4 +26,20 @@ class NotificationListener < BaseListener primary_actor: conversation ).perform end + + def message_created(event) + message, account = extract_message_and_account(event) + conversation = message.conversation + + # only want to notify agents about customer messages + return unless message.incoming? + return unless conversation.assignee + + NotificationBuilder.new( + notification_type: 'assigned_conversation_new_message', + user: conversation.assignee, + account: account, + primary_actor: conversation + ).perform + end end diff --git a/app/mailers/agent_notifications/conversation_notifications_mailer.rb b/app/mailers/agent_notifications/conversation_notifications_mailer.rb index 8bc872f20..1c9c186a6 100644 --- a/app/mailers/agent_notifications/conversation_notifications_mailer.rb +++ b/app/mailers/agent_notifications/conversation_notifications_mailer.rb @@ -19,6 +19,18 @@ class AgentNotifications::ConversationNotificationsMailer < ApplicationMailer send_mail_with_liquid(to: @agent.email, subject: subject) and return end + def assigned_conversation_new_message(conversation, agent) + return unless smtp_config_set_or_development? + # Don't spam with email notifications if agent is online + return if ::OnlineStatusTracker.get_presence(conversation.account.id, 'User', agent.id) + + @agent = agent + @conversation = conversation + subject = "#{@agent.available_name}, New message in your assigned 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/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index c40baeb7d..278039bd4 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -50,7 +50,13 @@ class ApplicationMailer < ActionMailer::Base } end + def locale_from_account(account) + I18n.available_locales.map(&:to_s).include?(account.locale) ? account.locale : nil + end + def ensure_current_account(account) Current.account = account if account.present? + locale ||= locale_from_account(account) if account.present? + I18n.locale = locale || I18n.default_locale end end diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index a56c2bb63..9b6caa0f6 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -6,6 +6,7 @@ class ConversationReplyMailer < ApplicationMailer return unless smtp_config_set_or_development? init_conversation_attributes(conversation) + return if conversation_already_viewed? recap_messages = @conversation.messages.chat.where('created_at < ?', message_queued_time).last(10) new_messages = @conversation.messages.chat.where('created_at >= ?', message_queued_time) @@ -26,6 +27,7 @@ class ConversationReplyMailer < ApplicationMailer return unless smtp_config_set_or_development? init_conversation_attributes(conversation) + return if conversation_already_viewed? @messages = @conversation.messages.chat.outgoing.where('created_at >= ?', message_queued_time) return false if @messages.count.zero? @@ -63,6 +65,18 @@ class ConversationReplyMailer < ApplicationMailer @agent = @conversation.assignee end + def conversation_already_viewed? + # whether contact already saw the message on widget + return unless @conversation.contact_last_seen_at + return unless last_outgoing_message&.created_at + + @conversation.contact_last_seen_at > last_outgoing_message&.created_at + end + + def last_outgoing_message + @conversation.messages.chat.where.not(message_type: :incoming)&.last + end + def assignee_name @assignee_name ||= @agent&.available_name || 'Notifications' end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 98d876982..5d5747a09 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -5,10 +5,10 @@ # id :integer not null, primary key # additional_attributes :jsonb # agent_last_seen_at :datetime +# contact_last_seen_at :datetime # identifier :string # locked :boolean default(FALSE) # status :integer default("open"), not null -# user_last_seen_at :datetime # uuid :uuid not null # created_at :datetime not null # updated_at :datetime not null @@ -166,7 +166,7 @@ class Conversation < ApplicationRecord { CONVERSATION_OPENED => -> { saved_change_to_status? && open? }, CONVERSATION_RESOLVED => -> { saved_change_to_status? && resolved? }, - CONVERSATION_READ => -> { saved_change_to_user_last_seen_at? }, + CONVERSATION_READ => -> { saved_change_to_contact_last_seen_at? }, CONVERSATION_LOCK_TOGGLE => -> { saved_change_to_locked? }, ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? }, CONVERSATION_CONTACT_CHANGED => -> { saved_change_to_contact_id? } diff --git a/app/models/message.rb b/app/models/message.rb index b61a660f3..df98c1786 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -150,14 +150,28 @@ class Message < ApplicationRecord ::MessageTemplates::HookExecutionService.new(message: self).perform end - def notify_via_mail - if Redis::Alfred.get(conversation_mail_key).nil? && conversation.contact.email? && outgoing? && !private - # set a redis key for the conversation so that we don't need to send email for every - # new message that comes in and we dont enque the delayed sidekiq job for every message - Redis::Alfred.setex(conversation_mail_key, Time.zone.now) + def email_notifiable_message? + return false unless outgoing? + return false if private? - # Since this is live chat, send the email after few minutes so the only one email with - # last few messages coupled together is sent rather than email for each message + true + end + + def can_notify_via_mail? + return unless email_notifiable_message? + return false if conversation.contact.email.blank? + return false unless %w[Website Email].include? inbox.inbox_type + + true + end + + def notify_via_mail + return unless can_notify_via_mail? + + # set a redis key for the conversation so that we don't need to send email for every new message + # last few messages coupled together is sent every 2 minutes rather than one email for each message + if Redis::Alfred.get(conversation_mail_key).nil? + Redis::Alfred.setex(conversation_mail_key, Time.zone.now) ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, Time.zone.now) end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 8863fbffa..4a8860367 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -31,7 +31,8 @@ class Notification < ApplicationRecord NOTIFICATION_TYPES = { conversation_creation: 1, - conversation_assignment: 2 + conversation_assignment: 2, + assigned_conversation_new_message: 3 }.freeze enum notification_type: NOTIFICATION_TYPES @@ -64,6 +65,8 @@ class Notification < ApplicationRecord return "A new conversation [ID -#{primary_actor.display_id}] has been assigned to you." if notification_type == 'conversation_assignment' + return "New message in your assigned conversation [ID -#{primary_actor.display_id}]." if notification_type == 'assigned_conversation_new_message' + '' end @@ -71,6 +74,7 @@ class Notification < ApplicationRecord def process_notification_delivery Notification::PushNotificationJob.perform_later(self) + # Should we do something about the case where user subscribed to both push and email ? # In future, we could probably add condition here to enqueue the job for 30 seconds later # when push enabled and then check in email job whether notification has been read already. diff --git a/app/presenters/conversations/event_data_presenter.rb b/app/presenters/conversations/event_data_presenter.rb index 0c1660fb0..bf09dd0b3 100644 --- a/app/presenters/conversations/event_data_presenter.rb +++ b/app/presenters/conversations/event_data_presenter.rb @@ -31,7 +31,7 @@ class Conversations::EventDataPresenter < SimpleDelegator def push_timestamps { agent_last_seen_at: agent_last_seen_at.to_i, - user_last_seen_at: user_last_seen_at.to_i, + contact_last_seen_at: contact_last_seen_at.to_i, timestamp: created_at.to_i } end diff --git a/app/services/notification/push_notification_service.rb b/app/services/notification/push_notification_service.rb index 76aa44478..d4510d59f 100644 --- a/app/services/notification/push_notification_service.rb +++ b/app/services/notification/push_notification_service.rb @@ -64,6 +64,8 @@ class Notification::PushNotificationService ) rescue Webpush::ExpiredSubscription subscription.destroy! + rescue Errno::ECONNRESET, Net::OpenTimeout, Net::ReadTimeout => e + Rails.logger.info "Webpush operation error: #{e.message}" end def send_fcm_push(subscription) diff --git a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder index 7b79ba077..deae107c1 100644 --- a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder +++ b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder @@ -22,7 +22,7 @@ json.status conversation.status json.muted conversation.muted? json.can_reply conversation.can_reply? json.timestamp conversation.messages.last.try(:created_at).try(:to_i) -json.user_last_seen_at conversation.user_last_seen_at.to_i +json.contact_last_seen_at conversation.contact_last_seen_at.to_i json.agent_last_seen_at conversation.agent_last_seen_at.to_i json.unread_count conversation.unread_incoming_messages.count json.additional_attributes conversation.additional_attributes diff --git a/app/views/api/v1/widget/conversations/index.json.jbuilder b/app/views/api/v1/widget/conversations/index.json.jbuilder index 8eb943785..45cd6a8e9 100644 --- a/app/views/api/v1/widget/conversations/index.json.jbuilder +++ b/app/views/api/v1/widget/conversations/index.json.jbuilder @@ -1,6 +1,6 @@ if @conversation json.id @conversation.display_id json.inbox_id @conversation.inbox_id - json.user_last_seen_at @conversation.user_last_seen_at.to_i + json.contact_last_seen_at @conversation.contact_last_seen_at.to_i json.status @conversation.status end diff --git a/app/views/mailers/agent_notifications/conversation_notifications_mailer/assigned_conversation_new_message.liquid b/app/views/mailers/agent_notifications/conversation_notifications_mailer/assigned_conversation_new_message.liquid new file mode 100644 index 000000000..841aace35 --- /dev/null +++ b/app/views/mailers/agent_notifications/conversation_notifications_mailer/assigned_conversation_new_message.liquid @@ -0,0 +1,7 @@ +

Hi {{user.available_name}},

+ +

You have received a new message in your assigned conversation.

+ +

+Click here to get cracking. +

diff --git a/db/migrate/20200907094912_rename_user_last_seen.rb b/db/migrate/20200907094912_rename_user_last_seen.rb new file mode 100644 index 000000000..66740c509 --- /dev/null +++ b/db/migrate/20200907094912_rename_user_last_seen.rb @@ -0,0 +1,5 @@ +class RenameUserLastSeen < ActiveRecord::Migration[6.0] + def change + rename_column :conversations, :user_last_seen_at, :contact_last_seen_at + end +end diff --git a/db/schema.rb b/db/schema.rb index b582ed1a1..bd8c4e7d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_08_28_175931) do +ActiveRecord::Schema.define(version: 2020_09_07_094912) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" @@ -219,7 +219,7 @@ ActiveRecord::Schema.define(version: 2020_08_28_175931) do t.datetime "updated_at", null: false t.bigint "contact_id" t.integer "display_id", null: false - t.datetime "user_last_seen_at" + t.datetime "contact_last_seen_at" t.datetime "agent_last_seen_at" t.boolean "locked", default: false t.jsonb "additional_attributes" diff --git a/docs/channels/api-channel/callback-url.md b/docs/channels/api-channel/callback-url.md index 09373e09f..54d587340 100644 --- a/docs/channels/api-channel/callback-url.md +++ b/docs/channels/api-channel/callback-url.md @@ -33,7 +33,7 @@ When a new message is created in the API channel, you will get a POST request to "inbox_id": 0, "status": "open", "agent_last_seen_at": 0, - "user_last_seen_at": 0, + "contact_last_seen_at": 0, "timestamp": 0 }, "account": { diff --git a/lib/integrations/facebook/delivery_status.rb b/lib/integrations/facebook/delivery_status.rb index 16ab5bdee..d38ece092 100644 --- a/lib/integrations/facebook/delivery_status.rb +++ b/lib/integrations/facebook/delivery_status.rb @@ -26,7 +26,7 @@ class Integrations::Facebook::DeliveryStatus def update_message_status return unless conversation - conversation.user_last_seen_at = @params.at + conversation.contact_last_seen_at = @params.at conversation.save! end end diff --git a/package.json b/package.json index 1a67e9c97..68ea85738 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ "git add" ], "!(*schema).rb": [ - "rubocop -a", + "bundle exec rubocop -a", "git add" ], "*.scss": [ diff --git a/spec/controllers/api/v1/widget/conversations_controller_spec.rb b/spec/controllers/api/v1/widget/conversations_controller_spec.rb index 89ce60734..a08b9b7bf 100644 --- a/spec/controllers/api/v1/widget/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/widget/conversations_controller_spec.rb @@ -47,7 +47,7 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do context 'with a conversation' do it 'returns the correct conversation params' do allow(Rails.configuration.dispatcher).to receive(:dispatch) - expect(conversation.user_last_seen_at).to eq(nil) + expect(conversation.contact_last_seen_at).to eq(nil) post '/api/v1/widget/conversations/update_last_seen', headers: { 'X-Auth-Token' => token }, @@ -56,7 +56,7 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do expect(response).to have_http_status(:success) - expect(conversation.reload.user_last_seen_at).not_to eq(nil) + expect(conversation.reload.contact_last_seen_at).not_to eq(nil) end end end diff --git a/spec/mailboxes/conversation_mailbox_spec.rb b/spec/mailboxes/conversation_mailbox_spec.rb index 65a757058..239d6e689 100644 --- a/spec/mailboxes/conversation_mailbox_spec.rb +++ b/spec/mailboxes/conversation_mailbox_spec.rb @@ -4,9 +4,10 @@ RSpec.describe ConversationMailbox, type: :mailbox do include ActionMailbox::TestHelper describe 'add mail as reply in a conversation' do - let(:agent) { create(:user, email: 'agent1@example.com') } + let(:account) { create(:account) } + let(:agent) { create(:user, email: 'agent1@example.com', account: account) } let(:reply_mail) { create_inbound_email_from_fixture('reply.eml') } - let(:conversation) { create(:conversation, assignee: agent, inbox: create(:inbox, greeting_enabled: false)) } + let(:conversation) { create(:conversation, assignee: agent, inbox: create(:inbox, account: account, greeting_enabled: false), account: account) } let(:described_subject) { described_class.receive reply_mail } let(:serialized_attributes) { %w[text_content html_content number_of_attachments subject date to from in_reply_to cc bcc message_id] } diff --git a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb index 2fb72d141..9182da9d8 100644 --- a/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb +++ b/spec/mailers/agent_notifications/conversation_notifications_mailer_spec.rb @@ -36,4 +36,21 @@ RSpec.describe AgentNotifications::ConversationNotificationsMailer, type: :maile expect(mail.to).to eq([agent.email]) end end + + describe 'assigned_conversation_new_message' do + let(:mail) { described_class.assigned_conversation_new_message(conversation, agent).deliver_now } + + it 'renders the subject' do + expect(mail.subject).to eq("#{agent.available_name}, New message in your assigned conversation [ID - #{conversation.display_id}].") + end + + it 'renders the receiver email' do + expect(mail.to).to eq([agent.email]) + end + + it 'will not send email if agent is online' do + ::OnlineStatusTracker.update_presence(conversation.account.id, 'User', agent.id) + expect(mail).to eq nil + end + end end diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index 3761f02b9..78262c22d 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -14,9 +14,9 @@ RSpec.describe ConversationReplyMailer, type: :mailer do end context 'with summary' do - let(:conversation) { create(:conversation, assignee: agent) } - let(:message) { create(:message, conversation: conversation) } - let(:private_message) { create(:message, content: 'This is a private message', conversation: conversation) } + let(:conversation) { create(:conversation, account: account, assignee: agent) } + let(:message) { create(:message, account: account, conversation: conversation) } + let(:private_message) { create(:message, account: account, content: 'This is a private message', conversation: conversation) } let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now } it 'renders the subject' do @@ -31,6 +31,12 @@ RSpec.describe ConversationReplyMailer, type: :mailer do expect(mail.body.decoded).not_to include(private_message.content) expect(mail.body.decoded).to include(message.content) end + + it 'will not send email if conversation is already viewed by contact' do + create(:message, message_type: 'outgoing', account: account, conversation: conversation) + conversation.update(contact_last_seen_at: Time.zone.now) + expect(mail).to eq nil + end end context 'without assignee' do @@ -75,6 +81,12 @@ RSpec.describe ConversationReplyMailer, type: :mailer do expect(mail.body.decoded).not_to include(message_1.content) expect(mail.body.decoded).to include(message_2.content) end + + it 'will not send email if conversation is already viewed by contact' do + create(:message, message_type: 'outgoing', account: account, conversation: conversation) + conversation.update(contact_last_seen_at: Time.zone.now) + expect(mail).to eq nil + end end context 'when custom domain and email is not enabled' do diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index b1e298b3d..4ec6717c8 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -69,7 +69,7 @@ RSpec.describe Conversation, type: :model do conversation.update( status: :resolved, locked: true, - user_last_seen_at: Time.now, + contact_last_seen_at: Time.now, assignee: new_assignee ) end @@ -317,7 +317,7 @@ RSpec.describe Conversation, type: :model do timestamp: conversation.created_at.to_i, can_reply: true, channel: 'Channel::WebWidget', - user_last_seen_at: conversation.user_last_seen_at.to_i, + contact_last_seen_at: conversation.contact_last_seen_at.to_i, agent_last_seen_at: conversation.agent_last_seen_at.to_i, unread_count: 0 } diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 7c3dea709..b7b78da9e 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Message, type: :model do end context 'when message is created' do - let(:message) { build(:message) } + let(:message) { build(:message, account: create(:account)) } it 'triggers ::MessageTemplates::HookExecutionService' do hook_execution_service = double @@ -23,10 +23,25 @@ RSpec.describe Message, type: :model do expect(hook_execution_service).to have_received(:perform) end - it 'calls notify email method on after save' do - allow(message).to receive(:notify_via_mail).and_return(true) + it 'calls notify email method on after save for outgoing messages' do + allow(ConversationReplyEmailWorker).to receive(:perform_in).and_return(true) + message.message_type = 'outgoing' message.save! - expect(message).to have_received(:notify_via_mail) + expect(ConversationReplyEmailWorker).to have_received(:perform_in) + end + + it 'wont call notify email method for private notes' do + message.private = true + allow(ConversationReplyEmailWorker).to receive(:perform_in).and_return(true) + message.save! + expect(ConversationReplyEmailWorker).not_to have_received(:perform_in) + end + + it 'wont call notify email method unless its website or email channel' do + message.inbox = create(:inbox, account: message.account, channel: build(:channel_api, account: message.account)) + allow(ConversationReplyEmailWorker).to receive(:perform_in).and_return(true) + message.save! + expect(ConversationReplyEmailWorker).not_to have_received(:perform_in) end end end diff --git a/spec/presenters/conversations/event_data_presenter_spec.rb b/spec/presenters/conversations/event_data_presenter_spec.rb index dea4ae6b8..430c72805 100644 --- a/spec/presenters/conversations/event_data_presenter_spec.rb +++ b/spec/presenters/conversations/event_data_presenter_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Conversations::EventDataPresenter do can_reply: conversation.can_reply?, channel: conversation.inbox.channel_type, timestamp: conversation.created_at.to_i, - user_last_seen_at: conversation.user_last_seen_at.to_i, + contact_last_seen_at: conversation.contact_last_seen_at.to_i, agent_last_seen_at: conversation.agent_last_seen_at.to_i, unread_count: 0 } diff --git a/swagger/definitions/resource/conversation.yml b/swagger/definitions/resource/conversation.yml index 05126e1ab..46c2efd2e 100644 --- a/swagger/definitions/resource/conversation.yml +++ b/swagger/definitions/resource/conversation.yml @@ -18,7 +18,7 @@ properties: timestamp: type: string description: The time at which conversation was created - user_last_seen_at: + contact_last_seen_at: type: string agent_last_seen_at: type: agent_last_seen_at diff --git a/swagger/swagger.json b/swagger/swagger.json index de008e55e..02e024190 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -1088,7 +1088,7 @@ "type": "string", "description": "The time at which conversation was created" }, - "user_last_seen_at": { + "contact_last_seen_at": { "type": "string" }, "agent_last_seen_at": {