Commit b4e215ae authored by Alex Buijs's avatar Alex Buijs Committed by Mayra Cabrera

Add experiments and experiment_users tables

For recording which users are subject to which
experiment on GitLab.com
parent 7561e2e7
......@@ -62,7 +62,11 @@ class RegistrationsController < Devise::RegistrationsController
result = ::Users::SignupService.new(current_user, user_params).execute
if result[:status] == :success
track_experiment_event(:onboarding_issues, 'signed_up') if ::Gitlab.com? && show_onboarding_issues_experiment?
if ::Gitlab.com? && show_onboarding_issues_experiment?
track_experiment_event(:onboarding_issues, 'signed_up')
record_experiment_user(:onboarding_issues)
end
return redirect_to new_users_sign_up_group_path if experiment_enabled?(:onboarding_issues) && show_onboarding_issues_experiment?
set_flash_message! :notice, :signed_up
......
# frozen_string_literal: true
class Experiment < ApplicationRecord
has_many :experiment_users
has_many :users, through: :experiment_users
has_many :control_group_users, -> { merge(ExperimentUser.control) }, through: :experiment_users, source: :user
has_many :experimental_group_users, -> { merge(ExperimentUser.experimental) }, through: :experiment_users, source: :user
validates :name, presence: true, uniqueness: true, length: { maximum: 255 }
def self.add_user(name, group_type, user)
experiment = find_or_create_by(name: name)
return unless experiment
return if experiment.experiment_users.where(user: user).exists?
group_type == ::Gitlab::Experimentation::GROUP_CONTROL ? experiment.add_control_user(user) : experiment.add_experimental_user(user)
end
def add_control_user(user)
control_group_users << user
end
def add_experimental_user(user)
experimental_group_users << user
end
end
# frozen_string_literal: true
class ExperimentUser < ApplicationRecord
belongs_to :experiment
belongs_to :user
enum group_type: { control: 0, experimental: 1 }
validates :group_type, presence: true
end
---
title: Add experiments and experiment_users tables for tracking which users are enrolled for which experiments.
merge_request: 38397
author:
type: added
# frozen_string_literal: true
class CreateExperiment < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
unless table_exists?(:experiments)
create_table :experiments do |t|
t.text :name, null: false
t.index :name, unique: true
end
end
add_text_limit :experiments, :name, 255
end
def down
drop_table :experiments
end
end
# frozen_string_literal: true
class CreateExperimentUser < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :experiment_users do |t|
t.bigint :experiment_id, null: false
t.bigint :user_id, null: false
t.integer :group_type, limit: 2, null: false, default: 0
t.timestamps_with_timezone null: false
end
add_index :experiment_users, :experiment_id
add_index :experiment_users, :user_id
end
def down
drop_table :experiment_users
end
end
# frozen_string_literal: true
class AddForeignKeyToExperimentOnExperimentUsers < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
# There is no need to use add_concurrent_foreign_key since it's an empty table
add_foreign_key :experiment_users, :experiments, column: :experiment_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
end
end
def down
with_lock_retries do
remove_foreign_key :experiment_users, column: :experiment_id
end
end
end
# frozen_string_literal: true
class AddForeignKeyToUserOnExperimentUsers < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
# There is no need to use add_concurrent_foreign_key since it's an empty table
add_foreign_key :experiment_users, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
end
end
def down
with_lock_retries do
remove_foreign_key :experiment_users, column: :user_id
end
end
end
aeeef4762f7ab7c7eefc28995ba198825455a43db38c1ff2aefad8c3c76b5fba
\ No newline at end of file
c5775e8150285927b74c2fbf6098d03920e7bea52c3b94ad372fec288835110c
\ No newline at end of file
02f3561a5b8fa79c58dccc23ebaecf2c1b8371c86bb360156456bd11e84d13a2
\ No newline at end of file
26aa28efa15ebd223feb879d5cb684a53a92bc71e483e8348d0cd9f1ad10e7ae
\ No newline at end of file
......@@ -11557,6 +11557,39 @@ CREATE SEQUENCE public.evidences_id_seq
ALTER SEQUENCE public.evidences_id_seq OWNED BY public.evidences.id;
CREATE TABLE public.experiment_users (
id bigint NOT NULL,
experiment_id bigint NOT NULL,
user_id bigint NOT NULL,
group_type smallint DEFAULT 0 NOT NULL,
created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL
);
CREATE SEQUENCE public.experiment_users_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE public.experiment_users_id_seq OWNED BY public.experiment_users.id;
CREATE TABLE public.experiments (
id bigint NOT NULL,
name text NOT NULL,
CONSTRAINT check_e2dda25ed0 CHECK ((char_length(name) <= 255))
);
CREATE SEQUENCE public.experiments_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE public.experiments_id_seq OWNED BY public.experiments.id;
CREATE TABLE public.external_pull_requests (
id bigint NOT NULL,
created_at timestamp with time zone NOT NULL,
......@@ -16874,6 +16907,10 @@ ALTER TABLE ONLY public.events ALTER COLUMN id SET DEFAULT nextval('public.event
ALTER TABLE ONLY public.evidences ALTER COLUMN id SET DEFAULT nextval('public.evidences_id_seq'::regclass);
ALTER TABLE ONLY public.experiment_users ALTER COLUMN id SET DEFAULT nextval('public.experiment_users_id_seq'::regclass);
ALTER TABLE ONLY public.experiments ALTER COLUMN id SET DEFAULT nextval('public.experiments_id_seq'::regclass);
ALTER TABLE ONLY public.external_pull_requests ALTER COLUMN id SET DEFAULT nextval('public.external_pull_requests_id_seq'::regclass);
ALTER TABLE ONLY public.feature_gates ALTER COLUMN id SET DEFAULT nextval('public.feature_gates_id_seq'::regclass);
......@@ -17892,6 +17929,12 @@ ALTER TABLE ONLY public.events
ALTER TABLE ONLY public.evidences
ADD CONSTRAINT evidences_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.experiment_users
ADD CONSTRAINT experiment_users_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.experiments
ADD CONSTRAINT experiments_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.external_pull_requests
ADD CONSTRAINT external_pull_requests_pkey PRIMARY KEY (id);
......@@ -19534,6 +19577,12 @@ CREATE UNIQUE INDEX index_events_on_target_type_and_target_id_and_fingerprint ON
CREATE INDEX index_evidences_on_release_id ON public.evidences USING btree (release_id);
CREATE INDEX index_experiment_users_on_experiment_id ON public.experiment_users USING btree (experiment_id);
CREATE INDEX index_experiment_users_on_user_id ON public.experiment_users USING btree (user_id);
CREATE UNIQUE INDEX index_experiments_on_name ON public.experiments USING btree (name);
CREATE INDEX index_expired_and_not_notified_personal_access_tokens ON public.personal_access_tokens USING btree (id, expires_at) WHERE ((impersonation = false) AND (revoked = false) AND (expire_notification_delivered = false));
CREATE UNIQUE INDEX index_external_pull_requests_on_project_and_branches ON public.external_pull_requests USING btree (project_id, source_branch, target_branch);
......@@ -22249,6 +22298,9 @@ ALTER TABLE ONLY public.terraform_states
ALTER TABLE ONLY public.group_deploy_keys
ADD CONSTRAINT fk_rails_5682fc07f8 FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE RESTRICT;
ALTER TABLE ONLY public.experiment_users
ADD CONSTRAINT fk_rails_56d4708b4a FOREIGN KEY (experiment_id) REFERENCES public.experiments(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.issue_user_mentions
ADD CONSTRAINT fk_rails_57581fda73 FOREIGN KEY (issue_id) REFERENCES public.issues(id) ON DELETE CASCADE;
......@@ -23011,6 +23063,9 @@ ALTER TABLE ONLY public.ci_job_variables
ALTER TABLE ONLY public.packages_nuget_metadata
ADD CONSTRAINT fk_rails_fc0c19f5b4 FOREIGN KEY (package_id) REFERENCES public.packages_packages(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.experiment_users
ADD CONSTRAINT fk_rails_fd805f771a FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.cluster_groups
ADD CONSTRAINT fk_rails_fdb8648a96 FOREIGN KEY (cluster_id) REFERENCES public.clusters(id) ON DELETE CASCADE;
......
......@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe RegistrationsController do
let_it_be(:user) { create(:user) }
describe '#create' do
context 'when the user opted-in' do
let(:user_params) { { user: attributes_for(:user, email_opted_in: '1') } }
......@@ -46,7 +48,7 @@ RSpec.describe RegistrationsController do
subject { get :welcome }
before do
sign_in(create(:user))
sign_in(user)
end
it 'renders the checkout layout' do
......@@ -56,7 +58,7 @@ RSpec.describe RegistrationsController do
describe '#update_registration' do
before do
sign_in(create(:user))
sign_in(user)
end
subject(:update_registration) { patch :update_registration, params: { user: { role: 'software_developer', setup_for_company: 'false' } } }
......@@ -87,10 +89,10 @@ RSpec.describe RegistrationsController do
end
end
describe 'tracking for the onboarding issues experiment' do
describe 'recording experiment user and track for the onboarding issues experiment' do
using RSpec::Parameterized::TableSyntax
where(:on_gitlab_com, :experiment_enabled, :in_subscription_flow, :in_invitation_flow, :experiment_enabled_for_user, :expected_tracking) do
where(:on_gitlab_com, :experiment_enabled, :in_subscription_flow, :in_invitation_flow, :experiment_enabled_for_user, :experiment_group) do
false | false | false | false | true | nil
false | false | false | true | true | nil
false | false | true | false | true | nil
......@@ -103,8 +105,8 @@ RSpec.describe RegistrationsController do
true | false | false | true | true | nil
true | false | true | false | true | nil
true | false | true | true | true | nil
true | true | false | false | true | 'experimental_group'
true | true | false | false | false | 'control_group'
true | true | false | false | true | :experimental
true | true | false | false | false | :control
true | true | false | true | true | nil
true | true | true | false | true | nil
true | true | true | true | true | nil
......@@ -119,13 +121,23 @@ RSpec.describe RegistrationsController do
stub_experiment_for_user(onboarding_issues: experiment_enabled_for_user)
end
it 'tracks when appropriate' do
if expected_tracking
it 'adds a user to experiments when appropriate' do
if experiment_group
expect(::Experiment).to receive(:add_user).with(:onboarding_issues, experiment_group, user)
else
expect(::Experiment).not_to receive(:add_user)
end
update_registration
end
it 'tracks a signed_up event when appropriate' do
if experiment_group
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Conversion::Experiment::OnboardingIssues',
'signed_up',
label: anything,
property: expected_tracking
property: "#{experiment_group}_group"
)
else
expect(Gitlab::Tracking).not_to receive(:event)
......
......@@ -62,6 +62,9 @@ module Gitlab
}
}.freeze
GROUP_CONTROL = :control
GROUP_EXPERIMENTAL = :experimental
# Controller concern that checks if an `experimentation_subject_id cookie` is present and sets it if absent.
# Used for A/B testing of experimental features. Exposes the `experiment_enabled?(experiment_name)` method
# to controllers and views. It returns true when the experiment is enabled and the user is selected as part
......@@ -106,6 +109,12 @@ module Gitlab
end
end
def record_experiment_user(experiment_key)
return unless Experimentation.enabled?(experiment_key) && current_user
::Experiment.add_user(experiment_key, tracking_group(experiment_key), current_user)
end
private
def dnt_enabled?
......@@ -132,7 +141,7 @@ module Gitlab
{
category: tracking_category(experiment_key),
action: action,
property: tracking_group(experiment_key),
property: "#{tracking_group(experiment_key)}_group",
label: experimentation_subject_id,
value: value
}.compact
......@@ -145,7 +154,7 @@ module Gitlab
def tracking_group(experiment_key)
return unless Experimentation.enabled?(experiment_key)
experiment_enabled?(experiment_key) ? 'experimental_group' : 'control_group'
experiment_enabled?(experiment_key) ? GROUP_EXPERIMENTAL : GROUP_CONTROL
end
def forced_enabled?(experiment_key)
......
# frozen_string_literal: true
FactoryBot.define do
factory :experiment do
name { generate(:title) }
end
end
......@@ -233,6 +233,68 @@ RSpec.describe Gitlab::Experimentation do
end
end
end
describe '#record_experiment_user' do
let(:user) { build(:user) }
context 'when the experiment is enabled' do
before do
stub_experiment(test_experiment: true)
allow(controller).to receive(:current_user).and_return(user)
end
context 'the user is part of the experimental group' do
before do
stub_experiment_for_user(test_experiment: true)
end
it 'calls add_user on the Experiment model' do
expect(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user)
controller.record_experiment_user(:test_experiment)
end
end
context 'the user is part of the control group' do
before do
allow_next_instance_of(described_class) do |instance|
allow(instance).to receive(:experiment_enabled?).with(:test_experiment).and_return(false)
end
end
it 'calls add_user on the Experiment model' do
expect(::Experiment).to receive(:add_user).with(:test_experiment, :control, user)
controller.record_experiment_user(:test_experiment)
end
end
end
context 'when the experiment is disabled' do
before do
stub_experiment(test_experiment: false)
allow(controller).to receive(:current_user).and_return(user)
end
it 'does not call add_user on the Experiment model' do
expect(::Experiment).not_to receive(:add_user)
controller.record_experiment_user(:test_experiment)
end
end
context 'when there is no current_user' do
before do
stub_experiment(test_experiment: true)
end
it 'does not call add_user on the Experiment model' do
expect(::Experiment).not_to receive(:add_user)
controller.record_experiment_user(:test_experiment)
end
end
end
end
describe '.enabled?' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Experiment do
subject { build(:experiment) }
describe 'associations' do
it { is_expected.to have_many(:experiment_users) }
it { is_expected.to have_many(:users) }
it { is_expected.to have_many(:control_group_users) }
it { is_expected.to have_many(:experimental_group_users) }
describe 'control_group_users and experimental_group_users' do
let(:experiment) { create(:experiment) }
let(:control_group_user) { build(:user) }
let(:experimental_group_user) { build(:user) }
before do
experiment.control_group_users << control_group_user
experiment.experimental_group_users << experimental_group_user
end
describe 'control_group_users' do
subject { experiment.control_group_users }
it { is_expected.to contain_exactly(control_group_user) }
end
describe 'experimental_group_users' do
subject { experiment.experimental_group_users }
it { is_expected.to contain_exactly(experimental_group_user) }
end
end
end
describe 'validations' do
it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_uniqueness_of(:name) }
it { is_expected.to validate_length_of(:name).is_at_most(255) }
end
describe '.add_user' do
let(:name) { :experiment_key }
let(:user) { build(:user) }
let!(:experiment) { create(:experiment, name: name) }
subject { described_class.add_user(name, :control, user) }
describe 'creating a new experiment record' do
context 'an experiment with the provided name already exists' do
it 'does not create a new experiment record' do
expect { subject }.not_to change(Experiment, :count)
end
end
context 'an experiment with the provided name does not exist yet' do
let(:experiment) { nil }
it 'creates a new experiment record' do
expect { subject }.to change(Experiment, :count).by(1)
end
end
end
describe 'creating a new experiment_user record' do
context 'an experiment_user record for this experiment already exists' do
before do
subject
end
it 'does not create a new experiment_user record' do
expect { subject }.not_to change(ExperimentUser, :count)
end
end
context 'an experiment_user record for this experiment does not exist yet' do
it 'creates a new experiment_user record' do
expect { subject }.to change(ExperimentUser, :count).by(1)
end
it 'assigns the correct group_type to the experiment_user' do
expect { subject }.to change { experiment.control_group_users.count }.by(1)
end
end
end
end
describe '#add_control_user' do
let(:experiment) { create(:experiment) }
let(:user) { build(:user) }
subject { experiment.add_control_user(user) }
it 'creates a new experiment_user record and assigns the correct group_type' do
expect { subject }.to change { experiment.control_group_users.count }.by(1)
end
end
describe '#add_experimental_user' do
let(:experiment) { create(:experiment) }
let(:user) { build(:user) }
subject { experiment.add_experimental_user(user) }
it 'creates a new experiment_user record and assigns the correct group_type' do
expect { subject }.to change { experiment.experimental_group_users.count }.by(1)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ExperimentUser do
describe 'Associations' do
it { is_expected.to belong_to(:experiment) }
it { is_expected.to belong_to(:user) }
end
describe 'Validations' do
it { is_expected.to validate_presence_of(:group_type) }
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