Show welcome page after sign up

The sign up flow experiment consists of multiple parts, where one part
is having a second step after signing up where we welcome the user and
ask for their role.

This MR extracts this behaviour and makes it the default of the sign up
flow for all users.

app/controllers/application_controller.rb

@@ -551,13 +551,9 @@ class ApplicationController < ActionController::Base
     "#{self.class.name}##{action_name}"
   end
 
-  # A user requires a role and have the setup_for_company attribute set when they are part of the experimental signup
-  # flow (executed by the Growth team). Users are redirected to the welcome page when their role is required and the
-  # experiment is enabled for the current user.
   def required_signup_info
     return unless current_user
     return unless current_user.role_required?
-    return unless experiment_enabled?(:signup_flow)
 
     store_location_for :user, request.fullpath
 

app/controllers/registrations_controller.rb

@@ -35,9 +35,9 @@ class RegistrationsController < Devise::RegistrationsController
       yield new_user if block_given?
     end
 
-    # Do not show the signed_up notice message when the signup_flow experiment is enabled.
-    # Instead, show it after successfully updating the role.
-    flash[:notice] = nil if experiment_enabled?(:signup_flow)
+    # Devise sets a flash message on `create` for a successful signup,
+    # we want to show this message after the welcome page.
+    flash[:notice] = nil
   rescue Gitlab::Access::AccessDeniedError
     redirect_to(new_user_session_path)
   end
@@ -89,7 +89,7 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def set_role_required(new_user)
-    new_user.set_role_required! if new_user.persisted? && experiment_enabled?(:signup_flow)
+    new_user.set_role_required! if new_user.persisted?
   end
 
   def destroy_confirmation_valid?
@@ -115,9 +115,7 @@ class RegistrationsController < Devise::RegistrationsController
   def after_sign_up_path_for(user)
     Gitlab::AppLogger.info(user_created_message(confirmed: user.confirmed?))
 
-    return users_sign_up_welcome_path if experiment_enabled?(:signup_flow)
-
-    path_for_signed_in_user(user)
+    users_sign_up_welcome_path
   end
 
   def after_inactive_sign_up_path_for(resource)
@@ -215,7 +213,7 @@ class RegistrationsController < Devise::RegistrationsController
   # 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 experiment_enabled?(:signup_flow)
+    if %w(welcome update_registration).include?(action_name) || experiment_enabled?(:signup_flow)
       'devise_experimental_separate_sign_up_flow'
     else
       'devise'

app/models/user.rb

@@ -1715,9 +1715,6 @@ class User < ApplicationRecord
     [last_activity, last_sign_in].compact.max
   end
 
-  # Below is used for the signup_flow experiment. Should be removed
-  # when experiment finishes.
-  # See https://gitlab.com/gitlab-org/growth/engineering/issues/64
   REQUIRES_ROLE_VALUE = 99
 
   def role_required?
@@ -1727,7 +1724,6 @@ class User < ApplicationRecord
   def set_role_required!
     update_column(:role, REQUIRES_ROLE_VALUE)
   end
-  # End of signup_flow experiment methods
 
   def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil)
     callouts = self.callouts.with_feature_name(feature_name)

changelogs/unreleased/nicolasdular-introduce-welcome-page.yml

+---
+title: Show welcome page after sign up
+merge_request: 41662
+author:
+type: added

ee/spec/features/signup_spec.rb

@@ -49,34 +49,27 @@ RSpec.describe 'Signup on EE' do
       end
     end
 
-    context 'when role is required' do
-      before do
-        stub_experiment(signup_flow: true)
-        stub_experiment_for_user(signup_flow: true)
-      end
-
-      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_first_name', with: user_attrs[:name].split(' ').first
-        fill_in 'new_user_last_name', with: user_attrs[:name].split(' ').last
-        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]
-        click_button 'Register'
-        visit new_project_path
-
-        expect(page).to have_current_path(users_sign_up_welcome_path)
-
-        select 'Software Developer', from: 'user_role'
-        choose 'user_setup_for_company_true'
-        click_button 'Get started!'
-        user = User.find_by_username(user_attrs[:username])
-
-        expect(user.software_developer_role?).to be_truthy
-        expect(user.setup_for_company).to be_truthy
-        expect(page).to have_current_path(new_project_path)
-      end
+    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_email_confirmation', with: user_attrs[:email]
+      fill_in 'new_user_password', with: user_attrs[:password]
+      click_button 'Register'
+      visit new_project_path
+
+      expect(page).to have_current_path(users_sign_up_welcome_path)
+
+      select 'Software Developer', from: 'user_role'
+      choose 'user_setup_for_company_true'
+      click_button 'Get started!'
+      user = User.find_by_username(user_attrs[:username])
+
+      expect(user.software_developer_role?).to be_truthy
+      expect(user.setup_for_company).to be_truthy
+      expect(page).to have_current_path(new_project_path)
     end
   end
 

