Commit 0bd88376 authored by Doug Stull's avatar Doug Stull Committed by Luke Duncalfe

Skip onboarding experience if user already has memberships

Not needed when user is invited or is already a member.

https://gitlab.com/gitlab-org/gitlab/-/issues/325287
parent 82a7f898
...@@ -43,7 +43,7 @@ module EE ...@@ -43,7 +43,7 @@ module EE
override :show_signup_onboarding? override :show_signup_onboarding?
def show_signup_onboarding? def show_signup_onboarding?
!helpers.in_subscription_flow? && !helpers.in_subscription_flow? &&
!helpers.in_invitation_flow? && !helpers.user_has_memberships? &&
!helpers.in_oauth_flow? && !helpers.in_oauth_flow? &&
!helpers.in_trial_flow? && !helpers.in_trial_flow? &&
helpers.signup_onboarding_enabled? helpers.signup_onboarding_enabled?
......
...@@ -24,10 +24,6 @@ module EE ...@@ -24,10 +24,6 @@ module EE
params[:hide_trial_activation_banner] == 'true' params[:hide_trial_activation_banner] == 'true'
end end
def in_invitation_flow?
redirect_path.present? && redirect_path.starts_with?('/-/invites/')
end
def in_oauth_flow? def in_oauth_flow?
redirect_path&.starts_with?(oauth_authorization_path) redirect_path&.starts_with?(oauth_authorization_path)
end end
...@@ -44,7 +40,7 @@ module EE ...@@ -44,7 +40,7 @@ module EE
def show_signup_flow_progress_bar? def show_signup_flow_progress_bar?
return true if in_subscription_flow? return true if in_subscription_flow?
return false if in_invitation_flow? || in_oauth_flow? || in_trial_flow? return false if user_has_memberships? || in_oauth_flow? || in_trial_flow?
signup_onboarding_enabled? signup_onboarding_enabled?
end end
...@@ -54,7 +50,7 @@ module EE ...@@ -54,7 +50,7 @@ module EE
get_started = _('Get started!') get_started = _('Get started!')
return continue if in_subscription_flow? || in_trial_flow? return continue if in_subscription_flow? || in_trial_flow?
return get_started if in_invitation_flow? || in_oauth_flow? return get_started if user_has_memberships? || in_oauth_flow?
signup_onboarding_enabled? ? continue : get_started signup_onboarding_enabled? ? continue : get_started
end end
...@@ -66,8 +62,10 @@ module EE ...@@ -66,8 +62,10 @@ module EE
} }
end end
def skip_setup_for_company? def user_has_memberships?
current_user.members.any? strong_memoize(:user_has_memberships) do
current_user.members.any?
end
end end
def signup_onboarding_enabled? def signup_onboarding_enabled?
......
- return unless Gitlab.dev_env_or_com? - return unless Gitlab.dev_env_or_com?
- if skip_setup_for_company? - if user_has_memberships?
= f.hidden_field :setup_for_company, value: true = f.hidden_field :setup_for_company, value: true
- else - else
.row .row
......
---
title: Skip onboarding experience for invited users
merge_request: 57117
author:
type: other
...@@ -179,7 +179,7 @@ RSpec.describe Registrations::WelcomeController do ...@@ -179,7 +179,7 @@ RSpec.describe Registrations::WelcomeController do
context 'when in invitation flow' do context 'when in invitation flow' do
before do before do
allow(controller.helpers).to receive(:in_invitation_flow?).and_return(true) allow(controller.helpers).to receive(:user_has_memberships?).and_return(true)
end end
it { is_expected.not_to redirect_to new_users_sign_up_group_path } it { is_expected.not_to redirect_to new_users_sign_up_group_path }
......
...@@ -6,7 +6,7 @@ RSpec.describe 'Welcome screen', :js do ...@@ -6,7 +6,7 @@ RSpec.describe 'Welcome screen', :js do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:signup_onboarding_enabled) { true } let(:signup_onboarding_enabled) { true }
let(:in_invitation_flow) { false } let(:user_has_memberships) { false }
let(:in_subscription_flow) { false } let(:in_subscription_flow) { false }
let(:in_trial_flow) { false } let(:in_trial_flow) { false }
...@@ -14,7 +14,7 @@ RSpec.describe 'Welcome screen', :js do ...@@ -14,7 +14,7 @@ RSpec.describe 'Welcome screen', :js do
before do before do
allow(Gitlab).to receive(:com?).and_return(true) allow(Gitlab).to receive(:com?).and_return(true)
gitlab_sign_in(user) gitlab_sign_in(user)
allow_any_instance_of(EE::WelcomeHelper).to receive(:in_invitation_flow?).and_return(in_invitation_flow) allow_any_instance_of(EE::WelcomeHelper).to receive(:user_has_memberships?).and_return(user_has_memberships)
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_subscription_flow?).and_return(in_subscription_flow)
allow_any_instance_of(EE::WelcomeHelper).to receive(:in_trial_flow?).and_return(in_trial_flow) allow_any_instance_of(EE::WelcomeHelper).to receive(:in_trial_flow?).and_return(in_trial_flow)
stub_feature_flags(signup_onboarding: signup_onboarding_enabled) stub_feature_flags(signup_onboarding: signup_onboarding_enabled)
...@@ -36,8 +36,8 @@ RSpec.describe 'Welcome screen', :js do ...@@ -36,8 +36,8 @@ RSpec.describe 'Welcome screen', :js do
end end
end end
context 'when in the invitation flow' do context 'when user has memberships' do
let(:in_invitation_flow) { true } let(:user_has_memberships) { true }
it 'does not show the progress bar' do it 'does not show the progress bar' do
expect(page).not_to have_content('Your profile') expect(page).not_to have_content('Your profile')
......
...@@ -37,23 +37,6 @@ RSpec.describe EE::WelcomeHelper do ...@@ -37,23 +37,6 @@ RSpec.describe EE::WelcomeHelper do
end 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 | 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_invitation_flow?).to eq(expected_result)
end
end
end
describe '#in_oauth_flow?' do describe '#in_oauth_flow?' do
where(:user_return_to_path, :expected_result) do where(:user_return_to_path, :expected_result) do
'/oauth/authorize?client_id=x&redirect_uri=y&response_type=code&state=z' | true '/oauth/authorize?client_id=x&redirect_uri=y&response_type=code&state=z' | true
...@@ -92,13 +75,13 @@ RSpec.describe EE::WelcomeHelper do ...@@ -92,13 +75,13 @@ RSpec.describe EE::WelcomeHelper do
shared_context 'with the various user flows' do shared_context 'with the various user flows' do
let(:in_subscription_flow) { false } let(:in_subscription_flow) { false }
let(:in_invitation_flow) { false } let(:user_has_memberships) { false }
let(:in_oauth_flow) { false } let(:in_oauth_flow) { false }
let(:in_trial_flow) { false } let(:in_trial_flow) { false }
before do before do
allow(helper).to receive(:in_subscription_flow?).and_return(in_subscription_flow) 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(:user_has_memberships?).and_return(user_has_memberships)
allow(helper).to receive(:in_oauth_flow?).and_return(in_oauth_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) allow(helper).to receive(:in_trial_flow?).and_return(in_trial_flow)
end end
...@@ -121,7 +104,7 @@ RSpec.describe EE::WelcomeHelper do ...@@ -121,7 +104,7 @@ RSpec.describe EE::WelcomeHelper do
context 'when in the subscription flow, regardless of all other flows' do context 'when in the subscription flow, regardless of all other flows' do
let(:in_subscription_flow) { true } let(:in_subscription_flow) { true }
where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do where(:user_has_memberships, :in_oauth_flow, :in_trial_flow) do
true | false | false true | false | false
false | true | false false | true | false
false | false | true false | false | true
...@@ -140,7 +123,7 @@ RSpec.describe EE::WelcomeHelper do ...@@ -140,7 +123,7 @@ RSpec.describe EE::WelcomeHelper do
context 'when not in the subscription flow' do context 'when not in the subscription flow' do
context 'but in the invitation, oauth, or trial flow' do context 'but in the invitation, oauth, or trial flow' do
where(:in_invitation_flow, :in_oauth_flow, :in_trial_flow) do where(:user_has_memberships, :in_oauth_flow, :in_trial_flow) do
true | false | false true | false | false
false | true | false false | true | false
false | false | true false | false | true
...@@ -197,7 +180,7 @@ RSpec.describe EE::WelcomeHelper do ...@@ -197,7 +180,7 @@ RSpec.describe EE::WelcomeHelper do
context 'when not in the subscription or trial flow' do context 'when not in the subscription or trial flow' do
context 'but in the invitation or oauth flow' do context 'but in the invitation or oauth flow' do
where(:in_invitation_flow, :in_oauth_flow) do where(:user_has_memberships, :in_oauth_flow) do
true | false true | false
false | true false | true
end end
...@@ -248,22 +231,20 @@ RSpec.describe EE::WelcomeHelper do ...@@ -248,22 +231,20 @@ RSpec.describe EE::WelcomeHelper do
end end
end end
describe '#skip_setup_for_company?' do describe '#user_has_memberships?' do
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
before do before do
allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:current_user).and_return(user)
end end
it 'will skip the setup if memberships are found' do it 'is true when the current_user has memberships' do
member = create(:project_member, :invited) create(:project_member, user: user)
member.accept_invite!(user)
expect(helper.skip_setup_for_company?).to be true expect(helper).to be_user_has_memberships
end end
it 'is false when the current_user has no memberships' do
it 'will not skip the setup when a user has no memberships' do expect(helper).not_to be_user_has_memberships
expect(helper.skip_setup_for_company?).to be false
end end
end end
......
...@@ -25,8 +25,6 @@ RSpec.describe 'registrations/welcome/show' do ...@@ -25,8 +25,6 @@ RSpec.describe 'registrations/welcome/show' do
'/-/subscriptions/new' | true | true | :subscription | true '/-/subscriptions/new' | true | true | :subscription | true
'/-/trials/new' | false | false | :trial | true '/-/trials/new' | false | false | :trial | true
'/-/trials/new' | true | 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' | false | false | nil | false
'/oauth/authorize/abc123' | true | false | nil | false '/oauth/authorize/abc123' | true | false | nil | false
nil | false | false | nil | false nil | false | false | nil | false
...@@ -46,11 +44,7 @@ RSpec.describe 'registrations/welcome/show' do ...@@ -46,11 +44,7 @@ RSpec.describe 'registrations/welcome/show' do
is_expected.to have_button(expected_text) is_expected.to have_button(expected_text)
end end
if params[:show_progress_bar] it { is_expected_to_have_progress_bar(status: show_progress_bar) }
it { is_expected.to have_selector('#progress-bar') }
else
it { is_expected.not_to have_selector('#progress-bar') }
end
context 'feature flag other_role_details is enabled' do context 'feature flag other_role_details is enabled' do
let_it_be(:user_other_role_details_enabled) { true } let_it_be(:user_other_role_details_enabled) { true }
...@@ -61,4 +55,14 @@ RSpec.describe 'registrations/welcome/show' do ...@@ -61,4 +55,14 @@ RSpec.describe 'registrations/welcome/show' do
end end
end end
end end
def is_expected_to_have_progress_bar(status: true)
allow(view).to receive(:show_signup_flow_progress_bar?).and_return(status)
if status
is_expected.to have_selector('#progress-bar')
else
is_expected.not_to have_selector('#progress-bar')
end
end
end end
...@@ -11,7 +11,7 @@ RSpec.describe 'registrations/welcome/show' do ...@@ -11,7 +11,7 @@ RSpec.describe 'registrations/welcome/show' do
allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:current_user).and_return(user)
allow(view).to receive(:in_subscription_flow?).and_return(false) allow(view).to receive(:in_subscription_flow?).and_return(false)
allow(view).to receive(:in_trial_flow?).and_return(false) allow(view).to receive(:in_trial_flow?).and_return(false)
allow(view).to receive(:in_invitation_flow?).and_return(false) allow(view).to receive(:user_has_memberships?).and_return(false)
allow(view).to receive(:in_oauth_flow?).and_return(false) allow(view).to receive(:in_oauth_flow?).and_return(false)
allow(Gitlab).to receive(:com?).and_return(false) allow(Gitlab).to receive(:com?).and_return(false)
......
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