chore: Search optimisations (#6644)
- Strip search term before searching - order messages by created_at desc - order contacts by last_activity_at desc - order conversations by created_at desc - Search only resolved contacts - Optimize resolved contacts query ref: #6583
This commit is contained in:
@@ -136,9 +136,7 @@ class Contact < ApplicationRecord
|
||||
end
|
||||
|
||||
def self.resolved_contacts
|
||||
where.not(email: [nil, '']).or(
|
||||
Current.account.contacts.where.not(phone_number: [nil, ''])
|
||||
).or(Current.account.contacts.where.not(identifier: [nil, '']))
|
||||
where("COALESCE(NULLIF(contacts.email,''),NULLIF(contacts.phone_number,''),NULLIF(contacts.identifier,'')) IS NOT NULL")
|
||||
end
|
||||
|
||||
def discard_invalid_attrs
|
||||
|
||||
@@ -24,6 +24,8 @@ class InstallationConfig < ApplicationRecord
|
||||
before_validation :set_lock
|
||||
validates :name, presence: true
|
||||
|
||||
# TODO: Get rid of default scope
|
||||
# https://stackoverflow.com/a/1834250/939299
|
||||
default_scope { order(created_at: :desc) }
|
||||
scope :editable, -> { where(locked: false) }
|
||||
|
||||
|
||||
@@ -81,6 +81,10 @@ class Message < ApplicationRecord
|
||||
scope :chat, -> { where.not(message_type: :activity).where(private: false) }
|
||||
scope :non_activity_messages, -> { where.not(message_type: :activity).reorder('id desc') }
|
||||
scope :today, -> { where("date_trunc('day', created_at) = ?", Date.current) }
|
||||
|
||||
# TODO: Get rid of default scope
|
||||
# https://stackoverflow.com/a/1834250/939299
|
||||
# if you want to change order, use `reorder`
|
||||
default_scope { order(created_at: :asc) }
|
||||
|
||||
belongs_to :account
|
||||
|
||||
@@ -42,6 +42,8 @@ class Notification < ApplicationRecord
|
||||
|
||||
after_create_commit :process_notification_delivery, :dispatch_create_event
|
||||
|
||||
# TODO: Get rid of default scope
|
||||
# https://stackoverflow.com/a/1834250/939299
|
||||
default_scope { order(id: :desc) }
|
||||
|
||||
PRIMARY_ACTORS = ['Conversation'].freeze
|
||||
|
||||
@@ -20,24 +20,31 @@ class SearchService
|
||||
@accessable_inbox_ids ||= @current_user.assigned_inboxes.pluck(:id)
|
||||
end
|
||||
|
||||
def search_query
|
||||
@search_query ||= params[:q].to_s.strip
|
||||
end
|
||||
|
||||
def filter_conversations
|
||||
@conversations = current_account.conversations.where(inbox_id: accessable_inbox_ids)
|
||||
.joins('INNER JOIN contacts ON conversations.contact_id = contacts.id')
|
||||
.where("cast(conversations.display_id as text) ILIKE :search OR contacts.name ILIKE :search OR contacts.email
|
||||
ILIKE :search OR contacts.phone_number ILIKE :search OR contacts.identifier ILIKE :search", search: "%#{params[:q]}%")
|
||||
ILIKE :search OR contacts.phone_number ILIKE :search OR contacts.identifier ILIKE :search", search: "%#{search_query}%")
|
||||
.order('conversations.created_at DESC')
|
||||
.limit(10)
|
||||
end
|
||||
|
||||
def filter_messages
|
||||
@messages = current_account.messages.where(inbox_id: accessable_inbox_ids)
|
||||
.where('messages.content ILIKE :search', search: "%#{params[:q]}%")
|
||||
.where('created_at >= ?', 3.months.ago).limit(10)
|
||||
.where('messages.content ILIKE :search', search: "%#{search_query}%")
|
||||
.where('created_at >= ?', 3.months.ago)
|
||||
.reorder('created_at DESC')
|
||||
.limit(10)
|
||||
end
|
||||
|
||||
def filter_contacts
|
||||
@contacts = current_account.contacts.where(
|
||||
"name ILIKE :search OR email ILIKE :search OR phone_number
|
||||
ILIKE :search OR identifier ILIKE :search", search: "%#{params[:q]}%"
|
||||
).limit(10)
|
||||
ILIKE :search OR identifier ILIKE :search", search: "%#{search_query}%"
|
||||
).resolved_contacts.order_on_last_activity_at('desc').limit(10)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -32,7 +32,7 @@ RSpec.describe 'Search', type: :request do
|
||||
expect(response).to have_http_status(:success)
|
||||
response_data = JSON.parse(response.body, symbolize_names: true)
|
||||
|
||||
expect(response_data[:payload][:messages].first[:content]).to eq 'test1'
|
||||
expect(response_data[:payload][:messages].first[:content]).to eq 'test2'
|
||||
expect(response_data[:payload].keys).to match_array [:contacts, :conversations, :messages]
|
||||
expect(response_data[:payload][:messages].length).to eq 2
|
||||
expect(response_data[:payload][:conversations].length).to eq 1
|
||||
|
||||
@@ -7,12 +7,12 @@ describe ::SearchService do
|
||||
let!(:account) { create(:account) }
|
||||
let!(:user) { create(:user, account: account) }
|
||||
let!(:inbox) { create(:inbox, account: account, enable_auto_assignment: false) }
|
||||
let(:harry) { create(:contact, name: 'Harry Potter', account_id: account.id) }
|
||||
let!(:harry) { create(:contact, name: 'Harry Potter', email: 'test@test.com', account_id: account.id) }
|
||||
let!(:conversation) { create(:conversation, contact: harry, inbox: inbox, account: account) }
|
||||
let!(:message) { create(:message, account: account, inbox: inbox, content: 'Harry Potter is a wizard') }
|
||||
|
||||
before do
|
||||
create(:inbox_member, user: user, inbox: inbox)
|
||||
create(:conversation, contact: harry, inbox: inbox, account: account)
|
||||
create(:message, account: account, inbox: inbox, content: 'Harry Potter is a wizard')
|
||||
Current.account = account
|
||||
end
|
||||
|
||||
@@ -50,38 +50,44 @@ describe ::SearchService do
|
||||
end
|
||||
|
||||
context 'when contact search' do
|
||||
it 'searches across name, email, phone_number and identifier' do
|
||||
it 'searches across name, email, phone_number and identifier and returns in the order of contact last_activity_at' do
|
||||
# random contact
|
||||
create(:contact, account_id: account.id)
|
||||
harry2 = create(:contact, email: 'HarryPotter@test.com', account_id: account.id)
|
||||
harry3 = create(:contact, identifier: 'Potter123', account_id: account.id)
|
||||
# unresolved contact -> no identifying info
|
||||
# will not appear in search results
|
||||
create(:contact, name: 'Harry Potter', account_id: account.id)
|
||||
harry2 = create(:contact, email: 'HarryPotter@test.com', account_id: account.id, last_activity_at: 2.days.ago)
|
||||
harry3 = create(:contact, identifier: 'Potter123', account_id: account.id, last_activity_at: 1.day.ago)
|
||||
harry4 = create(:contact, identifier: 'Potter1235', account_id: account.id, last_activity_at: 2.minutes.ago)
|
||||
|
||||
params = { q: 'Potter' }
|
||||
params = { q: 'Potter ' }
|
||||
search = described_class.new(current_user: user, current_account: account, params: params, search_type: 'Contact')
|
||||
expect(search.perform[:contacts].map(&:id)).to match_array([harry.id, harry2.id, harry3.id])
|
||||
expect(search.perform[:contacts].map(&:id)).to eq([harry4.id, harry3.id, harry2.id, harry.id])
|
||||
end
|
||||
end
|
||||
|
||||
context 'when message search' do
|
||||
it 'searches across message content' do
|
||||
# random messages in another inbox
|
||||
create(:message, account: account, inbox: create(:inbox, account: account), content: 'Harry Potter is a wizard')
|
||||
it 'searches across message content and return in created_at desc' do
|
||||
# random messages in another account
|
||||
create(:message, content: 'Harry Potter is a wizard')
|
||||
# random messsage in inbox with out access
|
||||
create(:message, account: account, inbox: create(:inbox, account: account), content: 'Harry Potter is a wizard')
|
||||
message2 = create(:message, account: account, inbox: inbox, content: 'harry is cool')
|
||||
params = { q: 'Harry' }
|
||||
search = described_class.new(current_user: user, current_account: account, params: params, search_type: 'Message')
|
||||
expect(search.perform[:messages].map(&:id)).to match_array([Message.first.id, message2.id])
|
||||
expect(search.perform[:messages].map(&:id)).to eq([message2.id, message.id])
|
||||
end
|
||||
end
|
||||
|
||||
context 'when conversation search' do
|
||||
it 'searches across conversations using contact information' do
|
||||
it 'searches across conversations using contact information and order by created_at desc' do
|
||||
# random messages in another inbox
|
||||
random = create(:contact, account_id: account.id)
|
||||
create(:conversation, contact: random, inbox: inbox, account: account)
|
||||
conv2 = create(:conversation, contact: harry, inbox: inbox, account: account)
|
||||
params = { q: 'Harry' }
|
||||
search = described_class.new(current_user: user, current_account: account, params: params, search_type: 'Conversation')
|
||||
expect(search.perform[:conversations].map(&:id)).to match_array([Conversation.first.id])
|
||||
expect(search.perform[:conversations].map(&:id)).to eq([conv2.id, conversation.id])
|
||||
end
|
||||
|
||||
it 'searches across conversations with display id' do
|
||||
|
||||
Reference in New Issue
Block a user