Use first and last name for new sign up flow

With the new paid sign up flow, we will create a customer on the
customer portal. The customer portal requires a first and last name.
We already store first and last name in the database.

Since the new sign up design  will in the future also be used for
the FOSS version of GitLab, we need to accept first and last name
there as well.

app/controllers/registrations_controller.rb

@@ -60,7 +60,7 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def update_registration
-    user_params = params.require(:user).permit(:name, :role, :setup_for_company)
+    user_params = params.require(:user).permit(:role, :setup_for_company)
     result = ::Users::SignupService.new(current_user, user_params).execute
 
     if result[:status] == :success
@@ -152,13 +152,7 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def sign_up_params
-    clean_params = params.require(:user).permit(:username, :email, :email_confirmation, :name, :password)
-
-    if experiment_enabled?(:signup_flow)
-      clean_params[:name] = clean_params[:username]
-    end
-
-    clean_params
+    params.require(:user).permit(:username, :email, :email_confirmation, :name, :first_name, :last_name, :password)
   end
 
   def resource_name

app/services/users/build_service.rb

@@ -86,6 +86,8 @@ module Users
         :email_confirmation,
         :password_automatically_set,
         :name,
+        :first_name,
+        :last_name,
         :password,
         :username
       ]
@@ -107,6 +109,12 @@ module Users
         if user_params[:skip_confirmation].nil?
           user_params[:skip_confirmation] = skip_user_confirmation_email_from_setting
         end
+
+        fallback_name = "#{user_params[:first_name]} #{user_params[:last_name]}"
+
+        if user_params[:name].blank? && fallback_name.present?
+          user_params = user_params.merge(name: fallback_name)
+        end
       end
 
       if user_default_internal_regex_enabled? && !user_params.key?(:external)

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

 - content_for(:page_title, _('Register for GitLab'))
+- max_name_length = 63
 - max_username_length = 255
 .signup-box.p-3.mb-2
   .signup-body
@@ -7,9 +8,16 @@
         = render "devise/shared/error_messages", resource: resource
       - if Feature.enabled?(:invisible_captcha)
         = invisible_captcha
