Fix error when suspending user with an already-existing canonical email block (#17036)

* Fix error when suspending user with an already-existing canonical email block

Fixes #17033

While attempting to create a `CanonicalEmailBlock` with an existing hash would
raise an `ActiveRecord::RecordNotUnique` error, this being done within a
transaction would cancel the whole transaction. For this reason, checking for
uniqueness in Rails would query the database within the transaction and avoid
invalidating the whole transaction for this reason.

A race condition is still possible, where multiple accounts sharing a canonical
email would be blocked in concurrent transactions, in which only one would
succeed, but that is way less likely to happen that the current issue, and can
always be retried after the first failure, unlike the current situation.

* Add tests

diff --git a/app/models/canonical_email_block.rb b/app/models/canonical_email_block.rb
index a8546d65af6ce785e3cc85f005f59151f35ee1f6..be8c45bfe3cb3135ff762d41fa6f7b85d3a3ee9f 100644 (file)
--- a/app/models/canonical_email_block.rb
+++ b/app/models/canonical_email_block.rb
@@ -15,7 +15,7 @@ class CanonicalEmailBlock < ApplicationRecord
 
   belongs_to :reference_account, class_name: 'Account'
 
-  validates :canonical_email_hash, presence: true
+  validates :canonical_email_hash, presence: true, uniqueness: true
 
   def email=(email)
     self.canonical_email_hash = email_to_canonical_email_hash(email)

diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
index 03d6f5fb0f547abceafaa2c27f42a30d6980a9d0..65e6714c0e7edf9c91d489a207b0f70d1b29c17f 100644 (file)
--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@@ -5,6 +5,37 @@ RSpec.describe Account, type: :model do
     let(:bob) { Fabricate(:account, username: 'bob') }
     subject { Fabricate(:account) }
 
+    describe '#suspend!' do
+      it 'marks the account as suspended' do
+        subject.suspend!
+        expect(subject.suspended?).to be true
+      end
+
+      it 'creates a deletion request' do
+        subject.suspend!
+        expect(AccountDeletionRequest.where(account: subject).exists?).to be true
+      end
+
+      context 'when the account is of a local user' do
+        let!(:subject) { Fabricate(:account, user: Fabricate(:user, email: 'foo+bar@domain.org')) }
+
+        it 'creates a canonical domain block' do
+          subject.suspend!
+          expect(CanonicalEmailBlock.block?(subject.user_email)).to be true
+        end
+
+        context 'when a canonical domain block already exists for that email' do
+          before do
+            Fabricate(:canonical_email_block, email: subject.user_email)
+          end
+
+          it 'does not raise an error' do
+            expect { subject.suspend! }.not_to raise_error
+          end
+        end
+      end
+    end
+
     describe '#follow!' do
       it 'creates a follow' do
         follow = subject.follow!(bob)