From 85d4b60a454ad6e7518ceb7f9c25a379fa75c9fd Mon Sep 17 00:00:00 2001
From: Dan Ungureanu <dan@ungureanu.me>
Date: Thu, 15 Apr 2021 14:46:32 +0300
Subject: [PATCH] FIX: Improve error messages for invites (#12714)
The error messages used to include an unnecessary 'Validation failed:
Email' prefix which was removed.
---
app/controllers/invites_controller.rb | 6 ++++--
app/models/invite.rb | 2 +-
config/locales/server.en.yml | 1 +
lib/validators/email_validator.rb | 10 ++++++++--
spec/models/invite_spec.rb | 4 ++--
5 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index cd1a25ab1b2..9caa9a1beb2 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -104,8 +104,10 @@ class InvitesController < ApplicationController
else
render json: failed_json, status: 422
end
- rescue Invite::UserExists, ActiveRecord::RecordInvalid => e
+ rescue Invite::UserExists => e
render_json_error(e.message)
+ rescue ActiveRecord::RecordInvalid => e
+ render_json_error(e.record.errors.full_messages.first)
end
end
@@ -175,7 +177,7 @@ class InvitesController < ApplicationController
begin
invite.update!(params.permit(:email, :custom_message, :max_redemptions_allowed, :expires_at))
rescue ActiveRecord::RecordInvalid => e
- return render_json_error(e.message)
+ return render_json_error(e.record.errors.full_messages.first)
end
end
diff --git a/app/models/invite.rb b/app/models/invite.rb
index 04ce1309e71..2a2c1d8404d 100644
--- a/app/models/invite.rb
+++ b/app/models/invite.rb
@@ -62,7 +62,7 @@ class Invite < ActiveRecord::Base
if user && user.id != self.invited_users&.first&.user_id
@email_already_exists = true
- errors.add(:email, I18n.t(
+ errors.add(:base, I18n.t(
"invite.user_exists",
email: email,
username: user.username,
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 1c044fcd366..b17c72222b6 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -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>
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>"
+ invalid_email: "%{email} isn't a valid email address."
confirm_email: "<p>You’re 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 doesn’t 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."
disabled_errors:
diff --git a/lib/validators/email_validator.rb b/lib/validators/email_validator.rb
index 47200da2b9f..eca8c28e5f5 100644
--- a/lib/validators/email_validator.rb
+++ b/lib/validators/email_validator.rb
@@ -4,14 +4,20 @@ class EmailValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
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
unless EmailValidator.allowed?(value)
record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
+ invalid = true
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'))
end
end
diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb
index fd722a15422..91316b4a86d 100644
--- a/spec/models/invite_spec.rb
+++ b/spec/models/invite_spec.rb
@@ -17,7 +17,7 @@ describe Invite do
it 'does not allow invites with invalid emails' do
invite = Fabricate.build(:invite, email: 'John Doe <john.doe@example.com>')
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
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
invite = Fabricate.build(:invite, email: 'asjdso')
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