diff --git a/app/listeners/reporting_event_listener.rb b/app/listeners/reporting_event_listener.rb index b31d899bb..9f22fe8de 100644 --- a/app/listeners/reporting_event_listener.rb +++ b/app/listeners/reporting_event_listener.rb @@ -47,6 +47,10 @@ class ReportingEventListener < BaseListener message = extract_message_and_account(event)[0] conversation = message.conversation waiting_since = event.data[:waiting_since] + + return if waiting_since.blank? + + # When waiting_since is nil, set reply_time to 0 reply_time = message.created_at.to_i - waiting_since.to_i reporting_event = ReportingEvent.new( diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 922118b09..d6c5d0e4a 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -198,11 +198,21 @@ class Conversation < ApplicationRecord private def execute_after_update_commit_callbacks + handle_resolved_status_change notify_status_change create_activity notify_conversation_updation end + def handle_resolved_status_change + # When conversation is resolved, clear waiting_since using update_column to avoid callbacks + return unless saved_change_to_status? && status == 'resolved' + + # rubocop:disable Rails/SkipsModelValidations + update_column(:waiting_since, nil) + # rubocop:enable Rails/SkipsModelValidations + end + def ensure_snooze_until_reset self.snoozed_until = nil unless snoozed? end diff --git a/spec/listeners/reporting_event_listener_spec.rb b/spec/listeners/reporting_event_listener_spec.rb index 9a6b8d123..0edf79556 100644 --- a/spec/listeners/reporting_event_listener_spec.rb +++ b/spec/listeners/reporting_event_listener_spec.rb @@ -66,6 +66,34 @@ describe ReportingEventListener do end describe '#reply_created' do + let(:contact) { create(:contact, account: account) } + + def create_customer_message(conversation, created_at: Time.current) + create(:message, + message_type: 'incoming', + account: account, + inbox: inbox, + conversation: conversation, + sender: contact, + created_at: created_at) + end + + def create_agent_message(conversation, created_at: Time.current, sender: user) + create(:message, + message_type: 'outgoing', + account: account, + inbox: inbox, + conversation: conversation, + sender: sender, + created_at: created_at) + end + + def create_reply_event(agent_message, waiting_since, event_time = nil) + Events::Base.new('reply.created', event_time || agent_message.created_at, + waiting_since: waiting_since, + message: agent_message) + end + it 'creates reply created event' do event = Events::Base.new('reply.created', Time.zone.now, waiting_since: 2.hours.ago, message: message) listener.reply_created(event) @@ -74,6 +102,88 @@ describe ReportingEventListener do expect(events.length).to be 1 expect(events.first.value).to be_within(1).of(7200) end + + context 'when conversation is reopened' do + let(:resolved_conversation) do + create(:conversation, account: account, inbox: inbox, assignee: user, + status: 'resolved', contact: contact) + end + + context 'when customer sends message after resolution' do + it 'calculates reply time from the reopening message' do + customer_message_time = 3.hours.ago + create_customer_message(resolved_conversation, created_at: customer_message_time) + + resolved_conversation.reload + expect(resolved_conversation.status).to eq('open') + + agent_reply_time = 1.hour.ago + agent_message = create_agent_message(resolved_conversation, created_at: agent_reply_time) + + event = create_reply_event(agent_message, customer_message_time) + listener.reply_created(event) + + events = account.reporting_events.where(name: 'reply_time', conversation_id: resolved_conversation.id) + expect(events.length).to be 1 + expect(events.first.value).to be_within(60).of(7200) + end + end + + context 'when conversation has multiple reopenings' do + it 'tracks reply time correctly for each reopening' do + create_customer_message(resolved_conversation, created_at: 5.hours.ago) + first_agent_reply = create_agent_message(resolved_conversation, created_at: 4.hours.ago) + + event = create_reply_event(first_agent_reply, 5.hours.ago) + listener.reply_created(event) + + resolved_conversation.update!(status: 'resolved') + + create_customer_message(resolved_conversation, created_at: 2.hours.ago) + second_agent_reply = create_agent_message(resolved_conversation, created_at: 1.5.hours.ago) + + event = create_reply_event(second_agent_reply, 2.hours.ago) + listener.reply_created(event) + + events = account.reporting_events.where(name: 'reply_time', conversation_id: resolved_conversation.id) + .order(created_at: :asc) + expect(events.length).to be 2 + expect(events.first.value).to be_within(60).of(3600) + expect(events.second.value).to be_within(60).of(1800) + end + end + + context 'when conversation is manually reopened' do + it 'sets waiting_since when first customer message arrives after manual reopening' do + resolved_conversation.update!(status: 'open') + + customer_message_time = 1.hour.ago + create_customer_message(resolved_conversation, created_at: customer_message_time) + + agent_reply_time = 15.minutes.ago + agent_message = create_agent_message(resolved_conversation, created_at: agent_reply_time) + + event = create_reply_event(agent_message, customer_message_time) + listener.reply_created(event) + + events = account.reporting_events.where(name: 'reply_time', conversation_id: resolved_conversation.id) + expect(events.length).to be 1 + expect(events.first.value).to be_within(60).of(2700) + end + end + + context 'when waiting_since is nil' do + it 'does not creates reply time events' do + agent_message = create_agent_message(resolved_conversation) + + event = create_reply_event(agent_message, nil) + listener.reply_created(event) + + events = account.reporting_events.where(name: 'reply_time', conversation_id: resolved_conversation.id) + expect(events.length).to be 0 + end + end + end end describe '#first_reply_created' do diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index aef91603d..a29359528 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -836,4 +836,117 @@ RSpec.describe Conversation do expect(message_window_service).to have_received(:can_reply?) end end + + describe 'reply time calculation flows' do + include ActiveJob::TestHelper + + let(:account) { create(:account) } + let(:inbox) { create(:inbox, account: account) } + let(:contact) { create(:contact, account: account) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:conversation) { create(:conversation, account: account, inbox: inbox, contact: contact, assignee: agent, waiting_since: nil) } + let(:conversation_start_time) { 5.hours.ago } + + before do + create(:inbox_member, user: agent, inbox: inbox) + # rubocop:disable Rails/SkipsModelValidations + conversation.update_column(:waiting_since, nil) + conversation.update_column(:created_at, conversation_start_time) + # rubocop:enable Rails/SkipsModelValidations + conversation.messages.destroy_all + conversation.reporting_events.destroy_all + conversation.reload + end + + def create_customer_message(conversation, created_at: Time.current) + message = nil + perform_enqueued_jobs do + message = create(:message, + message_type: 'incoming', + account: conversation.account, + inbox: conversation.inbox, + conversation: conversation, + sender: conversation.contact, + created_at: created_at) + end + message + end + + def create_agent_message(conversation, created_at: Time.current) + message = nil + perform_enqueued_jobs do + message = create(:message, + message_type: 'outgoing', + account: conversation.account, + inbox: conversation.inbox, + conversation: conversation, + sender: conversation.assignee, + created_at: created_at) + end + message + end + + it 'correctly tracks waiting_since and creates first response time events' do + create_customer_message(conversation, created_at: conversation_start_time) + conversation.reload + expect(conversation.waiting_since).to be_within(1.second).of(conversation_start_time) + + # Agent replies - this should create first response event + agent_reply1_time = 4.hours.ago + create_agent_message(conversation, created_at: agent_reply1_time) + + first_response_events = account.reporting_events.where(name: 'first_response', conversation_id: conversation.id) + expect(first_response_events.count).to eq(1) + expect(first_response_events.first.value).to be_within(1.second).of(1.hour) + + # the first response should also clear the waiting_since + conversation.reload + expect(conversation.waiting_since).to be_nil + end + + it 'does not reset waiting_since if customer sends another message' do + create_customer_message(conversation, created_at: conversation_start_time) + conversation.reload + expect(conversation.waiting_since).to be_within(1.second).of(conversation_start_time) + + create_customer_message(conversation, created_at: 3.hours.ago) + conversation.reload + expect(conversation.waiting_since).to be_within(1.second).of(conversation_start_time) + end + + it 'records the correct reply_time for subsequent messages' do + create_customer_message(conversation, created_at: conversation_start_time) + create_agent_message(conversation, created_at: 4.hours.ago) + create_customer_message(conversation, created_at: 3.hours.ago) + + create_agent_message(conversation, created_at: 2.hours.ago) + reply_events = account.reporting_events.where(name: 'reply_time', conversation_id: conversation.id) + expect(reply_events.count).to eq(1) + expect(reply_events.first.value).to be_within(1.second).of(1.hour) + + conversation.reload + expect(conversation.waiting_since).to be_nil + end + + it 'records zero reply time if an agent sends a message after resolution' do + create_customer_message(conversation, created_at: conversation_start_time) + create_agent_message(conversation, created_at: 4.hours.ago) + create_customer_message(conversation, created_at: 3.hours.ago) + + conversation.toggle_status + expect(conversation.status).to eq('resolved') + + conversation.toggle_status + expect(conversation.status).to eq('open') + + conversation.reload + expect(conversation.waiting_since).to be_nil + + create_agent_message(conversation, created_at: 1.hour.ago) + # update_waiting_since will ensure that no events were created since the waiting_since was nil + # if the event is created it should log zero value, we have handled that in the reporting_event_listener + reply_events = account.reporting_events.where(name: 'reply_time', conversation_id: conversation.id) + expect(reply_events.count).to eq(0) + end + end end