Merge branch '219544-eng-alternate-teammates-invitation-in-first-time-onboarding-2' into 'master'

Refactor groups registration for easier modification

See merge request gitlab-org/gitlab!54713

ee/app/controllers/registrations/groups_controller.rb

@@ -19,14 +19,43 @@ module Registrations
     def create
       @group = Groups::CreateService.new(current_user, group_params).execute
 
-      render_new && return unless @group.persisted?
+      if @group.persisted?
+        create_successful_flow
+      else
+        render action: :new
+      end
+    end
+
+    protected
+
+    def show_confirm_warning?
+      false
+    end
 
-      trial = params[:trial] == 'true'
-      url_params = { namespace_id: @group.id, trial: trial }
+    private
 
+    def check_signup_onboarding_enabled
+      access_denied! unless helpers.signup_onboarding_enabled?
+    end
+
+    def create_successful_flow
       if helpers.in_trial_onboarding_flow?
-        render_new && return unless apply_trial
+        apply_trial_for_trial_onboarding_flow
+      else
+        registration_onboarding_flow
+      end
+    end
 
+    def authorize_create_group!
+      access_denied! unless can?(current_user, :create_group)
+    end
+
+    def group_params
+      params.require(:group).permit(:name, :path, :visibility_level)
+    end
+
+    def apply_trial_for_trial_onboarding_flow
+      if apply_trial
         record_experiment_user(:remove_known_trial_form_fields, namespace_id: @group.id)
         record_experiment_user(:trimmed_skip_trial_copy, namespace_id: @group.id)
         record_experiment_user(:trial_registration_with_social_signin, namespace_id: @group.id)
@@ -36,46 +65,44 @@ module Registrations
         record_experiment_conversion_event(:trial_registration_with_social_signin)
         record_experiment_conversion_event(:trial_onboarding_issues)
 
-        url_params[:trial_onboarding_flow] = true
+        redirect_to new_users_sign_up_project_path(namespace_id: @group.id, trial: helpers.in_trial_during_signup_flow?, trial_onboarding_flow: true)
       else
-        record_experiment_user(:trial_during_signup, trial_chosen: trial, namespace_id: @group.id)
-
-        if experiment_enabled?(:trial_during_signup)
-          if trial
-            render_new && return unless create_lead && apply_trial
-
-            record_experiment_conversion_event(:trial_during_signup)
-          end
-        else
-          invite_members(@group)
-        end
+        render action: :new
       end
-
-      redirect_to new_users_sign_up_project_path(url_params)
     end
 
-    protected
+    def registration_onboarding_flow
+      record_experiment_user(:trial_during_signup, trial_chosen: helpers.in_trial_during_signup_flow?, namespace_id: @group.id)
 
-    def show_confirm_warning?
-      false
+      if experiment_enabled?(:trial_during_signup)
+        trial_during_signup_flow
+      else
+        invite_on_create
+      end
     end
 
-    private
+    def invite_on_create
+      invite_members(@group)
 
-    def check_signup_onboarding_enabled
-      access_denied! unless helpers.signup_onboarding_enabled?
+      redirect_to new_users_sign_up_project_path(namespace_id: @group.id, trial: helpers.in_trial_during_signup_flow?)
     end
 
-    def authorize_create_group!
-      access_denied! unless can?(current_user, :create_group)
+    def trial_during_signup_flow
+      if helpers.in_trial_during_signup_flow?
+        create_lead_and_apply_trial_flow
+      else
+        redirect_to new_users_sign_up_project_path(namespace_id: @group.id, trial: helpers.in_trial_during_signup_flow?)
+      end
     end
 
-    def group_params
-      params.require(:group).permit(:name, :path, :visibility_level)
-    end
+    def create_lead_and_apply_trial_flow
+      if create_lead && apply_trial
+        record_experiment_conversion_event(:trial_during_signup)
 
-    def render_new
-      render action: :new
+        redirect_to new_users_sign_up_project_path(namespace_id: @group.id, trial: helpers.in_trial_during_signup_flow?)
+      else
+        render action: :new
+      end
     end
 
     def create_lead

ee/app/controllers/registrations/projects_controller.rb

@@ -5,7 +5,10 @@ module Registrations
     layout 'checkout'
 
     before_action :check_signup_onboarding_enabled
-    before_action :find_namespace, only: :new
+    before_action only: [:new] do
+      set_namespace
+      authorize_create_project!
+    end
 
     feature_category :navigation
 
