Merge branch '20-split-sign-in-and-sign-up' into 'master'

Use separate routes for sign in and sign up

See merge request gitlab-org/gitlab!16482

app/assets/javascripts/pages/registrations/new/index.js

+import LengthValidator from '~/pages/sessions/new/length_validator';
+import UsernameValidator from '~/pages/sessions/new/username_validator';
+import NoEmojiValidator from '~/emoji/no_emoji_validator';
+
+document.addEventListener('DOMContentLoaded', () => {
+  new UsernameValidator(); // eslint-disable-line no-new
+  new LengthValidator(); // eslint-disable-line no-new
+  new NoEmojiValidator(); // eslint-disable-line no-new
+});

app/assets/stylesheets/pages/experimental_separate_sign_up.scss

+.signup-page {
+  .page-wrap {
+    background-color: $gray-light;
+  }
+
+  .gitlab-logo {
+    width: 80px;
+    height: 80px;
+  }
+
+  .signup-box-container {
+    max-width: 900px;
+
+    &.navless-container {
+      // overriding .devise-layout-html.navless-container to support the sticky footer
+      // without having a header on size xs
+      @include media-breakpoint-down(xs) {
+        padding: 65px $gl-padding; // height of footer
+        padding-top: $gl-padding;
+      }
+    }
+  }
+
+  .signup-heading h2 {
+    font-weight: $gl-font-weight-bold;
+
+    @include media-breakpoint-down(md) {
+      font-size: $gl-font-size-large;
+    }
+  }
+
+  .signup-box {
+    background-color: $white-light;
+    box-shadow: 0 0 0 1px $border-color;
+    border-radius: $border-radius;
+  }
+
+  .form-control {
+    &:active,
+    &:focus {
+      background-color: $white-light;
+    }
+  }
+
+  .devise-errors {
+    h2 {
+      font-size: $gl-font-size;
+      color: $red-700;
+    }
+  }
+}

app/assets/stylesheets/pages/login.scss

