fix: prevent NoMethodError in mute helpers when contact is nil (#13277)
## Linear Ticket https://linear.app/chatwoot/issue/CW-4569/nomethoderror-undefined-method-blocked-for-nil-nomethoderror ## Description Fixes NoMethodError in ConversationMuteHelpers that occurs during contact deletion race condition. 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, the muted? method crashes trying to call blocked? on nil.Fixes # (issue) ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Created orphaned conversations (contact_id = nil) - Called muted?, mute!, unmute! - all return gracefully - Verified async deletion still works correctly ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules Co-authored-by: Sojan Jose <sojan@pepalo.com>
This commit is contained in:
committed by
GitHub
parent
7d68c25c97
commit
d451615811
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user