@@ -28,9 +31,10 @@ module Registrations
 
           record_experiment_user(:trial_onboarding_issues, trial_onboarding_context)
           record_experiment_conversion_event(:trial_onboarding_issues)
+
           redirect_to trial_getting_started_users_sign_up_welcome_path(learn_gitlab_project_id: learn_gitlab_project.id)
         else
-          redirect_to users_sign_up_experience_level_path(namespace_path: @project.namespace, trial_onboarding_flow: params[:trial_onboarding_flow])
+          redirect_to users_sign_up_experience_level_path(namespace_path: @project.namespace)
         end
       else
         render :new
@@ -64,12 +68,14 @@ module Registrations
       learn_gitlab_project
     end
 
-    def find_namespace
-      @namespace = Namespace.find_by_id(params[:namespace_id])
-
+    def authorize_create_project!
       access_denied! unless can?(current_user, :create_projects, @namespace)
     end
 
+    def set_namespace
+      @namespace = Namespace.find_by_id(params[:namespace_id])
+    end
+
     def project_params
       params.require(:project).permit(project_params_attributes)
     end

ee/spec/controllers/registrations/groups_controller_spec.rb

@@ -98,186 +98,229 @@ RSpec.describe Registrations::GroupsController do
         allow(controller.helpers).to receive(:signup_onboarding_enabled?).and_return(signup_onboarding_enabled)
       end
 
-      it 'creates a group' do
-        expect { subject }.to change { Group.count }.by(1)
-      end
-
-      it { is_expected.to have_gitlab_http_status(:redirect) }
-      it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: user.groups.last.id, trial: false)) }
+      it_behaves_like 'hides email confirmation warning'
 
-      it 'calls the record user trial_during_signup experiment' do
-        group = create(:group)
-        expect_next_instance_of(Groups::CreateService) do |service|
-          expect(service).to receive(:execute).and_return(group)
-        end
-        expect(controller).to receive(:record_experiment_user).with(:trial_during_signup, trial_chosen: false, namespace_id: group.id)
+      context 'when signup onboarding is not enabled' do
+        let(:signup_onboarding_enabled) { false }
 
-        subject
+        it { is_expected.to have_gitlab_http_status(:not_found) }
       end
 
-      context 'in experiment group for trial_during_signup' do
-        let_it_be(:group) { create(:group) }
-        let_it_be(:glm_params) { { glm_source: 'gitlab.com', glm_content: 'content' } }
-        let_it_be(:trial_form_params) do
-          {
-            trial: 'true',
-            company_name: 'ACME',
-            company_size: '1-99',
-            phone_number: '11111111',
-            number_of_users: '17',
-            country: 'Norway'
-          }
-        end
-
-        let_it_be(:trial_user_params) do
-          {
-            work_email: user.email,
-            first_name: user.first_name,
-            last_name: user.last_name,
-            uid: user.id,
-            skip_email_confirmation:  true,
-            gitlab_com_trial: true,
-            provider: 'gitlab',
-            newsletter_segment: user.email_opted_in
-          }
+      context 'when group can be created' do
+        it 'creates a group' do
+          expect { subject }.to change { Group.count }.by(1)
         end
 
-        let_it_be(:trial_params) do
-          {
-            trial_user: ActionController::Parameters.new(trial_form_params.except(:trial).merge(trial_user_params)).permit!
-          }
-        end
+        context 'when the trial onboarding is active - apply_trial_for_trial_onboarding_flow' do
+          let_it_be(:group) { create(:group) }
+          let_it_be(:trial_onboarding_flow_params) { { trial_onboarding_flow: true, glm_source: 'about.gitlab.com', glm_content: 'content' } }
+          let_it_be(:trial_onboarding_issues_enabled) { true }
+          let_it_be(:apply_trial_params) do
+            {
+              uid: user.id,
+              trial_user: ActionController::Parameters.new(
+                {
+                  glm_source: 'about.gitlab.com',
+                  glm_content: 'content',
+                  namespace_id: group.id,
+                  gitlab_com_trial: true,
+                  sync_to_gl: true
+                }
+              ).permit!
+            }
+          end
 
