diff --git a/app/javascript/dashboard/store/modules/conversations/helpers.js b/app/javascript/dashboard/store/modules/conversations/helpers.js index 7996a34ab..910dca31b 100644 --- a/app/javascript/dashboard/store/modules/conversations/helpers.js +++ b/app/javascript/dashboard/store/modules/conversations/helpers.js @@ -25,10 +25,11 @@ export const filterByLabel = (shouldFilter, labels, chatLabels) => { export const filterByUnattended = ( shouldFilter, conversationType, - firstReplyOn + firstReplyOn, + waitingSince ) => { return conversationType === 'unattended' - ? !firstReplyOn && shouldFilter + ? (!firstReplyOn || !!waitingSince) && shouldFilter : shouldFilter; }; @@ -40,6 +41,7 @@ export const applyPageFilters = (conversation, filters) => { labels: chatLabels = [], meta = {}, first_reply_created_at: firstReplyOn, + waiting_since: waitingSince, } = conversation; const team = meta.team || {}; const { id: chatTeamId } = team; @@ -51,7 +53,8 @@ export const applyPageFilters = (conversation, filters) => { shouldFilter = filterByUnattended( shouldFilter, conversationType, - firstReplyOn + firstReplyOn, + waitingSince ); return shouldFilter; diff --git a/app/listeners/reporting_event_listener.rb b/app/listeners/reporting_event_listener.rb index 653d307bd..093452e92 100644 --- a/app/listeners/reporting_event_listener.rb +++ b/app/listeners/reporting_event_listener.rb @@ -38,8 +38,6 @@ class ReportingEventListener < BaseListener event_end_time: message.created_at ) - conversation.update(first_reply_created_at: message.created_at) - reporting_event.save! end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 2cdda4839..d764f84a0 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -72,7 +72,7 @@ class Conversation < ApplicationRecord scope :unassigned, -> { where(assignee_id: nil) } scope :assigned, -> { where.not(assignee_id: nil) } scope :assigned_to, ->(agent) { where(assignee_id: agent.id) } - scope :unattended, -> { where(first_reply_created_at: nil) } + scope :unattended, -> { where(first_reply_created_at: nil).or(where.not(waiting_since: nil)) } scope :resolvable, lambda { |auto_resolve_duration| return none if auto_resolve_duration.to_i.zero? @@ -218,7 +218,7 @@ class Conversation < ApplicationRecord end def ensure_waiting_since - self.waiting_since = Time.now.utc + self.waiting_since = created_at end def validate_additional_attributes @@ -242,7 +242,8 @@ class Conversation < ApplicationRecord def allowed_keys? ( - previous_changes.keys.intersect?(%w[team_id assignee_id status snoozed_until custom_attributes label_list first_reply_created_at priority]) || + previous_changes.keys.intersect?(%w[team_id assignee_id status snoozed_until custom_attributes label_list waiting_since first_reply_created_at + priority]) || (previous_changes['additional_attributes'].present? && previous_changes['additional_attributes'][1].keys.intersect?(%w[conversation_language])) ) end diff --git a/app/models/message.rb b/app/models/message.rb index dda5b51d5..477e9807d 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -247,7 +247,6 @@ class Message < ApplicationRecord send_reply execute_message_template_hooks update_contact_activity - update_waiting_since end def update_contact_activity @@ -261,7 +260,7 @@ class Message < ApplicationRecord ) conversation.update(waiting_since: nil) end - conversation.update(waiting_since: Time.now.utc) if incoming? && conversation.waiting_since.blank? + conversation.update(waiting_since: created_at) if incoming? && conversation.waiting_since.blank? end def human_response? @@ -276,8 +275,12 @@ class Message < ApplicationRecord def dispatch_create_events Rails.configuration.dispatcher.dispatch(MESSAGE_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by) + if valid_first_reply? Rails.configuration.dispatcher.dispatch(FIRST_REPLY_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by) + conversation.update(first_reply_created_at: created_at, waiting_since: nil) + else + update_waiting_since end end diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index 7578f0b7d..af3589ce5 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -15,8 +15,6 @@ RSpec.describe 'Conversations API', type: :request do context 'when it is an authenticated user' do let(:agent) { create(:user, account: account, role: :agent) } let(:conversation) { create(:conversation, account: account) } - let(:attended_conversation) { create(:conversation, account: account, first_reply_created_at: Time.now.utc) } - let(:unattended_conversation) { create(:conversation, account: account, first_reply_created_at: nil) } before do create(:inbox_member, user: agent, inbox: conversation.inbox) @@ -48,9 +46,16 @@ RSpec.describe 'Conversations API', type: :request do end it 'returns unattended conversations' do + attended_conversation = create(:conversation, account: account, first_reply_created_at: Time.now.utc) + # to ensure that waiting since value is populated + create(:message, message_type: :outgoing, conversation: attended_conversation, account: account) + unattended_conversation_no_first_reply = create(:conversation, account: account, first_reply_created_at: nil) + unattended_conversation_waiting_since = create(:conversation, account: account, first_reply_created_at: Time.now.utc) + agent_1 = create(:user, account: account, role: :agent) create(:inbox_member, user: agent_1, inbox: attended_conversation.inbox) - create(:inbox_member, user: agent_1, inbox: unattended_conversation.inbox) + create(:inbox_member, user: agent_1, inbox: unattended_conversation_no_first_reply.inbox) + create(:inbox_member, user: agent_1, inbox: unattended_conversation_waiting_since.inbox) get "/api/v1/accounts/#{account.id}/conversations", headers: agent_1.create_new_auth_token, @@ -59,8 +64,8 @@ RSpec.describe 'Conversations API', type: :request do expect(response).to have_http_status(:success) body = JSON.parse(response.body, symbolize_names: true) - expect(body[:data][:meta][:all_count]).to eq(1) - expect(body[:data][:payload].count).to eq(1) + expect(body[:data][:meta][:all_count]).to eq(2) + expect(body[:data][:payload].count).to eq(2) end end end diff --git a/spec/finders/conversation_finder_spec.rb b/spec/finders/conversation_finder_spec.rb index 134ab6d7c..2ce5b6bf9 100644 --- a/spec/finders/conversation_finder_spec.rb +++ b/spec/finders/conversation_finder_spec.rb @@ -160,9 +160,13 @@ describe ConversationFinder do let(:params) { { status: 'open', assignee_type: 'me', conversation_type: 'unattended' } } it 'returns unattended conversations' do - create_list(:conversation, 25, account: account, inbox: inbox, assignee: user_1) + create(:conversation, account: account, first_reply_created_at: Time.now.utc, assignee: user_1) # attended_conversation + create(:conversation, account: account, first_reply_created_at: nil, assignee: user_1) # unattended_conversation_no_first_reply + create(:conversation, account: account, first_reply_created_at: Time.now.utc, + assignee: user_1, waiting_since: Time.now.utc) # unattended_conversation_waiting_since + result = conversation_finder.perform - expect(result[:conversations].length).to be 25 + expect(result[:conversations].length).to be 2 end end end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 25504e776..88a0bd898 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -95,56 +95,81 @@ RSpec.describe Message do end end - describe 'Check if message is a valid first reply' do - it 'is valid if it is outgoing' do - outgoing_message = create(:message, message_type: :outgoing) - expect(outgoing_message.valid_first_reply?).to be true + describe 'message create event' do + let(:conversation) { create(:conversation) } + + it 'updates the conversation first reply created at if it is the first outgoing message' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at + + outgoing_message = create(:message, message_type: :outgoing, conversation: conversation) + + expect(conversation.first_reply_created_at).to eq outgoing_message.created_at + expect(conversation.waiting_since).to be_nil end - it 'is invalid if it is not outgoing' do - incoming_message = create(:message, message_type: :incoming) - expect(incoming_message.valid_first_reply?).to be false + it 'does not update the conversation first reply created at if the message is incoming' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at - activity_message = create(:message, message_type: :activity) - expect(activity_message.valid_first_reply?).to be false + create(:message, message_type: :incoming, conversation: conversation) - template_message = create(:message, message_type: :template) - expect(template_message.valid_first_reply?).to be false + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at end - it 'is invalid if it is outgoing but private' do - conversation = create(:conversation) + it 'does not update the conversation first reply created at if the message is template' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at - outgoing_message = create(:message, message_type: :outgoing, conversation: conversation, private: true) - expect(outgoing_message.valid_first_reply?).to be false + create(:message, message_type: :template, conversation: conversation) + + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at + end + + it 'does not update the conversation first reply created at if the message is activity' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at + + create(:message, message_type: :activity, conversation: conversation) + + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at + end + + it 'does not update the conversation first reply created at if the message is a private message' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at + + create(:message, message_type: :outgoing, conversation: conversation, private: true) + + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at - # next message should be a valid reply next_message = create(:message, message_type: :outgoing, conversation: conversation) - expect(next_message.valid_first_reply?).to be true + expect(conversation.first_reply_created_at).to eq next_message.created_at + expect(conversation.waiting_since).to be_nil end - it 'is invalid if it is not the first reply' do - conversation = create(:conversation) - first_message = create(:message, message_type: :outgoing, conversation: conversation) - expect(first_message.valid_first_reply?).to be true + it 'does not update first reply if the message is sent as campaign' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at - second_message = create(:message, message_type: :outgoing, conversation: conversation) - expect(second_message.valid_first_reply?).to be false + create(:message, message_type: :outgoing, conversation: conversation, additional_attributes: { campaign_id: 1 }) + + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at end - it 'is invalid if it is sent as campaign' do - conversation = create(:conversation) - campaign_message = create(:message, message_type: :outgoing, conversation: conversation, additional_attributes: { campaign_id: 1 }) - expect(campaign_message.valid_first_reply?).to be false + it 'does not update first reply if the message is sent by automation' do + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at - second_message = create(:message, message_type: :outgoing, conversation: conversation) - expect(second_message.valid_first_reply?).to be true - end + create(:message, message_type: :outgoing, conversation: conversation, content_attributes: { automation_rule_id: 1 }) - it 'is invalid if it is sent by automation' do - conversation = create(:conversation) - automation_message = create(:message, message_type: :outgoing, conversation: conversation, content_attributes: { automation_rule_id: 1 }) - expect(automation_message.valid_first_reply?).to be false + expect(conversation.first_reply_created_at).to be_nil + expect(conversation.waiting_since).to eq conversation.created_at end end