Merge branch '482910-move-glm-params-to-onboarding-status' into 'master'

Move glm_tracking_params into the onboarding status class

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/166669



Merged-by: Doug Stull <dstull@gitlab.com>
Approved-by: Jay Montal <jmontal@gitlab.com>
Approved-by: Dylan Griffith <dyl.griffith@gmail.com>
Approved-by: Smriti Garg <sgarg@gitlab.com>
Reviewed-by: Doug Stull <dstull@gitlab.com>

app/controllers/concerns/onboarding/status.rb

@@ -2,6 +2,10 @@
 
 module Onboarding
   class Status
+    def self.registration_path_params(params:, extra_params: {}) # rubocop:disable Lint/UnusedMethodArgument -- overridden in EE
+      {}
+    end
+
     def initialize(params, session, user)
       @params = params
       @session = session

app/controllers/omniauth_callbacks_controller.rb

@@ -200,7 +200,7 @@ def build_auth_user(auth_user_class)
     end
   end
 
-  # Overrided in EE
+  # Overridden in EE
   def set_session_active_since(id); end
 
   def sign_in_user_flow(auth_user_class)

app/controllers/registrations_controller.rb

@@ -28,7 +28,7 @@ class RegistrationsController < Devise::RegistrationsController
 
   feature_category :instance_resiliency
 
-  helper_method :arkose_labs_enabled?, :registration_path_params, :preregistration_tracking_label
+  helper_method :arkose_labs_enabled?, :preregistration_tracking_label
 
   def new
     @resource = build_resource
@@ -156,11 +156,6 @@ def allow_flash_content?(user)
     user.blocked_pending_approval? || onboarding_status.single_invite?
   end
 
-  # overridden in EE
-  def registration_path_params
-    {}
-  end
-
   def track_successful_user_creation(user)
     label = user_invited? ? 'invited' : 'signup'
     Gitlab::Tracking.event(self.class.name, 'create_user', label: label, user: user)

app/helpers/registrations_helper.rb

@@ -10,11 +10,6 @@ def signup_username_data_attributes
       testid: 'new-user-username-field'
     }
   end
-
-  # overridden in EE
-  def register_omniauth_params(_local_assigns)
-    {}
-  end
 end
 
 RegistrationsHelper.prepend_mod_with('RegistrationsHelper')

app/views/devise/registrations/new.html.haml

@@ -10,7 +10,7 @@
 .signup-page
   .gl-mb-3.gl-p-4
     = render 'signup_box_form',
-      url: registration_path(resource_name, registration_path_params),
+      url: registration_path(resource_name, ::Onboarding::Status.registration_path_params(params: params)),
       button_text: _('Continue')
 
     - if omniauth_enabled? && button_based_providers_enabled?
@@ -20,7 +20,7 @@
         .gl-flex.gl-flex-col.gl-gap-3
           - enabled_button_based_providers.each do |provider|
             = render 'devise/shared/omniauth_provider_button',
-              href: omniauth_authorize_path(:user, provider, register_omniauth_params({})),
+              href: omniauth_authorize_path(:user, provider, ::Onboarding::Status.registration_path_params(params: params)),
               provider: provider,
               classes: 'js-track-omni-auth',
               data: { provider: provider, track_action: "#{provider}_sso", track_label: preregistration_tracking_label }

ee/app/controllers/concerns/ee/onboarding/redirectable.rb

@@ -13,6 +13,10 @@ def onboarding_first_step_path
         users_sign_up_welcome_path(onboarding_params)
       end
 
+      def onboarding_params
+        ::Onboarding::Status.glm_tracking_params(params) # rubocop:disable Rails/StrongParams -- strong params are used in the method being called
+      end
+
       override :after_sign_up_path
       def after_sign_up_path
         if ::Onboarding.enabled?

ee/app/controllers/concerns/ee/onboarding/status.rb

@@ -10,6 +10,8 @@ module Status
         ::Onboarding::REGISTRATION_TYPE[:subscription] => ::Onboarding::SubscriptionRegistration
       }.freeze
 
+      GLM_PARAMS = [:glm_source, :glm_content].freeze
+
       attr_reader :registration_type
 
       # string delegations
@@ -23,6 +25,25 @@ module Status
       delegate :hide_setup_for_company_field?, :pre_parsed_email_opt_in?, to: :registration_type
       delegate :read_from_stored_user_location?, :preserve_stored_location?, to: :registration_type
 
+      module ClassMethods
+        extend ::Gitlab::Utils::Override
+
+        def glm_tracking_params(params)
+          params.permit(*GLM_PARAMS)
+        end
+
+        override :registration_path_params
+        def registration_path_params(params:, extra_params: {})
+          return super unless ::Onboarding.enabled?
+
+          glm_tracking_params(params).to_h.merge(extra_params)
+        end
+      end
+
+      def self.prepended(base)
+        base.singleton_class.prepend(ClassMethods)
+      end
+
       def initialize(*)
         super
 

