diff --git a/app/assets/stylesheets/pages/experimental_separate_sign_up.scss b/app/assets/stylesheets/pages/experimental_separate_sign_up.scss index 53dfdd10788ccb37393e058ec57738537a9dca96..5a80ea79600176750c3dcf60f7257da1c1075c8b 100644 --- a/app/assets/stylesheets/pages/experimental_separate_sign_up.scss +++ b/app/assets/stylesheets/pages/experimental_separate_sign_up.scss @@ -23,7 +23,7 @@ .signup-heading h2 { font-weight: $gl-font-weight-bold; - padding: 0 10px; + padding: 0 $gl-padding; @include media-breakpoint-down(md) { font-size: $gl-font-size-large; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 85602aa72deb31eb32e58462010e51ad4816b259..c85b192b34ad05d4dc0f9ae03ec3a9f7d0dac154 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -30,7 +30,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 + before_action :required_signup_info around_action :set_locale around_action :set_session_storage @@ -538,10 +538,13 @@ 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) + # A user requires a role and have the setup_for_company attribute set 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 required_signup_info + return unless current_user + return unless current_user.role_required? + return unless experiment_enabled?(:signup_flow) store_location_for :user, request.fullpath diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index e55156c619b728f5103d06d8796a7c928e10c2b6..84011e7643d479a3a0c8f9b38b274dab0b7c21dd 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -8,7 +8,7 @@ class RegistrationsController < Devise::RegistrationsController layout :choose_layout - skip_before_action :require_role, only: [:welcome, :update_role] + skip_before_action :required_signup_info, only: [:welcome, :update_registration] prepend_before_action :check_captcha, only: :create before_action :whitelist_query_limiting, only: [:destroy] before_action :ensure_terms_accepted, @@ -53,22 +53,22 @@ def destroy 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? + return redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) if current_user.role.present? && !current_user.setup_for_company.nil? - current_user.name = nil + current_user.name = nil if current_user.name == current_user.username 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 + def update_registration + user_params = params.require(:user).permit(:name, :role, :setup_for_company) + result = ::Users::SignupService.new(current_user, user_params).execute if result[:status] == :success track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group 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] + render :welcome, layout: 'devise_experimental_separate_sign_up_flow' end end diff --git a/app/models/user.rb b/app/models/user.rb index 07cd8431d1aab42d8525e9438554836eb2f3866a..f704589ad80e7f6ccfe195d4bd4157df57fed96a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -240,6 +240,7 @@ def update_tracked_fields!(request) delegate :time_display_relative, :time_display_relative=, to: :user_preference delegate :time_format_in_24h, :time_format_in_24h=, to: :user_preference delegate :show_whitespace_in_diffs, :show_whitespace_in_diffs=, to: :user_preference + delegate :setup_for_company, :setup_for_company=, to: :user_preference accepts_nested_attributes_for :user_preference, update_only: true diff --git a/app/services/users/signup_service.rb b/app/services/users/signup_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..1031cec44cb1d3ba8b2fa564be53fb208c18ba70 --- /dev/null +++ b/app/services/users/signup_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Users + class SignupService < BaseService + def initialize(current_user, params = {}) + @user = current_user + @params = params.dup + end + + def execute + assign_attributes + inject_validators + + if @user.save + success + else + error(@user.errors.full_messages.join('. ')) + end + end + + private + + def assign_attributes + @user.assign_attributes(params) unless params.empty? + end + + def inject_validators + class << @user + validates :role, presence: true + validates :setup_for_company, inclusion: { in: [true, false], message: :blank } + end + end + end +end diff --git a/app/views/registrations/welcome.html.haml b/app/views/registrations/welcome.html.haml index 76c4a935584ea5ca9ca062b8109815851e48f4af..7b92f5070df0a2697cfe888d6c4a3284988f1355 100644 --- a/app/views/registrations/welcome.html.haml +++ b/app/views/registrations/welcome.html.haml @@ -1,10 +1,10 @@ -- content_for(:page_title, _('Welcome to GitLab %{username}!') % { username: current_user.username }) +- content_for(:page_title, _('Welcome to GitLab @%{username}!') % { username: current_user.username }) - 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 + = _('In order to tailor your experience with GitLab we<br>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| + = 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 .name.form-group @@ -13,5 +13,14 @@ .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' diff --git a/changelogs/unreleased/48-add-company-question-to-profile-information.yml b/changelogs/unreleased/48-add-company-question-to-profile-information.yml new file mode 100644 index 0000000000000000000000000000000000000000..255f2289cdc924122509f352a8895ccac183920a --- /dev/null +++ b/changelogs/unreleased/48-add-company-question-to-profile-information.yml @@ -0,0 +1,5 @@ +--- +title: Ask if the user is setting up GitLab for a company during signup +merge_request: 17999 +author: +type: changed diff --git a/config/routes.rb b/config/routes.rb index e32c4f7415b7cb461ab598b737cd61808be3c8cc..23e97fe32dd148232ecb327a6bdb074ac66ee706 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -57,7 +57,7 @@ # Sign up get 'users/sign_up/welcome' => 'registrations#welcome' - patch 'users/sign_up/update_role' => 'registrations#update_role' + patch 'users/sign_up/update_registration' => 'registrations#update_registration' # Search get 'search' => 'search#show' diff --git a/db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb b/db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb new file mode 100644 index 0000000000000000000000000000000000000000..18a8a2306e264ba252bdc968d94f839bc3a6673a --- /dev/null +++ b/db/migrate/20191028162543_add_setup_for_company_to_user_preferences.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddSetupForCompanyToUserPreferences < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + add_column :user_preferences, :setup_for_company, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 36f82f9913b7a0156ef567640aa8ba45a8f10926..fa12e13f1b4d7150685beb853e0e69c9af8942e1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3754,6 +3754,7 @@ t.boolean "time_format_in_24h" t.string "projects_sort", limit: 64 t.boolean "show_whitespace_in_diffs", default: true, null: false + t.boolean "setup_for_company" t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bcf7405a7113ab525878a9c833c6e6cfebcea8dd..3f9e0e7805bcef466f0f8fe1a2b5bf50ab8619f3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1947,6 +1947,9 @@ msgstr "" msgid "Archiving the project will make it entirely read-only. It is hidden from the dashboard and doesn't show up in searches. <strong>The repository cannot be committed to, and no issues, comments or other entities can be created.</strong>" msgstr "" +msgid "Are you setting up GitLab for a company?" +msgstr "" + msgid "Are you sure that you want to archive this project?" msgstr "" @@ -9143,7 +9146,7 @@ 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." +msgid "In order to tailor your experience with GitLab we<br>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." @@ -19090,7 +19093,7 @@ msgstr "" msgid "Welcome to GitLab" msgstr "" -msgid "Welcome to GitLab %{username}!" +msgid "Welcome to GitLab @%{username}!" msgstr "" msgid "Welcome to the Guided GitLab Tour" diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index bb7f5ec2b28fa1b6f9d3ca2a97138eca6b2f3ad3..53896c7f5c7212fe13b3eb8dcbbb7a052432fe0a 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -827,7 +827,7 @@ def index end end - describe '#require_role' do + describe '#required_signup_info' do controller(described_class) do def index; end end @@ -849,7 +849,7 @@ def index; end it { is_expected.to redirect_to users_sign_up_welcome_path } end - context 'experiment enabled and user without a role' do + context 'experiment enabled and user without a required role' do before do sign_in(user) get :index @@ -858,7 +858,7 @@ def index; end it { is_expected.not_to redirect_to users_sign_up_welcome_path } end - context 'experiment disabled and user with required role' do + context 'experiment disabled' do let(:experiment_enabled) { false } before do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 8f260aa8b433c6ed1cc341945ea87bc9d09b0731..c5cfdd32619224c91c3f7e5b72a81bbee4ca9f53 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -381,7 +381,7 @@ def expect_success end end - describe '#update_role' do + describe '#update_registration' do before do stub_experiment(signup_flow: true) stub_experiment_for_user(signup_flow: true) @@ -395,7 +395,7 @@ def expect_success label: anything, property: 'experimental_group' ) - patch :update_role, params: { user: { name: 'New name', role: 'software_developer' } } + patch :update_registration, params: { user: { name: 'New name', role: 'software_developer', setup_for_company: 'false' } } end end end diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 5d4c30b6e8efc802bc44f5de8f3d41974c5cdf51..29ff0c67dbdded0a32c0d737bc15637f48128392 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -441,11 +441,13 @@ fill_in 'user_name', with: 'New name' 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.name).to eq 'New name' expect(new_user.software_developer_role?).to be_truthy + expect(new_user.setup_for_company).to be_truthy expect(page).to have_current_path(new_project_path) end end diff --git a/spec/services/users/signup_service_spec.rb b/spec/services/users/signup_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7d3cd614142fb0652dee3b744099d2403251be35 --- /dev/null +++ b/spec/services/users/signup_service_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Users::SignupService do + let(:user) { create(:user, setup_for_company: true) } + + describe '#execute' do + context 'when updating name' do + it 'updates the name attribute' do + result = update_user(user, name: 'New Name') + + expect(result).to eq(status: :success) + expect(user.reload.name).to eq('New Name') + end + + it 'returns an error result when name is missing' do + result = update_user(user, name: '') + + expect(user.reload.name).not_to be_blank + expect(result[:status]).to eq(:error) + expect(result[:message]).to include("Name can't be blank") + end + end + + context 'when updating role' do + it 'updates the role attribute' do + result = update_user(user, role: 'development_team_lead') + + expect(result).to eq(status: :success) + expect(user.reload.role).to eq('development_team_lead') + end + + it 'returns an error result when role is missing' do + result = update_user(user, role: '') + + expect(user.reload.role).not_to be_blank + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Role can't be blank") + end + end + + context 'when updating setup_for_company' do + it 'updates the setup_for_company attribute' do + result = update_user(user, setup_for_company: 'false') + + expect(result).to eq(status: :success) + 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: '') + + 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 + + def update_user(user, opts) + described_class.new(user, opts).execute + end + end +end