Merge branch 'nicolasdular/split-welcome-controller' into 'master'

Extract welcome page from registration controller

See merge request gitlab-org/gitlab!46830

app/controllers/registrations/welcome_controller.rb

+# frozen_string_literal: true
+
+module Registrations
+  class WelcomeController < ApplicationController
+    layout 'welcome'
+    skip_before_action :authenticate_user!, :required_signup_info, :check_two_factor_requirement, only: [:show, :update]
+    before_action :require_current_user
+
+    feature_category :authentication_and_authorization
+
+    def show
+      return redirect_to path_for_signed_in_user(current_user) if completed_welcome_step?
+    end
+
+    def update
+      result = ::Users::SignupService.new(current_user, update_params).execute
+
+      if result[:status] == :success
+        process_gitlab_com_tracking
+
+        return redirect_to new_users_sign_up_group_path if experiment_enabled?(:onboarding_issues) && show_onboarding_issues_experiment?
+
+        redirect_to path_for_signed_in_user(current_user)
+      else
+        render :show
+      end
+    end
+
+    private
+
+    def require_current_user
+      return redirect_to new_user_registration_path unless current_user
+    end
+
+    def completed_welcome_step?
+      current_user.role.present? && !current_user.setup_for_company.nil?
+    end
+
+    def process_gitlab_com_tracking
+      return false unless ::Gitlab.com?
+      return false unless show_onboarding_issues_experiment?
+
+      track_experiment_event(:onboarding_issues, 'signed_up')
+      record_experiment_user(:onboarding_issues)
+    end
+
+    def update_params
+      params.require(:user).permit(:role, :setup_for_company)
+    end
+
+    def requires_confirmation?(user)
+      return false if user.confirmed?
+      return false if Feature.enabled?(:soft_email_confirmation)
+      return false if experiment_enabled?(:signup_flow)
+
+      true
+    end
+
+    def path_for_signed_in_user(user)
+      return users_almost_there_path if requires_confirmation?(user)
+
+      stored_location_for(user) || dashboard_projects_path
+    end
+
+    def show_onboarding_issues_experiment?
+      !helpers.in_subscription_flow? &&
+        !helpers.in_invitation_flow? &&
+        !helpers.in_oauth_flow? &&
+        !helpers.in_trial_flow?
+    end
+  end
+end
+
+Registrations::WelcomeController.prepend_if_ee('EE::Registrations::WelcomeController')

app/controllers/registrations_controller.rb

@@ -8,9 +8,8 @@ class RegistrationsController < Devise::RegistrationsController
 
   BLOCKED_PENDING_APPROVAL_STATE = 'blocked_pending_approval'.freeze
 
-  layout :choose_layout
+  layout 'devise'
 
-  skip_before_action :required_signup_info, :check_two_factor_requirement, only: [:welcome, :update_registration]
   prepend_before_action :check_captcha, only: :create
   before_action :whitelist_query_limiting, :ensure_destroy_prerequisites_met, only: [:destroy]
   before_action :load_recaptcha, only: :new
@@ -49,30 +48,6 @@ class RegistrationsController < Devise::RegistrationsController
     end
   end
 
-  def welcome
-    return redirect_to new_user_registration_path unless current_user
-    return redirect_to path_for_signed_in_user(current_user) if current_user.role.present? && !current_user.setup_for_company.nil?
-  end
-
-  def update_registration
-    return redirect_to new_user_registration_path unless current_user
-
-    result = ::Users::SignupService.new(current_user, update_registration_params).execute
-
-    if result[:status] == :success
-      if ::Gitlab.com? && show_onboarding_issues_experiment?
-        track_experiment_event(:onboarding_issues, 'signed_up')
-        record_experiment_user(:onboarding_issues)
-      end
-
-      return redirect_to new_users_sign_up_group_path if experiment_enabled?(:onboarding_issues) && show_onboarding_issues_experiment?
-
-      redirect_to path_for_signed_in_user(current_user)
-    else
-      render :welcome
-    end
-  end
-
   protected
 
   def persist_accepted_terms_if_required(new_user)
@@ -160,10 +135,6 @@ class RegistrationsController < Devise::RegistrationsController
     params.require(:user).permit(:username, :email, :name, :first_name, :last_name, :password)
   end
 
-  def update_registration_params
-    params.require(:user).permit(:role, :setup_for_company)
-  end
-
   def resource_name
     :user
   end
@@ -180,43 +151,10 @@ class RegistrationsController < Devise::RegistrationsController
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42380')
   end
 
-  def path_for_signed_in_user(user)
-    if requires_confirmation?(user)
-      users_almost_there_path
-    else
-      stored_location_for(user) || dashboard_projects_path
-    end
-  end
-
-  def requires_confirmation?(user)
-    return false if user.confirmed?
-    return false if Feature.enabled?(:soft_email_confirmation)
-    return false if experiment_enabled?(:signup_flow)
-
-    true
-  end
-
   def load_recaptcha
     Gitlab::Recaptcha.load_configurations!
   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 %w(welcome update_registration).include?(action_name)
-      'welcome'
-    else
-      'devise'
-    end
-  end
-
-  def show_onboarding_issues_experiment?
-    !helpers.in_subscription_flow? &&
-      !helpers.in_invitation_flow? &&
-      !helpers.in_oauth_flow? &&
-      !helpers.in_trial_flow?
-  end
-
   def set_user_state
     return unless Gitlab::CurrentSettings.require_admin_approval_after_user_signup
 

app/views/registrations/welcome.html.haml → app/views/registrations/welcome/show.html.haml