ee/spec/features/trial_registrations/signup_spec.rb

@@ -44,6 +44,10 @@ RSpec.describe 'Trial Sign Up', :js do
 
         wait_for_requests
 
+        select 'Software Developer', from: 'user_role'
+        choose 'user_setup_for_company_true'
+        click_button 'Continue'
+
         expect(current_path).to eq(new_trial_path)
         expect(page).to have_content('Start your Free Gold Trial')
       end

spec/controllers/application_controller_spec.rb

@@ -795,13 +795,8 @@ RSpec.describe ApplicationController do
     end
 
     let(:user) { create(:user) }
-    let(:experiment_enabled) { true }
 
-    before do
-      stub_experiment_for_user(signup_flow: experiment_enabled)
-    end
-
-    context 'experiment enabled and user with required role' do
+    context 'user with required role' do
       before do
         user.set_role_required!
         sign_in(user)
@@ -811,7 +806,7 @@ RSpec.describe ApplicationController do
       it { is_expected.to redirect_to users_sign_up_welcome_path }
     end
 
-    context 'experiment enabled and user without a required role' do
+    context 'user without a required role' do
       before do
         sign_in(user)
         get :index
@@ -819,43 +814,31 @@ RSpec.describe ApplicationController do
 
       it { is_expected.not_to redirect_to users_sign_up_welcome_path }
     end
+  end
 
-    context 'experiment disabled' do
-      let(:experiment_enabled) { false }
+  describe 'rescue_from Gitlab::Auth::IpBlacklisted' do
+    controller(described_class) do
+      skip_before_action :authenticate_user!
 
-      before do
-        user.set_role_required!
-        sign_in(user)
-        get :index
+      def index
+        raise Gitlab::Auth::IpBlacklisted
       end
-
-      it { is_expected.not_to redirect_to users_sign_up_welcome_path }
     end
 
-    describe 'rescue_from Gitlab::Auth::IpBlacklisted' do
-      controller(described_class) do
-        skip_before_action :authenticate_user!
-
-        def index
-          raise Gitlab::Auth::IpBlacklisted
-        end
-      end
-
-      it 'returns a 403 and logs the request' do
-        expect(Gitlab::AuthLogger).to receive(:error).with({
-          message: 'Rack_Attack',
-          env: :blocklist,
-          remote_ip: '1.2.3.4',
-          request_method: 'GET',
-          path: '/anonymous'
-        })
+    it 'returns a 403 and logs the request' do
+      expect(Gitlab::AuthLogger).to receive(:error).with({
+        message: 'Rack_Attack',
+        env: :blocklist,
+        remote_ip: '1.2.3.4',
+        request_method: 'GET',
+        path: '/anonymous'
+      })
 
-        request.remote_addr = '1.2.3.4'
+      request.remote_addr = '1.2.3.4'
 
-        get :index
+      get :index
 
-        expect(response).to have_gitlab_http_status(:forbidden)
-      end
+      expect(response).to have_gitlab_http_status(:forbidden)
     end
   end
 

spec/controllers/registrations_controller_spec.rb

@@ -128,7 +128,7 @@ RSpec.describe RegistrationsController do
             post(:create, params: user_params)
 
             expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
-            expect(response).to redirect_to(dashboard_projects_path)
+            expect(response).to redirect_to(users_sign_up_welcome_path)
           end
         end
       end
@@ -164,10 +164,10 @@ RSpec.describe RegistrationsController do
         expect(flash[:alert]).to eq(_('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'))
       end
 
-      it 'redirects to the dashboard when the reCAPTCHA is solved' do
+      it 'redirects to the welcome page when the reCAPTCHA is solved' do
         post(:create, params: user_params)
 
-        expect(flash[:notice]).to eq(I18n.t('devise.registrations.signed_up'))
+        expect(response).to redirect_to(users_sign_up_welcome_path)
       end
     end
 
