From 1e6734fcc5c1aced7595f3b1a9cbf22e3fbe82c5 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 31 Jul 2025 14:55:04 +0530 Subject: [PATCH] feat: log all conversation open events --- app/helpers/reporting_event_helper.rb | 2 +- app/listeners/reporting_event_listener.rb | 27 ++- spec/helpers/reporting_event_helper_spec.rb | 177 ++++++++++++++++++ .../reporting_event_listener_spec.rb | 30 +-- 4 files changed, 215 insertions(+), 21 deletions(-) create mode 100644 spec/helpers/reporting_event_helper_spec.rb diff --git a/app/helpers/reporting_event_helper.rb b/app/helpers/reporting_event_helper.rb index 00b3c2de5..f0a419cc8 100644 --- a/app/helpers/reporting_event_helper.rb +++ b/app/helpers/reporting_event_helper.rb @@ -24,7 +24,7 @@ module ReportingEventHelper # So we pick the latest event event = ReportingEvent.where( conversation_id: conversation.id, - name: %w[conversation_bot_handoff conversation_reopened] + name: %w[conversation_bot_handoff conversation_opened] ).order(event_end_time: :desc).first return event.event_end_time if event&.event_end_time diff --git a/app/listeners/reporting_event_listener.rb b/app/listeners/reporting_event_listener.rb index b0e86907f..9f683a97f 100644 --- a/app/listeners/reporting_event_listener.rb +++ b/app/listeners/reporting_event_listener.rb @@ -99,27 +99,38 @@ class ReportingEventListener < BaseListener name: 'conversation_resolved' ).order(event_end_time: :desc).first - # If there's a previous resolved event, this is a reopening - return unless last_resolved_event + # For first-time openings, value is 0 + # For reopenings, calculate time since resolution + if last_resolved_event + time_since_resolved = conversation.updated_at.to_i - last_resolved_event.event_end_time.to_i + business_hours_value = business_hours(conversation.inbox, last_resolved_event.event_end_time, conversation.updated_at) + start_time = last_resolved_event.event_end_time + else + time_since_resolved = 0 + business_hours_value = 0 + start_time = conversation.created_at + end - time_since_resolved = conversation.updated_at.to_i - last_resolved_event.event_end_time.to_i + create_conversation_opened_event(conversation, time_since_resolved, business_hours_value, start_time) + end + private + + def create_conversation_opened_event(conversation, time_since_resolved, business_hours_value, start_time) reporting_event = ReportingEvent.new( - name: 'conversation_reopened', + name: 'conversation_opened', value: time_since_resolved, - value_in_business_hours: business_hours(conversation.inbox, last_resolved_event.event_end_time, conversation.updated_at), + value_in_business_hours: business_hours_value, account_id: conversation.account_id, inbox_id: conversation.inbox_id, user_id: conversation.assignee_id, conversation_id: conversation.id, - event_start_time: last_resolved_event.event_end_time, + event_start_time: start_time, event_end_time: conversation.updated_at ) reporting_event.save! end - private - def create_bot_resolved_event(conversation, reporting_event) return unless conversation.inbox.active_bot? # We don't want to create a bot_resolved event if there is user interaction on the conversation diff --git a/spec/helpers/reporting_event_helper_spec.rb b/spec/helpers/reporting_event_helper_spec.rb new file mode 100644 index 000000000..8d83cd61b --- /dev/null +++ b/spec/helpers/reporting_event_helper_spec.rb @@ -0,0 +1,177 @@ +require 'rails_helper' + +RSpec.describe ReportingEventHelper, type: :helper do + describe '#last_non_human_activity' do + let(:account) { create(:account) } + let(:inbox) { create(:inbox, account: account) } + let(:user) { create(:user, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) } + + context 'when conversation has no events' do + it 'returns conversation created_at' do + expect(helper.last_non_human_activity(conversation)).to eq(conversation.created_at) + end + end + + context 'when conversation has bot handoff event' do + let!(:handoff_event) do + create(:reporting_event, + name: 'conversation_bot_handoff', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_end_time: 2.hours.ago) + end + + it 'returns handoff event end time' do + expect(helper.last_non_human_activity(conversation)).to eq(handoff_event.event_end_time) + end + end + + context 'when conversation has bot resolved event' do + let!(:bot_resolved_event) do + create(:reporting_event, + name: 'conversation_bot_resolved', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_end_time: 3.hours.ago) + end + + it 'returns bot resolved event end time' do + expect(helper.last_non_human_activity(conversation)).to eq(bot_resolved_event.event_end_time) + end + end + + context 'when conversation is reopened after bot resolution' do + let(:creation_time) { 5.days.ago } + let(:bot_resolution_time) { 5.days.ago + 5.minutes } + let(:reopening_time) { 1.hour.ago } + + let!(:conversation) do + create(:conversation, + account: account, + inbox: inbox, + assignee: user, + created_at: creation_time) + end + + before do + # First opened event + create(:reporting_event, + name: 'conversation_opened', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + value: 0, + event_start_time: creation_time, + event_end_time: creation_time) + + # Bot resolved event + create(:reporting_event, + name: 'conversation_bot_resolved', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_start_time: creation_time, + event_end_time: bot_resolution_time) + + # Resolved event + create(:reporting_event, + name: 'conversation_resolved', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_start_time: creation_time, + event_end_time: bot_resolution_time) + + # Reopened event + create(:reporting_event, + name: 'conversation_opened', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + value: (reopening_time - bot_resolution_time).to_i, + event_start_time: bot_resolution_time, + event_end_time: reopening_time) + end + + it 'returns the reopening event time, not the creation time' do + # This is the key test: last_non_human_activity should return the reopening time + # so that first response time is calculated from when the conversation was reopened, + # not from when it was originally created + expect(helper.last_non_human_activity(conversation)).to eq(reopening_time) + + # Verify it's not returning the creation time or bot resolution time + expect(helper.last_non_human_activity(conversation)).not_to eq(creation_time) + expect(helper.last_non_human_activity(conversation)).not_to eq(bot_resolution_time) + end + end + + context 'when conversation has multiple types of events' do + let(:opened_event_time) { 1.hour.ago } + + before do + create(:reporting_event, + name: 'conversation_bot_resolved', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_end_time: 4.hours.ago) + + create(:reporting_event, + name: 'conversation_bot_handoff', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_end_time: 3.hours.ago) + + create(:reporting_event, + name: 'conversation_opened', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + event_end_time: opened_event_time) + end + + it 'returns the most recent handoff or opened event' do + # opened_event is more recent than handoff_event + expect(helper.last_non_human_activity(conversation)).to eq(opened_event_time) + end + end + + context 'when conversation has multiple reopenings' do + let(:third_opened_time) { 30.minutes.ago } + + before do + create(:reporting_event, + name: 'conversation_opened', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + value: 0, + event_end_time: 5.days.ago) + + create(:reporting_event, + name: 'conversation_opened', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + value: 3600, + event_end_time: 2.days.ago) + + create(:reporting_event, + name: 'conversation_opened', + conversation_id: conversation.id, + account_id: account.id, + inbox_id: inbox.id, + value: 7200, + event_end_time: third_opened_time) + end + + it 'returns the most recent opened event' do + expect(helper.last_non_human_activity(conversation)).to eq(third_opened_time) + end + end + end +end diff --git a/spec/listeners/reporting_event_listener_spec.rb b/spec/listeners/reporting_event_listener_spec.rb index 01c0c2155..b31a85df1 100644 --- a/spec/listeners/reporting_event_listener_spec.rb +++ b/spec/listeners/reporting_event_listener_spec.rb @@ -272,11 +272,17 @@ describe ReportingEventListener do context 'when conversation is opened for the first time' do let(:new_conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) } - it 'does not create conversation_reopened event' do - expect(account.reporting_events.where(name: 'conversation_reopened').count).to be 0 + it 'creates conversation_opened event with value 0' do + expect(account.reporting_events.where(name: 'conversation_opened').count).to be 0 event = Events::Base.new('conversation.opened', Time.zone.now, conversation: new_conversation) listener.conversation_opened(event) - expect(account.reporting_events.where(name: 'conversation_reopened').count).to be 0 + expect(account.reporting_events.where(name: 'conversation_opened').count).to be 1 + + opened_event = account.reporting_events.where(name: 'conversation_opened').first + expect(opened_event.value).to eq 0 + expect(opened_event.value_in_business_hours).to eq 0 + expect(opened_event.event_start_time).to be_within(1.second).of(new_conversation.created_at) + expect(opened_event.event_end_time).to be_within(1.second).of(new_conversation.updated_at) end end @@ -300,28 +306,28 @@ describe ReportingEventListener do event_end_time: resolved_time) end - it 'creates conversation_reopened event' do - expect(account.reporting_events.where(name: 'conversation_reopened').count).to be 0 + it 'creates conversation_opened event' do + expect(account.reporting_events.where(name: 'conversation_opened').count).to be 0 event = Events::Base.new('conversation.opened', reopened_time, conversation: reopened_conversation) listener.conversation_opened(event) - expect(account.reporting_events.where(name: 'conversation_reopened').count).to be 1 + expect(account.reporting_events.where(name: 'conversation_opened').count).to be 1 end it 'calculates correct time since resolution' do event = Events::Base.new('conversation.opened', reopened_time, conversation: reopened_conversation) listener.conversation_opened(event) - reopened_event = account.reporting_events.where(name: 'conversation_reopened').first + reopened_event = account.reporting_events.where(name: 'conversation_opened').first expect(reopened_event.value).to be_within(1).of(3600) # 1 hour = 3600 seconds expect(reopened_event.event_start_time).to be_within(1.second).of(resolved_time) expect(reopened_event.event_end_time).to be_within(1.second).of(reopened_time) end - it 'sets correct attributes for conversation_reopened event' do + it 'sets correct attributes for conversation_opened event' do event = Events::Base.new('conversation.opened', reopened_time, conversation: reopened_conversation) listener.conversation_opened(event) - reopened_event = account.reporting_events.where(name: 'conversation_reopened').first + reopened_event = account.reporting_events.where(name: 'conversation_opened').first expect(reopened_event.account_id).to eq(account.id) expect(reopened_event.inbox_id).to eq(inbox.id) expect(reopened_event.conversation_id).to eq(reopened_conversation.id) @@ -348,11 +354,11 @@ describe ReportingEventListener do event_end_time: resolved_time) end - it 'creates conversation_reopened event with business hour value' do + it 'creates conversation_opened event with business hour value' do event = Events::Base.new('conversation.opened', reopened_time, conversation: business_hours_conversation) listener.conversation_opened(event) - reopened_event = account.reporting_events.where(name: 'conversation_reopened').first + reopened_event = account.reporting_events.where(name: 'conversation_opened').first expect(reopened_event.value_in_business_hours).to be 18_000.0 # 5 business hours (26 hours total - 21 non-business hours) end end @@ -394,7 +400,7 @@ describe ReportingEventListener do event = Events::Base.new('conversation.opened', reopened_time, conversation: multiple_resolution_conversation) listener.conversation_opened(event) - reopened_event = account.reporting_events.where(name: 'conversation_reopened').first + reopened_event = account.reporting_events.where(name: 'conversation_opened').first expect(reopened_event.value).to be_within(1).of(1800) # 30 minutes from second resolution expect(reopened_event.event_start_time).to be_within(1.second).of(second_resolved_time) end