@@ -8,7 +8,7 @@
       %p
         .gl-text-center= html_escape(_('In order to personalize your experience with GitLab%{br_tag}we would like to know a bit more about you.')) % { br_tag: '<br/>'.html_safe }
 
-      = form_for(current_user, url: users_sign_up_update_registration_path, html: { class: 'card gl-w-full! gl-p-5', 'aria-live' => 'assertive' }) do |f|
+      = form_for(current_user, url: users_sign_up_welcome_path, html: { class: 'card gl-w-full! gl-p-5', 'aria-live' => 'assertive' }) do |f|
         .devise-errors
           = render 'devise/shared/error_messages', resource: current_user
         .row

config/routes.rb

@@ -45,8 +45,7 @@ Rails.application.routes.draw do
 
   # Sign up
   scope path: '/users/sign_up', module: :registrations, as: :users_sign_up do
-    get :welcome
-    patch :update_registration
+    resource :welcome, only: [:show, :update], controller: 'welcome'
     resource :experience_level, only: [:show, :update]
 
     Gitlab.ee do

ee/app/controllers/ee/registrations/welcome_controller.rb

+# frozen_string_literal: true
+
+module EE
+  module Registrations
+    module WelcomeController
+      extend ::Gitlab::Utils::Override
+
+      private
+
+      override :update_params
+      def update_params
+        clean_params = super.merge(params.require(:user).permit(:email_opted_in))
+
+        clean_params[:email_opted_in] = '1' if clean_params[:setup_for_company] == 'true'
+
+        if clean_params[:email_opted_in] == '1'
+          clean_params[:email_opted_in_ip] = request.remote_ip
+          clean_params[:email_opted_in_source_id] = User::EMAIL_OPT_IN_SOURCE_ID_GITLAB_COM
+          clean_params[:email_opted_in_at] = Time.zone.now
+        end
+
+        clean_params
+      end
+    end
+  end
+end

ee/app/controllers/ee/registrations_controller.rb

@@ -13,19 +13,5 @@ module EE
 
       super(confirmed: confirmed) + ", experiments:#{experiments}"
     end
-
-    def update_registration_params
-      clean_params = super.merge(params.require(:user).permit(:email_opted_in))
-
-      clean_params[:email_opted_in] = '1' if clean_params[:setup_for_company] == 'true'
-
-      if clean_params[:email_opted_in] == '1'
-        clean_params[:email_opted_in_ip] = request.remote_ip
-        clean_params[:email_opted_in_source_id] = User::EMAIL_OPT_IN_SOURCE_ID_GITLAB_COM
-        clean_params[:email_opted_in_at] = Time.zone.now
-      end
-
-      clean_params
-    end
   end
 end

ee/app/helpers/ee/registrations_helper.rb

@@ -4,32 +4,6 @@ module EE
   module RegistrationsHelper
     include ::Gitlab::Utils::StrongMemoize
 