-        let_it_be(:apply_trial_params) do
-          {
-            uid: user.id,
-            trial_user: ActionController::Parameters.new(
-              {
-                glm_source: 'gitlab.com',
-                glm_content: 'content',
-                namespace_id: group.id,
-                gitlab_com_trial: true,
-                sync_to_gl: true
-              }
-            ).permit!
-          }
-        end
+          before do
+            expect_next_instance_of(::Groups::CreateService) do |service|
+              expect(service).to receive(:execute).and_return(group)
+            end
+          end
 
-        before do
-          allow(controller).to receive(:experiment_enabled?).with(:trial_during_signup).and_return(true)
-        end
+          context 'when trial can be applied' do
+            before do
+              expect_next_instance_of(GitlabSubscriptions::ApplyTrialService) do |service|
+                expect(service).to receive(:execute).with(apply_trial_params).and_return({ success: true })
+              end
+              expect(controller).to receive(:record_experiment_user).with(:remove_known_trial_form_fields, namespace_id: group.id)
+              expect(controller).to receive(:record_experiment_user).with(:trimmed_skip_trial_copy, namespace_id: group.id)
+              expect(controller).to receive(:record_experiment_user).with(:trial_registration_with_social_signin, namespace_id: group.id)
+              expect(controller).to receive(:record_experiment_user).with(:trial_onboarding_issues, namespace_id: group.id)
+              expect(controller).to receive(:record_experiment_conversion_event).with(:remove_known_trial_form_fields)
+              expect(controller).to receive(:record_experiment_conversion_event).with(:trimmed_skip_trial_copy)
+              expect(controller).to receive(:record_experiment_conversion_event).with(:trial_registration_with_social_signin)
+              expect(controller).to receive(:record_experiment_conversion_event).with(:trial_onboarding_issues)
+            end
 
-        it 'calls the lead creation and trial apply services' do
-          expect_next_instance_of(Groups::CreateService) do |service|
-            expect(service).to receive(:execute).and_return(group)
-          end
-          expect_next_instance_of(GitlabSubscriptions::CreateLeadService) do |service|
-            expect(service).to receive(:execute).with(trial_params).and_return(success: true)
-          end
-          expect_next_instance_of(GitlabSubscriptions::ApplyTrialService) do |service|
-            expect(service).to receive(:execute).with(apply_trial_params).and_return({ success: true })
+            it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: group.id, trial: false, trial_onboarding_flow: true)) }
           end
 
-          subject
+          context 'when failing to apply trial' do
+            before do
+              expect_next_instance_of(GitlabSubscriptions::ApplyTrialService) do |service|
+                expect(service).to receive(:execute).with(apply_trial_params).and_return({ success: false })
+              end
+            end
+
+            it { is_expected.to render_template(:new) }
+          end
         end
 
-        context 'when user chooses no trial' do
-          let_it_be(:trial_form_params) { { trial: 'false' } }
+        context 'when not in the trial onboarding - registration_onboarding_flow' do
+          let_it_be(:group) { create(:group) }
 
           it 'calls the record user trial_during_signup experiment' do
-            group = create(:group)
             expect_next_instance_of(Groups::CreateService) do |service|
               expect(service).to receive(:execute).and_return(group)
             end
-
             expect(controller).to receive(:record_experiment_user).with(:trial_during_signup, trial_chosen: false, namespace_id: group.id)
 
             subject
           end
 
-          it 'does not call trial_during_signup experiment methods' do
-            expect(controller).not_to receive(:create_lead)
-            expect(controller).not_to receive(:apply_trial)
-
-            subject
-          end
-        end
-      end
+          context 'when in experiment group for trial_during_signup - trial_during_signup_flow' do
+            let_it_be(:glm_params) { { glm_source: 'gitlab.com', glm_content: 'content' } }
+            let_it_be(:trial_form_params) do
+              {
+                trial: 'true',
+                company_name: 'ACME',
+                company_size: '1-99',
+                phone_number: '11111111',
+                number_of_users: '17',
+                country: 'Norway'
+              }
+            end
 