@@ -464,42 +464,39 @@ RSpec.describe RegistrationsController do
   describe '#welcome' do
     subject { get :welcome }
 
-    context 'signup_flow experiment enabled' do
-      before do
-        stub_experiment_for_user(signup_flow: true)
-      end
+    it 'renders the devise_experimental_separate_sign_up_flow layout' do
+      sign_in(create(:user))
 
-      it 'renders the devise_experimental_separate_sign_up_flow layout' do
-        sign_in(create(:user))
+      expected_layout = Gitlab.ee? ? :checkout : :devise_experimental_separate_sign_up_flow
 
-        expected_layout = Gitlab.ee? ? :checkout : :devise_experimental_separate_sign_up_flow
+      expect(subject).to render_template(expected_layout)
+    end
 
-        expect(subject).to render_template(expected_layout)
+    context '2FA is required from group' do
+      before do
+        user = create(:user, require_two_factor_authentication_from_group: true)
+        sign_in(user)
       end
 
-      context '2FA is required from group' do
-        before do
-          user = create(:user, require_two_factor_authentication_from_group: true)
-          sign_in(user)
-        end
-
-        it 'does not perform a redirect' do
-          expect(subject).not_to redirect_to(profile_two_factor_auth_path)
-        end
+      it 'does not perform a redirect' do
+        expect(subject).not_to redirect_to(profile_two_factor_auth_path)
       end
     end
+  end
 
-    context 'signup_flow experiment disabled' do
-      before do
-        sign_in(create(:user))
-        stub_experiment_for_user(signup_flow: false)
-      end
+  describe '#update_registration' do
+    subject(:update_registration) do
+      patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } }
+    end
 
-      it 'renders the devise layout' do
-        expected_layout = Gitlab.ee? ? :checkout : :devise
+    before do
+      sign_in(create(:user))
+    end
 
-        expect(subject).to render_template(expected_layout)
-      end
+    it 'sets flash message' do
+      subject
+
+      expect(flash[:notice]).to eq(I18n.t('devise.registrations.signed_up'))
     end
   end
 end

spec/features/invites_spec.rb

@@ -38,6 +38,11 @@ RSpec.describe 'Invites', :aggregate_failures do
     click_button 'Sign in'
   end
 
+  def fill_in_welcome_form
+    select 'Software Developer', from: 'user_role'
+    click_button 'Get started!'
+  end
+
   context 'when signed out' do
     before do
       visit invite_path(group_invite.raw_invite_token)
@@ -94,6 +99,7 @@ RSpec.describe 'Invites', :aggregate_failures do
 
         it 'signs up and redirects to the dashboard page with all the projects/groups invitations automatically accepted' do
           fill_in_sign_up_form(new_user)
+          fill_in_welcome_form
 
           expect(current_path).to eq(dashboard_projects_path)
           expect(page).to have_content(project.full_name)
@@ -108,6 +114,7 @@ RSpec.describe 'Invites', :aggregate_failures do
 
           it 'signs up and redirects to the invitation page' do
             fill_in_sign_up_form(new_user)
+            fill_in_welcome_form
 
             expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
           end
@@ -126,6 +133,7 @@ RSpec.describe 'Invites', :aggregate_failures do
             fill_in_sign_up_form(new_user)
             confirm_email(new_user)
             fill_in_sign_in_form(new_user)
+            fill_in_welcome_form
 
             expect(current_path).to eq(root_path)
             expect(page).to have_content(project.full_name)
@@ -143,6 +151,7 @@ RSpec.describe 'Invites', :aggregate_failures do
 
           it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do
             fill_in_sign_up_form(new_user)
+            fill_in_welcome_form
             confirm_email(new_user)
 
             expect(current_path).to eq(root_path)
@@ -156,6 +165,7 @@ RSpec.describe 'Invites', :aggregate_failures do
 
         it "doesn't accept invitations until the user confirms their email" do
           fill_in_sign_up_form(new_user)
+          fill_in_welcome_form
           sign_in(owner)
 
           visit project_project_members_path(project)
@@ -175,6 +185,7 @@ RSpec.describe 'Invites', :aggregate_failures do
               fill_in_sign_up_form(new_user)
               confirm_email(new_user)
               fill_in_sign_in_form(new_user)