+      .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_name_length, :max_length_message => _("First Name is too long (maximum is %{max_length} characters).") % { max_length: max_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_name_length, :max_length_message => _("Last Name is too long (maximum is %{max_length} characters).") % { max_length: max_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 => { :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.")
+        = f.text_field :username, class: "form-control middle js-block-emoji js-validate-length js-validate-username", :data => { :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.")
         %p.validation-error.gl-field-error-ignore.field-validation.mt-1.hide.cred= _('Username is already taken.')
         %p.validation-success.gl-field-error-ignore.field-validation.mt-1.hide.cgreen= _('Username is available.')
         %p.validation-pending.gl-field-error-ignore.field-validation.mt-1.hide= _('Checking username availability...')

app/views/registrations/welcome.html.haml

-- content_for(:page_title, _('Welcome to GitLab @%{username}!') % { username: current_user.username })
-- max_name_length = 128
+- content_for(:page_title, _('Welcome to GitLab %{name}!') % { name: current_user.name })
 .text-center.mb-3
   = _('In order to tailor your experience with GitLab we<br>would like to know a bit more about you.').html_safe
 .signup-box.p-3.mb-2
@@ -7,9 +6,6 @@
     = form_for(current_user, url: users_sign_up_update_registration_path, html: { class: 'new_new_user gl-show-field-errors', 'aria-live' => 'assertive' }) do |f|
       .devise-errors.mt-0
         = render 'devise/shared/error_messages', resource: current_user
-      .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_('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.')
       .form-group
         = f.label :role, _('Role'), class: 'label-bold'
         = f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'form-control'

ee/app/services/ee/users/build_service.rb

@@ -60,18 +60,6 @@ module EE
         end
       end
 
-      override :build_user_params
-      def build_user_params(skip_authorization:)
-        user_params = super
-        fallback_name = "#{user_params[:first_name]} #{user_params[:last_name]}"
-
-        if user_params[:name].blank? && fallback_name.present?
-          user_params.merge(name: fallback_name)
-        else
-          user_params
-        end
-      end
-
       def saml_provider_id
         strong_memoize(:saml_provider_id) do
           group = GroupFinder.new(current_user).execute(id: group_id_for_saml)

locale/gitlab.pot

@@ -8136,6 +8136,9 @@ msgstr ""
 msgid "Finished"
 msgstr ""
 
+msgid "First Name is too long (maximum is %{max_length} characters)."
+msgstr ""
+
 msgid "First Seen"
 msgstr ""
 
@@ -10659,6 +10662,9 @@ msgstr ""
 msgid "Last Accessed On"
 msgstr ""
 
+msgid "Last Name is too long (maximum is %{max_length} characters)."
+msgstr ""
+
 msgid "Last Pipeline"
 msgstr ""
 
@@ -11982,9 +11988,6 @@ msgstr ""
 msgid "Name has already been taken"
 msgstr ""
 
-msgid "Name is too long (maximum is %{max_length} characters)."
-msgstr ""
-
 msgid "Name new label"
 msgstr ""
 
@@ -20449,6 +20452,9 @@ msgstr ""
 msgid "Username is available."
 msgstr ""
 
+msgid "Username is too long (maximum is %{max_length} characters)."
+msgstr ""
+
 msgid "Username or email"
 msgstr ""
 
@@ -20823,7 +20829,7 @@ msgstr ""
 msgid "Welcome to GitLab"
 msgstr ""
 
-msgid "Welcome to GitLab @%{username}!"
+msgid "Welcome to GitLab %{name}!"
 msgstr ""
 
 msgid "Welcome to the Guided GitLab Tour"

spec/controllers/registrations_controller_spec.rb

@@ -306,6 +306,23 @@ 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 }
+
+        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
   end
 
   describe '#destroy' do
@@ -395,7 +412,7 @@ describe RegistrationsController do
         label: anything,
         property: 'experimental_group'
       )
-      patch :update_registration, params: { user: { name: 'New name', role: 'software_developer', setup_for_company: 'false' } }
+      patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } }
     end
   end
 end

spec/features/users/signup_spec.rb

@@ -123,50 +123,6 @@ shared_examples 'Signup' do
     end
   end
 
-  describe 'user\'s full name validation', :js do
-    before do
-      if Gitlab::Experimentation.enabled?(:signup_flow)
-        user = create(:user, role: nil)
-        sign_in(user)
-        visit users_sign_up_welcome_path
-        @user_name_field = 'user_name'
-      else
-        visit new_user_registration_path
-        @user_name_field = 'new_user_name'
-      end
-    end
-
-    it 'does not show an error border if the user\'s fullname length is not longer than 128 characters' do
-      fill_in @user_name_field, with: 'u' * 128
-
-      expect(find('.name')).not_to have_css '.gl-field-error-outline'
-    end
-
-    it 'shows an error border if the user\'s fullname contains an emoji' do
-      simulate_input("##{@user_name_field}", 'Ehsan 🦋')
-
-      expect(find('.name')).to have_css '.gl-field-error-outline'
-    end
-
-    it 'shows an error border if the user\'s fullname is longer than 128 characters' do
-      fill_in @user_name_field, with: 'n' * 129
-
-      expect(find('.name')).to have_css '.gl-field-error-outline'
-    end
-
-    it 'shows an error message if the user\'s fullname is longer than 128 characters' do
-      fill_in @user_name_field, with: 'n' * 129
-
-      expect(page).to have_content("Name is too long (maximum is 128 characters).")
-    end
-
-    it 'shows an error message if the username contains emojis' do
-      simulate_input("##{@user_name_field}", 'Ehsan 🦋')
-
-      expect(page).to have_content("Invalid input, please avoid emojis")
-    end
-  end
-
   context 'with no errors' do
     context 'when sending confirmation email' do
       before do
