Merge branch '62980-username-availability-checker-breaks-inline-validation' into 'master'

Resolve "Username availability checker breaks inline validation"

Closes #62980

See merge request gitlab-org/gitlab-ce!29678

5 files changed, 98 insertions, 109 deletions

diff --git a/app/assets/javascripts/pages/sessions/new/index.js b/app/assets/javascripts/pages/sessions/new/index.js
index 3f5a3e15c2c..55bc93a2b13 100644
--- a/app/assets/javascripts/pages/sessions/new/index.js
+++ b/app/assets/javascripts/pages/sessions/new/index.js
@@ -7,8 +7,8 @@ import OAuthRememberMe from './oauth_remember_me';
 import preserveUrlFragment from './preserve_url_fragment';
 
 document.addEventListener('DOMContentLoaded', () => {
-  new LengthValidator(); // eslint-disable-line no-new
   new UsernameValidator(); // eslint-disable-line no-new
+  new LengthValidator(); // eslint-disable-line no-new
   new SigninTabsMemoizer(); // eslint-disable-line no-new
   new NoEmojiValidator(); // eslint-disable-line no-new
 
diff --git a/app/assets/javascripts/pages/sessions/new/username_validator.js b/app/assets/javascripts/pages/sessions/new/username_validator.js
index 7a41805bada..36d1e773134 100644
--- a/app/assets/javascripts/pages/sessions/new/username_validator.js
+++ b/app/assets/javascripts/pages/sessions/new/username_validator.js
@@ -1,133 +1,79 @@
-/* eslint-disable consistent-return, class-methods-use-this */
+import InputValidator from '~/validators/input_validator';
 
-import $ from 'jquery';
 import _ from 'underscore';
 import axios from '~/lib/utils/axios_utils';
 import flash from '~/flash';
 import { __ } from '~/locale';
 
 const debounceTimeoutDuration = 1000;
+const rootUrl = gon.relative_url_root;
 const invalidInputClass = 'gl-field-error-outline';
 const successInputClass = 'gl-field-success-outline';
-const unavailableMessageSelector = '.username .validation-error';
-const successMessageSelector = '.username .validation-success';
-const pendingMessageSelector = '.username .validation-pending';
-const invalidMessageSelector = '.username .gl-field-error';
+const successMessageSelector = '.validation-success';
+const pendingMessageSelector = '.validation-pending';
+const unavailableMessageSelector = '.validation-error';
 
-export default class UsernameValidator {
-  constructor() {
-    this.inputElement = $('#new_user_username');
-    this.inputDomElement = this.inputElement.get(0);
-    this.state = {
-      available: false,
-      valid: false,
-      pending: false,
-      empty: true,
-    };
+export default class UsernameValidator extends InputValidator {
+  constructor(opts = {}) {
+    super();
 
-    const debounceTimeout = _.debounce(username => {
-      this.validateUsername(username);
-    }, debounceTimeoutDuration);
-
-    this.inputElement.on('keyup.username_check', () => {
-      const username = this.inputElement.val();
-
-      this.state.valid = this.inputDomElement.validity.valid;
-      this.state.empty = !username.length;
+    const container = opts.container || '';
+    const validateLengthElements = document.querySelectorAll(`${container} .js-validate-username`);
 
-      if (this.state.valid) {
-        return debounceTimeout(username);
-      }
-
-      this.renderState();
-    });
+    this.debounceValidateInput = _.debounce(inputDomElement => {
+      UsernameValidator.validateUsernameInput(inputDomElement);
+    }, debounceTimeoutDuration);
 
-    // Override generic field validation
-    this.inputElement.on('invalid', this.interceptInvalid.bind(this));
+    validateLengthElements.forEach(element =>
+      element.addEventListener('input', this.eventHandler.bind(this)),
+    );
   }
 
-  renderState() {
-    // Clear all state
-    this.clearFieldValidationState();
-
-    if (this.state.valid && this.state.available) {
-      return this.setSuccessState();
-    }
-
-    if (this.state.empty) {
-      return this.clearFieldValidationState();
-    }
-
-    if (this.state.pending) {
-      return this.setPendingState();
-    }
+  eventHandler(event) {
+    const inputDomElement = event.target;
 
-    if (!this.state.valid) {
-      return this.setInvalidState();
-    }
-
-    if (!this.state.available) {
-      return this.setUnavailableState();
-    }
-  }
-
-  interceptInvalid(event) {
-    event.preventDefault();
-    event.stopPropagation();
+    UsernameValidator.resetInputState(inputDomElement);
+    this.debounceValidateInput(inputDomElement);
   }
 
-  validateUsername(username) {
-    if (this.state.valid) {
-      this.state.pending = true;
-      this.state.available = false;
-      this.renderState();
-      axios
-        .get(`${gon.relative_url_root}/users/${username}/exists`)
-        .then(({ data }) => this.setAvailabilityState(data.exists))
+  static validateUsernameInput(inputDomElement) {
+    const username = inputDomElement.value;
+
+    if (inputDomElement.checkValidity() && username.length > 0) {
+      UsernameValidator.setMessageVisibility(inputDomElement, pendingMessageSelector);
+      UsernameValidator.fetchUsernameAvailability(username)
+        .then(usernameTaken => {
+          UsernameValidator.setInputState(inputDomElement, !usernameTaken);
+          UsernameValidator.setMessageVisibility(inputDomElement, pendingMessageSelector, false);
+          UsernameValidator.setMessageVisibility(
+            inputDomElement,
+            usernameTaken ? unavailableMessageSelector : successMessageSelector,
+          );
+        })
         .catch(() => flash(__('An error occurred while validating username')));
     }
   }
 
-  setAvailabilityState(usernameTaken) {
-    if (usernameTaken) {
-      this.state.available = false;
-    } else {
-      this.state.available = true;
-    }
-    this.state.pending = false;
-    this.renderState();
+  static fetchUsernameAvailability(username) {
+    return axios.get(`${rootUrl}/users/${username}/exists`).then(({ data }) => data.exists);
   }
 
-  clearFieldValidationState() {
-    this.inputElement.siblings('p').hide();
-
-    this.inputElement.removeClass(invalidInputClass).removeClass(successInputClass);
+  static setMessageVisibility(inputDomElement, messageSelector, isVisible = true) {
+    const messageElement = inputDomElement.parentElement.querySelector(messageSelector);
+    messageElement.classList.toggle('hide', !isVisible);
   }
 
-  setUnavailableState() {
-    const $usernameUnavailableMessage = this.inputElement.siblings(unavailableMessageSelector);
-    this.inputElement.addClass(invalidInputClass).removeClass(successInputClass);
-    $usernameUnavailableMessage.show();
+  static setInputState(inputDomElement, success = true) {
+    inputDomElement.classList.toggle(successInputClass, success);
+    inputDomElement.classList.toggle(invalidInputClass, !success);
   }
 
-  setSuccessState() {
-    const $usernameSuccessMessage = this.inputElement.siblings(successMessageSelector);
-    this.inputElement.addClass(successInputClass).removeClass(invalidInputClass);
-    $usernameSuccessMessage.show();
-  }
+  static resetInputState(inputDomElement) {
+    UsernameValidator.setMessageVisibility(inputDomElement, successMessageSelector, false);
+    UsernameValidator.setMessageVisibility(inputDomElement, unavailableMessageSelector, false);
 
-  setPendingState() {
-    const $usernamePendingMessage = $(pendingMessageSelector);
-    if (this.state.pending) {
-      $usernamePendingMessage.show();
-    } else {
-      $usernamePendingMessage.hide();
+    if (inputDomElement.checkValidity()) {
+      inputDomElement.classList.remove(successInputClass, invalidInputClass);
     }
   }
-
-  setInvalidState() {
-    const $inputErrorMessage = $(invalidMessageSelector);
-    this.inputElement.addClass(invalidInputClass).removeClass(successInputClass);
-    $inputErrorMessage.show();
-  }
 }
diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml
index 5eba819172b..eae3ee6339f 100644
--- a/app/views/devise/shared/_signup_box.html.haml
+++ b/app/views/devise/shared/_signup_box.html.haml
@@ -10,10 +10,10 @@
         = f.text_field :name, class: "form-control top qa-new-user-name js-block-emoji js-validate-length", :data => { :max_length => max_name_length, :max_length_message => s_("SignUp|Name is too long (maximum is %{max_length} characters).") % { max_length: max_name_length } }, required: true, title: _("This field is required.")
       .username.form-group
         = f.label :username, class: 'label-bold'
-        = f.text_field :username, class: "form-control middle qa-new-user-username js-block-emoji js-validate-length", :data => { :max_length => max_username_length, :max_length_message => s_("SignUp|Username is too long (maximum is %{max_length} characters).") % { max_length: max_username_length } }, pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")
-        %p.validation-error.field-validation.hide= _('Username is already taken.')
-        %p.validation-success.field-validation.hide= _('Username is available.')
-        %p.validation-pending.field-validation.hide= _('Checking username availability...')
+        = f.text_field :username, class: "form-control middle qa-new-user-username js-block-emoji js-validate-length js-validate-username", :data => { :max_length => max_username_length, :max_length_message => s_("SignUp|Username is too long (maximum is %{max_length} characters).") % { max_length: max_username_length } }, pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")
+        %p.validation-error.gl-field-error-ignore.field-validation.hide= _('Username is already taken.')
+        %p.validation-success.gl-field-error-ignore.field-validation.hide= _('Username is available.')
+        %p.validation-pending.gl-field-error-ignore.field-validation.hide= _('Checking username availability...')
       .form-group
         = f.label :email, class: 'label-bold'
         = f.email_field :email, class: "form-control middle qa-new-user-email", required: true, title: _("Please provide a valid email address.")
diff --git a/changelogs/unreleased/62980-username-availability-checker-breaks-inline-validation.yml b/changelogs/unreleased/62980-username-availability-checker-breaks-inline-validation.yml
new file mode 100644
index 00000000000..7436f5d278e
--- /dev/null
+++ b/changelogs/unreleased/62980-username-availability-checker-breaks-inline-validation.yml
@@ -0,0 +1,5 @@
+---
+title: Fix the signup form's username validation messages not displaying
+merge_request: 29678
+author: Jiaan Louw
+type: fixed
diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb
index 1a9caf0ffbb..8a6901ea4e9 100644
--- a/spec/features/users/signup_spec.rb
+++ b/spec/features/users/signup_spec.rb
@@ -19,7 +19,7 @@ describe 'Signup' do
     end
 
     it 'does not show an error border if the username contains dots (.)' do
-      fill_in 'new_user_username', with: 'new.user.username'
+      simulate_input('#new_user_username', 'new.user.username')
       wait_for_requests
 
       expect(find('.username')).not_to have_css '.gl-field-error-outline'
@@ -41,7 +41,14 @@ describe 'Signup' do
       expect(find('.username')).to have_css '.gl-field-error-outline'
     end
 
-    it 'shows an  error border if the username contains special characters' do
+    it 'shows a success border if the username is available' do
+      fill_in 'new_user_username', with: 'new-user'
+      wait_for_requests
+
+      expect(find('.username')).to have_css '.gl-field-success-outline'
+    end
+
+    it 'shows an error border if the username contains special characters' do
       fill_in 'new_user_username', with: 'new$user!username'
       wait_for_requests
 
@@ -71,7 +78,7 @@ describe 'Signup' do
       expect(page).to have_content("Please create a username with only alphanumeric characters.")
     end
 
-    it 'shows an  error border if the username contains emojis' do
+    it 'shows an error border if the username contains emojis' do
       simulate_input('#new_user_username', 'ehsan😀')
 
       expect(find('.username')).to have_css '.gl-field-error-outline'
@@ -82,6 +89,37 @@ describe 'Signup' do
 
       expect(page).to have_content("Invalid input, please avoid emojis")
     end
+
+    it 'shows a pending message if the username availability is being fetched' do
+      fill_in 'new_user_username', with: 'new-user'
+
+      expect(find('.username > .validation-pending')).not_to have_css '.hide'
+    end
+
+    it 'shows a success message if the username is available' do
+      fill_in 'new_user_username', with: 'new-user'
+      wait_for_requests
+
+      expect(find('.username > .validation-success')).not_to have_css '.hide'
+    end
+
+    it 'shows an error message if the username is unavailable' do
+      existing_user = create(:user)
+
+      fill_in 'new_user_username', with: existing_user.username
+      wait_for_requests
+
+      expect(find('.username > .validation-error')).not_to have_css '.hide'
+    end
+
+    it 'shows a success message if the username is corrected and then available' do
+      fill_in 'new_user_username', with: 'new-user$'
+      wait_for_requests
+      fill_in 'new_user_username', with: 'new-user'
+      wait_for_requests
+
+      expect(page).to have_content("Username is available.")
+    end
   end
 
   describe 'user\'s full name validation', :js do