diff --git a/app/jobs/conversations/resolution_job.rb b/app/jobs/conversations/resolution_job.rb index d8f34f755..7f61f22ab 100644 --- a/app/jobs/conversations/resolution_job.rb +++ b/app/jobs/conversations/resolution_job.rb @@ -16,10 +16,12 @@ class Conversations::ResolutionJob < ApplicationJob private def conversation_scope(account) - 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 + 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) end end diff --git a/app/models/concerns/conversation_mute_helpers.rb b/app/models/concerns/conversation_mute_helpers.rb index c6ea4c7b1..ebc0542d2 100644 --- a/app/models/concerns/conversation_mute_helpers.rb +++ b/app/models/concerns/conversation_mute_helpers.rb @@ -2,17 +2,21 @@ module ConversationMuteHelpers extend ActiveSupport::Concern def mute! + return unless contact + resolved! contact.update(blocked: true) create_muted_message end def unmute! + return unless contact + contact.update(blocked: false) create_unmuted_message end def muted? - contact.blocked? + contact&.blocked? || false end end diff --git a/spec/jobs/conversations/resolution_job_spec.rb b/spec/jobs/conversations/resolution_job_spec.rb index a80c39498..3bc443e20 100644 --- a/spec/jobs/conversations/resolution_job_spec.rb +++ b/spec/jobs/conversations/resolution_job_spec.rb @@ -48,6 +48,21 @@ 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 5a4acf329..e1883b54d 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -390,6 +390,20 @@ RSpec.describe Conversation do .to(have_been_enqueued.at_least(:once).with(conversation, { account_id: conversation.account_id, inbox_id: conversation.inbox_id, message_type: :activity, content: "#{user.name} has muted the conversation" })) end + + context 'when contact is missing' do + before do + conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + end + + it 'does not change conversation status' do + expect { mute! }.not_to(change { conversation.reload.status }) + end + + it 'does not enqueue an activity message' do + expect { mute! }.not_to have_enqueued_job(Conversations::ActivityMessageJob) + end + end end describe '#unmute!' do @@ -418,6 +432,22 @@ RSpec.describe Conversation do .to(have_been_enqueued.at_least(:once).with(conversation, { account_id: conversation.account_id, inbox_id: conversation.inbox_id, message_type: :activity, content: "#{user.name} has unmuted the conversation" })) end + + context 'when contact is missing' do + let(:conversation) { create(:conversation) } + + before do + conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + end + + it 'does not change conversation status' do + expect { unmute! }.not_to(change { conversation.reload.status }) + end + + it 'does not enqueue an activity message' do + expect { unmute! }.not_to have_enqueued_job(Conversations::ActivityMessageJob) + end + end end describe '#muted?' do @@ -433,6 +463,16 @@ RSpec.describe Conversation do it 'returns false if conversation is not muted' do expect(muted?).to be(false) end + + context 'when contact is missing' do + before do + conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + end + + it 'returns false' do + expect(muted?).to be(false) + end + end end describe 'unread_messages' do