diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 4acbb0482f3f475858a136c09287629011c8d046..f96e080ecf801085a47376efdaf0e2a1c9efac05 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -38,6 +38,8 @@ 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, @@ -378,6 +380,12 @@ 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 1cadac7e7d46fb2af9f147f164028045169bd773..78390ddd0998b1ace1bd89c1b3eb9f5531b10d39 100644 --- a/app/experiments/require_verification_for_namespace_creation_experiment.rb +++ b/app/experiments/require_verification_for_namespace_creation_experiment.rb @@ -1,6 +1,10 @@ # 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 @@ -24,4 +28,10 @@ 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 7296560a450e190cada30c6bffee9ab880c1f0de..3fbea0c0472ece31eb10e0d5b1fb7c6ffd098a6c 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_group_creation_enabled? + def require_verification_for_namespace_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 deleted file mode 100644 index 767d5f55bce0796bb633a00d728b4467bb778c7d..0000000000000000000000000000000000000000 --- a/config/feature_flags/experiment/require_verification_for_group_creation.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -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 58e1e9f0536590afcb189d062b9df774856b7bf8..8ee5768492208bc0d133bc92215af5899384c9e1 100644 --- a/ee/app/helpers/ee/groups_helper.rb +++ b/ee/app/helpers/ee/groups_helper.rb @@ -60,22 +60,18 @@ def show_product_purchase_success_alert? !params[:purchased_product].blank? end - override :require_verification_for_group_creation_enabled? - def require_verification_for_group_creation_enabled? + override :require_verification_for_namespace_creation_enabled? + def require_verification_for_namespace_creation_enabled? # Experiment should only run when creating top-level groups return false if params[:parent_id] - experiment(:require_verification_for_group_creation, user: current_user) do |e| - e.candidate { true } - e.control { false } - e.run - end + experiment(:require_verification_for_namespace_creation, user: current_user).run end override :verification_for_group_creation_data def verification_for_group_creation_data { - verification_required: require_verification_for_group_creation_enabled?.to_s, + verification_required: require_verification_for_namespace_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 1ae1aac9da045367eab36e2eec9e089f477f04fa..0d39b20444e2bb174ae36995002ab46f6ad80265 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_group_creation, user: current_user).track(:converted, namespace: group) + experiment(:require_verification_for_namespace_creation, user: current_user).track(:finish_create_group, namespace: group) end end end diff --git a/ee/spec/features/groups/new_spec.rb b/ee/spec/features/groups/new_spec.rb index 7b21aa6cb7865b6096263fc91306c65d850324cd..9d614c05dcce26dffdb1e12bb6bd47a1c28e37bc 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_group_creation: variant) + stub_experiments(require_verification_for_namespace_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 c7bd30c337981d971f1c2fd212ea8e26c57a7eb1..c5ac3b8cc2b4cf22e2326a97d8f4ab06663b8cc6 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_group_creation_enabled?' do + describe '#require_verification_for_namespace_creation_enabled?' do let(:variant) { :control } - subject { helper.require_verification_for_group_creation_enabled? } + subject { helper.require_verification_for_namespace_creation_enabled? } before do - stub_experiments(require_verification_for_group_creation: variant) + stub_experiments(require_verification_for_namespace_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 b899c23f9b6f3d0ae024d0d1c24d0479d61a7bc9..47a012c373a98b6290a583ca6a561c2e67f9582c 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_group_creation experiment conversion tracking', :experiment do + describe 'require_verification_for_namespace_creation experiment conversion tracking', :experiment do subject(:execute) { create_group(user, group_params) } before do - stub_experiments(require_verification_for_group_creation: :control) + stub_experiments(require_verification_for_namespace_creation: :control) end - it 'tracks a "converted" event with the correct context and payload' do - expect(experiment(:require_verification_for_group_creation)).to track( - :converted, + 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, 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 "converted" event' do - expect(experiment(:require_verification_for_group_creation)).not_to track(:converted) + it 'does not track a "finish_create_group" event' do + expect(experiment(:require_verification_for_namespace_creation)).not_to track(:finish_create_group) execute end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 62171528695ec698bcaa3c7cd7f416db28ef032f..a82c5681911ba93ffd57cc059722353192f90f05 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -132,6 +132,29 @@ 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 87417fe1637dd15d158a607a5dac63ba5e9f70c3..5d9b0770fb511191f8cc80f273ad8ec6cf1fc311 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_it_be(:user) { create(:user) } + let(:user) { create(:user, created_at: RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE + 1.hour) } describe '#candidate?' do context 'when experiment subject is candidate' do @@ -56,4 +56,20 @@ 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