Commit 26ec9653 authored by Eugie Limpin's avatar Eugie Limpin Committed by Alper Akgun

Revert "Merge branch 'revert-b59c6630' into 'master'"

This reverts commit 6d4d3531, reversing
changes made to a125e8fb.

Changelog: fixed
parent 866f71d0
......@@ -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 @@ class GroupsController < Groups::ApplicationController
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')
# 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 @@ class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment
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
......@@ -139,7 +139,7 @@ module GroupsHelper
{}
end
def require_verification_for_group_creation_enabled?
def require_verification_for_namespace_creation_enabled?
# overridden in EE
false
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,24 +60,20 @@ module EE
!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?
# Skip the verification for admins and auditors (added mainly for E2E tests)
return false if current_user.can_read_all_resources?
# 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
}
......
......@@ -70,7 +70,7 @@ module EE
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
......
......@@ -40,6 +40,8 @@ RSpec.describe Registrations::GroupsProjectsController, :experiment do
end
shared_context 'records a conversion event' do
let_it_be(:user_created_at) { RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE + 1.hour }
let_it_be(:user) { create(:user, created_at: user_created_at) }
let_it_be(:experiment) { create(:experiment, name: :require_verification_for_namespace_creation) }
let_it_be(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) }
......
......@@ -3,9 +3,9 @@
require 'spec_helper'
RSpec.describe 'New Group page' do
describe 'toggling the invite members section', :js do
let_it_be(:user) { create(:user) }
describe 'toggling the invite members section', :js do
before do
sign_in(user)
visit new_group_path
......@@ -27,13 +27,16 @@ RSpec.describe 'New Group page' do
let(:variant) { :control }
let(:query_params) { {} }
let_it_be(:user_created_at) { RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE + 1.hour }
let_it_be(:user) { create(:user, created_at: user_created_at) }
subject(:visit_new_group_page) do
sign_in(user)
visit new_group_path(query_params)
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
......
......@@ -223,13 +223,15 @@ RSpec.describe GroupsHelper do
end
end
describe '#require_verification_for_group_creation_enabled?' do
describe '#require_verification_for_namespace_creation_enabled?' do
let(:user_created_at) { RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE + 1.hour }
let(:owner) { create(:user, created_at: user_created_at) }
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
......
......@@ -163,16 +163,16 @@ RSpec.describe Groups::CreateService, '#execute' do
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 @@ RSpec.describe Groups::CreateService, '#execute' do
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
......
......@@ -132,6 +132,29 @@ RSpec.describe GroupsController, factory_default: :keep do
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
......
......@@ -5,7 +5,8 @@ require 'spec_helper'
RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do
subject(:experiment) { described_class.new(user: user) }
let_it_be(:user) { create(:user) }
let(:user_created_at) { RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE + 1.hour }
let(:user) { create(:user, created_at: user_created_at) }
describe '#candidate?' do
context 'when experiment subject is candidate' do
......@@ -56,4 +57,21 @@ RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do
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_created_at) { RequireVerificationForNamespaceCreationExperiment::EXPERIMENT_START_DATE - 1.day }
let(:user) { create(:user, created_at: user_created_at) }
it 'is excluded' do
expect(subject).to exclude(user: user)
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