Merge branch 'nicolasdular/introduce-welcome-page' into 'master'

Show welcome page after sign up

See merge request gitlab-org/gitlab!41662

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,19 +49,13 @@ 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_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
@@ -78,7 +72,6 @@ RSpec.describe 'Signup on EE' do
       expect(page).to have_current_path(new_project_path)
     end
   end
-  end
 
   context 'not for Gitlab.com' do
     before do

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,17 +814,6 @@ RSpec.describe ApplicationController do
 
       it { is_expected.not_to redirect_to users_sign_up_welcome_path }
     end
-
-    context 'experiment disabled' do
-      let(:experiment_enabled) { false }
-
-      before do
-        user.set_role_required!
-        sign_in(user)
-        get :index
-      end
-
-      it { is_expected.not_to redirect_to users_sign_up_welcome_path }
   end
 
   describe 'rescue_from Gitlab::Auth::IpBlacklisted' do
@@ -857,7 +841,6 @@ RSpec.describe ApplicationController do
       expect(response).to have_gitlab_http_status(:forbidden)
     end
   end
-  end
 
   describe '#set_current_context' do
     controller(described_class) do

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,11 +464,6 @@ 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))
 
@@ -489,17 +484,19 @@ RSpec.describe RegistrationsController do
     end
   end
 
-    context 'signup_flow experiment disabled' do
+  describe '#update_registration' do
+    subject(:update_registration) do
+      patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } }
+    end
+
     before do
       sign_in(create(:user))
-        stub_experiment_for_user(signup_flow: false)
     end
 
-      it 'renders the devise layout' do
-        expected_layout = Gitlab.ee? ? :checkout : :devise
+    it 'sets flash message' do
+      subject
 
-        expect(subject).to render_template(expected_layout)
-      end
+      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
         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
       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
       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
     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