From dc0e5eb4652571b65e7568f5d42cd97312974f2b Mon Sep 17 00:00:00 2001 From: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com> Date: Wed, 11 Mar 2026 05:16:20 +0530 Subject: [PATCH] fix: optimize message query with account_id filter (#13759) ## Description This PR optimizes message queries by explicitly filtering with `account_id` so the database can use the existing indexes more efficiently. Changes: - Add `account_id` to message query filters to improve index utilization. - Update `last_incoming_message` query to include `account_id`. - Avoid unnecessary preloading of `contact_inboxes` where it is not required. - Update specs to ensure `account_id` is set correctly in message-related tests. These changes reduce query cost and improve performance for message lookups, especially on large accounts. --------- Co-authored-by: Pranav --- .../api/v1/accounts/contacts_controller.rb | 4 +- app/models/conversation.rb | 2 +- .../hook_execution_service_spec.rb | 44 +++++++++---------- .../hook_execution_service_spec.rb | 32 +++++++------- 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb index 14d4f2c89..dd5346bd6 100644 --- a/app/controllers/api/v1/accounts/contacts_controller.rb +++ b/app/controllers/api/v1/accounts/contacts_controller.rb @@ -201,7 +201,9 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController end def fetch_contact - @contact = Current.account.contacts.includes(contact_inboxes: [:inbox]).find(params[:id]) + contact_scope = Current.account.contacts + contact_scope = contact_scope.includes(contact_inboxes: [:inbox]) if @include_contact_inboxes + @contact = contact_scope.find(params[:id]) end def process_avatar_from_url diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 6dd0e9df5..911cfdac6 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -143,7 +143,7 @@ class Conversation < ApplicationRecord end def last_incoming_message - messages&.incoming&.last + messages.where(account_id: account_id)&.incoming&.last end def toggle_status diff --git a/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb b/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb index 151e9101a..9eacb0ba1 100644 --- a/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb +++ b/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb @@ -24,7 +24,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'schedules captain response job for incoming messages on pending conversations' do expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(conversation, assistant) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end end @@ -43,7 +43,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'schedules captain response job outside business hours (Captain always responds when configured)' do expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(conversation, assistant) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end it 'performs captain handoff when quota is exceeded (OOO template will kick in after handoff)' do @@ -52,7 +52,7 @@ RSpec.describe MessageTemplates::HookExecutionService do custom_attributes: account.custom_attributes.merge('captain_responses_usage' => 100) ) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) expect(conversation.reload.status).to eq('open') end @@ -62,7 +62,7 @@ RSpec.describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) allow(out_of_office_service).to receive(:perform).and_return(true) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) end @@ -76,7 +76,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'schedules captain response job regardless of time' do expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(conversation, assistant) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end end @@ -95,7 +95,7 @@ RSpec.describe MessageTemplates::HookExecutionService do end it 'performs handoff within business hours when quota exceeded' do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) expect(conversation.reload.status).to eq('open') end @@ -110,7 +110,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'does not schedule captain response job' do expect(Captain::Conversation::ResponseBuilderJob).not_to receive(:perform_later) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end end @@ -122,7 +122,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'does not schedule captain response job' do expect(Captain::Conversation::ResponseBuilderJob).not_to receive(:perform_later) - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end end @@ -130,7 +130,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'does not schedule captain response job' do expect(Captain::Conversation::ResponseBuilderJob).not_to receive(:perform_later) - create(:message, conversation: conversation, message_type: :outgoing) + create(:message, conversation: conversation, message_type: :outgoing, account: account) end end @@ -144,7 +144,7 @@ RSpec.describe MessageTemplates::HookExecutionService do inbox.update!(greeting_enabled: true, greeting_message: 'Hello! How can we help you?', enable_email_collect: false) expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.not_to(change { conversation.reload.messages.template.count }) end @@ -160,7 +160,7 @@ RSpec.describe MessageTemplates::HookExecutionService do ) expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.not_to(change { conversation.reload.messages.template.count }) end end @@ -174,7 +174,7 @@ RSpec.describe MessageTemplates::HookExecutionService do inbox.update!(greeting_enabled: true, greeting_message: 'Hello! How can we help you?', enable_email_collect: false) expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.to change { conversation.reload.messages.template.count }.by(1) greeting_message = conversation.reload.messages.template.last @@ -193,7 +193,7 @@ RSpec.describe MessageTemplates::HookExecutionService do ) expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.to change { conversation.reload.messages.template.count }.by(1) out_of_office_message = conversation.reload.messages.template.last @@ -211,7 +211,7 @@ RSpec.describe MessageTemplates::HookExecutionService do inbox.update!(greeting_enabled: true, greeting_message: 'Hello! How can we help you?', enable_email_collect: false) expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.to change { conversation.reload.messages.template.count }.by(1) greeting_message = conversation.reload.messages.template.last @@ -230,7 +230,7 @@ RSpec.describe MessageTemplates::HookExecutionService do ) expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.to change { conversation.reload.messages.template.count }.by(1) out_of_office_message = conversation.reload.messages.template.last @@ -245,7 +245,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'schedules captain response job for incoming messages on pending campaign conversations' do expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(campaign_conversation, assistant) - create(:message, conversation: campaign_conversation, message_type: :incoming) + create(:message, conversation: campaign_conversation, message_type: :incoming, account: account) end it 'does not send greeting template on campaign conversations' do @@ -255,7 +255,7 @@ RSpec.describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service) allow(greeting_service).to receive(:perform).and_return(true) - create(:message, conversation: campaign_conversation, message_type: :incoming) + create(:message, conversation: campaign_conversation, message_type: :incoming, account: account) expect(MessageTemplates::Template::Greeting).not_to have_received(:new) end @@ -271,7 +271,7 @@ RSpec.describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) allow(out_of_office_service).to receive(:perform).and_return(true) - create(:message, conversation: campaign_conversation, message_type: :incoming) + create(:message, conversation: campaign_conversation, message_type: :incoming, account: account) expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) end @@ -284,7 +284,7 @@ RSpec.describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(email_collect_service) allow(email_collect_service).to receive(:perform).and_return(true) - create(:message, conversation: campaign_conversation, message_type: :incoming) + create(:message, conversation: campaign_conversation, message_type: :incoming, account: account) expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new) end @@ -304,7 +304,7 @@ RSpec.describe MessageTemplates::HookExecutionService do ) expect do - create(:message, conversation: campaign_conversation, message_type: :incoming) + create(:message, conversation: campaign_conversation, message_type: :incoming, account: account) end.not_to(change { campaign_conversation.messages.template.count }) end end @@ -332,7 +332,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'sends out of office message after handoff due to quota exceeded' do expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.to change { conversation.messages.template.count }.by(1) expect(conversation.reload.status).to eq('open') @@ -356,7 +356,7 @@ RSpec.describe MessageTemplates::HookExecutionService do it 'does not send out of office message after handoff' do expect do - create(:message, conversation: conversation, message_type: :incoming) + create(:message, conversation: conversation, message_type: :incoming, account: account) end.not_to(change { conversation.messages.template.count }) expect(conversation.reload.status).to eq('open') diff --git a/spec/services/message_templates/hook_execution_service_spec.rb b/spec/services/message_templates/hook_execution_service_spec.rb index fb9437111..e186227be 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -15,7 +15,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::Greeting).to receive(:new) # described class gets called in message after commit - create(:message, conversation: conversation, message_type: 'activity', content: 'Conversation marked resolved!!') + create(:message, conversation: conversation, account: conversation.account, message_type: 'activity', content: 'Conversation marked resolved!!') expect(MessageTemplates::Template::Greeting).not_to have_received(:new) expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new) @@ -36,7 +36,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::Greeting).to receive(:new) # described class gets called in message after commit - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::Greeting).not_to have_received(:new) expect(MessageTemplates::Template::EmailCollect).to have_received(:new).with(conversation: message.conversation) @@ -54,7 +54,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service) allow(greeting_service).to receive(:perform).and_return(true) - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::Greeting).not_to have_received(:new).with(conversation: message.conversation) end end @@ -75,7 +75,7 @@ describe MessageTemplates::HookExecutionService do allow(greeting_service).to receive(:perform).and_return(true) # described class gets called in message after commit - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::Greeting).to have_received(:new).with(conversation: message.conversation) expect(greeting_service).to have_received(:perform) @@ -90,7 +90,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(true) # described class gets called in message after commit - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new).with(conversation: message.conversation) end @@ -105,7 +105,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(true) # described class gets called in message after commit - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new).with(conversation: message.conversation) end @@ -123,7 +123,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service) allow(greeting_service).to receive(:perform).and_return(true) - create(:message, conversation: conversation) + create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::Greeting).not_to have_received(:new) end @@ -139,7 +139,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) allow(out_of_office_service).to receive(:perform).and_return(true) - create(:message, conversation: conversation) + create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) end @@ -151,7 +151,7 @@ describe MessageTemplates::HookExecutionService do conversation = create(:conversation, contact: contact) conversation.inbox.update(greeting_enabled: true, enable_email_collect: true, greeting_message: 'Hi, this is a greeting message') - message = create(:message, conversation: conversation, content_type: :incoming_email) + message = create(:message, conversation: conversation, account: conversation.account, content_type: :incoming_email) message.content_attributes = { email: { auto_reply: true } } message.save! @@ -188,7 +188,7 @@ describe MessageTemplates::HookExecutionService do allow(out_of_office_service).to receive(:perform).and_return(true) # described class gets called in message after commit - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::OutOfOffice).to have_received(:new).with(conversation: message.conversation) expect(out_of_office_service).to have_received(:perform) @@ -202,13 +202,13 @@ describe MessageTemplates::HookExecutionService do conversation.inbox.update(working_hours_enabled: true, out_of_office_message: 'We are out of office') conversation.inbox.working_hours.today.update!(closed_all_day: true) - create(:message, conversation: conversation, message_type: :outgoing, created_at: 2.minutes.ago) + create(:message, conversation: conversation, account: conversation.account, message_type: :outgoing, created_at: 2.minutes.ago) out_of_office_service = double allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) allow(out_of_office_service).to receive(:perform).and_return(true) - create(:message, conversation: conversation) + create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) expect(out_of_office_service).not_to have_received(:perform) @@ -221,13 +221,13 @@ describe MessageTemplates::HookExecutionService do conversation.inbox.update(working_hours_enabled: true, out_of_office_message: 'We are out of office') conversation.inbox.working_hours.today.update!(closed_all_day: true) - create(:message, conversation: conversation, private: true, message_type: :outgoing, created_at: 2.minutes.ago) + create(:message, conversation: conversation, account: conversation.account, private: true, message_type: :outgoing, created_at: 2.minutes.ago) out_of_office_service = double allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) allow(out_of_office_service).to receive(:perform).and_return(true) - create(:message, conversation: conversation) + create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::OutOfOffice).to have_received(:new).with(conversation: conversation) expect(out_of_office_service).to have_received(:perform) @@ -247,7 +247,7 @@ describe MessageTemplates::HookExecutionService do allow(out_of_office_service).to receive(:perform).and_return(true) # described class gets called in message after commit - message = create(:message, conversation: conversation, message_type: 'outgoing') + message = create(:message, conversation: conversation, account: conversation.account, message_type: 'outgoing') expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new).with(conversation: message.conversation) expect(out_of_office_service).not_to have_received(:perform) @@ -265,7 +265,7 @@ describe MessageTemplates::HookExecutionService do allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) allow(out_of_office_service).to receive(:perform).and_return(false) - message = create(:message, conversation: conversation) + message = create(:message, conversation: conversation, account: conversation.account) expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new).with(conversation: message.conversation) expect(out_of_office_service).not_to receive(:perform) end