Merge branch 'nicolasdular/split-first-last-name' into 'master'

Split full name to first and last name for signup

See merge request gitlab-org/gitlab!42346

app/views/devise/shared/_experimental_separate_sign_up_flow_box.html.haml

@@ -11,10 +11,10 @@
       .name.form-row
         .col.form-group
           = f.label :first_name, _('First name'), for: 'new_user_first_name', class: 'label-bold'
-          = f.text_field :first_name, class: 'form-control top js-block-emoji js-validate-length', :data => { :max_length => max_first_name_length, :max_length_message => _("First Name is too long (maximum is %{max_length} characters).") % { max_length: max_first_name_length }, :qa_selector => 'new_user_firstname_field' }, required: true, title: _("This field is required.")
+          = f.text_field :first_name, class: 'form-control top js-block-emoji js-validate-length', :data => { :max_length => max_first_name_length, :max_length_message => _("First name is too long (maximum is %{max_length} characters).") % { max_length: max_first_name_length }, :qa_selector => 'new_user_firstname_field' }, required: true, title: _("This field is required.")
         .col.form-group
           = f.label :last_name, _('Last name'), for: 'new_user_last_name', class: 'label-bold'
-          = f.text_field :last_name, class: "form-control top js-block-emoji js-validate-length", :data => { :max_length => max_last_name_length, :max_length_message => _("Last Name is too long (maximum is %{max_length} characters).") % { max_length: max_last_name_length }, :qa_selector => 'new_user_lastname_field' }, required: true, title: _("This field is required.")
+          = f.text_field :last_name, class: "form-control top js-block-emoji js-validate-length", :data => { :max_length => max_last_name_length, :max_length_message => _("Last name is too long (maximum is %{max_length} characters).") % { max_length: max_last_name_length }, :qa_selector => 'new_user_lastname_field' }, required: true, title: _("This field is required.")
       .username.form-group
         = f.label :username, class: 'label-bold'
         = f.text_field :username, class: "form-control middle js-block-emoji js-validate-length js-validate-username", :data => { :min_length => min_username_length, :min_length_message => s_("SignUp|Username is too short (minimum is %{min_length} characters).") % { min_length: min_username_length }, :max_length => max_username_length, :max_length_message => _("Username is too long (maximum is %{max_length} characters).") % { max_length: max_username_length }, :qa_selector => 'new_user_username_field' }, pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")

app/views/devise/shared/_signup_box.html.haml

-- max_name_length = 255
+- max_first_name_length = max_last_name_length = 127
 - max_username_length = 255
 - min_username_length = 2
 #register-pane.tab-pane.login-box{ role: 'tabpanel' }
@@ -8,9 +8,13 @@
         = render "devise/shared/error_messages", resource: resource
       - if Feature.enabled?(:invisible_captcha)
         = invisible_captcha
-      .name.form-group
-        = f.label :name, _('Full name'), class: 'label-bold'
-        = f.text_field :name, class: "form-control top 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 }, :qa_selector => 'new_user_name_field' }, required: true, title: _("This field is required.")
+      .name.form-row
+        .col.form-group
+          = f.label :first_name, _('First name'), for: 'new_user_first_name', class: 'label-bold'
+          = f.text_field :first_name, class: 'form-control top js-block-emoji js-validate-length', :data => { :max_length => max_first_name_length, :max_length_message => _("First name is too long (maximum is %{max_length} characters).") % { max_length: max_first_name_length }, :qa_selector => 'new_user_first_name_field' }, required: true, title: _("This field is required.")
+        .col.form-group
+          = f.label :last_name, _('Last name'), for: 'new_user_last_name', class: 'label-bold'
+          = f.text_field :last_name, class: "form-control top js-block-emoji js-validate-length", :data => { :max_length => max_last_name_length, :max_length_message => _("Last name is too long (maximum is %{max_length} characters).") % { max_length: max_last_name_length }, :qa_selector => 'new_user_last_name_field' }, required: true, title: _("This field is required.")
       .username.form-group
         = f.label :username, class: 'label-bold'
         = f.text_field :username, class: "form-control middle js-block-emoji js-validate-length js-validate-username", :data => { :min_length => min_username_length, :min_length_message => s_("SignUp|Username is too short (minimum is %{min_length} characters).") % { min_length: min_username_length }, :max_length => max_username_length, :max_length_message => s_("SignUp|Username is too long (maximum is %{max_length} characters).") % { max_length: max_username_length }, :qa_selector => 'new_user_username_field' }, pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")

