chore: Improve search, list performance of contact/conversation APIs (#2696)
This commit is contained in:
@@ -11,6 +11,7 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
|
||||
before_action :check_authorization
|
||||
before_action :set_current_page, only: [:index, :active, :search]
|
||||
before_action :fetch_contact, only: [:show, :update, :contactable_inboxes]
|
||||
before_action :set_include_contact_inboxes, only: [:index, :search]
|
||||
|
||||
def index
|
||||
@contacts_count = resolved_contacts.count
|
||||
@@ -87,11 +88,15 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
|
||||
end
|
||||
|
||||
def fetch_contacts_with_conversation_count(contacts)
|
||||
filtrate(contacts).left_outer_joins(:conversations)
|
||||
.select('contacts.*, COUNT(conversations.id) as conversations_count')
|
||||
.group('contacts.id')
|
||||
.includes([{ avatar_attachment: [:blob] }, { contact_inboxes: [:inbox] }])
|
||||
.page(@current_page).per(RESULTS_PER_PAGE)
|
||||
contacts_with_conversation_count = filtrate(contacts).left_outer_joins(:conversations)
|
||||
.select('contacts.*, COUNT(conversations.id) as conversations_count')
|
||||
.group('contacts.id')
|
||||
.includes([{ avatar_attachment: [:blob] }])
|
||||
.page(@current_page).per(RESULTS_PER_PAGE)
|
||||
|
||||
return contacts_with_conversation_count.includes([{ contact_inboxes: [:inbox] }]) if @include_contact_inboxes
|
||||
|
||||
contacts_with_conversation_count
|
||||
end
|
||||
|
||||
def build_contact_inbox
|
||||
@@ -117,6 +122,14 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
|
||||
contact_params.except(:custom_attributes).merge({ custom_attributes: contact_custom_attributes })
|
||||
end
|
||||
|
||||
def set_include_contact_inboxes
|
||||
@include_contact_inboxes = if params[:include_contact_inboxes].present?
|
||||
params[:include_contact_inboxes] == 'true'
|
||||
else
|
||||
true
|
||||
end
|
||||
end
|
||||
|
||||
def fetch_contact
|
||||
@contact = Current.account.contacts.includes(contact_inboxes: [:inbox]).find(params[:id])
|
||||
end
|
||||
|
||||
@@ -119,7 +119,7 @@ class ConversationFinder
|
||||
|
||||
def conversations
|
||||
@conversations = @conversations.includes(
|
||||
:taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }
|
||||
:taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team
|
||||
)
|
||||
current_page ? @conversations.latest.page(current_page) : @conversations.latest
|
||||
end
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
import ApiClient from './ApiClient';
|
||||
|
||||
export const buildContactParams = (page, sortAttr, label, search) => {
|
||||
let params = `page=${page}&sort=${sortAttr}`;
|
||||
let params = `include_contact_inboxes=false&page=${page}&sort=${sortAttr}`;
|
||||
if (search) {
|
||||
params = `${params}&q=${search}`;
|
||||
}
|
||||
|
||||
@@ -17,7 +17,7 @@ describe('#ContactsAPI', () => {
|
||||
it('#get', () => {
|
||||
contactAPI.get(1, 'name', 'customer-support');
|
||||
expect(context.axiosMock.get).toHaveBeenCalledWith(
|
||||
'/api/v1/contacts?page=1&sort=name&labels[]=customer-support'
|
||||
'/api/v1/contacts?include_contact_inboxes=false&page=1&sort=name&labels[]=customer-support'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -56,7 +56,7 @@ describe('#ContactsAPI', () => {
|
||||
it('#search', () => {
|
||||
contactAPI.search('leads', 1, 'date', 'customer-support');
|
||||
expect(context.axiosMock.get).toHaveBeenCalledWith(
|
||||
'/api/v1/contacts/search?page=1&sort=date&q=leads&labels[]=customer-support'
|
||||
'/api/v1/contacts/search?include_contact_inboxes=false&page=1&sort=date&q=leads&labels[]=customer-support'
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -64,12 +64,16 @@ describe('#ContactsAPI', () => {
|
||||
|
||||
describe('#buildContactParams', () => {
|
||||
it('returns correct string', () => {
|
||||
expect(buildContactParams(1, 'name', '', '')).toBe('page=1&sort=name');
|
||||
expect(buildContactParams(1, 'name', '', '')).toBe(
|
||||
'include_contact_inboxes=false&page=1&sort=name'
|
||||
);
|
||||
expect(buildContactParams(1, 'name', 'customer-support', '')).toBe(
|
||||
'page=1&sort=name&labels[]=customer-support'
|
||||
'include_contact_inboxes=false&page=1&sort=name&labels[]=customer-support'
|
||||
);
|
||||
expect(
|
||||
buildContactParams(1, 'name', 'customer-support', 'message-content')
|
||||
).toBe('page=1&sort=name&q=message-content&labels[]=customer-support');
|
||||
).toBe(
|
||||
'include_contact_inboxes=false&page=1&sort=name&q=message-content&labels[]=customer-support'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,10 +17,11 @@
|
||||
#
|
||||
# Indexes
|
||||
#
|
||||
# index_contacts_on_account_id (account_id)
|
||||
# index_contacts_on_pubsub_token (pubsub_token) UNIQUE
|
||||
# uniq_email_per_account_contact (email,account_id) UNIQUE
|
||||
# uniq_identifier_per_account_contact (identifier,account_id) UNIQUE
|
||||
# index_contacts_on_account_id (account_id)
|
||||
# index_contacts_on_phone_number_and_account_id (phone_number,account_id)
|
||||
# index_contacts_on_pubsub_token (pubsub_token) UNIQUE
|
||||
# uniq_email_per_account_contact (email,account_id) UNIQUE
|
||||
# uniq_identifier_per_account_contact (identifier,account_id) UNIQUE
|
||||
#
|
||||
|
||||
class Contact < ApplicationRecord
|
||||
|
||||
@@ -24,11 +24,13 @@
|
||||
#
|
||||
# Indexes
|
||||
#
|
||||
# index_conversations_on_account_id (account_id)
|
||||
# index_conversations_on_account_id_and_display_id (account_id,display_id) UNIQUE
|
||||
# index_conversations_on_campaign_id (campaign_id)
|
||||
# index_conversations_on_contact_inbox_id (contact_inbox_id)
|
||||
# index_conversations_on_team_id (team_id)
|
||||
# index_conversations_on_account_id (account_id)
|
||||
# index_conversations_on_account_id_and_display_id (account_id,display_id) UNIQUE
|
||||
# index_conversations_on_assignee_id_and_account_id (assignee_id,account_id)
|
||||
# index_conversations_on_campaign_id (campaign_id)
|
||||
# index_conversations_on_contact_inbox_id (contact_inbox_id)
|
||||
# index_conversations_on_status_and_account_id (status,account_id)
|
||||
# index_conversations_on_team_id (team_id)
|
||||
#
|
||||
# Foreign Keys
|
||||
#
|
||||
|
||||
@@ -5,6 +5,6 @@ end
|
||||
|
||||
json.payload do
|
||||
json.array! @contacts do |contact|
|
||||
json.partial! 'api/v1/models/contact.json.jbuilder', resource: contact, with_contact_inboxes: true
|
||||
json.partial! 'api/v1/models/contact.json.jbuilder', resource: contact, with_contact_inboxes: @include_contact_inboxes
|
||||
end
|
||||
end
|
||||
|
||||
@@ -5,6 +5,6 @@ end
|
||||
|
||||
json.payload do
|
||||
json.array! @contacts do |contact|
|
||||
json.partial! 'api/v1/models/contact.json.jbuilder', resource: contact, with_contact_inboxes: true
|
||||
json.partial! 'api/v1/models/contact.json.jbuilder', resource: contact, with_contact_inboxes: @include_contact_inboxes
|
||||
end
|
||||
end
|
||||
|
||||
@@ -2,5 +2,5 @@ json.id resource.id
|
||||
json.name resource.name
|
||||
json.description resource.description
|
||||
json.allow_auto_assign resource.allow_auto_assign
|
||||
json.account_id resource.account.id
|
||||
json.account_id resource.account_id
|
||||
json.is_member Current.user.teams.include?(resource)
|
||||
|
||||
6
db/migrate/20210723094412_add_index_to_conversations.rb
Normal file
6
db/migrate/20210723094412_add_index_to_conversations.rb
Normal file
@@ -0,0 +1,6 @@
|
||||
class AddIndexToConversations < ActiveRecord::Migration[6.0]
|
||||
def change
|
||||
add_index :conversations, [:status, :account_id]
|
||||
add_index :conversations, [:assignee_id, :account_id]
|
||||
end
|
||||
end
|
||||
5
db/migrate/20210723095657_add_index_to_contacts.rb
Normal file
5
db/migrate/20210723095657_add_index_to_contacts.rb
Normal file
@@ -0,0 +1,5 @@
|
||||
class AddIndexToContacts < ActiveRecord::Migration[6.0]
|
||||
def change
|
||||
add_index :contacts, [:phone_number, :account_id]
|
||||
end
|
||||
end
|
||||
@@ -10,7 +10,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema.define(version: 2021_07_22_095814) do
|
||||
ActiveRecord::Schema.define(version: 2021_07_23_095657) do
|
||||
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pg_stat_statements"
|
||||
@@ -246,6 +246,7 @@ ActiveRecord::Schema.define(version: 2021_07_22_095814) do
|
||||
t.index ["account_id"], name: "index_contacts_on_account_id"
|
||||
t.index ["email", "account_id"], name: "uniq_email_per_account_contact", unique: true
|
||||
t.index ["identifier", "account_id"], name: "uniq_identifier_per_account_contact", unique: true
|
||||
t.index ["phone_number", "account_id"], name: "index_contacts_on_phone_number_and_account_id"
|
||||
t.index ["pubsub_token"], name: "index_contacts_on_pubsub_token", unique: true
|
||||
end
|
||||
|
||||
@@ -270,8 +271,10 @@ ActiveRecord::Schema.define(version: 2021_07_22_095814) do
|
||||
t.datetime "snoozed_until"
|
||||
t.index ["account_id", "display_id"], name: "index_conversations_on_account_id_and_display_id", unique: true
|
||||
t.index ["account_id"], name: "index_conversations_on_account_id"
|
||||
t.index ["assignee_id", "account_id"], name: "index_conversations_on_assignee_id_and_account_id"
|
||||
t.index ["campaign_id"], name: "index_conversations_on_campaign_id"
|
||||
t.index ["contact_inbox_id"], name: "index_conversations_on_contact_inbox_id"
|
||||
t.index ["status", "account_id"], name: "index_conversations_on_status_and_account_id"
|
||||
t.index ["team_id"], name: "index_conversations_on_team_id"
|
||||
end
|
||||
|
||||
|
||||
@@ -29,6 +29,17 @@ RSpec.describe 'Contacts API', type: :request do
|
||||
expect(response_body['payload'].first['contact_inboxes'].first['inbox']['name']).to eq(contact_inbox.inbox.name)
|
||||
end
|
||||
|
||||
it 'returns all contacts without contact inboxes' do
|
||||
get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false",
|
||||
headers: admin.create_new_auth_token,
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
response_body = JSON.parse(response.body)
|
||||
expect(response_body['payload'].first['email']).to eq(contact.email)
|
||||
expect(response_body['payload'].first['contact_inboxes'].blank?).to eq(true)
|
||||
end
|
||||
|
||||
it 'returns includes conversations count and last seen at' do
|
||||
create(:conversation, contact: contact, account: account, inbox: contact_inbox.inbox, contact_last_seen_at: Time.now.utc)
|
||||
get "/api/v1/accounts/#{account.id}/contacts",
|
||||
|
||||
Reference in New Issue
Block a user