From ee6382109ad3fafdb7aaa51257be0f11fb801fc7 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Mon, 15 Jun 2026 17:06:40 +0530 Subject: [PATCH] feat: prevent deleted email conversations from syncing again (#14612) # Pull Request Template ## Description Prevent deleted email conversations from being synced into Chatwoot again while they are still within the IMAP sync window. When an admin explicitly deletes an email conversation, the incoming email message IDs are stored temporarily in Redis. IMAP sync checks these recently deleted message IDs in addition to existing message records. Each Redis key expires automatically after two days. This applies only to explicit conversation deletion. Individual message deletion, inbox deletion, and account deletion keep their existing behavior. Fixes [CW-7214](https://linear.app/chatwoot/issue/CW-7214/deleted-mails-in-gmail-inbox-gets-synced-again) --- .../v1/accounts/conversations_controller.rb | 2 +- app/services/conversations/delete_service.rb | 16 +++++++++ app/services/imap/base_fetch_email_service.rb | 6 +++- app/services/imap/deleted_message_tracker.rb | 28 +++++++++++++++ lib/redis/redis_keys.rb | 2 ++ .../conversations/delete_service_spec.rb | 35 +++++++++++++++++++ .../services/imap/fetch_email_service_spec.rb | 22 ++++++++++++ 7 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 app/services/conversations/delete_service.rb create mode 100644 app/services/imap/deleted_message_tracker.rb create mode 100644 spec/services/conversations/delete_service_spec.rb diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 2856c7817..2e53fa7c9 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -140,7 +140,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def destroy authorize @conversation, :destroy? - ::DeleteObjectJob.perform_later(@conversation, Current.user, request.ip) + ::Conversations::DeleteService.new(conversation: @conversation, user: Current.user, ip: request.ip).perform head :ok end diff --git a/app/services/conversations/delete_service.rb b/app/services/conversations/delete_service.rb new file mode 100644 index 000000000..6608cfdcd --- /dev/null +++ b/app/services/conversations/delete_service.rb @@ -0,0 +1,16 @@ +class Conversations::DeleteService + pattr_initialize [:conversation!, :user, :ip] + + def perform + track_deleted_email_messages + ::DeleteObjectJob.perform_later(conversation, user, ip) + end + + private + + def track_deleted_email_messages + return unless conversation.inbox.email? + + Imap::DeletedMessageTracker.new(inbox: conversation.inbox).record(conversation.messages.incoming.pluck(:source_id)) + end +end diff --git a/app/services/imap/base_fetch_email_service.rb b/app/services/imap/base_fetch_email_service.rb index 24dd22589..e55355f3a 100644 --- a/app/services/imap/base_fetch_email_service.rb +++ b/app/services/imap/base_fetch_email_service.rb @@ -38,7 +38,11 @@ class Imap::BaseFetchEmailService end def email_already_present?(channel, message_id) - channel.inbox.messages.find_by(source_id: message_id).present? + channel.inbox.messages.find_by(source_id: message_id).present? || deleted_message_tracker.deleted?(message_id) + end + + def deleted_message_tracker + @deleted_message_tracker ||= Imap::DeletedMessageTracker.new(inbox: channel.inbox) end def fetch_mail_for_channel diff --git a/app/services/imap/deleted_message_tracker.rb b/app/services/imap/deleted_message_tracker.rb new file mode 100644 index 000000000..7892f258b --- /dev/null +++ b/app/services/imap/deleted_message_tracker.rb @@ -0,0 +1,28 @@ +require 'digest' + +class Imap::DeletedMessageTracker + TTL = 2.days.to_i + + pattr_initialize [:inbox!] + + def record(source_ids) + return unless inbox.email? + + keys = source_ids.compact_blank.map { |source_id| redis_key(source_id) } + return if keys.blank? + + Redis::Alfred.pipelined do |pipeline| + keys.each { |key| pipeline.set(key, true, ex: TTL) } + end + end + + def deleted?(source_id) + Redis::Alfred.exists?(redis_key(source_id)) + end + + private + + def redis_key(source_id) + format(Redis::RedisKeys::IMAP_DELETED_MESSAGE, inbox_id: inbox.id, message_id_digest: Digest::SHA256.hexdigest(source_id)) + end +end diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index fff60c342..bc7fb551e 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -2,6 +2,8 @@ module Redis::RedisKeys ## Inbox Keys # Array storing the ordered ids for agent round robin assignment ROUND_ROBIN_AGENTS = 'ROUND_ROBIN_AGENTS:%d'.freeze + # Track recently deleted IMAP messages to prevent them from being synced again + IMAP_DELETED_MESSAGE = 'IMAP_DELETED_MESSAGE::%d::%s'.freeze ## Conversation keys # Detect whether to send an email reply to the conversation diff --git a/spec/services/conversations/delete_service_spec.rb b/spec/services/conversations/delete_service_spec.rb new file mode 100644 index 000000000..2ff14db49 --- /dev/null +++ b/spec/services/conversations/delete_service_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +RSpec.describe Conversations::DeleteService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:ip) { '127.0.0.1' } + let(:service) { described_class.new(conversation: conversation, user: user, ip: ip) } + + context 'when deleting an email conversation' do + let(:inbox) { create(:channel_email, :imap_email, account: account).inbox } + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + let!(:incoming_message) { create(:message, account: account, inbox: inbox, conversation: conversation, source_id: 'incoming@example.com') } + let(:deleted_message_tracker) { instance_double(Imap::DeletedMessageTracker, record: true) } + + before do + allow(Imap::DeletedMessageTracker).to receive(:new).with(inbox: inbox).and_return(deleted_message_tracker) + end + + it 'records incoming message source ids and enqueues the deletion job' do + expect { service.perform }.to have_enqueued_job(DeleteObjectJob).with(conversation, user, ip) + + expect(deleted_message_tracker).to have_received(:record).with([incoming_message.source_id]) + end + end + + context 'when deleting a non-email conversation' do + let(:conversation) { create(:conversation, account: account) } + + it 'enqueues the deletion job without recording message source ids' do + expect(Imap::DeletedMessageTracker).not_to receive(:new) + + expect { service.perform }.to have_enqueued_job(DeleteObjectJob).with(conversation, user, ip) + end + end +end diff --git a/spec/services/imap/fetch_email_service_spec.rb b/spec/services/imap/fetch_email_service_spec.rb index 1f74cf0b7..b88e548d9 100644 --- a/spec/services/imap/fetch_email_service_spec.rb +++ b/spec/services/imap/fetch_email_service_spec.rb @@ -119,6 +119,28 @@ RSpec.describe Imap::FetchEmailService do end end + it 'does not return recently deleted emails' do + travel_to '26.10.2020 10:00'.to_datetime do + email_object = create_inbound_email_from_fixture('only_text.eml') + email_header = Net::IMAP::FetchData.new(1, 'BODY[HEADER]' => eml_content_with_message_id) + redis_key = format(Redis::RedisKeys::IMAP_DELETED_MESSAGE, + inbox_id: imap_email_channel.inbox.id, + message_id_digest: Digest::SHA256.hexdigest(email_object.message_id)) + + Imap::DeletedMessageTracker.new(inbox: imap_email_channel.inbox).record([email_object.message_id]) + allow(imap).to receive(:search).with(%w[SINCE 25-Oct-2020]).and_return([1]) + allow(imap).to receive(:fetch).with([1], 'BODY.PEEK[HEADER]').and_return([email_header]) + allow(imap).to receive(:logout) + + result = described_class.new(channel: imap_email_channel).perform + + expect(result).to be_empty + expect(imap).not_to have_received(:fetch).with(1, 'RFC822') + ensure + Redis::Alfred.delete(redis_key) if redis_key + end + end + it 'does not count emails without message ids toward the sync limit' do travel_to '26.10.2020 10:00'.to_datetime do email_object = create_inbound_email_from_fixture('only_text.eml')