Commit 42f44036 authored by Alex Buijs's avatar Alex Buijs

Prevent onboarding experiment for users in oauth flow

When users should be redirected to an OAuth request
accept page, do not enroll them in the onboarding
issues flow.
parent 323a4b72
...@@ -64,8 +64,8 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -64,8 +64,8 @@ class RegistrationsController < Devise::RegistrationsController
if result[:status] == :success 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(: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? 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) && !helpers.in_subscription_flow? && !helpers.in_invitation_flow? return redirect_to new_users_sign_up_group_path if experiment_enabled?(:onboarding_issues) && show_onboarding_issues_experiment?
set_flash_message! :notice, :signed_up set_flash_message! :notice, :signed_up
redirect_to path_for_signed_in_user(current_user) redirect_to path_for_signed_in_user(current_user)
...@@ -210,6 +210,10 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -210,6 +210,10 @@ class RegistrationsController < Devise::RegistrationsController
'devise' 'devise'
end end
end end
def show_onboarding_issues_experiment?
!helpers.in_subscription_flow? && !helpers.in_invitation_flow? && !helpers.in_oauth_flow?
end
end end
RegistrationsController.prepend_if_ee('EE::RegistrationsController') RegistrationsController.prepend_if_ee('EE::RegistrationsController')
...@@ -16,6 +16,10 @@ module EE ...@@ -16,6 +16,10 @@ module EE
redirect_path&.starts_with?('/-/invites/') redirect_path&.starts_with?('/-/invites/')
end end
def in_oauth_flow?
redirect_path&.starts_with?(oauth_authorization_path)
end
def setup_for_company_label_text def setup_for_company_label_text
if in_subscription_flow? if in_subscription_flow?
_('Who will be using this GitLab subscription?') _('Who will be using this GitLab subscription?')
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
.row.flex-grow-1.bg-gray-light .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 .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 .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 } } #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) } %h2.center= _('Welcome to GitLab.com<br>@%{name}!').html_safe % { name: html_escape(current_user.first_name) }
%p %p
...@@ -31,4 +31,4 @@ ...@@ -31,4 +31,4 @@
.row .row
.form-group.col-sm-12.mb-0 .form-group.col-sm-12.mb-0
= button_tag class: %w[btn btn-success w-100] do = 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!')
...@@ -53,4 +53,20 @@ RSpec.describe EE::RegistrationsHelper do ...@@ -53,4 +53,20 @@ RSpec.describe EE::RegistrationsHelper do
end end
end 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 end
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'registrations/welcome' do RSpec.describe 'registrations/welcome' do
using RSpec::Parameterized::TableSyntax
let_it_be(:user) { User.new } let_it_be(:user) { User.new }
before do before do
...@@ -10,6 +12,7 @@ RSpec.describe 'registrations/welcome' 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_subscription_flow?).and_return(in_subscription_flow)
allow(view).to receive(:in_trial_flow?).and_return(in_trial_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_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) allow(view).to receive(:experiment_enabled?).with(:onboarding_issues).and_return(onboarding_issues_experiment_enabled)
render render
...@@ -17,54 +20,44 @@ RSpec.describe 'registrations/welcome' do ...@@ -17,54 +20,44 @@ RSpec.describe 'registrations/welcome' do
subject { rendered } subject { rendered }
context 'in subscription flow' do where(:in_subscription_flow, :in_trial_flow, :in_invitation_flow, :in_oauth_flow, :onboarding_issues_experiment_enabled, :flow) do
let(:in_subscription_flow) { true } false | false | false | false | false | :regular
let(:in_trial_flow) { false } true | false | false | false | false | :subscription
let(:in_invitation_flow) { false } false | true | false | false | false | :trial
let(:onboarding_issues_experiment_enabled) { false } false | false | true | false | false | :invitation
false | false | false | true | false | :oauth
it { is_expected.to have_button('Continue') } false | false | false | false | true | :onboarding
it { is_expected.to have_selector('#progress-bar') } false | false | true | false | true | :onboarding_invitation
it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: 'Who will be using this GitLab subscription?') } false | false | false | true | true | :onboarding_oauth
end end
context 'in trial flow' do def button_text
let(:in_subscription_flow) { false } if %i(subscription trial onboarding).include?(flow)
let(:in_trial_flow) { true } 'Continue'
let(:in_invitation_flow) { false } else
let(:onboarding_issues_experiment_enabled) { false } 'Get started!'
end
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?') }
end end
context 'when onboarding issues experiment is enabled' do def label_text
let(:in_subscription_flow) { false } if flow == :subscription
let(:in_trial_flow) { false } 'Who will be using this GitLab subscription?'
let(:in_invitation_flow) { false } elsif flow == :trial
let(:onboarding_issues_experiment_enabled) { true } 'Who will be using this GitLab trial?'
else
it { is_expected.to have_button('Continue') } 'Who will be using GitLab?'
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') }
end end
end end
context 'when neither in subscription nor in trial flow and onboarding issues experiment is disabled' do with_them do
let(:in_subscription_flow) { false } it { is_expected.to have_button(button_text) }
let(:in_trial_flow) { false } it { is_expected.to have_selector('label[for="user_setup_for_company"]', text: label_text) }
let(:in_invitation_flow) { false } it do
let(:onboarding_issues_experiment_enabled) { false } if %i(subscription onboarding).include?(flow)
is_expected.to have_selector('#progress-bar')
it { is_expected.to have_button('Get started!') } else
it { is_expected.not_to have_selector('#progress-bar') } 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?') } end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment