FIX: Improve error messages for invites (#12714)

The error messages used to include an unnecessary 'Validation failed:
Email' prefix which was removed.
This commit is contained in:
Dan Ungureanu 2021-04-15 14:46:32 +03:00 committed by GitHub
parent c60668a052
commit 85d4b60a45
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 16 additions and 7 deletions

View File

@ -104,8 +104,10 @@ class InvitesController < ApplicationController
else else
render json: failed_json, status: 422 render json: failed_json, status: 422
end end
rescue Invite::UserExists, ActiveRecord::RecordInvalid => e rescue Invite::UserExists => e
render_json_error(e.message) render_json_error(e.message)
rescue ActiveRecord::RecordInvalid => e
render_json_error(e.record.errors.full_messages.first)
end end
end end
@ -175,7 +177,7 @@ class InvitesController < ApplicationController
begin begin
invite.update!(params.permit(:email, :custom_message, :max_redemptions_allowed, :expires_at)) invite.update!(params.permit(:email, :custom_message, :max_redemptions_allowed, :expires_at))
rescue ActiveRecord::RecordInvalid => e rescue ActiveRecord::RecordInvalid => e
return render_json_error(e.message) return render_json_error(e.record.errors.full_messages.first)
end end
end end

View File

@ -62,7 +62,7 @@ class Invite < ActiveRecord::Base
if user && user.id != self.invited_users&.first&.user_id if user && user.id != self.invited_users&.first&.user_id
@email_already_exists = true @email_already_exists = true
errors.add(:email, I18n.t( errors.add(:base, I18n.t(
"invite.user_exists", "invite.user_exists",
email: email, email: email,
username: user.username, username: user.username,

View File

@ -241,6 +241,7 @@ en:
<p>The invitation to <a href="%{base_url}">%{site_name}</a> can no longer be redeemed. Please ask the person who invited you to send you a new invitation.</p> <p>The invitation to <a href="%{base_url}">%{site_name}</a> can no longer be redeemed. Please ask the person who invited you to send you a new invitation.</p>
error_message: "There was an error accepting invite. Please contact the site's administrator." error_message: "There was an error accepting invite. Please contact the site's administrator."
user_exists: "There's no need to invite <b>%{email}</b>, they <a href='%{base_path}/u/%{username}/summary'>already have an account!</a>" user_exists: "There's no need to invite <b>%{email}</b>, they <a href='%{base_path}/u/%{username}/summary'>already have an account!</a>"
invalid_email: "%{email} isn't a valid email address."
confirm_email: "<p>Youre almost done! We sent an activation mail to your email address. Please follow the instructions in the mail to activate your account.</p><p>If it doesnt arrive, check your spam folder.</p>" confirm_email: "<p>Youre almost done! We sent an activation mail to your email address. Please follow the instructions in the mail to activate your account.</p><p>If it doesnt arrive, check your spam folder.</p>"
cant_invite_to_group: "You are not allowed to invite users to specified group(s). Make sure you are owner of the group(s) you are trying to invite to." cant_invite_to_group: "You are not allowed to invite users to specified group(s). Make sure you are owner of the group(s) you are trying to invite to."
disabled_errors: disabled_errors:

View File

@ -4,14 +4,20 @@ class EmailValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
unless value =~ EmailValidator.email_regex unless value =~ EmailValidator.email_regex
record.errors.add(attribute, I18n.t(:'user.email.invalid')) if Invite === record && attribute == :email
record.errors.add(:base, I18n.t(:'invite.invalid_email', email: value))
else
record.errors.add(attribute, I18n.t(:'user.email.invalid'))
end
invalid = true
end end
unless EmailValidator.allowed?(value) unless EmailValidator.allowed?(value)
record.errors.add(attribute, I18n.t(:'user.email.not_allowed')) record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
invalid = true
end end
if record.errors[attribute].blank? && value && ScreenedEmail.should_block?(value) if !invalid && ScreenedEmail.should_block?(value)
record.errors.add(attribute, I18n.t(:'user.email.blocked')) record.errors.add(attribute, I18n.t(:'user.email.blocked'))
end end
end end

View File

@ -17,7 +17,7 @@ describe Invite do
it 'does not allow invites with invalid emails' do it 'does not allow invites with invalid emails' do
invite = Fabricate.build(:invite, email: 'John Doe <john.doe@example.com>') invite = Fabricate.build(:invite, email: 'John Doe <john.doe@example.com>')
expect(invite.valid?).to eq(false) expect(invite.valid?).to eq(false)
expect(invite.errors.details[:email].first[:error]).to eq(I18n.t('user.email.invalid')) expect(invite.errors.full_messages).to include(I18n.t('invite.invalid_email', email: invite.email))
end end
it 'does not allow an invite with the same email as an existing user' do it 'does not allow an invite with the same email as an existing user' do
@ -36,7 +36,7 @@ describe Invite do
it 'does not allow an invalid email address' do it 'does not allow an invalid email address' do
invite = Fabricate.build(:invite, email: 'asjdso') invite = Fabricate.build(:invite, email: 'asjdso')
expect(invite.valid?).to eq(false) expect(invite.valid?).to eq(false)
expect(invite.errors.details[:email].first[:error]).to eq(I18n.t('user.email.invalid')) expect(invite.errors.full_messages).to include(I18n.t('invite.invalid_email', email: invite.email))
end end
end end