diff --git a/app/builders/v2/reports/agent_summary_builder.rb b/app/builders/v2/reports/agent_summary_builder.rb new file mode 100644 index 000000000..382c67bf5 --- /dev/null +++ b/app/builders/v2/reports/agent_summary_builder.rb @@ -0,0 +1,53 @@ +class V2::Reports::AgentSummaryBuilder < V2::Reports::BaseSummaryBuilder + pattr_initialize [:account!, :params!] + + def build + set_grouped_conversations_count + set_grouped_avg_reply_time + set_grouped_avg_first_response_time + set_grouped_avg_resolution_time + prepare_report + end + + private + + def set_grouped_conversations_count + @grouped_conversations_count = Current.account.conversations.where(created_at: range).group('assignee_id').count + end + + def set_grouped_avg_resolution_time + @grouped_avg_resolution_time = get_grouped_average(reporting_events.where(name: 'conversation_resolved')) + end + + def set_grouped_avg_first_response_time + @grouped_avg_first_response_time = get_grouped_average(reporting_events.where(name: 'first_response')) + end + + def set_grouped_avg_reply_time + @grouped_avg_reply_time = get_grouped_average(reporting_events.where(name: 'reply_time')) + end + + def group_by_key + :user_id + end + + def reporting_events + @reporting_events ||= Current.account.reporting_events.where(created_at: range) + end + + def prepare_report + account.account_users.each_with_object([]) do |account_user, arr| + arr << { + id: account_user.user_id, + conversations_count: @grouped_conversations_count[account_user.user_id], + avg_resolution_time: @grouped_avg_resolution_time[account_user.user_id], + avg_first_response_time: @grouped_avg_first_response_time[account_user.user_id], + avg_reply_time: @grouped_avg_reply_time[account_user.user_id] + } + end + end + + def average_value_key + ActiveModel::Type::Boolean.new.cast(params[:business_hours]).present? ? :value_in_business_hours : :value + end +end diff --git a/app/builders/v2/reports/base_summary_builder.rb b/app/builders/v2/reports/base_summary_builder.rb new file mode 100644 index 000000000..33fe5e400 --- /dev/null +++ b/app/builders/v2/reports/base_summary_builder.rb @@ -0,0 +1,17 @@ +class V2::Reports::BaseSummaryBuilder + include DateRangeHelper + + private + + def group_by_key + # Override this method + end + + def get_grouped_average(events) + events.group(group_by_key).average(average_value_key) + end + + def average_value_key + params[:business_hours].present? ? :value_in_business_hours : :value + end +end diff --git a/app/builders/v2/reports/team_summary_builder.rb b/app/builders/v2/reports/team_summary_builder.rb new file mode 100644 index 000000000..3d7e765b3 --- /dev/null +++ b/app/builders/v2/reports/team_summary_builder.rb @@ -0,0 +1,49 @@ +class V2::Reports::TeamSummaryBuilder < V2::Reports::BaseSummaryBuilder + pattr_initialize [:account!, :params!] + + def build + set_grouped_conversations_count + set_grouped_avg_reply_time + set_grouped_avg_first_response_time + set_grouped_avg_resolution_time + prepare_report + end + + private + + def set_grouped_conversations_count + @grouped_conversations_count = Current.account.conversations.where(created_at: range).group('team_id').count + end + + def set_grouped_avg_resolution_time + @grouped_avg_resolution_time = get_grouped_average(reporting_events.where(name: 'conversation_resolved')) + end + + def set_grouped_avg_first_response_time + @grouped_avg_first_response_time = get_grouped_average(reporting_events.where(name: 'first_response')) + end + + def set_grouped_avg_reply_time + @grouped_avg_reply_time = get_grouped_average(reporting_events.where(name: 'reply_time')) + end + + def reporting_events + @reporting_events ||= Current.account.reporting_events.where(created_at: range).joins(:conversation) + end + + def group_by_key + 'conversations.team_id' + end + + def prepare_report + account.teams.each_with_object([]) do |team, arr| + arr << { + id: team.id, + conversations_count: @grouped_conversations_count[team.id], + avg_resolution_time: @grouped_avg_resolution_time[team.id], + avg_first_response_time: @grouped_avg_first_response_time[team.id], + avg_reply_time: @grouped_avg_reply_time[team.id] + } + end + end +end diff --git a/app/controllers/api/v1/accounts/agents_controller.rb b/app/controllers/api/v1/accounts/agents_controller.rb index 768d1a3ff..221c96b85 100644 --- a/app/controllers/api/v1/accounts/agents_controller.rb +++ b/app/controllers/api/v1/accounts/agents_controller.rb @@ -43,7 +43,11 @@ class Api::V1::Accounts::AgentsController < Api::V1::Accounts::BaseController inviter: current_user, account: Current.account ) - builder.perform + begin + builder.perform + rescue ActiveRecord::RecordInvalid => e + Rails.logger.info "[Agent#bulk_create] ignoring email #{email}, errors: #{e.record.errors}" + end end head :ok end diff --git a/app/controllers/api/v2/accounts/summary_reports_controller.rb b/app/controllers/api/v2/accounts/summary_reports_controller.rb new file mode 100644 index 000000000..0cbd6dd8e --- /dev/null +++ b/app/controllers/api/v2/accounts/summary_reports_controller.rb @@ -0,0 +1,36 @@ +class Api::V2::Accounts::SummaryReportsController < Api::V1::Accounts::BaseController + before_action :check_authorization + before_action :prepare_builder_params, only: [:agent, :team] + + def agent + render_report_with(V2::Reports::AgentSummaryBuilder) + end + + def team + render_report_with(V2::Reports::TeamSummaryBuilder) + end + + private + + def check_authorization + authorize :report, :view? + end + + def prepare_builder_params + @builder_params = { + since: permitted_params[:since], + until: permitted_params[:until], + business_hours: ActiveModel::Type::Boolean.new.cast(permitted_params[:business_hours]) + } + end + + def render_report_with(builder_class) + builder = builder_class.new(account: Current.account, params: @builder_params) + data = builder.build + render json: data + end + + def permitted_params + params.permit(:since, :until, :business_hours) + end +end 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/mailboxes/mailbox_helper.rb b/app/mailboxes/mailbox_helper.rb index 5d1033d20..6b3afc133 100644 --- a/app/mailboxes/mailbox_helper.rb +++ b/app/mailboxes/mailbox_helper.rb @@ -2,6 +2,7 @@ module MailboxHelper private def create_message + Rails.logger.info "[MailboxHelper] Creating message #{processed_mail.message_id}" return if @conversation.messages.find_by(source_id: processed_mail.message_id).present? @message = @conversation.messages.create!( @@ -36,6 +37,7 @@ module MailboxHelper end def process_regular_attachments(attachments) + Rails.logger.info "[MailboxHelper] Processing regular attachments for message with ID: #{processed_mail.message_id}" attachments.each do |mail_attachment| attachment = @message.attachments.new( account_id: @conversation.account_id, @@ -46,6 +48,8 @@ module MailboxHelper end def process_inline_attachments(attachments) + Rails.logger.info "[MailboxHelper] Processing inline attachments for message with ID: #{processed_mail.message_id}" + # create an instance variable here, the `embed_inline_image_source` # updates them directly. And then the value is eventaully used to update the message content @html_content = processed_mail.serialized_data[:html_content][:full] @@ -98,7 +102,9 @@ module MailboxHelper } } ).perform + @contact = @contact_inbox.contact + Rails.logger.info "[MailboxHelper] Contact created with ID: #{@contact.id} for inbox with ID: #{@inbox.id}" end def notification_email_from_chatwoot? diff --git a/app/policies/report_policy.rb b/app/policies/report_policy.rb new file mode 100644 index 000000000..7e7eeffdf --- /dev/null +++ b/app/policies/report_policy.rb @@ -0,0 +1,5 @@ +class ReportPolicy < ApplicationPolicy + def view? + @account_user.administrator? + 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/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 99974f4f7..440fc43cb 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -148,6 +148,12 @@ class Rack::Attack match_data[:account_id] if match_data.present? end + ## Prevent abuse of contact search api + throttle('/api/v1/accounts/:account_id/contacts/search', limit: 5, period: 1.minute) do |req| + match_data = %r{/api/v1/accounts/(?\d+)/contacts/search}.match(req.path) + match_data[:account_id] if match_data.present? + end + ## ----------------------------------------------- ## end 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/config/routes.rb b/config/routes.rb index 210d5bc0e..8c4d4124b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -294,6 +294,12 @@ Rails.application.routes.draw do namespace :v2 do resources :accounts, only: [:create] do scope module: :accounts do + resources :summary_reports, only: [] do + collection do + get :agent + get :team + end + end resources :reports, only: [:index] do collection do get :summary diff --git a/spec/controllers/api/v1/accounts/agents_controller_spec.rb b/spec/controllers/api/v1/accounts/agents_controller_spec.rb index 10693443a..82d5fe9c6 100644 --- a/spec/controllers/api/v1/accounts/agents_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/agents_controller_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Agents API', type: :request do let(:account) { create(:account) } let!(:admin) { create(:user, custom_attributes: { test: 'test' }, account: account, role: :administrator) } - let!(:agent) { create(:user, account: account, role: :agent) } + let!(:agent) { create(:user, account: account, email: 'exists@example.com', role: :agent) } describe 'GET /api/v1/accounts/{account.id}/agents' do context 'when it is an unauthenticated user' do @@ -196,6 +196,17 @@ RSpec.describe 'Agents API', type: :request do expect(response).to have_http_status(:ok) end + + it 'ignores errors if account_user already exists' do + params = { emails: ['exists@example.com', 'test1@example.com', 'test2@example.com'] } + + expect do + post "/api/v1/accounts/#{account.id}/agents/bulk_create", params: params, + headers: admin.create_new_auth_token + end.to change(User, :count).by(2) + + expect(response).to have_http_status(:ok) + end end end end diff --git a/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb b/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb new file mode 100644 index 000000000..23ed3e65c --- /dev/null +++ b/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb @@ -0,0 +1,112 @@ +require 'rails_helper' + +RSpec.describe 'Summary Reports API', type: :request do + let(:account) { create(:account) } + let(:admin) { create(:user, account: account, role: :administrator) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:default_timezone) { ActiveSupport::TimeZone[0]&.name } + 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 } + + describe 'GET /api/v2/accounts/:account_id/summary_reports/agent' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v2/accounts/#{account.id}/summary_reports/agent" + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + let(:params) do + { + since: start_of_today.to_s, + until: end_of_today.to_s, + business_hours: true + } + end + + it 'returns unauthorized for agents' do + get "/api/v2/accounts/#{account.id}/summary_reports/agent", + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'calls V2::Reports::AgentSummaryBuilder with the right params if the user is an admin' do + agent_summary_builder = double + allow(V2::Reports::AgentSummaryBuilder).to receive(:new).and_return(agent_summary_builder) + allow(agent_summary_builder).to receive(:build).and_return([{ id: 1, conversations_count: 110 }]) + + get "/api/v2/accounts/#{account.id}/summary_reports/agent", + params: params, + headers: admin.create_new_auth_token, + as: :json + + expect(V2::Reports::AgentSummaryBuilder).to have_received(:new).with(account: account, params: params) + expect(agent_summary_builder).to have_received(:build) + + expect(response).to have_http_status(:success) + json_response = response.parsed_body + + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(1) + expect(json_response.first['conversations_count']).to eq(110) + expect(json_response.first['avg_reply_time']).to be_nil + end + end + end + + describe 'GET /api/v2/accounts/:account_id/summary_reports/team' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v2/accounts/#{account.id}/summary_reports/team" + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + let(:params) do + { + since: start_of_today.to_s, + until: end_of_today.to_s, + business_hours: true + } + end + + it 'returns unauthorized for agents' do + get "/api/v2/accounts/#{account.id}/summary_reports/team", + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'calls V2::Reports::TeamSummaryBuilder with the right params if the user is an admin' do + team_summary_builder = double + allow(V2::Reports::TeamSummaryBuilder).to receive(:new).and_return(team_summary_builder) + allow(team_summary_builder).to receive(:build).and_return([{ id: 1, conversations_count: 110 }]) + + get "/api/v2/accounts/#{account.id}/summary_reports/team", + params: params, + headers: admin.create_new_auth_token, + as: :json + + expect(V2::Reports::TeamSummaryBuilder).to have_received(:new).with(account: account, params: params) + expect(team_summary_builder).to have_received(:build) + + expect(response).to have_http_status(:success) + json_response = response.parsed_body + + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(1) + expect(json_response.first['conversations_count']).to eq(110) + expect(json_response.first['avg_reply_time']).to be_nil + end + end + end +end 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