Commit 7282f476 authored by Max Woolf's avatar Max Woolf

Merge branch '327239-engineering-increase-invite-acceptance-by-removing-exit-ctas' into 'master'

Experiment - Customize Registration page for invites [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60939
parents 6b66a1e3 f22f07f4
...@@ -106,6 +106,10 @@ class ApplicationController < ActionController::Base ...@@ -106,6 +106,10 @@ class ApplicationController < ActionController::Base
redirect_back(fallback_location: default, **options) redirect_back(fallback_location: default, **options)
end end
def check_if_gl_com_or_dev
render_404 unless ::Gitlab.dev_env_or_com?
end
def not_found def not_found
render_404 render_404
end end
......
...@@ -86,7 +86,18 @@ class InvitesController < ApplicationController ...@@ -86,7 +86,18 @@ class InvitesController < ApplicationController
if user_sign_up? if user_sign_up?
set_session_invite_params set_session_invite_params
redirect_to new_user_registration_path(invite_email: member.invite_email), notice: _("To accept this invitation, create an account or sign in.") experiment(:invite_signup_page_interaction, actor: member) do |experiment_instance|
set_originating_member_id if experiment_instance.enabled?
experiment_instance.use do
redirect_to new_user_registration_path(invite_email: member.invite_email), notice: _("To accept this invitation, create an account or sign in.")
end
experiment_instance.try do
redirect_to new_users_sign_up_invite_path(invite_email: member.invite_email)
end
experiment_instance.track(:view)
end
else else
redirect_to new_user_session_path(sign_in_redirect_params), notice: sign_in_notice redirect_to new_user_session_path(sign_in_redirect_params), notice: sign_in_notice
end end
...@@ -95,7 +106,11 @@ class InvitesController < ApplicationController ...@@ -95,7 +106,11 @@ class InvitesController < ApplicationController
def set_session_invite_params def set_session_invite_params
session[:invite_email] = member.invite_email session[:invite_email] = member.invite_email
session[:originating_member_id] = member.id if Members::InviteEmailExperiment.initial_invite_email?(params[:invite_type]) set_originating_member_id if Members::InviteEmailExperiment.initial_invite_email?(params[:invite_type])
end
def set_originating_member_id
session[:originating_member_id] = member.id
end end
def sign_in_redirect_params def sign_in_redirect_params
......
# frozen_string_literal: true
module Registrations
class InvitesController < RegistrationsController
layout 'simple_registration'
before_action :check_if_gl_com_or_dev
end
end
...@@ -199,6 +199,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -199,6 +199,7 @@ class RegistrationsController < Devise::RegistrationsController
return unless member return unless member
experiment(:invite_signup_page_interaction, actor: member).track(:form_submission)
experiment('members/invite_email', actor: member).track(:accepted) experiment('members/invite_email', actor: member).track(:accepted)
end end
end end
......
...@@ -16,7 +16,7 @@ module AuthHelper ...@@ -16,7 +16,7 @@ module AuthHelper
twitter twitter
).freeze ).freeze
LDAP_PROVIDER = /\Aldap/.freeze LDAP_PROVIDER = /\Aldap/.freeze
TRIAL_REGISTRATION_PROVIDERS = %w(google_oauth2 github).freeze POPULAR_PROVIDERS = %w(google_oauth2 github).freeze
def ldap_enabled? def ldap_enabled?
Gitlab::Auth::Ldap::Config.enabled? Gitlab::Auth::Ldap::Config.enabled?
...@@ -116,19 +116,12 @@ module AuthHelper ...@@ -116,19 +116,12 @@ module AuthHelper
providers = button_based_providers.map(&:to_s) - disabled_providers providers = button_based_providers.map(&:to_s) - disabled_providers
providers.sort_by do |provider| providers.sort_by do |provider|
case provider POPULAR_PROVIDERS.index(provider) || POPULAR_PROVIDERS.length
when 'google_oauth2'
0
when 'github'
1
else
2
end
end end
end end
def trial_enabled_button_based_providers def popular_enabled_button_based_providers
enabled_button_based_providers & TRIAL_REGISTRATION_PROVIDERS enabled_button_based_providers & POPULAR_PROVIDERS
end end
def button_based_providers_enabled? def button_based_providers_enabled?
......
# frozen_string_literal: true
module RegistrationsHelper
def social_signin_enabled?
::Gitlab.dev_env_or_com? &&
omniauth_enabled? &&
devise_mapping.omniauthable? &&
button_based_providers_enabled?
end
end
RegistrationsHelper.prepend_mod_with('RegistrationsHelper')
= render 'devise/shared/signup_omniauth_provider_list', providers: trial_enabled_button_based_providers = render 'devise/shared/signup_omniauth_provider_list', providers: popular_enabled_button_based_providers
.omniauth-divider.d-flex.align-items-center.text-center .omniauth-divider.d-flex.align-items-center.text-center
= _("or") = _("or")
!!! 5 !!! 5
%html{ lang: "en" } %html{ lang: "en" }
= render "layouts/head" = render "layouts/head"
%body.ui-indigo.login-page.application.navless %body.login-page.application.navless{ class: user_application_theme, data: { page: body_data_page } }
= render "layouts/header/logo_with_title" = render "layouts/header/logo_with_title"
= render "layouts/broadcast" = render "layouts/broadcast"
.container.navless-container.pt-0 .container.navless-container.pt-0
......
- page_title _('Join your team')
- add_page_specific_style 'page_bundles/signup'
- content_for :page_specific_javascripts do
= render "layouts/google_tag_manager_head"
= render "layouts/google_tag_manager_body"
%h2.center.pt-6.pb-3.gl-mb-0
= _('Join your team')
%p.gl-text-center= _('Create your own profile to collaborate with your teammates in issues, merge requests, and more.')
.signup-page
= render 'devise/shared/signup_box',
url: users_sign_up_invites_path,
button_text: _('Continue'),
show_omniauth_providers: social_signin_enabled?,
omniauth_providers_placement: :top,
suggestion_path: nil
= render 'devise/shared/sign_in_link'
---
name: invite_signup_page_interaction
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60939
rollout_issue_url: https://gitlab.com/gitlab-org/growth/team-tasks/-/issues/379
milestone: '13.12'
type: experiment
group: group::expansion
default_enabled: false
...@@ -56,6 +56,7 @@ Rails.application.routes.draw do ...@@ -56,6 +56,7 @@ Rails.application.routes.draw do
end end
resource :experience_level, only: [:show, :update] resource :experience_level, only: [:show, :update]
resources :invites, only: [:new, :create]
Gitlab.ee do Gitlab.ee do
resources :groups, only: [:new, :create] resources :groups, only: [:new, :create]
......
...@@ -9,10 +9,6 @@ module EE ...@@ -9,10 +9,6 @@ module EE
around_action :set_current_ip_address around_action :set_current_ip_address
end end
def check_if_gl_com_or_dev
render_404 unless ::Gitlab.dev_env_or_com?
end
def verify_namespace_plan_check_enabled def verify_namespace_plan_check_enabled
render_404 unless ::Gitlab::CurrentSettings.should_check_namespace_plan? render_404 unless ::Gitlab::CurrentSettings.should_check_namespace_plan?
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
class TrialRegistrationsController < RegistrationsController class TrialRegistrationsController < RegistrationsController
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
layout 'trial' layout 'simple_registration'
skip_before_action :require_no_authentication skip_before_action :require_no_authentication
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
class TrialsController < ApplicationController class TrialsController < ApplicationController
include ActionView::Helpers::SanitizeHelper include ActionView::Helpers::SanitizeHelper
layout 'trial' layout 'simple_registration'
before_action :check_if_gl_com_or_dev before_action :check_if_gl_com_or_dev
before_action :authenticate_user! before_action :authenticate_user!
......
# frozen_string_literal: true
module EE
module TrialRegistrationHelper
def social_signin_enabled?
::Gitlab.com? &&
omniauth_enabled? &&
devise_mapping.omniauthable? &&
button_based_providers_enabled?
end
end
end
...@@ -9427,6 +9427,9 @@ msgstr "" ...@@ -9427,6 +9427,9 @@ msgstr ""
msgid "Create your group" msgid "Create your group"
msgstr "" msgstr ""
msgid "Create your own profile to collaborate with your teammates in issues, merge requests, and more."
msgstr ""
msgid "Create/import your first project" msgid "Create/import your first project"
msgstr "" msgstr ""
...@@ -18807,6 +18810,9 @@ msgstr "" ...@@ -18807,6 +18810,9 @@ msgstr ""
msgid "Join Zoom meeting" msgid "Join Zoom meeting"
msgstr "" msgstr ""
msgid "Join your team"
msgstr ""
msgid "Joined %{time_ago}" msgid "Joined %{time_ago}"
msgstr "" msgstr ""
......
...@@ -127,10 +127,38 @@ RSpec.describe InvitesController do ...@@ -127,10 +127,38 @@ RSpec.describe InvitesController do
expect(flash[:notice]).to include('create an account or sign in') expect(flash[:notice]).to include('create an account or sign in')
end end
it 'is redirected to a new registration with invite email param' do context 'when it is part of our invite email experiment', :experiment, :aggregate_failures do
request let(:experience) { :control }
before do
stub_experiments(invite_signup_page_interaction: experience)
end
it 'sets originating_member_id session key' do
request
expect(session[:originating_member_id]).to eq(member.id)
end
context 'with control experience' do
it 'is redirected to a new registration with invite email param and flash message' do
request
expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email))
expect(flash[:notice]).to eq 'To accept this invitation, create an account or sign in.'
end
end
expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email)) context 'with candidate experience' do
let(:experience) { :candidate }
it 'is redirected to a new invite registration with invite email param and no flash message' do
request
expect(response).to redirect_to(new_users_sign_up_invite_path(invite_email: member.invite_email))
expect(flash[:notice]).to be_nil
end
end
end end
it 'sets session keys for auto email confirmation on sign up' do it 'sets session keys for auto email confirmation on sign up' do
......
...@@ -187,6 +187,38 @@ RSpec.describe RegistrationsController do ...@@ -187,6 +187,38 @@ RSpec.describe RegistrationsController do
end end
end end
context 'when it is part of our invite_signup_page_interaction experiment', :experiment do
let_it_be(:member) { create(:project_member, :invited, invite_email: user_params.dig(:user, :email)) }
let(:originating_member_id) { member.id }
let(:session_params) do
{
invite_email: user_params.dig(:user, :email),
originating_member_id: originating_member_id
}
end
context 'when member exists from the session key value' do
it 'tracks the experiment' do
expect(experiment(:invite_signup_page_interaction)).to track(:form_submission)
.with_context(actor: member)
.on_next_instance
subject
end
end
context 'when member does not exist from the session key value' do
let(:originating_member_id) { -1 }
it 'tracks the experiment' do
expect(experiment(:invite_signup_page_interaction)).not_to track(:form_submission)
subject
end
end
end
context 'when invite email matches email used on registration' do context 'when invite email matches email used on registration' do
let(:session_params) { { invite_email: user_params.dig(:user, :email) } } let(:session_params) { { invite_email: user_params.dig(:user, :email) } }
......
...@@ -24,13 +24,13 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -24,13 +24,13 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
visit user_confirmation_path(confirmation_token: new_user_token) visit user_confirmation_path(confirmation_token: new_user_token)
end end
def fill_in_sign_up_form(new_user) def fill_in_sign_up_form(new_user, submit_button_text = 'Register')
fill_in 'new_user_first_name', with: new_user.first_name fill_in 'new_user_first_name', with: new_user.first_name
fill_in 'new_user_last_name', with: new_user.last_name fill_in 'new_user_last_name', with: new_user.last_name
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
fill_in 'new_user_password', with: new_user.password fill_in 'new_user_password', with: new_user.password
click_button 'Register' click_button submit_button_text
end end
def fill_in_sign_in_form(user) def fill_in_sign_in_form(user)
...@@ -50,7 +50,7 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -50,7 +50,7 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
visit invite_path(group_invite.raw_invite_token) visit invite_path(group_invite.raw_invite_token)
end end
it 'renders sign in page with sign in notice' do it 'renders sign up page with sign up notice' do
expect(current_path).to eq(new_user_registration_path) expect(current_path).to eq(new_user_registration_path)
expect(page).to have_content('To accept this invitation, create an account or sign in') expect(page).to have_content('To accept this invitation, create an account or sign in')
end end
...@@ -149,30 +149,11 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -149,30 +149,11 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
end end
end end
context 'when soft email confirmation is not enabled' do it 'signs up and redirects to the group activity page with all the project/groups invitation automatically accepted' do
before do fill_in_sign_up_form(new_user)
allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 fill_in_welcome_form
end
it 'signs up and redirects to the group activity page with all the project/groups invitation automatically accepted' do
fill_in_sign_up_form(new_user)
fill_in_welcome_form
expect(current_path).to eq(activity_group_path(group))
end
end
context 'when soft email confirmation is enabled' do
before do
allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
end
it 'signs up and redirects to to the group activity page with all the project/groups invitation automatically accepted' do
fill_in_sign_up_form(new_user)
fill_in_welcome_form
expect(current_path).to eq(activity_group_path(group)) expect(current_path).to eq(activity_group_path(group))
end
end end
context 'the user sign-up using a different email address' do context 'the user sign-up using a different email address' do
...@@ -211,6 +192,57 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -211,6 +192,57 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
end end
end end
context 'with invite_signup_page_interaction experiment on', :experiment do
context 'with control experience' do
before do
stub_experiments(invite_signup_page_interaction: :control)
end
it 'lands on invite sign up page and tracks the accepted invite' do
expect(experiment(:invite_signup_page_interaction)).to track(:view)
.with_context(actor: group_invite)
.on_next_instance
visit invite_path(group_invite.raw_invite_token)
expect(current_path).to eq(new_user_registration_path)
expect(experiment(:invite_signup_page_interaction)).to track(:form_submission)
.with_context(actor: group_invite)
.on_next_instance
fill_in_sign_up_form(new_user, 'Register')
expect(current_path).to eq(users_sign_up_welcome_path)
end
end
context 'with candidate experience on .com' do
before do
allow(Gitlab).to receive(:dev_env_or_com?).and_return(true)
stub_experiments(invite_signup_page_interaction: :candidate)
end
it 'lands on invite sign up page and tracks the accepted invite' do
expect(experiment(:invite_signup_page_interaction)).to track(:view)
.with_context(actor: group_invite)
.on_next_instance
visit invite_path(group_invite.raw_invite_token)
expect(current_path).to eq(new_users_sign_up_invite_path)
expect(experiment(:invite_signup_page_interaction)).to track(:form_submission)
.with_context(actor: group_invite)
.on_next_instance
fill_in_sign_up_form(new_user, 'Continue')
expect(current_path).to eq(users_sign_up_welcome_path)
end
end
end
context 'when declining the invitation' do context 'when declining the invitation' do
context 'as an existing user' do context 'as an existing user' do
let(:group_invite) { create(:group_member, user: user, group: group, created_by: owner) } let(:group_invite) { create(:group_member, user: user, group: group, created_by: owner) }
......
...@@ -77,8 +77,8 @@ RSpec.describe AuthHelper do ...@@ -77,8 +77,8 @@ RSpec.describe AuthHelper do
end end
context 'all providers are enabled to sign in' do context 'all providers are enabled to sign in' do
it 'returns all the enabled providers from settings' do it 'returns all the enabled providers from settings in expected order' do
expect(helper.enabled_button_based_providers).to include('twitter', 'github', 'google_oauth2', 'openid_connect') expect(helper.enabled_button_based_providers).to match(%w[google_oauth2 github twitter openid_connect])
end end
it 'puts google and github in the beginning' do it 'puts google and github in the beginning' do
...@@ -99,19 +99,19 @@ RSpec.describe AuthHelper do ...@@ -99,19 +99,19 @@ RSpec.describe AuthHelper do
end end
end end
describe 'trial_enabled_button_based_providers' do describe 'popular_enabled_button_based_providers' do
it 'returns the intersection set of github & google_oauth2 with enabled providers' do it 'returns the intersection set of popular & enabled providers', :aggregate_failures do
allow(helper).to receive(:enabled_button_based_providers) { %w(twitter github google_oauth2) } allow(helper).to receive(:enabled_button_based_providers) { %w(twitter github google_oauth2) }
expect(helper.trial_enabled_button_based_providers).to eq(%w(github google_oauth2)) expect(helper.popular_enabled_button_based_providers).to eq(%w(github google_oauth2))
allow(helper).to receive(:enabled_button_based_providers) { %w(google_oauth2 bitbucket) } allow(helper).to receive(:enabled_button_based_providers) { %w(google_oauth2 bitbucket) }
expect(helper.trial_enabled_button_based_providers).to eq(%w(google_oauth2)) expect(helper.popular_enabled_button_based_providers).to eq(%w(google_oauth2))
allow(helper).to receive(:enabled_button_based_providers) { %w(bitbucket) } allow(helper).to receive(:enabled_button_based_providers) { %w(bitbucket) }
expect(helper.trial_enabled_button_based_providers).to be_empty expect(helper.popular_enabled_button_based_providers).to be_empty
end end
end end
......
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe EE::TrialRegistrationHelper do RSpec.describe RegistrationsHelper do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
describe '#social_signin_enabled?' do describe '#social_signin_enabled?' do
before do before do
allow(::Gitlab).to receive(:com?).and_return(com) allow(::Gitlab).to receive(:dev_env_or_com?).and_return(com)
allow(view).to receive(:omniauth_enabled?).and_return(omniauth_enabled) allow(view).to receive(:omniauth_enabled?).and_return(omniauth_enabled)
allow(view).to receive(:button_based_providers_enabled?).and_return(button_based_providers_enabled) allow(view).to receive(:button_based_providers_enabled?).and_return(button_based_providers_enabled)
allow(view).to receive(:devise_mapping).and_return(double(omniauthable?: omniauthable)) allow(view).to receive(:devise_mapping).and_return(double(omniauthable?: omniauthable))
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Registering from an invite' do
let(:com) { true }
before do
allow(Gitlab).to receive(:dev_env_or_com?).and_return(com)
end
describe 'GET /users/sign_up/invites/new' do
subject(:request) { get '/users/sign_up/invites/new' }
context 'when on .com' do
it 'renders the template with expected text', :aggregate_failures do
request
expect(response).to render_template('layouts/simple_registration')
expect(response).to render_template(:new)
expect(response.body).to include('Join your team')
end
end
context 'when not on .com' do
let(:com) { false }
it 'returns not found' do
request
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
describe 'POST /users/sign_up/invites' do
subject(:request) do
post '/users/sign_up/invites',
params: {
user: {
first_name: 'first',
last_name: 'last',
username: 'new_username',
email: 'new@user.com',
password: 'Any_password'
}
}
end
context 'when on .com' do
it 'creates a user' do
expect { request }.to change(User, :count).by(1)
expect(response).to have_gitlab_http_status(:found)
end
end
context 'when not on .com' do
let(:com) { false }
it 'returns not found' do
request
expect(response).to have_gitlab_http_status(:not_found)
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