Commit 0a3578e2 authored by Kushal Pandya's avatar Kushal Pandya

Merge branch '224455-remove-automated-user-name-create-on-trial-registration' into 'master'

Remove automated username suggestion on trial registration

See merge request gitlab-org/gitlab!78686
parents 7c5aaf81 f8664743
...@@ -170,7 +170,6 @@ Rails.application.routes.draw do ...@@ -170,7 +170,6 @@ Rails.application.routes.draw do
Gitlab.ee do Gitlab.ee do
draw :security draw :security
draw :smartcard draw :smartcard
draw :username
draw :trial draw :trial
draw :trial_registration draw :trial_registration
draw :country draw :country
......
...@@ -5,12 +5,10 @@ import NoEmojiValidator from '~/emoji/no_emoji_validator'; ...@@ -5,12 +5,10 @@ import NoEmojiValidator from '~/emoji/no_emoji_validator';
import LengthValidator from '~/pages/sessions/new/length_validator'; import LengthValidator from '~/pages/sessions/new/length_validator';
import SigninTabsMemoizer from '~/pages/sessions/new/signin_tabs_memoizer'; import SigninTabsMemoizer from '~/pages/sessions/new/signin_tabs_memoizer';
import UsernameValidator from '~/pages/sessions/new/username_validator'; import UsernameValidator from '~/pages/sessions/new/username_validator';
import UsernameSuggester from 'ee/trial_registrations/username_suggester';
new UsernameValidator(); // eslint-disable-line no-new new UsernameValidator(); // eslint-disable-line no-new
new LengthValidator(); // eslint-disable-line no-new new LengthValidator(); // eslint-disable-line no-new
new SigninTabsMemoizer(); // eslint-disable-line no-new new SigninTabsMemoizer(); // eslint-disable-line no-new
new NoEmojiValidator(); // eslint-disable-line no-new new NoEmojiValidator(); // eslint-disable-line no-new
new UsernameSuggester('new_user_username', ['new_user_first_name', 'new_user_last_name']); // eslint-disable-line no-new
trackFreeTrialAccountSubmissions(); trackFreeTrialAccountSubmissions();
import { debounce } from 'lodash';
import createFlash from '~/flash';
import axios from '~/lib/utils/axios_utils';
import { __ } from '~/locale';
const USERNAME_SUGGEST_DEBOUNCE_TIME = 300;
export default class UsernameSuggester {
/**
* Creates an instance of UsernameSuggester.
* @param {string} targetElement target input element id for suggested username
* @param {string[]} sourceElementsIds array of HTML input element ids used for generating username
*/
constructor(targetElement, sourceElementsIds = []) {
if (!targetElement) {
throw new Error("Required argument 'targetElement' is missing");
}
this.usernameElement = document.getElementById(targetElement);
if (!this.usernameElement) {
// eslint-disable-next-line @gitlab/require-i18n-strings
throw new Error('The target element is missing.');
}
this.apiPath = this.usernameElement.dataset.apiPath;
if (!this.apiPath) {
// eslint-disable-next-line @gitlab/require-i18n-strings
throw new Error('The API path was not specified.');
}
this.sourceElements = sourceElementsIds
.map((id) => document.getElementById(id))
.filter(Boolean);
this.isLoading = false;
this.debouncedSuggestWrapper = debounce(
this.suggestUsername.bind(this),
USERNAME_SUGGEST_DEBOUNCE_TIME,
);
this.bindEvents();
this.cleanupWrapper = this.cleanup.bind(this);
window.addEventListener('beforeunload', this.cleanupWrapper);
}
bindEvents() {
this.sourceElements.forEach((sourceElement) => {
sourceElement.addEventListener('change', this.debouncedSuggestWrapper);
});
}
suggestUsername() {
if (this.isLoading) {
return;
}
const name = this.joinSources();
if (!name) {
return;
}
axios
.get(this.apiPath, { params: { name } })
.then(({ data }) => {
this.usernameElement.value = data.username;
})
.catch(() => {
createFlash({
message: __('An error occurred while generating a username. Please try again.'),
});
})
.finally(() => {
this.isLoading = false;
});
}
/**
* Joins values from HTML input elements to a string separated by `_` (underscore).
*/
joinSources() {
return this.sourceElements
.map((el) => el.value)
.filter(Boolean)
.join('_');
}
cleanup() {
window.removeEventListener('beforeunload', this.cleanupWrapper);
this.sourceElements.forEach((sourceElement) =>
sourceElement.removeEventListener('change', this.debouncedSuggestWrapper),
);
}
}
# frozen_string_literal: true
class UsernamesController < ApplicationController
skip_before_action :authenticate_user!, only: [:suggest]
feature_category :users
def suggest
if validate_params
username = ::User.username_suggestion(params[:name])
render json: { username: username }, status: :ok
else
render json: { message: 'Invalid input provided' }, status: :bad_request
end
end
private
def validate_params
!params[:name].blank?
end
end
...@@ -3,12 +3,6 @@ ...@@ -3,12 +3,6 @@
module EE module EE
module RegistrationsHelper module RegistrationsHelper
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
extend ::Gitlab::Utils::Override
override :signup_username_data_attributes
def signup_username_data_attributes
super.merge(api_path: suggestion_path)
end
def shuffled_registration_objective_options def shuffled_registration_objective_options
options = registration_objective_options options = registration_objective_options
......
...@@ -13,7 +13,6 @@ module EE ...@@ -13,7 +13,6 @@ module EE
DEFAULT_ROADMAP_LAYOUT = 'months' DEFAULT_ROADMAP_LAYOUT = 'months'
DEFAULT_GROUP_VIEW = 'details' DEFAULT_GROUP_VIEW = 'details'
MAX_USERNAME_SUGGESTION_ATTEMPTS = 15
prepended do prepended do
include UsageStatistics include UsageStatistics
...@@ -141,19 +140,6 @@ module EE ...@@ -141,19 +140,6 @@ module EE
.find_by(smartcard_identities: { subject: certificate_subject, issuer: certificate_issuer }) .find_by(smartcard_identities: { subject: certificate_subject, issuer: certificate_issuer })
end end
def username_suggestion(base_name)
suffix = nil
base_name = base_name.parameterize(separator: '_')
MAX_USERNAME_SUGGESTION_ATTEMPTS.times do |attempt|
username = "#{base_name}#{suffix}"
return username unless ::Namespace.find_by_path_or_name(username)
suffix = attempt + 1
end
''
end
# Limits the users to those who have an identity that belongs to # Limits the users to those who have an identity that belongs to
# the given SAML Provider # the given SAML Provider
def limit_to_saml_provider(saml_provider_id) def limit_to_saml_provider(saml_provider_id)
......
# frozen_string_literal: true
scope :username do
get 'suggestion', to: 'usernames#suggest'
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe UsernamesController do
describe 'GET #suggest' do
context 'namespace does not exist' do
it 'returns JSON with the suggested username' do
get :suggest, params: { name: 'Arthur' }
expected_json = { username: 'arthur' }.to_json
expect(response.body).to eq(expected_json)
end
end
context 'namespace exists' do
before do
create(:user, name: 'disney')
end
it 'returns JSON with the parameterized username and suffix as a suggestion' do
get :suggest, params: { name: 'Disney' }
expected_json = { username: 'disney1' }.to_json
expect(response.body).to eq(expected_json)
end
end
context 'no name provided' do
it 'returns bad request response' do
get :suggest
expect(response).to have_gitlab_http_status(:bad_request)
end
end
end
end
...@@ -50,28 +50,5 @@ RSpec.describe 'Trial Sign Up' do ...@@ -50,28 +50,5 @@ RSpec.describe 'Trial Sign Up' do
expect(page).to have_content('Start your Free Ultimate Trial') expect(page).to have_content('Start your Free Ultimate Trial')
end end
end end
context 'entering', :js do
using RSpec::Parameterized::TableSyntax
where(:case_name, :first_name, :last_name, :suggested_username) do
'first name' | 'foobar' | nil | 'foobar'
'last name' | nil | 'foobar' | 'foobar'
'first name and last name' | 'foo' | 'bar' | 'foo_bar'
end
with_them do
it 'suggests the username' do
visit new_trial_registration_path
click_on 'Continue'
fill_in 'new_user_first_name', with: first_name if first_name
fill_in 'new_user_last_name', with: last_name if last_name
find('body').click
expect(page).to have_field('new_user_username', with: suggested_username)
end
end
end
end end
end end
import MockAdapter from 'axios-mock-adapter';
import UsernameSuggester from 'ee/trial_registrations/username_suggester';
import { setHTMLFixture } from 'helpers/fixtures';
import axios from '~/lib/utils/axios_utils';
describe('UsernameSuggester', () => {
let axiosMock;
let suggester;
let firstName;
let lastName;
let username;
const usernameEndPoint = '/-/username';
const expectedUsername = 'foo_bar';
const setupSuggester = (dstElement, srcElementIds) => {
suggester = new UsernameSuggester(dstElement, srcElementIds);
};
beforeEach(() => {
setHTMLFixture(`
<div class="flash-container"></div>
<input type="text" id="first_name">
<input type="text" id="last_name">
<input type="text" id="username" data-api-path="${usernameEndPoint}">
`);
firstName = document.getElementById('first_name');
lastName = document.getElementById('last_name');
username = document.getElementById('username');
});
describe('constructor', () => {
it('sets isLoading to false', () => {
setupSuggester('username', ['first_name']);
expect(suggester.isLoading).toBe(false);
});
it(`sets the apiPath to ${usernameEndPoint}`, () => {
setupSuggester('username', ['first_name']);
expect(suggester.apiPath).toBe(usernameEndPoint);
});
it('throws an error if target element is missing', () => {
expect(() => {
setupSuggester('id_with_that_id_does_not_exist', ['first_name']);
}).toThrow('The target element is missing.');
});
it('throws an error if api path is missing', () => {
setHTMLFixture(`
<input type="text" id="first_name">
<input type="text" id="last_name">
<input type="text" id="username">
`);
expect(() => {
setupSuggester('username', ['first_name']);
}).toThrow('The API path was not specified.');
});
it('throws an error when no arguments were provided', () => {
expect(() => {
setupSuggester();
}).toThrow("Required argument 'targetElement' is missing");
});
});
describe('joinSources', () => {
it('does not add `_` (underscore) with the only input specified', () => {
setupSuggester('username', ['first_name']);
firstName.value = 'foo';
const name = suggester.joinSources();
expect(name).toBe('foo');
});
it('joins values from multiple inputs specified by `_` (underscore)', () => {
setupSuggester('username', ['first_name', 'last_name']);
firstName.value = 'foo';
lastName.value = 'bar';
const name = suggester.joinSources();
expect(name).toBe(expectedUsername);
});
it('returns an empty string if 0 inputs specified', () => {
setupSuggester('username', []);
const name = suggester.joinSources();
expect(name).toBe('');
});
});
describe('suggestUsername', () => {
beforeEach(() => {
axiosMock = new MockAdapter(axios);
setupSuggester('username', ['first_name', 'last_name']);
});
afterEach(() => {
axiosMock.restore();
});
it('does not suggests username if suggester is already running', () => {
suggester.isLoading = true;
expect(axiosMock.history.get).toHaveLength(0);
expect(username).toHaveValue('');
});
it('suggests username successfully', () => {
axiosMock
.onGet(usernameEndPoint, { param: { name: expectedUsername } })
.reply(200, { username: expectedUsername });
expect(suggester.isLoading).toBe(false);
firstName.value = 'foo';
lastName.value = 'bar';
suggester.suggestUsername();
setImmediate(() => {
expect(axiosMock.history.get).toHaveLength(1);
expect(suggester.isLoading).toBe(false);
expect(username).toHaveValue(expectedUsername);
});
});
it('shows a flash message if request fails', (done) => {
axiosMock.onGet(usernameEndPoint).replyOnce(500);
expect(suggester.isLoading).toBe(false);
firstName.value = 'foo';
lastName.value = 'bar';
suggester.suggestUsername();
setImmediate(() => {
expect(axiosMock.history.get).toHaveLength(1);
expect(suggester.isLoading).toBe(false);
expect(username).toHaveValue('');
expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
'An error occurred while generating a username. Please try again.',
);
done();
});
});
});
});
...@@ -5,12 +5,6 @@ require 'spec_helper' ...@@ -5,12 +5,6 @@ require 'spec_helper'
RSpec.describe EE::RegistrationsHelper do RSpec.describe EE::RegistrationsHelper do
include Devise::Test::ControllerHelpers include Devise::Test::ControllerHelpers
describe '#signup_username_data_attributes' do
it 'has expected attributes' do
expect(helper.signup_username_data_attributes.keys).to include(:api_path)
end
end
describe '#shuffled_registration_objective_options' do describe '#shuffled_registration_objective_options' do
subject(:shuffled_options) { helper.shuffled_registration_objective_options } subject(:shuffled_options) { helper.shuffled_registration_objective_options }
......
...@@ -1202,63 +1202,6 @@ RSpec.describe User do ...@@ -1202,63 +1202,6 @@ RSpec.describe User do
end end
end end
describe '.username_suggestion' do
context 'namespace with input name does not exist' do
let(:name) { 'Arthur Morgan' }
it 'returns the parameterized name' do
username = described_class.username_suggestion(name)
expect(username).to eq('arthur_morgan')
end
end
context 'namespace with input name exists' do
let(:name) { 'Disney' }
before do
create(:user, name: 'disney')
end
it 'returns the parameterized name with a suffix' do
username = described_class.username_suggestion(name)
expect(username).to eq('disney1')
end
end
context 'namespace with input name and suffix exists' do
let(:name) { 'Disney' }
before do
create(:user, name: 'disney')
create(:user, name: 'disney1')
end
it 'loops through parameterized name with suffixes, until it finds one that does not exist' do
username = described_class.username_suggestion(name)
expect(username).to eq('disney2')
end
end
context 'when max attempts for suggestion is exceeded' do
let(:name) { 'Disney' }
let(:max_attempts) { described_class::MAX_USERNAME_SUGGESTION_ATTEMPTS }
before do
allow(::Namespace).to receive(:find_by_path_or_name).with("disney").and_return(true)
max_attempts.times { |count| allow(::Namespace).to receive(:find_by_path_or_name).with("disney#{count}").and_return(true) }
end
it 'returns an empty response' do
username = described_class.username_suggestion(name)
expect(username).to eq('')
end
end
end
describe '#manageable_groups_eligible_for_trial', :saas do describe '#manageable_groups_eligible_for_trial', :saas do
let_it_be(:user) { create :user } let_it_be(:user) { create :user }
let_it_be(:non_trialed_group_z) { create :group_with_plan, name: 'Zeta', plan: :free_plan } let_it_be(:non_trialed_group_z) { create :group_with_plan, name: 'Zeta', plan: :free_plan }
......
...@@ -3815,9 +3815,6 @@ msgstr "" ...@@ -3815,9 +3815,6 @@ msgstr ""
msgid "An error occurred while fetching this tab." msgid "An error occurred while fetching this tab."
msgstr "" msgstr ""
msgid "An error occurred while generating a username. Please try again."
msgstr ""
msgid "An error occurred while getting autocomplete data. Please refresh the page and try again." msgid "An error occurred while getting autocomplete data. Please refresh the page and try again."
msgstr "" msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment