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 <tejaswini@chatwoot.com>
This commit is contained in:
Shivam Mishra
2023-04-20 12:55:04 +05:30
committed by GitHub
parent d1584eea72
commit 6b2736aa63
6 changed files with 36 additions and 28 deletions

View File

@@ -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

View File

@@ -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)

View File

@@ -40,6 +40,7 @@ class ReportsAPI extends ApiClient {
id,
group_by: groupBy,
business_hours: businessHours,
timezone_offset: getTimeOffset(),
},
});
}

View File

@@ -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,
},
}
);

View File

@@ -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
}

View File

@@ -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