From 78eeb3e09d020ac69ba620d1959f2a8db3c1d65e Mon Sep 17 00:00:00 2001 From: Jiaan <3468028-jiaan@users.noreply.gitlab.com> Date: Wed, 19 Jun 2019 13:48:30 +0000 Subject: Resolve "Username availability checker breaks inline validation" --- app/assets/javascripts/pages/sessions/new/index.js | 2 +- .../pages/sessions/new/username_validator.js | 148 +++++++-------------- app/views/devise/shared/_signup_box.html.haml | 8 +- ...ailability-checker-breaks-inline-validation.yml | 5 + spec/features/users/signup_spec.rb | 44 +++++- 5 files changed, 98 insertions(+), 109 deletions(-) create mode 100644 changelogs/unreleased/62980-username-availability-checker-breaks-inline-validation.yml 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 -- cgit v1.2.1