@@ -184,7 +140,10 @@ shared_examples 'Signup' do
           fill_in 'new_user_username', with: new_user.username
           fill_in 'new_user_email', with: new_user.email
 
-          unless Gitlab::Experimentation.enabled?(:signup_flow)
+          if Gitlab::Experimentation.enabled?(:signup_flow)
+            fill_in 'new_user_first_name', with: new_user.first_name
+            fill_in 'new_user_last_name', with: new_user.last_name
+          else
             fill_in 'new_user_name', with: new_user.name
             fill_in 'new_user_email_confirmation', with: new_user.email
           end
@@ -209,7 +168,10 @@ shared_examples 'Signup' do
           fill_in 'new_user_username', with: new_user.username
           fill_in 'new_user_email', with: new_user.email
 
-          unless Gitlab::Experimentation.enabled?(:signup_flow)
+          if Gitlab::Experimentation.enabled?(:signup_flow)
+            fill_in 'new_user_first_name', with: new_user.first_name
+            fill_in 'new_user_last_name', with: new_user.last_name
+          else
             fill_in 'new_user_name', with: new_user.name
             fill_in 'new_user_email_confirmation', with: new_user.email
           end
@@ -235,7 +197,10 @@ shared_examples 'Signup' do
         fill_in 'new_user_username', with: new_user.username
         fill_in 'new_user_email', with: new_user.email
 
-        unless Gitlab::Experimentation.enabled?(:signup_flow)
+        if Gitlab::Experimentation.enabled?(:signup_flow)
+          fill_in 'new_user_first_name', with: new_user.first_name
+          fill_in 'new_user_last_name', with: new_user.last_name
+        else
           fill_in 'new_user_name', with: new_user.name
           fill_in 'new_user_email_confirmation', with: new_user.email.capitalize
         end
@@ -263,7 +228,10 @@ shared_examples 'Signup' do
         fill_in 'new_user_username', with: new_user.username
         fill_in 'new_user_email', with: new_user.email
 
-        unless Gitlab::Experimentation.enabled?(:signup_flow)
+        if Gitlab::Experimentation.enabled?(:signup_flow)
+          fill_in 'new_user_first_name', with: new_user.first_name
+          fill_in 'new_user_last_name', with: new_user.last_name
+        else
           fill_in 'new_user_name', with: new_user.name
           fill_in 'new_user_email_confirmation', with: new_user.email
         end
@@ -287,7 +255,10 @@ shared_examples 'Signup' do
 
       visit new_user_registration_path
 
-      unless Gitlab::Experimentation.enabled?(:signup_flow)
+      if Gitlab::Experimentation.enabled?(:signup_flow)
+        fill_in 'new_user_first_name', with: new_user.first_name
+        fill_in 'new_user_last_name', with: new_user.last_name
+      else
         fill_in 'new_user_name', with: new_user.name
       end
 
@@ -313,7 +284,10 @@ shared_examples 'Signup' do
 
       visit new_user_registration_path
 
-      unless Gitlab::Experimentation.enabled?(:signup_flow)
+      if Gitlab::Experimentation.enabled?(:signup_flow)
+        fill_in 'new_user_first_name', with: new_user.first_name
+        fill_in 'new_user_last_name', with: new_user.last_name
+      else
         fill_in 'new_user_name', with: new_user.name
       end
 
@@ -338,7 +312,10 @@ shared_examples 'Signup' do
       fill_in 'new_user_username', with: new_user.username
       fill_in 'new_user_email', with: new_user.email
 
-      unless Gitlab::Experimentation.enabled?(:signup_flow)
+      if Gitlab::Experimentation.enabled?(:signup_flow)
+        fill_in 'new_user_first_name', with: new_user.first_name
+        fill_in 'new_user_last_name', with: new_user.last_name
+      else
         fill_in 'new_user_name', with: new_user.name
         fill_in 'new_user_email_confirmation', with: new_user.email
       end
