diff --git a/app/builders/v2/report_builder.rb b/app/builders/v2/report_builder.rb index fb986d335..b935bd520 100644 --- a/app/builders/v2/report_builder.rb +++ b/app/builders/v2/report_builder.rb @@ -1,6 +1,7 @@ class V2::ReportBuilder include DateRangeHelper include ReportHelper + attr_reader :account, :params DEFAULT_GROUP_BY = 'day'.freeze diff --git a/app/builders/v2/reports/conversations/base_report_builder.rb b/app/builders/v2/reports/conversations/base_report_builder.rb index a7961b0d6..57b115945 100644 --- a/app/builders/v2/reports/conversations/base_report_builder.rb +++ b/app/builders/v2/reports/conversations/base_report_builder.rb @@ -3,23 +3,10 @@ class V2::Reports::Conversations::BaseReportBuilder private - AVG_METRICS = %w[avg_first_response_time avg_resolution_time reply_time].freeze - COUNT_METRICS = %w[ - conversations_count - incoming_messages_count - outgoing_messages_count - resolutions_count - bot_resolutions_count - bot_handoffs_count - ].freeze - def builder_class(metric) - case metric - when *AVG_METRICS - V2::Reports::Timeseries::AverageReportBuilder - when *COUNT_METRICS - V2::Reports::Timeseries::CountReportBuilder - end + return unless ReportingEvents::MetricRegistry.supported_metric?(metric) + + V2::Reports::Timeseries::ReportBuilder end def log_invalid_metric diff --git a/app/builders/v2/reports/timeseries/average_report_builder.rb b/app/builders/v2/reports/timeseries/average_report_builder.rb deleted file mode 100644 index 5df718b6a..000000000 --- a/app/builders/v2/reports/timeseries/average_report_builder.rb +++ /dev/null @@ -1,48 +0,0 @@ -class V2::Reports::Timeseries::AverageReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder - def timeseries - grouped_average_time = reporting_events.average(average_value_key) - grouped_event_count = reporting_events.count - grouped_average_time.each_with_object([]) do |element, arr| - event_date, average_time = element - arr << { - value: average_time, - timestamp: event_date.in_time_zone(timezone).to_i, - count: grouped_event_count[event_date] - } - end - end - - def aggregate_value - object_scope.average(average_value_key) - end - - private - - def event_name - metric_to_event_name = { - avg_first_response_time: :first_response, - avg_resolution_time: :conversation_resolved, - reply_time: :reply_time - } - metric_to_event_name[params[:metric].to_sym] - end - - def object_scope - scope.reporting_events.where(name: event_name, created_at: range, account_id: account.id) - end - - def reporting_events - @grouped_values = object_scope.group_by_period( - group_by, - :created_at, - default_value: 0, - range: range, - permit: %w[day week month year hour], - time_zone: timezone - ) - end - - def average_value_key - @average_value_key ||= params[:business_hours].present? ? :value_in_business_hours : :value - end -end diff --git a/app/builders/v2/reports/timeseries/base_timeseries_builder.rb b/app/builders/v2/reports/timeseries/base_timeseries_builder.rb index 50699417d..0eaab7e52 100644 --- a/app/builders/v2/reports/timeseries/base_timeseries_builder.rb +++ b/app/builders/v2/reports/timeseries/base_timeseries_builder.rb @@ -1,12 +1,13 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder include TimezoneHelper include DateRangeHelper + DEFAULT_GROUP_BY = 'day'.freeze pattr_initialize :account, :params def scope - case params[:type].to_sym + case dimension_type.to_sym when :account account when :inbox @@ -20,6 +21,21 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder end end + def data_source + @data_source ||= Reports::DataSource.for( + account: account, + metric: params[:metric], + dimension_type: dimension_type, + dimension_id: params[:id], + scope: scope, + range: range, + group_by: group_by, + timezone: timezone, + timezone_offset: params[:timezone_offset], + business_hours: params[:business_hours] + ) + end + def inbox @inbox ||= account.inboxes.find(params[:id]) end @@ -41,6 +57,12 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder end def timezone - @timezone ||= timezone_name_from_offset(params[:timezone_offset]) + @timezone ||= timezone_name_from_params(params[:timezone], params[:timezone_offset]) + end + + private + + def dimension_type + (params[:type].presence || 'account').to_s end end diff --git a/app/builders/v2/reports/timeseries/count_report_builder.rb b/app/builders/v2/reports/timeseries/count_report_builder.rb deleted file mode 100644 index bb3b1250c..000000000 --- a/app/builders/v2/reports/timeseries/count_report_builder.rb +++ /dev/null @@ -1,78 +0,0 @@ -class V2::Reports::Timeseries::CountReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder - def timeseries - grouped_count.each_with_object([]) do |element, arr| - event_date, event_count = element - - # The `event_date` is in Date format (without time), such as "Wed, 15 May 2024". - # We need a timestamp for the start of the day. However, we can't use `event_date.to_time.to_i` - # because it converts the date to 12:00 AM server timezone. - # The desired output should be 12:00 AM in the specified timezone. - arr << { value: event_count, timestamp: event_date.in_time_zone(timezone).to_i } - end - end - - def aggregate_value - object_scope.count - end - - private - - def metric - @metric ||= params[:metric] - end - - def object_scope - send("scope_for_#{metric}") - end - - def scope_for_conversations_count - scope.conversations.where(account_id: account.id, created_at: range) - end - - def scope_for_incoming_messages_count - scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order) - end - - def scope_for_outgoing_messages_count - scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order) - end - - def scope_for_resolutions_count - scope.reporting_events.where( - name: :conversation_resolved, - account_id: account.id, - created_at: range - ) - end - - def scope_for_bot_resolutions_count - scope.reporting_events.where( - name: :conversation_bot_resolved, - account_id: account.id, - created_at: range - ) - end - - def scope_for_bot_handoffs_count - scope.reporting_events.joins(:conversation).select(:conversation_id).where( - name: :conversation_bot_handoff, - account_id: account.id, - created_at: range - ).distinct - end - - def grouped_count - # IMPORTANT: time_zone parameter affects both data grouping AND output timestamps - # It converts timestamps to the target timezone before grouping, which means - # the same event can fall into different day buckets depending on timezone - # Example: 2024-01-15 00:00 UTC becomes 2024-01-14 16:00 PST (falls on different day) - @grouped_values = object_scope.group_by_period( - group_by, - :created_at, - default_value: 0, - range: range, - permit: %w[day week month year hour], - time_zone: timezone - ).count - end -end diff --git a/app/builders/v2/reports/timeseries/report_builder.rb b/app/builders/v2/reports/timeseries/report_builder.rb new file mode 100644 index 000000000..a3e83c78a --- /dev/null +++ b/app/builders/v2/reports/timeseries/report_builder.rb @@ -0,0 +1,9 @@ +class V2::Reports::Timeseries::ReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder + def timeseries + data_source.timeseries + end + + def aggregate_value + data_source.aggregate + end +end diff --git a/spec/builders/v2/reports/conversations/metric_builder_spec.rb b/spec/builders/v2/reports/conversations/metric_builder_spec.rb index 1b0ed7a38..018bd567b 100644 --- a/spec/builders/v2/reports/conversations/metric_builder_spec.rb +++ b/spec/builders/v2/reports/conversations/metric_builder_spec.rb @@ -5,12 +5,10 @@ RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do let(:account) { create(:account) } let(:params) { { since: '2023-01-01', until: '2024-01-01' } } - let(:count_builder_instance) { instance_double(V2::Reports::Timeseries::CountReportBuilder, aggregate_value: 42) } - let(:avg_builder_instance) { instance_double(V2::Reports::Timeseries::AverageReportBuilder, aggregate_value: 42) } + let(:builder_instance) { instance_double(V2::Reports::Timeseries::ReportBuilder, aggregate_value: 42) } before do - allow(V2::Reports::Timeseries::CountReportBuilder).to receive(:new).and_return(count_builder_instance) - allow(V2::Reports::Timeseries::AverageReportBuilder).to receive(:new).and_return(avg_builder_instance) + allow(V2::Reports::Timeseries::ReportBuilder).to receive(:new).and_return(builder_instance) end describe '#summary' do @@ -31,8 +29,8 @@ RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do it 'creates builders with proper params' do subject.summary - expect(V2::Reports::Timeseries::CountReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count')) - expect(V2::Reports::Timeseries::AverageReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time')) + expect(V2::Reports::Timeseries::ReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count')) + expect(V2::Reports::Timeseries::ReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time')) end end diff --git a/spec/builders/v2/reports/conversations/report_builder_spec.rb b/spec/builders/v2/reports/conversations/report_builder_spec.rb index db7a0ac45..a2b293461 100644 --- a/spec/builders/v2/reports/conversations/report_builder_spec.rb +++ b/spec/builders/v2/reports/conversations/report_builder_spec.rb @@ -4,19 +4,19 @@ describe V2::Reports::Conversations::ReportBuilder do subject { described_class.new(account, params) } let(:account) { create(:account) } - let(:average_builder) { V2::Reports::Timeseries::AverageReportBuilder } - let(:count_builder) { V2::Reports::Timeseries::CountReportBuilder } + let(:builder) { V2::Reports::Timeseries::ReportBuilder } - shared_examples 'valid metric handler' do |metric, method, builder| + shared_examples 'valid metric handler' do |metric, method| context 'when a valid metric is given' do let(:params) { { metric: metric } } - it "calls the correct #{method} builder for #{metric}" do + it "calls the shared #{method} builder for #{metric}" do builder_instance = instance_double(builder) allow(builder).to receive(:new).and_return(builder_instance) - allow(builder_instance).to receive(method) + allow(builder_instance).to receive(method).and_return(:result) - builder_instance.public_send(method) + expect(subject.public_send(method)).to eq(:result) + expect(builder).to have_received(:new).with(account, params) expect(builder_instance).to have_received(method) end end @@ -33,12 +33,12 @@ describe V2::Reports::Conversations::ReportBuilder do end describe '#timeseries' do - it_behaves_like 'valid metric handler', 'avg_first_response_time', :timeseries, V2::Reports::Timeseries::AverageReportBuilder - it_behaves_like 'valid metric handler', 'conversations_count', :timeseries, V2::Reports::Timeseries::CountReportBuilder + it_behaves_like 'valid metric handler', 'avg_first_response_time', :timeseries + it_behaves_like 'valid metric handler', 'conversations_count', :timeseries end describe '#aggregate_value' do - it_behaves_like 'valid metric handler', 'avg_first_response_time', :aggregate_value, V2::Reports::Timeseries::AverageReportBuilder - it_behaves_like 'valid metric handler', 'conversations_count', :aggregate_value, V2::Reports::Timeseries::CountReportBuilder + it_behaves_like 'valid metric handler', 'avg_first_response_time', :aggregate_value + it_behaves_like 'valid metric handler', 'conversations_count', :aggregate_value end end diff --git a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb b/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb deleted file mode 100644 index 4f6036f07..000000000 --- a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb +++ /dev/null @@ -1,174 +0,0 @@ -require 'rails_helper' - -describe V2::Reports::Timeseries::AverageReportBuilder do - subject { described_class.new(account, params) } - - let(:account) { create(:account) } - let(:team) { create(:team, account: account) } - let(:inbox) { create(:inbox, account: account) } - let(:label) { create(:label, title: 'spec-billing', account: account) } - let!(:conversation) { create(:conversation, account: account, inbox: inbox, team: team) } - let(:current_time) { '26.10.2020 10:00'.to_datetime } - - let(:params) do - { - type: filter_type, - business_hours: business_hours, - timezone_offset: timezone_offset, - group_by: group_by, - metric: metric, - since: (current_time - 1.week).beginning_of_day.to_i.to_s, - until: current_time.end_of_day.to_i.to_s, - id: filter_id - } - end - let(:timezone_offset) { nil } - let(:group_by) { 'day' } - let(:metric) { 'avg_first_response_time' } - let(:business_hours) { false } - let(:filter_type) { :account } - let(:filter_id) { '' } - - before do - travel_to current_time - conversation.label_list.add(label.title) - conversation.save! - create(:reporting_event, name: 'first_response', value: 80, value_in_business_hours: 10, account: account, created_at: Time.zone.now, - conversation: conversation, inbox: inbox) - create(:reporting_event, name: 'first_response', value: 100, value_in_business_hours: 20, account: account, created_at: 1.hour.ago) - create(:reporting_event, name: 'first_response', value: 93, value_in_business_hours: 30, account: account, created_at: 1.week.ago) - end - - describe '#timeseries' do - context 'when there is no filter applied' do - it 'returns the correct values' do - timeseries_values = subject.timeseries - - expect(timeseries_values).to eq( - [ - { count: 1, timestamp: 1_603_065_600, value: 93.0 }, - { count: 0, timestamp: 1_603_152_000, value: 0 }, - { count: 0, timestamp: 1_603_238_400, value: 0 }, - { count: 0, timestamp: 1_603_324_800, value: 0 }, - { count: 0, timestamp: 1_603_411_200, value: 0 }, - { count: 0, timestamp: 1_603_497_600, value: 0 }, - { count: 0, timestamp: 1_603_584_000, value: 0 }, - { count: 2, timestamp: 1_603_670_400, value: 90.0 } - ] - ) - end - - context 'when business hours is provided' do - let(:business_hours) { true } - - it 'returns correct timeseries' do - timeseries_values = subject.timeseries - - expect(timeseries_values).to eq( - [ - { count: 1, timestamp: 1_603_065_600, value: 30.0 }, - { count: 0, timestamp: 1_603_152_000, value: 0 }, - { count: 0, timestamp: 1_603_238_400, value: 0 }, - { count: 0, timestamp: 1_603_324_800, value: 0 }, - { count: 0, timestamp: 1_603_411_200, value: 0 }, - { count: 0, timestamp: 1_603_497_600, value: 0 }, - { count: 0, timestamp: 1_603_584_000, value: 0 }, - { count: 2, timestamp: 1_603_670_400, value: 15.0 } - ] - ) - end - end - - context 'when group_by is provided' do - let(:group_by) { 'week' } - - it 'returns correct timeseries' do - timeseries_values = subject.timeseries - expect(timeseries_values).to eq( - [ - { count: 1, timestamp: (current_time - 1.week).beginning_of_week(:sunday).to_i, value: 93.0 }, - { count: 2, timestamp: current_time.beginning_of_week(:sunday).to_i, value: 90.0 } - ] - ) - end - end - - context 'when timezone offset is provided' do - let(:timezone_offset) { '5.5' } - let(:group_by) { 'week' } - - it 'returns correct timeseries' do - timeseries_values = subject.timeseries - expect(timeseries_values).to eq( - [ - { count: 1, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 93.0 }, - { count: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 90.0 } - ] - ) - end - end - end - - context 'when the label filter is applied' do - let(:group_by) { 'week' } - let(:filter_type) { 'label' } - let(:filter_id) { label.id } - - it 'returns correct timeseries' do - timeseries_values = subject.timeseries - start_of_the_week = current_time.beginning_of_week(:sunday).to_i - last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i - expect(timeseries_values).to eq( - [ - { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, - { count: 1, timestamp: start_of_the_week, value: 80.0 } - ] - ) - end - end - - context 'when the inbox filter is applied' do - let(:group_by) { 'week' } - let(:filter_type) { 'inbox' } - let(:filter_id) { inbox.id } - - it 'returns correct timeseries' do - timeseries_values = subject.timeseries - start_of_the_week = current_time.beginning_of_week(:sunday).to_i - last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i - expect(timeseries_values).to eq( - [ - { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, - { count: 1, timestamp: start_of_the_week, value: 80.0 } - ] - ) - end - end - - context 'when the team filter is applied' do - let(:group_by) { 'week' } - let(:filter_type) { 'team' } - let(:filter_id) { team.id } - - it 'returns correct timeseries' do - timeseries_values = subject.timeseries - start_of_the_week = current_time.beginning_of_week(:sunday).to_i - last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i - expect(timeseries_values).to eq( - [ - { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, - { count: 1, timestamp: start_of_the_week, value: 80.0 } - ] - ) - end - end - end - - describe '#aggregate_value' do - context 'when there is no filter applied' do - it 'returns the correct average value' do - expect(subject.aggregate_value).to eq 91.0 - end - end - end -end diff --git a/spec/builders/v2/reports/timeseries/count_report_builder_spec.rb b/spec/builders/v2/reports/timeseries/count_report_builder_spec.rb deleted file mode 100644 index 038bd61c2..000000000 --- a/spec/builders/v2/reports/timeseries/count_report_builder_spec.rb +++ /dev/null @@ -1,113 +0,0 @@ -require 'rails_helper' - -describe V2::Reports::Timeseries::CountReportBuilder do - subject { described_class.new(account, params) } - - let(:account) { create(:account) } - let(:account2) { create(:account) } - let(:user) { create(:user, email: 'agent1@example.com') } - let(:inbox) { create(:inbox, account: account) } - let(:inbox2) { create(:inbox, account: account2) } - let(:current_time) { Time.current } - - let(:params) do - { - type: 'agent', - metric: 'resolutions_count', - since: (current_time - 1.day).beginning_of_day.to_i.to_s, - until: current_time.end_of_day.to_i.to_s, - id: user.id.to_s - } - end - - before do - travel_to current_time - - # Add the same user to both accounts - create(:account_user, account: account, user: user) - create(:account_user, account: account2, user: user) - - # Create conversations in account1 - conversation1 = create(:conversation, account: account, inbox: inbox, assignee: user) - conversation2 = create(:conversation, account: account, inbox: inbox, assignee: user) - - # Create conversations in account2 - conversation3 = create(:conversation, account: account2, inbox: inbox2, assignee: user) - conversation4 = create(:conversation, account: account2, inbox: inbox2, assignee: user) - - # User resolves 2 conversations in account1 - create(:reporting_event, - name: 'conversation_resolved', - account: account, - user: user, - conversation: conversation1, - created_at: current_time - 12.hours) - - create(:reporting_event, - name: 'conversation_resolved', - account: account, - user: user, - conversation: conversation2, - created_at: current_time - 6.hours) - - # Same user resolves 3 conversations in account2 - these should NOT be counted for account1 - create(:reporting_event, - name: 'conversation_resolved', - account: account2, - user: user, - conversation: conversation3, - created_at: current_time - 8.hours) - - create(:reporting_event, - name: 'conversation_resolved', - account: account2, - user: user, - conversation: conversation4, - created_at: current_time - 4.hours) - - # Create another conversation in account2 for testing - conversation5 = create(:conversation, account: account2, inbox: inbox2, assignee: user) - create(:reporting_event, - name: 'conversation_resolved', - account: account2, - user: user, - conversation: conversation5, - created_at: current_time - 2.hours) - end - - describe '#aggregate_value' do - it 'returns only resolutions performed by the user in the specified account' do - # User should have 2 resolutions in account1, not 5 (total across both accounts) - expect(subject.aggregate_value).to eq(2) - end - - context 'when querying account2' do - subject { described_class.new(account2, params) } - - it 'returns only resolutions for account2' do - # User should have 3 resolutions in account2 - expect(subject.aggregate_value).to eq(3) - end - end - end - - describe '#timeseries' do - it 'filters resolutions by account' do - result = subject.timeseries - # Should only count the 2 resolutions from account1 - total_count = result.sum { |r| r[:value] } - expect(total_count).to eq(2) - end - end - - describe 'account isolation' do - it 'does not leak data between accounts' do - # If account isolation works correctly, the counts should be different - account1_count = described_class.new(account, params).aggregate_value - account2_count = described_class.new(account2, params).aggregate_value - - expect(account1_count).to eq(2) - expect(account2_count).to eq(3) - end - end -end diff --git a/spec/builders/v2/reports/timeseries/report_builder_spec.rb b/spec/builders/v2/reports/timeseries/report_builder_spec.rb new file mode 100644 index 000000000..29540a924 --- /dev/null +++ b/spec/builders/v2/reports/timeseries/report_builder_spec.rb @@ -0,0 +1,470 @@ +require 'rails_helper' + +describe V2::Reports::Timeseries::ReportBuilder do + describe 'average metrics' do + subject { described_class.new(account, params) } + + let(:account) { create(:account) } + let(:team) { create(:team, account: account) } + let(:inbox) { create(:inbox, account: account) } + let(:label) { create(:label, title: 'spec-billing', account: account) } + let!(:conversation) { create(:conversation, account: account, inbox: inbox, team: team) } + let(:current_time) { '26.10.2020 10:00'.to_datetime } + + let(:params) do + { + type: filter_type, + business_hours: business_hours, + timezone: timezone, + timezone_offset: timezone_offset, + group_by: group_by, + metric: metric, + since: (current_time - 1.week).beginning_of_day.to_i.to_s, + until: current_time.end_of_day.to_i.to_s, + id: filter_id + } + end + let(:timezone) { nil } + let(:timezone_offset) { nil } + let(:group_by) { 'day' } + let(:metric) { 'avg_first_response_time' } + let(:business_hours) { false } + let(:filter_type) { :account } + let(:filter_id) { '' } + + before do + travel_to current_time + conversation.label_list.add(label.title) + conversation.save! + create(:reporting_event, name: 'first_response', value: 80, value_in_business_hours: 10, account: account, created_at: Time.zone.now, + conversation: conversation, inbox: inbox) + create(:reporting_event, name: 'first_response', value: 100, value_in_business_hours: 20, account: account, created_at: 1.hour.ago) + create(:reporting_event, name: 'first_response', value: 93, value_in_business_hours: 30, account: account, created_at: 1.week.ago) + end + + describe '#timeseries' do + it 'returns the correct values' do + timeseries_values = subject.timeseries + + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: 1_603_065_600, value: 93.0 }, + { count: 0, timestamp: 1_603_152_000, value: 0 }, + { count: 0, timestamp: 1_603_238_400, value: 0 }, + { count: 0, timestamp: 1_603_324_800, value: 0 }, + { count: 0, timestamp: 1_603_411_200, value: 0 }, + { count: 0, timestamp: 1_603_497_600, value: 0 }, + { count: 0, timestamp: 1_603_584_000, value: 0 }, + { count: 2, timestamp: 1_603_670_400, value: 90.0 } + ] + ) + end + + context 'when business hours is provided' do + let(:business_hours) { true } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: 1_603_065_600, value: 30.0 }, + { count: 0, timestamp: 1_603_152_000, value: 0 }, + { count: 0, timestamp: 1_603_238_400, value: 0 }, + { count: 0, timestamp: 1_603_324_800, value: 0 }, + { count: 0, timestamp: 1_603_411_200, value: 0 }, + { count: 0, timestamp: 1_603_497_600, value: 0 }, + { count: 0, timestamp: 1_603_584_000, value: 0 }, + { count: 2, timestamp: 1_603_670_400, value: 15.0 } + ] + ) + end + end + + context 'when rollups are enabled' do + let(:timezone_offset) { '5.5' } + + before do + account.update!(reporting_timezone: 'Chennai') + allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) + + create(:reporting_events_rollup, + account: account, + date: (current_time - 1.week).to_date, + dimension_type: 'account', + dimension_id: account.id, + metric: 'first_response', + count: 1, + sum_value: 93.0, + sum_value_business_hours: 30.0) + + create(:reporting_events_rollup, + account: account, + date: current_time.to_date, + dimension_type: 'account', + dimension_id: account.id, + metric: 'first_response', + count: 2, + sum_value: 180.0, + sum_value_business_hours: 30.0) + end + + it 'preserves empty buckets in the timeseries' do + rollup_timezone = ActiveSupport::TimeZone['Chennai'] + rollup_start_date = DateTime.strptime(params[:since], '%s').in_time_zone(rollup_timezone).to_date + rollup_end_date = (DateTime.strptime(params[:until], '%s') - 1.second).in_time_zone(rollup_timezone).to_date + rollup_dates = rollup_start_date..rollup_end_date + + expected_timeseries = rollup_dates.map do |date| + value = if date == (current_time - 1.week).to_date + 93.0 + elsif date == current_time.to_date + 90.0 + else + 0 + end + + count = if date == (current_time - 1.week).to_date + 1 + elsif date == current_time.to_date + 2 + else + 0 + end + + { count: count, timestamp: date.in_time_zone('Chennai').to_i, value: value } + end + + expect(subject.timeseries).to eq(expected_timeseries) + end + end + + context 'when group_by is provided' do + let(:group_by) { 'week' } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: (current_time - 1.week).beginning_of_week(:sunday).to_i, value: 93.0 }, + { count: 2, timestamp: current_time.beginning_of_week(:sunday).to_i, value: 90.0 } + ] + ) + end + end + + context 'when timezone offset is provided' do + let(:timezone_offset) { '5.5' } + let(:group_by) { 'week' } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + expect(timeseries_values).to eq( + [ + { count: 1, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 93.0 }, + { count: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 90.0 } + ] + ) + end + end + + context 'when timezone is provided' do + let(:timezone) { 'Asia/Kolkata' } + let(:timezone_offset) { '0' } + let(:group_by) { 'week' } + + it 'uses the timezone name instead of the offset for timestamps' do + expect(subject.timeseries).to eq( + [ + { count: 1, timestamp: (current_time - 1.week).in_time_zone('Asia/Kolkata').beginning_of_week(:sunday).to_i, value: 93.0 }, + { count: 2, timestamp: current_time.in_time_zone('Asia/Kolkata').beginning_of_week(:sunday).to_i, value: 90.0 } + ] + ) + end + end + + context 'when weekly rollups are enabled' do + let(:group_by) { 'week' } + let(:timezone_offset) { '5.5' } + + before do + account.update!(reporting_timezone: 'Chennai') + allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) + + create(:reporting_events_rollup, + account: account, + date: current_time.to_date - 1.day, + dimension_type: 'account', + dimension_id: account.id, + metric: 'first_response', + count: 1, + sum_value: 80.0, + sum_value_business_hours: 10.0) + + create(:reporting_events_rollup, + account: account, + date: current_time.to_date, + dimension_type: 'account', + dimension_id: account.id, + metric: 'first_response', + count: 1, + sum_value: 100.0, + sum_value_business_hours: 20.0) + end + + it 'groups weeks using sunday boundaries' do + expect(subject.timeseries).to eq( + [ + { count: 0, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 0 }, + { count: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 90.0 } + ] + ) + end + end + + context 'when the label filter is applied' do + let(:group_by) { 'week' } + let(:filter_type) { 'label' } + let(:filter_id) { label.id } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + start_of_the_week = current_time.beginning_of_week(:sunday).to_i + last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i + expect(timeseries_values).to eq( + [ + { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, + { count: 1, timestamp: start_of_the_week, value: 80.0 } + ] + ) + end + end + + context 'when the inbox filter is applied' do + let(:group_by) { 'week' } + let(:filter_type) { 'inbox' } + let(:filter_id) { inbox.id } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + start_of_the_week = current_time.beginning_of_week(:sunday).to_i + last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i + expect(timeseries_values).to eq( + [ + { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, + { count: 1, timestamp: start_of_the_week, value: 80.0 } + ] + ) + end + end + + context 'when the team filter is applied' do + let(:group_by) { 'week' } + let(:filter_type) { 'team' } + let(:filter_id) { team.id } + + it 'returns correct timeseries' do + timeseries_values = subject.timeseries + start_of_the_week = current_time.beginning_of_week(:sunday).to_i + last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i + expect(timeseries_values).to eq( + [ + { count: 0, timestamp: last_week_start_of_the_week, value: 0 }, + { count: 1, timestamp: start_of_the_week, value: 80.0 } + ] + ) + end + end + end + + describe '#aggregate_value' do + context 'when there is no filter applied' do + it 'returns the correct average value' do + expect(subject.aggregate_value).to eq 91.0 + end + end + + context 'when rollups are enabled and the agent does not exist' do + let(:filter_type) { :agent } + let(:filter_id) { '999999' } + let(:timezone_offset) { '0' } + + before do + account.update!(reporting_timezone: 'Etc/UTC') + allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) + end + + it 'raises record not found to preserve raw path behavior' do + expect { subject.aggregate_value }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + end + + describe 'count metrics' do + subject { described_class.new(account, params) } + + let(:account) { create(:account) } + let(:account2) { create(:account) } + let(:user) { create(:user, email: 'agent1@example.com') } + let(:inbox) { create(:inbox, account: account) } + let(:inbox2) { create(:inbox, account: account2) } + let(:current_time) { Time.current } + + let(:params) do + { + type: 'agent', + metric: 'resolutions_count', + since: since_time.beginning_of_day.to_i.to_s, + until: current_time.end_of_day.to_i.to_s, + timezone: timezone, + timezone_offset: timezone_offset, + group_by: group_by, + id: user.id.to_s + } + end + let(:group_by) { 'day' } + let(:since_time) { current_time - 1.day } + let(:timezone) { nil } + let(:timezone_offset) { nil } + + before do + travel_to current_time + + create(:account_user, account: account, user: user) + create(:account_user, account: account2, user: user) + + conversation1 = create(:conversation, account: account, inbox: inbox, assignee: user) + conversation2 = create(:conversation, account: account, inbox: inbox, assignee: user) + + conversation3 = create(:conversation, account: account2, inbox: inbox2, assignee: user) + conversation4 = create(:conversation, account: account2, inbox: inbox2, assignee: user) + + create(:reporting_event, + name: 'conversation_resolved', + account: account, + user: user, + conversation: conversation1, + created_at: current_time - 12.hours) + + create(:reporting_event, + name: 'conversation_resolved', + account: account, + user: user, + conversation: conversation2, + created_at: current_time - 6.hours) + + create(:reporting_event, + name: 'conversation_resolved', + account: account2, + user: user, + conversation: conversation3, + created_at: current_time - 8.hours) + + create(:reporting_event, + name: 'conversation_resolved', + account: account2, + user: user, + conversation: conversation4, + created_at: current_time - 4.hours) + + conversation5 = create(:conversation, account: account2, inbox: inbox2, assignee: user) + create(:reporting_event, + name: 'conversation_resolved', + account: account2, + user: user, + conversation: conversation5, + created_at: current_time - 2.hours) + end + + describe '#aggregate_value' do + it 'returns only resolutions performed by the user in the specified account' do + expect(subject.aggregate_value).to eq(2) + end + + context 'when rollups are enabled and the agent does not exist' do + let(:timezone_offset) { '0' } + + let(:params) do + super().merge(id: '999999') + end + + before do + account.update!(reporting_timezone: 'Etc/UTC') + allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) + end + + it 'raises record not found to preserve raw path behavior' do + expect { subject.aggregate_value }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'when querying account2' do + subject { described_class.new(account2, params) } + + it 'returns only resolutions for account2' do + expect(subject.aggregate_value).to eq(3) + end + end + end + + describe '#timeseries' do + it 'filters resolutions by account' do + result = subject.timeseries + total_count = result.sum { |row| row[:value] } + expect(total_count).to eq(2) + end + + context 'when rollups are enabled and grouped by week' do + let(:group_by) { 'week' } + let(:current_time) { Time.zone.parse('2020-10-26 10:00:00 UTC') } + let(:since_time) { current_time - 1.week } + let(:timezone_offset) { '5.5' } + + before do + account.update!(reporting_timezone: 'Chennai') + allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) + + create(:reporting_events_rollup, + account: account, + date: current_time.to_date - 1.day, + dimension_type: 'agent', + dimension_id: user.id, + metric: 'resolutions_count', + count: 1, + sum_value: 0.0, + sum_value_business_hours: 0.0) + + create(:reporting_events_rollup, + account: account, + date: current_time.to_date, + dimension_type: 'agent', + dimension_id: user.id, + metric: 'resolutions_count', + count: 1, + sum_value: 0.0, + sum_value_business_hours: 0.0) + end + + it 'groups weeks using sunday boundaries' do + expect(subject.timeseries).to eq( + [ + { value: 0, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i }, + { value: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i } + ] + ) + end + end + end + + describe 'account isolation' do + it 'does not leak data between accounts' do + account1_count = described_class.new(account, params).aggregate_value + account2_count = described_class.new(account2, params).aggregate_value + + expect(account1_count).to eq(2) + expect(account2_count).to eq(3) + end + end + end +end