FIX: Do not validate email in TL promotion (#20892)

There is no need to validate the user's emails when
promoting/demoting their trust level, this can cause
issues in things like Jobs::Tl3Promotions, we don't
need to fail in that case when all we are doing is changing
trust level.
This commit is contained in:
Martin Brennan 2023-03-30 13:52:10 +10:00 committed by GitHub
parent 795e6d72a4
commit 84ff96bd07
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 30 additions and 1 deletions

View File

@ -149,7 +149,7 @@ class User < ActiveRecord::Base
validate :name_validator, if: :will_save_change_to_name?
validates :name, user_full_name: true, if: :will_save_change_to_name?, length: { maximum: 255 }
validates :ip_address, allowed_ip_address: { on: :create, message: :signup_not_allowed }
validates :primary_email, presence: true
validates :primary_email, presence: true, unless: :skip_email_validation
validates :validatable_user_fields_values, watched_words: true, unless: :custom_fields_clean?
validates_associated :primary_email,
message: ->(_, user_email) { user_email[:value]&.errors[:email]&.first }

View File

@ -82,6 +82,7 @@ class Promotion
new_value: new_level,
)
end
@user.skip_email_validation = true
@user.save!
@user.user_profile.recook_bio
@user.user_profile.save!

View File

@ -133,5 +133,19 @@ RSpec.describe Jobs::Tl3Promotions do
run_job
expect(user.reload.trust_level).to eq(TrustLevel[3])
end
it "doesn't error if user is missing email records" do
user = nil
freeze_time 4.days.ago do
user = create_leader_user
end
user.user_emails.delete_all
TrustLevel3Requirements.any_instance.stubs(:requirements_met?).returns(false)
TrustLevel3Requirements.any_instance.stubs(:requirements_lost?).returns(true)
run_job
expect(user.reload.trust_level).to eq(TrustLevel[2])
end
end
end

View File

@ -273,4 +273,18 @@ RSpec.describe Promotion do
end
end
end
describe "#change_trust_level!" do
fab!(:user) { Fabricate(:user, trust_level: TrustLevel[0]) }
let(:promotion) { Promotion.new(user) }
context "when the user has no emails" do
before { user.user_emails.delete_all }
it "does not error" do
expect { promotion.change_trust_level!(TrustLevel[1]) }.not_to raise_error
expect(user.reload.trust_level).to eq(TrustLevel[1])
end
end
end
end