Commit 70c2d2ad authored by Alex Buijs's avatar Alex Buijs

Integrate experimentation framework

instead of a custom helper
parent efdafde6
...@@ -550,7 +550,7 @@ class ApplicationController < ActionController::Base ...@@ -550,7 +550,7 @@ class ApplicationController < ActionController::Base
end end
def require_role def require_role
return unless current_user && current_user.role.blank? && helpers.use_experimental_separate_sign_up_flow? return unless current_user && current_user.role.blank? && experiment_enabled?(:signup_flow)
redirect_to users_sign_up_welcome_path redirect_to users_sign_up_welcome_path
end end
......
...@@ -5,6 +5,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController ...@@ -5,6 +5,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
include Gitlab::Allowable include Gitlab::Allowable
include PageLayoutHelper include PageLayoutHelper
include OauthApplications include OauthApplications
include Gitlab::Experimentation::ControllerConcern
before_action :verify_user_oauth_applications_enabled, except: :index before_action :verify_user_oauth_applications_enabled, except: :index
before_action :authenticate_user! before_action :authenticate_user!
......
# frozen_string_literal: true # frozen_string_literal: true
class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
include Gitlab::Experimentation::ControllerConcern
layout 'profile' layout 'profile'
# Overridden from Doorkeeper::AuthorizationsController to # Overridden from Doorkeeper::AuthorizationsController to
......
...@@ -15,7 +15,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -15,7 +15,7 @@ class RegistrationsController < Devise::RegistrationsController
if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? } if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
def new def new
if helpers.use_experimental_separate_sign_up_flow? if experiment_enabled?(:signup_flow)
@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')
...@@ -98,7 +98,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -98,7 +98,7 @@ class RegistrationsController < Devise::RegistrationsController
def after_sign_up_path_for(user) def after_sign_up_path_for(user)
Gitlab::AppLogger.info(user_created_message(confirmed: user.confirmed?)) Gitlab::AppLogger.info(user_created_message(confirmed: user.confirmed?))
return users_sign_up_welcome_path if helpers.use_experimental_separate_sign_up_flow? return users_sign_up_welcome_path if experiment_enabled?(:signup_flow)
confirmed_or_unconfirmed_access_allowed(user) ? stored_location_or_dashboard(user) : users_almost_there_path confirmed_or_unconfirmed_access_allowed(user) ? stored_location_or_dashboard(user) : users_almost_there_path
end end
...@@ -140,7 +140,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -140,7 +140,7 @@ class RegistrationsController < Devise::RegistrationsController
def sign_up_params def sign_up_params
clean_params = params.require(:user).permit(:username, :email, :email_confirmation, :name, :password) clean_params = params.require(:user).permit(:username, :email, :email_confirmation, :name, :password)
if helpers.use_experimental_separate_sign_up_flow? if experiment_enabled?(:signup_flow)
clean_params[:name] = clean_params[:username] clean_params[:name] = clean_params[:username]
end end
...@@ -184,7 +184,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -184,7 +184,7 @@ class RegistrationsController < Devise::RegistrationsController
# Part of an experiment to build a new sign up flow. Will be resolved # Part of an experiment to build a new sign up flow. Will be resolved
# with https://gitlab.com/gitlab-org/growth/engineering/issues/64 # with https://gitlab.com/gitlab-org/growth/engineering/issues/64
def choose_layout def choose_layout
if helpers.use_experimental_separate_sign_up_flow? if experiment_enabled?(:signup_flow)
'devise_experimental_separate_sign_up_flow' 'devise_experimental_separate_sign_up_flow'
else else
'devise' 'devise'
......
...@@ -4,8 +4,4 @@ module SessionsHelper ...@@ -4,8 +4,4 @@ module SessionsHelper
def unconfirmed_email? def unconfirmed_email?
flash[:alert] == t(:unconfirmed, scope: [:devise, :failure]) flash[:alert] == t(:unconfirmed, scope: [:devise, :failure])
end end
def use_experimental_separate_sign_up_flow?
::Gitlab.dev_env_or_com? && Feature.enabled?(:experimental_separate_sign_up_flow)
end
end end
- page_title "Sign up" - page_title "Sign up"
- if use_experimental_separate_sign_up_flow? - if experiment_enabled?(:signup_flow)
= render 'devise/shared/experimental_separate_sign_up_flow_box' = render 'devise/shared/experimental_separate_sign_up_flow_box'
- else - else
= render 'devise/shared/signup_box' = render 'devise/shared/signup_box'
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
- if form_based_providers.any? - if form_based_providers.any?
= render 'devise/shared/tabs_ldap' = render 'devise/shared/tabs_ldap'
- else - else
- unless use_experimental_separate_sign_up_flow? - unless experiment_enabled?(:signup_flow)
= render 'devise/shared/tabs_normal' = render 'devise/shared/tabs_normal'
.tab-content .tab-content
- if password_authentication_enabled_for_web? || ldap_enabled? || crowd_enabled? - if password_authentication_enabled_for_web? || ldap_enabled? || crowd_enabled?
......
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
.login-body .login-body
= render 'devise/sessions/new_base' = render 'devise/sessions/new_base'
- if use_experimental_separate_sign_up_flow? - if experiment_enabled?(:signup_flow)
%p.light.mt-2 %p.light.mt-2
= _("Don't have an account yet?") = _("Don't have an account yet?")
= link_to _("Register now"), new_registration_path(:user) = link_to _("Register now"), new_registration_path(:user)
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
-# anything from this page beforehand. -# anything from this page beforehand.
-# Part of an experiment to build a new sign up flow. Will be removed again with -# Part of an experiment to build a new sign up flow. Will be removed again with
-# https://gitlab.com/gitlab-org/growth/engineering/issues/64 -# https://gitlab.com/gitlab-org/growth/engineering/issues/64
- if use_experimental_separate_sign_up_flow? && current_path?("sessions#new") - if experiment_enabled?(:signup_flow) && current_path?("sessions#new")
= javascript_tag nonce: true do = javascript_tag nonce: true do
:plain :plain
if (window.location.hash === '#register-pane') { if (window.location.hash === '#register-pane') {
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
= form_for(current_user, url: users_sign_up_update_role_path, html: { class: 'new_new_user gl-show-field-errors', 'aria-live' => 'assertive' }) do |f| = form_for(current_user, url: users_sign_up_update_role_path, html: { class: 'new_new_user gl-show-field-errors', 'aria-live' => 'assertive' }) do |f|
.devise-errors.mt-0 .devise-errors.mt-0
= render 'devise/shared/error_messages', resource: current_user = render 'devise/shared/error_messages', resource: current_user
.form-group .name.form-group
= f.label :name, _('Full name'), class: 'label-bold' = f.label :name, _('Full name'), class: 'label-bold'
= f.text_field :name, class: 'form-control top js-block-emoji js-validate-length', :data => { :max_length => max_name_length, :max_length_message => s_('Name is too long (maximum is %{max_length} characters).') % { max_length: max_name_length }, :qa_selector => 'new_user_name_field' }, required: true, title: _('This field is required.') = f.text_field :name, class: 'form-control top js-block-emoji js-validate-length', :data => { :max_length => max_name_length, :max_length_message => s_('Name is too long (maximum is %{max_length} characters).') % { max_length: max_name_length }, :qa_selector => 'new_user_name_field' }, required: true, title: _('This field is required.')
.form-group .form-group
......
...@@ -23,7 +23,7 @@ module EE ...@@ -23,7 +23,7 @@ module EE
clean_params[:email_opted_in_at] = Time.zone.now clean_params[:email_opted_in_at] = Time.zone.now
end end
if helpers.use_experimental_separate_sign_up_flow? if experiment_enabled?(:signup_flow)
clean_params[:name] = clean_params[:username] clean_params[:name] = clean_params[:username]
end end
......
...@@ -129,7 +129,7 @@ shared_examples 'Signup' do ...@@ -129,7 +129,7 @@ shared_examples 'Signup' do
describe 'user\'s full name validation', :js do describe 'user\'s full name validation', :js do
before do before do
if Feature.enabled?(:experimental_separate_sign_up_flow) if Gitlab::Experimentation.enabled?(:signup_flow)
user = create(:user, role: nil) user = create(:user, role: nil)
sign_in(user) sign_in(user)
visit users_sign_up_welcome_path visit users_sign_up_welcome_path
...@@ -188,7 +188,7 @@ shared_examples 'Signup' do ...@@ -188,7 +188,7 @@ shared_examples 'Signup' do
fill_in 'new_user_username', with: new_user.username fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email fill_in 'new_user_email', with: new_user.email
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email fill_in 'new_user_email_confirmation', with: new_user.email
end end
...@@ -213,7 +213,7 @@ shared_examples 'Signup' do ...@@ -213,7 +213,7 @@ shared_examples 'Signup' do
fill_in 'new_user_username', with: new_user.username fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email fill_in 'new_user_email', with: new_user.email
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email fill_in 'new_user_email_confirmation', with: new_user.email
end end
...@@ -222,7 +222,7 @@ shared_examples 'Signup' do ...@@ -222,7 +222,7 @@ shared_examples 'Signup' do
expect { click_button 'Register' }.to change { User.count }.by(1) expect { click_button 'Register' }.to change { User.count }.by(1)
if Feature.enabled?(:experimental_separate_sign_up_flow) if Gitlab::Experimentation.enabled?(:signup_flow)
expect(current_path).to eq users_sign_up_welcome_path expect(current_path).to eq users_sign_up_welcome_path
else else
expect(current_path).to eq dashboard_projects_path expect(current_path).to eq dashboard_projects_path
...@@ -239,7 +239,7 @@ shared_examples 'Signup' do ...@@ -239,7 +239,7 @@ shared_examples 'Signup' do
fill_in 'new_user_username', with: new_user.username fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email fill_in 'new_user_email', with: new_user.email
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email.capitalize fill_in 'new_user_email_confirmation', with: new_user.email.capitalize
end end
...@@ -247,7 +247,7 @@ shared_examples 'Signup' do ...@@ -247,7 +247,7 @@ shared_examples 'Signup' do
fill_in 'new_user_password', with: new_user.password fill_in 'new_user_password', with: new_user.password
click_button "Register" click_button "Register"
if Feature.enabled?(:experimental_separate_sign_up_flow) if Gitlab::Experimentation.enabled?(:signup_flow)
expect(current_path).to eq users_sign_up_welcome_path expect(current_path).to eq users_sign_up_welcome_path
else else
expect(current_path).to eq dashboard_projects_path expect(current_path).to eq dashboard_projects_path
...@@ -267,7 +267,7 @@ shared_examples 'Signup' do ...@@ -267,7 +267,7 @@ shared_examples 'Signup' do
fill_in 'new_user_username', with: new_user.username fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email fill_in 'new_user_email', with: new_user.email
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email fill_in 'new_user_email_confirmation', with: new_user.email
end end
...@@ -275,7 +275,7 @@ shared_examples 'Signup' do ...@@ -275,7 +275,7 @@ shared_examples 'Signup' do
fill_in 'new_user_password', with: new_user.password fill_in 'new_user_password', with: new_user.password
click_button "Register" click_button "Register"
if Feature.enabled?(:experimental_separate_sign_up_flow) if Gitlab::Experimentation.enabled?(:signup_flow)
expect(current_path).to eq users_sign_up_welcome_path expect(current_path).to eq users_sign_up_welcome_path
else else
expect(current_path).to eq dashboard_projects_path expect(current_path).to eq dashboard_projects_path
...@@ -291,7 +291,7 @@ shared_examples 'Signup' do ...@@ -291,7 +291,7 @@ shared_examples 'Signup' do
visit new_user_registration_path visit new_user_registration_path
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
end end
...@@ -302,7 +302,7 @@ shared_examples 'Signup' do ...@@ -302,7 +302,7 @@ shared_examples 'Signup' do
expect(current_path).to eq user_registration_path expect(current_path).to eq user_registration_path
if Feature.enabled?(:experimental_separate_sign_up_flow) if Gitlab::Experimentation.enabled?(:signup_flow)
expect(page).to have_content("error prohibited this user from being saved") expect(page).to have_content("error prohibited this user from being saved")
else else
expect(page).to have_content("errors prohibited this user from being saved") expect(page).to have_content("errors prohibited this user from being saved")
...@@ -317,7 +317,7 @@ shared_examples 'Signup' do ...@@ -317,7 +317,7 @@ shared_examples 'Signup' do
visit new_user_registration_path visit new_user_registration_path
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
end end
...@@ -342,7 +342,7 @@ shared_examples 'Signup' do ...@@ -342,7 +342,7 @@ shared_examples 'Signup' do
fill_in 'new_user_username', with: new_user.username fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email fill_in 'new_user_email', with: new_user.email
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email fill_in 'new_user_email_confirmation', with: new_user.email
end end
...@@ -361,7 +361,7 @@ shared_examples 'Signup' do ...@@ -361,7 +361,7 @@ shared_examples 'Signup' do
fill_in 'new_user_username', with: new_user.username fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email fill_in 'new_user_email', with: new_user.email
unless Feature.enabled?(:experimental_separate_sign_up_flow) unless Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_name', with: new_user.name fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email fill_in 'new_user_email_confirmation', with: new_user.email
end end
...@@ -371,7 +371,7 @@ shared_examples 'Signup' do ...@@ -371,7 +371,7 @@ shared_examples 'Signup' do
click_button "Register" click_button "Register"
if Feature.enabled?(:experimental_separate_sign_up_flow) if Gitlab::Experimentation.enabled?(:signup_flow)
expect(current_path).to eq users_sign_up_welcome_path expect(current_path).to eq users_sign_up_welcome_path
else else
expect(current_path).to eq dashboard_projects_path expect(current_path).to eq dashboard_projects_path
...@@ -382,16 +382,15 @@ end ...@@ -382,16 +382,15 @@ end
describe 'With original flow' do describe 'With original flow' do
before do before do
stub_feature_flags(experimental_separate_sign_up_flow: false) stub_experiment(signup_flow: false)
end end
it_behaves_like 'Signup' it_behaves_like 'Signup'
end end
describe 'With experimental flow on GitLab.com' do describe 'With experimental flow' do
before do before do
allow(::Gitlab).to receive(:com?).and_return(true) stub_experiment(signup_flow: true)
stub_feature_flags(experimental_separate_sign_up_flow: true)
end end
it_behaves_like 'Signup' it_behaves_like 'Signup'
......
...@@ -634,40 +634,21 @@ describe API::Users do ...@@ -634,40 +634,21 @@ describe API::Users do
end end
describe "GET /users/sign_up" do describe "GET /users/sign_up" do
context 'when experimental_separate_sign_up_flow is active' do context 'when experimental signup_flow is active' do
before do before do
stub_feature_flags(experimental_separate_sign_up_flow: true) stub_experiment(signup_flow: true)
end end
context 'on gitlab.com' do it "shows sign up page" do
before do get "/users/sign_up"
allow(::Gitlab).to receive(:com?).and_return(true) expect(response).to have_gitlab_http_status(200)
end expect(response).to render_template(:new)
it "shows sign up page" do
get "/users/sign_up"
expect(response).to have_gitlab_http_status(200)
expect(response).to render_template(:new)
end
end
context 'not on gitlab.com' do
before do
allow(::Gitlab).to receive(:com?).and_return(false)
end
it "redirects to sign in page" do
get "/users/sign_up"
expect(response).to have_gitlab_http_status(302)
expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane'))
end
end end
end end
context 'when experimental_separate_sign_up_flow is not active' do context 'when experimental signup_flow is not active' do
before do before do
allow(::Gitlab).to receive(:com?).and_return(true) stub_experiment(signup_flow: false)
stub_feature_flags(experimental_separate_sign_up_flow: false)
end end
it "redirects to sign in page" do it "redirects to sign in page" do
......
...@@ -88,6 +88,7 @@ RSpec.configure do |config| ...@@ -88,6 +88,7 @@ RSpec.configure do |config|
config.include FixtureHelpers config.include FixtureHelpers
config.include GitlabRoutingHelper config.include GitlabRoutingHelper
config.include StubFeatureFlags config.include StubFeatureFlags
config.include StubExperiments
config.include StubGitlabCalls config.include StubGitlabCalls
config.include StubGitlabData config.include StubGitlabData
config.include NextInstanceOf config.include NextInstanceOf
......
# frozen_string_literal: true
module StubExperiments
# Stub Experiment with `key: true/false`
#
# @param [Hash] experiment where key is feature name and value is boolean whether enabled or not.
#
# Examples
# - `stub_experiment(signup_flow: false)` ... Disable `signup_flow` experiment globally.
def stub_experiment(experiments)
experiments.each do |experiment_key, enabled|
allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key, any_args) { enabled }
end
end
end
...@@ -10,6 +10,7 @@ describe 'devise/shared/_signin_box' do ...@@ -10,6 +10,7 @@ describe 'devise/shared/_signin_box' do
allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)
allow(view).to receive(:captcha_enabled?).and_return(false) allow(view).to receive(:captcha_enabled?).and_return(false)
allow(view).to receive(:captcha_on_login_required?).and_return(false) allow(view).to receive(:captcha_on_login_required?).and_return(false)
allow(view).to receive(:experiment_enabled?).and_return(false)
end end
it 'is shown when Crowd is enabled' do it 'is shown when Crowd is enabled' do
......
...@@ -7,6 +7,7 @@ describe 'layouts/_head' do ...@@ -7,6 +7,7 @@ describe 'layouts/_head' do
before do before do
allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)
allow(view).to receive(:experiment_enabled?).and_return(false)
end end
it 'escapes HTML-safe strings in page_title' do it 'escapes HTML-safe strings in page_title' do
......
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