-      it_behaves_like 'hides email confirmation warning'
-      it_behaves_like GroupInviteMembers
-
-      context 'when the trial onboarding is active' do
-        let_it_be(:group) { create(:group) }
-        let_it_be(:trial_onboarding_flow_params) { { trial_onboarding_flow: true, glm_source: 'about.gitlab.com', glm_content: 'content' } }
-        let_it_be(:trial_onboarding_issues_enabled) { true }
-        let_it_be(:apply_trial_params) do
-          {
-            uid: user.id,
-            trial_user: ActionController::Parameters.new(
+            let_it_be(:trial_user_params) do
               {
-                glm_source: 'about.gitlab.com',
-                glm_content: 'content',
-                namespace_id: group.id,
+                work_email: user.email,
+                first_name: user.first_name,
+                last_name: user.last_name,
+                uid: user.id,
+                skip_email_confirmation: true,
                 gitlab_com_trial: true,
-                sync_to_gl: true
+                provider: 'gitlab',
+                newsletter_segment: user.email_opted_in
               }
-            ).permit!
-          }
-        end
+            end
+
+            let_it_be(:trial_params) do
+              {
+                trial_user: ActionController::Parameters.new(trial_form_params.except(:trial).merge(trial_user_params)).permit!
+              }
+            end
+
+            let_it_be(:apply_trial_params) do
+              {
+                uid: user.id,
+                trial_user: ActionController::Parameters.new(
+                  {
+                    glm_source: 'gitlab.com',
+                    glm_content: 'content',
+                    namespace_id: group.id,
+                    gitlab_com_trial: true,
+                    sync_to_gl: true
+                  }
+                ).permit!
+              }
+            end
+
+            before do
+              allow(controller).to receive(:experiment_enabled?).with(:trial_during_signup).and_return(true)
+            end
+
+            context 'when a user chooses a trial - create_lead_and_apply_trial_flow' do
+              context 'when successfully creating a lead and applying trial' do
+                before do
+                  expect_next_instance_of(Groups::CreateService) do |service|
+                    expect(service).to receive(:execute).and_return(group)
+                  end
+                  expect_next_instance_of(GitlabSubscriptions::CreateLeadService) do |service|
+                    expect(service).to receive(:execute).with(trial_params).and_return(success: true)
+                  end
+                  expect_next_instance_of(GitlabSubscriptions::ApplyTrialService) do |service|
+                    expect(service).to receive(:execute).with(apply_trial_params).and_return({ success: true })
+                  end
+                end
+
+                it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: group.id, trial: true)) }
+              end
+
+              context 'when failing to create a lead and apply trial' do
+                before do
+                  expect_next_instance_of(Groups::CreateService) do |service|
+                    expect(service).to receive(:execute).and_return(group)
+                  end
+                  expect_next_instance_of(GitlabSubscriptions::CreateLeadService) do |service|
+                    expect(service).to receive(:execute).with(trial_params).and_return(success: false)
+                  end
+                end
+
+                it { is_expected.to render_template(:new) }
+              end
+            end
+
+            context 'when user chooses no trial' do
+              let_it_be(:trial_form_params) { { trial: 'false' } }
+
+              it 'calls the record user trial_during_signup experiment' do
+                expect_next_instance_of(Groups::CreateService) do |service|
+                  expect(service).to receive(:execute).and_return(group)
+                end
+
+                expect(controller).to receive(:record_experiment_user).with(:trial_during_signup, trial_chosen: false, namespace_id: group.id)
+
+                expect(subject).to redirect_to(new_users_sign_up_project_path(namespace_id: group.id, trial: false))
+              end
 
-        it 'applies the trial to the group and redirects to the project path' do
-          expect_next_instance_of(::Groups::CreateService) do |service|
-            expect(service).to receive(:execute).and_return(group)
+              it 'does not call trial_during_signup experiment methods' do
+                expect(controller).not_to receive(:create_lead)
+                expect(controller).not_to receive(:apply_trial)
+
+                subject
+              end
+
+              it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: user.groups.last.id, trial: false)) }
+            end
           end
-          expect_next_instance_of(GitlabSubscriptions::ApplyTrialService) do |service|
-            expect(service).to receive(:execute).with(apply_trial_params).and_return({ success: true })
+
+          context 'when not in experiment group for trial_during_signup' do
+            before do
+              allow(controller).to receive(:experiment_enabled?).with(:trial_during_signup).and_return(false)
+            end
+
+            it 'tracks experiment as expected', :experiment do
+              expect_next_instance_of(Groups::CreateService) do |service|
+                expect(service).to receive(:execute).and_return(group)
+              end
+
+              subject
+            end
+
+            context 'with invite on group creation' do
+              it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: user.groups.last.id, trial: false)) }
+
+              it_behaves_like GroupInviteMembers
+            end
           end
