diff --git a/app/actions/contact_merge_action.rb b/app/actions/contact_merge_action.rb index 2633c907d..54825df0f 100644 --- a/app/actions/contact_merge_action.rb +++ b/app/actions/contact_merge_action.rb @@ -31,19 +31,27 @@ class ContactMergeAction end def merge_conversations - Conversation.where(contact_id: @mergee_contact.id).update(contact_id: @base_contact.id) + Conversation.where(contact_id: @mergee_contact.id).find_each do |conversation| + conversation.update!(contact_id: @base_contact.id) + end end def merge_contact_notes - Note.where(contact_id: @mergee_contact.id, account_id: @mergee_contact.account_id).update(contact_id: @base_contact.id) + Note.where(contact_id: @mergee_contact.id, account_id: @mergee_contact.account_id).find_each do |note| + note.update!(contact_id: @base_contact.id) + end end def merge_messages - Message.where(sender: @mergee_contact).update(sender: @base_contact) + Message.where(sender: @mergee_contact).find_each do |message| + message.update!(sender: @base_contact) + end end def merge_contact_inboxes - ContactInbox.where(contact_id: @mergee_contact.id).update(contact_id: @base_contact.id) + ContactInbox.where(contact_id: @mergee_contact.id).find_each do |contact_inbox| + contact_inbox.update!(contact_id: @base_contact.id) + end end def merge_and_remove_mergee_contact diff --git a/app/jobs/conversations/resolution_job.rb b/app/jobs/conversations/resolution_job.rb index 7f61f22ab..d8f34f755 100644 --- a/app/jobs/conversations/resolution_job.rb +++ b/app/jobs/conversations/resolution_job.rb @@ -16,12 +16,10 @@ class Conversations::ResolutionJob < ApplicationJob private def conversation_scope(account) - base_scope = if account.auto_resolve_ignore_waiting - account.conversations.resolvable_not_waiting(account.auto_resolve_after) - else - account.conversations.resolvable_all(account.auto_resolve_after) - end - # Exclude orphan conversations where contact was deleted but conversation cleanup is pending - base_scope.where.not(contact_id: nil) + if account.auto_resolve_ignore_waiting + account.conversations.resolvable_not_waiting(account.auto_resolve_after) + else + account.conversations.resolvable_all(account.auto_resolve_after) + end end end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 42c01929b..eea88584d 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -24,7 +24,7 @@ # assignee_agent_bot_id :bigint # assignee_id :integer # campaign_id :bigint -# contact_id :bigint +# contact_id :bigint not null # contact_inbox_id :bigint # display_id :integer not null # inbox_id :integer not null diff --git a/db/migrate/20260422170000_enforce_contact_fk_on_conversations.rb b/db/migrate/20260422170000_enforce_contact_fk_on_conversations.rb new file mode 100644 index 000000000..6b1ff6417 --- /dev/null +++ b/db/migrate/20260422170000_enforce_contact_fk_on_conversations.rb @@ -0,0 +1,19 @@ +class EnforceContactFkOnConversations < ActiveRecord::Migration[7.1] + def up + orphan_scope = Conversation.where(contact_id: nil).or( + Conversation.where('NOT EXISTS (SELECT 1 FROM contacts WHERE contacts.id = conversations.contact_id)') + ) + + count = orphan_scope.count + if count.positive? + say "Destroying #{count} orphan conversations via Rails so dependent associations are cleaned up" + orphan_scope.find_each(&:destroy!) + end + + change_column_null :conversations, :contact_id, false + end + + def down + change_column_null :conversations, :contact_id, true + end +end diff --git a/db/schema.rb b/db/schema.rb index d3ab66a24..d7f1cf1fa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2026_04_18_020000) do +ActiveRecord::Schema[7.1].define(version: 2026_04_22_170000) do # These extensions should be enabled to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" @@ -704,7 +704,7 @@ ActiveRecord::Schema[7.1].define(version: 2026_04_18_020000) do t.integer "assignee_id" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.bigint "contact_id" + t.bigint "contact_id", null: false t.integer "display_id", null: false t.datetime "contact_last_seen_at", precision: nil t.datetime "agent_last_seen_at", precision: nil diff --git a/spec/jobs/conversations/resolution_job_spec.rb b/spec/jobs/conversations/resolution_job_spec.rb index 5764874e8..8e314fb46 100644 --- a/spec/jobs/conversations/resolution_job_spec.rb +++ b/spec/jobs/conversations/resolution_job_spec.rb @@ -48,21 +48,6 @@ RSpec.describe Conversations::ResolutionJob do end end - # When a contact is deleted, there's a brief window (~50-150ms) where contact_id becomes nil - # but conversations still exist. If ResolutionJob runs during this window, muted? can crash - # trying to call blocked? on nil. Fixes # (issue). - it 'skips orphan conversations without a contact' do - account.update!(auto_resolve_after: 14_400, auto_resolve_ignore_waiting: false) # 10 days in minutes - orphan_conversation = create(:conversation, account: account, last_activity_at: 13.days.ago, waiting_since: nil) - orphan_conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations - resolvable_conversation = create(:conversation, account: account, last_activity_at: 13.days.ago, waiting_since: nil) - - described_class.perform_now(account: account) - - expect(orphan_conversation.reload.status).to eq('open') - expect(resolvable_conversation.reload.status).to eq('resolved') - end - it 'adds a label after resolution' do account.update!(auto_resolve_label: 'auto-resolved', auto_resolve_after: 14_400) conversation = create(:conversation, account: account, last_activity_at: 13.days.ago, waiting_since: 13.days.ago) diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 6600bda02..c82f1c375 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -434,7 +434,7 @@ RSpec.describe Conversation do context 'when contact is missing' do before do - conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + allow(conversation).to receive(:contact).and_return(nil) end it 'does not change conversation status' do @@ -478,7 +478,7 @@ RSpec.describe Conversation do let(:conversation) { create(:conversation) } before do - conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + allow(conversation).to receive(:contact).and_return(nil) end it 'does not change conversation status' do @@ -507,7 +507,7 @@ RSpec.describe Conversation do context 'when contact is missing' do before do - conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + allow(conversation).to receive(:contact).and_return(nil) end it 'returns false' do