Merge branch...

Merge branch '332987-experiment-clean-up-registrations_group_invite-group-invite-on-separate-page' into 'master'

Experiment Clean Up: make registrations_group_invite permanent [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!64001

ee/app/controllers/registrations/group_invites_controller.rb

@@ -4,6 +4,7 @@ module Registrations
   class GroupInvitesController < Groups::ApplicationController
     layout 'checkout'
 
+    before_action :check_if_gl_com_or_dev
     before_action :authorize_invite_to_group!
 
     feature_category :navigation
@@ -12,12 +13,7 @@ module Registrations
     end
 
     def create
-      result = Members::CreateService.new(current_user, invite_params).execute
-
-      if result[:status] == :success
-        experiment(:registrations_group_invite, actor: current_user)
-          .track(:invites_sent, property: group.id.to_s, value: group.members.invite.size)
-      end
+      Members::CreateService.new(current_user, invite_params).execute
 
       redirect_to new_users_sign_up_project_path(namespace_id: group.id,
                                                  trial: helpers.in_trial_during_signup_flow?,

ee/app/controllers/registrations/groups_controller.rb

@@ -6,7 +6,7 @@ module Registrations
 
     layout 'minimal'
 
-    before_action :check_signup_onboarding_enabled
+    before_action :check_if_gl_com_or_dev
     before_action :authorize_create_group!, only: :new
 
     feature_category :onboarding
@@ -37,10 +37,6 @@ module Registrations
 
     private
 
-    def check_signup_onboarding_enabled
-      access_denied! unless helpers.signup_onboarding_enabled?
-    end
-
     def create_successful_flow
       if helpers.in_trial_onboarding_flow?
         apply_trial_for_trial_onboarding_flow
@@ -64,7 +60,7 @@ module Registrations
         record_experiment_conversion_event(:remove_known_trial_form_fields)
         record_experiment_conversion_event(:trial_onboarding_issues)
 
-        registrations_group_invite_flow(trial_onboarding_flow: true)
+        redirect_to new_users_sign_up_group_invite_path(group_id: @group.id, trial: helpers.in_trial_during_signup_flow?, trial_onboarding_flow: true)
       else
         render action: :new
       end
@@ -74,26 +70,18 @@ module Registrations
       record_experiment_conversion_event(:learn_gitlab_a, namespace_id: @group.id)
       record_experiment_conversion_event(:learn_gitlab_b, namespace_id: @group.id)
 
-      create_lead_and_apply_trial_flow
-    end
-
-    def create_lead_and_apply_trial_flow
       if helpers.in_trial_during_signup_flow?
-        if create_lead && apply_trial
-          registrations_group_invite_flow
-        else
-          render action: :new
-        end
+        create_lead_and_apply_trial_flow
       else
-        registrations_group_invite_flow
+        redirect_to new_users_sign_up_group_invite_path(group_id: @group.id, trial: false)
       end
     end
 
-    def registrations_group_invite_flow(options = {})
-      experiment(:registrations_group_invite, actor: current_user) do |experiment_instance|
-        experiment_instance.use { redirect_to new_users_sign_up_project_path({ namespace_id: @group.id, trial: helpers.in_trial_during_signup_flow? }.merge(options) ) } # control
-        experiment_instance.try(:invite_page) { redirect_to new_users_sign_up_group_invite_path({ group_id: @group.id, trial: helpers.in_trial_during_signup_flow? }.merge(options)) } # with separate page
-        experiment_instance.track(:created, property: @group.id.to_s)
+    def create_lead_and_apply_trial_flow
+      if create_lead && apply_trial
+        redirect_to new_users_sign_up_group_invite_path(group_id: @group.id, trial: true)
+      else
+        render action: :new
       end
     end
 

ee/app/controllers/registrations/projects_controller.rb

@@ -8,7 +8,7 @@ module Registrations
     LEARN_GITLAB_TEMPLATE = 'learn_gitlab.tar.gz'
     LEARN_GITLAB_ULTIMATE_TEMPLATE = 'learn_gitlab_ultimate_trial.tar.gz'
 
-    before_action :check_signup_onboarding_enabled
+    before_action :check_if_gl_com_or_dev
     before_action only: [:new] do
       set_namespace
       authorize_create_project!
@@ -31,8 +31,6 @@ module Registrations
           learn_gitlab_project_id: learn_gitlab_project.id
         }
 
-        experiment(:registrations_group_invite, actor: current_user)
-          .track(:signup_successful, property: @project.namespace_id.to_s)
         experiment(:jobs_to_be_done, user: current_user)
           .track(:create_project, project: @project)
 
@@ -58,10 +56,6 @@ module Registrations
 
     private
 
-    def check_signup_onboarding_enabled
-      access_denied! unless helpers.signup_onboarding_enabled?
-    end
-
     def create_learn_gitlab_project
       File.open(learn_gitlab_template_path) do |archive|
         ::Projects::GitlabProjectsImportService.new(

ee/app/views/registrations/groups/new.html.haml

@@ -44,11 +44,7 @@
             = render partial: 'shared/groups/visibility_level', locals: { f: f }
         - if !in_trial_onboarding_flow? && show_trial_during_signup?
           = render partial: 'shared/groups/trial_form'
-        - else
-          - experiment(:registrations_group_invite, actor: current_user) do |experiment_instance|
-            - experiment_instance.use do
-              = render partial: 'shared/groups/invite_members'
-            - experiment_instance.try(:invite_page) {}
+
         .row
           .form-group.col-sm-12.gl-mb-0
             = button_tag class: %w[btn gl-button btn-success gl-w-full!] do

ee/config/feature_flags/experiment/registrations_group_invite.yml

----
-name: registrations_group_invite
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52371
-rollout_issue_url: https://gitlab.com/gitlab-org/growth/team-tasks/-/issues/351
-milestone: '13.10'
-type: experiment
-group: group::expansion
-default_enabled: false

ee/spec/controllers/registrations/projects_controller_spec.rb

@@ -18,32 +18,34 @@ RSpec.describe Registrations::ProjectsController do
     end
 
     context 'with an authenticated user' do
-      let(:signup_onboarding_enabled) { true }
+      let(:dev_env_or_com) { true }
 
       before do
         sign_in(user)
-        allow(controller.helpers).to receive(:signup_onboarding_enabled?).and_return(signup_onboarding_enabled)
+        allow(::Gitlab).to receive(:dev_env_or_com?).and_return(dev_env_or_com)
       end
 
-      it { is_expected.to have_gitlab_http_status(:not_found) }
+      context 'when on .com' do
+        it { is_expected.to have_gitlab_http_status(:not_found) }
 
-      context 'with a namespace in the URL' do
-        subject { get :new, params: { namespace_id: namespace.id } }
+        context 'with a namespace in the URL' do
+          subject { get :new, params: { namespace_id: namespace.id } }
 
-        it { is_expected.to have_gitlab_http_status(:not_found) }
+          it { is_expected.to have_gitlab_http_status(:not_found) }
 
-        context 'with sufficient access' do
-          before do
-            namespace.add_owner(user)
-          end
+          context 'with sufficient access' do
+            before do
+              namespace.add_owner(user)
+            end
 
-          it { is_expected.to have_gitlab_http_status(:ok) }
-          it { is_expected.to render_template(:new) }
+            it { is_expected.to have_gitlab_http_status(:ok) }
+            it { is_expected.to render_template(:new) }
+          end
         end
       end
 
-      context 'with signup onboarding not enabled' do
-        let(:signup_onboarding_enabled) { false }
+      context 'when not on .com' do
+        let(:dev_env_or_com) { false }
 
         it { is_expected.to have_gitlab_http_status(:not_found) }
       end
@@ -56,7 +58,7 @@ RSpec.describe Registrations::ProjectsController do
     let_it_be(:trial_onboarding_flow_params) { {} }
 
     let(:params) { { namespace_id: namespace.id, name: 'New project', path: 'project-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
-    let(:signup_onboarding_enabled) { true }
+    let(:dev_env_or_com) { true }
 
     context 'with an unauthenticated user' do
       it { is_expected.to have_gitlab_http_status(:redirect) }
@@ -74,7 +76,7 @@ RSpec.describe Registrations::ProjectsController do
         namespace.add_owner(user)
         sign_in(user)
         stub_experiment_for_subject(trial_onboarding_issues: trial_onboarding_issues_enabled)
-        allow(controller.helpers).to receive(:signup_onboarding_enabled?).and_return(signup_onboarding_enabled)
+        allow(::Gitlab).to receive(:dev_env_or_com?).and_return(dev_env_or_com)
       end
 
       it 'creates a new project, a "Learn GitLab" project, sets a cookie and redirects to the experience level page' do
@@ -108,15 +110,6 @@ RSpec.describe Registrations::ProjectsController do
         subject
       end
 
-      it 'tracks the registrations_group_invite experiment as expected', :experiment do
-        expect(experiment(:registrations_group_invite))
-          .to track(:signup_successful, { property: namespace.id.to_s })
-                .on_next_instance
-                .with_context(actor: user)
-
-        subject
-      end
-
       context 'learn gitlab project' do
         using RSpec::Parameterized::TableSyntax
 
@@ -183,7 +176,7 @@ RSpec.describe Registrations::ProjectsController do
       end
 
       context 'with signup onboarding not enabled' do
-        let(:signup_onboarding_enabled) { false }
+        let(:dev_env_or_com) { false }
 
         it { is_expected.to have_gitlab_http_status(:not_found) }
       end

ee/spec/features/registrations/group_invites_during_signup_flow_spec.rb

@@ -10,7 +10,6 @@ RSpec.describe 'User is able to invite members to group during signup', :js, :ex
 
   before do
     allow(Gitlab).to receive(:dev_env_or_com?).and_return(true)
-    stub_experiments(registrations_group_invite: :invite_page)
     sign_in(user)
   end
 

ee/spec/features/registrations/trial_during_signup_flow_spec.rb

@@ -8,7 +8,6 @@ RSpec.describe 'User sees new onboarding flow', :js do
 
   before do
     allow(Gitlab).to receive(:com?).and_return(true)
-    stub_feature_flags(registrations_group_invite: false)
     sign_in(user)
     visit users_sign_up_welcome_path
 
@@ -20,13 +19,13 @@ RSpec.describe 'User sees new onboarding flow', :js do
     expect(page).to have_content('GitLab Ultimate trial (optional)')
   end
 
-  it 'shows the expected behavior with no trial chosen' do
+  it 'shows the expected behavior with no trial chosen', :aggregate_failures do
     fill_in 'group_name', with: 'test'
 
     click_on 'Create group'
 
     expect(page).not_to have_content('Congratulations, your free trial is activated.')
-    expect(page).to have_content('Create/import your first project')
+    expect(page).to have_content('Invite your teammates')
   end
 
   it 'shows the expected behavior with trial chosen' do
@@ -73,6 +72,6 @@ RSpec.describe 'User sees new onboarding flow', :js do
     click_on 'Create group'
 
     expect(page).to have_content('Congratulations, your free trial is activated.')
-    expect(page).to have_content('Create/import your first project')
+    expect(page).to have_content('Invite your teammates')
   end
 end

ee/spec/features/registrations/user_sees_new_onboarding_flow_spec.rb

@@ -5,7 +5,6 @@ require 'spec_helper'
 RSpec.describe 'User sees new onboarding flow', :js do
   before do
     stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 200)
-    stub_feature_flags(registration_group_invite: false)
     allow(Gitlab).to receive(:com?).and_return(true)
     gitlab_sign_in(:user)
     visit users_sign_up_welcome_path
@@ -29,6 +28,10 @@ RSpec.describe 'User sees new onboarding flow', :js do
 
     click_on 'Create group'
 
+    expect(page).to have_content('Invite your teammates')
+
+    click_on 'Skip this for now'
+
     expect(page).to have_content('Create/import your first project')
     expect(page).to have_content('Your profile Your GitLab group Your first project')
     expect(page).to have_css('li.current', text: 'Your first project')

ee/spec/requests/registrations/group_invites_spec.rb

@@ -7,12 +7,15 @@ RSpec.describe 'view group invites' do
   let_it_be(:user) { create(:user) }
   let_it_be(:not_authorized_group) { create(:group) }
 
+  let(:dev_env_or_com) { true }
+
   before_all do
     group.add_owner(user)
   end
 
   before do
     login_as(user)
+    allow(::Gitlab).to receive(:dev_env_or_com?).and_return(dev_env_or_com)
   end
 
   describe 'GET /users/sign_up/group_invites/new' do
@@ -28,6 +31,16 @@ RSpec.describe 'view group invites' do
       end
     end
 
+    context 'when not on .com' do
+      let(:dev_env_or_com) { false }
+
+      it 'returns not_found' do
+        get_request
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
     context 'when user is not authorized to invite for the group' do
       let(:group_params) { { group_id: not_authorized_group.id } }
 
@@ -59,12 +72,6 @@ RSpec.describe 'view group invites' do
 
             expect(group.members.invite).to be_empty
           end
-
-          it 'does not track experiment', :experiment do
-            expect(experiment(:registrations_group_invite)).not_to track(:invites_sent)
-
-            post_request
-          end
         end
 
         context 'with valid emails in the params' do
@@ -85,15 +92,6 @@ RSpec.describe 'view group invites' do
               user: user
             )
           end
-
-          it 'tracks experiment as expected', :experiment do
-            expect(experiment(:registrations_group_invite))
-              .to track(:invites_sent, { property: group.id.to_s, value: valid_emails.size })
-                    .on_next_instance
-                    .with_context(actor: user)
-
-            post_request
-          end
         end
       end
 
@@ -109,6 +107,16 @@ RSpec.describe 'view group invites' do
       end
     end
 
+    context 'when not on .com' do
+      let(:dev_env_or_com) { false }
+
+      it 'returns not_found' do
+        post_request
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
     context 'when user is not authorized to invite for the group' do
       let(:group_params) { { group_id: not_authorized_group.id } }
 

ee/spec/views/registrations/groups/new.html.haml_spec.rb

@@ -35,9 +35,8 @@ RSpec.describe 'registrations/groups/new' do
   context 'in trial onboarding' do
     let_it_be(:trial_onboarding_flow) { true }
 
-    it 'hides trial form and shows invite members' do
+    it 'hides trial form' do
       is_expected.not_to have_content('Company name')
-      is_expected.to have_selector('.js-invite-members')
     end
 
     it 'hides the progress bar' do