Merge branch 'oauth-flow-onboarding-issues' into 'master'

Prevent onboarding experiment for users in oauth flow

See merge request gitlab-org/gitlab!36584

app/controllers/registrations_controller.rb

@@ -64,8 +64,8 @@ class RegistrationsController < Devise::RegistrationsController
     if result[:status] == :success
       track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group
 
-      track_experiment_event(:onboarding_issues, 'signed_up') if ::Gitlab.com? && !helpers.in_subscription_flow? && !helpers.in_invitation_flow?
-      return redirect_to new_users_sign_up_group_path if experiment_enabled?(:onboarding_issues) && !helpers.in_subscription_flow? && !helpers.in_invitation_flow?
+      track_experiment_event(:onboarding_issues, 'signed_up') if ::Gitlab.com? && show_onboarding_issues_experiment?
+      return redirect_to new_users_sign_up_group_path if experiment_enabled?(:onboarding_issues) && show_onboarding_issues_experiment?
 
       set_flash_message! :notice, :signed_up
       redirect_to path_for_signed_in_user(current_user)
@@ -210,6 +210,10 @@ class RegistrationsController < Devise::RegistrationsController
       'devise'
     end
   end
+
+  def show_onboarding_issues_experiment?
+    !helpers.in_subscription_flow? && !helpers.in_invitation_flow? && !helpers.in_oauth_flow?
+  end
 end
 
 RegistrationsController.prepend_if_ee('EE::RegistrationsController')

ee/app/helpers/ee/registrations_helper.rb

@@ -16,6 +16,10 @@ module EE
       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?')

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

@@ -4,7 +4,7 @@
 .row.flex-grow-1.bg-gray-light
   .d-flex.flex-column.align-items-center.w-100.gl-p-3-deprecated-no-really-do-not-use-me
     .edit-profile.login-page.d-flex.flex-column.align-items-center.pt-lg-3
-      - if in_subscription_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow?)
+      - if in_subscription_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow?)
         #progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled.to_s } }
       %h2.center= _('Welcome to GitLab.com<br>@%{name}!').html_safe % { name: html_escape(current_user.first_name) }
       %p
@@ -31,4 +31,4 @@
         .row
           .form-group.col-sm-12.mb-0
             = button_tag class: %w[btn btn-success w-100] do
-              = in_subscription_flow? || in_trial_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow?) ? _('Continue') : _('Get started!')
+              = in_subscription_flow? || in_trial_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow?) ? _('Continue') : _('Get started!')

ee/spec/helpers/ee/registrations_helper_spec.rb

@@ -53,4 +53,20 @@ RSpec.describe EE::RegistrationsHelper do
       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
 end

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

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'registrations/welcome' do
+  using RSpec::Parameterized::TableSyntax
+
   let_it_be(:user) { User.new }
 
   before do
@@ -10,6 +12,7 @@ RSpec.describe 'registrations/welcome' do
     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(:experiment_enabled?).with(:onboarding_issues).and_return(onboarding_issues_experiment_enabled)
 
     render
@@ -17,54 +20,44 @@ RSpec.describe 'registrations/welcome' do
 
   subject { rendered }
 
-  context 'in subscription flow' do
-    let(:in_subscription_flow) { true }
-    let(:in_trial_flow) { false }
-    let(:in_invitation_flow) { false }
-    let(:onboarding_issues_experiment_enabled) { false }
-
-    it { is_expected.to have_button('Continue') }
-    it { is_expected.to have_selector('#progress-bar') }
-    it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: 'Who will be using this GitLab subscription?') }
+  where(:in_subscription_flow, :in_trial_flow, :in_invitation_flow, :in_oauth_flow, :onboarding_issues_experiment_enabled, :flow) do
+    false | false | false | false | false | :regular
+    true  | false | false | false | false | :subscription
+    false | true  | false | false | false | :trial
+    false | false | true  | false | false | :invitation
+    false | false | false | true  | false | :oauth
+    false | false | false | false | true  | :onboarding
+    false | false | true  | false | true  | :onboarding_invitation
+    false | false | false | true  | true  | :onboarding_oauth
   end
 
-  context 'in trial flow' do
-    let(:in_subscription_flow) { false }
-    let(:in_trial_flow) { true }
-    let(:in_invitation_flow) { false }
-    let(:onboarding_issues_experiment_enabled) { false }
-
-    it { is_expected.to have_button('Continue') }
-    it { is_expected.not_to have_selector('#progress-bar') }
-    it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: 'Who will be using this GitLab trial?') }
+  def button_text
+    if %i(subscription trial onboarding).include?(flow)
+      'Continue'
+    else
+      'Get started!'
+    end
   end
 
-  context 'when onboarding issues experiment is enabled' do
-    let(:in_subscription_flow) { false }
-    let(:in_trial_flow) { false }
-    let(:in_invitation_flow) { false }
-    let(:onboarding_issues_experiment_enabled) { true }
-
-    it { is_expected.to have_button('Continue') }
-    it { is_expected.to have_selector('#progress-bar') }
-    it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: 'Who will be using GitLab?') }
-
-    context 'when in invitation flow' do
-      let(:in_invitation_flow) { true }
-
-      it { is_expected.to have_button('Get started!') }
-      it { is_expected.not_to have_selector('#progress-bar') }
+  def label_text
+    if flow == :subscription
+      'Who will be using this GitLab subscription?'
+    elsif flow == :trial
+      'Who will be using this GitLab trial?'
+    else
+      'Who will be using GitLab?'
     end
   end
 
-  context 'when neither in subscription nor in trial flow and onboarding issues experiment is disabled' do
-    let(:in_subscription_flow) { false }
-    let(:in_trial_flow) { false }
-    let(:in_invitation_flow) { false }
-    let(:onboarding_issues_experiment_enabled) { false }
-
-    it { is_expected.to have_button('Get started!') }
-    it { is_expected.not_to have_selector('#progress-bar') }
-    it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: 'Who will be using GitLab?') }
+  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 %i(subscription onboarding).include?(flow)
+        is_expected.to have_selector('#progress-bar')
+      else
+        is_expected.not_to have_selector('#progress-bar')
+      end
+    end
   end
 end