Commit fac363ef authored by Doug Stull's avatar Doug Stull

Merge branch 'jswain_remove_force_company_trial' into 'master'

Remove `force_company_trial` experiment

See merge request gitlab-org/gitlab!76982
parents 5ccd9798 e5b9c0d4
...@@ -75,10 +75,6 @@ module Registrations ...@@ -75,10 +75,6 @@ module Registrations
MemberTask.for_members(current_user.members).exists? MemberTask.for_members(current_user.members).exists?
end end
# overridden in EE
def trial_params
end
# overridden in EE # overridden in EE
def update_success_path def update_success_path
end end
......
...@@ -7,14 +7,12 @@ class CombinedRegistrationExperiment < ApplicationExperiment # rubocop:disable G ...@@ -7,14 +7,12 @@ class CombinedRegistrationExperiment < ApplicationExperiment # rubocop:disable G
super(source, 'force_company_trial') super(source, 'force_company_trial')
end end
def redirect_path(trial_params) def redirect_path
@trial_params = trial_params
run run
end end
def control_behavior def control_behavior
new_users_sign_up_group_path(@trial_params) new_users_sign_up_group_path
end end
def candidate_behavior def candidate_behavior
......
---
name: force_company_trial
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65287
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335050
milestone: '14.1'
type: experiment
group: group::adoption
default_enabled: false
...@@ -72,16 +72,6 @@ module EE ...@@ -72,16 +72,6 @@ module EE
helpers.signup_onboarding_enabled? helpers.signup_onboarding_enabled?
end end
override :trial_params
def trial_params
return if combined_registration_experiment.variant.name == 'candidate'
experiment(:force_company_trial, user: current_user) do |e|
e.try { { trial: true } }
e.run
end
end
def authorized_for_trial_onboarding! def authorized_for_trial_onboarding!
access_denied! unless can?(current_user, :owner_access, learn_gitlab_project) access_denied! unless can?(current_user, :owner_access, learn_gitlab_project)
end end
...@@ -107,7 +97,7 @@ module EE ...@@ -107,7 +97,7 @@ module EE
path_for_signed_in_user(current_user) path_for_signed_in_user(current_user)
else else
bypass_registration_event(:creating_project) bypass_registration_event(:creating_project)
experiment(:combined_registration, user: current_user).redirect_path(trial_params) experiment(:combined_registration, user: current_user).redirect_path
end end
end end
......
...@@ -23,8 +23,6 @@ module Registrations ...@@ -23,8 +23,6 @@ module Registrations
if @group.persisted? if @group.persisted?
experiment(:combined_registration, user: current_user).track(:create_group, namespace: @group) experiment(:combined_registration, user: current_user).track(:create_group, namespace: @group)
force_company_trial_experiment.track(:create_group, namespace: @group, user: current_user)
create_successful_flow create_successful_flow
else else
render action: :new render action: :new
...@@ -33,11 +31,6 @@ module Registrations ...@@ -33,11 +31,6 @@ module Registrations
private private
def force_company_trial_experiment
@force_company_trial_experiment ||=
experiment(:force_company_trial, user: current_user)
end
def create_successful_flow def create_successful_flow
if helpers.in_trial_onboarding_flow? if helpers.in_trial_onboarding_flow?
apply_trial_for_trial_onboarding_flow apply_trial_for_trial_onboarding_flow
...@@ -119,11 +112,7 @@ module Registrations ...@@ -119,11 +112,7 @@ module Registrations
result = GitlabSubscriptions::ApplyTrialService.new.execute(apply_trial_params) result = GitlabSubscriptions::ApplyTrialService.new.execute(apply_trial_params)
flash[:alert] = result&.dig(:errors) unless result&.dig(:success) flash[:alert] = result&.dig(:errors) unless result&.dig(:success)
success = result&.dig(:success) result&.dig(:success)
force_company_trial_experiment.track(:create_trial, namespace: @group, user: current_user, label: 'registrations_groups_controller') if success
success
end end
end end
end end
...@@ -28,9 +28,6 @@ module Registrations ...@@ -28,9 +28,6 @@ module Registrations
@learn_gitlab_project = create_learn_gitlab_project @learn_gitlab_project = create_learn_gitlab_project
experiment(:force_company_trial, user: current_user)
.track(:create_project, namespace: @project.namespace, project: @project, user: current_user)
if helpers.registration_verification_enabled? if helpers.registration_verification_enabled?
redirect_to new_users_sign_up_verification_path(url_params) redirect_to new_users_sign_up_verification_path(url_params)
elsif helpers.in_trial_onboarding_flow? elsif helpers.in_trial_onboarding_flow?
......
...@@ -89,7 +89,6 @@ class SubscriptionsController < ApplicationController ...@@ -89,7 +89,6 @@ class SubscriptionsController < ApplicationController
).execute ).execute
if response[:success] if response[:success]
experiment(:force_company_trial, user: current_user).track(:create_subscription, namespace: group, user: current_user)
response[:data] = { location: redirect_location(group) } response[:data] = { location: redirect_location(group) }
end end
......
...@@ -244,7 +244,6 @@ class TrialsController < ApplicationController ...@@ -244,7 +244,6 @@ class TrialsController < ApplicationController
experiment(:trial_registration_with_reassurance, actor: current_user) experiment(:trial_registration_with_reassurance, actor: current_user)
.track(:apply_trial, label: 'trials:apply', namespace: @namespace, user: current_user) .track(:apply_trial, label: 'trials:apply', namespace: @namespace, user: current_user)
experiment(:force_company_trial, user: current_user).track(:create_trial, namespace: @namespace, user: current_user, label: 'trials_controller') if @namespace.created_at > 24.hours.ago
experiment(:combined_registration, user: current_user).track(:create_trial) experiment(:combined_registration, user: current_user).track(:create_trial)
......
...@@ -189,16 +189,6 @@ RSpec.describe Registrations::GroupsController do ...@@ -189,16 +189,6 @@ RSpec.describe Registrations::GroupsController do
it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: group.id, trial: true)) } it { is_expected.to redirect_to(new_users_sign_up_project_path(namespace_id: group.id, trial: true)) }
end end
it 'tracks for the force_company_trial experiment', :experiment do
wrapped_experiment(experiment(:force_company_trial)) do |e|
expect(e.context.value).to include(user: user)
expect(e).to receive(:track).with(:create_group, namespace: an_instance_of(Group), user: user)
expect(e).to receive(:track).with(:create_trial, namespace: an_instance_of(Group), user: user, label: 'registrations_groups_controller')
end
post_create
end
it 'tracks for the combined_registration experiment', :experiment do it 'tracks for the combined_registration experiment', :experiment do
expect(experiment(:combined_registration)).to track(:create_group, namespace: an_instance_of(Group)).on_next_instance expect(experiment(:combined_registration)).to track(:create_group, namespace: an_instance_of(Group)).on_next_instance
subject subject
...@@ -236,16 +226,6 @@ RSpec.describe Registrations::GroupsController do ...@@ -236,16 +226,6 @@ RSpec.describe Registrations::GroupsController do
post_create post_create
end end
it 'selectively tracks for the force_company_trial experiment', :experiment do
wrapped_experiment(experiment(:force_company_trial)) do |e|
expect(e.context.value).to include(user: user)
expect(e).to receive(:track).with(:create_group, namespace: an_instance_of(Group), user: user)
expect(e).not_to receive(:track).with(:create_trial, namespace: an_instance_of(Group), user: user)
end
post_create
end
end end
end end
end end
......
...@@ -54,28 +54,5 @@ RSpec.describe Registrations::ProjectsController do ...@@ -54,28 +54,5 @@ RSpec.describe Registrations::ProjectsController do
let(:combined_registration?) { false } let(:combined_registration?) { false }
it_behaves_like "Registrations::ProjectsController POST #create" it_behaves_like "Registrations::ProjectsController POST #create"
context 'force_company_trial_experiment' do
let(:project) { create(:project, namespace: namespace) }
let(:params) { { namespace_id: namespace.id, name: 'New project', path: 'project-path', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
before do
namespace.add_owner(user)
sign_in(user)
allow(::Gitlab).to receive(:dev_env_or_com?).and_return(true)
allow_next_instance_of(::Projects::CreateService) do |service|
allow(service).to receive(:execute).and_return(project)
end
end
it 'tracks an event for the force_company_trial experiment', :experiment do
expect(experiment(:force_company_trial)).to track(:create_project, namespace: namespace, project: an_instance_of(Project), user: user)
.with_context(user: user)
.on_next_instance
post :create, params: { project: params }
end
end
end end
end end
...@@ -287,25 +287,9 @@ RSpec.describe Registrations::WelcomeController do ...@@ -287,25 +287,9 @@ RSpec.describe Registrations::WelcomeController do
end end
it { is_expected.to redirect_to new_users_sign_up_groups_project_path } it { is_expected.to redirect_to new_users_sign_up_groups_project_path }
it "doesn't call the force_company_trial experiment" do
expect(controller).not_to receive(:experiment).with(:force_company_trial, user: user)
subject
end
end end
end end
context 'and force_company_trial experiment is candidate' do
let(:setup_for_company) { 'true' }
before do
stub_experiments(combined_registration: :control, force_company_trial: :candidate)
end
it { is_expected.to redirect_to new_users_sign_up_group_path(trial: true) }
end
it { is_expected.to redirect_to new_users_sign_up_group_path } it { is_expected.to redirect_to new_users_sign_up_group_path }
context 'when in subscription flow' do context 'when in subscription flow' do
......
...@@ -420,12 +420,6 @@ RSpec.describe SubscriptionsController do ...@@ -420,12 +420,6 @@ RSpec.describe SubscriptionsController do
expect(response.body).to eq({ location: "/#{selected_group.path}?plan_id=#{plan_id}&purchased_quantity=#{quantity}" }.to_json) expect(response.body).to eq({ location: "/#{selected_group.path}?plan_id=#{plan_id}&purchased_quantity=#{quantity}" }.to_json)
end end
it 'tracks for the force_company_trial experiment', :experiment do
expect(experiment(:force_company_trial)).to track(:create_subscription, namespace: selected_group, user: user).with_context(user: user).on_next_instance
subject
end
context 'when having an explicit redirect' do context 'when having an explicit redirect' do
let_it_be(:redirect_after_success) { '/-/path/to/redirect' } let_it_be(:redirect_after_success) { '/-/path/to/redirect' }
......
...@@ -382,7 +382,6 @@ RSpec.describe TrialsController, :saas do ...@@ -382,7 +382,6 @@ RSpec.describe TrialsController, :saas do
it 'calls the record conversion method for the experiments' do it 'calls the record conversion method for the experiments' do
expect(controller).to receive(:record_experiment_user).with(:remove_known_trial_form_fields_welcoming, namespace_id: namespace.id) expect(controller).to receive(:record_experiment_user).with(:remove_known_trial_form_fields_welcoming, namespace_id: namespace.id)
expect(controller).to receive(:record_experiment_conversion_event).with(:remove_known_trial_form_fields_welcoming) expect(controller).to receive(:record_experiment_conversion_event).with(:remove_known_trial_form_fields_welcoming)
expect(experiment(:force_company_trial)).to track(:create_trial, namespace: namespace, user: user, label: 'trials_controller').with_context(user: user).on_next_instance
post_apply post_apply
end end
...@@ -447,18 +446,6 @@ RSpec.describe TrialsController, :saas do ...@@ -447,18 +446,6 @@ RSpec.describe TrialsController, :saas do
expect { post_apply }.to change { Group.count }.by(1) expect { post_apply }.to change { Group.count }.by(1)
end end
end end
context 'with an old namespace' do
it 'does not track for the force_company_trial experiment' do
allow(controller).to receive(:experiment).and_call_original
namespace.update!(created_at: 2.days.ago)
expect(controller).not_to receive(:experiment).with(:force_company_trial, user: user)
post_apply
end
end
end end
context 'on failure' do context 'on failure' do
......
...@@ -17,16 +17,16 @@ RSpec.describe CombinedRegistrationExperiment, :experiment do ...@@ -17,16 +17,16 @@ RSpec.describe CombinedRegistrationExperiment, :experiment do
end end
describe '#redirect_path' do describe '#redirect_path' do
it 'when control passes trial_params to path' do it 'control returns path' do
stub_experiments(combined_registration: :control) stub_experiments(combined_registration: :control)
expect(subject.redirect_path(trial: true)).to eq(Rails.application.routes.url_helpers.new_users_sign_up_group_path(trial: true)) expect(subject.redirect_path).to eq(Rails.application.routes.url_helpers.new_users_sign_up_group_path)
end end
it 'when candidate returns path' do it 'candidate returns path' do
stub_experiments(combined_registration: :candidate) stub_experiments(combined_registration: :candidate)
expect(subject.redirect_path(trial: true)).to eq(Rails.application.routes.url_helpers.new_users_sign_up_groups_project_path) expect(subject.redirect_path).to eq(Rails.application.routes.url_helpers.new_users_sign_up_groups_project_path)
end end
end end
end end
...@@ -7,10 +7,7 @@ RSpec.describe 'User sees new onboarding flow', :js do ...@@ -7,10 +7,7 @@ RSpec.describe 'User sees new onboarding flow', :js do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:trial_fields) { ['Company name', 'Number of employees', 'How many employees will use Gitlab?', 'Telephone number', 'Country'] } let_it_be(:trial_fields) { ['Company name', 'Number of employees', 'How many employees will use Gitlab?', 'Telephone number', 'Country'] }
let(:experiments) { {} }
before do before do
stub_experiments(experiments)
allow(Gitlab).to receive(:com?).and_return(true) allow(Gitlab).to receive(:com?).and_return(true)
sign_in(user) sign_in(user)
visit users_sign_up_welcome_path visit users_sign_up_welcome_path
...@@ -23,14 +20,6 @@ RSpec.describe 'User sees new onboarding flow', :js do ...@@ -23,14 +20,6 @@ RSpec.describe 'User sees new onboarding flow', :js do
expect(page).to have_content('GitLab Ultimate trial (optional)') expect(page).to have_content('GitLab Ultimate trial (optional)')
end end
context 'when force_company_trial experiment is candidate' do
let(:experiments) { { force_company_trial: :candidate } }
it 'shows the trial fields' do
trial_fields.each { |field| expect(page).to have_content(field) }
end
end
it 'shows the expected behavior with no trial chosen', :aggregate_failures do it 'shows the expected behavior with no trial chosen', :aggregate_failures do
fill_in 'group_name', with: 'test' fill_in 'group_name', with: 'test'
......
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