changelogs/unreleased/nicolasdular-split-first-last-name.yml

+---
+title: Split name to first and last name for signup
+merge_request: 42346
+author:
+type: changed

ee/spec/controllers/registrations_controller_spec.rb

@@ -6,8 +6,11 @@ RSpec.describe RegistrationsController do
   let_it_be(:user) { create(:user) }
 
   describe '#create' do
+    let(:base_user_params) { build_stubbed(:user).slice(:first_name, :last_name, :username, :email, :password) }
+    let(:user_params) { { user: base_user_params } }
+
     context 'when the user opted-in' do
-      let(:user_params) { { user: attributes_for(:user, email_opted_in: '1') } }
+      let(:user_params) { { user: base_user_params.merge(email_opted_in: '1') } }
 
       it 'sets the rest of the email_opted_in fields' do
         post :create, params: user_params
@@ -20,7 +23,7 @@ RSpec.describe RegistrationsController do
     end
 
     context 'when the user opted-out' do
-      let(:user_params) { { user: attributes_for(:user, email_opted_in: '0') } }
+      let(:user_params) { { user: base_user_params.merge(email_opted_in: '0') } }
 
       it 'does not set the rest of the email_opted_in fields' do
         post :create, params: user_params
@@ -34,7 +37,6 @@ RSpec.describe RegistrationsController do
 
     context 'when reCAPTCHA experiment enabled' do
       it "logs a 'User Created' message including the experiment state" do
-        user_params = { user: attributes_for(:user) }
         allow_any_instance_of(EE::RecaptchaExperimentHelper).to receive(:show_recaptcha_sign_up?).and_return(true)
 
         expect(Gitlab::AppLogger).to receive(:info).with(/\AUser Created: .+experiment_growth_recaptcha\?true\z/).and_call_original

ee/spec/features/invites_spec.rb

@@ -16,7 +16,8 @@ RSpec.describe 'Group or Project invitations' do
   end
 
   def fill_in_sign_up_form(user)
-    fill_in 'new_user_name', with: user.name
+    fill_in 'new_user_first_name', with: user.first_name
+    fill_in 'new_user_last_name', with: user.last_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

ee/spec/features/signup_spec.rb

@@ -3,7 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe 'Signup on EE' do
-  let(:user_attrs) { attributes_for(:user) }
+  let(:new_user) { build_stubbed(:user) }
+
+  def fill_in_signup_form
+    fill_in 'new_user_username', with: new_user.username
+    fill_in 'new_user_email', with: new_user.email
+    fill_in 'new_user_first_name', with: new_user.first_name
+    fill_in 'new_user_last_name', with: new_user.last_name
+    fill_in 'new_user_password', with: new_user.password
+  end
 
   context 'for Gitlab.com' do
     before do
@@ -14,14 +22,11 @@ RSpec.describe 'Signup on EE' do
       it 'creates the user and sets the email_opted_in field truthy' do
         visit root_path
 
-        fill_in 'new_user_name',                with: user_attrs[:name]
-        fill_in 'new_user_username',            with: user_attrs[:username]
-        fill_in 'new_user_email',               with: user_attrs[:email]
-        fill_in 'new_user_password',            with: user_attrs[:password]
-        check   'new_user_email_opted_in'
+        fill_in_signup_form
+        check 'new_user_email_opted_in'
         click_button "Register"
 
-        user = User.find_by_username!(user_attrs[:username])
+        user = User.find_by_username!(new_user[:username])
         expect(user.email_opted_in).to be_truthy
         expect(user.email_opted_in_ip).to be_present
         expect(user.email_opted_in_source).to eq('GitLab.com')