@@ -215,6 +215,12 @@
   body {
     // offset height of fixed header + 1 to avoid scroll
     height: calc(100% - 51px);
+
+    // offset without the header
+    &.navless {
+      height: calc(100% - 11px);
+    }
+
     margin: 0;
     padding: 0;
 

app/controllers/registrations_controller.rb

@@ -6,13 +6,19 @@ class RegistrationsController < Devise::RegistrationsController
   include RecaptchaExperimentHelper
   include InvisibleCaptcha
 
+  layout :choose_layout
+
   prepend_before_action :check_captcha, only: :create
   before_action :whitelist_query_limiting, only: [:destroy]
   before_action :ensure_terms_accepted,
     if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
 
   def new
-    redirect_to(new_user_session_path)
+    if helpers.use_experimental_separate_sign_up_flow?
+      @resource = build_resource
+    else
+      redirect_to new_user_session_path(anchor: 'register-pane')
+    end
   end
 
   def create
@@ -144,6 +150,16 @@ class RegistrationsController < Devise::RegistrationsController
   def stored_location_or_dashboard(user)
     stored_location_for(user) || dashboard_projects_path
   end
+
+  # Part of an experiment to build a new sign up flow. Will be resolved
+  # with https://gitlab.com/gitlab-org/growth/engineering/issues/64
+  def choose_layout
+    if helpers.use_experimental_separate_sign_up_flow?
+      'devise_experimental_separate_sign_up_flow'
+    else
+      'devise'
+    end
+  end
 end
 
 RegistrationsController.prepend_if_ee('EE::RegistrationsController')

app/helpers/sessions_helper.rb

@@ -4,4 +4,8 @@ module SessionsHelper
   def unconfirmed_email?
     flash[:alert] == t(:unconfirmed, scope: [:devise, :failure])
   end
+
+  def use_experimental_separate_sign_up_flow?
+    ::Gitlab.dev_env_or_com? && Feature.enabled?(:experimental_separate_sign_up_flow)
+  end
 end

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

 - page_title "Sign up"
-= render 'devise/shared/signup_box'
+- if use_experimental_separate_sign_up_flow?
+  = render 'devise/shared/experimental_separate_sign_up_flow_box'
+- else
+  = render 'devise/shared/signup_box'
 
 = render 'devise/shared/sign_in_link'

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

@@ -4,7 +4,8 @@
   - if form_based_providers.any?
     = render 'devise/shared/tabs_ldap'
   - else
-    = render 'devise/shared/tabs_normal'
+    - unless use_experimental_separate_sign_up_flow?
+      = render 'devise/shared/tabs_normal'
   .tab-content
     - if password_authentication_enabled_for_web? || ldap_enabled? || crowd_enabled?
       = render 'devise/shared/signin_box'

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

+- max_name_length = 128
+- max_username_length = 255
+.signup-box.p-3.mb-2
+  .signup-body
+    = form_for(resource, as: "new_#{resource_name}", url: registration_path(resource_name), html: { class: "new_new_user gl-show-field-errors", "aria-live" => "assertive" }) do |f|
+      .devise-errors.mt-0
+        = render "devise/shared/error_messages", resource: resource
+      = 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.")
+      .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.")
+        %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...')
+      .form-group
+        = f.label :email, class: 'label-bold'
+        = f.email_field :email, class: "form-control middle", data: { qa_selector: 'new_user_email_field' }, required: true, title: _("Please provide a valid email address.")
+      .form-group.append-bottom-20#password-strength
+        = f.label :password, class: 'label-bold'
+        = f.password_field :password, class: "form-control bottom", data: { qa_selector: 'new_user_password_field' }, required: true, pattern: ".{#{@minimum_password_length},}", title: _("Minimum length is %{minimum_password_length} characters.") % { minimum_password_length: @minimum_password_length }
+        %p.gl-field-hint.text-secondary= _('Minimum length is %{minimum_password_length} characters') % { minimum_password_length: @minimum_password_length }
+      - if Gitlab::CurrentSettings.current_application_settings.enforce_terms?
+        .form-group
+          = check_box_tag :terms_opt_in, '1', false, required: true, data: { qa_selector: 'new_user_accept_terms_checkbox' }
+          = label_tag :terms_opt_in do
+            - terms_link = link_to s_("I accept the|Terms of Service and Privacy Policy"), terms_path, target: "_blank"
+            - accept_terms_label = _("I accept the %{terms_link}") % { terms_link: terms_link }
+            = accept_terms_label.html_safe
+      = render_if_exists 'devise/shared/email_opted_in', f: f
+      .submit-container.mt-3
+        = f.submit _("Register"), class: "btn-register btn btn-block btn-success mb-0 p-2", data: { qa_selector: 'new_user_register_button' }

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

-%p
+%p.text-center
   %span.light
     Already have login and password?
     = link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes')

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

@@ -22,3 +22,8 @@
   .login-box.tab-pane.active{ id: 'login-pane', role: 'tabpanel' }
     .login-body
       = render 'devise/sessions/new_base'
+
+- if use_experimental_separate_sign_up_flow?
+  %p.light.mt-2
+    = _("Don't have an account yet?")
+    = link_to _("Register now"), new_registration_path(:user)

app/views/layouts/_head.html.haml

 - page_description brand_title unless page_description
 
+-# Needs a redirect on the client side since it's using an anchor to distuingish
+-# between sign in and registration. We need to inline the JS to not render
+-# anything from this page beforehand.
+-# Part of an experiment to build a new sign up flow. Will be removed again with
+-# https://gitlab.com/gitlab-org/growth/engineering/issues/64
+- if use_experimental_separate_sign_up_flow? && current_path?("sessions#new")
+  = javascript_tag nonce: true do
+    :plain
+      if (window.location.hash === '#register-pane') {
+        window.location.replace("/users/sign_up")
+      }
+
 - site_name = "GitLab"
 %head{ prefix: "og: http://ogp.me/ns#" }
   %meta{ charset: "utf-8" }

app/views/layouts/devise_experimental_separate_sign_up_flow.html.haml

+!!! 5
+%html.devise-layout-html.navless{ class: system_message_class }
+  = render "layouts/head"
+  %body.ui-indigo.signup-page.application.navless{ class: "#{client_class_list}", data: { page: body_data_page, qa_selector: 'signup_page' } }
+    = header_message
+    = render "layouts/init_client_detection_flags"
+    .page-wrap
+      .container.signup-box-container.navless-container.mt-0
+        = render "layouts/broadcast"
+        .content
+          = render "layouts/flash"
+          .row.mb-3
+            .col-sm-8.offset-sm-2.col-md-6.offset-md-3.new-session-forms-container
+              = render_if_exists 'layouts/devise_help_text'
+              .text-center.signup-heading.mt-3.mb-3
+                = image_tag(image_url('logo.svg'), class: 'gitlab-logo', alt: 'GitLab Logo')
+                %h2= _('Register for GitLab.com')
+              = yield
+      %hr.footer-fixed
+      .footer-container
+        .container
+          .footer-links
+            = link_to _("Help"), help_path
+            = link_to _("About GitLab"), "https://about.gitlab.com/"
+      = footer_message

changelogs/unreleased/16482-split-sign-in-and-sign-up.yml

+---
+title: Experimental separate sign up flow
+merge_request: 16482
+author:
+type: other

locale/gitlab.pot

@@ -5472,6 +5472,9 @@ msgstr ""
 msgid "Domain verification is an essential security measure for public GitLab sites. Users are required to demonstrate they control a domain before it is enabled"
 msgstr ""
 
+msgid "Don't have an account yet?"
+msgstr ""
+
 msgid "Don't paste the private part of the GPG key. Paste the public part which begins with '-----BEGIN PGP PUBLIC KEY BLOCK-----'."
 msgstr ""
 
@@ -13081,6 +13084,12 @@ msgstr ""
 msgid "Register and see your runners for this project."
 msgstr ""
 
+msgid "Register for GitLab.com"
+msgstr ""
+
+msgid "Register now"
+msgstr ""
+
 msgid "Register with two-factor app"
 msgstr ""
 

spec/features/users/signup_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe 'Signup' do
+shared_examples 'Signup' do
   include TermsHelper
 
   before do
@@ -13,8 +13,7 @@ describe 'Signup' do
 
   describe 'username validation', :js do
     before do
-      visit root_path
-      click_link 'Register'
+      visit new_user_registration_path
     end
 
     it 'does not show an error border if the username is available' do
@@ -130,8 +129,7 @@ describe 'Signup' do
 
   describe 'user\'s full name validation', :js do
     before do
-      visit root_path
-      click_link 'Register'
+      visit new_user_registration_path
     end
 
     it 'does not show an error border if the user\'s fullname length is not longer than 128 characters' do
@@ -177,13 +175,17 @@ describe 'Signup' do
         end
 
         it 'creates the user account and sends a confirmation email' do
-          visit root_path
+          visit new_user_registration_path
 
-          fill_in 'new_user_name',                with: new_user.name
-          fill_in 'new_user_username',            with: new_user.username
-          fill_in 'new_user_email',               with: new_user.email
-          fill_in 'new_user_email_confirmation',  with: new_user.email
-          fill_in 'new_user_password',            with: new_user.password
+          fill_in 'new_user_name', with: new_user.name
+          fill_in 'new_user_username', with: new_user.username
+          fill_in 'new_user_email', with: new_user.email
+
+          unless Feature.enabled?(:experimental_separate_sign_up_flow)
+            fill_in 'new_user_email_confirmation', with: new_user.email
+          end
+
+          fill_in 'new_user_password', with: new_user.password
 
           expect { click_button 'Register' }.to change { User.count }.by(1)
 
@@ -198,13 +200,17 @@ describe 'Signup' do
         end
 
         it 'creates the user account and sends a confirmation email' do
-          visit root_path
+          visit new_user_registration_path
+
+          fill_in 'new_user_name', with: new_user.name
+          fill_in 'new_user_username', with: new_user.username
+          fill_in 'new_user_email', with: new_user.email
+
+          unless Feature.enabled?(:experimental_separate_sign_up_flow)
+            fill_in 'new_user_email_confirmation', with: new_user.email
+          end
 
-          fill_in 'new_user_name',                with: new_user.name
-          fill_in 'new_user_username',            with: new_user.username
-          fill_in 'new_user_email',               with: new_user.email
-          fill_in 'new_user_email_confirmation',  with: new_user.email
-          fill_in 'new_user_password',            with: new_user.password
+          fill_in 'new_user_password', with: new_user.password
 
           expect { click_button 'Register' }.to change { User.count }.by(1)
 
@@ -216,13 +222,17 @@ describe 'Signup' do
 
     context "when sigining up with different cased emails" do
       it "creates the user successfully" do
-        visit root_path
+        visit new_user_registration_path
 
-        fill_in 'new_user_name',                with: new_user.name
-        fill_in 'new_user_username',            with: new_user.username
-        fill_in 'new_user_email',               with: new_user.email
-        fill_in 'new_user_email_confirmation',  with: new_user.email.capitalize
-        fill_in 'new_user_password',            with: new_user.password
+        fill_in 'new_user_name', with: new_user.name
+        fill_in 'new_user_username', with: new_user.username
+        fill_in 'new_user_email', with: new_user.email
+
+        unless Feature.enabled?(:experimental_separate_sign_up_flow)
+          fill_in 'new_user_email_confirmation', with: new_user.email.capitalize
+        end
+
+        fill_in 'new_user_password', with: new_user.password
         click_button "Register"
 
         expect(current_path).to eq dashboard_projects_path
@@ -236,13 +246,17 @@ describe 'Signup' do
       end
 
       it 'creates the user account and goes to dashboard' do
-        visit root_path
+        visit new_user_registration_path
+
+        fill_in 'new_user_name', with: new_user.name
+        fill_in 'new_user_username', with: new_user.username
+        fill_in 'new_user_email', with: new_user.email
 
-        fill_in 'new_user_name',                with: new_user.name
-        fill_in 'new_user_username',            with: new_user.username
-        fill_in 'new_user_email',               with: new_user.email
-        fill_in 'new_user_email_confirmation',  with: new_user.email
-        fill_in 'new_user_password',            with: new_user.password
+        unless Feature.enabled?(:experimental_separate_sign_up_flow)
+          fill_in 'new_user_email_confirmation', with: new_user.email
+        end
+
+        fill_in 'new_user_password', with: new_user.password
         click_button "Register"
 
         expect(current_path).to eq dashboard_projects_path
@@ -255,28 +269,34 @@ describe 'Signup' do
     it "displays the errors" do
       existing_user = create(:user)
 
-      visit root_path
+      visit new_user_registration_path
 
-      fill_in 'new_user_name',     with: new_user.name
+      fill_in 'new_user_name', with: new_user.name
       fill_in 'new_user_username', with: new_user.username
-      fill_in 'new_user_email',    with: existing_user.email
+      fill_in 'new_user_email', with: existing_user.email
       fill_in 'new_user_password', with: new_user.password
       click_button "Register"
 
       expect(current_path).to eq user_registration_path
-      expect(page).to have_content("errors prohibited this user from being saved")
-      expect(page).to have_content("Email has already been taken")
-      expect(page).to have_content("Email confirmation doesn't match")
+
+      if Feature.enabled?(:experimental_separate_sign_up_flow)
+        expect(page).to have_content("error prohibited this user from being saved")
+        expect(page).to have_content("Email has already been taken")
+      else
+        expect(page).to have_content("errors prohibited this user from being saved")
+        expect(page).to have_content("Email has already been taken")
+        expect(page).to have_content("Email confirmation doesn't match")
+      end
     end
 
     it 'does not redisplay the password' do
       existing_user = create(:user)
 
-      visit root_path
+      visit new_user_registration_path
 
-      fill_in 'new_user_name',     with: new_user.name
+      fill_in 'new_user_name', with: new_user.name
       fill_in 'new_user_username', with: new_user.username
-      fill_in 'new_user_email',    with: existing_user.email
+      fill_in 'new_user_email', with: existing_user.email
       fill_in 'new_user_password', with: new_user.password
       click_button "Register"
 
@@ -291,13 +311,17 @@ describe 'Signup' do
     end
 
     it 'requires the user to check the checkbox' do
-      visit root_path
+      visit new_user_registration_path
+
+      fill_in 'new_user_name', with: new_user.name
+      fill_in 'new_user_username', with: new_user.username
+      fill_in 'new_user_email', with: new_user.email
+
+      unless Feature.enabled?(:experimental_separate_sign_up_flow)
+        fill_in 'new_user_email_confirmation', with: new_user.email
+      end
 
-      fill_in 'new_user_name',                with: new_user.name
-      fill_in 'new_user_username',            with: new_user.username
-      fill_in 'new_user_email',               with: new_user.email
-      fill_in 'new_user_email_confirmation',  with: new_user.email
-      fill_in 'new_user_password',            with: new_user.password
+      fill_in 'new_user_password', with: new_user.password
 
       click_button 'Register'
 
@@ -306,13 +330,17 @@ describe 'Signup' do
     end
 
     it 'asks the user to accept terms before going to the dashboard' do
-      visit root_path
+      visit new_user_registration_path
+
+      fill_in 'new_user_name', with: new_user.name
+      fill_in 'new_user_username', with: new_user.username
+      fill_in 'new_user_email', with: new_user.email
+
+      unless Feature.enabled?(:experimental_separate_sign_up_flow)
+        fill_in 'new_user_email_confirmation', with: new_user.email
+      end
 
-      fill_in 'new_user_name',                with: new_user.name
-      fill_in 'new_user_username',            with: new_user.username
-      fill_in 'new_user_email',               with: new_user.email
-      fill_in 'new_user_email_confirmation',  with: new_user.email
-      fill_in 'new_user_password',            with: new_user.password
+      fill_in 'new_user_password', with: new_user.password
       check :terms_opt_in
 
       click_button "Register"
@@ -321,3 +349,20 @@ describe 'Signup' do
     end
   end
 end
+
+describe 'With original flow' do
+  it_behaves_like 'Signup' do
+    before do
+      stub_feature_flags(experimental_separate_sign_up_flow: false)
+    end
+  end
+end
+
+describe 'With experimental flow on GitLab.com' do
+  it_behaves_like 'Signup' do
+    before do
+      expect(Gitlab).to receive(:com?).and_return(true).at_least(:once)
+      stub_feature_flags(experimental_separate_sign_up_flow: true)
+    end
+  end
+end

spec/requests/api/users_spec.rb

@@ -634,10 +634,47 @@ describe API::Users do
   end
 
   describe "GET /users/sign_up" do
-    it "redirects to sign in page" do
-      get "/users/sign_up"
-      expect(response).to have_gitlab_http_status(302)
-      expect(response).to redirect_to(new_user_session_path)
+    context 'when experimental_separate_sign_up_flow is active' do
+      before do
+        stub_feature_flags(experimental_separate_sign_up_flow: true)
+      end
+
+      context 'on gitlab.com' do
+        before do
+          allow(::Gitlab).to receive(:com?).and_return(true)
+        end
+
+        it "shows sign up page" do
+          get "/users/sign_up"
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to render_template(:new)
+        end
+      end
+
+      context 'not on gitlab.com' do
+        before do
+          allow(::Gitlab).to receive(:com?).and_return(false)
+        end
+
+        it "redirects to sign in page" do
+          get "/users/sign_up"
+          expect(response).to have_gitlab_http_status(302)
+          expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane'))
+        end
+      end
+    end
+
+    context 'when experimental_separate_sign_up_flow is not active' do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+        stub_feature_flags(experimental_separate_sign_up_flow: false)
+      end
+
+      it "redirects to sign in page" do
+        get "/users/sign_up"
+        expect(response).to have_gitlab_http_status(302)
+        expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane'))
+      end
     end
   end