From d7566c453d3ab48632ffdd85ea4cfa7c0db52447 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Wed, 19 Jul 2023 12:12:30 -0700 Subject: [PATCH] chore: Take the count directly rather than grouping the conversations (#7535) --- app/builders/v2/report_builder.rb | 14 ++++----- app/helpers/report_helper.rb | 39 ++++++++++++++++++++----- spec/builders/v2/report_builder_spec.rb | 8 +++-- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/app/builders/v2/report_builder.rb b/app/builders/v2/report_builder.rb index b5fb8045b..b0f6ba25e 100644 --- a/app/builders/v2/report_builder.rb +++ b/app/builders/v2/report_builder.rb @@ -33,18 +33,18 @@ class V2::ReportBuilder def summary { - conversations_count: conversations_count.values.sum, - incoming_messages_count: incoming_messages_count.values.sum, - outgoing_messages_count: outgoing_messages_count.values.sum, + conversations_count: conversations.count, + incoming_messages_count: incoming_messages.count, + outgoing_messages_count: outgoing_messages.count, avg_first_response_time: avg_first_response_time_summary, avg_resolution_time: avg_resolution_time_summary, - resolutions_count: resolutions_count.values.sum + resolutions_count: resolutions.count } end def conversation_metrics if params[:type].equal?(:account) - conversations + live_conversations else agent_metrics.sort_by { |hash| hash[:metric][:open] }.reverse end @@ -89,12 +89,12 @@ class V2::ReportBuilder email: @user.email, thumbnail: @user.avatar_url, availability: account_user.availability_status, - metric: conversations + metric: live_conversations } end end - def conversations + def live_conversations @open_conversations = scope.conversations.where(account_id: @account.id).open metric = { open: @open_conversations.count, diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index 8ee747eb9..796986e5b 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -17,21 +17,36 @@ module ReportHelper end def conversations_count - (get_grouped_values scope.conversations.where(account_id: account.id)).count + (get_grouped_values conversations).count end def incoming_messages_count - (get_grouped_values scope.messages.where(account_id: account.id).incoming.unscope(:order)).count + (get_grouped_values incoming_messages).count end def outgoing_messages_count - (get_grouped_values scope.messages.where(account_id: account.id).outgoing.unscope(:order)).count + (get_grouped_values outgoing_messages).count end def resolutions_count - object_scope = scope.reporting_events.joins(:conversation).select(:conversation_id).where(account_id: account.id, name: :conversation_resolved, - conversations: { status: :resolved }).distinct - (get_grouped_values object_scope).count + (get_grouped_values resolutions).count + end + + def conversations + scope.conversations.where(account_id: account.id, created_at: range) + end + + def incoming_messages + scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order) + end + + def outgoing_messages + scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order) + end + + def resolutions + scope.reporting_events.joins(:conversation).select(:conversation_id).where(account_id: account.id, name: :conversation_resolved, + conversations: { status: :resolved }, created_at: range).distinct end def avg_first_response_time @@ -51,7 +66,11 @@ module ReportHelper def avg_resolution_time_summary reporting_events = scope.reporting_events .where(name: 'conversation_resolved', account_id: account.id, created_at: range) - avg_rt = params[:business_hours] ? reporting_events.average(:value_in_business_hours) : reporting_events.average(:value) + avg_rt = if params[:business_hours].present? + reporting_events.average(:value_in_business_hours) + else + reporting_events.average(:value) + end return 0 if avg_rt.blank? @@ -61,7 +80,11 @@ module ReportHelper def avg_first_response_time_summary reporting_events = scope.reporting_events .where(name: 'first_response', account_id: account.id, created_at: range) - avg_frt = params[:business_hours] ? reporting_events.average(:value_in_business_hours) : reporting_events.average(:value) + avg_frt = if params[:business_hours].present? + reporting_events.average(:value_in_business_hours) + else + reporting_events.average(:value) + end return 0 if avg_frt.blank? diff --git a/spec/builders/v2/report_builder_spec.rb b/spec/builders/v2/report_builder_spec.rb index b19ff3343..0b0273a0f 100644 --- a/spec/builders/v2/report_builder_spec.rb +++ b/spec/builders/v2/report_builder_spec.rb @@ -6,6 +6,8 @@ describe V2::ReportBuilder do let_it_be(:label_1) { create(:label, title: 'Label_1', account: account) } let_it_be(:label_2) { create(:label, title: 'Label_2', account: account) } + # Update this spec to use travel_to + # This spec breaks in certain timezone describe '#timeseries' do before_all do user = create(:user, account: account) @@ -156,13 +158,14 @@ describe V2::ReportBuilder do it 'returns argument error for incorrect group by' do params = { type: :account, + metric: 'avg_first_response_time', since: (Time.zone.today - 3.days).to_time.to_i.to_s, until: Time.zone.today.end_of_day.to_time.to_i.to_s, group_by: 'test'.to_s } builder = described_class.new(account, params) - expect { builder.summary }.to raise_error(ArgumentError) + expect { builder.timeseries }.to raise_error(ArgumentError) end end @@ -296,6 +299,7 @@ describe V2::ReportBuilder do it 'returns argument error for incorrect group by' do params = { + metric: 'avg_first_response_time', type: :label, id: label_2.id, since: (Time.zone.today - 3.days).to_time.to_i.to_s, @@ -304,7 +308,7 @@ describe V2::ReportBuilder do } builder = described_class.new(account, params) - expect { builder.summary }.to raise_error(ArgumentError) + expect { builder.timeseries }.to raise_error(ArgumentError) end end end