Commit e7241987 authored by Nicolas Dular's avatar Nicolas Dular

Remove accept terms checkbox for signup

To remove obstacles in our signup process, we now only render a text
that states that the user accepts the terms of service when clicking
register instead of checking the checkbox.
parent ce117b05
......@@ -13,16 +13,12 @@ class RegistrationsController < Devise::RegistrationsController
skip_before_action :required_signup_info, :check_two_factor_requirement, only: [:welcome, :update_registration]
prepend_before_action :check_captcha, only: :create
before_action :whitelist_query_limiting, :ensure_destroy_prerequisites_met, only: [:destroy]
before_action :ensure_terms_accepted,
if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? }
before_action :load_recaptcha, only: :new
feature_category :authentication_and_authorization
def new
if experiment_enabled?(:signup_flow)
track_experiment_event(:terms_opt_in, 'start')
@resource = build_resource
else
redirect_to new_user_session_path(anchor: 'register-pane')
......@@ -36,7 +32,6 @@ class RegistrationsController < Devise::RegistrationsController
super do |new_user|
persist_accepted_terms_if_required(new_user)
set_role_required(new_user)
track_terms_experiment(new_user)
yield new_user if block_given?
end
......@@ -86,10 +81,8 @@ class RegistrationsController < Devise::RegistrationsController
return unless new_user.persisted?
return unless Gitlab::CurrentSettings.current_application_settings.enforce_terms?
if terms_accepted?
terms = ApplicationSetting::Term.latest
Users::RespondToTermsService.new(new_user, terms).execute(accepted: true)
end
terms = ApplicationSetting::Term.latest
Users::RespondToTermsService.new(new_user, terms).execute(accepted: true)
end
def set_role_required(new_user)
......@@ -185,18 +178,6 @@ class RegistrationsController < Devise::RegistrationsController
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42380')
end
def ensure_terms_accepted
return if terms_accepted?
redirect_to new_user_session_path, alert: _('You must accept our Terms of Service and privacy policy in order to register an account')
end
def terms_accepted?
return true if experiment_enabled?(:terms_opt_in)
Gitlab::Utils.to_boolean(params[:terms_opt_in])
end
def path_for_signed_in_user(user)
if requires_confirmation?(user)
users_almost_there_path
......@@ -213,13 +194,6 @@ class RegistrationsController < Devise::RegistrationsController
true
end
def track_terms_experiment(new_user)
return unless new_user.persisted?
track_experiment_event(:terms_opt_in, 'end')
record_experiment_user(:terms_opt_in)
end
def load_recaptcha
Gitlab::Recaptcha.load_configurations!
end
......
......@@ -28,21 +28,12 @@
= f.label :password, class: 'label-bold'
= f.password_field :password, class: "form-control bottom", data: { qa_selector: 'new_user_password_field' }, required: true, pattern: ".{#{@minimum_password_length},}", title: _("Minimum length is %{minimum_password_length} characters.") % { minimum_password_length: @minimum_password_length }
%p.gl-field-hint.text-secondary= _('Minimum length is %{minimum_password_length} characters') % { minimum_password_length: @minimum_password_length }
- if Gitlab::CurrentSettings.current_application_settings.enforce_terms? && !experiment_enabled?(:terms_opt_in)
.form-group
= check_box_tag :terms_opt_in, '1', false, required: true, data: { qa_selector: 'new_user_accept_terms_checkbox' }
= label_tag :terms_opt_in do
- terms_link = link_to s_("I accept the|Terms of Service and Privacy Policy"), terms_path, target: "_blank"
- accept_terms_label = _("I accept the %{terms_link}") % { terms_link: terms_link }
= accept_terms_label.html_safe
= render_if_exists 'devise/shared/email_opted_in', f: f
%div
- if show_recaptcha_sign_up?
= recaptcha_tags
.submit-container.mt-3
= f.submit _("Register"), class: "btn-register btn btn-block btn-success mb-0 p-2", data: { qa_selector: 'new_user_register_button' }
- if experiment_enabled?(:terms_opt_in)
%p.gl-text-gray-500.gl-mt-5.gl-mb-0
= html_escape(_("By clicking Register, I agree that I have read and accepted the GitLab %{linkStart}Terms of Use and Privacy Policy%{linkEnd}")) % { linkStart: "<a href='#{terms_path}' target='_blank' rel='noreferrer noopener'>".html_safe, linkEnd: '</a>'.html_safe }
= render 'devise/shared/terms_of_service_notice'
- if omniauth_enabled? && button_based_providers_enabled?
= render 'devise/shared/experimental_separate_sign_up_flow_omniauth_box'
......@@ -28,16 +28,10 @@
= f.label :password, class: 'label-bold'
= f.password_field :password, class: "form-control bottom", data: { qa_selector: 'new_user_password_field' }, required: true, pattern: ".{#{@minimum_password_length},}", title: _("Minimum length is %{minimum_password_length} characters.") % { minimum_password_length: @minimum_password_length }
%p.gl-field-hint.text-secondary= _('Minimum length is %{minimum_password_length} characters') % { minimum_password_length: @minimum_password_length }
- if Gitlab::CurrentSettings.current_application_settings.enforce_terms?
.form-group
= check_box_tag :terms_opt_in, '1', false, required: true, data: { qa_selector: 'new_user_accept_terms_checkbox' }
= label_tag :terms_opt_in do
- terms_link = link_to s_("I accept the|Terms of Service and Privacy Policy"), terms_path, target: "_blank"
- accept_terms_label = _("I accept the %{terms_link}") % { terms_link: terms_link }
= accept_terms_label.html_safe
= render_if_exists 'devise/shared/email_opted_in', f: f
%div
- if show_recaptcha_sign_up?
= recaptcha_tags
.submit-container
= f.submit _("Register"), class: "btn-register btn", data: { qa_selector: 'new_user_register_button' }
= render 'devise/shared/terms_of_service_notice'
- company_name = Gitlab.com? ? 'GitLab' : ''
- if Gitlab::CurrentSettings.current_application_settings.enforce_terms?
%p.gl-text-gray-500.gl-mt-5.gl-mb-0
= html_escape(_("By clicking Register, I agree that I have read and accepted the %{company_name} %{linkStart}Terms of Use and Privacy Policy%{linkEnd}")) % { linkStart: "<a href='#{terms_path}' target='_blank' rel='noreferrer noopener'>".html_safe, linkEnd: '</a>'.html_safe, company_name: company_name }
---
title: Remove accept terms checkbox for signup
merge_request: 42581
author:
type: changed
......@@ -27,11 +27,6 @@
= f.label :password, for: 'new_user_password', class: 'label-bold'
= f.password_field :password, class: 'form-control bottom', data: { qa_selector: 'new_user_password_field' }, required: true, pattern: ".{#{@minimum_password_length},}", title: _("Minimum length is %{minimum_password_length} characters.") % { minimum_password_length: @minimum_password_length }
%p.gl-field-hint.text-secondary= _('Minimum length is %{minimum_password_length} characters') % { minimum_password_length: @minimum_password_length }
.form-group
= check_box_tag :terms_opt_in, '1', false, required: true, data: { qa_selector: 'new_user_accept_terms_checkbox' }
= label_tag :terms_opt_in, for: 'terms_opt_in', class: 'form-check-label' do
- terms_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: terms_path }
= _("I accept the %{terms_link_start}Terms of Service and Privacy Policy%{terms_link_end}").html_safe % { terms_link_start: terms_link_start, terms_link_end: '</a>'.html_safe }
.form-group
= f.check_box :email_opted_in, data: { qa_selector: 'new_user_email_opted_in_checkbox' }
= f.label :email_opted_in, _("I'd like to receive updates via email about GitLab"), class: 'form-check-label'
......@@ -40,3 +35,4 @@
= recaptcha_tags
.submit-container
= f.submit _("Continue"), class: "btn-register btn", data: { qa_selector: 'new_user_register_button' }
= render 'devise/shared/terms_of_service_notice'
......@@ -37,8 +37,6 @@ RSpec.describe 'Trial Sign Up', :js do
fill_in 'new_user_email', with: user_attrs[:email]
fill_in 'new_user_password', with: user_attrs[:password]
check 'terms_opt_in'
click_button 'Continue'
end
......
......@@ -51,9 +51,6 @@ module Gitlab
new_create_project_ui: {
tracking_category: 'Manage::Import::Experiment::NewCreateProjectUi'
},
terms_opt_in: {
tracking_category: 'Growth::Acquisition::Experiment::TermsOptIn'
},
contact_sales_btn_in_app: {
tracking_category: 'Growth::Conversion::Experiment::ContactSalesInApp'
},
......
......@@ -4433,7 +4433,7 @@ msgstr ""
msgid "By URL"
msgstr ""
msgid "By clicking Register, I agree that I have read and accepted the GitLab %{linkStart}Terms of Use and Privacy Policy%{linkEnd}"
msgid "By clicking Register, I agree that I have read and accepted the %{company_name} %{linkStart}Terms of Use and Privacy Policy%{linkEnd}"
msgstr ""
msgid "By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format."
......@@ -13285,9 +13285,6 @@ msgstr ""
msgid "However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation."
msgstr ""
msgid "I accept the %{terms_link_start}Terms of Service and Privacy Policy%{terms_link_end}"
msgstr ""
msgid "I accept the %{terms_link}"
msgstr ""
......@@ -29781,9 +29778,6 @@ msgstr ""
msgid "You may close the milestone now."
msgstr ""
msgid "You must accept our Terms of Service and privacy policy in order to register an account"
msgstr ""
msgid "You must be logged in to search across all of GitLab"
msgstr ""
......
......@@ -11,7 +11,6 @@ module QA
element :new_user_email_field
element :new_user_password_field
element :new_user_register_button
element :new_user_accept_terms_checkbox
end
view 'app/views/registrations/welcome.html.haml' do
......@@ -25,8 +24,6 @@ module QA
fill_element :new_user_email_field, user.email
fill_element :new_user_password_field, user.password
check_element :new_user_accept_terms_checkbox if has_element?(:new_user_accept_terms_checkbox)
signed_in = retry_until do
click_element :new_user_register_button if has_element?(:new_user_register_button)
......
......@@ -37,46 +37,6 @@ RSpec.describe RegistrationsController do
expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane'))
end
end
context 'with sign up flow and terms_opt_in experiment being enabled' do
before do
stub_experiment(signup_flow: true, terms_opt_in: true)
end
context 'when user is not part of the experiment' do
before do
stub_experiment_for_user(signup_flow: true, terms_opt_in: false)
end
it 'tracks event with right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::TermsOptIn',
'start',
label: anything,
property: 'control_group'
)
subject
end
end
context 'when user is part of the experiment' do
before do
stub_experiment_for_user(signup_flow: true, terms_opt_in: true)
end
it 'tracks event with right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::TermsOptIn',
'start',
label: anything,
property: 'experimental_group'
)
subject
end
end
end
end
describe '#create' do
......@@ -353,100 +313,26 @@ RSpec.describe RegistrationsController do
end
end
context 'when terms are enforced' do
before do
enforce_terms
end
it 'redirects back with a notice when the checkbox was not checked' do
subject
expect(flash[:alert]).to eq(_('You must accept our Terms of Service and privacy policy in order to register an account'))
end
it 'creates the user with agreement when terms are accepted' do
post :create, params: user_params.merge(terms_opt_in: '1')
expect(controller.current_user).to be_present
expect(controller.current_user.terms_accepted?).to be(true)
end
context 'when experiment terms_opt_in is enabled' do
context 'terms of service' do
context 'when terms are enforced' do
before do
stub_experiment(terms_opt_in: true)
enforce_terms
end
context 'when user is part of the experiment' do
before do
stub_experiment_for_user(terms_opt_in: true)
end
it 'creates the user with accepted terms' do
subject
expect(controller.current_user).to be_present
expect(controller.current_user.terms_accepted?).to be(true)
end
end
context 'when user is not part of the experiment' do
before do
stub_experiment_for_user(terms_opt_in: false)
end
it 'creates the user without accepted terms' do
subject
it 'creates the user with accepted terms' do
subject
expect(flash[:alert]).to eq(_('You must accept our Terms of Service and privacy policy in order to register an account'))
end
expect(controller.current_user).to be_present
expect(controller.current_user.terms_accepted?).to be(true)
end
end
end
describe 'tracking data' do
context 'with sign up flow and terms_opt_in experiment being enabled' do
before do
stub_experiment(signup_flow: true, terms_opt_in: true)
end
it 'records user for the terms_opt_in experiment' do
expect(controller).to receive(:record_experiment_user).with(:terms_opt_in)
context 'when terms are not enforced' do
it 'creates the user without accepted terms' do
subject
end
context 'when user is not part of the experiment' do
before do
stub_experiment_for_user(signup_flow: true, terms_opt_in: false)
end
it 'tracks event with right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::TermsOptIn',
'end',
label: anything,
property: 'control_group'
)
subject
end
end
context 'when user is part of the experiment' do
before do
stub_experiment_for_user(signup_flow: true, terms_opt_in: true)
end
it 'tracks event with right parameters' do
expect(Gitlab::Tracking).to receive(:event).with(
'Growth::Acquisition::Experiment::TermsOptIn',
'end',
label: anything,
property: 'experimental_group'
)
subject
end
expect(controller.current_user).to be_present
expect(controller.current_user.terms_accepted?).to be(false)
end
end
end
......
......@@ -222,22 +222,13 @@ RSpec.shared_examples 'Signup' do
enforce_terms
end
it 'requires the user to check the checkbox' do
it 'renders text that the user confirms terms by clicking register' do
visit new_user_registration_path
fill_in_signup_form
click_button 'Register'
expect(current_path).to eq new_user_session_path
expect(page).to have_content(/you must accept our terms of service/i)
end
it 'asks the user to accept terms before going to the dashboard' do
visit new_user_registration_path
expect(page).to have_content(/By clicking Register, I agree that I have read and accepted the Terms of Use and Privacy Policy/)
fill_in_signup_form
check :terms_opt_in
click_button "Register"
click_button 'Register'
expect(current_path).to eq users_sign_up_welcome_path
end
......@@ -364,28 +355,4 @@ RSpec.describe 'With experimental flow' do
it_behaves_like 'Signup'
it_behaves_like 'Signup name validation', 'new_user_first_name', 127, 'First name'
it_behaves_like 'Signup name validation', 'new_user_last_name', 127, 'Last name'
context 'when terms_opt_in experimental is enabled' do
include TermsHelper
before do
enforce_terms
stub_experiment(signup_flow: true, terms_opt_in: true)
stub_experiment_for_user(signup_flow: true, terms_opt_in: true)
end
it 'terms are checked by default' do
new_user = build_stubbed(:user)
visit new_user_registration_path
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_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email
fill_in 'new_user_password', with: new_user.password
click_button 'Register'
expect(current_path).to eq users_sign_up_welcome_path
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