Merge branch 'develop' into feat/onboarding

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

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

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

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

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

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

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?

app/policies/report_policy.rb

@@ -0,0 +1,5 @@
+class ReportPolicy < ApplicationPolicy
+  def view?
+    @account_user.administrator?
+  end
+end

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

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'))] %>

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

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'))] %>

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'))] %>

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'))] %>

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/(?<account_id>\d+)/contacts/search}.match(req.path)
+    match_data[:account_id] if match_data.present?
+  end
+
   ## ----------------------------------------------- ##
 end
 

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"

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

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

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

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