@@ -357,7 +334,10 @@ shared_examples 'Signup' do
       fill_in 'new_user_username', with: new_user.username
       fill_in 'new_user_email', with: new_user.email
 
-      unless Gitlab::Experimentation.enabled?(:signup_flow)
+      if Gitlab::Experimentation.enabled?(:signup_flow)
+        fill_in 'new_user_first_name', with: new_user.first_name
+        fill_in 'new_user_last_name', with: new_user.last_name
+      else
         fill_in 'new_user_name', with: new_user.name
         fill_in 'new_user_email_confirmation', with: new_user.email
       end
@@ -394,7 +374,10 @@ shared_examples 'Signup' do
       fill_in 'new_user_username', with: new_user.username
       fill_in 'new_user_email', with: new_user.email
 
-      unless Gitlab::Experimentation.enabled?(:signup_flow)
+      if Gitlab::Experimentation.enabled?(:signup_flow)
+        fill_in 'new_user_first_name', with: new_user.first_name
+        fill_in 'new_user_last_name', with: new_user.last_name
+      else
         fill_in 'new_user_name', with: new_user.name
         fill_in 'new_user_email_confirmation', with: new_user.email
       end
@@ -412,6 +395,44 @@ shared_examples 'Signup' do
   end
 end
 
+shared_examples 'Signup name validation' do |field, max_length|
+  before do
+    visit new_user_registration_path
+  end
+
+  describe "#{field} validation", :js do
+    it "does not show an error border if the user's fullname length is not longer than #{max_length} characters" do
+      fill_in field, with: 'u' * max_length
+
+      expect(find('.name')).not_to have_css '.gl-field-error-outline'
+    end
+
+    it 'shows an error border if the user\'s fullname contains an emoji' do
+      simulate_input("##{field}", 'Ehsan 🦋')
+
+      expect(find('.name')).to have_css '.gl-field-error-outline'
+    end
+
+    it "shows an error border if the user\'s fullname is longer than #{max_length} characters" do
+      fill_in field, with: 'n' * (max_length + 1)
+
+      expect(find('.name')).to have_css '.gl-field-error-outline'
+    end
+
+    it "shows an error message if the user\'s fullname is longer than #{max_length} characters" do
+      fill_in field, with: 'n' * (max_length + 1)
+
+      expect(page).to have_content("Name is too long (maximum is #{max_length} characters).")
+    end
+
+    it 'shows an error message if the username contains emojis' do
+      simulate_input("##{field}", 'Ehsan 🦋')
+
+      expect(page).to have_content("Invalid input, please avoid emojis")
+    end
+  end
+end
+
 describe 'With original flow' do
   before do
     stub_experiment(signup_flow: false)
@@ -419,6 +440,7 @@ describe 'With original flow' do
   end
 
   it_behaves_like 'Signup'
+  it_behaves_like 'Signup name validation', 'new_user_name', 128
 end
 
 describe 'With experimental flow' do
@@ -428,11 +450,15 @@ describe 'With experimental flow' do
   end
 
   it_behaves_like 'Signup'
+  it_behaves_like 'Signup name validation', 'new_user_first_name', 63
+  it_behaves_like 'Signup name validation', 'new_user_last_name', 63
 
   describe 'when role is required' do
     it 'after registering, it redirects to step 2 of the signup process, sets the name and role and then redirects to the original requested url' do
       new_user = build_stubbed(:user)
       visit new_user_registration_path
+      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
@@ -441,13 +467,11 @@ describe 'With experimental flow' do
 
       expect(page).to have_current_path(users_sign_up_welcome_path)
 
-      fill_in 'user_name', with: 'New name'
       select 'Software Developer', from: 'user_role'
       choose 'user_setup_for_company_true'
       click_button 'Get started!'
       new_user = User.find_by_username(new_user.username)
 
-      expect(new_user.name).to eq 'New name'
       expect(new_user.software_developer_role?).to be_truthy
       expect(new_user.setup_for_company).to be_truthy
       expect(page).to have_current_path(new_project_path)