ee/app/controllers/concerns/gitlab_subscriptions/trials/duo_common.rb

@@ -7,7 +7,6 @@ module DuoCommon
 
       include OneTrustCSP
       include GoogleAnalyticsCSP
-      include RegistrationsTracking
       include ::Gitlab::Utils::StrongMemoize
       include SafeFormatHelper
 
@@ -43,8 +42,9 @@ def general_params
 
       def lead_params
         params.permit(
+          *::Onboarding::Status::GLM_PARAMS,
           :company_name, :company_size, :first_name, :last_name, :phone_number,
-          :country, :state, :website_url, :glm_content, :glm_source
+          :country, :state, :website_url
         ).to_h
       end
 

ee/app/controllers/concerns/registrations_tracking.rb

-# frozen_string_literal: true
-
-module RegistrationsTracking
-  extend ActiveSupport::Concern
-
-  included do
-    helper_method :glm_tracking_params
-  end
-
-  private
-
-  def glm_tracking_params
-    params.permit(:glm_source, :glm_content)
-  end
-  alias_method :onboarding_params, :glm_tracking_params
-end
-
-RegistrationsTracking.prepend_mod

ee/app/controllers/ee/registrations_controller.rb

@@ -11,7 +11,6 @@ module RegistrationsController
     prepended do
       include Arkose::ContentSecurityPolicy
       include Arkose::TokenVerifiable
-      include RegistrationsTracking
       include GoogleAnalyticsCSP
       include GoogleSyndicationCSP
 
@@ -171,11 +170,6 @@ def log_audit_event(user)
       })
     end
 
-    override :registration_path_params
-    def registration_path_params
-      glm_tracking_params.to_h
-    end
-
     def record_arkose_data(user)
       return unless arkose_labs_enabled?(user: user)
       return unless arkose_labs_verify_response

ee/app/controllers/gitlab_subscriptions/trials/duo_enterprise_controller.rb

@@ -76,7 +76,7 @@ def track_event(action)
       end
 
       def trial_params
-        params.permit(:namespace_id, :glm_source, :glm_content).to_h
+        params.permit(*::Onboarding::Status::GLM_PARAMS, :namespace_id).to_h
       end
 
       def success_flash_message

ee/app/controllers/gitlab_subscriptions/trials/duo_pro_controller.rb

@@ -65,7 +65,7 @@ def track_event(action)
       end
 
       def trial_params
-        params.permit(:namespace_id, :trial_entity, :glm_source, :glm_content).to_h
+        params.permit(*::Onboarding::Status::GLM_PARAMS, :namespace_id, :trial_entity).to_h
       end
 
       def success_flash_message

ee/app/controllers/gitlab_subscriptions/trials_controller.rb

@@ -5,7 +5,6 @@ module GitlabSubscriptions
   class TrialsController < ApplicationController
     include OneTrustCSP
     include GoogleAnalyticsCSP
-    include RegistrationsTracking
 
     layout 'minimal'
 
@@ -74,18 +73,22 @@ def stored_location_or_provided_path(path)
     def authenticate_user!
       return if current_user
 
-      redirect_to new_trial_registration_path(glm_tracking_params), alert: I18n.t('devise.failure.unauthenticated')
+      redirect_to(
+        new_trial_registration_path(::Onboarding::Status.glm_tracking_params(params)),
+        alert: I18n.t('devise.failure.unauthenticated')
+      )
     end
 
     def lead_params
       params.permit(
+        *::Onboarding::Status::GLM_PARAMS,
         :company_name, :company_size, :first_name, :last_name, :phone_number,
-        :country, :state, :website_url, :glm_content, :glm_source
+        :country, :state, :website_url
       ).to_h
     end
 
     def trial_params
-      params.permit(:new_group_name, :namespace_id, :trial_entity, :glm_source, :glm_content)
+      params.permit(*::Onboarding::Status::GLM_PARAMS, :new_group_name, :namespace_id, :trial_entity)
       .with_defaults(organization_id: Current.organization_id).to_h
     end
 

ee/app/controllers/registrations/company_controller.rb

@@ -5,7 +5,6 @@ class CompanyController < ApplicationController
     include OneTrustCSP
     include GoogleAnalyticsCSP
     include GoogleSyndicationCSP
-    include RegistrationsTracking
     include ::Onboarding::SetRedirect
 
     layout 'minimal'
@@ -26,8 +25,9 @@ def create
       if result.success?
         track_event('successfully_submitted_form', onboarding_status.tracking_label)
 
-        response = Onboarding::StatusStepUpdateService
-                     .new(current_user, new_users_sign_up_group_path(glm_tracking_params)).execute
+        response = Onboarding::StatusStepUpdateService.new(
+          current_user, new_users_sign_up_group_path(::Onboarding::Status.glm_tracking_params(params))
+        ).execute
 
         redirect_to response[:step_url]
       else
