Merge branch '325287-do-not-enroll-invited-users-into-required-onboarding-experience' into 'master'

Skip onboarding experience for invited users

See merge request gitlab-org/gitlab!57117

ee/app/controllers/ee/registrations/welcome_controller.rb

@@ -43,7 +43,7 @@ module EE
       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?

ee/app/helpers/ee/welcome_helper.rb

@@ -24,10 +24,6 @@ module EE
       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 @@ module EE
 
     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 @@ module EE
       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,9 +62,11 @@ module EE
       }
     end
 
-    def skip_setup_for_company?
+    def user_has_memberships?
+      strong_memoize(:user_has_memberships) do
         current_user.members.any?
       end
+    end
 
     def signup_onboarding_enabled?
       ::Gitlab.dev_env_or_com? && ::Feature.enabled?(:signup_onboarding, default_enabled: true)

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

 - 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

ee/changelogs/unreleased/325287-do-not-enroll-invited-users-into-required-onboarding-experience.yml

+---
+title: Skip onboarding experience for invited users
+merge_request: 57117
+author:
+type: other

ee/spec/controllers/registrations/welcome_controller_spec.rb

@@ -179,7 +179,7 @@ RSpec.describe Registrations::WelcomeController do
 
           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 }

ee/spec/features/registrations/welcome_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe 'Welcome screen', :js do
   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 @@ RSpec.describe 'Welcome screen', :js do
     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 @@ RSpec.describe 'Welcome screen', :js do
       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')

ee/spec/helpers/ee/welcome_helper_spec.rb

@@ -37,23 +37,6 @@ RSpec.describe EE::WelcomeHelper do
     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 @@ RSpec.describe EE::WelcomeHelper do
 
   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 @@ RSpec.describe EE::WelcomeHelper do
     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 @@ RSpec.describe EE::WelcomeHelper do
 
     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 @@ RSpec.describe EE::WelcomeHelper do
 
     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 @@ RSpec.describe EE::WelcomeHelper do
     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
 

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

@@ -25,8 +25,6 @@ RSpec.describe 'registrations/welcome/show' do
     '/-/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 @@ RSpec.describe 'registrations/welcome/show' do
       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 @@ RSpec.describe 'registrations/welcome/show' do
       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

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

@@ -11,7 +11,7 @@ RSpec.describe 'registrations/welcome/show' do
     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)