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
This commit is contained in:
parent
e5113a8cad
commit
3c18311d86
2 changed files with 32 additions and 1 deletions
|
@ -15,7 +15,7 @@ class CanonicalEmailBlock < ApplicationRecord
|
||||||
|
|
||||||
belongs_to :reference_account, class_name: 'Account'
|
belongs_to :reference_account, class_name: 'Account'
|
||||||
|
|
||||||
validates :canonical_email_hash, presence: true
|
validates :canonical_email_hash, presence: true, uniqueness: true
|
||||||
|
|
||||||
def email=(email)
|
def email=(email)
|
||||||
self.canonical_email_hash = email_to_canonical_email_hash(email)
|
self.canonical_email_hash = email_to_canonical_email_hash(email)
|
||||||
|
|
|
@ -5,6 +5,37 @@ RSpec.describe Account, type: :model do
|
||||||
let(:bob) { Fabricate(:account, username: 'bob') }
|
let(:bob) { Fabricate(:account, username: 'bob') }
|
||||||
subject { Fabricate(:account) }
|
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
|
describe '#follow!' do
|
||||||
it 'creates a follow' do
|
it 'creates a follow' do
|
||||||
follow = subject.follow!(bob)
|
follow = subject.follow!(bob)
|
||||||
|
|
Loading…
Reference in a new issue