@@ -44,6 +44,7 @@ def create
 
     def permitted_params
       params.permit(
+        *::Onboarding::Status::GLM_PARAMS,
         :company_name,
         :company_size,
         :first_name,
@@ -56,7 +57,7 @@ def permitted_params
         :role,
         :registration_objective,
         :jobs_to_be_done_other
-      ).merge(glm_tracking_params)
+      )
     end
 
     def track_event(action, label)

ee/app/controllers/registrations/groups_controller.rb

@@ -159,11 +159,11 @@ def project_params
     end
 
     def general_params
-      params.permit(:import_url).merge(glm_params)
+      params.permit(*::Onboarding::Status::GLM_PARAMS, :import_url)
     end
 
     def glm_params
-      params.permit(:glm_source, :glm_content)
+      ::Onboarding::Status.glm_tracking_params(params) # rubocop:disable Rails/StrongParams -- strong params are used in the method being called.
     end
   end
 end

ee/app/controllers/registrations/welcome_controller.rb

@@ -8,7 +8,6 @@ class WelcomeController < ApplicationController
     include ::Gitlab::Utils::StrongMemoize
     include ::Onboarding::Redirectable
     include ::Onboarding::SetRedirect
-    include RegistrationsTracking
 
     layout 'minimal'
 
@@ -76,7 +75,7 @@ def onboarding_status_params
     def passed_through_params
       update_params.slice(:role, :registration_objective)
                    .merge(params.permit(:jobs_to_be_done_other))
-                   .merge(glm_tracking_params)
+                   .merge(::Onboarding::Status.glm_tracking_params(params))
     end
 
     def iterable_params

ee/app/helpers/ee/registrations_helper.rb

@@ -21,11 +21,6 @@ def arkose_labs_data
       data.compact
     end
 
-    override :register_omniauth_params
-    def register_omniauth_params(local_assigns)
-      super.merge(glm_tracking_params.to_h).merge(local_assigns.slice(:trial))
-    end
-
     def unconfirmed_email_feature_enabled?
       ::Gitlab::CurrentSettings.delete_unconfirmed_users? &&
         (::Gitlab::CurrentSettings.email_confirmation_setting_soft? ||

ee/app/helpers/gitlab_subscriptions/trials_helper.rb

@@ -56,7 +56,7 @@ def should_ask_company_question?
 
     def glm_params
       strong_memoize(:glm_params) do
-        params.slice(:glm_source, :glm_content).to_unsafe_h
+        params.slice(*::Onboarding::Status::GLM_PARAMS).to_unsafe_h
       end
     end
 

ee/app/services/gitlab_subscriptions/trials/base_create_service.rb

@@ -53,7 +53,7 @@ def after_lead_success_hook
           # trigger new creation for next step...
           trial_selection_params = {
             step: TRIAL
-          }.merge(lead_params.slice(:glm_content, :glm_source))
+          }.merge(lead_params.slice(*::Onboarding::Status::GLM_PARAMS))
            .merge(trial_params.slice(:namespace_id))
 
           ServiceResponse.error(

ee/app/views/registrations/groups/new.html.haml

 - @html_class = "subscriptions-layout-html"
 - @hide_flash = true
 - page_title _('Your GitLab group')
-- form_params = { glm_source: params[:glm_source], glm_content: params[:glm_content] }
 - content_for :page_specific_javascripts do
   = render "layouts/google_tag_manager_head"
   = render "layouts/one_trust"
@@ -29,7 +28,7 @@
 
         .tab-content.gitlab-tab-content.gl-bg-white.js-group-project-tab-contents
           #blank-project-pane.tab-pane.js-toggle-container.active{ role: 'tabpanel' }
-            = gitlab_ui_form_for :project_group, url: users_sign_up_groups_path(form_params), html: { class: 'gl-show-field-errors gl-w-full gl-p-4 js-groups-projects-form' } do |f|
+            = gitlab_ui_form_for :project_group, url: users_sign_up_groups_path(::Onboarding::Status.glm_tracking_params(params)), html: { class: 'gl-show-field-errors gl-w-full gl-p-4 js-groups-projects-form' } do |f|
               = form_errors(@group, type: "Group")
               = form_errors(@project, type: "Project")
               = render 'layouts/flash'
@@ -50,7 +49,7 @@
 
           #import-project-pane.tab-pane.import-project-pane.js-toggle-container{ role: 'tabpanel' }
             - if import_sources_enabled?
-              = form_tag users_sign_up_groups_path(form_params), class: 'gl-show-field-errors gl-w-full gl-p-4 js-import-project-form' do
+              = form_tag users_sign_up_groups_path(::Onboarding::Status.glm_tracking_params(params)), class: 'gl-show-field-errors gl-w-full gl-p-4 js-import-project-form' do
                 = form_errors(@group, type: "Group")
                 = render 'layouts/flash'