UX: Improve error messages for minimum and maximum username lengths.

config/locales/server.en.yml

@@ -163,10 +163,6 @@ en:
       invalid_choice:
         one: 'You specified the invalid choice %{name}'
         other: 'You specified the invalid choices %{name}'
-      min_username_length_exists: "You cannot set the minimum username length above the shortest username."
-      min_username_length_range: "You cannot set the minimum above the maximum."
-      max_username_length_exists: "You cannot set the maximum username length below the longest username."
-      max_username_length_range: "You cannot set the maximum below the minimum."
       default_categories_already_selected: "You cannot select a category used in another list."
       s3_upload_bucket_is_required: "You cannot enable uploads to S3 unless you've provided the 's3_upload_bucket'."
     conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to <br><a href="https://meta.discourse.org/t/76575">https://meta.discourse.org/t/76575</a>'
@@ -1839,6 +1835,10 @@ en:
       reply_by_email_disabled: "You must first enable 'reply by email' before enabling this setting."
       sso_url_is_empty: "You must set a 'sso url' before enabling this setting."
       enable_local_logins_disabled: "You must first enable 'enable local logins' before enabling this setting."
+      min_username_length_exists: "You cannot set the minimum username length above the shortest username (%{username})."
+      min_username_length_range: "You cannot set the minimum above the maximum."
+      max_username_length_exists: "You cannot set the maximum username length below the longest username (%{username})."
+      max_username_length_range: "You cannot set the maximum below the minimum."
 
   search:
     within_post: "#%{post_number} by %{username}"

config/site_settings.yml

@@ -375,11 +375,13 @@ users:
     default: 3
     min: 1
     max: 60
+    validator: "MinUsernameLengthValidator"
   max_username_length:
     client: true
     default: 20
     min: 8
     max: 60
+    validator: "MaxUsernameLengthValidator"
   reserved_usernames:
     type: list
     list_type: compact

lib/site_settings/validations.rb

@@ -5,16 +5,6 @@ module SiteSettings::Validations
     raise Discourse::InvalidParameters.new(I18n.t("errors.site_settings.#{key}"))
   end
 
-  def validate_min_username_length(new_val)
-    validate_error :min_username_length_range if new_val > SiteSetting.max_username_length
-    validate_error :min_username_length_exists if User.where('length(username) < ?', new_val).exists?
-  end
-
-  def validate_max_username_length(new_val)
-    validate_error :min_username_length_range if new_val < SiteSetting.min_username_length
-    validate_error :max_username_length_exists if User.where('length(username) > ?', new_val).exists?
-  end
-
   def validate_default_categories(new_val, default_categories_selected)
     validate_error :default_categories_already_selected if (new_val.split("|").to_set & default_categories_selected).size > 0
   end

lib/validators/max_username_length_validator.rb

@@ -0,0 +1,19 @@
+class MaxUsernameLengthValidator
+  def initialize(opts = {})
+    @opts = opts
+  end
+
+  def valid_value?(value)
+    return false if value < SiteSetting.min_username_length
+    @username = User.where('length(username) > ?', value).pluck(:username).first
+    @username.blank? ? true : false
+  end
+
+  def error_message
+    if @username.blank?
+      I18n.t("site_settings.errors.max_username_length_range")
+    else
+      I18n.t("site_settings.errors.max_username_length_exists", username: @username)
+    end
+  end
+end

lib/validators/min_username_length_validator.rb

@@ -0,0 +1,19 @@
+class MinUsernameLengthValidator
+  def initialize(opts = {})
+    @opts = opts
+  end
+
+  def valid_value?(value)
+    return false if value > SiteSetting.max_username_length
+    @username = User.where('length(username) < ?', value).pluck(:username).first
+    @username.blank? ? true : false
+  end
+
+  def error_message
+    if @username.blank?
+      I18n.t("site_settings.errors.min_username_length_range")
+    else
+      I18n.t("site_settings.errors.min_username_length_exists", username: @username)
+    end
+  end
+end

spec/components/validators/max_username_length_validator_spec.rb

@@ -0,0 +1,26 @@
+require 'rails_helper'
+
+describe MaxUsernameLengthValidator do
+  it "checks for minimum range" do
+    SiteSetting.min_username_length = 6
+
+    validator = described_class.new
+    expect(validator.valid_value?(5)).to eq(false)
+    expect(validator.error_message).to eq(I18n.t("site_settings.errors.max_username_length_range"))
+  end
+
+  it "checks for users with short usernames" do
+    user = Fabricate(:user, username: 'jackjackjack')
+
+    validator = described_class.new
+    expect(validator.valid_value?(12)).to eq(true)
+
+    validator = described_class.new
+    expect(validator.valid_value?(11)).to eq(false)
+
+    expect(validator.error_message).to eq(I18n.t(
+      "site_settings.errors.max_username_length_exists",
+      username: 'jackjackjack'
+    ))
+  end
+end

spec/components/validators/min_username_length_validator_spec.rb

@@ -0,0 +1,26 @@
+require 'rails_helper'
+
+describe MinUsernameLengthValidator do
+  it "checks for maximum range" do
+    SiteSetting.max_username_length = 10
+
+    validator = described_class.new
+    expect(validator.valid_value?(11)).to eq(false)
+    expect(validator.error_message).to eq(I18n.t("site_settings.errors.min_username_length_range"))
+  end
+
+  it "checks for users with short usernames" do
+    user = Fabricate(:user, username: 'jack')
+
+    validator = described_class.new
+    expect(validator.valid_value?(4)).to eq(true)
+
+    validator = described_class.new
+    expect(validator.valid_value?(5)).to eq(false)
+
+    expect(validator.error_message).to eq(I18n.t(
+      "site_settings.errors.min_username_length_exists",
+      username: 'jack'
+    ))
+  end
+end