diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index f96e080ecf801085a47376efdaf0e2a1c9efac05..4acbb0482f3f475858a136c09287629011c8d046 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -38,8 +38,6 @@ class GroupsController < Groups::ApplicationController before_action :check_export_rate_limit!, only: [:export, :download_export] - before_action :track_experiment_event, only: [:new] - helper_method :captcha_required? skip_cross_project_access_check :index, :new, :create, :edit, :update, @@ -380,12 +378,6 @@ def captcha_enabled? def captcha_required? captcha_enabled? && !params[:parent_id] end - - def track_experiment_event - return if params[:parent_id] - - experiment(:require_verification_for_namespace_creation, user: current_user).track(:start_create_group) - end end GroupsController.prepend_mod_with('GroupsController') diff --git a/app/experiments/require_verification_for_namespace_creation_experiment.rb b/app/experiments/require_verification_for_namespace_creation_experiment.rb index 78390ddd0998b1ace1bd89c1b3eb9f5531b10d39..1cadac7e7d46fb2af9f147f164028045169bd773 100644 --- a/app/experiments/require_verification_for_namespace_creation_experiment.rb +++ b/app/experiments/require_verification_for_namespace_creation_experiment.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass - exclude :existing_user - - EXPERIMENT_START_DATE = Date.new(2022, 1, 31) - def control_behavior false end @@ -28,10 +24,4 @@ def record_conversion(namespace) def subject context.value[:user] end - - def existing_user - return false unless user_or_actor - - user_or_actor.created_at < EXPERIMENT_START_DATE - end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 3fbea0c0472ece31eb10e0d5b1fb7c6ffd098a6c..7296560a450e190cada30c6bffee9ab880c1f0de 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -139,7 +139,7 @@ def verification_for_group_creation_data {} end - def require_verification_for_namespace_creation_enabled? + def require_verification_for_group_creation_enabled? # overridden in EE false end diff --git a/config/feature_flags/experiment/require_verification_for_group_creation.yml b/config/feature_flags/experiment/require_verification_for_group_creation.yml new file mode 100644 index 0000000000000000000000000000000000000000..767d5f55bce0796bb633a00d728b4467bb778c7d --- /dev/null +++ b/config/feature_flags/experiment/require_verification_for_group_creation.yml @@ -0,0 +1,8 @@ +--- +name: require_verification_for_group_creation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77569 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349857 +milestone: '14.7' +type: experiment +group: group::activation +default_enabled: false diff --git a/ee/app/helpers/ee/groups_helper.rb b/ee/app/helpers/ee/groups_helper.rb index 8ee5768492208bc0d133bc92215af5899384c9e1..58e1e9f0536590afcb189d062b9df774856b7bf8 100644 --- a/ee/app/helpers/ee/groups_helper.rb +++ b/ee/app/helpers/ee/groups_helper.rb @@ -60,18 +60,22 @@ def show_product_purchase_success_alert? !params[:purchased_product].blank? end - override :require_verification_for_namespace_creation_enabled? - def require_verification_for_namespace_creation_enabled? + override :require_verification_for_group_creation_enabled? + def require_verification_for_group_creation_enabled? # Experiment should only run when creating top-level groups return false if params[:parent_id] - experiment(:require_verification_for_namespace_creation, user: current_user).run + experiment(:require_verification_for_group_creation, user: current_user) do |e| + e.candidate { true } + e.control { false } + e.run + end end override :verification_for_group_creation_data def verification_for_group_creation_data { - verification_required: require_verification_for_namespace_creation_enabled?.to_s, + verification_required: require_verification_for_group_creation_enabled?.to_s, verification_form_url: ::Gitlab::SubscriptionPortal::REGISTRATION_VALIDATION_FORM_URL, subscriptions_url: ::Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL } diff --git a/ee/app/services/ee/groups/create_service.rb b/ee/app/services/ee/groups/create_service.rb index 0d39b20444e2bb174ae36995002ab46f6ad80265..1ae1aac9da045367eab36e2eec9e089f477f04fa 100644 --- a/ee/app/services/ee/groups/create_service.rb +++ b/ee/app/services/ee/groups/create_service.rb @@ -70,7 +70,7 @@ def track_verification_experiment_conversion return unless group.persisted? return unless group.root? - experiment(:require_verification_for_namespace_creation, user: current_user).track(:finish_create_group, namespace: group) + experiment(:require_verification_for_group_creation, user: current_user).track(:converted, namespace: group) end end end diff --git a/ee/spec/features/groups/new_spec.rb b/ee/spec/features/groups/new_spec.rb index 9d614c05dcce26dffdb1e12bb6bd47a1c28e37bc..7b21aa6cb7865b6096263fc91306c65d850324cd 100644 --- a/ee/spec/features/groups/new_spec.rb +++ b/ee/spec/features/groups/new_spec.rb @@ -33,7 +33,7 @@ end before do - stub_experiments(require_verification_for_namespace_creation: variant) + stub_experiments(require_verification_for_group_creation: variant) end context 'when creating a top-level group' do diff --git a/ee/spec/helpers/ee/groups_helper_spec.rb b/ee/spec/helpers/ee/groups_helper_spec.rb index c5ac3b8cc2b4cf22e2326a97d8f4ab06663b8cc6..c7bd30c337981d971f1c2fd212ea8e26c57a7eb1 100644 --- a/ee/spec/helpers/ee/groups_helper_spec.rb +++ b/ee/spec/helpers/ee/groups_helper_spec.rb @@ -223,13 +223,13 @@ end end - describe '#require_verification_for_namespace_creation_enabled?' do + describe '#require_verification_for_group_creation_enabled?' do let(:variant) { :control } - subject { helper.require_verification_for_namespace_creation_enabled? } + subject { helper.require_verification_for_group_creation_enabled? } before do - stub_experiments(require_verification_for_namespace_creation: variant) + stub_experiments(require_verification_for_group_creation: variant) end context 'when in candidate path' do diff --git a/ee/spec/services/groups/create_service_spec.rb b/ee/spec/services/groups/create_service_spec.rb index 47a012c373a98b6290a583ca6a561c2e67f9582c..b899c23f9b6f3d0ae024d0d1c24d0479d61a7bc9 100644 --- a/ee/spec/services/groups/create_service_spec.rb +++ b/ee/spec/services/groups/create_service_spec.rb @@ -163,16 +163,16 @@ end end - describe 'require_verification_for_namespace_creation experiment conversion tracking', :experiment do + describe 'require_verification_for_group_creation experiment conversion tracking', :experiment do subject(:execute) { create_group(user, group_params) } before do - stub_experiments(require_verification_for_namespace_creation: :control) + stub_experiments(require_verification_for_group_creation: :control) end - it 'tracks a "finish_create_group" event with the correct context and payload' do - expect(experiment(:require_verification_for_namespace_creation)).to track( - :finish_create_group, + it 'tracks a "converted" event with the correct context and payload' do + expect(experiment(:require_verification_for_group_creation)).to track( + :converted, namespace: an_instance_of(Group) ).on_next_instance.with_context(user: user) @@ -180,8 +180,8 @@ end shared_examples 'does not track' do - it 'does not track a "finish_create_group" event' do - expect(experiment(:require_verification_for_namespace_creation)).not_to track(:finish_create_group) + it 'does not track a "converted" event' do + expect(experiment(:require_verification_for_group_creation)).not_to track(:converted) execute end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index a82c5681911ba93ffd57cc059722353192f90f05..62171528695ec698bcaa3c7cd7f416db28ef032f 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -132,29 +132,6 @@ end end end - - describe 'require_verification_for_namespace_creation experiment', :experiment do - before do - sign_in(owner) - stub_experiments(require_verification_for_namespace_creation: :candidate) - end - - it 'tracks a "start_create_group" event' do - expect(experiment(:require_verification_for_namespace_creation)).to track( - :start_create_group - ).on_next_instance.with_context(user: owner) - - get :new - end - - context 'when creating a sub-group' do - it 'does not track a "start_create_group" event' do - expect(experiment(:require_verification_for_namespace_creation)).not_to track(:start_create_group) - - get :new, params: { parent_id: group.id } - end - end - end end describe 'GET #activity' do diff --git a/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb b/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb index 5d9b0770fb511191f8cc80f273ad8ec6cf1fc311..87417fe1637dd15d158a607a5dac63ba5e9f70c3 100644 --- a/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb +++ b/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb @@ -5,7 +5,7 @@ RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do subject(:experiment) { described_class.new(user: user) } - let(:user) { create(:user, created_at: RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE + 1.hour) } + let_it_be(:user) { create(:user) } describe '#candidate?' do context 'when experiment subject is candidate' do @@ -56,20 +56,4 @@ end end end - - describe 'exclusions' do - context 'when user is new' do - it 'is not excluded' do - expect(subject).not_to exclude(user: user) - end - end - - context 'when user is NOT new' do - let(:user) { create(:user, created_at: RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE - 1.day) } - - it 'is excluded' do - expect(subject).to exclude(user: user) - end - end - end end