Commit 57f0bd6b authored by Thong Kuah's avatar Thong Kuah

Merge branch '20-add-signup-step-2' into 'master'

Add step 2 of the experimental signup flow

See merge request gitlab-org/gitlab!16583
parents 6476a51c 838d0fac
import LengthValidator from '~/pages/sessions/new/length_validator';
import NoEmojiValidator from '~/emoji/no_emoji_validator';
document.addEventListener('DOMContentLoaded', () => {
new LengthValidator(); // eslint-disable-line no-new
new NoEmojiValidator(); // eslint-disable-line no-new
});
......@@ -29,6 +29,7 @@ class ApplicationController < ActionController::Base
before_action :active_user_check, unless: :devise_controller?
before_action :set_usage_stats_consent_flag
before_action :check_impersonation_availability
before_action :require_role
around_action :set_locale
around_action :set_session_storage
......@@ -547,6 +548,16 @@ class ApplicationController < ActionController::Base
def current_user_mode
@current_user_mode ||= Gitlab::Auth::CurrentUserMode.new(current_user)
end
# A user requires a role when they are part of the experimental signup flow (executed by the Growth team). Users
# are redirected to the welcome page when their role is required and the experiment is enabled for the current user.
def require_role
return unless current_user && current_user.role_required? && experiment_enabled?(:signup_flow)
store_location_for :user, request.fullpath
redirect_to users_sign_up_welcome_path
end
end
ApplicationController.prepend_if_ee('EE::ApplicationController')
......@@ -8,7 +8,7 @@ module InvisibleCaptcha
end
def on_honeypot_spam_callback
return unless Feature.enabled?(:invisible_captcha)
return unless Feature.enabled?(:invisible_captcha) || experiment_enabled?(:signup_flow)
invisible_captcha_honeypot_counter.increment
log_request('Invisible_Captcha_Honeypot_Request')
......@@ -17,7 +17,7 @@ module InvisibleCaptcha
end
def on_timestamp_spam_callback
return unless Feature.enabled?(:invisible_captcha)
return unless Feature.enabled?(:invisible_captcha) || experiment_enabled?(:signup_flow)
invisible_captcha_timestamp_counter.increment
log_request('Invisible_Captcha_Timestamp_Request')
......
......@@ -5,6 +5,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
include Gitlab::Allowable
include PageLayoutHelper
include OauthApplications
include Gitlab::Experimentation::ControllerConcern
before_action :verify_user_oauth_applications_enabled, except: :index
before_action :authenticate_user!
......
# frozen_string_literal: true
class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
include Gitlab::Experimentation::ControllerConcern
layout 'profile'
# Overridden from Doorkeeper::AuthorizationsController to
......
......@@ -100,6 +100,7 @@ class ProfilesController < Profiles::ApplicationController
:avatar,
:bio,
:email,
:role,
:hide_no_password,
:hide_no_ssh_key,
:hide_project_limit,
......
......@@ -8,13 +8,14 @@ class RegistrationsController < Devise::RegistrationsController
layout :choose_layout
skip_before_action :require_role, only: [:welcome, :update_role]
prepend_before_action :check_captcha, only: :create
before_action :whitelist_query_limiting, only: [:destroy]
before_action :ensure_terms_accepted,
if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
def new
if helpers.use_experimental_separate_sign_up_flow?
if experiment_enabled?(:signup_flow)
@resource = build_resource
else
redirect_to new_user_session_path(anchor: 'register-pane')
......@@ -26,8 +27,13 @@ class RegistrationsController < Devise::RegistrationsController
super do |new_user|
persist_accepted_terms_if_required(new_user)
set_role_required(new_user)
yield new_user if block_given?
end
# Do not show the signed_up notice message when the signup_flow experiment is enabled.
# Instead, show it after succesfully updating the role.
flash[:notice] = nil if experiment_enabled?(:signup_flow)
rescue Gitlab::Access::AccessDeniedError
redirect_to(new_user_session_path)
end
......@@ -42,6 +48,26 @@ class RegistrationsController < Devise::RegistrationsController
end
end
def welcome
return redirect_to new_user_registration_path unless current_user
return redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) if current_user.role.present?
current_user.name = nil
render layout: 'devise_experimental_separate_sign_up_flow'
end
def update_role
user_params = params.require(:user).permit(:name, :role)
result = ::Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute
if result[:status] == :success
set_flash_message! :notice, :signed_up
redirect_to stored_location_or_dashboard_or_almost_there_path(current_user)
else
redirect_to users_sign_up_welcome_path, alert: result[:message]
end
end
protected
def persist_accepted_terms_if_required(new_user)
......@@ -54,6 +80,10 @@ class RegistrationsController < Devise::RegistrationsController
end
end
def set_role_required(new_user)
new_user.set_role_required! if new_user.persisted? && experiment_enabled?(:signup_flow)
end
def destroy_confirmation_valid?
if current_user.confirm_deletion_with_password?
current_user.valid_password?(params[:password])
......@@ -76,7 +106,10 @@ class RegistrationsController < Devise::RegistrationsController
def after_sign_up_path_for(user)
Gitlab::AppLogger.info(user_created_message(confirmed: user.confirmed?))
confirmed_or_unconfirmed_access_allowed(user) ? stored_location_or_dashboard(user) : users_almost_there_path
return users_sign_up_welcome_path if experiment_enabled?(:signup_flow)
stored_location_or_dashboard_or_almost_there_path(user)
end
def after_inactive_sign_up_path_for(resource)
......@@ -103,6 +136,7 @@ class RegistrationsController < Devise::RegistrationsController
ensure_correct_params!
return unless Feature.enabled?(:registrations_recaptcha, default_enabled: true) # reCAPTCHA on the UI will still display however
return if experiment_enabled?(:signup_flow) # when the experimental signup flow is enabled for the current user, disable the reCAPTCHA check
return unless show_recaptcha_sign_up?
return unless Gitlab::Recaptcha.load_configurations!
......@@ -114,7 +148,13 @@ class RegistrationsController < Devise::RegistrationsController
end
def sign_up_params
params.require(:user).permit(:username, :email, :email_confirmation, :name, :password)
clean_params = params.require(:user).permit(:username, :email, :email_confirmation, :name, :password)
if experiment_enabled?(:signup_flow)
clean_params[:name] = clean_params[:username]
end
clean_params
end
def resource_name
......@@ -144,17 +184,21 @@ class RegistrationsController < Devise::RegistrationsController
end
def confirmed_or_unconfirmed_access_allowed(user)
user.confirmed? || Feature.enabled?(:soft_email_confirmation)
user.confirmed? || Feature.enabled?(:soft_email_confirmation) || experiment_enabled?(:signup_flow)
end
def stored_location_or_dashboard(user)
stored_location_for(user) || dashboard_projects_path
end
def stored_location_or_dashboard_or_almost_there_path(user)
confirmed_or_unconfirmed_access_allowed(user) ? stored_location_or_dashboard(user) : users_almost_there_path
end
# Part of an experiment to build a new sign up flow. Will be resolved
# with https://gitlab.com/gitlab-org/growth/engineering/issues/64
def choose_layout
if helpers.use_experimental_separate_sign_up_flow?
if experiment_enabled?(:signup_flow)
'devise_experimental_separate_sign_up_flow'
else
'devise'
......
......@@ -4,8 +4,4 @@ module SessionsHelper
def unconfirmed_email?
flash[:alert] == t(:unconfirmed, scope: [:devise, :failure])
end
def use_experimental_separate_sign_up_flow?
::Gitlab.dev_env_or_com? && Feature.enabled?(:experimental_separate_sign_up_flow)
end
end
......@@ -231,6 +231,10 @@ class User < ApplicationRecord
# Note: When adding an option, it MUST go on the end of the array.
enum project_view: [:readme, :activity, :files]
# User's role
# Note: When adding an option, it MUST go on the end of the array.
enum role: [:software_developer, :development_team_lead, :devops_engineer, :systems_administrator, :security_analyst, :data_analyst, :product_manager, :product_designer, :other], _suffix: true
delegate :path, to: :namespace, allow_nil: true, prefix: true
delegate :notes_filter_for, to: :user_preference
delegate :set_notes_filter, to: :user_preference
......@@ -1571,6 +1575,20 @@ class User < ApplicationRecord
[last_activity, last_sign_in].compact.max
end
# Below is used for the signup_flow experiment. Should be removed
# when experiment finishes.
# See https://gitlab.com/gitlab-org/growth/engineering/issues/64
REQUIRES_ROLE_VALUE = 99
def role_required?
role_before_type_cast == REQUIRES_ROLE_VALUE
end
def set_role_required!
update_column(:role, REQUIRES_ROLE_VALUE)
end
# End of signup_flow experiment methods
# @deprecated
alias_method :owned_or_masters_groups, :owned_or_maintainers_groups
......
- 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'
- else
= render 'devise/shared/signup_box'
......
......@@ -4,7 +4,7 @@
- if form_based_providers.any?
= render 'devise/shared/tabs_ldap'
- else
- unless use_experimental_separate_sign_up_flow?
- unless experiment_enabled?(:signup_flow)
= render 'devise/shared/tabs_normal'
.tab-content
- if password_authentication_enabled_for_web? || ldap_enabled? || crowd_enabled?
......
- max_name_length = 128
- content_for(:page_title, _('Register for GitLab'))
- max_username_length = 255
.signup-box.p-3.mb-2
.signup-body
......@@ -6,9 +6,6 @@
.devise-errors.mt-0
= render "devise/shared/error_messages", resource: resource
= invisible_captcha
.name.form-group
= 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_("SignUp|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.")
.username.form-group
= f.label :username, class: 'label-bold'
= f.text_field :username, class: "form-control middle js-block-emoji js-validate-length js-validate-username", :data => { :max_length => max_username_length, :max_length_message => s_("SignUp|Username is too long (maximum is %{max_length} characters).") % { max_length: max_username_length }, :qa_selector => 'new_user_username_field' }, pattern: Gitlab::PathRegex::NAMESPACE_FORMAT_REGEX_JS, required: true, title: _("Please create a username with only alphanumeric characters.")
......
......@@ -23,7 +23,7 @@
.login-body
= render 'devise/sessions/new_base'
- if use_experimental_separate_sign_up_flow?
- if experiment_enabled?(:signup_flow)
%p.light.mt-2
= _("Don't have an account yet?")
= link_to _("Register now"), new_registration_path(:user)
......@@ -5,7 +5,7 @@
-# anything from this page beforehand.
-# 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
- 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
:plain
if (window.location.hash === '#register-pane') {
......
......@@ -14,7 +14,8 @@
= render_if_exists 'layouts/devise_help_text'
.text-center.signup-heading.mt-3.mb-3
= image_tag(image_url('logo.svg'), class: 'gitlab-logo', alt: 'GitLab Logo')
%h2= _('Register for GitLab.com')
- if content_for?(:page_title)
%h2= yield :page_title
= yield
%hr.footer-fixed
.footer-container
......
......@@ -94,6 +94,7 @@
- else
= f.text_field :name, label: s_('Profiles|Full name'), required: true, title: s_("Profiles|Using emojis in names seems fun, but please try to set a status message instead"), wrapper: { class: 'col-md-9 qa-full-name rspec-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you")
= f.text_field :id, readonly: true, label: s_('Profiles|User ID'), wrapper: { class: 'col-md-3' }
= f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'input-md'
= render_if_exists 'profiles/email_settings', form: f
= f.text_field :skype, class: 'input-md', placeholder: s_("Profiles|username")
......
- content_for(:page_title, _('Welcome to GitLab<br>%{username}!' % { username: html_escape(current_user.username) }).html_safe)
- max_name_length = 128
.text-center.mb-3
= _('In order to tailor your experience with GitLab<br>we would like to know a bit more about you.').html_safe
.signup-box.p-3.mb-2
.signup-body
= 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
= render 'devise/shared/error_messages', resource: current_user
.name.form-group
= 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.')
.form-group
= f.label :role, _('Role'), class: 'label-bold'
= f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'form-control'
.submit-container.mt-3
= f.submit _('Get started!'), class: 'btn-register btn btn-block mb-0 p-2'
---
title: Add step 2 of the experimental signup flow
merge_request: 16583
author:
type: changed
......@@ -55,6 +55,10 @@ Rails.application.routes.draw do
get '/autocomplete/project_groups' => 'autocomplete#project_groups'
end
# Sign up
get 'users/sign_up/welcome' => 'registrations#welcome'
patch 'users/sign_up/update_role' => 'registrations#update_role'
# Search
get 'search' => 'search#show'
get 'search/autocomplete' => 'search#autocomplete', as: :search_autocomplete
......
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddRoleToUsers < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :users, :role, :smallint
end
end
......@@ -3780,6 +3780,7 @@ ActiveRecord::Schema.define(version: 2019_10_16_220135) do
t.string "first_name", limit: 255
t.string "last_name", limit: 255
t.string "static_object_token", limit: 255
t.integer "role", limit: 2
t.index "lower((name)::text)", name: "index_on_users_name_lower"
t.index ["accepted_term_id"], name: "index_users_on_accepted_term_id"
t.index ["admin"], name: "index_users_on_admin"
......
......@@ -15,7 +15,7 @@ module EE
end
def sign_up_params
clean_params = params.require(:user).permit(:username, :email, :email_confirmation, :name, :password, :email_opted_in)
clean_params = super.merge(params.require(:user).permit(:email_opted_in))
if clean_params[:email_opted_in] == '1'
clean_params[:email_opted_in_ip] = request.remote_ip
......
......@@ -3,10 +3,6 @@
require 'spec_helper'
describe RegistrationsController do
before do
stub_feature_flags(invisible_captcha: false)
end
describe '#create' do
context 'when the user opted-in' do
let(:user_params) { { user: attributes_for(:user, email_opted_in: '1') } }
......
......@@ -45,7 +45,6 @@ describe TrialRegistrationsController do
describe '#create' do
before do
stub_feature_flags(invisible_captcha: false)
stub_application_setting(send_user_confirmation_email: true)
end
......
......@@ -5,10 +5,6 @@ require 'spec_helper'
describe 'Signup on EE' do
let(:user_attrs) { attributes_for(:user) }
before do
stub_feature_flags(invisible_captcha: false)
end
context 'for Gitlab.com' do
before do
expect(Gitlab).to receive(:com?).and_return(true).at_least(:once)
......
......@@ -7,7 +7,6 @@ describe 'Trial Sign Up', :js do
describe 'on GitLab.com' do
before do
stub_feature_flags(invisible_captcha: false)
stub_feature_flags(improved_trial_signup: true)
allow(Gitlab).to receive(:com?).and_return(true).at_least(:once)
end
......
......@@ -7,7 +7,6 @@ describe 'Trial Capture Lead', :js do
let(:user) { create(:user) }
before do
stub_feature_flags(invisible_captcha: false)
stub_feature_flags(improved_trial_signup: true)
allow(Gitlab).to receive(:com?).and_return(true).at_least(:once)
sign_in(user)
......
......@@ -9,7 +9,6 @@ describe 'Trial Select Namespace', :js do
let(:user) { create(:user) }
before do
stub_feature_flags(invisible_captcha: false)
stub_feature_flags(improved_trial_signup: true)
allow(Gitlab).to receive(:com?).and_return(true).at_least(:once)
sign_in(user)
......
......@@ -40,8 +40,8 @@ module Gitlab
}
end
def experiment_enabled?(experiment)
Experimentation.enabled?(experiment, experimentation_subject_index)
def experiment_enabled?(experiment_key)
Experimentation.enabled?(experiment_key, experimentation_subject_index)
end
private
......@@ -55,10 +55,14 @@ module Gitlab
end
class << self
def experiment(key)
Experiment.new(EXPERIMENTS[key].merge(key: key))
end
def enabled?(experiment_key, experimentation_subject_index)
return false unless EXPERIMENTS.key?(experiment_key)
experiment = Experiment.new(EXPERIMENTS[experiment_key].merge(key: experiment_key))
experiment = experiment(experiment_key)
experiment.feature_toggle_enabled? &&
experiment.enabled_for_environment? &&
......
......@@ -7841,6 +7841,9 @@ msgstr ""
msgid "Get started with performance monitoring"
msgstr ""
msgid "Get started!"
msgstr ""
msgid "Getting started with releases"
msgstr ""
......@@ -8917,6 +8920,9 @@ msgstr ""
msgid "In order to gather accurate feature usage data, it can take 1 to 2 weeks to see your index."
msgstr ""
msgid "In order to tailor your experience with GitLab<br>we would like to know a bit more about you."
msgstr ""
msgid "In the next step, you'll be able to select the projects you want to import."
msgstr ""
......@@ -10708,6 +10714,9 @@ msgstr ""
msgid "Name has already been taken"
msgstr ""
msgid "Name is too long (maximum is %{max_length} characters)."
msgstr ""
msgid "Name new label"
msgstr ""
......@@ -13513,7 +13522,7 @@ msgstr ""
msgid "Register and see your runners for this project."
msgstr ""
msgid "Register for GitLab.com"
msgid "Register for GitLab"
msgstr ""
msgid "Register now"
......@@ -14092,6 +14101,9 @@ msgstr ""
msgid "Roadmap"
msgstr ""
msgid "Role"
msgstr ""
msgid "Rollback"
msgstr ""
......
......@@ -842,4 +842,48 @@ describe ApplicationController do
end
end
end
describe '#require_role' do
controller(described_class) do
def index; end
end
let(:user) { create(:user) }
let(:experiment_enabled) { true }
before do
stub_experiment(signup_flow: experiment_enabled)
end
context 'experiment enabled and user with required role' do
before do
user.set_role_required!
sign_in(user)
get :index
end
it { is_expected.to redirect_to users_sign_up_welcome_path }
end
context 'experiment enabled and user without a role' do
before do
sign_in(user)
get :index
end
it { is_expected.not_to redirect_to users_sign_up_welcome_path }
end
context 'experiment disabled and user with required role' do
let(:experiment_enabled) { false }
before do
user.set_role_required!
sign_in(user)
get :index
end
it { is_expected.not_to redirect_to users_sign_up_welcome_path }
end
end
end
......@@ -114,9 +114,14 @@ describe RegistrationsController do
context 'when invisible captcha is enabled' do
before do
stub_feature_flags(invisible_captcha: true)
InvisibleCaptcha.timestamp_enabled = true
InvisibleCaptcha.timestamp_threshold = treshold
end
after do
InvisibleCaptcha.timestamp_enabled = false
end
let(:treshold) { 4 }
let(:session_params) { { invisible_captcha_timestamp: form_rendered_time.iso8601 } }
let(:form_rendered_time) { Time.current }
......
......@@ -6,6 +6,7 @@ FactoryBot.define do
name { generate(:name) }
username { generate(:username) }
password { "12345678" }
role { 'software_developer' }
confirmed_at { Time.now }
confirmation_token { nil }
can_create_group { true }
......
......@@ -10,7 +10,6 @@ describe 'Invites' do
let(:group_invite) { group.group_members.invite.last }
before do
stub_feature_flags(invisible_captcha: false)
project.add_maintainer(owner)
group.add_user(owner, Gitlab::Access::OWNER)
group.add_developer('user@example.com', owner)
......
This diff is collapsed.
......@@ -634,40 +634,21 @@ describe API::Users do
end
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
stub_feature_flags(experimental_separate_sign_up_flow: true)
stub_experiment(signup_flow: true)
end
context 'on gitlab.com' do
before do
allow(::Gitlab).to receive(:com?).and_return(true)
end
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
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 'when experimental_separate_sign_up_flow is not active' do
context 'when experimental signup_flow is not active' do
before do
allow(::Gitlab).to receive(:com?).and_return(true)
stub_feature_flags(experimental_separate_sign_up_flow: false)
stub_experiment(signup_flow: false)
end
it "redirects to sign in page" do
......
......@@ -88,6 +88,7 @@ RSpec.configure do |config|
config.include FixtureHelpers
config.include GitlabRoutingHelper
config.include StubFeatureFlags
config.include StubExperiments
config.include StubGitlabCalls
config.include StubGitlabData
config.include NextInstanceOf
......@@ -378,3 +379,6 @@ end
# Prevent Rugged from picking up local developer gitconfig.
Rugged::Settings['search_path_global'] = Rails.root.join('tmp/tests').to_s
# Disable timestamp checks for invisible_captcha
InvisibleCaptcha.timestamp_enabled = false
# 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
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_on_login_required?).and_return(false)
allow(view).to receive(:experiment_enabled?).and_return(false)
end
it 'is shown when Crowd is enabled' do
......
......@@ -7,6 +7,7 @@ describe 'layouts/_head' do
before do
allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)
allow(view).to receive(:experiment_enabled?).and_return(false)
end
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