fix(conversations): enforce NOT NULL on contact_id + cleanup orphans (#273)

* fix(conversations): enforce NOT NULL + FK on contact_id

Conversations had contact_id nullable with no FK to contacts. Combined
with dependent: :destroy_async on Contact#conversations, deleting a
contact could leave conversations pointing to a missing contact,
breaking the conversations#index API with
"undefined method 'additional_attributes' for nil" from the contact
partial.

Changes:
- Migration cleans up existing orphans, sets contact_id NOT NULL and
  adds a FK with ON DELETE CASCADE so the invariant is enforced at the
  DB level (complements the existing Rails presence validation).
- ContactMergeAction uses update_all for conversations/messages/notes/
  contact_inboxes so a failing callback cannot silently leave records
  pointing to the mergee contact before it is destroyed.
- Drop the now-redundant orphan filter in Conversations::ResolutionJob
  and its spec; the invariant is enforced at the schema level.

* fix: address review feedback

- Drop the ON DELETE CASCADE FK on conversations.contact_id. Several
  conversation-owned tables (messages, mentions, conversation_participants,
  reporting_events, csat_survey_responses, calls, applied_slas, sla_events,
  and polymorphic notifications) still have plain conversation_id references
  without FK cascades. The DB-level cascade would skip Conversation's
  dependent: cleanup and replace the NULL-contact bug with orphan children,
  and would also conflict with the existing non-cascade FKs on
  scheduled_messages/recurring_scheduled_messages. Keep the invariant at the
  Rails layer (NOT NULL + presence validation + dependent: :destroy_async).
- Clean up orphan conversations in the migration via Rails destroy so
  dependent associations are propagated correctly, instead of a raw
  DELETE FROM conversations that would orphan all child rows.
- Revert ContactMergeAction.merge_* methods back to per-record update! so
  Conversation#after_update_commit still fires (notify_status_change /
  CONVERSATION_CONTACT_CHANGED) for contact_id changes. The bang form
  still removes the silent-failure risk of the original .update call.

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

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

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

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

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

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)

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