Merge branch...

Merge branch '330130-remedy-the-suggestion_path-issues-with-registration-between-ee-and-foss' into 'master'

Remedy the suggestion_path issues with registration between ee and FOSS [RUN AS-IF-FOSS] [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!61629

app/helpers/registrations_helper.rb

@@ -7,6 +7,16 @@ module RegistrationsHelper
       devise_mapping.omniauthable? &&
       button_based_providers_enabled?
   end
+
+  def signup_username_data_attributes
+    {
+      min_length: User::MIN_USERNAME_LENGTH,
+      min_length_message: s_('SignUp|Username is too short (minimum is %{min_length} characters).') % { min_length: User::MIN_USERNAME_LENGTH },
+      max_length: User::MAX_USERNAME_LENGTH,
+      max_length_message: s_('SignUp|Username is too long (maximum is %{max_length} characters).') % { max_length: User::MAX_USERNAME_LENGTH },
+      qa_selector: 'new_user_username_field'
+    }
+  end
 end
 
 RegistrationsHelper.prepend_mod_with('RegistrationsHelper')

app/models/user.rb

@@ -35,6 +35,9 @@ class User < ApplicationRecord
 
   COUNT_CACHE_VALIDITY_PERIOD = 24.hours
 
+  MAX_USERNAME_LENGTH = 255
+  MIN_USERNAME_LENGTH = 2
+
   add_authentication_token_field :incoming_email_token, token_generator: -> { SecureRandom.hex.to_i(16).to_s(36) }
   add_authentication_token_field :feed_token
   add_authentication_token_field :static_object_token

app/views/devise/registrations/new.html.haml

@@ -8,6 +8,5 @@
   = render 'devise/shared/signup_box',
     url: registration_path(resource_name),
     button_text: _('Register'),
-    show_omniauth_providers: omniauth_enabled? && button_based_providers_enabled?,
-    suggestion_path: nil
+    show_omniauth_providers: omniauth_enabled? && button_based_providers_enabled?
   = render 'devise/shared/sign_in_link'

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

 - max_first_name_length = max_last_name_length = 127
-- max_username_length = 255
-- min_username_length = 2
 - omniauth_providers_placement ||= :bottom
 
 .gl-mb-3.gl-p-4.gl-border-gray-100.gl-border-1.gl-border-solid.gl-rounded-base
@@ -15,22 +13,49 @@
     .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 gl-form-input top js-block-emoji js-validate-length', :data => { :max_length => max_first_name_length, :max_length_message => s_('SignUp|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.')
+        = f.text_field :first_name,
+          class: 'form-control gl-form-input top js-block-emoji js-validate-length',
+          data: { max_length: max_first_name_length,
+          max_length_message: s_('SignUp|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 gl-form-input top js-block-emoji js-validate-length', :data => { :max_length => max_last_name_length, :max_length_message => s_('SignUp|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.')
+        = f.text_field :last_name,
+          class: 'form-control gl-form-input top js-block-emoji js-validate-length',
+          data: { max_length: max_last_name_length,
+          max_length_message: s_('SignUp|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 gl-form-input middle js-block-emoji js-validate-length js-validate-username', :data => { :api_path => suggestion_path, :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.')
+      = f.text_field :username,
+        class: 'form-control gl-form-input middle js-block-emoji js-validate-length js-validate-username',
+        data: signup_username_data_attributes,
+        pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS,
+        required: true,
+        title: _('Please create a username with only alphanumeric characters.')
       %p.validation-error.gl-text-red-500.gl-field-error-ignore.gl-mt-2.field-validation.hide= _('Username is already taken.')
       %p.validation-success.gl-text-green-600.gl-field-error-ignore.gl-mt-2.field-validation.hide= _('Username is available.')
       %p.validation-pending.gl-field-error-ignore.gl-mt-2.field-validation.hide= _('Checking username availability...')
     .form-group
       = f.label :email, class: 'label-bold'
-      = f.email_field :email, value: @invite_email, class: 'form-control gl-form-input middle', data: { qa_selector: 'new_user_email_field' }, required: true, title: _('Please provide a valid email address.')
+      = f.email_field :email,
+        value: @invite_email,
+        class: 'form-control gl-form-input middle',
+        data: { qa_selector: 'new_user_email_field' },
+        required: true,
+        title: _('Please provide a valid email address.')
     .form-group.gl-mb-5#password-strength
       = f.label :password, class: 'label-bold'
-      = f.password_field :password, class: 'form-control gl-form-input bottom', data: { qa_selector: 'new_user_password_field' }, required: true, pattern: ".{#{@minimum_password_length},}", title: s_('SignUp|Minimum length is %{minimum_password_length} characters.') % { minimum_password_length: @minimum_password_length }
+      = f.password_field :password,
+        class: 'form-control gl-form-input bottom',
+        data: { qa_selector: 'new_user_password_field' },
+        required: true,
+        pattern: ".{#{@minimum_password_length},}",
+        title: s_('SignUp|Minimum length is %{minimum_password_length} characters.') % { minimum_password_length: @minimum_password_length }
       %p.gl-field-hint.text-secondary= s_('SignUp|Minimum length is %{minimum_password_length} characters.') % { minimum_password_length: @minimum_password_length }
     %div
     - if show_recaptcha_sign_up?

app/views/registrations/invites/new.html.haml

@@ -13,6 +13,5 @@
     url: users_sign_up_invites_path,
     button_text: _('Continue'),
     show_omniauth_providers: social_signin_enabled?,
-    omniauth_providers_placement: :top,
-    suggestion_path: nil
+    omniauth_providers_placement: :top
   = render 'devise/shared/sign_in_link'

ee/app/helpers/ee/registrations_helper.rb

@@ -3,6 +3,12 @@
 module EE
   module RegistrationsHelper
     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
 
     private
 

ee/spec/helpers/ee/registrations_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::RegistrationsHelper do
+  describe '#signup_username_data_attributes' do
+    it 'has expected attributes' do
+      expect(helper.signup_username_data_attributes.keys).to include(:api_path)
+    end
+  end
+end

qa/qa/page/registration/sign_up.rb

@@ -7,12 +7,15 @@ module QA
         view 'app/views/devise/shared/_signup_box.html.haml' do
           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
           element :new_user_register_button
         end
 
+        view 'app/helpers/registrations_helper.rb' do
+          element :new_user_username_field
+        end
+
         view 'app/views/registrations/welcome/show.html.haml' do
           element :get_started_button
         end

spec/helpers/registrations_helper_spec.rb

@@ -26,4 +26,10 @@ RSpec.describe RegistrationsHelper do
       it { is_expected.to eq(result) }
     end
   end
+
+  describe '#signup_username_data_attributes' do
+    it 'has expected attributes' do
+      expect(helper.signup_username_data_attributes.keys).to include(:min_length, :min_length_message, :max_length, :max_length_message, :qa_selector)
+    end
+  end
 end

spec/models/user_spec.rb

@@ -22,6 +22,8 @@ RSpec.describe User do
 
   describe 'constants' do
     it { expect(described_class::COUNT_CACHE_VALIDITY_PERIOD).to be_a(Integer) }
+    it { expect(described_class::MAX_USERNAME_LENGTH).to be_a(Integer) }
+    it { expect(described_class::MIN_USERNAME_LENGTH).to be_a(Integer) }
   end
 
   describe 'delegations' do

spec/views/devise/shared/_signup_box.html.haml_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe 'devise/shared/_signup_box' do
     allow(view).to receive(:url).and_return('_url_')
     allow(view).to receive(:terms_path).and_return('_terms_path_')
     allow(view).to receive(:button_text).and_return('_button_text_')
-    allow(view).to receive(:suggestion_path).and_return('_suggestion_path_')
+    allow(view).to receive(:signup_username_data_attributes).and_return({})
     stub_template 'devise/shared/_error_messages.html.haml' => ''
   end