From d4e7eaeccebbb000ae84863380464cd1a8bc51bc Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Tue, 28 Feb 2023 22:00:36 +0530 Subject: [PATCH] feat: New APIs for search (#6564) - Adding new API endpoints for search - Migrations to add appropriate indexes --- .../api/v1/accounts/search_controller.rb | 28 +++++ app/jobs/migration/add_search_indexes_job.rb | 17 +++ app/models/contact.rb | 9 +- app/models/message.rb | 2 + app/services/search_service.rb | 43 +++++++ .../v1/accounts/search/_agent.json.jbuilder | 5 + .../v1/accounts/search/_contact.json.jbuilder | 5 + .../v1/accounts/search/_inbox.json.jbuilder | 4 + .../v1/accounts/search/_message.json.jbuilder | 14 +++ .../v1/accounts/search/contacts.json.jbuilder | 7 ++ .../search/conversations.json.jbuilder | 21 ++++ .../v1/accounts/search/index.json.jbuilder | 32 +++++ .../v1/accounts/search/messages.json.jbuilder | 7 ++ config/routes.rb | 8 ++ ...4124632_add_index_for_search_operations.rb | 6 + db/schema.rb | 6 +- .../api/v1/accounts/search_controller_spec.rb | 118 ++++++++++++++++++ spec/services/search_service_spec.rb | 96 ++++++++++++++ 18 files changed, 423 insertions(+), 5 deletions(-) create mode 100644 app/controllers/api/v1/accounts/search_controller.rb create mode 100644 app/jobs/migration/add_search_indexes_job.rb create mode 100644 app/services/search_service.rb create mode 100644 app/views/api/v1/accounts/search/_agent.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/_contact.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/_inbox.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/_message.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/contacts.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/conversations.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/index.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/messages.json.jbuilder create mode 100644 db/migrate/20230224124632_add_index_for_search_operations.rb create mode 100644 spec/controllers/api/v1/accounts/search_controller_spec.rb create mode 100644 spec/services/search_service_spec.rb diff --git a/app/controllers/api/v1/accounts/search_controller.rb b/app/controllers/api/v1/accounts/search_controller.rb new file mode 100644 index 000000000..35979f70f --- /dev/null +++ b/app/controllers/api/v1/accounts/search_controller.rb @@ -0,0 +1,28 @@ +class Api::V1::Accounts::SearchController < Api::V1::Accounts::BaseController + def index + @result = search('all') + end + + def conversations + @result = search('Conversation') + end + + def contacts + @result = search('Contact') + end + + def messages + @result = search('Message') + end + + private + + def search(search_type) + SearchService.new( + current_user: Current.user, + current_account: Current.account, + search_type: search_type, + params: params + ).perform + end +end diff --git a/app/jobs/migration/add_search_indexes_job.rb b/app/jobs/migration/add_search_indexes_job.rb new file mode 100644 index 000000000..c5c727203 --- /dev/null +++ b/app/jobs/migration/add_search_indexes_job.rb @@ -0,0 +1,17 @@ +# Delete migration and spec after 2 consecutive releases. +class Migration::AddSearchIndexesJob < ApplicationJob + queue_as :scheduled_jobs + + def perform + ActiveRecord::Migration[6.1].add_index(:messages, [:account_id, :inbox_id], algorithm: :concurrently) + ActiveRecord::Migration[6.1].add_index(:messages, :content, using: 'gin', opclass: :gin_trgm_ops, algorithm: :concurrently) + ActiveRecord::Migration[6.1].add_index( + :contacts, + [:name, :email, :phone_number, :identifier], + using: 'gin', + opclass: :gin_trgm_ops, + name: 'index_contacts_on_name_email_phone_number_identifier', + algorithm: :concurrently + ) + end +end diff --git a/app/models/contact.rb b/app/models/contact.rb index ec6053eb5..83019b969 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -16,10 +16,11 @@ # # Indexes # -# index_contacts_on_account_id (account_id) -# index_contacts_on_phone_number_and_account_id (phone_number,account_id) -# 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_name_email_phone_number_identifier (name,email,phone_number,identifier) USING gin +# index_contacts_on_phone_number_and_account_id (phone_number,account_id) +# uniq_email_per_account_contact (email,account_id) UNIQUE +# uniq_identifier_per_account_contact (identifier,account_id) UNIQUE # class Contact < ApplicationRecord diff --git a/app/models/message.rb b/app/models/message.rb index 40f4af757..01b385f27 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -23,7 +23,9 @@ # Indexes # # index_messages_on_account_id (account_id) +# index_messages_on_account_id_and_inbox_id (account_id,inbox_id) # index_messages_on_additional_attributes_campaign_id (((additional_attributes -> 'campaign_id'::text))) USING gin +# index_messages_on_content (content) USING gin # index_messages_on_conversation_id (conversation_id) # index_messages_on_inbox_id (inbox_id) # index_messages_on_sender_type_and_sender_id (sender_type,sender_id) diff --git a/app/services/search_service.rb b/app/services/search_service.rb new file mode 100644 index 000000000..77994c26f --- /dev/null +++ b/app/services/search_service.rb @@ -0,0 +1,43 @@ +class SearchService + pattr_initialize [:current_user!, :current_account!, :params!, :search_type!] + + def perform + case search_type + when 'Message' + { messages: filter_messages } + when 'Conversation' + { conversations: filter_conversations } + when 'Contact' + { contacts: filter_contacts } + else + { contacts: filter_contacts, messages: filter_messages, conversations: filter_conversations } + end + end + + private + + def accessable_inbox_ids + @accessable_inbox_ids ||= @current_user.assigned_inboxes.pluck(:id) + 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]}%") + .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) + 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) + end +end diff --git a/app/views/api/v1/accounts/search/_agent.json.jbuilder b/app/views/api/v1/accounts/search/_agent.json.jbuilder new file mode 100644 index 000000000..98a9e2c08 --- /dev/null +++ b/app/views/api/v1/accounts/search/_agent.json.jbuilder @@ -0,0 +1,5 @@ +json.id agent.id +json.available_name agent.available_name +json.email agent.email +json.name agent.name +json.role agent.role diff --git a/app/views/api/v1/accounts/search/_contact.json.jbuilder b/app/views/api/v1/accounts/search/_contact.json.jbuilder new file mode 100644 index 000000000..8d6ebcc96 --- /dev/null +++ b/app/views/api/v1/accounts/search/_contact.json.jbuilder @@ -0,0 +1,5 @@ +json.email contact.email +json.id contact.id +json.name contact.name +json.phone_number contact.phone_number +json.identifier contact.identifier diff --git a/app/views/api/v1/accounts/search/_inbox.json.jbuilder b/app/views/api/v1/accounts/search/_inbox.json.jbuilder new file mode 100644 index 000000000..a5f9e8b63 --- /dev/null +++ b/app/views/api/v1/accounts/search/_inbox.json.jbuilder @@ -0,0 +1,4 @@ +json.id inbox.id +json.channel_id inbox.channel_id +json.name inbox.name +json.channel_type inbox.channel_type diff --git a/app/views/api/v1/accounts/search/_message.json.jbuilder b/app/views/api/v1/accounts/search/_message.json.jbuilder new file mode 100644 index 000000000..a5e7bce42 --- /dev/null +++ b/app/views/api/v1/accounts/search/_message.json.jbuilder @@ -0,0 +1,14 @@ +json.id message.id +json.content message.content +json.message_type message.message_type_before_type_cast +json.content_type message.content_type +json.source_id message.source_id +json.inbox_id message.inbox_id +json.conversation_id message.conversation.try(:display_id) +json.created_at message.created_at.to_i +json.agent do + json.partial! 'agent', formats: [:json], agent: message.conversation.try(:assignee) if message.conversation.try(:assignee).present? +end +json.inbox do + json.partial! 'inbox', formats: [:json], inbox: message.inbox if message.inbox.present? && message.try(:inbox).present? +end diff --git a/app/views/api/v1/accounts/search/contacts.json.jbuilder b/app/views/api/v1/accounts/search/contacts.json.jbuilder new file mode 100644 index 000000000..505b50273 --- /dev/null +++ b/app/views/api/v1/accounts/search/contacts.json.jbuilder @@ -0,0 +1,7 @@ +json.payload do + json.contacts do + json.array! @result[:contacts] do |contact| + json.partial! 'contact', formats: [:json], contact: contact + end + end +end diff --git a/app/views/api/v1/accounts/search/conversations.json.jbuilder b/app/views/api/v1/accounts/search/conversations.json.jbuilder new file mode 100644 index 000000000..570cf3b4c --- /dev/null +++ b/app/views/api/v1/accounts/search/conversations.json.jbuilder @@ -0,0 +1,21 @@ +json.payload do + json.conversations do + json.array! @result[:conversations] do |conversation| + json.id conversation.display_id + json.account_id conversation.account_id + json.created_at conversation.created_at.to_i + json.message do + json.partial! 'message', formats: [:json], message: conversation.messages.try(:first) + end + json.contact do + json.partial! 'contact', formats: [:json], contact: conversation.contact if conversation.try(:contact).present? + end + json.inbox do + json.partial! 'inbox', formats: [:json], inbox: conversation.inbox if conversation.try(:inbox).present? + end + json.agent do + json.partial! 'agent', formats: [:json], agent: conversation.assignee if conversation.try(:assignee).present? + end + end + end +end diff --git a/app/views/api/v1/accounts/search/index.json.jbuilder b/app/views/api/v1/accounts/search/index.json.jbuilder new file mode 100644 index 000000000..1c6e86284 --- /dev/null +++ b/app/views/api/v1/accounts/search/index.json.jbuilder @@ -0,0 +1,32 @@ +json.payload do + json.conversations do + json.array! @result[:conversations] do |conversation| + json.id conversation.display_id + json.account_id conversation.account_id + json.created_at conversation.created_at.to_i + json.message do + json.partial! 'message', formats: [:json], message: conversation.messages.try(:first) + end + json.contact do + json.partial! 'contact', formats: [:json], contact: conversation.contact if conversation.try(:contact).present? + end + json.inbox do + json.partial! 'inbox', formats: [:json], inbox: conversation.inbox if conversation.try(:inbox).present? + end + json.agent do + json.partial! 'agent', formats: [:json], agent: conversation.assignee if conversation.try(:assignee).present? + end + end + end + json.contacts do + json.array! @result[:contacts] do |contact| + json.partial! 'contact', formats: [:json], contact: contact + end + end + + json.messages do + json.array! @result[:messages] do |message| + json.partial! 'message', formats: [:json], message: message + end + end +end diff --git a/app/views/api/v1/accounts/search/messages.json.jbuilder b/app/views/api/v1/accounts/search/messages.json.jbuilder new file mode 100644 index 000000000..1bd024dca --- /dev/null +++ b/app/views/api/v1/accounts/search/messages.json.jbuilder @@ -0,0 +1,7 @@ +json.payload do + json.messages do + json.array! @result[:messages] do |message| + json.partial! 'message', formats: [:json], message: message + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 0ac64012e..d318d6fa8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -96,6 +96,14 @@ Rails.application.routes.draw do end end + resources :search, only: [:index] do + collection do + get :conversations + get :messages + get :contacts + end + end + resources :contacts, only: [:index, :show, :update, :create, :destroy] do collection do get :active diff --git a/db/migrate/20230224124632_add_index_for_search_operations.rb b/db/migrate/20230224124632_add_index_for_search_operations.rb new file mode 100644 index 000000000..77386a50c --- /dev/null +++ b/db/migrate/20230224124632_add_index_for_search_operations.rb @@ -0,0 +1,6 @@ +class AddIndexForSearchOperations < ActiveRecord::Migration[6.1] + def change + enable_extension('pg_trgm') + Migration::AddSearchIndexesJob.perform_later + end +end diff --git a/db/schema.rb b/db/schema.rb index bd4e95b2a..a9b7217ca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,10 +10,11 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_02_14_025901) do +ActiveRecord::Schema.define(version: 2023_02_24_124632) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" + enable_extension "pg_trgm" enable_extension "pgcrypto" enable_extension "plpgsql" @@ -385,6 +386,7 @@ ActiveRecord::Schema.define(version: 2023_02_14_025901) 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 ["name", "email", "phone_number", "identifier"], name: "index_contacts_on_name_email_phone_number_identifier", opclass: :gin_trgm_ops, using: :gin t.index ["phone_number", "account_id"], name: "index_contacts_on_phone_number_and_account_id" end @@ -631,7 +633,9 @@ ActiveRecord::Schema.define(version: 2023_02_14_025901) do t.jsonb "external_source_ids", default: {} t.jsonb "additional_attributes", default: {} t.index "((additional_attributes -> 'campaign_id'::text))", name: "index_messages_on_additional_attributes_campaign_id", using: :gin + t.index ["account_id", "inbox_id"], name: "index_messages_on_account_id_and_inbox_id" t.index ["account_id"], name: "index_messages_on_account_id" + t.index ["content"], name: "index_messages_on_content", opclass: :gin_trgm_ops, using: :gin t.index ["conversation_id"], name: "index_messages_on_conversation_id" t.index ["inbox_id"], name: "index_messages_on_inbox_id" t.index ["sender_type", "sender_id"], name: "index_messages_on_sender_type_and_sender_id" diff --git a/spec/controllers/api/v1/accounts/search_controller_spec.rb b/spec/controllers/api/v1/accounts/search_controller_spec.rb new file mode 100644 index 000000000..d12898d9e --- /dev/null +++ b/spec/controllers/api/v1/accounts/search_controller_spec.rb @@ -0,0 +1,118 @@ +require 'rails_helper' + +RSpec.describe 'Search', type: :request do + let(:account) { create(:account) } + let(:agent) { create(:user, account: account, role: :agent) } + + before do + contact = create(:contact, email: 'test@example.com', account: account) + conversation = create(:conversation, account: account, contact_id: contact.id) + create(:message, conversation: conversation, account: account, content: 'test1') + create(:message, conversation: conversation, account: account, content: 'test2') + create(:contact_inbox, contact_id: contact.id, inbox_id: conversation.inbox.id) + create(:inbox_member, user: agent, inbox: conversation.inbox) + end + + describe 'GET /api/v1/accounts/{account.id}/search' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v1/accounts/#{account.id}/search", params: { q: 'test' } + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'returns all conversations with messages containing the search query' do + get "/api/v1/accounts/#{account.id}/search", + headers: agent.create_new_auth_token, + params: { q: 'test' }, + as: :json + + 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].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 + expect(response_data[:payload][:contacts].length).to eq 1 + end + end + end + + describe 'GET /api/v1/accounts/{account.id}/search/contacts' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v1/accounts/#{account.id}/search/contacts", params: { q: 'test' } + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'returns all conversations with messages containing the search query' do + get "/api/v1/accounts/#{account.id}/search/contacts", + headers: agent.create_new_auth_token, + params: { q: 'test' }, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + + expect(response_data[:payload].keys).to match_array [:contacts] + expect(response_data[:payload][:contacts].length).to eq 1 + end + end + end + + describe 'GET /api/v1/accounts/{account.id}/search/conversations' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v1/accounts/#{account.id}/search/conversations", params: { q: 'test' } + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'returns all conversations with messages containing the search query' do + get "/api/v1/accounts/#{account.id}/search/conversations", + headers: agent.create_new_auth_token, + params: { q: 'test' }, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + + expect(response_data[:payload].keys).to match_array [:conversations] + expect(response_data[:payload][:conversations].length).to eq 1 + end + end + end + + describe 'GET /api/v1/accounts/{account.id}/search/messages' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v1/accounts/#{account.id}/search/messages", params: { q: 'test' } + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'returns all conversations with messages containing the search query' do + get "/api/v1/accounts/#{account.id}/search/messages", + headers: agent.create_new_auth_token, + params: { q: 'test' }, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + + expect(response_data[:payload].keys).to match_array [:messages] + expect(response_data[:payload][:messages].length).to eq 2 + end + end + end +end diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb new file mode 100644 index 000000000..1419eeac6 --- /dev/null +++ b/spec/services/search_service_spec.rb @@ -0,0 +1,96 @@ +require 'rails_helper' + +describe ::SearchService do + subject(:search) { described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) } + + let(:search_type) { 'all' } + 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) } + + 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 + + after do + Current.account = nil + end + + describe '#perform' do + context 'when search types' do + let(:params) { { q: 'Potter' } } + + it 'returns all for all' do + search_type = 'all' + search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) + expect(search.perform.keys).to match_array(%i[contacts messages conversations]) + end + + it 'returns contacts for contacts' do + search_type = 'Contact' + search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) + expect(search.perform.keys).to match_array(%i[contacts]) + end + + it 'returns messages for messages' do + search_type = 'Message' + search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) + expect(search.perform.keys).to match_array(%i[messages]) + end + + it 'returns conversations for conversations' do + search_type = 'Conversation' + search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) + expect(search.perform.keys).to match_array(%i[conversations]) + end + end + + context 'when contact search' do + it 'searches across name, email, phone_number and identifier' 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) + + 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]) + 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') + create(:message, 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]) + end + end + + context 'when conversation search' do + it 'searches across conversations using contact information' do + # random messages in another inbox + random = create(:contact, account_id: account.id) + create(:conversation, contact: random, 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]) + end + + it 'searches across conversations with display id' do + random = create(:contact, account_id: account.id, name: 'random', email: 'random@random.test', identifier: 'random') + new_converstion = create(:conversation, contact: random, inbox: inbox, account: account) + params = { q: new_converstion.display_id } + search = described_class.new(current_user: user, current_account: account, params: params, search_type: 'Conversation') + expect(search.perform[:conversations].map(&:id)).to match_array([new_converstion.id]) + end + end + end +end