diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 139ef5e0692d82b589d6552f8c30a871d3a976ec..037c83b062b980426eafe148bbe1f4930108aa5f 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -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 @@ def create
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 @@ def persist_accepted_terms_if_required(new_user)
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 @@ def whitelist_query_limiting
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 @@ def requires_confirmation?(user)
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
diff --git a/app/views/devise/shared/_experimental_separate_sign_up_flow_box.html.haml b/app/views/devise/shared/_experimental_separate_sign_up_flow_box.html.haml
index ecabb4495cce0f6fee2a49b31c1da5425a3a679c..621bbb32a1394d53443e63e5c613e475503e00f2 100644
--- a/app/views/devise/shared/_experimental_separate_sign_up_flow_box.html.haml
+++ b/app/views/devise/shared/_experimental_separate_sign_up_flow_box.html.haml
@@ -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'
diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml
index 1fd81cdbac2625c53311a3083e05aaaac3178e9f..f4ac9ad696bd59fd026c0069085088d34e660614 100644
--- a/app/views/devise/shared/_signup_box.html.haml
+++ b/app/views/devise/shared/_signup_box.html.haml
@@ -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'
diff --git a/app/views/devise/shared/_terms_of_service_notice.html.haml b/app/views/devise/shared/_terms_of_service_notice.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..46b043b28318056a7d136d4ece4f2d81530ae93d
--- /dev/null
+++ b/app/views/devise/shared/_terms_of_service_notice.html.haml
@@ -0,0 +1,5 @@
+- 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 }
diff --git a/changelogs/unreleased/nicolasdular-remove-tos-checkobx.yml b/changelogs/unreleased/nicolasdular-remove-tos-checkobx.yml
new file mode 100644
index 0000000000000000000000000000000000000000..704bc965ff4bff8881ee08d8bf22998fc9441325
--- /dev/null
+++ b/changelogs/unreleased/nicolasdular-remove-tos-checkobx.yml
@@ -0,0 +1,5 @@
+---
+title: Remove accept terms checkbox for signup
+merge_request: 42581
+author:
+type: changed
diff --git a/ee/app/views/trial_registrations/_signup_box.html.haml b/ee/app/views/trial_registrations/_signup_box.html.haml
index dc51f3860158c29f4ee5b7ef30aa75e41727377a..ec880154ba5d06f90df689b1a63342c145b75661 100644
--- a/ee/app/views/trial_registrations/_signup_box.html.haml
+++ b/ee/app/views/trial_registrations/_signup_box.html.haml
@@ -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'
diff --git a/ee/spec/features/trial_registrations/signup_spec.rb b/ee/spec/features/trial_registrations/signup_spec.rb
index 21c3c1eea0b707ab43a56eb01c0b2b86967f4eb5..7bb68a23061351861115ea87a70c400be781cf9b 100644
--- a/ee/spec/features/trial_registrations/signup_spec.rb
+++ b/ee/spec/features/trial_registrations/signup_spec.rb
@@ -37,8 +37,6 @@
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
diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb
index 4457c65c519faa6b3769af55557f671dcab20c00..229caebc268d34f8f5de45ff2bf025c2e57239e5 100644
--- a/lib/gitlab/experimentation.rb
+++ b/lib/gitlab/experimentation.rb
@@ -51,9 +51,6 @@ module Experimentation
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'
},
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8d38319762e2e25bcbe9666a0711f24b3eed58be..663ed9d4a3cd54adc72ceea9ebf01dc050e7c4e5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -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 ""
diff --git a/qa/qa/page/main/sign_up.rb b/qa/qa/page/main/sign_up.rb
index 8be3b2291b608775bd03c3c1849cf036dc795e1d..98bbbc530278af274797cf0f87ebafeb61bdeb52 100644
--- a/qa/qa/page/main/sign_up.rb
+++ b/qa/qa/page/main/sign_up.rb
@@ -11,7 +11,6 @@ class SignUp < Page::Base
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 @@ def sign_up!(user)
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)
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index c3080b50f443dc468fad51a221abcb75e757bda3..cca453fd0754d2f12ff194528f0fb84fa67e27fd 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -37,46 +37,6 @@
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 @@
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
diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb
index 0ede19810d9ff3f53faef5dd111defaa7e54241c..c59121626f034f850f26c47ca453d0a39a71bbb5 100644
--- a/spec/features/users/signup_spec.rb
+++ b/spec/features/users/signup_spec.rb
@@ -222,22 +222,13 @@ def fill_in_signup_form
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 @@ def fill_in_signup_form
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