@@ -33,13 +38,10 @@ RSpec.describe 'Signup on EE' do
       it 'creates the user and sets the email_opted_in field falsey' do
         visit root_path
 
-        fill_in 'new_user_name',                with: user_attrs[:name]
-        fill_in 'new_user_username',            with: user_attrs[:username]
-        fill_in 'new_user_email',               with: user_attrs[:email]
-        fill_in 'new_user_password',            with: user_attrs[:password]
+        fill_in_signup_form
         click_button "Register"
 
-        user = User.find_by_username!(user_attrs[:username])
+        user = User.find_by_username!(new_user[:username])
         expect(user.email_opted_in).to be_falsey
         expect(user.email_opted_in_ip).to be_blank
         expect(user.email_opted_in_source).to be_blank
@@ -50,10 +52,7 @@ RSpec.describe 'Signup on EE' do
     it 'redirects to step 2 of the signup process, sets the role and setup for company and redirects back' do
       visit new_user_registration_path
 
-      fill_in 'new_user_name', with: user_attrs[:name].split(' ').first
-      fill_in 'new_user_username', with: user_attrs[:username]
-      fill_in 'new_user_email', with: user_attrs[:email]
-      fill_in 'new_user_password', with: user_attrs[:password]
+      fill_in_signup_form
       click_button 'Register'
       visit new_project_path
 
@@ -62,7 +61,7 @@ RSpec.describe 'Signup on EE' do
       select 'Software Developer', from: 'user_role'
       choose 'user_setup_for_company_true'
       click_button 'Get started!'
-      user = User.find_by_username(user_attrs[:username])
+      user = User.find_by_username(new_user[:username])
 
       expect(user.software_developer_role?).to be_truthy
       expect(user.setup_for_company).to be_truthy
@@ -80,13 +79,10 @@ RSpec.describe 'Signup on EE' do
 
       expect(page).not_to have_selector("[name='new_user_email_opted_in']")
 
-      fill_in 'new_user_name',                with: user_attrs[:name]
-      fill_in 'new_user_username',            with: user_attrs[:username]
-      fill_in 'new_user_email',               with: user_attrs[:email]
-      fill_in 'new_user_password',            with: user_attrs[:password]
+      fill_in_signup_form
       click_button "Register"
 
-      user = User.find_by_username!(user_attrs[:username])
+      user = User.find_by_username!(new_user[:username])
       expect(user.email_opted_in).to be_falsey
       expect(user.email_opted_in_ip).to be_blank
       expect(user.email_opted_in_source).to be_blank

locale/gitlab.pot

@@ -11398,9 +11398,6 @@ msgstr ""
 msgid "Finished"
 msgstr ""
 
-msgid "First Name is too long (maximum is %{max_length} characters)."
-msgstr ""
-
 msgid "First Seen"
 msgstr ""
 
@@ -11410,6 +11407,9 @@ msgstr ""
 msgid "First name"
 msgstr ""
 
+msgid "First name is too long (maximum is %{max_length} characters)."
+msgstr ""
+
 msgid "First seen"
 msgstr ""
 
@@ -14893,9 +14893,6 @@ msgstr ""
 msgid "Last Accessed On"
 msgstr ""
 
-msgid "Last Name is too long (maximum is %{max_length} characters)."
-msgstr ""
-
 msgid "Last Pipeline"
 msgstr ""
 
@@ -14929,6 +14926,9 @@ msgstr ""
 msgid "Last name"
 msgstr ""
 
+msgid "Last name is too long (maximum is %{max_length} characters)."
+msgstr ""
+
 msgid "Last reply by"
 msgstr ""
 
@@ -23918,9 +23918,6 @@ msgstr ""
 msgid "SignUp|Last Name is too long (maximum is %{max_length} characters)."
 msgstr ""
 
-msgid "SignUp|Name is too long (maximum is %{max_length} characters)."
-msgstr ""
-
 msgid "SignUp|Username is too long (maximum is %{max_length} characters)."
 msgstr ""
 

qa/qa/page/main/sign_up.rb

