From 58a1c31fbba9ef112a17aa974ca92396be4bf25c Mon Sep 17 00:00:00 2001 From: Nicolas Dular <ndular@gitlab.com> Date: Fri, 4 Sep 2020 15:16:57 +0000 Subject: [PATCH] Merge CE and EE welcome page To not let these two pages diverge (which already happened) we want to merge them and only embedd the parts from EE that are necessery. For self managed, we also remove the question if the user sets up the account for their company, since this only makes sense for Gitlab.com where it is more likely that someone starts a group and is not already using the GitLab instance soley for their company. --- app/services/users/signup_service.rb | 2 +- app/views/registrations/welcome.html.haml | 48 ++++++++++--------- ee/app/views/registrations/welcome.html.haml | 34 ------------- .../registrations/welcome/_button.html.haml | 4 ++ .../welcome/_progress_bar.html.haml | 5 ++ .../welcome/_setup_for_company.html.haml | 11 +++++ .../user_sees_new_onboarding_flow_spec.rb | 2 +- .../features/registrations/welcome_spec.rb | 2 +- ee/spec/features/signup_spec.rb | 30 ++++++++++++ .../registrations/welcome.html.haml_spec.rb | 1 + locale/gitlab.pot | 11 +---- spec/features/users/signup_spec.rb | 14 +++--- spec/services/users/signup_service_spec.rb | 25 ++++++++-- .../registrations/welcome.html.haml_spec.rb | 26 ++++++++++ 14 files changed, 133 insertions(+), 82 deletions(-) delete mode 100644 ee/app/views/registrations/welcome.html.haml create mode 100644 ee/app/views/registrations/welcome/_button.html.haml create mode 100644 ee/app/views/registrations/welcome/_progress_bar.html.haml create mode 100644 ee/app/views/registrations/welcome/_setup_for_company.html.haml create mode 100644 spec/views/registrations/welcome.html.haml_spec.rb diff --git a/app/services/users/signup_service.rb b/app/services/users/signup_service.rb index 1031cec44cb1d..1087ae76216e5 100644 --- a/app/services/users/signup_service.rb +++ b/app/services/users/signup_service.rb @@ -27,7 +27,7 @@ def assign_attributes def inject_validators class << @user validates :role, presence: true - validates :setup_for_company, inclusion: { in: [true, false], message: :blank } + validates :setup_for_company, inclusion: { in: [true, false], message: :blank } if Gitlab.com? end end end diff --git a/app/views/registrations/welcome.html.haml b/app/views/registrations/welcome.html.haml index ef3e0b1b4c00a..bdde5de0f6174 100644 --- a/app/views/registrations/welcome.html.haml +++ b/app/views/registrations/welcome.html.haml @@ -1,22 +1,26 @@ -- content_for(:page_title, _('Welcome to GitLab %{name}!') % { name: current_user.name }) -.text-center.mb-3 - = html_escape(_('In order to tailor your experience with GitLab we%{br_tag}would like to know a bit more about you.')) % { br_tag: '<br/>'.html_safe } -.signup-box.p-3.mb-2 - .signup-body - = form_for(current_user, url: users_sign_up_update_registration_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 - .form-group - = f.label :role, _('Role'), class: 'label-bold' - = f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'form-control' - .form-group - = f.label :setup_for_company, _('Are you setting up GitLab for a company?'), class: 'label-bold' - .d-flex.justify-content-center - .w-25 - = f.radio_button :setup_for_company, true - = f.label :setup_for_company, _('Yes'), value: 'true' - .w-25 - = f.radio_button :setup_for_company, false - = f.label :setup_for_company, _('No'), value: 'false' - .submit-container.mt-3 - = f.submit _('Get started!'), class: 'btn-register btn btn-block mb-0 p-2' +- page_title _('Your profile') + +.row.gl-flex-grow-1.gl-bg-gray-10 + .d-flex.gl-flex-direction-column.gl-align-items-center.gl-w-full.gl-p-5 + .edit-profile.login-page.d-flex.flex-column.gl-align-items-center.pt-lg-3 + = render_if_exists "registrations/welcome/progress_bar" + %h2.gl-text-center= html_escape(_('Welcome to GitLab%{br_tag}%{name}!')) % { name: html_escape(current_user.first_name), br_tag: '<br/>'.html_safe } + %p + .gl-text-center= html_escape(_('In order to personalize your experience with GitLab%{br_tag}we would like to know a bit more about you.')) % { br_tag: '<br/>'.html_safe } + + = form_for(current_user, url: users_sign_up_update_registration_path, html: { class: 'card gl-w-full! gl-p-5', 'aria-live' => 'assertive' }) do |f| + .devise-errors + = render 'devise/shared/error_messages', resource: current_user + .row + .form-group.col-sm-12 + = f.label :role, _('Role'), class: 'label-bold' + = f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'form-control', autofocus: true + .form-text.gl-text-gray-500.gl-mt-3= _('This will help us personalize your onboarding experience.') + = render_if_exists "registrations/welcome/setup_for_company", f: f + .row + .form-group.col-sm-12.gl-mb-0 + - if partial_exists? "registrations/welcome/button" + = render "registrations/welcome/button" + - else + = f.submit _('Get started!'), class: 'btn-register btn btn-block gl-mb-0 gl-p-3' + diff --git a/ee/app/views/registrations/welcome.html.haml b/ee/app/views/registrations/welcome.html.haml deleted file mode 100644 index c00924771dec1..0000000000000 --- a/ee/app/views/registrations/welcome.html.haml +++ /dev/null @@ -1,34 +0,0 @@ -- page_title _('Your profile') -- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues) - -.row.flex-grow-1.bg-gray-light - .d-flex.flex-column.align-items-center.w-100.gl-p-5 - .edit-profile.login-page.d-flex.flex-column.align-items-center.pt-lg-3 - - if in_subscription_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow? && !in_trial_flow?) - #progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled.to_s } } - %h2.center= html_escape(_('Welcome to GitLab.com%{br_tag}@%{name}!')) % { name: html_escape(current_user.first_name), br_tag: '<br/>'.html_safe } - %p - .center= html_escape(_('In order to personalize your experience with GitLab%{br_tag}we would like to know a bit more about you.')) % { br_tag: '<br/>'.html_safe } - - = form_for(current_user, url: users_sign_up_update_registration_path, html: { class: 'card w-100 gl-p-5', 'aria-live' => 'assertive' }) do |f| - .devise-errors - = render 'devise/shared/error_messages', resource: current_user - .row - .form-group.col-sm-12 - = f.label :role, _('Role'), class: 'label-bold' - = f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'form-control input-lg', autofocus: true - .form-text.text-muted= _('This will help us personalize your onboarding experience.') - .row - .form-group.col-sm-12 - = f.label :setup_for_company, setup_for_company_label_text, class: 'label-bold' - .d-flex.flex-column.flex-lg-row - .flex-grow-1 - = f.radio_button :setup_for_company, true - = f.label :setup_for_company, _('My company or team'), class: 'normal', value: 'true' - .flex-grow-1 - = f.radio_button :setup_for_company, false - = f.label :setup_for_company, _('Just me'), class: 'normal', value: 'false' - .row - .form-group.col-sm-12.mb-0 - = button_tag class: %w[btn btn-success w-100] do - = in_subscription_flow? || in_trial_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow?) ? _('Continue') : _('Get started!') diff --git a/ee/app/views/registrations/welcome/_button.html.haml b/ee/app/views/registrations/welcome/_button.html.haml new file mode 100644 index 0000000000000..e400161e2c3cf --- /dev/null +++ b/ee/app/views/registrations/welcome/_button.html.haml @@ -0,0 +1,4 @@ +- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues) + += button_tag class: %w[btn btn-success w-100] do + = in_subscription_flow? || in_trial_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow?) ? _('Continue') : _('Get started!') diff --git a/ee/app/views/registrations/welcome/_progress_bar.html.haml b/ee/app/views/registrations/welcome/_progress_bar.html.haml new file mode 100644 index 0000000000000..3bbea356570b4 --- /dev/null +++ b/ee/app/views/registrations/welcome/_progress_bar.html.haml @@ -0,0 +1,5 @@ +- onboarding_issues_experiment_enabled = experiment_enabled?(:onboarding_issues) + +- if in_subscription_flow? || (onboarding_issues_experiment_enabled && !in_invitation_flow? && !in_oauth_flow? && !in_trial_flow?) + #progress-bar{ data: { is_in_subscription_flow: in_subscription_flow?.to_s, is_onboarding_issues_experiment_enabled: onboarding_issues_experiment_enabled.to_s } } + diff --git a/ee/app/views/registrations/welcome/_setup_for_company.html.haml b/ee/app/views/registrations/welcome/_setup_for_company.html.haml new file mode 100644 index 0000000000000..86b43a6799277 --- /dev/null +++ b/ee/app/views/registrations/welcome/_setup_for_company.html.haml @@ -0,0 +1,11 @@ +- if Gitlab.com? + .row + .form-group.col-sm-12 + = f.label :setup_for_company, setup_for_company_label_text, class: 'label-bold' + .d-flex.flex-column.flex-lg-row + .flex-grow-1 + = f.radio_button :setup_for_company, true + = f.label :setup_for_company, _('My company or team'), class: 'normal', value: 'true' + .flex-grow-1 + = f.radio_button :setup_for_company, false + = f.label :setup_for_company, _('Just me'), class: 'normal', value: 'false' diff --git a/ee/spec/features/registrations/user_sees_new_onboarding_flow_spec.rb b/ee/spec/features/registrations/user_sees_new_onboarding_flow_spec.rb index 6715e0207b928..3dc44d7257296 100644 --- a/ee/spec/features/registrations/user_sees_new_onboarding_flow_spec.rb +++ b/ee/spec/features/registrations/user_sees_new_onboarding_flow_spec.rb @@ -12,7 +12,7 @@ end it 'shows the expected pages' do - expect(page).to have_content('Welcome to GitLab.com') + expect(page).to have_content('Welcome to GitLab') expect(page).to have_content('Your profile Your GitLab group Your first project') expect(page).to have_css('li.current', text: 'Your profile') diff --git a/ee/spec/features/registrations/welcome_spec.rb b/ee/spec/features/registrations/welcome_spec.rb index f454a81b03f7b..799b0bfd65d65 100644 --- a/ee/spec/features/registrations/welcome_spec.rb +++ b/ee/spec/features/registrations/welcome_spec.rb @@ -23,7 +23,7 @@ end it 'shows the welcome page without a progress bar' do - expect(page).to have_content('Welcome to GitLab.com') + expect(page).to have_content('Welcome to GitLab') expect(page).not_to have_content('Your profile') end diff --git a/ee/spec/features/signup_spec.rb b/ee/spec/features/signup_spec.rb index 4b7c08fcbf455..d8984f1dd5079 100644 --- a/ee/spec/features/signup_spec.rb +++ b/ee/spec/features/signup_spec.rb @@ -48,6 +48,36 @@ expect(user.email_opted_in_at).to be_nil end end + + context 'when role is required' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + end + + it 'redirects to step 2 of the signup process, sets the role and setup for company and redirects back' do + visit new_user_registration_path + + fill_in 'new_user_first_name', with: user_attrs[:name].split(' ').first + fill_in 'new_user_last_name', with: user_attrs[:name].split(' ').last + fill_in 'new_user_username', with: user_attrs[:username] + fill_in 'new_user_email', with: user_attrs[:email] + fill_in 'new_user_password', with: user_attrs[:password] + click_button 'Register' + visit new_project_path + + expect(page).to have_current_path(users_sign_up_welcome_path) + + select 'Software Developer', from: 'user_role' + choose 'user_setup_for_company_true' + click_button 'Get started!' + user = User.find_by_username(user_attrs[:username]) + + expect(user.software_developer_role?).to be_truthy + expect(user.setup_for_company).to be_truthy + expect(page).to have_current_path(new_project_path) + end + end end context 'not for Gitlab.com' do diff --git a/ee/spec/views/registrations/welcome.html.haml_spec.rb b/ee/spec/views/registrations/welcome.html.haml_spec.rb index 1d7048f6ff56d..349d186c54471 100644 --- a/ee/spec/views/registrations/welcome.html.haml_spec.rb +++ b/ee/spec/views/registrations/welcome.html.haml_spec.rb @@ -14,6 +14,7 @@ allow(view).to receive(:in_invitation_flow?).and_return(in_invitation_flow) allow(view).to receive(:in_oauth_flow?).and_return(in_oauth_flow) allow(view).to receive(:experiment_enabled?).with(:onboarding_issues).and_return(onboarding_issues_experiment_enabled) + allow(Gitlab).to receive(:com?).and_return(true) render end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index dc3e6580b26e9..14d275f0be1ff 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3214,9 +3214,6 @@ msgstr "" msgid "Are you ABSOLUTELY SURE you wish to delete this project?" msgstr "" -msgid "Are you setting up GitLab for a company?" -msgstr "" - msgid "Are you sure that you want to archive this project?" msgstr "" @@ -13136,9 +13133,6 @@ msgstr "" msgid "In order to personalize your experience with GitLab%{br_tag}we would like to know a bit more about you." msgstr "" -msgid "In order to tailor your experience with GitLab we%{br_tag}would like to know a bit more about you." -msgstr "" - msgid "In progress" msgstr "" @@ -27982,15 +27976,12 @@ msgstr "" msgid "Welcome to GitLab" msgstr "" -msgid "Welcome to GitLab %{name}!" +msgid "Welcome to GitLab%{br_tag}%{name}!" msgstr "" msgid "Welcome to GitLab, %{first_name}!" msgstr "" -msgid "Welcome to GitLab.com%{br_tag}@%{name}!" -msgstr "" - msgid "Welcome to the guided GitLab tour" msgstr "" diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 332be05502778..522ad708f427b 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -485,8 +485,8 @@ it_behaves_like 'Signup name validation', 'new_user_first_name', 127 it_behaves_like 'Signup name validation', 'new_user_last_name', 127 - describe 'when role is required' do - it 'after registering, it redirects to step 2 of the signup process, sets the name and role and then redirects to the original requested url' do + context 'when role is required' do + it 'redirects to step 2 of the signup process, sets the role and redirects back' do new_user = build_stubbed(:user) visit new_user_registration_path fill_in 'new_user_first_name', with: new_user.first_name @@ -500,12 +500,11 @@ expect(page).to have_current_path(users_sign_up_welcome_path) select 'Software Developer', from: 'user_role' - choose 'user_setup_for_company_true' click_button 'Get started!' new_user = User.find_by_username(new_user.username) expect(new_user.software_developer_role?).to be_truthy - expect(new_user.setup_for_company).to be_truthy + expect(new_user.setup_for_company).to be_nil expect(page).to have_current_path(new_project_path) end end @@ -521,14 +520,13 @@ it 'terms are checked by default' do new_user = build_stubbed(:user) - visit new_user_registration_path - fill_in 'new_user_username', with: new_user.username - fill_in 'new_user_email', with: new_user.email + 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 diff --git a/spec/services/users/signup_service_spec.rb b/spec/services/users/signup_service_spec.rb index cc234309817a2..7169401ab34c3 100644 --- a/spec/services/users/signup_service_spec.rb +++ b/spec/services/users/signup_service_spec.rb @@ -48,12 +48,27 @@ expect(user.reload.setup_for_company).to be(false) end - it 'returns an error result when setup_for_company is missing' do - result = update_user(user, setup_for_company: '') + context 'when on .com' do + before do + allow(Gitlab).to receive(:com?).and_return(true) + end - expect(user.reload.setup_for_company).not_to be_blank - expect(result[:status]).to eq(:error) - expect(result[:message]).to eq("Setup for company can't be blank") + it 'returns an error result when setup_for_company is missing' do + result = update_user(user, setup_for_company: '') + + expect(user.reload.setup_for_company).not_to be_blank + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Setup for company can't be blank") + end + end + + context 'when not on .com' do + it 'returns success when setup_for_company is blank' do + result = update_user(user, setup_for_company: '') + + expect(result).to eq(status: :success) + expect(user.reload.setup_for_company).to be(nil) + end end end diff --git a/spec/views/registrations/welcome.html.haml_spec.rb b/spec/views/registrations/welcome.html.haml_spec.rb new file mode 100644 index 0000000000000..56a7784a1344c --- /dev/null +++ b/spec/views/registrations/welcome.html.haml_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'registrations/welcome' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:user) { User.new } + + before do + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:in_subscription_flow?).and_return(false) + allow(view).to receive(:in_trial_flow?).and_return(false) + allow(view).to receive(:in_invitation_flow?).and_return(false) + allow(view).to receive(:in_oauth_flow?).and_return(false) + allow(view).to receive(:experiment_enabled?).with(:onboarding_issues).and_return(false) + allow(Gitlab).to receive(:com?).and_return(false) + + render + end + + subject { rendered } + + it { is_expected.not_to have_selector('label[for="user_setup_for_company"]') } + it { is_expected.to have_button('Get started!') } +end -- GitLab