From f83415f29910d09629f623e7cbcbe7d06c0fe803 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Sat, 7 Feb 2026 17:29:27 -0800 Subject: [PATCH] fix(account-deletion): normalize deleted email suffix and handle collisions safely (#13472) ## Summary This PR fixes account deletion failures by changing how orphaned user emails are rewritten during `AccountDeletionService`. Ref: https://chatwoot-p3.sentry.io/issues/6715254765/events/e228a5d045ad47348d6c32448bc33b7a/ ## Changes (develop -> this branch) - Updated soft-delete email rewrite from: - `#{original_email}-deleted.com` - To deterministic value: - `#{user.id}@chatwoot-deleted.invalid` - Added reserved non-deliverable domain constant: - `@chatwoot-deleted.invalid` - Replaced the "other accounts" check from `count.zero?` to `exists?` (same behavior, cheaper query). - Updated service spec expectation to match deterministic email value and assert it differs from original email. ## Files changed - `app/services/account_deletion_service.rb` - `spec/services/account_deletion_service_spec.rb` ## How to verify - Run: `bundle exec rspec spec/services/account_deletion_service_spec.rb` - Run: `bundle exec rubocop app/services/account_deletion_service.rb spec/services/account_deletion_service_spec.rb` --- app/services/account_deletion_service.rb | 16 +++++++++------- spec/services/account_deletion_service_spec.rb | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/services/account_deletion_service.rb b/app/services/account_deletion_service.rb index 62bce601b..ab9555efb 100644 --- a/app/services/account_deletion_service.rb +++ b/app/services/account_deletion_service.rb @@ -1,4 +1,6 @@ class AccountDeletionService + SOFT_DELETE_EMAIL_DOMAIN = '@chatwoot-deleted.invalid'.freeze + attr_reader :account, :soft_deleted_users def initialize(account:) @@ -25,15 +27,11 @@ class AccountDeletionService def soft_delete_orphaned_users account.users.each do |user| - # Find all account_users for this user excluding the current account - other_accounts = user.account_users.where.not(account_id: account.id).count + # Skip users who are still associated with another account. + next if user.account_users.where.not(account_id: account.id).exists? - # If user has no other accounts, soft delete them - next unless other_accounts.zero? - - # Soft delete user by appending -deleted.com to email original_email = user.email - user.email = "#{original_email}-deleted.com" + user.email = soft_deleted_email_for(user) user.skip_reconfirmation! user.save! @@ -47,4 +45,8 @@ class AccountDeletionService Rails.logger.info("Soft deleted user #{user.id} with email #{original_email}") end end + + def soft_deleted_email_for(user) + "#{user.id}#{SOFT_DELETE_EMAIL_DOMAIN}" + end end diff --git a/spec/services/account_deletion_service_spec.rb b/spec/services/account_deletion_service_spec.rb index 6b263c5b3..bd1dc88ee 100644 --- a/spec/services/account_deletion_service_spec.rb +++ b/spec/services/account_deletion_service_spec.rb @@ -46,7 +46,8 @@ RSpec.describe AccountDeletionService do # Reload the user to get the updated email user_with_one_account.reload - expect(user_with_one_account.email).to eq("#{original_email}-deleted.com") + expect(user_with_one_account.email).to eq("#{user_with_one_account.id}@chatwoot-deleted.invalid") + expect(user_with_one_account.email).not_to eq(original_email) end it 'does not modify emails for users belonging to multiple accounts' do