Commit fd2e3b0f authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch...

Merge branch '300297-experiment-cleanup-experiment-requiring-group-trials-group_only_trials' into 'master'

Cleanup the experiment requiring group trials [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54920
parents 2f63ed28 35a1e635
...@@ -8,7 +8,6 @@ class TrialsController < ApplicationController ...@@ -8,7 +8,6 @@ class TrialsController < ApplicationController
before_action :check_if_gl_com_or_dev before_action :check_if_gl_com_or_dev
before_action :authenticate_user! before_action :authenticate_user!
before_action :find_or_create_namespace, only: :apply before_action :find_or_create_namespace, only: :apply
before_action :record_user_for_group_only_trials_experiment, only: :select
feature_category :purchase feature_category :purchase
...@@ -121,10 +120,6 @@ class TrialsController < ApplicationController ...@@ -121,10 +120,6 @@ class TrialsController < ApplicationController
group group
end end
def record_user_for_group_only_trials_experiment
record_experiment_user(:group_only_trials)
end
def remove_known_trial_form_fields_context def remove_known_trial_form_fields_context
{ {
first_name_present: current_user.first_name.present?, first_name_present: current_user.first_name.present?,
......
...@@ -24,11 +24,7 @@ module EE ...@@ -24,11 +24,7 @@ module EE
end end
def trial_selection_intro_text def trial_selection_intro_text
if any_trial_user_namespaces? && any_trial_group_namespaces? if any_trial_group_namespaces?
s_('Trials|You can apply your trial to a new group, an existing group, or your personal account.')
elsif any_trial_user_namespaces?
s_('Trials|You can apply your trial to a new group or your personal account.')
elsif any_trial_group_namespaces?
s_('Trials|You can apply your trial to a new group or an existing group.') s_('Trials|You can apply your trial to a new group or an existing group.')
else else
s_('Trials|Create a new group to start your GitLab Ultimate trial.') s_('Trials|Create a new group to start your GitLab Ultimate trial.')
...@@ -36,14 +32,13 @@ module EE ...@@ -36,14 +32,13 @@ module EE
end end
def show_trial_namespace_select? def show_trial_namespace_select?
any_trial_group_namespaces? || any_trial_user_namespaces? any_trial_group_namespaces?
end end
def namespace_options_for_select(selected = nil) def namespace_options_for_select(selected = nil)
grouped_options = { grouped_options = {
'New' => [[_('Create group'), 0]], 'New' => [[_('Create group'), 0]],
'Groups' => trial_group_namespaces.map { |n| [n.name, n.id] }, 'Groups' => trial_group_namespaces.map { |n| [n.name, n.id] }
'Users' => trial_user_namespaces.map { |n| [n.name, n.id] }
} }
grouped_options_for_select(grouped_options, selected, prompt: _('Please select')) grouped_options_for_select(grouped_options, selected, prompt: _('Please select'))
...@@ -65,21 +60,8 @@ module EE ...@@ -65,21 +60,8 @@ module EE
end end
end end
def trial_user_namespaces
return [] if experiment_enabled?(:group_only_trials)
strong_memoize(:trial_user_namespaces) do
user_namespace = current_user.namespace
user_namespace.eligible_for_trial? ? [user_namespace] : []
end
end
def any_trial_group_namespaces? def any_trial_group_namespaces?
trial_group_namespaces.any? trial_group_namespaces.any?
end end
def any_trial_user_namespaces?
trial_user_namespaces.any?
end
end end
end end
---
title: Allow only group trials
merge_request: 54920
author:
type: added
---
name: group_only_trials_experiment_percentage
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40564
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/258629
milestone: '13.5'
type: experiment
group: group::conversion
default_enabled: false
...@@ -162,52 +162,23 @@ RSpec.describe TrialsController do ...@@ -162,52 +162,23 @@ RSpec.describe TrialsController do
end end
describe '#select' do describe '#select' do
def get_select
get :select
end
subject do subject do
get_select get :select
response response
end end
it_behaves_like 'an authenticated endpoint' it_behaves_like 'an authenticated endpoint'
it_behaves_like 'a dot-com only feature' it_behaves_like 'a dot-com only feature'
context 'when the group-only trials experiment is active' do
before do
stub_experiment(group_only_trials: true)
stub_experiment_for_subject(group_only_trials: user_is_in_experiment?)
end
def expected_group_type
user_is_in_experiment? ? 'experimental' : 'control'
end
where(user_is_in_experiment?: [true, false])
with_them do
it 'records the user as being part of the experiment' do
expect { get_select }.to change { ExperimentUser.count }.by(1)
expect(ExperimentUser.last.group_type).to eq(expected_group_type)
end
end
end
context 'when the group-only trials experiment is not active' do
it 'does not record the user as being part of the experiment' do
expect { get_select }.not_to change { ExperimentUser.count }
end
end
end end
describe '#apply' do describe '#apply' do
let_it_be(:namespace) { create(:namespace, owner_id: user.id, path: 'namespace-test') } let_it_be(:namespace) { create(:group, path: 'namespace-test') }
let(:apply_trial_result) { nil } let(:apply_trial_result) { nil }
let(:post_params) { { namespace_id: namespace.id } } let(:post_params) { { namespace_id: namespace.id } }
before do before do
namespace.add_owner(user)
allow_any_instance_of(GitlabSubscriptions::ApplyTrialService).to receive(:execute) do allow_any_instance_of(GitlabSubscriptions::ApplyTrialService).to receive(:execute) do
{ success: apply_trial_result } { success: apply_trial_result }
end end
...@@ -240,7 +211,7 @@ RSpec.describe TrialsController do ...@@ -240,7 +211,7 @@ RSpec.describe TrialsController do
let(:post_params) { { new_group_name: 'GitLab' } } let(:post_params) { { new_group_name: 'GitLab' } }
it 'creates the Group' do it 'creates the Group' do
expect { subject }.to change { Group.count }.to(1) expect { subject }.to change { Group.count }.by(1)
end end
end end
end end
...@@ -262,7 +233,7 @@ RSpec.describe TrialsController do ...@@ -262,7 +233,7 @@ RSpec.describe TrialsController do
it { is_expected.to render_template(:select) } it { is_expected.to render_template(:select) }
it 'does not create the Group' do it 'does not create the Group' do
expect { subject }.not_to change { Group.count }.from(0) expect { subject }.not_to change { Group.count }
end end
end end
end end
......
...@@ -5,10 +5,12 @@ require 'spec_helper' ...@@ -5,10 +5,12 @@ require 'spec_helper'
RSpec.describe 'Trial Select Namespace', :js do RSpec.describe 'Trial Select Namespace', :js do
include Select2Helper include Select2Helper
let(:new_group_name) { 'GitLab' } let_it_be(:group) { create(:group, path: 'group-test') }
let(:user) { create(:user) } let_it_be(:new_group_name) { 'GitLab' }
let_it_be(:user) { create(:user) }
before do before do
group.add_owner(user)
allow(Gitlab).to receive(:com?).and_return(true).at_least(:once) allow(Gitlab).to receive(:com?).and_return(true).at_least(:once)
sign_in(user) sign_in(user)
end end
...@@ -117,7 +119,7 @@ RSpec.describe 'Trial Select Namespace', :js do ...@@ -117,7 +119,7 @@ RSpec.describe 'Trial Select Namespace', :js do
context 'selects an existing group' do context 'selects an existing group' do
before do before do
select2 user.namespace.id, from: '#namespace_id' select2 group.id, from: '#namespace_id'
end end
context 'without trial plan' do context 'without trial plan' do
...@@ -135,7 +137,7 @@ RSpec.describe 'Trial Select Namespace', :js do ...@@ -135,7 +137,7 @@ RSpec.describe 'Trial Select Namespace', :js do
wait_for_requests wait_for_requests
expect(current_path).to eq("/#{user.namespace.path}") expect(current_path).to eq("/#{group.path}")
end end
end end
...@@ -151,7 +153,7 @@ RSpec.describe 'Trial Select Namespace', :js do ...@@ -151,7 +153,7 @@ RSpec.describe 'Trial Select Namespace', :js do
expect(find('.flash-text')).to have_text(error_message) expect(find('.flash-text')).to have_text(error_message)
expect(current_path).to eq(apply_trials_path) expect(current_path).to eq(apply_trials_path)
expect(find('#namespace_id', visible: false).value).to eq(user.namespace.id.to_s) expect(find('#namespace_id', visible: false).value).to eq(group.id.to_s)
# new group name should be functional # new group name should be functional
select2 '0', from: '#namespace_id' select2 '0', from: '#namespace_id'
......
...@@ -62,29 +62,24 @@ RSpec.describe EE::TrialHelper do ...@@ -62,29 +62,24 @@ RSpec.describe EE::TrialHelper do
end end
describe '#namespace_options_for_select' do describe '#namespace_options_for_select' do
let_it_be(:user) { create :user }
let_it_be(:group1) { create :group } let_it_be(:group1) { create :group }
let_it_be(:group2) { create :group } let_it_be(:group2) { create :group }
let(:trial_user_namespaces) { [] }
let(:trial_group_namespaces) { [] } let(:trial_group_namespaces) { [] }
let(:new_optgroup_regex) { /<optgroup label="New"><option/ } let(:new_optgroup_regex) { /<optgroup label="New"><option/ }
let(:groups_optgroup_regex) { /<optgroup label="Groups"><option/ } let(:groups_optgroup_regex) { /<optgroup label="Groups"><option/ }
let(:users_optgroup_regex) { /<optgroup label="Users"><option/ }
before do before do
allow(helper).to receive(:trial_group_namespaces).and_return(trial_group_namespaces) allow(helper).to receive(:trial_group_namespaces).and_return(trial_group_namespaces)
allow(helper).to receive(:trial_user_namespaces).and_return(trial_user_namespaces)
end end
subject { helper.namespace_options_for_select } subject { helper.namespace_options_for_select }
context 'when there are no eligible group or user namespaces' do context 'when there is no eligible group' do
it 'returns just the "New" option group', :aggregate_failures do it 'returns just the "New" option group', :aggregate_failures do
is_expected.to match(new_optgroup_regex) is_expected.to match(new_optgroup_regex)
is_expected.not_to match(groups_optgroup_regex) is_expected.not_to match(groups_optgroup_regex)
is_expected.not_to match(users_optgroup_regex)
end end
end end
...@@ -94,45 +89,29 @@ RSpec.describe EE::TrialHelper do ...@@ -94,45 +89,29 @@ RSpec.describe EE::TrialHelper do
it 'returns the "New" and "Groups" option groups', :aggregate_failures do it 'returns the "New" and "Groups" option groups', :aggregate_failures do
is_expected.to match(new_optgroup_regex) is_expected.to match(new_optgroup_regex)
is_expected.to match(groups_optgroup_regex) is_expected.to match(groups_optgroup_regex)
is_expected.not_to match(users_optgroup_regex)
end end
end end
context 'when only the user namespace is eligible' do context 'when some group namespaces are eligible' do
let(:trial_user_namespaces) { [user.namespace] }
it 'returns the "New" and "Users" option groups', :aggregate_failures do
is_expected.to match(new_optgroup_regex)
is_expected.to match(users_optgroup_regex)
is_expected.not_to match(groups_optgroup_regex)
end
end
context 'when some group namespaces & the user namespace are eligible' do
let(:trial_group_namespaces) { [group1, group2] } let(:trial_group_namespaces) { [group1, group2] }
let(:trial_user_namespaces) { [user.namespace] }
it 'returns the "New", "Groups", and "Users" option groups', :aggregate_failures do it 'returns the "New", "Groups" option groups', :aggregate_failures do
is_expected.to match(new_optgroup_regex) is_expected.to match(new_optgroup_regex)
is_expected.to match(groups_optgroup_regex) is_expected.to match(groups_optgroup_regex)
is_expected.to match(users_optgroup_regex)
end end
end end
end end
describe '#trial_selection_intro_text' do describe '#trial_selection_intro_text' do
before do before do
allow(helper).to receive(:any_trial_user_namespaces?).and_return(have_user_namespace)
allow(helper).to receive(:any_trial_group_namespaces?).and_return(have_group_namespace) allow(helper).to receive(:any_trial_group_namespaces?).and_return(have_group_namespace)
end end
subject { helper.trial_selection_intro_text } subject { helper.trial_selection_intro_text }
where(:have_user_namespace, :have_group_namespace, :text) do where(:have_group_namespace, :text) do
true | true | 'You can apply your trial to a new group, an existing group, or your personal account.' true | 'You can apply your trial to a new group or an existing group.'
true | false | 'You can apply your trial to a new group or your personal account.' false | 'Create a new group to start your GitLab Ultimate trial.'
false | true | 'You can apply your trial to a new group or an existing group.'
false | false | 'Create a new group to start your GitLab Ultimate trial.'
end end
with_them do with_them do
...@@ -141,22 +120,19 @@ RSpec.describe EE::TrialHelper do ...@@ -141,22 +120,19 @@ RSpec.describe EE::TrialHelper do
end end
describe '#show_trial_namespace_select?' do describe '#show_trial_namespace_select?' do
let_it_be(:have_group_namespace) { false }
before do before do
allow(helper).to receive(:any_trial_group_namespaces?).and_return(have_group_namespace) allow(helper).to receive(:any_trial_group_namespaces?).and_return(have_group_namespace)
allow(helper).to receive(:any_trial_user_namespaces?).and_return(have_user_namespace)
end end
subject { helper.show_trial_namespace_select? } subject { helper.show_trial_namespace_select? }
where(:have_user_namespace, :have_group_namespace, :result) do it { is_expected.to eq(false) }
true | true | true
true | false | true
false | true | true
false | false | false
end
with_them do context 'with some trial group namespaces' do
it { is_expected.to eq(result) } let_it_be(:have_group_namespace) { true }
it { is_expected.to eq(true) }
end end
end end
......
...@@ -54,10 +54,6 @@ module Gitlab ...@@ -54,10 +54,6 @@ module Gitlab
tracking_category: 'Growth::Conversion::Experiment::ContactSalesInApp', tracking_category: 'Growth::Conversion::Experiment::ContactSalesInApp',
use_backwards_compatible_subject_index: true use_backwards_compatible_subject_index: true
}, },
group_only_trials: {
tracking_category: 'Growth::Conversion::Experiment::GroupOnlyTrials',
use_backwards_compatible_subject_index: true
},
remove_known_trial_form_fields: { remove_known_trial_form_fields: {
tracking_category: 'Growth::Conversion::Experiment::RemoveKnownTrialFormFields' tracking_category: 'Growth::Conversion::Experiment::RemoveKnownTrialFormFields'
}, },
......
...@@ -31570,12 +31570,6 @@ msgstr "" ...@@ -31570,12 +31570,6 @@ msgstr ""
msgid "Trials|You can apply your trial to a new group or an existing group." msgid "Trials|You can apply your trial to a new group or an existing group."
msgstr "" msgstr ""
msgid "Trials|You can apply your trial to a new group or your personal account."
msgstr ""
msgid "Trials|You can apply your trial to a new group, an existing group, or your personal account."
msgstr ""
msgid "Trials|You won't get a free trial right now but you can always resume this process by clicking on your avatar and choosing 'Start a free trial'" msgid "Trials|You won't get a free trial right now but you can always resume this process by clicking on your avatar and choosing 'Start a free trial'"
msgstr "" msgstr ""
......
...@@ -11,8 +11,7 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do ...@@ -11,8 +11,7 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do
:invite_members_version_a, :invite_members_version_a,
:invite_members_version_b, :invite_members_version_b,
:invite_members_empty_group_version_a, :invite_members_empty_group_version_a,
:contact_sales_btn_in_app, :contact_sales_btn_in_app
:group_only_trials
] ]
backwards_compatible_experiment_keys = described_class.filter { |_, v| v[:use_backwards_compatible_subject_index] }.keys backwards_compatible_experiment_keys = described_class.filter { |_, v| v[:use_backwards_compatible_subject_index] }.keys
......
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