-          expect(controller).to receive(:record_experiment_user).with(:remove_known_trial_form_fields, namespace_id: group.id)
-          expect(controller).to receive(:record_experiment_user).with(:trimmed_skip_trial_copy, namespace_id: group.id)
-          expect(controller).to receive(:record_experiment_user).with(:trial_registration_with_social_signin, namespace_id: group.id)
-          expect(controller).to receive(:record_experiment_user).with(:trial_onboarding_issues, namespace_id: group.id)
-          expect(controller).to receive(:record_experiment_conversion_event).with(:remove_known_trial_form_fields)
-          expect(controller).to receive(:record_experiment_conversion_event).with(:trimmed_skip_trial_copy)
-          expect(controller).to receive(:record_experiment_conversion_event).with(:trial_registration_with_social_signin)
-          expect(controller).to receive(:record_experiment_conversion_event).with(:trial_onboarding_issues)
-
-          is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: group.id, trial: false, trial_onboarding_flow: true))
         end
       end
 
-      context 'when the group cannot be saved' do
+      context 'when the group cannot be created' do
         let(:group_params) { { name: '', path: '' } }
 
-        it 'does not create a group' do
+        it 'does not create a group', :aggregate_failures do
           expect { subject }.not_to change { Group.count }
           expect(assigns(:group).errors).not_to be_blank
         end
 
-        it 'does not call trial_during_signup experiment methods' do
-          expect(controller).not_to receive(:create_lead)
-          expect(controller).not_to receive(:apply_trial)
+        it 'does not call call the successful flow' do
+          expect(controller).not_to receive(:create_successful_flow)
 
           subject
         end
 
         it { is_expected.to have_gitlab_http_status(:ok) }
         it { is_expected.to render_template(:new) }
-
-        context 'signup onboarding not enabled' do
-          let(:signup_onboarding_enabled) { false }
-
-          it { is_expected.to have_gitlab_http_status(:not_found) }
-        end
-
-        context 'when the trial onboarding is active' do
-          let_it_be(:group) { create(:group) }
-          let_it_be(:trial_onboarding_flow_params) { { trial_onboarding_flow: true } }
-          let_it_be(:trial_onboarding_issues_enabled) { true }
-
-          it { is_expected.not_to receive(:apply_trial) }
-          it { is_expected.to render_template(:new) }
-        end
       end
     end
   end

ee/spec/support/shared_examples/controllers/concerns/group_invite_members_shared_examples.rb

@@ -2,43 +2,35 @@
 
 RSpec.shared_examples GroupInviteMembers do
   context 'when inviting members', :snowplow do
+    before do
+      allow(Gitlab::Tracking).to receive(:event) # rubocop:disable RSpec/ExpectGitlabTracking
+    end
+
     context 'without valid emails in the params' do
-      it 'only adds creator as member' do
-        expect { subject }.to change(Member, :count).by(1)
+      it 'no invites generated by default' do
+        subject
+
+        expect(assigns(:group).members.invite).to be_empty
       end
 
       it 'does not track the event' do
         subject
 
-        expect_no_snowplow_event
+        expect(Gitlab::Tracking).not_to have_received(:event).with(anything, 'invite_members', label: 'new_group_form') # rubocop:disable RSpec/ExpectGitlabTracking
       end
     end
 
     context 'with valid emails in the params' do
-      before do
-        group_params[:emails] = ['a@a.a', 'b@b.b', '', '', 'x', 'y']
-      end
-
-      it 'adds users with developer access and ignores blank emails' do
-        expect_next_instance_of(Group) do |group|
-          expect(group).to receive(:add_users).with(
-            %w[a@a.a b@b.b x y],
-            Gitlab::Access::DEVELOPER,
-            expires_at: nil,
-            current_user: user
-          ).and_call_original
-        end
+      let(:valid_emails) { %w[a@a.a b@b.b] }
 
-        subject
+      before do
+        group_params[:emails] = valid_emails + ['', '', 'x', 'y']
       end
 
-      it 'sends invitations to valid emails only' do
+      it 'adds users with developer access and ignores blank and invalid emails' do
         subject
 
-        emails = assigns(:group).members.pluck(:invite_email)
-
-        expect(emails).to include('a@a.a', 'b@b.b')
-        expect(emails).not_to include('x', 'y')
+        expect(assigns(:group).members.invite.pluck(:invite_email)).to match_array(valid_emails)
       end
 
       it 'tracks the event' do