Commit 30037ed9 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'tracking-experimental-signup-flow' into 'master'

Tracking experimental signup flow

See merge request gitlab-org/gitlab!17521
parents 323972f3 fb4dbe27
...@@ -5,6 +5,7 @@ import NoEmojiValidator from '../../../emoji/no_emoji_validator'; ...@@ -5,6 +5,7 @@ import NoEmojiValidator from '../../../emoji/no_emoji_validator';
import SigninTabsMemoizer from './signin_tabs_memoizer'; import SigninTabsMemoizer from './signin_tabs_memoizer';
import OAuthRememberMe from './oauth_remember_me'; import OAuthRememberMe from './oauth_remember_me';
import preserveUrlFragment from './preserve_url_fragment'; import preserveUrlFragment from './preserve_url_fragment';
import Tracking from '~/tracking';
document.addEventListener('DOMContentLoaded', () => { document.addEventListener('DOMContentLoaded', () => {
new UsernameValidator(); // eslint-disable-line no-new new UsernameValidator(); // eslint-disable-line no-new
...@@ -19,4 +20,12 @@ document.addEventListener('DOMContentLoaded', () => { ...@@ -19,4 +20,12 @@ document.addEventListener('DOMContentLoaded', () => {
// Save the URL fragment from the current window location. This will be present if the user was // Save the URL fragment from the current window location. This will be present if the user was
// redirected to sign-in after attempting to access a protected URL that included a fragment. // redirected to sign-in after attempting to access a protected URL that included a fragment.
preserveUrlFragment(window.location.hash); preserveUrlFragment(window.location.hash);
if (gon.tracking_data) {
const tab = document.querySelector(".new-session-tabs a[href='#register-pane']");
const { category, action, ...data } = gon.tracking_data;
tab.addEventListener('click', () => {
Tracking.event(category, action, data);
});
}
}); });
...@@ -16,6 +16,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -16,6 +16,7 @@ class RegistrationsController < Devise::RegistrationsController
def new def new
if experiment_enabled?(:signup_flow) if experiment_enabled?(:signup_flow)
track_experiment_event(:signup_flow, 'start') # We want this event to be tracked when the user is _in_ the experimental group
@resource = build_resource @resource = build_resource
else else
redirect_to new_user_session_path(anchor: 'register-pane') redirect_to new_user_session_path(anchor: 'register-pane')
...@@ -23,6 +24,8 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -23,6 +24,8 @@ class RegistrationsController < Devise::RegistrationsController
end end
def create def create
track_experiment_event(:signup_flow, 'end') unless experiment_enabled?(:signup_flow) # We want this event to be tracked when the user is _in_ the control group
accept_pending_invitations accept_pending_invitations
super do |new_user| super do |new_user|
...@@ -61,6 +64,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -61,6 +64,7 @@ class RegistrationsController < Devise::RegistrationsController
result = ::Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute result = ::Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute
if result[:status] == :success if result[:status] == :success
track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group
set_flash_message! :notice, :signed_up set_flash_message! :notice, :signed_up
redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) redirect_to stored_location_or_dashboard_or_almost_there_path(current_user)
else else
......
...@@ -24,6 +24,7 @@ class SessionsController < Devise::SessionsController ...@@ -24,6 +24,7 @@ class SessionsController < Devise::SessionsController
before_action :store_unauthenticated_sessions, only: [:new] before_action :store_unauthenticated_sessions, only: [:new]
before_action :save_failed_login, if: :action_new_and_failed_login? before_action :save_failed_login, if: :action_new_and_failed_login?
before_action :load_recaptcha before_action :load_recaptcha
before_action :frontend_tracking_data, only: [:new]
after_action :log_failed_login, if: :action_new_and_failed_login? after_action :log_failed_login, if: :action_new_and_failed_login?
...@@ -293,6 +294,11 @@ class SessionsController < Devise::SessionsController ...@@ -293,6 +294,11 @@ class SessionsController < Devise::SessionsController
"standard" "standard"
end end
end end
def frontend_tracking_data
# We want tracking data pushed to the frontend when the user is _in_ the control group
frontend_experimentation_tracking_data(:signup_flow, 'start') unless experiment_enabled?(:signup_flow)
end
end end
SessionsController.prepend_if_ee('EE::SessionsController') SessionsController.prepend_if_ee('EE::SessionsController')
---
title: Track the starting and stopping of the current signup flow and the experimental signup flow
merge_request: 17521
author:
type: other
...@@ -14,13 +14,15 @@ module Gitlab ...@@ -14,13 +14,15 @@ module Gitlab
signup_flow: { signup_flow: {
feature_toggle: :experimental_separate_sign_up_flow, feature_toggle: :experimental_separate_sign_up_flow,
environment: ::Gitlab.dev_env_or_com?, environment: ::Gitlab.dev_env_or_com?,
enabled_ratio: 0.1 enabled_ratio: 0.1,
tracking_category: 'Growth::Acquisition::Experiment::SignUpFlow'
} }
}.freeze }.freeze
# Controller concern that checks if an experimentation_subject_id cookie is present and sets it if absent. # 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 # Used for A/B testing of experimental features. Exposes the `experiment_enabled?(experiment_name)` method
# to controllers and views. # to controllers and views. It returns true when the experiment is enabled and the user is selected as part
# of the experimental group.
# #
module ControllerConcern module ControllerConcern
extend ActiveSupport::Concern extend ActiveSupport::Concern
...@@ -41,17 +43,57 @@ module Gitlab ...@@ -41,17 +43,57 @@ module Gitlab
end end
def experiment_enabled?(experiment_key) def experiment_enabled?(experiment_key)
Experimentation.enabled?(experiment_key, experimentation_subject_index) Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index)
end
def track_experiment_event(experiment_key, action)
track_experiment_event_for(experiment_key, action) do |tracking_data|
::Gitlab::Tracking.event(tracking_data.delete(:category), tracking_data.delete(:action), tracking_data)
end
end
def frontend_experimentation_tracking_data(experiment_key, action)
track_experiment_event_for(experiment_key, action) do |tracking_data|
gon.push(tracking_data: tracking_data)
end
end end
private private
def experimentation_subject_id
cookies.signed[:experimentation_subject_id]
end
def experimentation_subject_index def experimentation_subject_index
experimentation_subject_id = cookies.signed[:experimentation_subject_id]
return if experimentation_subject_id.blank? return if experimentation_subject_id.blank?
experimentation_subject_id.delete('-').hex % 100 experimentation_subject_id.delete('-').hex % 100
end end
def track_experiment_event_for(experiment_key, action)
return unless Experimentation.enabled?(experiment_key)
yield experimentation_tracking_data(experiment_key, action)
end
def experimentation_tracking_data(experiment_key, action)
{
category: tracking_category(experiment_key),
action: action,
property: tracking_group(experiment_key),
label: experimentation_subject_id
}
end
def tracking_category(experiment_key)
Experimentation.experiment(experiment_key).tracking_category
end
def tracking_group(experiment_key)
return unless Experimentation.enabled?(experiment_key)
experiment_enabled?(experiment_key) ? 'experimental_group' : 'control_group'
end
end end
class << self class << self
...@@ -59,18 +101,20 @@ module Gitlab ...@@ -59,18 +101,20 @@ module Gitlab
Experiment.new(EXPERIMENTS[key].merge(key: key)) Experiment.new(EXPERIMENTS[key].merge(key: key))
end end
def enabled?(experiment_key, experimentation_subject_index) def enabled?(experiment_key)
return false unless EXPERIMENTS.key?(experiment_key) return false unless EXPERIMENTS.key?(experiment_key)
experiment = experiment(experiment_key) experiment = experiment(experiment_key)
experiment.feature_toggle_enabled? && experiment.enabled_for_environment?
end
experiment.feature_toggle_enabled? && def enabled_for_user?(experiment_key, experimentation_subject_index)
experiment.enabled_for_environment? && enabled?(experiment_key) &&
experiment.enabled_for_experimentation_subject?(experimentation_subject_index) experiment(experiment_key).enabled_for_experimentation_subject?(experimentation_subject_index)
end end
end end
Experiment = Struct.new(:key, :feature_toggle, :environment, :enabled_ratio, keyword_init: true) do Experiment = Struct.new(:key, :feature_toggle, :environment, :enabled_ratio, :tracking_category, keyword_init: true) do
def feature_toggle_enabled? def feature_toggle_enabled?
return Feature.enabled?(key, default_enabled: true) if feature_toggle.nil? return Feature.enabled?(key, default_enabled: true) if feature_toggle.nil?
......
...@@ -836,7 +836,7 @@ describe ApplicationController do ...@@ -836,7 +836,7 @@ describe ApplicationController do
let(:experiment_enabled) { true } let(:experiment_enabled) { true }
before do before do
stub_experiment(signup_flow: experiment_enabled) stub_experiment_for_user(signup_flow: experiment_enabled)
end end
context 'experiment enabled and user with required role' do context 'experiment enabled and user with required role' do
......
...@@ -9,6 +9,49 @@ describe RegistrationsController do ...@@ -9,6 +9,49 @@ describe RegistrationsController do
stub_feature_flags(invisible_captcha: false) stub_feature_flags(invisible_captcha: false)
end end
describe '#new' do
subject { get :new }
context 'with the experimental signup flow enabled and the user is part of the experimental group' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: true)
end
it 'tracks the event with the right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::SignUpFlow',
'start',
label: anything,
property: 'experimental_group'
)
subject
end
it 'renders new template and sets the resource variable' do
expect(subject).to render_template(:new)
expect(assigns(:resource)).to be_a(User)
end
end
context 'with the experimental signup flow enabled and the user is part of the control group' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: false)
end
it 'does not track the event' do
expect(Gitlab::Tracking).not_to receive(:event)
subject
end
it 'renders new template and sets the resource variable' do
subject
expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane'))
end
end
end
describe '#create' do describe '#create' do
let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } }
let(:user_params) { { user: base_user_params } } let(:user_params) { { user: base_user_params } }
...@@ -217,6 +260,37 @@ describe RegistrationsController do ...@@ -217,6 +260,37 @@ describe RegistrationsController do
end end
end end
describe 'tracking data' do
context 'with the experimental signup flow enabled and the user is part of the control group' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: false)
end
it 'tracks the event with the right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::SignUpFlow',
'end',
label: anything,
property: 'control_group'
)
post :create, params: user_params
end
end
context 'with the experimental signup flow enabled and the user is part of the experimental group' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: true)
end
it 'does not track the event' do
expect(Gitlab::Tracking).not_to receive(:event)
post :create, params: user_params
end
end
end
it "logs a 'User Created' message" do it "logs a 'User Created' message" do
stub_feature_flags(registrations_recaptcha: false) stub_feature_flags(registrations_recaptcha: false)
...@@ -304,4 +378,22 @@ describe RegistrationsController do ...@@ -304,4 +378,22 @@ describe RegistrationsController do
end end
end end
end end
describe '#update_role' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: true)
sign_in(create(:user))
end
it 'tracks the event with the right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::SignUpFlow',
'end',
label: anything,
property: 'experimental_group'
)
patch :update_role, params: { user: { name: 'New name', role: 'software_developer' } }
end
end
end end
...@@ -34,6 +34,39 @@ describe SessionsController do ...@@ -34,6 +34,39 @@ describe SessionsController do
end end
end end
end end
describe 'tracking data' do
context 'when the user is part of the experimental group' do
before do
stub_experiment_for_user(signup_flow: true)
end
it 'doesn\'t pass tracking parameters to the frontend' do
get(:new)
expect(Gon.tracking_data).to be_nil
end
end
context 'with the experimental signup flow enabled and the user is part of the control group' do
before do
stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: false)
allow_any_instance_of(described_class).to receive(:experimentation_subject_id).and_return('uuid')
end
it 'passes the right tracking parameters to the frontend' do
get(:new)
expect(Gon.tracking_data).to eq(
{
category: 'Growth::Acquisition::Experiment::SignUpFlow',
action: 'start',
label: 'uuid',
property: 'control_group'
}
)
end
end
end
end end
describe '#create' do describe '#create' do
......
...@@ -413,6 +413,7 @@ end ...@@ -413,6 +413,7 @@ end
describe 'With original flow' do describe 'With original flow' do
before do before do
stub_experiment(signup_flow: false) stub_experiment(signup_flow: false)
stub_experiment_for_user(signup_flow: false)
end end
it_behaves_like 'Signup' it_behaves_like 'Signup'
...@@ -421,6 +422,7 @@ end ...@@ -421,6 +422,7 @@ end
describe 'With experimental flow' do describe 'With experimental flow' do
before do before do
stub_experiment(signup_flow: true) stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: true)
end end
it_behaves_like 'Signup' it_behaves_like 'Signup'
......
This diff is collapsed.
...@@ -636,7 +636,7 @@ describe API::Users do ...@@ -636,7 +636,7 @@ describe API::Users do
describe "GET /users/sign_up" do describe "GET /users/sign_up" do
context 'when experimental signup_flow is active' do context 'when experimental signup_flow is active' do
before do before do
stub_experiment(signup_flow: true) stub_experiment_for_user(signup_flow: true)
end end
it "shows sign up page" do it "shows sign up page" do
...@@ -648,7 +648,7 @@ describe API::Users do ...@@ -648,7 +648,7 @@ describe API::Users do
context 'when experimental signup_flow is not active' do context 'when experimental signup_flow is not active' do
before do before do
stub_experiment(signup_flow: false) stub_experiment_for_user(signup_flow: false)
end end
it "redirects to sign in page" do it "redirects to sign in page" do
......
...@@ -9,7 +9,19 @@ module StubExperiments ...@@ -9,7 +9,19 @@ module StubExperiments
# - `stub_experiment(signup_flow: false)` ... Disable `signup_flow` experiment globally. # - `stub_experiment(signup_flow: false)` ... Disable `signup_flow` experiment globally.
def stub_experiment(experiments) def stub_experiment(experiments)
experiments.each do |experiment_key, enabled| experiments.each do |experiment_key, enabled|
allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key, any_args) { enabled } allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key) { enabled }
end
end
# Stub Experiment for user with `key: true/false`
#
# @param [Hash] experiment where key is feature name and value is boolean whether enabled or not.
#
# Examples
# - `stub_experiment_for_user(signup_flow: false)` ... Disable `signup_flow` experiment for user.
def stub_experiment_for_user(experiments)
experiments.each do |experiment_key, enabled|
allow(Gitlab::Experimentation).to receive(:enabled_for_user?).with(experiment_key, anything) { enabled }
end 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