diff --git a/app/assets/javascripts/pages/shared/form_error_tracker.js b/app/assets/javascripts/pages/shared/form_error_tracker.js index 294183c9fdb3beba970e0093cedaf40146567071..6e83307f9a41a87d3d50cb9a7c3bc29467eaba41 100644 --- a/app/assets/javascripts/pages/shared/form_error_tracker.js +++ b/app/assets/javascripts/pages/shared/form_error_tracker.js @@ -8,19 +8,29 @@ export default class FormErrorTracker { this.trackErrorOnEmptyField = FormErrorTracker.trackErrorOnEmptyField.bind(this); this.elements.forEach((element) => { - // on input change - element.addEventListener('input', this.trackErrorOnChange); - - // on invalid input - adding separately to track submit click without - // changing any input field - element.addEventListener('invalid', this.trackErrorOnEmptyField); + // https://gitlab.com/gitlab-org/gitlab/-/issues/494329 to revert this condition when + // blocker is implemented + const actionItem = element.hasChildNodes() ? element.firstElementChild : element; + + if (actionItem) { + // on item change + actionItem.addEventListener('input', this.trackErrorOnChange); + + // on invalid item - adding separately to track submit click without + // changing any field + actionItem.addEventListener('invalid', this.trackErrorOnEmptyField); + } }); } destroy() { this.elements.forEach((element) => { - element.removeEventListener('input', this.trackErrorOnChange); - element.removeEventListener('invalid', this.trackErrorOnEmptyField); + const actionItem = element.hasChildNodes() ? element.firstElementChild : element; + + if (actionItem) { + actionItem.removeEventListener('input', this.trackErrorOnChange); + actionItem.removeEventListener('invalid', this.trackErrorOnEmptyField); + } }); } diff --git a/ee/app/assets/javascripts/registrations/company/new/index.js b/ee/app/assets/javascripts/registrations/company/new/index.js index f8ab993908870e2cdaf5aca03862f737444e0b86..57547565417b6c98a99c2884d3fe04b5baf6ff2a 100644 --- a/ee/app/assets/javascripts/registrations/company/new/index.js +++ b/ee/app/assets/javascripts/registrations/company/new/index.js @@ -5,7 +5,7 @@ import { parseBoolean } from '~/lib/utils/common_utils'; export default () => { const el = document.querySelector('#js-registrations-company-form'); - const { submitPath, firstName, lastName, initialTrial } = el.dataset; + const { submitPath, firstName, lastName, initialTrial, trackActionForErrors } = el.dataset; return new Vue({ el, @@ -16,6 +16,7 @@ export default () => { lastName, }, submitPath, + trackActionForErrors, initialTrial: parseBoolean(initialTrial), }, render(createElement) { diff --git a/ee/app/assets/javascripts/registrations/components/company_form.vue b/ee/app/assets/javascripts/registrations/components/company_form.vue index 43cf273a5ab9428642930b8bc0829ed0636d21a3..c5a7690ddc0dfaab87e302eb7320d92a86d7cee0 100644 --- a/ee/app/assets/javascripts/registrations/components/company_form.vue +++ b/ee/app/assets/javascripts/registrations/components/company_form.vue @@ -10,6 +10,7 @@ import { } from 'ee/vue_shared/leads/constants'; import csrf from '~/lib/utils/csrf'; import { __ } from '~/locale'; +import FormErrorTracker from '~/pages/shared/form_error_tracker'; import CountryOrRegionSelector from 'ee/trials/components/country_or_region_selector.vue'; import { TRIAL_COMPANY_SIZE_PROMPT, @@ -44,6 +45,10 @@ export default { type: Boolean, default: false, }, + trackActionForErrors: { + type: String, + required: false, + }, }, data() { return { @@ -80,6 +85,9 @@ export default { return this.initialTrial ? '' : this.$options.i18n.footerDescriptionRegistration; }, }, + mounted() { + new FormErrorTracker(); // eslint-disable-line no-new + }, methods: { trackCompanyForm() { trackCompanyForm('ultimate_trial'); @@ -123,7 +131,9 @@ export default { id="first_name" :value="firstName" name="first_name" + class="js-track-error" data-testid="first_name" + :data-track-action-for-errors="trackActionForErrors" required /> </gl-form-group> @@ -137,7 +147,9 @@ export default { id="last_name" :value="lastName" name="last_name" + class="js-track-error" data-testid="last_name" + :data-track-action-for-errors="trackActionForErrors" required /> </gl-form-group> @@ -147,7 +159,9 @@ export default { id="company_name" :value="companyName" name="company_name" + class="js-track-error" data-testid="company_name" + :data-track-action-for-errors="trackActionForErrors" required /> </gl-form-group> @@ -156,14 +170,24 @@ export default { id="company_size" :value="companySize" name="company_size" + class="js-track-error" :options="companySizeOptionsWithDefault" value-field="id" text-field="name" data-testid="company_size" + :data-track-action-for-errors="trackActionForErrors" required /> </gl-form-group> - <country-or-region-selector :country="country" :state="state" data-testid="country" required /> + + <country-or-region-selector + :country="country" + :state="state" + data-testid="country" + :track-action-for-errors="trackActionForErrors" + required + /> + <gl-form-group :label="$options.i18n.phoneNumberLabel" :optional-text="$options.i18n.optional" diff --git a/ee/app/assets/javascripts/trials/components/country_or_region_selector.vue b/ee/app/assets/javascripts/trials/components/country_or_region_selector.vue index 66664a744ed02770ad8f60d0260bd8caad13875d..9a7711194182e5fb7b4b4b28722cf161ac2edc5b 100644 --- a/ee/app/assets/javascripts/trials/components/country_or_region_selector.vue +++ b/ee/app/assets/javascripts/trials/components/country_or_region_selector.vue @@ -33,6 +33,11 @@ export default { default: false, required: false, }, + trackActionForErrors: { + type: String, + default: null, + required: false, + }, }, data() { return { selectedCountry: this.country, selectedState: this.state, countries: [], states: [] }; @@ -44,6 +49,9 @@ export default { stateSelectPrompt: TRIAL_STATE_PROMPT, }, computed: { + countryClass() { + return this.trackActionForErrors ? 'js-track-error' : ''; + }, countryOptionsWithDefault() { return [ { @@ -118,10 +126,12 @@ export default { id="country" v-model="selectedCountry" name="country" + :class="countryClass" :options="countryOptionsWithDefault" value-field="id" text-field="name" data-testid="country-dropdown" + :data-track-action-for-errors="trackActionForErrors" :required="required" @change="selected" /> diff --git a/ee/app/controllers/registrations/company_controller.rb b/ee/app/controllers/registrations/company_controller.rb index 6aef178373f8dafb99ea2c7e782b34811b1630fd..599e1ab3ac9f2fab82bb353df290c8413699e663 100644 --- a/ee/app/controllers/registrations/company_controller.rb +++ b/ee/app/controllers/registrations/company_controller.rb @@ -17,20 +17,24 @@ class CompanyController < ApplicationController helper_method :onboarding_status def new - track_event('render') + track_event('render', onboarding_status.tracking_label) end def create result = GitlabSubscriptions::CreateCompanyLeadService.new(user: current_user, params: permitted_params).execute if result.success? - track_event('successfully_submitted_form') + 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 redirect_to response[:step_url] else + result.errors.each do |error| + track_event("track_#{onboarding_status.tracking_label}_error", error.parameterize.underscore) + end + flash.now[:alert] = result.errors.to_sentence render :new, status: :unprocessable_entity end @@ -55,8 +59,8 @@ def permitted_params ).merge(glm_tracking_params) end - def track_event(action) - ::Gitlab::Tracking.event(self.class.name, action, user: current_user, label: onboarding_status.tracking_label) + def track_event(action, label) + ::Gitlab::Tracking.event(self.class.name, action, user: current_user, label: label) end def onboarding_status diff --git a/ee/app/helpers/gitlab_subscriptions/trials_helper.rb b/ee/app/helpers/gitlab_subscriptions/trials_helper.rb index 6e61082f52a3737db67f92be3b419e6606957a07..bd40a954ae67f15b2d770f37bbfff276703eb8fe 100644 --- a/ee/app/helpers/gitlab_subscriptions/trials_helper.rb +++ b/ee/app/helpers/gitlab_subscriptions/trials_helper.rb @@ -45,7 +45,8 @@ def create_company_form_data(onboarding_status) submit_path: users_sign_up_company_path(submit_params), first_name: current_user.first_name, last_name: current_user.last_name, - initial_trial: onboarding_status.initial_trial?.to_s + initial_trial: onboarding_status.initial_trial?.to_s, + track_action_for_errors: onboarding_status.tracking_label } end diff --git a/ee/spec/controllers/registrations/company_controller_spec.rb b/ee/spec/controllers/registrations/company_controller_spec.rb index 84780f886ce20cb26dff561dfbdbafa97f99ddcd..51cb60cd0ef30c413f190aa606fd92fcc2817f09 100644 --- a/ee/spec/controllers/registrations/company_controller_spec.rb +++ b/ee/spec/controllers/registrations/company_controller_spec.rb @@ -78,7 +78,6 @@ describe '#create' do using RSpec::Parameterized::TableSyntax - let(:trial_registration) { 'false' } let(:glm_params) do { glm_source: 'some_source', @@ -120,8 +119,6 @@ end context 'when it is a trial registration' do - let(:trial_registration) { 'true' } - before do user.update!( onboarding_status_registration_type: 'trial', onboarding_status_initial_registration_type: 'trial' @@ -141,8 +138,6 @@ end context 'when driving from the onboarding_status.initial_registration_type' do - let(:trial_registration) { 'false' } - before do user.update!(onboarding_status_initial_registration_type: 'trial') end @@ -259,6 +254,17 @@ end context 'with snowplow tracking' do + it 'tracks error event' do + post_create + + expect_snowplow_event( + category: described_class.name, + action: 'track_free_registration_error', + user: user, + label: 'failed' + ) + end + it 'does not track successful submission event' do post_create @@ -271,7 +277,22 @@ end context 'when in trial flow' do - it 'tracks successful submission event' do + before do + user.update!(onboarding_status_registration_type: 'trial') + end + + it 'tracks error event' do + post_create + + expect_snowplow_event( + category: described_class.name, + action: 'track_trial_registration_error', + user: user, + label: 'failed' + ) + end + + it 'does not track successful submission event' do post_create expect_no_snowplow_event( diff --git a/ee/spec/frontend/registrations/company/new/components/company_form_spec.js b/ee/spec/frontend/registrations/company/new/components/company_form_spec.js index 3f3610afa021a2c25028d2498e7f7c0bf43a8a79..c91ab7a60e5c4a858b465019901db3e275c0b344 100644 --- a/ee/spec/frontend/registrations/company/new/components/company_form_spec.js +++ b/ee/spec/frontend/registrations/company/new/components/company_form_spec.js @@ -18,6 +18,7 @@ describe('CompanyForm', () => { firstName: 'Joe', lastName: 'Doe', }, + trackActionForErrors: '_trackActionForErrors_', ...provideData, }, }); diff --git a/ee/spec/frontend/trials/components/country_or_region_selector_spec.js b/ee/spec/frontend/trials/components/country_or_region_selector_spec.js index d3203a8f4f1bd211eb2752ad01a26c053a6faf2d..938c8f86fa0510a7b9d3bdf8fdd7a9b08e283e54 100644 --- a/ee/spec/frontend/trials/components/country_or_region_selector_spec.js +++ b/ee/spec/frontend/trials/components/country_or_region_selector_spec.js @@ -51,6 +51,16 @@ describe('CountryOrRegionSelector', () => { `('has the default injected value for $testid', ({ testid, value }) => { expect(findFormInput(testid).attributes('value')).toBe(value); }); + + describe('with trackActionForErrors', () => { + beforeEach(() => { + wrapper = createComponent({ trackActionForErrors: '_trackActionForErrors_' }); + }); + + it('adds track error class for country selector', () => { + expect(findFormInput('country-dropdown').attributes('class')).toContain('js-track-error'); + }); + }); }); describe.each` diff --git a/ee/spec/helpers/gitlab_subscriptions/trials_helper_spec.rb b/ee/spec/helpers/gitlab_subscriptions/trials_helper_spec.rb index dcf0e9f19768617e84aa7b212d42b912fc49eac6..0934500bff17eefdbc396fc311f514905c320788 100644 --- a/ee/spec/helpers/gitlab_subscriptions/trials_helper_spec.rb +++ b/ee/spec/helpers/gitlab_subscriptions/trials_helper_spec.rb @@ -219,7 +219,8 @@ submit_path: "/users/sign_up/company?#{extra_params.to_query}", first_name: user.first_name, last_name: user.last_name, - initial_trial: 'false' + initial_trial: 'false', + track_action_for_errors: 'free_registration' } expect(helper.create_company_form_data(::Onboarding::Status.new({}, {}, user))).to match(attributes) diff --git a/ee/spec/views/registrations/company/new.html.haml_spec.rb b/ee/spec/views/registrations/company/new.html.haml_spec.rb index f62a0f78ce35a332549ab1f54cbc63428b40e6ee..c4e1c08b1a8bc19b7b1b0c9cb7916c81f0c6f534 100644 --- a/ee/spec/views/registrations/company/new.html.haml_spec.rb +++ b/ee/spec/views/registrations/company/new.html.haml_spec.rb @@ -6,7 +6,7 @@ let(:user) { build_stubbed(:user) } let(:initial_trial?) { false } let(:onboarding_status) do - instance_double(::Onboarding::Status, initial_trial?: initial_trial?) + instance_double(::Onboarding::Status, initial_trial?: initial_trial?, tracking_label: 'free_registration') end before do