Merge branch 'move-complex-logic-from-welcome-view-into-helper-methods' into 'master'

Extract complex view logic into helper methods

See merge request gitlab-org/gitlab!40657

app/views/registrations/welcome.html.haml

@@ -23,4 +23,3 @@
               = render "registrations/welcome/button"
             - else
               = f.submit _('Get started!'), class: 'btn-register btn btn-block gl-mb-0 gl-p-3'
-

ee/app/helpers/ee/registrations_helper.rb

@@ -40,6 +40,27 @@ 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 onboarding_issues_experiment_enabled?
+      experiment_enabled?(:onboarding_issues)
+    end
+
     private
 
     def redirect_path

ee/app/views/registrations/welcome/_button.html.haml

-- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues)
-
-= button_tag class: %w[btn btn-success w-100], data: { qa_selector: 'get_started_button' } do
-  = in_subscription_flow? || in_trial_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow?) ? _('Continue') : _('Get started!')
+= button_tag welcome_submit_button_text, class: %w[btn btn-success w-100], data: { qa_selector: 'get_started_button' }

ee/app/views/registrations/welcome/_progress_bar.html.haml

-- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues)
-
-- if in_subscription_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow? && !in_trial_flow?)
-  #progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled.to_s } }
-
+- if show_signup_flow_progress_bar?
+  #progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled?.to_s } }

ee/spec/helpers/ee/registrations_helper_spec.rb

@@ -106,4 +106,142 @@ 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
 end

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

@@ -9,10 +9,7 @@ RSpec.describe 'registrations/welcome' do
 
   before do
     allow(view).to receive(:current_user).and_return(user)
-    allow(view).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
-    allow(view).to receive(:in_trial_flow?).and_return(in_trial_flow)
-    allow(view).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
-    allow(view).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
+    allow(view).to receive(:redirect_path).and_return(redirect_path)
     allow(view).to receive(:experiment_enabled?).with(:onboarding_issues).and_return(onboarding_issues_experiment_enabled)
     allow(Gitlab).to receive(:com?).and_return(true)
 
@@ -21,42 +18,36 @@ RSpec.describe 'registrations/welcome' do
 
   subject { rendered }
 
-  where(:in_subscription_flow, :in_trial_flow, :in_invitation_flow, :in_oauth_flow, :onboarding_issues_experiment_enabled, :shows_progress_bar, :label_key, :is_continue_btn) do
-    false | false | false | false | false | false | nil           | false # regular
-    true  | false | false | false | false | true  | :subscription | true  # subscription
-    false | true  | false | false | false | false | :trial        | true  # trial
-    false | false | true  | false | false | false | nil           | false # invitation
-    false | false | false | true  | false | false | nil           | false # oauth
-    false | false | false | false | true  | true  | nil           | true  # onboarding
-    true  | false | false | false | true  | true  | :subscription | true  # onboarding + subscription
-    false | true  | false | false | true  | false | :trial        | true  # onboarding + trial
-    false | false | true  | false | true  | false | nil           | false # onboarding + invitation
-    false | false | false | true  | true  | false | nil           | false # onboarding + oauth
+  where(:redirect_path, :onboarding_issues_experiment_enabled, :show_progress_bar, :flow, :is_continue) do
+    '/-/subscriptions/new'    | false | true  | :subscription | true
+    '/-/subscriptions/new'    | true  | true  | :subscription | true
+    '/-/trials/new'           | false | false | :trial        | true
+    '/-/trials/new'           | true  | false | :trial        | true
+    '/-/invites/abc123'       | false | false | nil           | false
+    '/-/invites/abc123'       | true  | false | nil           | false
+    '/oauth/authorize/abc123' | false | false | nil           | false
+    '/oauth/authorize/abc123' | true  | false | nil           | false
+    nil                       | false | false | nil           | false
+    nil                       | true  | true  | nil           | true
   end
 
-  def button_text
-    is_continue_btn ? 'Continue' : 'Get started!'
-  end
+  with_them do
+    it 'shows the correct text for the :setup_for_company label' do
+      expected_text = "Who will be using #{flow.nil? ? 'GitLab' : "this GitLab #{flow}"}?"
 
-  def label_text
-    if label_key == :subscription
-      'Who will be using this GitLab subscription?'
-    elsif label_key == :trial
-      'Who will be using this GitLab trial?'
-    else
-      'Who will be using GitLab?'
+      is_expected.to have_selector('label[for="user_setup_for_company"]', text: expected_text)
     end
+
+    it 'shows the correct text for the submit button' do
+      expected_text = is_continue ? 'Continue' : 'Get started!'
+
+      is_expected.to have_button(expected_text)
     end
 
-  with_them do
-    it { is_expected.to have_button(button_text) }
-    it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: label_text) }
-    it do
-      if shows_progress_bar
-        is_expected.to have_selector('#progress-bar')
+    if params[:show_progress_bar]
+      it { is_expected.to have_selector('#progress-bar') }
     else
-        is_expected.not_to have_selector('#progress-bar')
-      end
+      it { is_expected.not_to have_selector('#progress-bar') }
     end
   end
 end