From 0bd883762c16876a19034df717db65a4ef03ef54 Mon Sep 17 00:00:00 2001
From: Doug Stull <dstull@gitlab.com>
Date: Mon, 29 Mar 2021 01:51:20 +0000
Subject: [PATCH] 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
---
.../ee/registrations/welcome_controller.rb | 2 +-
ee/app/helpers/ee/welcome_helper.rb | 14 +++---
.../welcome/_setup_for_company.html.haml | 2 +-
...rs-into-required-onboarding-experience.yml | 5 +++
.../registrations/welcome_controller_spec.rb | 2 +-
.../features/registrations/welcome_spec.rb | 8 ++--
ee/spec/helpers/ee/welcome_helper_spec.rb | 43 ++++++-------------
.../welcome/show.html.haml_spec.rb | 18 +++++---
.../welcome/show.html.haml_spec.rb | 2 +-
9 files changed, 42 insertions(+), 54 deletions(-)
create mode 100644 ee/changelogs/unreleased/325287-do-not-enroll-invited-users-into-required-onboarding-experience.yml
diff --git a/ee/app/controllers/ee/registrations/welcome_controller.rb b/ee/app/controllers/ee/registrations/welcome_controller.rb
index 4cf23e5174581..886f1c9a8e7e6 100644
--- a/ee/app/controllers/ee/registrations/welcome_controller.rb
+++ b/ee/app/controllers/ee/registrations/welcome_controller.rb
@@ -43,7 +43,7 @@ def update_params
override :show_signup_onboarding?
def show_signup_onboarding?
!helpers.in_subscription_flow? &&
- !helpers.in_invitation_flow? &&
+ !helpers.user_has_memberships? &&
!helpers.in_oauth_flow? &&
!helpers.in_trial_flow? &&
helpers.signup_onboarding_enabled?
diff --git a/ee/app/helpers/ee/welcome_helper.rb b/ee/app/helpers/ee/welcome_helper.rb
index e17116466208e..3d149b38ece8d 100644
--- a/ee/app/helpers/ee/welcome_helper.rb
+++ b/ee/app/helpers/ee/welcome_helper.rb
@@ -24,10 +24,6 @@ def already_showed_trial_activation?
params[:hide_trial_activation_banner] == 'true'
end
- def in_invitation_flow?
- redirect_path.present? && redirect_path.starts_with?('/-/invites/')
- end
-
def in_oauth_flow?
redirect_path&.starts_with?(oauth_authorization_path)
end
@@ -44,7 +40,7 @@ def setup_for_company_label_text
def show_signup_flow_progress_bar?
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?
end
@@ -54,7 +50,7 @@ def welcome_submit_button_text
get_started = _('Get started!')
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
end
@@ -66,8 +62,10 @@ def data_attributes_for_progress_bar_js_component
}
end
- def skip_setup_for_company?
- current_user.members.any?
+ def user_has_memberships?
+ strong_memoize(:user_has_memberships) do
+ current_user.members.any?
+ end
end
def signup_onboarding_enabled?
diff --git a/ee/app/views/registrations/welcome/_setup_for_company.html.haml b/ee/app/views/registrations/welcome/_setup_for_company.html.haml
index 2ea2a5f082e0b..0ccbd740a1124 100644
--- a/ee/app/views/registrations/welcome/_setup_for_company.html.haml
+++ b/ee/app/views/registrations/welcome/_setup_for_company.html.haml
@@ -1,6 +1,6 @@
- return unless Gitlab.dev_env_or_com?
-- if skip_setup_for_company?
+- if user_has_memberships?
= f.hidden_field :setup_for_company, value: true
- else
.row
diff --git a/ee/changelogs/unreleased/325287-do-not-enroll-invited-users-into-required-onboarding-experience.yml b/ee/changelogs/unreleased/325287-do-not-enroll-invited-users-into-required-onboarding-experience.yml
new file mode 100644
index 0000000000000..f70f0d8ee492d
--- /dev/null
+++ b/ee/changelogs/unreleased/325287-do-not-enroll-invited-users-into-required-onboarding-experience.yml
@@ -0,0 +1,5 @@
+---
+title: Skip onboarding experience for invited users
+merge_request: 57117
+author:
+type: other
diff --git a/ee/spec/controllers/registrations/welcome_controller_spec.rb b/ee/spec/controllers/registrations/welcome_controller_spec.rb
index 32563aad87e98..e6218861440dd 100644
--- a/ee/spec/controllers/registrations/welcome_controller_spec.rb
+++ b/ee/spec/controllers/registrations/welcome_controller_spec.rb
@@ -179,7 +179,7 @@
context 'when in invitation flow' 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
it { is_expected.not_to redirect_to new_users_sign_up_group_path }
diff --git a/ee/spec/features/registrations/welcome_spec.rb b/ee/spec/features/registrations/welcome_spec.rb
index 8cf97b17fba8f..3d17089764241 100644
--- a/ee/spec/features/registrations/welcome_spec.rb
+++ b/ee/spec/features/registrations/welcome_spec.rb
@@ -6,7 +6,7 @@
let_it_be(:user) { create(:user) }
let(:signup_onboarding_enabled) { true }
- let(:in_invitation_flow) { false }
+ let(:user_has_memberships) { false }
let(:in_subscription_flow) { false }
let(:in_trial_flow) { false }
@@ -14,7 +14,7 @@
before do
allow(Gitlab).to receive(:com?).and_return(true)
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_trial_flow?).and_return(in_trial_flow)
stub_feature_flags(signup_onboarding: signup_onboarding_enabled)
@@ -36,8 +36,8 @@
end
end
- context 'when in the invitation flow' do
- let(:in_invitation_flow) { true }
+ context 'when user has memberships' do
+ let(:user_has_memberships) { true }
it 'does not show the progress bar' do
expect(page).not_to have_content('Your profile')
diff --git a/ee/spec/helpers/ee/welcome_helper_spec.rb b/ee/spec/helpers/ee/welcome_helper_spec.rb
index 7ef1679c5a5a0..4b8d0db85acc2 100644
--- a/ee/spec/helpers/ee/welcome_helper_spec.rb
+++ b/ee/spec/helpers/ee/welcome_helper_spec.rb
@@ -37,23 +37,6 @@
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
where(:user_return_to_path, :expected_result) do
'/oauth/authorize?client_id=x&redirect_uri=y&response_type=code&state=z' | true
@@ -92,13 +75,13 @@
shared_context 'with the various user flows' do
let(:in_subscription_flow) { false }
- let(:in_invitation_flow) { false }
+ let(:user_has_memberships) { 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(: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_trial_flow?).and_return(in_trial_flow)
end
@@ -121,7 +104,7 @@
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
+ where(:user_has_memberships, :in_oauth_flow, :in_trial_flow) do
true | false | false
false | true | false
false | false | true
@@ -140,7 +123,7 @@
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
+ where(:user_has_memberships, :in_oauth_flow, :in_trial_flow) do
true | false | false
false | true | false
false | false | true
@@ -197,7 +180,7 @@
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
+ where(:user_has_memberships, :in_oauth_flow) do
true | false
false | true
end
@@ -248,22 +231,20 @@
end
end
- describe '#skip_setup_for_company?' do
- let(:user) { create(:user) }
+ describe '#user_has_memberships?' do
+ let_it_be(: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)
+ it 'is true when the current_user has memberships' do
+ create(:project_member, user: user)
- expect(helper.skip_setup_for_company?).to be true
+ expect(helper).to be_user_has_memberships
end
-
- it 'will not skip the setup when a user has no memberships' do
- expect(helper.skip_setup_for_company?).to be false
+ it 'is false when the current_user has no memberships' do
+ expect(helper).not_to be_user_has_memberships
end
end
diff --git a/ee/spec/views/registrations/welcome/show.html.haml_spec.rb b/ee/spec/views/registrations/welcome/show.html.haml_spec.rb
index d055fce846848..aa58d692e8537 100644
--- a/ee/spec/views/registrations/welcome/show.html.haml_spec.rb
+++ b/ee/spec/views/registrations/welcome/show.html.haml_spec.rb
@@ -25,8 +25,6 @@
'/-/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
@@ -46,11 +44,7 @@
is_expected.to have_button(expected_text)
end
- if params[:show_progress_bar]
- it { is_expected.to have_selector('#progress-bar') }
- else
- it { is_expected.not_to have_selector('#progress-bar') }
- end
+ it { is_expected_to_have_progress_bar(status: show_progress_bar) }
context 'feature flag other_role_details is enabled' do
let_it_be(:user_other_role_details_enabled) { true }
@@ -61,4 +55,14 @@
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
diff --git a/spec/views/registrations/welcome/show.html.haml_spec.rb b/spec/views/registrations/welcome/show.html.haml_spec.rb
index f731594e9ee24..d9774582545a6 100644
--- a/spec/views/registrations/welcome/show.html.haml_spec.rb
+++ b/spec/views/registrations/welcome/show.html.haml_spec.rb
@@ -11,7 +11,7 @@
allow(view).to receive(:current_user).and_return(user)
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_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(Gitlab).to receive(:com?).and_return(false)
--
GitLab