Add `details` to error response for `POST /api/v1/accounts` in REST API (#15803)

12 files changed, 72 insertions, 23 deletions

diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index 953874e1a..996f1b79b 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -27,6 +27,8 @@ class Api::V1::AccountsController < Api::BaseController
 
     self.response_body = Oj.dump(response.body)
     self.status        = response.status
+  rescue ActiveRecord::RecordInvalid => e
+    render json: ValidationErrorFormatter.new(e, :'account.username' => :username, :'invite_request.text' => :reason).as_json, status: :unprocessable_entity
   end
 
   def follow
diff --git a/app/lib/validation_error_formatter.rb b/app/lib/validation_error_formatter.rb
new file mode 100644
index 000000000..3f964f739
--- /dev/null
+++ b/app/lib/validation_error_formatter.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class ValidationErrorFormatter
+  def initialize(error, aliases = {})
+    @error   = error
+    @aliases = aliases
+  end
+
+  def as_json
+    { error: @error.to_s, details: details }
+  end
+
+  private
+
+  def details
+    h = {}
+
+    errors.details.each_pair do |attribute_name, attribute_errors|
+      messages = errors.messages[attribute_name]
+
+      h[@aliases[attribute_name] || attribute_name] = attribute_errors.map.with_index do |error, index|
+        { error: 'ERR_' + error[:error].to_s.upcase, description: messages[index] }
+      end
+    end
+
+    h
+  end
+
+  def errors
+    @errors ||= @error.record.errors
+  end
+end
diff --git a/app/validators/blacklisted_email_validator.rb b/app/validators/blacklisted_email_validator.rb
index 20a1587cc..1ca73fdcc 100644
--- a/app/validators/blacklisted_email_validator.rb
+++ b/app/validators/blacklisted_email_validator.rb
@@ -2,11 +2,11 @@
 
 class BlacklistedEmailValidator < ActiveModel::Validator
   def validate(user)
-    return if user.valid_invitation?
+    return if user.valid_invitation? || user.email.blank?
 
     @email = user.email
 
-    user.errors.add(:email, I18n.t('users.blocked_email_provider')) if blocked_email?
+    user.errors.add(:email, :blocked) if blocked_email?
   end
 
   private
diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb
index ef1554494..9f70a1469 100644
--- a/app/validators/email_mx_validator.rb
+++ b/app/validators/email_mx_validator.rb
@@ -4,16 +4,19 @@ require 'resolv'
 
 class EmailMxValidator < ActiveModel::Validator
   def validate(user)
+    return if user.email.blank?
+
     domain = get_domain(user.email)
 
-    if domain.nil?
-      user.errors.add(:email, I18n.t('users.invalid_email'))
+    if domain.blank?
+      user.errors.add(:email, :invalid)
     else
       ips, hostnames = resolve_mx(domain)
+
       if ips.empty?
-        user.errors.add(:email, I18n.t('users.invalid_email_mx'))
+        user.errors.add(:email, :unreachable)
       elsif on_blacklist?(hostnames + ips)
-        user.errors.add(:email, I18n.t('users.blocked_email_provider'))
+        user.errors.add(:email, :blocked)
       end
     end
   end
diff --git a/app/validators/note_length_validator.rb b/app/validators/note_length_validator.rb
index 5ff6df6df..7ea2bb3e5 100644
--- a/app/validators/note_length_validator.rb
+++ b/app/validators/note_length_validator.rb
@@ -2,7 +2,7 @@
 
 class NoteLengthValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
-    record.errors.add(attribute, I18n.t('statuses.over_character_limit', max: options[:maximum])) if too_long?(value)
+    record.errors.add(attribute, :too_long, message: I18n.t('statuses.over_character_limit', max: options[:maximum]), count: options[:maximum]) if too_long?(value)
   end
 
   private
diff --git a/app/validators/unique_username_validator.rb b/app/validators/unique_username_validator.rb
index f87eb06ba..09c8fadb5 100644
--- a/app/validators/unique_username_validator.rb
+++ b/app/validators/unique_username_validator.rb
@@ -4,7 +4,7 @@
 
 class UniqueUsernameValidator < ActiveModel::Validator
   def validate(account)
-    return if account.username.nil?
+    return if account.username.blank?
 
     normalized_username = account.username.downcase
     normalized_domain = account.domain&.downcase
diff --git a/app/validators/unreserved_username_validator.rb b/app/validators/unreserved_username_validator.rb
index 634ceb06e..974f3ba62 100644
--- a/app/validators/unreserved_username_validator.rb
+++ b/app/validators/unreserved_username_validator.rb
@@ -3,9 +3,10 @@
 class UnreservedUsernameValidator < ActiveModel::Validator
   def validate(account)
     @username = account.username
-    return if @username.nil?
 
-    account.errors.add(:username, I18n.t('accounts.reserved_username')) if reserved_username?
+    return if @username.blank?
+
+    account.errors.add(:username, :reserved) if reserved_username?
   end
 
   private
diff --git a/config/locales/activerecord.en.yml b/config/locales/activerecord.en.yml
index 8533418cc..ec8dad1b1 100644
--- a/config/locales/activerecord.en.yml
+++ b/config/locales/activerecord.en.yml
@@ -5,13 +5,28 @@ en:
       poll:
         expires_at: Deadline
         options: Choices
+      user:
+        agreement: Service agreement
+        email: E-mail address
+        locale: Locale
+        password: Password
+      user/account:
+        username: Username
+      user/invite_request:
+        text: Reason
     errors:
       models:
         account:
           attributes:
             username:
-              invalid: only letters, numbers and underscores
+              invalid: must contain only letters, numbers and underscores
+              reserved: is reserved
         status:
           attributes:
             reblog:
               taken: of status already exists
+        user:
+          attributes:
+            email:
+              blocked: uses a disallowed e-mail provider
+              unreachable: does not seem to exist
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 0c38c5ae1..beb568346 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -80,7 +80,6 @@ en:
       other: Toots
     posts_tab_heading: Toots
     posts_with_replies: Toots and replies
-    reserved_username: The username is reserved
     roles:
       admin: Admin
       bot: Bot
@@ -1410,11 +1409,8 @@ en:
       tips: Tips
       title: Welcome aboard, %{name}!
   users:
-    blocked_email_provider: This e-mail provider isn't allowed
     follow_limit_reached: You cannot follow more than %{limit} people
     generic_access_help_html: Trouble accessing your account? You may get in touch with %{email} for assistance
-    invalid_email: The e-mail address is invalid
-    invalid_email_mx: The e-mail address does not seem to exist
     invalid_otp_token: Invalid two-factor code
     invalid_sign_in_token: Invalid security code
     otp_lost_help_html: If you lost access to both, you may get in touch with %{email}
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index d3a9a11eb..d03ae51e8 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/spec/controllers/auth/sessions_controller_spec.rb
@@ -69,7 +69,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
         end
 
         it 'shows a login error' do
-          expect(flash[:alert]).to match I18n.t('devise.failure.invalid', authentication_keys: 'Email')
+          expect(flash[:alert]).to match I18n.t('devise.failure.invalid', authentication_keys: I18n.t('activerecord.attributes.user.email'))
         end
 
         it "doesn't log the user in" do
@@ -136,7 +136,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
         end
 
         it 'shows a login error' do
-          expect(flash[:alert]).to match I18n.t('devise.failure.invalid', authentication_keys: 'Email')
+          expect(flash[:alert]).to match I18n.t('devise.failure.invalid', authentication_keys: I18n.t('activerecord.attributes.user.email'))
         end
 
         it "doesn't log the user in" do
diff --git a/spec/validators/blacklisted_email_validator_spec.rb b/spec/validators/blacklisted_email_validator_spec.rb
index f0708dc46..53b355a57 100644
--- a/spec/validators/blacklisted_email_validator_spec.rb
+++ b/spec/validators/blacklisted_email_validator_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do
       let(:blocked_email) { true }
 
       it 'calls errors.add' do
-        expect(errors).to have_received(:add).with(:email, I18n.t('users.blocked_email_provider'))
+        expect(errors).to have_received(:add).with(:email, :blocked)
       end
     end
 
@@ -25,7 +25,7 @@ RSpec.describe BlacklistedEmailValidator, type: :validator do
       let(:blocked_email) { false }
 
       it 'not calls errors.add' do
-        expect(errors).not_to have_received(:add).with(:email, I18n.t('users.blocked_email_provider'))
+        expect(errors).not_to have_received(:add).with(:email, :blocked)
       end
     end
   end
diff --git a/spec/validators/unreserved_username_validator_spec.rb b/spec/validators/unreserved_username_validator_spec.rb
index 0187941b0..cabd6d386 100644
--- a/spec/validators/unreserved_username_validator_spec.rb
+++ b/spec/validators/unreserved_username_validator_spec.rb
@@ -13,7 +13,7 @@ RSpec.describe UnreservedUsernameValidator, type: :validator do
     let(:account)   { double(username: username, errors: errors) }
     let(:errors )   { double(add: nil) }
 
-    context '@username.nil?' do
+    context '@username.blank?' do
       let(:username)  { nil }
 
       it 'not calls errors.add' do
@@ -21,14 +21,14 @@ RSpec.describe UnreservedUsernameValidator, type: :validator do
       end
     end
 
-    context '!@username.nil?' do
-      let(:username)  { '' }
+    context '!@username.blank?' do
+      let(:username)  { 'f' }
 
       context 'reserved_username?' do
         let(:reserved_username) { true }
 
         it 'calls erros.add' do
-          expect(errors).to have_received(:add).with(:username, I18n.t('accounts.reserved_username'))
+          expect(errors).to have_received(:add).with(:username, :reserved)
         end
       end