From c607f09be0102e11bf2c37b01744ca744033401c Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Mon, 12 Feb 2024 20:01:58 -0800 Subject: [PATCH] feat: Improve the rendering of CSV exports (#8914) Improve the rendering of the datestamp --- app/helpers/api/v2/accounts/reports_helper.rb | 8 +- .../reports/time_format_presenter.rb | 33 +++++++++ .../api/v2/accounts/reports/agents.csv.erb | 3 +- .../accounts/reports/conversation_traffic.erb | 1 + .../api/v2/accounts/reports/inboxes.csv.erb | 3 +- .../api/v2/accounts/reports/labels.csv.erb | 3 +- .../api/v2/accounts/reports/teams.csv.erb | 3 +- config/locales/en.yml | 31 +++++--- .../reports/time_format_presenter_spec.rb | 73 +++++++++++++++++++ 9 files changed, 139 insertions(+), 19 deletions(-) create mode 100644 app/presenters/reports/time_format_presenter.rb create mode 100644 spec/presenters/reports/time_format_presenter_spec.rb diff --git a/app/helpers/api/v2/accounts/reports_helper.rb b/app/helpers/api/v2/accounts/reports_helper.rb index 3d87154fd..74c7af43e 100644 --- a/app/helpers/api/v2/accounts/reports_helper.rb +++ b/app/helpers/api/v2/accounts/reports_helper.rb @@ -45,12 +45,8 @@ module Api::V2::Accounts::ReportsHelper def generate_readable_report_metrics(report_metric) [ report_metric[:conversations_count], - time_to_minutes(report_metric[:avg_first_response_time]), - time_to_minutes(report_metric[:avg_resolution_time]) + Reports::TimeFormatPresenter.new(report_metric[:avg_first_response_time]).format, + Reports::TimeFormatPresenter.new(report_metric[:avg_resolution_time]).format ] end - - def time_to_minutes(time_in_seconds) - (time_in_seconds / 60).to_i - end end diff --git a/app/presenters/reports/time_format_presenter.rb b/app/presenters/reports/time_format_presenter.rb new file mode 100644 index 000000000..b69618499 --- /dev/null +++ b/app/presenters/reports/time_format_presenter.rb @@ -0,0 +1,33 @@ +class Reports::TimeFormatPresenter + include ActionView::Helpers::TextHelper + + attr_reader :seconds + + def initialize(seconds) + @seconds = seconds.to_i + end + + def format + return '--' if seconds.nil? || seconds.zero? + + days, remainder = seconds.divmod(86_400) + hours, remainder = remainder.divmod(3600) + minutes, seconds = remainder.divmod(60) + + format_components(days: days, hours: hours, minutes: minutes, seconds: seconds) + end + + private + + def format_components(components) + formatted_components = components.filter_map do |unit, value| + next if value.zero? + + I18n.t("time_units.#{unit}", count: value) + end + + return I18n.t('time_units.seconds', count: 0) if formatted_components.empty? + + formatted_components.first(2).join(' ') + end +end diff --git a/app/views/api/v2/accounts/reports/agents.csv.erb b/app/views/api/v2/accounts/reports/agents.csv.erb index 99dbacd38..dfc90f016 100644 --- a/app/views/api/v2/accounts/reports/agents.csv.erb +++ b/app/views/api/v2/accounts/reports/agents.csv.erb @@ -1,3 +1,5 @@ +<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> + <% headers = [ I18n.t('reports.agent_csv.agent_name'), I18n.t('reports.agent_csv.conversations_count'), @@ -9,4 +11,3 @@ <% @report_data.each do |row| %> <%= CSVSafe.generate_line row -%> <% end %> -<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> diff --git a/app/views/api/v2/accounts/reports/conversation_traffic.erb b/app/views/api/v2/accounts/reports/conversation_traffic.erb index 6616101f0..d0cb245f4 100644 --- a/app/views/api/v2/accounts/reports/conversation_traffic.erb +++ b/app/views/api/v2/accounts/reports/conversation_traffic.erb @@ -1,4 +1,5 @@ <%= CSV.generate_line [I18n.t('reports.conversation_traffic_csv.timezone'), @timezone] %> + <% @report_data.each do |row| %> <%= CSVSafe.generate_line row -%> <% end %> diff --git a/app/views/api/v2/accounts/reports/inboxes.csv.erb b/app/views/api/v2/accounts/reports/inboxes.csv.erb index e6466cf01..df038d5fe 100644 --- a/app/views/api/v2/accounts/reports/inboxes.csv.erb +++ b/app/views/api/v2/accounts/reports/inboxes.csv.erb @@ -1,3 +1,5 @@ +<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> + <% headers = [ I18n.t('reports.inbox_csv.inbox_name'), I18n.t('reports.inbox_csv.inbox_type'), @@ -10,4 +12,3 @@ <% @report_data.each do |row| %> <%= CSVSafe.generate_line row -%> <% end %> -<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> diff --git a/app/views/api/v2/accounts/reports/labels.csv.erb b/app/views/api/v2/accounts/reports/labels.csv.erb index 5da742790..fdf578415 100644 --- a/app/views/api/v2/accounts/reports/labels.csv.erb +++ b/app/views/api/v2/accounts/reports/labels.csv.erb @@ -1,3 +1,5 @@ +<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> + <% headers = [ I18n.t('reports.label_csv.label_title'), I18n.t('reports.label_csv.conversations_count'), @@ -9,4 +11,3 @@ <% @report_data.each do |row| %> <%= CSVSafe.generate_line row -%> <% end %> -<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> diff --git a/app/views/api/v2/accounts/reports/teams.csv.erb b/app/views/api/v2/accounts/reports/teams.csv.erb index d9ae31dfa..1c2e6084a 100644 --- a/app/views/api/v2/accounts/reports/teams.csv.erb +++ b/app/views/api/v2/accounts/reports/teams.csv.erb @@ -1,3 +1,5 @@ +<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> + <% headers = [ I18n.t('reports.team_csv.team_name'), I18n.t('reports.team_csv.conversations_count'), @@ -9,4 +11,3 @@ <% @report_data.each do |row| %> <%= CSVSafe.generate_line row -%> <% end %> -<%= CSVSafe.generate_line [I18n.t('reports.period', since: Date.strptime(params[:since], '%s'), until: Date.strptime(params[:until], '%s'))] %> diff --git a/config/locales/en.yml b/config/locales/en.yml index da4f35032..78f1f9490 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -83,25 +83,25 @@ en: utc_warning: The report generated is in UTC timezone agent_csv: agent_name: Agent name - conversations_count: Conversations count - avg_first_response_time: Avg first response time (Minutes) - avg_resolution_time: Avg resolution time (Minutes) + conversations_count: Assigned conversations + avg_first_response_time: Avg first response time + avg_resolution_time: Avg resolution time inbox_csv: inbox_name: Inbox name inbox_type: Inbox type conversations_count: No. of conversations - avg_first_response_time: Avg first response time (Minutes) - avg_resolution_time: Avg resolution time (Minutes) + avg_first_response_time: Avg first response time + avg_resolution_time: Avg resolution time label_csv: label_title: Label conversations_count: No. of conversations - avg_first_response_time: Avg first response time (Minutes) - avg_resolution_time: Avg resolution time (Minutes) + avg_first_response_time: Avg first response time + avg_resolution_time: Avg resolution time team_csv: team_name: Team name conversations_count: Conversations count - avg_first_response_time: Avg first response time (Minutes) - avg_resolution_time: Avg resolution time (Minutes) + avg_first_response_time: Avg first response time + avg_resolution_time: Avg resolution time conversation_traffic_csv: timezone: Timezone default_group_by: day @@ -245,3 +245,16 @@ en: inbox_name: Inbox inbox_type: Inbox Type button: Open conversation + time_units: + days: + one: "%{count} day" + other: "%{count} days" + hours: + one: "%{count} hour" + other: "%{count} hours" + minutes: + one: "%{count} minute" + other: "%{count} minutes" + seconds: + one: "%{count} second" + other: "%{count} seconds" diff --git a/spec/presenters/reports/time_format_presenter_spec.rb b/spec/presenters/reports/time_format_presenter_spec.rb new file mode 100644 index 000000000..64d75bb08 --- /dev/null +++ b/spec/presenters/reports/time_format_presenter_spec.rb @@ -0,0 +1,73 @@ +require 'rails_helper' + +RSpec.describe Reports::TimeFormatPresenter do + describe '#format' do + context 'when formatting days' do + it 'formats single day correctly' do + expect(described_class.new(86_400).format).to eq '1 day' + end + + it 'formats multiple days correctly' do + expect(described_class.new(172_800).format).to eq '2 days' + end + + it 'includes seconds with days correctly' do + expect(described_class.new(86_401).format).to eq '1 day 1 second' + end + + it 'includes hours with days correctly' do + expect(described_class.new(93_600).format).to eq '1 day 2 hours' + end + + it 'includes minutes with days correctly' do + expect(described_class.new(86_461).format).to eq '1 day 1 minute' + end + end + + context 'when formatting hours' do + it 'formats single hour correctly' do + expect(described_class.new(3600).format).to eq '1 hour' + end + + it 'formats multiple hours correctly' do + expect(described_class.new(7200).format).to eq '2 hours' + end + + it 'includes seconds with hours correctly' do + expect(described_class.new(3601).format).to eq '1 hour 1 second' + end + + it 'includes minutes with hours correctly' do + expect(described_class.new(3660).format).to eq '1 hour 1 minute' + end + end + + context 'when formatting minutes' do + it 'formats single minute correctly' do + expect(described_class.new(60).format).to eq '1 minute' + end + + it 'formats multiple minutes correctly' do + expect(described_class.new(120).format).to eq '2 minutes' + end + + it 'includes seconds with minutes correctly' do + expect(described_class.new(62).format).to eq '1 minute 2 seconds' + end + end + + context 'when formatting seconds' do + it 'formats multiple seconds correctly' do + expect(described_class.new(56).format).to eq '56 seconds' + end + + it 'handles floating-point seconds by truncating to the nearest lower second' do + expect(described_class.new(55.2).format).to eq '55 seconds' + end + + it 'formats single second correctly' do + expect(described_class.new(1).format).to eq '1 second' + end + end + end +end