@@ -5,7 +5,8 @@ module QA
     module Main
       class SignUp < Page::Base
         view 'app/views/devise/shared/_signup_box.html.haml' do
-          element :new_user_name_field
+          element :new_user_first_name_field
+          element :new_user_last_name_field
           element :new_user_username_field
           element :new_user_email_field
           element :new_user_password_field
@@ -18,7 +19,8 @@ module QA
         end
 
         def sign_up!(user)
-          fill_element :new_user_name_field, user.name
+          fill_element :new_user_first_name_field, user.first_name
+          fill_element :new_user_last_name_field, user.last_name
           fill_element :new_user_username_field, user.username
           fill_element :new_user_email_field, user.email
           fill_element :new_user_password_field, user.password

qa/qa/resource/user.rb

@@ -11,6 +11,8 @@ module QA
 
       attribute :id
       attribute :name
+      attribute :first_name
+      attribute :last_name
       attribute :email
 
       def initialize
@@ -34,6 +36,14 @@ module QA
         @name ||= api_resource&.dig(:name) || "QA User #{unique_id}"
       end
 
+      def first_name
+        name.split(' ').first
+      end
+
+      def last_name
+        name.split(' ').drop(1).join(' ')
+      end
+
       def email
         @email ||= begin
           api_email = api_resource&.dig(:email)

spec/controllers/registrations_controller_spec.rb

@@ -80,7 +80,7 @@ RSpec.describe RegistrationsController do
   end
 
   describe '#create' do
-    let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } }
+    let(:base_user_params) { { first_name: 'first', last_name: 'last', username: 'new_username', email: 'new@user.com', password: 'Any_password' } }
     let(:user_params) { { user: base_user_params } }
 
     context 'email confirmation' do
@@ -370,21 +370,12 @@ RSpec.describe RegistrationsController do
       expect(subject.current_user).not_to be_nil
     end
 
-    context 'with the experimental signup flow enabled and the user is part of the experimental group' do
-      before do
-        stub_experiment(signup_flow: true)
-        stub_experiment_for_user(signup_flow: true)
-      end
-
-      let(:base_user_params) { { first_name: 'First', last_name: 'Last', username: 'new_username', email: 'new@user.com', password: 'Any_password' } }
+    it 'sets name from first and last name' do
+      post :create, params: { new_user: base_user_params }
 
-      it 'sets name from first and last name' do
-        post :create, params: { new_user: base_user_params }
-
-        expect(User.last.first_name).to eq(base_user_params[:first_name])
-        expect(User.last.last_name).to eq(base_user_params[:last_name])
-        expect(User.last.name).to eq("#{base_user_params[:first_name]} #{base_user_params[:last_name]}")
-      end
+      expect(User.last.first_name).to eq(base_user_params[:first_name])
+      expect(User.last.last_name).to eq(base_user_params[:last_name])
+      expect(User.last.name).to eq("#{base_user_params[:first_name]} #{base_user_params[:last_name]}")
     end
   end
 

spec/features/invites_spec.rb

@@ -23,7 +23,8 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
   end
 
   def fill_in_sign_up_form(new_user)
-    fill_in 'new_user_name', with: new_user.name
+    fill_in 'new_user_first_name', with: new_user.first_name
+    fill_in 'new_user_last_name', with: new_user.last_name
     fill_in 'new_user_username', with: new_user.username
     fill_in 'new_user_email', with: new_user.email
     fill_in 'new_user_password', with: new_user.password

spec/services/users/build_service_spec.rb

@@ -4,11 +4,11 @@ require 'spec_helper'
 
 RSpec.describe Users::BuildService do
   describe '#execute' do
-    let(:params) do
-      { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' }
-    end
+    let(:params) { build_stubbed(:user).slice(:first_name, :last_name, :username, :email, :password) }
 
     context 'with an admin user' do
+      let(:params) { build_stubbed(:user).slice(:name, :username, :email, :password) }
+
       let(:admin_user) { create(:admin) }
       let(:service) { described_class.new(admin_user, ActionController::Parameters.new(params).permit!) }