diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 59f19430b576b5b83b3ed12feec59938e2e9ac88..69b273444edcf149304c0f9d036181f3a9e6fcbd 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -4024,7 +4024,6 @@ Layout/LineLength: - 'spec/models/todo_spec.rb' - 'spec/models/upload_spec.rb' - 'spec/models/uploads/fog_spec.rb' - - 'spec/models/user_detail_spec.rb' - 'spec/models/user_spec.rb' - 'spec/models/wiki_page_spec.rb' - 'spec/models/x509_certificate_spec.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index ff159a848f2c196e5266f4fa3aeac104b2b5e39e..de0f42138559b7d9cd7657093943e29f0c961db4 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -4679,7 +4679,6 @@ RSpec/FeatureCategory: - 'spec/models/user_agent_detail_spec.rb' - 'spec/models/user_canonical_email_spec.rb' - 'spec/models/user_custom_attribute_spec.rb' - - 'spec/models/user_detail_spec.rb' - 'spec/models/user_highest_role_spec.rb' - 'spec/models/user_mentions/commit_user_mention_spec.rb' - 'spec/models/user_mentions/issue_user_mention_spec.rb' diff --git a/app/models/user_detail.rb b/app/models/user_detail.rb index bbb08ed577430c047ddd50b96e9ba58124a57460..e6dc99d114b7c3c791009c5d2e35ae2b04c0bea0 100644 --- a/app/models/user_detail.rb +++ b/app/models/user_detail.rb @@ -39,6 +39,7 @@ class UserDetail < MainClusterwide::ApplicationRecord validates :skype, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true validates :twitter, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true validates :website_url, length: { maximum: DEFAULT_FIELD_LENGTH }, url: true, allow_blank: true, if: :website_url_changed? + validates :onboarding_status, json_schema: { filename: 'user_detail_onboarding_status' } before_validation :sanitize_attrs before_save :prevent_nil_fields diff --git a/app/validators/json_schemas/user_detail_onboarding_status.json b/app/validators/json_schemas/user_detail_onboarding_status.json new file mode 100644 index 0000000000000000000000000000000000000000..548e81f1955a5aca5b190536b7d185edcfbe13dc --- /dev/null +++ b/app/validators/json_schemas/user_detail_onboarding_status.json @@ -0,0 +1,17 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Onboarding Status", + "description": "Onboarding Status items recorded during onboarding/registration", + "type": "object", + "properties": { + "step_url": { + "description": "Onboarding step the user is currently on or last step before finishing", + "type": "string" + }, + "email_opt_in": { + "description": "Setting to guide marketing email opt-ins outside of the product. See https://gitlab.com/gitlab-org/gitlab/-/issues/435741", + "type": "boolean" + } + }, + "additionalProperties": false +} diff --git a/db/migrate/20240116212237_add_onboarding_status_to_user_details.rb b/db/migrate/20240116212237_add_onboarding_status_to_user_details.rb new file mode 100644 index 0000000000000000000000000000000000000000..9bb2e85f8b974e5aae6b15169c9f4b6e22bc4dcf --- /dev/null +++ b/db/migrate/20240116212237_add_onboarding_status_to_user_details.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddOnboardingStatusToUserDetails < Gitlab::Database::Migration[2.2] + milestone '16.9' + enable_lock_retries! + + def change + add_column :user_details, :onboarding_status, :jsonb, default: {}, null: false + end +end diff --git a/db/schema_migrations/20240116212237 b/db/schema_migrations/20240116212237 new file mode 100644 index 0000000000000000000000000000000000000000..24059535076854706ffe721d0cfe899d022efb89 --- /dev/null +++ b/db/schema_migrations/20240116212237 @@ -0,0 +1 @@ +d58f59f84c1d9c08f8ba3466c844b01a1ab8ea429de9b0fb43dcd53e7611e2d6 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c7651cd9dee60529a3c29f6554797b962a07aca2..9879415c0e1c020fc0e94d7db40685b661a1526b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24956,6 +24956,7 @@ CREATE TABLE user_details ( email_reset_offered_at timestamp with time zone, mastodon text DEFAULT ''::text NOT NULL, project_authorizations_recalculated_at timestamp with time zone DEFAULT '2010-01-01 00:00:00+00'::timestamp with time zone NOT NULL, + onboarding_status jsonb DEFAULT '{}'::jsonb NOT NULL, CONSTRAINT check_245664af82 CHECK ((char_length(webauthn_xid) <= 100)), CONSTRAINT check_444573ee52 CHECK ((char_length(skype) <= 500)), CONSTRAINT check_466a25be35 CHECK ((char_length(twitter) <= 500)), diff --git a/ee/app/controllers/concerns/onboarding/set_redirect.rb b/ee/app/controllers/concerns/onboarding/set_redirect.rb index 4ac55c71b87d31ca45d2928e354c418506ae4be3..307205330a7d37d4f4702e30e94b0e47adaaf3fc 100644 --- a/ee/app/controllers/concerns/onboarding/set_redirect.rb +++ b/ee/app/controllers/concerns/onboarding/set_redirect.rb @@ -11,8 +11,10 @@ def verify_onboarding_enabled! end def save_onboarding_step_url(onboarding_step_url, user) - Onboarding.user_onboarding_in_progress?(user) && - user.user_detail.update(onboarding_step_url: onboarding_step_url) + return unless Onboarding.user_onboarding_in_progress?(user) + + user.user_detail.onboarding_step_url = onboarding_step_url + user.update_onboarding_status(:step_url, onboarding_step_url) end def start_onboarding(onboarding_step_url, user) @@ -20,6 +22,7 @@ def start_onboarding(onboarding_step_url, user) user.onboarding_in_progress = true user.user_detail.onboarding_step_url = onboarding_step_url + user.onboarding_status_step_url = onboarding_step_url user end diff --git a/ee/app/controllers/registrations/company_controller.rb b/ee/app/controllers/registrations/company_controller.rb index 7422408068ad286a0fafd5971023ce6a37632449..0eb6d16d3844dfb23285fcdead1ae8b8681d6450 100644 --- a/ee/app/controllers/registrations/company_controller.rb +++ b/ee/app/controllers/registrations/company_controller.rb @@ -50,8 +50,7 @@ def permitted_params # passed through params :role, :registration_objective, - :jobs_to_be_done_other, - :opt_in + :jobs_to_be_done_other ).merge(glm_tracking_params) end diff --git a/ee/app/controllers/registrations/welcome_controller.rb b/ee/app/controllers/registrations/welcome_controller.rb index ccb6198faf68c667bed20ebdcaf74a3aabda4a79..fa71a93b93d8b02d3786085e4e91261d4d13e418 100644 --- a/ee/app/controllers/registrations/welcome_controller.rb +++ b/ee/app/controllers/registrations/welcome_controller.rb @@ -57,19 +57,16 @@ def completed_welcome_step? end def update_params - params.require(:user).permit(:role, :setup_for_company, :registration_objective) + params.require(:user) + .permit(:role, :setup_for_company, :registration_objective, :onboarding_status_email_opt_in) + .merge(onboarding_status_email_opt_in: parsed_opt_in) end def passed_through_params - opt_in_param = { - opt_in: ::Gitlab::Utils.to_boolean(params[:opt_in_to_email], default: onboarding_status.setup_for_company?) - } - update_params.slice(:role, :registration_objective) .merge(params.permit(:jobs_to_be_done_other)) .merge(glm_tracking_params) .merge(params.permit(:trial)) - .merge(opt_in_param) end def iterable_params @@ -80,7 +77,7 @@ def iterable_params comment: params[:jobs_to_be_done_other], jtbd: update_params[:registration_objective], product_interaction: onboarding_status.iterable_product_interaction, - opt_in: ::Gitlab::Utils.to_boolean(params[:opt_in_to_email], default: false), + opt_in: current_user.onboarding_status_email_opt_in, preferred_language: ::Gitlab::I18n.trimmed_language_name(current_user.preferred_language) }.merge(update_params.slice(:setup_for_company, :role).to_h.symbolize_keys) end @@ -121,6 +118,15 @@ def signup_onboarding_path end end + def parsed_opt_in + return false if onboarding_status.invite? # order matters here as invites are treated differently + # The below would override DOM setting, but DOM is interwoven with JS to hide the opt in checkbox if + # setup for company is toggled, so this is where this is a bit complex to think about + return true if onboarding_status.setup_for_company? + + ::Gitlab::Utils.to_boolean(params.dig(:user, :onboarding_status_email_opt_in), default: false) + end + def track_joining_a_project_event return unless onboarding_status.joining_a_project? diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index e52f36d4e6bff8cf02e5de6ab6c06c16beea8a88..066cf5316c096d6447bfc3c335d47cff4243164a 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -53,7 +53,8 @@ module User to: :namespace delegate :provisioned_by_group, :provisioned_by_group=, :provisioned_by_group_id, :provisioned_by_group_id=, - :onboarding_step_url=, + :onboarding_step_url=, :onboarding_status_step_url, :onboarding_status_step_url=, + :onboarding_status_email_opt_in, :onboarding_status_email_opt_in=, :onboarding_status, to: :user_detail, allow_nil: true delegate :enabled_zoekt?, :enabled_zoekt, :enabled_zoekt=, @@ -410,6 +411,11 @@ def has_current_license? License.current.present? end + def update_onboarding_status(field, value) + onboarding_status[field] = value + save + end + def using_license_seat? active? && !internal? && diff --git a/ee/app/models/ee/user_detail.rb b/ee/app/models/ee/user_detail.rb index c05db8168475af82afc0e0bd8e6b64fa86945a18..fd2c0e283dab16f2ce8ae5ca68d1878042683f8a 100644 --- a/ee/app/models/ee/user_detail.rb +++ b/ee/app/models/ee/user_detail.rb @@ -9,6 +9,9 @@ module UserDetail belongs_to :enterprise_group, class_name: 'Group', optional: true scope :with_enterprise_group, -> { where.not(enterprise_group_id: nil) } + + attribute :onboarding_status, :ind_jsonb + store_accessor :onboarding_status, :step_url, :email_opt_in, prefix: true end def provisioned_by_group? diff --git a/ee/app/services/gitlab_subscriptions/create_company_lead_service.rb b/ee/app/services/gitlab_subscriptions/create_company_lead_service.rb index 8f3f1d34fcbf4383f5472a12844baa85be8af14c..8ff52a7555b92a19046908d1b46d4b421ea60c02 100644 --- a/ee/app/services/gitlab_subscriptions/create_company_lead_service.rb +++ b/ee/app/services/gitlab_subscriptions/create_company_lead_service.rb @@ -30,7 +30,8 @@ def user_values(user) uid: user.id, work_email: user.email, setup_for_company: user.setup_for_company, - preferred_language: ::Gitlab::I18n.trimmed_language_name(user.preferred_language) + preferred_language: ::Gitlab::I18n.trimmed_language_name(user.preferred_language), + opt_in: user.onboarding_status_email_opt_in } end diff --git a/ee/app/views/registrations/welcome/_opt_in_to_email.html.haml b/ee/app/views/registrations/welcome/_opt_in_to_email.html.haml index ee993723cc41e4514898aaaf039e98d4928bf664..ee4563968c1982408906fd559ba9dc0558386329 100644 --- a/ee/app/views/registrations/welcome/_opt_in_to_email.html.haml +++ b/ee/app/views/registrations/welcome/_opt_in_to_email.html.haml @@ -1,8 +1,7 @@ - return unless onboarding_status.enabled? +- return if onboarding_status.invite? .gl-mb-3.js-opt-in-to-email.hidden .gl-font-weight-bold.gl-mb-3 = _('Email updates (optional)') - = render Pajamas::CheckboxTagComponent.new(name: :opt_in_to_email, value: nil) do |c| - - c.with_label do - = _("I'd like to receive updates about GitLab via email") + = f.gitlab_ui_checkbox_component :onboarding_status_email_opt_in, _("I'd like to receive updates about GitLab via email") diff --git a/ee/app/views/registrations/welcome/show.html.haml b/ee/app/views/registrations/welcome/show.html.haml index d38fbf81e81be1efa3ba1771ed5fcaa2cf42a0fc..5056bc29df0086458c7618c1b37aa3edf607d66e 100644 --- a/ee/app/views/registrations/welcome/show.html.haml +++ b/ee/app/views/registrations/welcome/show.html.haml @@ -33,7 +33,7 @@ = render 'jobs_to_be_done', f: f = render 'setup_for_company', f: f = render 'joining_project' - = render 'opt_in_to_email' + = render 'opt_in_to_email', f: f .row .form-group.col-sm-12.gl-mb-0 = render Pajamas::ButtonComponent.new(variant: :confirm, block: true, diff --git a/ee/spec/controllers/registrations/company_controller_spec.rb b/ee/spec/controllers/registrations/company_controller_spec.rb index f6db4b41bc913e29d382311c4dba08130d7db9b2..c5a70eb84c05ba264218667da91511d4b6a3b019 100644 --- a/ee/spec/controllers/registrations/company_controller_spec.rb +++ b/ee/spec/controllers/registrations/company_controller_spec.rb @@ -92,7 +92,6 @@ country: 'US', state: 'CA', website_url: 'gitlab.com', - opt_in: 'true', trial: trial_registration }.merge(glm_params) end diff --git a/ee/spec/controllers/registrations/welcome_controller_spec.rb b/ee/spec/controllers/registrations/welcome_controller_spec.rb index a5590275caa89addc4ed1484e91e8c3e0201974f..0801c6771cef42fbe8851c2453fdfbbf0b4ca7b6 100644 --- a/ee/spec/controllers/registrations/welcome_controller_spec.rb +++ b/ee/spec/controllers/registrations/welcome_controller_spec.rb @@ -152,13 +152,14 @@ let(:setup_for_company) { 'false' } let(:joining_project) { 'false' } let(:extra_params) { {} } + let(:extra_user_params) { {} } let(:update_params) do { user: { role: 'software_developer', setup_for_company: setup_for_company, registration_objective: 'code_storage' - }, + }.merge(extra_user_params), joining_project: joining_project, jobs_to_be_done_other: '_jobs_to_be_done_other_', glm_source: 'some_source', @@ -349,6 +350,65 @@ end end + context 'for email opt_in' do + using RSpec::Parameterized::TableSyntax + + let(:invite?) { true } + let(:setup_for_company?) { false } + + before do + allow_next_instance_of(::Onboarding::Status) do |instance| + allow(instance).to receive(:invite?).and_return(invite?) + allow(instance).to receive(:setup_for_company?).and_return(setup_for_company?) + end + end + + where(:extra_user_params, :opt_in) do + { onboarding_status_email_opt_in: 'true' } | true + { onboarding_status_email_opt_in: 'false' } | false + { onboarding_status_email_opt_in: nil } | false + { onboarding_status_email_opt_in: '1' } | true + { onboarding_status_email_opt_in: '0' } | false + { onboarding_status_email_opt_in: '' } | false + {} | false + end + + with_them do + context 'for an invite' do + specify do + patch_update + user.reload + + expect(user.onboarding_status_email_opt_in).to eq(false) + end + end + + context 'for a non-invite' do + let(:invite?) { false } + + context 'when setup_for_company is true' do + let(:setup_for_company?) { true } + + specify do + patch_update + user.reload + + expect(user.onboarding_status_email_opt_in).to eq(true) + end + end + + context 'when setup_for_company is false' do + specify do + patch_update + user.reload + + expect(user.onboarding_status_email_opt_in).to eq(opt_in) + end + end + end + end + end + context 'when setup_for_company is "true"' do let(:setup_for_company) { 'true' } let(:trial_concerns) { {} } @@ -359,8 +419,7 @@ role: 'software_developer', jobs_to_be_done_other: '_jobs_to_be_done_other_', glm_source: 'some_source', - glm_content: 'some_content', - opt_in: 'true' + glm_content: 'some_content' }.merge(trial_concerns) end @@ -373,6 +432,8 @@ expect(user.onboarding_in_progress).to be_truthy expect(user.user_detail.onboarding_step_url).to eq(redirect_path) + expect(user.onboarding_status_step_url).to eq(redirect_path) + expect(user.onboarding_status_email_opt_in).to eq(true) expect(response).to redirect_to redirect_path end @@ -413,12 +474,16 @@ context 'when trial is true' do using RSpec::Parameterized::TableSyntax - where(:extra_params, :opt_in) do - { trial: 'true', opt_in_to_email: 'true' } | 'true' - { trial: 'true', opt_in_to_email: 'false' } | 'false' - { trial: 'true', opt_in_to_email: nil } | 'false' - { trial: 'true', opt_in_to_email: '' } | 'false' - { trial: 'true' } | 'false' + let(:extra_params) { { trial: 'true' } } + + where(:extra_user_params, :opt_in) do + { onboarding_status_email_opt_in: 'true' } | true + { onboarding_status_email_opt_in: 'false' } | false + { onboarding_status_email_opt_in: nil } | false + { onboarding_status_email_opt_in: '1' } | true + { onboarding_status_email_opt_in: '0' } | false + { onboarding_status_email_opt_in: '' } | false + {} | false end with_them do @@ -429,8 +494,7 @@ jobs_to_be_done_other: '_jobs_to_be_done_other_', glm_source: 'some_source', glm_content: 'some_content', - trial: 'true', - opt_in: opt_in + trial: 'true' } patch_update @@ -439,6 +503,8 @@ expect(user.onboarding_in_progress).to be_truthy expect(user.user_detail.onboarding_step_url).to eq(path) + expect(user.onboarding_status_step_url).to eq(path) + expect(user.onboarding_status_email_opt_in).to eq(opt_in) expect(response).to redirect_to path end end @@ -527,8 +593,7 @@ glm_source: 'some_source', jobs_to_be_done_other: '_jobs_to_be_done_other_', registration_objective: 'code_storage', - role: 'software_developer', - opt_in: false + role: 'software_developer' }.merge(extra_params) ) diff --git a/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb b/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb index 7299d64a47fb27e6a69492930a615d8dbfcecddd..76fcf59ce34dc7f5ca0da80839ffc6b8ce53513b 100644 --- a/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb +++ b/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb @@ -49,6 +49,7 @@ def fills_in_welcome_form fill_in 'Why are you signing up? (optional)', with: 'My reason' choose 'Just me' + check _("I'd like to receive updates about GitLab via email") choose 'Create a new project' # does not matter here if choose 'Join a project' end end diff --git a/ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb b/ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb index 81b66d14cbe95bec77c321df76035c5c15d8e427..32b0a3e735bbd1c7af5f58644236af92082b11fe 100644 --- a/ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb +++ b/ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb @@ -99,7 +99,7 @@ expect(page).to have_native_text_validation_message('last_name') # success - fill_in_company_form(with_last_name: true, glm: false, opt_in_email: true, trial: true) + fill_in_company_form(with_last_name: true, glm: false, trial: true) click_on 'Continue' ensure_onboarding { expect_to_see_group_and_project_creation_form } diff --git a/ee/spec/features/registrations/saas/trial_flow_company_importing_project_spec.rb b/ee/spec/features/registrations/saas/trial_flow_company_importing_project_spec.rb index 8ae8235fbc2b18877e841c4a1dc890d4d502ad80..ad4d15f87e1f577c5ec854ec2ba1a5fed8542c22 100644 --- a/ee/spec/features/registrations/saas/trial_flow_company_importing_project_spec.rb +++ b/ee/spec/features/registrations/saas/trial_flow_company_importing_project_spec.rb @@ -21,7 +21,7 @@ expect_to_see_company_form - fill_in_company_form(glm: false, opt_in_email: true, trial: true) + fill_in_company_form(glm: false, trial: true) click_on 'Continue' expect_to_see_group_and_project_creation_form diff --git a/ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb b/ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb index 849d71f1abca34a8acda62eebf3f5878e3de994d..3f680765882b048fec0f6c1a413a030277f80220 100644 --- a/ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb +++ b/ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb @@ -22,7 +22,7 @@ expect_to_see_company_form - fill_in_company_form(opt_in_email: true, trial: true) + fill_in_company_form(trial: true) click_on 'Continue' expect_to_see_group_and_project_creation_form diff --git a/ee/spec/features/registrations/saas/trial_flow_just_me_importing_project_spec.rb b/ee/spec/features/registrations/saas/trial_flow_just_me_importing_project_spec.rb index e1a15087869eddba2f67dc80efd45afa8fac20b5..dee59d2a90f274b89ffb417d03efe3b999bf6982 100644 --- a/ee/spec/features/registrations/saas/trial_flow_just_me_importing_project_spec.rb +++ b/ee/spec/features/registrations/saas/trial_flow_just_me_importing_project_spec.rb @@ -21,7 +21,7 @@ expect_to_see_company_form - fill_in_company_form(glm: false, opt_in_email: true, trial: true) + fill_in_company_form(glm: false, trial: true) click_on 'Continue' expect_to_see_group_and_project_creation_form diff --git a/ee/spec/features/registrations/welcome_spec.rb b/ee/spec/features/registrations/welcome_spec.rb index 735f6c3b0cd936cfd268516c5b067b7ecec2260c..f0c11e23ccbf2de2bc7e514f021eab1c92c44a4e 100644 --- a/ee/spec/features/registrations/welcome_spec.rb +++ b/ee/spec/features/registrations/welcome_spec.rb @@ -5,6 +5,7 @@ RSpec.describe 'Welcome screen on SaaS', :js, :saas, feature_category: :onboarding do context 'with email opt in' do let(:user) { create(:user, role: nil) } + let(:opt_in_selector) { 'input[name="user[onboarding_status_email_opt_in]"]' } before do gitlab_sign_in(user) @@ -14,20 +15,20 @@ it 'does not show the email opt in checkbox when setting up for a company' do expect(page).to have_content('We won\'t share this information with anyone') - expect(page).not_to have_selector('input[name="opt_in_to_email]', visible: :visible) + expect(page).not_to have_selector(opt_in_selector, visible: :visible) choose 'user_setup_for_company_true' - expect(page).not_to have_selector('input[name="opt_in_to_email]', visible: :visible) + expect(page).not_to have_selector(opt_in_selector, visible: :visible) end it 'shows the email opt in checkbox when setting up for just me' do expect(page).to have_content('We won\'t share this information with anyone') - expect(page).not_to have_selector('input[name="opt_in_to_email', visible: :visible) + expect(page).not_to have_selector(opt_in_selector, visible: :visible) choose 'user_setup_for_company_false' - expect(page).to have_selector('input[name="opt_in_to_email', visible: :visible) + expect(page).to have_selector(opt_in_selector, visible: :visible) end end end diff --git a/ee/spec/models/ee/user_detail_spec.rb b/ee/spec/models/ee/user_detail_spec.rb index a6b09d5b25ad288c721c51a71c52442d1eaacba5..c80913f1c307e07ddd32cf681e461818c9f82e4b 100644 --- a/ee/spec/models/ee/user_detail_spec.rb +++ b/ee/spec/models/ee/user_detail_spec.rb @@ -6,6 +6,17 @@ it { is_expected.to belong_to(:provisioned_by_group) } it { is_expected.to belong_to(:enterprise_group) } + describe 'validations' do + context 'with support for hash with indifferent access - ind_jsonb' do + specify do + user_detail = build(:user_detail, onboarding_status: { 'step_url' => '_string_' }) + user_detail.onboarding_status[:email_opt_in] = true + + expect(user_detail).to be_valid + end + end + end + describe 'scopes' do describe '.with_enterprise_group' do subject(:scope) { described_class.with_enterprise_group } diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index 8acf897bdf69c52100a0c77b81c00af9cb4c6cd1..8bee1e1c14b6e041ecfd227073d43b7d529b41ab 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -17,6 +17,15 @@ it { is_expected.to delegate_method(:shared_runners_minutes_limit).to(:namespace) } it { is_expected.to delegate_method(:shared_runners_minutes_limit=).to(:namespace).with_arguments(133) } it { is_expected.to delegate_method(:onboarding_step_url=).to(:user_detail).with_arguments('_url_').allow_nil } + it { is_expected.to delegate_method(:onboarding_status_step_url=).to(:user_detail).with_arguments('url').allow_nil } + it { is_expected.to delegate_method(:onboarding_status_step_url).to(:user_detail).allow_nil } + + it do + is_expected.to delegate_method(:onboarding_status_email_opt_in=).to(:user_detail).with_arguments(true).allow_nil + end + + it { is_expected.to delegate_method(:onboarding_status_email_opt_in).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:onboarding_status).to(:user_detail).allow_nil } end describe 'associations' do @@ -702,6 +711,43 @@ end end + describe '#update_onboarding_status' do + let_it_be(:user, reload: true) { create(:user) } + + context 'with adding attribute' do + it 'sets the attribute' do + expect(user.onboarding_status_step_url).to eq(nil) + expect(user.update_onboarding_status(:step_url, '_step_url_')).to be_truthy + expect(user.reload.onboarding_status_step_url).to eq('_step_url_') + end + end + + context 'with overriding existing attribute' do + before do + user.update!(onboarding_status_step_url: '_foo_') + end + + it 'sets the attribute' do + expect(user.update_onboarding_status(:step_url, '_step_url_')).to be_truthy + expect(user.reload.onboarding_status_step_url).to eq('_step_url_') + end + + context 'when overriding with string field value' do + it 'sets the attribute' do + expect(user.update_onboarding_status('step_url', '_step_url_')).to be_truthy + expect(user.reload.onboarding_status_step_url).to eq('_step_url_') + end + end + end + + context 'with an unknown attribute' do + it 'does not set the attribute' do + expect(user.update_onboarding_status(:foo, '_foo_')).to be(false) + expect(user).to be_invalid + end + end + end + describe '#can_read_all_resources?' do it 'returns true for auditor user' do user = build(:user, :auditor) diff --git a/ee/spec/services/gitlab_subscriptions/create_company_lead_service_spec.rb b/ee/spec/services/gitlab_subscriptions/create_company_lead_service_spec.rb index 3c3a6762e226b73dc207f9867ed870f679a0c355..c4f45b8fbc4ff43527b09e7841091433f376235e 100644 --- a/ee/spec/services/gitlab_subscriptions/create_company_lead_service_spec.rb +++ b/ee/spec/services/gitlab_subscriptions/create_company_lead_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe GitlabSubscriptions::CreateCompanyLeadService, feature_category: :subscription_management do - let(:user) { build(:user, last_name: 'Jones') } + let(:user) { build(:user, last_name: 'Jones', onboarding_status_email_opt_in: true) } describe '#execute' do using RSpec::Parameterized::TableSyntax @@ -16,6 +16,7 @@ last_name: user.last_name, work_email: user.email, setup_for_company: user.setup_for_company, + opt_in: user.onboarding_status_email_opt_in, preferred_language: 'English', provider: 'gitlab', skip_email_confirmation: true, diff --git a/ee/spec/support/helpers/saas_registration_helpers.rb b/ee/spec/support/helpers/saas_registration_helpers.rb index 5148f67530553e6434b972d7fb41f3cb744fbe07..f74f94bbeaefe73e87698f0fba998149d473666c 100644 --- a/ee/spec/support/helpers/saas_registration_helpers.rb +++ b/ee/spec/support/helpers/saas_registration_helpers.rb @@ -399,7 +399,7 @@ def fills_in_group_and_project_creation_form_with_trial(glm: true) ).and_call_original end - def fill_in_company_form(with_last_name: false, glm: true, success: true, opt_in_email: false, trial: false) + def fill_in_company_form(with_last_name: false, glm: true, success: true, trial: false) result = if success ServiceResponse.success else @@ -408,7 +408,7 @@ def fill_in_company_form(with_last_name: false, glm: true, success: true, opt_in expect(GitlabSubscriptions::CreateCompanyLeadService).to receive(:new).with( user: user, - params: company_params(user, glm: glm, opt_in: opt_in_email, trial: trial) + params: company_params(user, glm: glm, trial: trial) ).and_return(instance_double(GitlabSubscriptions::CreateCompanyLeadService, execute: result)) fill_in_company_user_last_name if with_last_name @@ -428,7 +428,7 @@ def fill_company_form_fields fill_in 'website_url', with: 'https://gitlab.com' end - def company_params(user, opt_in:, glm: true, trial: false) + def company_params(user, glm: true, trial: false) base_params = ActionController::Parameters.new( company_name: 'Test Company', company_size: '1-99', @@ -443,7 +443,6 @@ def company_params(user, opt_in:, glm: true, trial: false) role: 'software_developer', registration_objective: 'other', jobs_to_be_done_other: 'My reason', - opt_in: (opt_in || user.setup_for_company).to_s, trial: trial ).permit! diff --git a/ee/spec/views/registrations/welcome/show.html.haml_spec.rb b/ee/spec/views/registrations/welcome/show.html.haml_spec.rb index dc80a17c19ba2ae1c0631dd33752a985e8c4f15c..1e7d2367bde3594cb07b1684803af6476376c16c 100644 --- a/ee/spec/views/registrations/welcome/show.html.haml_spec.rb +++ b/ee/spec/views/registrations/welcome/show.html.haml_spec.rb @@ -3,9 +3,11 @@ require 'spec_helper' RSpec.describe 'registrations/welcome/show', :saas, feature_category: :onboarding do + let(:invite?) { false } + let(:trial?) { false } let(:onboarding_status) do instance_double( - ::Onboarding::Status, invite?: false, enabled?: true, subscription?: false, trial?: false, oauth?: false + ::Onboarding::Status, invite?: invite?, enabled?: true, subscription?: false, trial?: trial?, oauth?: false ) end @@ -13,24 +15,28 @@ allow(view).to receive(:onboarding_status).and_return(onboarding_status) allow(view).to receive(:current_user).and_return(build_stubbed(:user)) allow(view).to receive(:glm_tracking_params).and_return({}) - end - context 'with basic form items' do - before do - render - end + render + end - subject { rendered } + subject { rendered } + context 'with basic form items' do it 'the text for the :setup_for_company label' do is_expected.to have_selector('label[for="user_setup_for_company"]', text: _('Who will be using GitLab?')) end - it 'shows the correct text for the submit button' do - is_expected.to have_button('Continue') + it 'shows the text for the submit button' do + is_expected.to have_button(_('Continue')) end - it { is_expected.to have_selector('#joining_project_true') } + it 'has the joining_project fields' do + is_expected.to have_selector('#joining_project_true') + end + + it 'has the hidden opt in to email field' do + is_expected.to have_selector('input[name="user[onboarding_status_email_opt_in]"]') + end it 'renders a select and text field for additional information' do is_expected.to have_selector('select[name="user[registration_objective]"]') @@ -38,10 +44,35 @@ end end - context 'for rendering the hidden opt in to email checkbox' do - subject { render } + context 'when it is an invite' do + let(:invite?) { true } - it { is_expected.to have_selector('input[name="opt_in_to_email"]') } - it { is_expected.to have_css('.js-opt-in-to-email.hidden') } + it 'does not have setup_for_company label' do + is_expected.not_to have_selector('label[for="user_setup_for_company"]') + end + + it 'has a hidden input for setup_for_company' do + is_expected.to have_field('user[setup_for_company]', type: :hidden) + end + + it 'does not have the joining_project fields' do + is_expected.not_to have_selector('#joining_project_true') + end + + it 'does not have opt in to email field' do + is_expected.not_to have_selector('input[name="user[onboarding_status_email_opt_in]"]') + end + end + + context 'when it is a trial' do + let(:trial?) { true } + + it 'has setup_for_company label' do + is_expected.to have_selector('label[for="user_setup_for_company"]') + end + + it 'does not have the joining_project fields' do + is_expected.not_to have_selector('#joining_project_true') + end end end diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index b443988cde9aa200ec0797ec9eba17b52e137275..81cc59cec49d1dfaecc434b1568e9b5bbae45b3f 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -2,11 +2,76 @@ require 'spec_helper' -RSpec.describe UserDetail do +RSpec.describe UserDetail, feature_category: :system_access do it { is_expected.to belong_to(:user) } - it { is_expected.to define_enum_for(:registration_objective).with_values([:basics, :move_repository, :code_storage, :exploring, :ci, :other, :joining_team]).with_suffix } + + specify do + values = [:basics, :move_repository, :code_storage, :exploring, :ci, :other, :joining_team] + is_expected.to define_enum_for(:registration_objective).with_values(values).with_suffix + end describe 'validations' do + context 'for onboarding_status json schema' do + let(:step_url) { '_some_string_' } + let(:email_opt_in) { true } + let(:onboarding_status) do + { + step_url: step_url, + email_opt_in: email_opt_in + } + end + + it { is_expected.to allow_value(onboarding_status).for(:onboarding_status) } + + context 'for step_url' do + let(:onboarding_status) do + { + step_url: step_url + } + end + + it { is_expected.to allow_value(onboarding_status).for(:onboarding_status) } + + context "when 'step_url' is invalid" do + let(:step_url) { [] } + + it { is_expected.not_to allow_value(onboarding_status).for(:onboarding_status) } + end + end + + context 'for email_opt_in' do + let(:onboarding_status) do + { + email_opt_in: email_opt_in + } + end + + it { is_expected.to allow_value(onboarding_status).for(:onboarding_status) } + + context "when 'email_opt_in' is invalid" do + let(:email_opt_in) { 'true' } + + it { is_expected.not_to allow_value(onboarding_status).for(:onboarding_status) } + end + end + + context 'when there is no data' do + let(:onboarding_status) { {} } + + it { is_expected.to allow_value(onboarding_status).for(:onboarding_status) } + end + + context 'when trying to store an unsupported key' do + let(:onboarding_status) do + { + unsupported_key: '_some_value_' + } + end + + it { is_expected.not_to allow_value(onboarding_status).for(:onboarding_status) } + end + end + describe '#job_title' do it { is_expected.not_to validate_presence_of(:job_title) } it { is_expected.to validate_length_of(:job_title).is_at_most(200) } @@ -75,7 +140,8 @@ user_detail.mastodon = '@robin' expect(user_detail).not_to be_valid - expect(user_detail.errors.full_messages).to match_array([_('Mastodon must contain only a mastodon username.')]) + expect(user_detail.errors.full_messages) + .to match_array([_('Mastodon must contain only a mastodon username.')]) end end end