Commit 2355297f authored by Eugie Limpin's avatar Eugie Limpin Committed by Alper Akgun

Enable experiment only for new users

Credit card verification experiment should only be enabled for users
created _after_ the experiment was made active.
parent c7c5c096
...@@ -38,6 +38,8 @@ class GroupsController < Groups::ApplicationController ...@@ -38,6 +38,8 @@ class GroupsController < Groups::ApplicationController
before_action :check_export_rate_limit!, only: [:export, :download_export] before_action :check_export_rate_limit!, only: [:export, :download_export]
before_action :track_experiment_event, only: [:new]
helper_method :captcha_required? helper_method :captcha_required?
skip_cross_project_access_check :index, :new, :create, :edit, :update, skip_cross_project_access_check :index, :new, :create, :edit, :update,
...@@ -378,6 +380,12 @@ class GroupsController < Groups::ApplicationController ...@@ -378,6 +380,12 @@ class GroupsController < Groups::ApplicationController
def captcha_required? def captcha_required?
captcha_enabled? && !params[:parent_id] captcha_enabled? && !params[:parent_id]
end 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 end
GroupsController.prepend_mod_with('GroupsController') GroupsController.prepend_mod_with('GroupsController')
# frozen_string_literal: true # frozen_string_literal: true
class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass
exclude :existing_user
EXPERIMENT_START_DATE = Date.new(2022, 1, 31)
def control_behavior def control_behavior
false false
end end
...@@ -24,4 +28,10 @@ class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment ...@@ -24,4 +28,10 @@ class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment
def subject def subject
context.value[:user] context.value[:user]
end end
def existing_user
return false unless user_or_actor
user_or_actor.created_at < EXPERIMENT_START_DATE
end
end end
...@@ -139,7 +139,7 @@ module GroupsHelper ...@@ -139,7 +139,7 @@ module GroupsHelper
{} {}
end end
def require_verification_for_group_creation_enabled? def require_verification_for_namespace_creation_enabled?
# overridden in EE # overridden in EE
false false
end end
......
---
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
...@@ -60,22 +60,18 @@ module EE ...@@ -60,22 +60,18 @@ module EE
!params[:purchased_product].blank? !params[:purchased_product].blank?
end end
override :require_verification_for_group_creation_enabled? override :require_verification_for_namespace_creation_enabled?
def require_verification_for_group_creation_enabled? def require_verification_for_namespace_creation_enabled?
# Experiment should only run when creating top-level groups # Experiment should only run when creating top-level groups
return false if params[:parent_id] return false if params[:parent_id]
experiment(:require_verification_for_group_creation, user: current_user) do |e| experiment(:require_verification_for_namespace_creation, user: current_user).run
e.candidate { true }
e.control { false }
e.run
end
end end
override :verification_for_group_creation_data override :verification_for_group_creation_data
def 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, verification_form_url: ::Gitlab::SubscriptionPortal::REGISTRATION_VALIDATION_FORM_URL,
subscriptions_url: ::Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL subscriptions_url: ::Gitlab::SubscriptionPortal::SUBSCRIPTIONS_URL
} }
......
...@@ -70,7 +70,7 @@ module EE ...@@ -70,7 +70,7 @@ module EE
return unless group.persisted? return unless group.persisted?
return unless group.root? 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 end
end end
......
...@@ -33,7 +33,7 @@ RSpec.describe 'New Group page' do ...@@ -33,7 +33,7 @@ RSpec.describe 'New Group page' do
end end
before do before do
stub_experiments(require_verification_for_group_creation: variant) stub_experiments(require_verification_for_namespace_creation: variant)
end end
context 'when creating a top-level group' do context 'when creating a top-level group' do
......
...@@ -223,13 +223,13 @@ RSpec.describe GroupsHelper do ...@@ -223,13 +223,13 @@ RSpec.describe GroupsHelper do
end end
end end
describe '#require_verification_for_group_creation_enabled?' do describe '#require_verification_for_namespace_creation_enabled?' do
let(:variant) { :control } let(:variant) { :control }
subject { helper.require_verification_for_group_creation_enabled? } subject { helper.require_verification_for_namespace_creation_enabled? }
before do before do
stub_experiments(require_verification_for_group_creation: variant) stub_experiments(require_verification_for_namespace_creation: variant)
end end
context 'when in candidate path' do context 'when in candidate path' do
......
...@@ -163,16 +163,16 @@ RSpec.describe Groups::CreateService, '#execute' do ...@@ -163,16 +163,16 @@ RSpec.describe Groups::CreateService, '#execute' do
end end
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) } subject(:execute) { create_group(user, group_params) }
before do before do
stub_experiments(require_verification_for_group_creation: :control) stub_experiments(require_verification_for_namespace_creation: :control)
end end
it 'tracks a "converted" event with the correct context and payload' do it 'tracks a "finish_create_group" event with the correct context and payload' do
expect(experiment(:require_verification_for_group_creation)).to track( expect(experiment(:require_verification_for_namespace_creation)).to track(
:converted, :finish_create_group,
namespace: an_instance_of(Group) namespace: an_instance_of(Group)
).on_next_instance.with_context(user: user) ).on_next_instance.with_context(user: user)
...@@ -180,8 +180,8 @@ RSpec.describe Groups::CreateService, '#execute' do ...@@ -180,8 +180,8 @@ RSpec.describe Groups::CreateService, '#execute' do
end end
shared_examples 'does not track' do shared_examples 'does not track' do
it 'does not track a "converted" event' do it 'does not track a "finish_create_group" event' do
expect(experiment(:require_verification_for_group_creation)).not_to track(:converted) expect(experiment(:require_verification_for_namespace_creation)).not_to track(:finish_create_group)
execute execute
end end
......
...@@ -132,6 +132,29 @@ RSpec.describe GroupsController, factory_default: :keep do ...@@ -132,6 +132,29 @@ RSpec.describe GroupsController, factory_default: :keep do
end end
end 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 end
describe 'GET #activity' do describe 'GET #activity' do
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do
subject(:experiment) { described_class.new(user: user) } 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 describe '#candidate?' do
context 'when experiment subject is candidate' do context 'when experiment subject is candidate' do
...@@ -56,4 +56,20 @@ RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do ...@@ -56,4 +56,20 @@ RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do
end end
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment