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 <pranav@chatwoot.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user