Improve uniqueness of field names on the signup form
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15075
This commit is contained in:
parent
6a19467c41
commit
a6ba8647f9
4 changed files with 29 additions and 22 deletions
|
@ -8,6 +8,13 @@ class RegistrationsController < Devise::RegistrationsController
|
|||
|
||||
def create
|
||||
if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
|
||||
# To avoid duplicate form fields on the login page, the registration form
|
||||
# names fields using `new_user`, but Devise still wants the params in
|
||||
# `user`.
|
||||
if params["new_#{resource_name}"].present? && params[resource_name].blank?
|
||||
params[resource_name] = params.delete(:"new_#{resource_name}")
|
||||
end
|
||||
|
||||
super
|
||||
else
|
||||
flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code."
|
||||
|
|
|
@ -6,7 +6,7 @@
|
|||
.login-heading
|
||||
%h3 Create an account
|
||||
.login-body
|
||||
= form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f|
|
||||
= form_for(resource, as: "new_#{resource_name}", url: registration_path(resource_name)) do |f|
|
||||
.devise-errors
|
||||
= devise_error_messages!
|
||||
%div
|
||||
|
@ -16,7 +16,7 @@
|
|||
%div
|
||||
= f.email_field :email, class: "form-control middle", placeholder: "Email", required: true
|
||||
.form-group.append-bottom-20#password-strength
|
||||
= f.password_field :password, class: "form-control bottom", id: "user_password_sign_up", placeholder: "Password", required: true
|
||||
= f.password_field :password, class: "form-control bottom", placeholder: "Password", required: true
|
||||
%div
|
||||
- if current_application_settings.recaptcha_enabled
|
||||
= recaptcha_tags
|
||||
|
|
|
@ -7,10 +7,10 @@ feature 'Signup', feature: true do
|
|||
|
||||
visit root_path
|
||||
|
||||
fill_in 'user_name', with: user.name
|
||||
fill_in 'user_username', with: user.username
|
||||
fill_in 'user_email', with: user.email
|
||||
fill_in 'user_password_sign_up', with: user.password
|
||||
fill_in 'new_user_name', with: user.name
|
||||
fill_in 'new_user_username', with: user.username
|
||||
fill_in 'new_user_email', with: user.email
|
||||
fill_in 'new_user_password', with: user.password
|
||||
click_button "Sign up"
|
||||
|
||||
expect(current_path).to eq user_session_path
|
||||
|
@ -25,10 +25,10 @@ feature 'Signup', feature: true do
|
|||
|
||||
visit root_path
|
||||
|
||||
fill_in 'user_name', with: user.name
|
||||
fill_in 'user_username', with: user.username
|
||||
fill_in 'user_email', with: existing_user.email
|
||||
fill_in 'user_password_sign_up', with: user.password
|
||||
fill_in 'new_user_name', with: user.name
|
||||
fill_in 'new_user_username', with: user.username
|
||||
fill_in 'new_user_email', with: existing_user.email
|
||||
fill_in 'new_user_password', with: user.password
|
||||
click_button "Sign up"
|
||||
|
||||
expect(current_path).to eq user_registration_path
|
||||
|
@ -42,10 +42,10 @@ feature 'Signup', feature: true do
|
|||
|
||||
visit root_path
|
||||
|
||||
fill_in 'user_name', with: user.name
|
||||
fill_in 'user_username', with: user.username
|
||||
fill_in 'user_email', with: existing_user.email
|
||||
fill_in 'user_password_sign_up', with: user.password
|
||||
fill_in 'new_user_name', with: user.name
|
||||
fill_in 'new_user_username', with: user.username
|
||||
fill_in 'new_user_email', with: existing_user.email
|
||||
fill_in 'new_user_password', with: user.password
|
||||
click_button "Sign up"
|
||||
|
||||
expect(current_path).to eq user_registration_path
|
||||
|
|
|
@ -5,10 +5,10 @@ feature 'Users', feature: true do
|
|||
|
||||
scenario 'GET /users/sign_in creates a new user account' do
|
||||
visit new_user_session_path
|
||||
fill_in 'user_name', with: 'Name Surname'
|
||||
fill_in 'user_username', with: 'Great'
|
||||
fill_in 'user_email', with: 'name@mail.com'
|
||||
fill_in 'user_password_sign_up', with: 'password1234'
|
||||
fill_in 'new_user_name', with: 'Name Surname'
|
||||
fill_in 'new_user_username', with: 'Great'
|
||||
fill_in 'new_user_email', with: 'name@mail.com'
|
||||
fill_in 'new_user_password', with: 'password1234'
|
||||
expect { click_button 'Sign up' }.to change { User.count }.by(1)
|
||||
end
|
||||
|
||||
|
@ -31,10 +31,10 @@ feature 'Users', feature: true do
|
|||
|
||||
scenario 'Should show one error if email is already taken' do
|
||||
visit new_user_session_path
|
||||
fill_in 'user_name', with: 'Another user name'
|
||||
fill_in 'user_username', with: 'anotheruser'
|
||||
fill_in 'user_email', with: user.email
|
||||
fill_in 'user_password_sign_up', with: '12341234'
|
||||
fill_in 'new_user_name', with: 'Another user name'
|
||||
fill_in 'new_user_username', with: 'anotheruser'
|
||||
fill_in 'new_user_email', with: user.email
|
||||
fill_in 'new_user_password', with: '12341234'
|
||||
expect { click_button 'Sign up' }.to change { User.count }.by(0)
|
||||
expect(page).to have_text('Email has already been taken')
|
||||
expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}'
|
||||
|
|
Loading…
Reference in a new issue