+              fill_in_welcome_form
 
               expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
             end
@@ -188,6 +199,7 @@ RSpec.describe 'Invites', :aggregate_failures do
 
             it 'signs up and redirects to the invitation page' do
               fill_in_sign_up_form(new_user)
+              fill_in_welcome_form
 
               expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
             end

spec/features/users/signup_spec.rb

@@ -187,12 +187,7 @@ RSpec.shared_examples 'Signup' do
 
           expect { click_button 'Register' }.to change { User.count }.by(1)
 
-          if Gitlab::Experimentation.enabled?(:signup_flow)
-            expect(current_path).to eq users_sign_up_welcome_path
-          else
-            expect(current_path).to eq dashboard_projects_path
-            expect(page).to have_content("Please check your email (#{new_user.email}) to verify that you own this address and unlock the power of CI/CD.")
-          end
+          expect(current_path).to eq users_sign_up_welcome_path
         end
       end
     end
@@ -215,12 +210,7 @@ RSpec.shared_examples 'Signup' do
         fill_in 'new_user_password', with: new_user.password
         click_button "Register"
 
-        if Gitlab::Experimentation.enabled?(:signup_flow)
-          expect(current_path).to eq users_sign_up_welcome_path
-        else
-          expect(current_path).to eq dashboard_projects_path
-          expect(page).to have_content("Welcome! You have signed up successfully.")
-        end
+        expect(current_path).to eq users_sign_up_welcome_path
       end
     end
 
@@ -246,12 +236,7 @@ RSpec.shared_examples 'Signup' do
         fill_in 'new_user_password', with: new_user.password
         click_button "Register"
 
-        if Gitlab::Experimentation.enabled?(:signup_flow)
-          expect(current_path).to eq users_sign_up_welcome_path
-        else
-          expect(current_path).to eq dashboard_projects_path
-          expect(page).to have_content("Welcome! You have signed up successfully.")
-        end
+        expect(current_path).to eq users_sign_up_welcome_path
       end
     end
   end
@@ -354,11 +339,7 @@ RSpec.shared_examples 'Signup' do
 
       click_button "Register"
 
-      if Gitlab::Experimentation.enabled?(:signup_flow)
-        expect(current_path).to eq users_sign_up_welcome_path
-      else
-        expect(current_path).to eq dashboard_projects_path
-      end
+      expect(current_path).to eq users_sign_up_welcome_path
     end
   end
 
@@ -425,6 +406,37 @@ RSpec.shared_examples 'Signup' do
       end
     end
   end
+
+  it 'redirects to step 2 of the signup process, sets the role and redirects back' do
+    new_user = build_stubbed(:user)
+    visit new_user_registration_path
+
+    fill_in 'new_user_username', with: new_user.username
+    fill_in 'new_user_email', with: new_user.email
+
+    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
+
+    fill_in 'new_user_password', with: new_user.password
+    click_button 'Register'
+    visit new_project_path
+
+    expect(page).to have_current_path(users_sign_up_welcome_path)
+
+    select 'Software Developer', from: 'user_role'
+    click_button 'Get started!'
+    new_user = User.find_by_username(new_user.username)
+
+    expect(new_user.software_developer_role?).to be_truthy
+    expect(new_user.setup_for_company).to be_nil
+    expect(page).to have_current_path(new_project_path)
+    expect(page).to have_content("Welcome! You have signed up successfully.")
+  end
 end
 
 RSpec.shared_examples 'Signup name validation' do |field, max_length|
@@ -485,30 +497,6 @@ RSpec.describe 'With experimental flow' do
   it_behaves_like 'Signup name validation', 'new_user_first_name', 127
   it_behaves_like 'Signup name validation', 'new_user_last_name', 127
 
-  context 'when role is required' do
-    it 'redirects to step 2 of the signup process, sets the role and redirects back' 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
-      click_button 'Register'
-      visit new_project_path
-
-      expect(page).to have_current_path(users_sign_up_welcome_path)
-
-      select 'Software Developer', from: 'user_role'
-      click_button 'Get started!'
-      new_user = User.find_by_username(new_user.username)
-
-      expect(new_user.software_developer_role?).to be_truthy
-      expect(new_user.setup_for_company).to be_nil
-      expect(page).to have_current_path(new_project_path)
-    end
-  end
-
   context 'when terms_opt_in experimental is enabled' do
     include TermsHelper