From 6b2736aa63cfc49be998910d205716f7e525fae9 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 20 Apr 2023 12:55:04 +0530 Subject: [PATCH] fix: inconsistency in report and summary for metric counts (#6817) * feat: include timezone offset in summary calculation * fix: exlcude end in date range * test: explicit end of day * fix: test for report builder * fix: reports.spec.js --------- Co-authored-by: Tejaswini Chile --- .../api/v2/accounts/reports_controller.rb | 6 +++-- app/helpers/date_range_helper.rb | 2 +- app/javascript/dashboard/api/reports.js | 1 + .../dashboard/api/specs/reports.spec.js | 6 ++++- spec/builders/v2/report_builder_spec.rb | 24 +++++++++--------- .../api/v2/accounts/report_controller_spec.rb | 25 ++++++++++--------- 6 files changed, 36 insertions(+), 28 deletions(-) diff --git a/app/controllers/api/v2/accounts/reports_controller.rb b/app/controllers/api/v2/accounts/reports_controller.rb index 8e95ea594..260400feb 100644 --- a/app/controllers/api/v2/accounts/reports_controller.rb +++ b/app/controllers/api/v2/accounts/reports_controller.rb @@ -72,14 +72,16 @@ class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController def current_summary_params common_params.merge({ since: range[:current][:since], - until: range[:current][:until] + until: range[:current][:until], + timezone_offset: params[:timezone_offset] }) end def previous_summary_params common_params.merge({ since: range[:previous][:since], - until: range[:previous][:until] + until: range[:previous][:until], + timezone_offset: params[:timezone_offset] }) end diff --git a/app/helpers/date_range_helper.rb b/app/helpers/date_range_helper.rb index 175d6c72b..090512c4f 100644 --- a/app/helpers/date_range_helper.rb +++ b/app/helpers/date_range_helper.rb @@ -7,7 +7,7 @@ module DateRangeHelper def range return if params[:since].blank? || params[:until].blank? - parse_date_time(params[:since])..parse_date_time(params[:until]) + parse_date_time(params[:since])...parse_date_time(params[:until]) end def parse_date_time(datetime) diff --git a/app/javascript/dashboard/api/reports.js b/app/javascript/dashboard/api/reports.js index 89b8554a5..7bc07b565 100644 --- a/app/javascript/dashboard/api/reports.js +++ b/app/javascript/dashboard/api/reports.js @@ -40,6 +40,7 @@ class ReportsAPI extends ApiClient { id, group_by: groupBy, business_hours: businessHours, + timezone_offset: getTimeOffset(), }, }); } diff --git a/app/javascript/dashboard/api/specs/reports.spec.js b/app/javascript/dashboard/api/specs/reports.spec.js index b59a23ad5..368997859 100644 --- a/app/javascript/dashboard/api/specs/reports.spec.js +++ b/app/javascript/dashboard/api/specs/reports.spec.js @@ -42,9 +42,13 @@ describe('#Reports API', () => { '/api/v2/reports/summary', { params: { + business_hours: undefined, + group_by: undefined, + id: undefined, since: 1621103400, - until: 1621621800, + timezone_offset: -0, type: 'account', + until: 1621621800, }, } ); diff --git a/spec/builders/v2/report_builder_spec.rb b/spec/builders/v2/report_builder_spec.rb index 714e5fda1..7c6c78179 100644 --- a/spec/builders/v2/report_builder_spec.rb +++ b/spec/builders/v2/report_builder_spec.rb @@ -57,7 +57,7 @@ describe ::V2::ReportBuilder do metric: 'conversations_count', type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -72,7 +72,7 @@ describe ::V2::ReportBuilder do metric: 'incoming_messages_count', type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -87,7 +87,7 @@ describe ::V2::ReportBuilder do metric: 'outgoing_messages_count', type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -102,7 +102,7 @@ describe ::V2::ReportBuilder do metric: 'resolutions_count', type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } conversations = account.conversations.where('created_at < ?', 1.day.ago) @@ -119,7 +119,7 @@ describe ::V2::ReportBuilder do metric: 'avg_first_response_time', type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -132,7 +132,7 @@ describe ::V2::ReportBuilder do params = { type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -149,7 +149,7 @@ describe ::V2::ReportBuilder do params = { type: :account, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s, + until: Time.zone.today.end_of_day.to_time.to_i.to_s, group_by: 'test'.to_s } @@ -165,7 +165,7 @@ describe ::V2::ReportBuilder do type: :label, id: label_2.id, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -231,7 +231,7 @@ describe ::V2::ReportBuilder do type: :label, id: label_2.id, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -244,7 +244,7 @@ describe ::V2::ReportBuilder do type: :label, id: label_2.id, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s + until: Time.zone.today.end_of_day.to_time.to_i.to_s } builder = V2::ReportBuilder.new(account, params) @@ -262,7 +262,7 @@ describe ::V2::ReportBuilder do type: :label, id: label_2.id, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s, + until: Time.zone.today.end_of_day.to_time.to_i.to_s, group_by: 'week'.to_s } @@ -281,7 +281,7 @@ describe ::V2::ReportBuilder do type: :label, id: label_2.id, since: (Time.zone.today - 3.days).to_time.to_i.to_s, - until: Time.zone.today.to_time.to_i.to_s, + until: Time.zone.today.end_of_day.to_time.to_i.to_s, group_by: 'test'.to_s } diff --git a/spec/controllers/api/v2/accounts/report_controller_spec.rb b/spec/controllers/api/v2/accounts/report_controller_spec.rb index 6087b426a..8b2c51450 100644 --- a/spec/controllers/api/v2/accounts/report_controller_spec.rb +++ b/spec/controllers/api/v2/accounts/report_controller_spec.rb @@ -8,7 +8,8 @@ RSpec.describe 'Reports API', type: :request do let!(:inbox) { create(:inbox, account: account) } let(:inbox_member) { create(:inbox_member, user: user, inbox: inbox) } let(:default_timezone) { ActiveSupport::TimeZone[0]&.name } - let(:date_timestamp) { Time.current.in_time_zone(default_timezone).beginning_of_day.to_i } + let(:start_of_today) { Time.current.in_time_zone(default_timezone).beginning_of_day.to_i } + let(:end_of_today) { Time.current.in_time_zone(default_timezone).end_of_day.to_i } let(:params) { { timezone_offset: Time.zone.utc_offset } } let(:new_account) { create(:account) } @@ -31,8 +32,8 @@ RSpec.describe 'Reports API', type: :request do super().merge( metric: 'conversations_count', type: :account, - since: date_timestamp.to_s, - until: date_timestamp.to_s + since: start_of_today.to_s, + until: end_of_today.to_s ) end @@ -54,7 +55,7 @@ RSpec.describe 'Reports API', type: :request do expect(response).to have_http_status(:success) json_response = JSON.parse(response.body) - current_day_metric = json_response.select { |x| x['timestamp'] == date_timestamp } + current_day_metric = json_response.select { |x| x['timestamp'] == start_of_today } expect(current_day_metric.length).to eq(1) expect(current_day_metric[0]['value']).to eq(10) end @@ -162,8 +163,8 @@ RSpec.describe 'Reports API', type: :request do let(:params) do super().merge( type: :account, - since: date_timestamp.to_s, - until: date_timestamp.to_s + since: start_of_today.to_s, + until: end_of_today.to_s ) end @@ -203,7 +204,7 @@ RSpec.describe 'Reports API', type: :request do let(:params) do super().merge( since: 30.days.ago.to_i.to_s, - until: date_timestamp.to_s + until: end_of_today.to_s ) end @@ -232,7 +233,7 @@ RSpec.describe 'Reports API', type: :request do super().merge( type: :agent, since: 30.days.ago.to_i.to_s, - until: date_timestamp.to_s + until: end_of_today.to_s ) end @@ -272,7 +273,7 @@ RSpec.describe 'Reports API', type: :request do let(:params) do super().merge( since: 30.days.ago.to_i.to_s, - until: date_timestamp.to_s + until: end_of_today.to_s ) end @@ -307,7 +308,7 @@ RSpec.describe 'Reports API', type: :request do let(:params) do super().merge( since: 30.days.ago.to_i.to_s, - until: date_timestamp.to_s + until: end_of_today.to_s ) end @@ -342,7 +343,7 @@ RSpec.describe 'Reports API', type: :request do let(:params) do super().merge( since: 30.days.ago.to_i.to_s, - until: date_timestamp.to_s + until: end_of_today.to_s ) end @@ -377,7 +378,7 @@ RSpec.describe 'Reports API', type: :request do let(:params) do super().merge( since: 7.days.ago.to_i.to_s, - until: date_timestamp.to_s + until: end_of_today.to_s ) end