-    def in_subscription_flow?
-      redirect_path == new_subscriptions_path
-    end
-
-    def in_trial_flow?
-      redirect_path == new_trial_path
-    end
-
-    def in_invitation_flow?
-      redirect_path&.starts_with?('/-/invites/')
-    end
-
-    def in_oauth_flow?
-      redirect_path&.starts_with?(oauth_authorization_path)
-    end
-
-    def setup_for_company_label_text
-      if in_subscription_flow?
-        _('Who will be using this GitLab subscription?')
-      elsif in_trial_flow?
-        _('Who will be using this GitLab trial?')
-      else
-        _('Who will be using GitLab?')
-      end
-    end
-
     def visibility_level_options
       available_visibility_levels(@group).map do |level|
         {
@@ -40,34 +14,6 @@ module EE
       end
     end
 
-    def show_signup_flow_progress_bar?
-      return true if in_subscription_flow?
-      return false if in_invitation_flow? || in_oauth_flow? || in_trial_flow?
-
-      onboarding_issues_experiment_enabled?
-    end
-
-    def welcome_submit_button_text
-      continue = _('Continue')
-      get_started = _('Get started!')
-
-      return continue if in_subscription_flow? || in_trial_flow?
-      return get_started if in_invitation_flow? || in_oauth_flow?
-
-      onboarding_issues_experiment_enabled? ? continue : get_started
-    end
-
-    def data_attributes_for_progress_bar_js_component
-      {
-        is_in_subscription_flow: in_subscription_flow?.to_s,
-        is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled?.to_s
-      }
-    end
-
-    def skip_setup_for_company?
-      current_user.members.any?
-    end
-
     private
 
     def redirect_path
@@ -76,9 +22,5 @@ module EE
         URI.parse(redirect_to).path if redirect_to
       end
     end
-
-    def onboarding_issues_experiment_enabled?
-      experiment_enabled?(:onboarding_issues)
-    end
   end
 end

ee/app/helpers/ee/welcome_helper.rb

+# frozen_string_literal: true
+
+module EE
+  module WelcomeHelper
+    include ::Gitlab::Utils::StrongMemoize
+
+    def in_subscription_flow?
+      redirect_path == new_subscriptions_path
+    end
+
+    def in_trial_flow?
+      redirect_path == new_trial_path
+    end
+
+    def in_invitation_flow?
+      redirect_path&.starts_with?('/-/invites/')
+    end
+
+    def in_oauth_flow?
+      redirect_path&.starts_with?(oauth_authorization_path)
+    end
+
+    def setup_for_company_label_text
+      if in_subscription_flow?
+        _('Who will be using this GitLab subscription?')
+      elsif in_trial_flow?
+        _('Who will be using this GitLab trial?')
+      else
+        _('Who will be using GitLab?')
+      end
+    end
+
+    def show_signup_flow_progress_bar?
+      return true if in_subscription_flow?
+      return false if in_invitation_flow? || in_oauth_flow? || in_trial_flow?
+
+      onboarding_issues_experiment_enabled?
+    end
+
+    def welcome_submit_button_text
+      continue = _('Continue')
+      get_started = _('Get started!')
+
+      return continue if in_subscription_flow? || in_trial_flow?
+      return get_started if in_invitation_flow? || in_oauth_flow?
+
+      onboarding_issues_experiment_enabled? ? continue : get_started
+    end
+
+    def data_attributes_for_progress_bar_js_component
+      {
+        is_in_subscription_flow: in_subscription_flow?.to_s,
+        is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled?.to_s
+      }
+    end
+
+    def skip_setup_for_company?
+      current_user.members.any?
+    end
+
+    private
+
+    def onboarding_issues_experiment_enabled?
+      experiment_enabled?(:onboarding_issues)
+    end
+  end
+end

ee/app/helpers/subscriptions_helper.rb

@@ -27,7 +27,7 @@ module SubscriptionsHelper
   def new_user?
     return false unless request.referer.present?
 
-    URI.parse(request.referer).path.in?([users_sign_up_welcome_path, users_sign_up_update_registration_path])
+    URI.parse(request.referer).path == users_sign_up_welcome_path
   end
 
   def plan_data

ee/spec/controllers/registrations/welcome_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Registrations::WelcomeController do
+  let_it_be(:user) { create(:user) }
+
+  describe '#update' do
+    let(:setup_for_company) { 'false' }
+    let(:email_opted_in) { '0' }
+
+    subject(:update) do
+      patch :update, params: {
+        user: {
+          role: 'software_developer',
+          setup_for_company: setup_for_company,
+          email_opted_in: email_opted_in
+        }
+      }
+    end
+
+    context 'without a signed in user' do
+      it { is_expected.to redirect_to new_user_registration_path }
+    end
+
+    context 'with a signed in user' do
+      before do
+        sign_in(user)
+      end
+
+      context 'email updates' do
+        context 'when setup for company is false' do
+          context 'when the user opted in' do
+            let(:email_opted_in) { '1' }
+
+            it 'sets the email_opted_in fields' do
+              subject
+
+              expect(controller.current_user.email_opted_in).to be_truthy
+              expect(controller.current_user.email_opted_in_ip).to be_present
+              expect(controller.current_user.email_opted_in_source).to eq('GitLab.com')
+              expect(controller.current_user.email_opted_in_at).not_to be_nil
+            end
+          end
+
+          context 'when user opted out' do
+            let(:email_opted_in) { '0' }
+
+            it 'does not set the rest of the email_opted_in fields' do
+              subject
+
+              expect(controller.current_user.email_opted_in).to be_falsey
+              expect(controller.current_user.email_opted_in_ip).to be_blank
+              expect(controller.current_user.email_opted_in_source).to be_blank
+              expect(controller.current_user.email_opted_in_at).to be_nil
+            end
+          end
+        end
+
+        context 'when setup for company is true' do
+          let(:setup_for_company) { 'true' }
+
+          it 'sets email_opted_in fields' do
+            subject
+
+            expect(controller.current_user.email_opted_in).to be_truthy
+            expect(controller.current_user.email_opted_in_ip).to be_present
+            expect(controller.current_user.email_opted_in_source).to eq('GitLab.com')
+            expect(controller.current_user.email_opted_in_at).not_to be_nil
+          end
+        end
+      end
+
+      describe 'redirection' do
+        it { is_expected.to redirect_to dashboard_projects_path }
+
+        context 'when part of the onboarding issues experiment' do
+          before do
+            stub_experiment_for_user(onboarding_issues: true)
+          end
+
+          it { is_expected.to redirect_to new_users_sign_up_group_path }
+
+          context 'when in subscription flow' do
+            before do
+              allow(controller.helpers).to receive(:in_subscription_flow?).and_return(true)
+            end
+
+            it { is_expected.not_to redirect_to new_users_sign_up_group_path }
+          end
+
+          context 'when in invitation flow' do
+            before do
+              allow(controller.helpers).to receive(:in_invitation_flow?).and_return(true)
+            end
+
+            it { is_expected.not_to redirect_to new_users_sign_up_group_path }
+          end
+
+          context 'when in trial flow' do
+            before do
+              allow(controller.helpers).to receive(:in_trial_flow?).and_return(true)
+            end
+
+            it { is_expected.not_to redirect_to new_users_sign_up_group_path }
+          end
+        end
+      end
+    end
+
+    describe 'recording the user and tracking events for the onboarding issues experiment' do
+      using RSpec::Parameterized::TableSyntax
+
+      let(:on_gitlab_com) { false }
+      let(:experiment_enabled) { false }
+      let(:experiment_enabled_for_user) { false }
+      let(:in_subscription_flow) { false }
+      let(:in_invitation_flow) { false }
+      let(:in_oauth_flow) { false }
+      let(:in_trial_flow) { false }
+
+      before do
+        sign_in(user)
+        allow(::Gitlab).to receive(:com?).and_return(on_gitlab_com)
+        stub_experiment(onboarding_issues: experiment_enabled)
+        stub_experiment_for_user(onboarding_issues: experiment_enabled_for_user)
+        allow(controller.helpers).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
+        allow(controller.helpers).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
+        allow(controller.helpers).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
+        allow(controller.helpers).to receive(:in_trial_flow?).and_return(in_trial_flow)
+      end
+
+      context 'when on GitLab.com' do
+        let(:on_gitlab_com) { true }
+
+        context 'and the onboarding issues experiment is enabled' do
+          let(:experiment_enabled) { true }
+
+          context 'and we’re not in the subscription, invitation, oauth, or trial flow' do
+            where(:experiment_enabled_for_user, :group_type) do
+              true  | :experimental
+              false | :control
+            end
+
+            with_them do
+              it 'adds the user to the experiments table with the correct group_type' do
+                expect(::Experiment).to receive(:add_user).with(:onboarding_issues, group_type, user)
+
+                subject
+              end
+
+              it 'tracks a signed_up event', :snowplow do
+                subject
+
+                expect_snowplow_event(
+                  category: 'Growth::Conversion::Experiment::OnboardingIssues',
+                  action: 'signed_up',
+                  label: anything,
+                  property: "#{group_type}_group"
+                )
+              end
+            end
+          end
+
+          context 'but we’re in the subscription, invitation, oauth, or trial flow' do
+            where(:in_subscription_flow, :in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
+              true  | false | false | false
+              false | true  | false | false
+              false | false | true  | false
+              false | false | false | true
+            end
+
+            with_them do
+              it 'does not add the user to the experiments table' do
+                expect(::Experiment).not_to receive(:add_user)
+
+                subject
+              end
+
+              it 'does not track a signed_up event', :snowplow do
+                subject
+
+                expect_no_snowplow_event
+              end
+            end
+          end
+        end
+      end
+
+      context 'when not on GitLab.com, regardless of whether or not the experiment is enabled' do
+        where(experiment_enabled: [true, false])
+
+        with_them do
+          it 'does not add the user to the experiments table' do
+            expect(::Experiment).not_to receive(:add_user)
+
+            subject
+          end
+
+          it 'does not track a signed_up event', :snowplow do
+            subject
+
+            expect_no_snowplow_event
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/registrations_controller_spec.rb

@@ -19,206 +19,4 @@ RSpec.describe RegistrationsController do
       end
     end
   end
-
-  describe '#update_registration' do
-    let(:setup_for_company) { 'false' }
-    let(:email_opted_in) { '0' }
-
-    subject(:update_registration) do
-      patch :update_registration, params: {
-        user: {
-          role: 'software_developer',
-          setup_for_company: setup_for_company,
-          email_opted_in: email_opted_in
-        }
-      }
-    end
-
-    context 'without a signed in user' do
-      it { is_expected.to redirect_to new_user_registration_path }
-    end
-
-    context 'with a signed in user' do
-      before do
-        sign_in(user)
-      end
-
-      context 'email updates' do
-        context 'when setup for company is false' do
-          context 'when the user opted in' do
-            let(:email_opted_in) { '1' }
-
-            it 'sets the email_opted_in fields' do
-              subject
-
-              expect(controller.current_user.email_opted_in).to be_truthy
-              expect(controller.current_user.email_opted_in_ip).to be_present
-              expect(controller.current_user.email_opted_in_source).to eq('GitLab.com')
-              expect(controller.current_user.email_opted_in_at).not_to be_nil
-            end
-          end
-
-          context 'when user opted out' do
-            let(:email_opted_in) { '0' }
-
-            it 'does not set the rest of the email_opted_in fields' do
-              subject
-
-              expect(controller.current_user.email_opted_in).to be_falsey
-              expect(controller.current_user.email_opted_in_ip).to be_blank
-              expect(controller.current_user.email_opted_in_source).to be_blank
-              expect(controller.current_user.email_opted_in_at).to be_nil
-            end
-          end
-        end
-
-        context 'when setup for company is true' do
-          let(:setup_for_company) { 'true' }
-
-          it 'sets email_opted_in fields' do
-            subject
-
-            expect(controller.current_user.email_opted_in).to be_truthy
-            expect(controller.current_user.email_opted_in_ip).to be_present
-            expect(controller.current_user.email_opted_in_source).to eq('GitLab.com')
-            expect(controller.current_user.email_opted_in_at).not_to be_nil
-          end
-        end
-      end
-
-      describe 'redirection' do
-        it { is_expected.to redirect_to dashboard_projects_path }
-
-        context 'when part of the onboarding issues experiment' do
-          before do
-            stub_experiment_for_user(onboarding_issues: true)
-          end
-
-          it { is_expected.to redirect_to new_users_sign_up_group_path }
-
-          context 'when in subscription flow' do
-            before do
-              allow(controller.helpers).to receive(:in_subscription_flow?).and_return(true)
-            end
-
-            it { is_expected.not_to redirect_to new_users_sign_up_group_path }
-          end
-
-          context 'when in invitation flow' do
-            before do
-              allow(controller.helpers).to receive(:in_invitation_flow?).and_return(true)
-            end
-
-            it { is_expected.not_to redirect_to new_users_sign_up_group_path }
-          end
-
-          context 'when in trial flow' do
-            before do
-              allow(controller.helpers).to receive(:in_trial_flow?).and_return(true)
-            end
-
-            it { is_expected.not_to redirect_to new_users_sign_up_group_path }
-          end
-        end
-      end
-    end
-
-    describe 'recording the user and tracking events for the onboarding issues experiment' do
-      using RSpec::Parameterized::TableSyntax
-
-      let(:on_gitlab_com) { false }
-      let(:experiment_enabled) { false }
-      let(:experiment_enabled_for_user) { false }
-      let(:in_subscription_flow) { false }
-      let(:in_invitation_flow) { false }
-      let(:in_oauth_flow) { false }
-      let(:in_trial_flow) { false }
-
-      before do
-        sign_in(user)
-        allow(::Gitlab).to receive(:com?).and_return(on_gitlab_com)
-        stub_experiment(onboarding_issues: experiment_enabled)
-        stub_experiment_for_user(onboarding_issues: experiment_enabled_for_user)
-        allow(controller.helpers).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
-        allow(controller.helpers).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
-        allow(controller.helpers).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
-        allow(controller.helpers).to receive(:in_trial_flow?).and_return(in_trial_flow)
-      end
-
-      context 'when on GitLab.com' do
-        let(:on_gitlab_com) { true }
-
-        context 'and the onboarding issues experiment is enabled' do
-          let(:experiment_enabled) { true }
-
-          context 'and we’re not in the subscription, invitation, oauth, or trial flow' do
-            where(:experiment_enabled_for_user, :group_type) do
-              true  | :experimental
-              false | :control
-            end
-
-            with_them do
-              it 'adds the user to the experiments table with the correct group_type' do
-                expect(::Experiment).to receive(:add_user).with(:onboarding_issues, group_type, user)
-
-                update_registration
-              end
-
-              it 'tracks a signed_up event', :snowplow do
-                update_registration
-
-                expect_snowplow_event(
-                  category: 'Growth::Conversion::Experiment::OnboardingIssues',
-                  action: 'signed_up',
-                  label: anything,
-                  property: "#{group_type}_group"
-                )
-              end
-            end
-          end
-
-          context 'but we’re in the subscription, invitation, oauth, or trial flow' do
-            where(:in_subscription_flow, :in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
-              true  | false | false | false
-              false | true  | false | false
-              false | false | true  | false
-              false | false | false | true
-            end
-
-            with_them do
-              it 'does not add the user to the experiments table' do
-                expect(::Experiment).not_to receive(:add_user)
-
-                update_registration
-              end
-
-              it 'does not track a signed_up event', :snowplow do
-                update_registration
-
-                expect_no_snowplow_event
-              end
-            end
-          end
-        end
-      end
-
-      context 'when not on GitLab.com, regardless of whether or not the experiment is enabled' do
-        where(experiment_enabled: [true, false])
-
-        with_them do
-          it 'does not add the user to the experiments table' do
-            expect(::Experiment).not_to receive(:add_user)
-
-            update_registration
-          end
-
-          it 'does not track a signed_up event', :snowplow do
-            update_registration
-
-            expect_no_snowplow_event
-          end
-        end
-      end
-    end
-  end
 end

ee/spec/features/registrations/welcome_spec.rb

@@ -14,9 +14,9 @@ RSpec.describe 'Welcome screen', :js do
     before do
       allow(Gitlab).to receive(:com?).and_return(true)
       gitlab_sign_in(user)
-      allow_any_instance_of(EE::RegistrationsHelper).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
-      allow_any_instance_of(EE::RegistrationsHelper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
-      allow_any_instance_of(EE::RegistrationsHelper).to receive(:in_trial_flow?).and_return(in_trial_flow)
+      allow_any_instance_of(EE::WelcomeHelper).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
+      allow_any_instance_of(EE::WelcomeHelper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
+      allow_any_instance_of(EE::WelcomeHelper).to receive(:in_trial_flow?).and_return(in_trial_flow)
       stub_experiment_for_user(onboarding_issues: part_of_onboarding_issues_experiment)
 
       visit users_sign_up_welcome_path

ee/spec/helpers/ee/registrations_helper_spec.rb

@@ -5,91 +5,6 @@ require 'spec_helper'
 RSpec.describe EE::RegistrationsHelper do
   using RSpec::Parameterized::TableSyntax
 
-  describe '#in_subscription_flow?' do
-    where(:user_return_to_path, :expected_result) do
-      '/-/subscriptions/new?plan_id=bronze_plan' | true
-      '/foo'                                     | false
-      nil                                        | false
-    end
-
-    with_them do
-      it 'returns the expected_result' do
-        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
-
-        expect(helper.in_subscription_flow?).to eq(expected_result)
-      end
-    end
-  end
-
-  describe '#in_trial_flow?' do
-    where(:user_return_to_path, :expected_result) do
-      '/-/trials/new?glm_content=free-trial&glm_source=about.gitlab.com' | true
-      '/foo'                                                             | false
-      nil                                                                | false
-    end
-
-    with_them do
-      it 'returns the expected_result' do
-        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
-
-        expect(helper.in_trial_flow?).to eq(expected_result)
-      end
-    end
-  end
-
-  describe '#in_invitation_flow?' do
-    where(:user_return_to_path, :expected_result) do
-      '/-/invites/xxx' | true
-      '/invites/xxx'   | false
-      '/foo'           | false
-      nil              | nil
-    end
-
-    with_them do
-      it 'returns the expected_result' do
-        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
-
-        expect(helper.in_invitation_flow?).to eq(expected_result)
-      end
-    end
-  end
-
-  describe '#in_oauth_flow?' do
-    where(:user_return_to_path, :expected_result) do
-      '/oauth/authorize?client_id=x&redirect_uri=y&response_type=code&state=z' | true
-      '/foo'                                                                   | false
-      nil                                                                      | nil
-    end
-
-    with_them do
-      it 'returns the expected_result' do
-        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
-
-        expect(helper.in_oauth_flow?).to eq(expected_result)
-      end
-    end
-  end
-
-  describe '#setup_for_company_label_text' do
-    before do
-      allow(helper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
-      allow(helper).to receive(:in_trial_flow?).and_return(in_trial_flow)
-    end
-
-    subject { helper.setup_for_company_label_text }
-
-    where(:in_subscription_flow, :in_trial_flow, :text) do
-      true | true | 'Who will be using this GitLab subscription?'
-      true | false | 'Who will be using this GitLab subscription?'
-      false | true | 'Who will be using this GitLab trial?'
-      false | false | 'Who will be using GitLab?'
-    end
-
-    with_them do
-      it { is_expected.to eq(text) }
-    end
-  end
-
   describe '#visibility_level_options' do
     let(:user) { build(:user) }
 
@@ -106,181 +21,4 @@ RSpec.describe EE::RegistrationsHelper do
       ]
     end
   end
-
-  shared_context 'with the various user flows' do
-    let(:in_subscription_flow) { false }
-    let(:in_invitation_flow) { false }
-    let(:in_oauth_flow) { false }
-    let(:in_trial_flow) { false }
-
-    before do
-      allow(helper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
-      allow(helper).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
-      allow(helper).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
-      allow(helper).to receive(:in_trial_flow?).and_return(in_trial_flow)
-    end
-  end
-
-  shared_context 'with the onboarding issues experiment' do
-    let(:onboarding_issues_experiment_enabled) { false }
-
-    before do
-      allow(helper).to receive(:onboarding_issues_experiment_enabled?).and_return(onboarding_issues_experiment_enabled)
-    end
-  end
-
-  describe '#show_signup_flow_progress_bar?' do
-    include_context 'with the various user flows'
-    include_context 'with the onboarding issues experiment'
-
-    subject { helper.show_signup_flow_progress_bar? }
-
-    context 'when in the subscription flow, regardless of all other flows' do
-      let(:in_subscription_flow) { true }
-
-      where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
-        true  | false | false
-        false | true  | false
-        false | false | true
-      end
-
-      with_them do
-        context 'regardless of if the onboarding issues experiment is enabled' do
-          where(onboarding_issues_experiment_enabled: [true, false])
-
-          with_them do
-            it { is_expected.to be_truthy }
-          end
-        end
-      end
-    end
-
-    context 'when not in the subscription flow' do
-      context 'but in the invitation, oauth, or trial flow' do
-        where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
-          true  | false | false
-          false | true  | false
-          false | false | true
-        end
-
-        with_them do
-          context 'regardless of if the onboarding issues experiment is enabled' do
-            where(onboarding_issues_experiment_enabled: [true, false])
-
-            with_them do
-              it { is_expected.to be_falsey }
-            end
-          end
-        end
-      end
-
-      context 'and not in the invitation, oauth, or trial flow' do
-        where(:onboarding_issues_experiment_enabled, :result) do
-          true  | true
-          false | false
-        end
-
-        with_them do
-          it 'depends on whether or not the onboarding issues experiment is enabled' do
-            is_expected.to eq(result)
-          end
-        end
-      end
-    end
-  end
-
-  describe '#welcome_submit_button_text' do
-    include_context 'with the various user flows'
-    include_context 'with the onboarding issues experiment'
-
-    subject { helper.welcome_submit_button_text }
-
-    context 'when in the subscription or trial flow' do
-      where(:in_subscription_flow, :in_trial_flow) do
-        true  | false
-        false | true
-      end
-
-      with_them do
-        context 'regardless of if the onboarding issues experiment is enabled' do
-          where(onboarding_issues_experiment_enabled: [true, false])
-
-          with_them do
-            it { is_expected.to eq('Continue') }
-          end
-        end
-      end
-    end
-
-    context 'when not in the subscription or trial flow' do
-      context 'but in the invitation or oauth flow' do
-        where(:in_invitation_flow, :in_oauth_flow) do
-          true  | false
-          false | true
-        end
-
-        with_them do
-          context 'regardless of if the onboarding issues experiment is enabled' do
-            where(onboarding_issues_experiment_enabled: [true, false])
-
-            with_them do
-              it { is_expected.to eq('Get started!') }
-            end
-          end
-        end
-      end
-
-      context 'and not in the invitation or oauth flow' do
-        where(:onboarding_issues_experiment_enabled, :result) do
-          true  | 'Continue'
-          false | 'Get started!'
-        end
-
-        with_them do
-          it 'depends on whether or not the onboarding issues experiment is enabled' do
-            is_expected.to eq(result)
-          end
-        end
-      end
-    end
-  end
-
-  describe '#data_attributes_for_progress_bar_js_component' do
-    before do
-      allow(helper).to receive(:in_subscription_flow?).and_return(options_enabled)
-      allow(helper).to receive(:onboarding_issues_experiment_enabled?).and_return(options_enabled)
-    end
-
-    subject { helper.tag(:div, data: helper.data_attributes_for_progress_bar_js_component) }
-
-    where(:options_enabled, :attr_values) do
-      true  | 'true'
-      false | 'false'
-    end
-
-    with_them do
-      it 'always includes both attributes with stringified boolean values' do
-        is_expected.to eq(%{<div data-is-in-subscription-flow="#{attr_values}" data-is-onboarding-issues-experiment-enabled="#{attr_values}" />})
-      end
-    end
-  end
-
-  describe '#skip_setup_for_company?' do
-    let(:user) { create(:user) }
-
-    before do
-      allow(helper).to receive(:current_user).and_return(user)
-    end
-
-    it 'will skip the setup if memberships are found' do
-      member = create(:project_member, :invited)
-      member.accept_invite!(user)
-
-      expect(helper.skip_setup_for_company?).to be true
-    end
-
-    it 'will not skip the setup when a user has no memberships' do
-      expect(helper.skip_setup_for_company?).to be false
-    end
-  end
 end

ee/spec/helpers/ee/welcome_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::WelcomeHelper do
+  using RSpec::Parameterized::TableSyntax
+
+  describe '#in_subscription_flow?' do
+    where(:user_return_to_path, :expected_result) do
+      '/-/subscriptions/new?plan_id=bronze_plan' | true
+      '/foo'                                     | false
+      nil                                        | false
+    end
+
+    with_them do
+      it 'returns the expected_result' do
+        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
+
+        expect(helper.in_subscription_flow?).to eq(expected_result)
+      end
+    end
+  end
+
+  describe '#in_trial_flow?' do
+    where(:user_return_to_path, :expected_result) do
+      '/-/trials/new?glm_content=free-trial&glm_source=about.gitlab.com' | true
+      '/foo'                                                             | false
+      nil                                                                | false
+    end
+
+    with_them do
+      it 'returns the expected_result' do
+        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
+
+        expect(helper.in_trial_flow?).to eq(expected_result)
+      end
+    end
+  end
+
+  describe '#in_invitation_flow?' do
+    where(:user_return_to_path, :expected_result) do
+      '/-/invites/xxx' | true
+      '/invites/xxx'   | false
+      '/foo'           | false
+      nil              | nil
+    end
+
+    with_them do
+      it 'returns the expected_result' do
+        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
+
+        expect(helper.in_invitation_flow?).to eq(expected_result)
+      end
+    end
+  end
+
+  describe '#in_oauth_flow?' do
+    where(:user_return_to_path, :expected_result) do
+      '/oauth/authorize?client_id=x&redirect_uri=y&response_type=code&state=z' | true
+      '/foo'                                                                   | false
+      nil                                                                      | nil
+    end
+
+    with_them do
+      it 'returns the expected_result' do
+        allow(helper).to receive(:session).and_return('user_return_to' => user_return_to_path)
+
+        expect(helper.in_oauth_flow?).to eq(expected_result)
+      end
+    end
+  end
+
+  describe '#setup_for_company_label_text' do
+    before do
+      allow(helper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
+      allow(helper).to receive(:in_trial_flow?).and_return(in_trial_flow)
+    end
+
+    subject { helper.setup_for_company_label_text }
+
+    where(:in_subscription_flow, :in_trial_flow, :text) do
+      true | true | 'Who will be using this GitLab subscription?'
+      true | false | 'Who will be using this GitLab subscription?'
+      false | true | 'Who will be using this GitLab trial?'
+      false | false | 'Who will be using GitLab?'
+    end
+
+    with_them do
+      it { is_expected.to eq(text) }
+    end
+  end
+
+  shared_context 'with the various user flows' do
+    let(:in_subscription_flow) { false }
+    let(:in_invitation_flow) { false }
+    let(:in_oauth_flow) { false }
+    let(:in_trial_flow) { false }
+
+    before do
+      allow(helper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
+      allow(helper).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
+      allow(helper).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
+      allow(helper).to receive(:in_trial_flow?).and_return(in_trial_flow)
+    end
+  end
+
+  shared_context 'with the onboarding issues experiment' do
+    let(:onboarding_issues_experiment_enabled) { false }
+
+    before do
+      allow(helper).to receive(:onboarding_issues_experiment_enabled?).and_return(onboarding_issues_experiment_enabled)
+    end
+  end
+
+  describe '#show_signup_flow_progress_bar?' do
+    include_context 'with the various user flows'
+    include_context 'with the onboarding issues experiment'
+
+    subject { helper.show_signup_flow_progress_bar? }
+
+    context 'when in the subscription flow, regardless of all other flows' do
+      let(:in_subscription_flow) { true }
+
+      where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
+        true  | false | false
+        false | true  | false
+        false | false | true
+      end
+
+      with_them do
+        context 'regardless of if the onboarding issues experiment is enabled' do
+          where(onboarding_issues_experiment_enabled: [true, false])
+
+          with_them do
+            it { is_expected.to be_truthy }
+          end
+        end
+      end
+    end
+
+    context 'when not in the subscription flow' do
+      context 'but in the invitation, oauth, or trial flow' do
+        where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do
+          true  | false | false
+          false | true  | false
+          false | false | true
+        end
+
+        with_them do
+          context 'regardless of if the onboarding issues experiment is enabled' do
+            where(onboarding_issues_experiment_enabled: [true, false])
+
+            with_them do
+              it { is_expected.to be_falsey }
+            end
+          end
+        end
+      end
+
+      context 'and not in the invitation, oauth, or trial flow' do
+        where(:onboarding_issues_experiment_enabled, :result) do
+          true  | true
+          false | false
+        end
+
+        with_them do
+          it 'depends on whether or not the onboarding issues experiment is enabled' do
+            is_expected.to eq(result)
+          end
+        end
+      end
+    end
+  end
+
+  describe '#welcome_submit_button_text' do
+    include_context 'with the various user flows'
+    include_context 'with the onboarding issues experiment'
+
+    subject { helper.welcome_submit_button_text }
+
+    context 'when in the subscription or trial flow' do
+      where(:in_subscription_flow, :in_trial_flow) do
+        true  | false
+        false | true
+      end
+
+      with_them do
+        context 'regardless of if the onboarding issues experiment is enabled' do
+          where(onboarding_issues_experiment_enabled: [true, false])
+
+          with_them do
+            it { is_expected.to eq('Continue') }
+          end
+        end
+      end
+    end
+
+    context 'when not in the subscription or trial flow' do
+      context 'but in the invitation or oauth flow' do
+        where(:in_invitation_flow, :in_oauth_flow) do
+          true  | false
+          false | true
+        end
+
+        with_them do
+          context 'regardless of if the onboarding issues experiment is enabled' do
+            where(onboarding_issues_experiment_enabled: [true, false])
+
+            with_them do
+              it { is_expected.to eq('Get started!') }
+            end
+          end
+        end
+      end
+
+      context 'and not in the invitation or oauth flow' do
+        where(:onboarding_issues_experiment_enabled, :result) do
+          true  | 'Continue'
+          false | 'Get started!'
+        end
+
+        with_them do
+          it 'depends on whether or not the onboarding issues experiment is enabled' do
+            is_expected.to eq(result)
+          end
+        end
+      end
+    end
+  end
+
+  describe '#data_attributes_for_progress_bar_js_component' do
+    before do
+      allow(helper).to receive(:in_subscription_flow?).and_return(options_enabled)
+      allow(helper).to receive(:onboarding_issues_experiment_enabled?).and_return(options_enabled)
+    end
+
+    subject { helper.tag(:div, data: helper.data_attributes_for_progress_bar_js_component) }
+
+    where(:options_enabled, :attr_values) do
+      true  | 'true'
+      false | 'false'
+    end
+
+    with_them do
+      it 'always includes both attributes with stringified boolean values' do
+        is_expected.to eq(%{<div data-is-in-subscription-flow="#{attr_values}" data-is-onboarding-issues-experiment-enabled="#{attr_values}" />})
+      end
+    end
+  end
+
+  describe '#skip_setup_for_company?' do
+    let(:user) { create(:user) }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    it 'will skip the setup if memberships are found' do
+      member = create(:project_member, :invited)
+      member.accept_invite!(user)
+
+      expect(helper.skip_setup_for_company?).to be true
+    end
+
+    it 'will not skip the setup when a user has no memberships' do
+      expect(helper.skip_setup_for_company?).to be false
+    end
+  end
+end

ee/spec/helpers/subscriptions_helper_spec.rb

@@ -50,7 +50,6 @@ RSpec.describe SubscriptionsHelper do
     describe 'new_user' do
       where(:referer, :expected_result) do
         'http://example.com/users/sign_up/welcome?foo=bar'             | 'true'
-        'http://example.com/users/sign_up/update_registration?foo=bar' | 'true'
         'http://example.com'                                           | 'false'
         nil                                                            | 'false'
       end

ee/spec/views/registrations/welcome.html.haml_spec.rb → ee/spec/views/registrations/welcome/show.html.haml_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'registrations/welcome' do
+RSpec.describe 'registrations/welcome/show' do
   using RSpec::Parameterized::TableSyntax
 
   let_it_be(:user) { User.new }

qa/qa/page/main/sign_up.rb

@@ -13,7 +13,7 @@ module QA
           element :new_user_register_button
         end
 
-        view 'app/views/registrations/welcome.html.haml' do
+        view 'app/views/registrations/welcome/show.html.haml' do
           element :get_started_button
         end
 

spec/controllers/registrations/welcome_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Registrations::WelcomeController do
+  let(:user) { create(:user) }
+
+  describe '#welcome' do
+    subject(:show) { get :show }
+
+    context 'without a signed in user' do
+      it { is_expected.to redirect_to new_user_registration_path }
+    end
+
+    context 'when role or setup_for_company is not set' do
+      before do
+        sign_in(user)
+      end
+
+      it { is_expected.to render_template(:show) }
+    end
+
+    context 'when role is required and setup_for_company is not set' do
+      before do
+        user.set_role_required!
+        sign_in(user)
+      end
+
+      it { is_expected.to render_template(:show) }
+    end
+
+    context 'when role and setup_for_company is set' do
+      before do
+        user.update!(setup_for_company: false)
+        sign_in(user)
+      end
+
+      it { is_expected.to redirect_to(dashboard_projects_path)}
+    end
+
+    context 'when role is set and setup_for_company is not set' do
+      before do
+        user.update!(role: :software_developer)
+        sign_in(user)
+      end
+
+      it { is_expected.to render_template(:show) }
+    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
+    end
+  end
+
+  describe '#update' do
+    subject(:update) do
+      patch :update, params: { user: { role: 'software_developer', setup_for_company: 'false' } }
+    end
+
+    context 'without a signed in user' do
+      it { is_expected.to redirect_to new_user_registration_path }
+    end
+
+    context 'with a signed in user' do
+      before do
+        sign_in(user)
+      end
+
+      it { is_expected.to redirect_to(dashboard_projects_path)}
+    end
+  end
+end

spec/controllers/registrations_controller_spec.rb

@@ -402,43 +402,4 @@ RSpec.describe RegistrationsController do
       end
     end
   end
-
-  describe '#welcome' do
-    subject { get :welcome }
-
-    it 'renders the welcome layout' do
-      sign_in(create(:user))
-
-      expect(subject).to render_template(:welcome)
-    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
-    end
-  end
-
-  describe '#update_registration' do
-    subject(:update_registration) do
-      patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } }
-    end
-
-    context 'without a signed in user' do
-      it { is_expected.to redirect_to new_user_registration_path }
-    end
-
-    context 'with a signed in user' do
-      before do
-        sign_in(create(:user))
-      end
-
-      it { is_expected.to redirect_to(dashboard_projects_path)}
-    end
-  end
 end

spec/views/registrations/welcome.html.haml_spec.rb → spec/views/registrations/welcome/show.html.haml_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'registrations/welcome' do
+RSpec.describe 'registrations/welcome/show' do
   using RSpec::Parameterized::TableSyntax
 
   let_it_be(:user) { User.new }