diff --git a/ee/app/controllers/registrations/groups_controller.rb b/ee/app/controllers/registrations/groups_controller.rb index 53812689de93be5a52a042b3876638a22f7b5b22..8406639db03621559bf6c47706a2ab78bfe33d57 100644 --- a/ee/app/controllers/registrations/groups_controller.rb +++ b/ee/app/controllers/registrations/groups_controller.rb @@ -19,14 +19,43 @@ def new 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 @@ def create 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 diff --git a/ee/app/controllers/registrations/projects_controller.rb b/ee/app/controllers/registrations/projects_controller.rb index f13763cebc618a5ad7c717fa0f9cc69224274c01..cdc2ff126eabeda5b3b2e6ba57d5bdccfd06a746 100644 --- a/ee/app/controllers/registrations/projects_controller.rb +++ b/ee/app/controllers/registrations/projects_controller.rb @@ -5,7 +5,10 @@ class ProjectsController < ApplicationController 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 @@ def create 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 @@ def create_learn_gitlab_project 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 diff --git a/ee/spec/controllers/registrations/groups_controller_spec.rb b/ee/spec/controllers/registrations/groups_controller_spec.rb index 01409b1ae6c5135ec469ac0fe8a9605940bec0b2..5c38a6fbb42b0871a6dead74ac827fc041023320 100644 --- a/ee/spec/controllers/registrations/groups_controller_spec.rb +++ b/ee/spec/controllers/registrations/groups_controller_spec.rb @@ -98,186 +98,229 @@ 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 diff --git a/ee/spec/support/shared_examples/controllers/concerns/group_invite_members_shared_examples.rb b/ee/spec/support/shared_examples/controllers/concerns/group_invite_members_shared_examples.rb index f52c0c9f0f9d458cffccd180fac3b84d76cc845a..c538fe59f3554a2d66fa15b8472efc8d35ba11df 100644 --- a/ee/spec/support/shared_examples/controllers/concerns/group_invite_members_shared_examples.rb +++ b/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