diff --git a/app/assets/javascripts/google_tag_manager/index.js b/app/assets/javascripts/google_tag_manager/index.js new file mode 100644 index 00000000000..f9f0a5fb90c --- /dev/null +++ b/app/assets/javascripts/google_tag_manager/index.js @@ -0,0 +1,125 @@ +import { logError } from '~/lib/logger'; + +const isSupported = () => Boolean(window.dataLayer) && gon.features?.gitlabGtmDatalayer; + +const pushEvent = (event, args = {}) => { + if (!window.dataLayer) { + return; + } + + try { + window.dataLayer.push({ + event, + ...args, + }); + } catch (e) { + logError('Unexpected error while pushing to dataLayer', e); + } +}; + +const pushAccountSubmit = (accountType, accountMethod) => + pushEvent('accountSubmit', { accountType, accountMethod }); + +const trackFormSubmission = (accountType) => { + const form = document.getElementById('new_new_user'); + form.addEventListener('submit', () => { + pushAccountSubmit(accountType, 'form'); + }); +}; + +const trackOmniAuthSubmission = (accountType) => { + const links = document.querySelectorAll('.js-oauth-login'); + links.forEach((link) => { + const { provider } = link.dataset; + link.addEventListener('click', () => { + pushAccountSubmit(accountType, provider); + }); + }); +}; + +export const trackFreeTrialAccountSubmissions = () => { + if (!isSupported()) { + return; + } + + trackFormSubmission('freeThirtyDayTrial'); + trackOmniAuthSubmission('freeThirtyDayTrial'); +}; + +export const trackNewRegistrations = () => { + if (!isSupported()) { + return; + } + + trackFormSubmission('standardSignUp'); + trackOmniAuthSubmission('standardSignUp'); +}; + +export const trackSaasTrialSubmit = () => { + if (!isSupported()) { + return; + } + + const form = document.getElementById('new_trial'); + form.addEventListener('submit', () => { + pushEvent('saasTrialSubmit'); + }); +}; + +export const trackSaasTrialSkip = () => { + if (!isSupported()) { + return; + } + + const skipLink = document.querySelector('.js-skip-trial'); + skipLink.addEventListener('click', () => { + pushEvent('saasTrialSkip'); + }); +}; + +export const trackSaasTrialGroup = () => { + if (!isSupported()) { + return; + } + + const form = document.getElementById('new_group'); + form.addEventListener('submit', () => { + pushEvent('saasTrialGroup'); + }); +}; + +export const trackSaasTrialProject = () => { + if (!isSupported()) { + return; + } + + const form = document.getElementById('new_project'); + form.addEventListener('submit', () => { + pushEvent('saasTrialProject'); + }); +}; + +export const trackSaasTrialProjectImport = () => { + if (!isSupported()) { + return; + } + + const importButtons = document.querySelectorAll('.js-import-project-btn'); + importButtons.forEach((button) => { + button.addEventListener('click', () => { + const { platform } = button.dataset; + pushEvent('saasTrialProjectImport', { saasProjectImport: platform }); + }); + }); +}; + +export const trackSaasTrialGetStarted = () => { + if (!isSupported()) { + return; + } + + const getStartedButton = document.querySelector('.js-get-started-btn'); + getStartedButton.addEventListener('click', () => { + pushEvent('saasTrialGetStarted'); + }); +}; diff --git a/app/assets/javascripts/pages/registrations/new/index.js b/app/assets/javascripts/pages/registrations/new/index.js index ae605edeaf0..8bbe81a9ed5 100644 --- a/app/assets/javascripts/pages/registrations/new/index.js +++ b/app/assets/javascripts/pages/registrations/new/index.js @@ -1,3 +1,5 @@ +import { trackNewRegistrations } from '~/google_tag_manager'; + import NoEmojiValidator from '~/emoji/no_emoji_validator'; import LengthValidator from '~/pages/sessions/new/length_validator'; import UsernameValidator from '~/pages/sessions/new/username_validator'; @@ -5,3 +7,5 @@ import UsernameValidator from '~/pages/sessions/new/username_validator'; new UsernameValidator(); // eslint-disable-line no-new new LengthValidator(); // eslint-disable-line no-new new NoEmojiValidator(); // eslint-disable-line no-new + +trackNewRegistrations(); diff --git a/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue b/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue index ce7cbafb97d..709d3592828 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue @@ -67,7 +67,7 @@ export default { diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index ed3facd72c5..bd7631c7e78 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -18,6 +18,7 @@ class RegistrationsController < Devise::RegistrationsController def new @resource = build_resource + push_frontend_feature_flag(:gitlab_gtm_datalayer, type: :ops) end def create diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index c1a74382d46..736d0167c69 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -177,15 +177,13 @@ module AuthHelper def google_tag_manager_enabled? return false unless Gitlab.dev_env_or_com? - has_config_key = if Feature.enabled?(:gtm_nonce, type: :ops) - extra_config.has_key?('google_tag_manager_nonce_id') && - extra_config.google_tag_manager_nonce_id.present? - else - extra_config.has_key?('google_tag_manager_id') && - extra_config.google_tag_manager_id.present? - end - - has_config_key && !current_user + if Feature.enabled?(:gtm_nonce, type: :ops) + extra_config.has_key?('google_tag_manager_nonce_id') && + extra_config.google_tag_manager_nonce_id.present? + else + extra_config.has_key?('google_tag_manager_id') && + extra_config.google_tag_manager_id.present? + end end def google_tag_manager_id diff --git a/app/helpers/tracking_helper.rb b/app/helpers/tracking_helper.rb index 3f53bd535b2..1beb88548c5 100644 --- a/app/helpers/tracking_helper.rb +++ b/app/helpers/tracking_helper.rb @@ -13,6 +13,10 @@ module TrackingHelper } end + def tracking_attrs_data(label, action, property) + tracking_attrs(label, action, property).fetch(:data, {}) + end + private def tracking_enabled? diff --git a/app/views/devise/shared/_signup_omniauth_provider_list.haml b/app/views/devise/shared/_signup_omniauth_provider_list.haml index c24e8770f05..6688308cd71 100644 --- a/app/views/devise/shared/_signup_omniauth_provider_list.haml +++ b/app/views/devise/shared/_signup_omniauth_provider_list.haml @@ -2,7 +2,7 @@ = _("Create an account using:") .gl-display-flex.gl-justify-content-between.gl-flex-wrap - providers.each do |provider| - = link_to omniauth_authorize_path(:user, provider), method: :post, class: "btn gl-button btn-default gl-w-full gl-mb-3 js-oauth-login #{qa_class_for_provider(provider)}", id: "oauth-login-#{provider}" do + = link_to omniauth_authorize_path(:user, provider), method: :post, class: "btn gl-button btn-default gl-w-full gl-mb-3 js-oauth-login #{qa_class_for_provider(provider)}", data: { provider: provider }, id: "oauth-login-#{provider}" do - if provider_has_icon?(provider) = provider_image_tag(provider) %span.gl-button-text diff --git a/app/views/projects/_import_project_pane.html.haml b/app/views/projects/_import_project_pane.html.haml index 81d9726fcdc..9b89f6ceea0 100644 --- a/app/views/projects/_import_project_pane.html.haml +++ b/app/views/projects/_import_project_pane.html.haml @@ -8,22 +8,22 @@ .import-buttons - if gitlab_project_import_enabled? .import_gitlab_project.has-tooltip{ data: { container: 'body', qa_selector: 'gitlab_import_button' } } - = link_to new_import_gitlab_project_path, class: 'gl-button btn-default btn btn_import_gitlab_project', **tracking_attrs(track_label, 'click_button', 'gitlab_export') do + = link_to new_import_gitlab_project_path, class: 'gl-button btn-default btn btn_import_gitlab_project js-import-project-btn', **tracking_attrs(track_label, 'click_button', 'gitlab_export') do .gl-button-icon = sprite_icon('tanuki') = _("GitLab export") - if github_import_enabled? %div - = link_to new_import_github_path, class: 'gl-button btn-default btn js-import-github', **tracking_attrs(track_label, 'click_button', 'github') do + = link_to new_import_github_path, class: 'gl-button btn-default btn js-import-github js-import-project-btn', data: { platform: 'github', **tracking_attrs_data(track_label, 'click_button', 'github') } do .gl-button-icon = sprite_icon('github') GitHub - if bitbucket_import_enabled? %div - = link_to status_import_bitbucket_path, class: "gl-button btn-default btn import_bitbucket #{'how_to_import_link' unless bitbucket_import_configured?}", - **tracking_attrs(track_label, 'click_button', 'bitbucket_cloud') do + = link_to status_import_bitbucket_path, class: "gl-button btn-default btn import_bitbucket js-import-project-btn #{'how_to_import_link' unless bitbucket_import_configured?}", + data: { platform: 'bitbucket_cloud', **tracking_attrs_data(track_label, 'click_button', 'bitbucket_cloud') } do .gl-button-icon = sprite_icon('bitbucket') Bitbucket Cloud @@ -31,15 +31,14 @@ = render 'projects/bitbucket_import_modal' - if bitbucket_server_import_enabled? %div - = link_to status_import_bitbucket_server_path, class: "gl-button btn-default btn import_bitbucket", **tracking_attrs(track_label, 'click_button', 'bitbucket_server') do + = link_to status_import_bitbucket_server_path, class: "gl-button btn-default btn import_bitbucket js-import-project-btn", data: { platform: 'bitbucket_server', **tracking_attrs_data(track_label, 'click_button', 'bitbucket_server') } do .gl-button-icon = sprite_icon('bitbucket') Bitbucket Server %div - if gitlab_import_enabled? %div - = link_to status_import_gitlab_path, class: "gl-button btn-default btn import_gitlab #{'how_to_import_link' unless gitlab_import_configured?}", - **tracking_attrs(track_label, 'click_button', 'gitlab_com') do + = link_to status_import_gitlab_path, class: "gl-button btn-default btn import_gitlab js-import-project-btn #{'how_to_import_link' unless gitlab_import_configured?}", data: { platform: 'gitlab_com', **tracking_attrs_data(track_label, 'click_button', 'gitlab_com') } do .gl-button-icon = sprite_icon('tanuki') = _("GitLab.com") @@ -48,35 +47,35 @@ - if fogbugz_import_enabled? %div - = link_to new_import_fogbugz_path, class: 'gl-button btn-default btn import_fogbugz', **tracking_attrs(track_label, 'click_button', 'fogbugz') do + = link_to new_import_fogbugz_path, class: 'gl-button btn-default btn import_fogbugz js-import-project-btn', data: { platform: 'fogbugz', **tracking_attrs_data(track_label, 'click_button', 'fogbugz') } do .gl-button-icon = sprite_icon('bug') FogBugz - if gitea_import_enabled? %div - = link_to new_import_gitea_path, class: 'gl-button btn-default btn import_gitea', **tracking_attrs(track_label, 'click_button', 'gitea') do + = link_to new_import_gitea_path, class: 'gl-button btn-default btn import_gitea js-import-project-btn', data: { platform: 'gitea', **tracking_attrs_data(track_label, 'click_button', 'gitea') } do .gl-button-icon = custom_icon('gitea_logo') Gitea - if git_import_enabled? %div - %button.gl-button.btn-default.btn.btn-svg.js-toggle-button.js-import-git-toggle-button{ type: "button", data: { toggle_open_class: 'active' }, **tracking_attrs(track_label, 'click_button', 'repo_url') } + %button.gl-button.btn-default.btn.btn-svg.js-toggle-button.js-import-git-toggle-button.js-import-project-btn{ type: "button", data: { platform: 'repo_url', toggle_open_class: 'active', **tracking_attrs_data(track_label, 'click_button', 'repo_url') } } .gl-button-icon = sprite_icon('link', css_class: 'gl-icon') = _('Repo by URL') - if manifest_import_enabled? %div - = link_to new_import_manifest_path, class: 'gl-button btn-default btn import_manifest', **tracking_attrs(track_label, 'click_button', 'manifest_file') do + = link_to new_import_manifest_path, class: 'gl-button btn-default btn import_manifest js-import-project-btn', data: { platform: 'manifest_file', **tracking_attrs_data(track_label, 'click_button', 'manifest_file') } do .gl-button-icon = sprite_icon('doc-text') Manifest file - if phabricator_import_enabled? %div - = link_to new_import_phabricator_path, class: 'gl-button btn-default btn import_phabricator', data: { track_label: "#{track_label}", track_action: "click_button", track_property: "phabricator" } do + = link_to new_import_phabricator_path, class: 'gl-button btn-default btn import_phabricator js-import-project-btn', data: { platform: 'phabricator', track_label: "#{track_label}", track_action: "click_button", track_property: "phabricator" } do .gl-button-icon = custom_icon('issues') = _("Phabricator Tasks") diff --git a/app/views/registrations/welcome/show.html.haml b/app/views/registrations/welcome/show.html.haml index 65a1ffa3e46..47fb8f9d253 100644 --- a/app/views/registrations/welcome/show.html.haml +++ b/app/views/registrations/welcome/show.html.haml @@ -2,6 +2,10 @@ - page_title _('Your profile') - add_page_specific_style 'page_bundles/signup' - gitlab_experience_text = _('To personalize your GitLab experience, we\'d like to know a bit more about you') +- content_for :page_specific_javascripts do + = render "layouts/google_tag_manager_head" + = render "layouts/one_trust" += render "layouts/google_tag_manager_body" .row.gl-flex-grow-1 .d-flex.gl-flex-direction-column.gl-align-items-center.gl-w-full.gl-px-5.gl-pb-5 diff --git a/config/feature_flags/ops/gitlab_gtm_datalayer.yml b/config/feature_flags/ops/gitlab_gtm_datalayer.yml new file mode 100644 index 00000000000..f41506ce114 --- /dev/null +++ b/config/feature_flags/ops/gitlab_gtm_datalayer.yml @@ -0,0 +1,8 @@ +--- +name: gitlab_gtm_datalayer +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76305 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348932 +milestone: '14.6' +type: ops +group: group::buyer experience +default_enabled: false diff --git a/config/initializers/active_record_lifecycle.rb b/config/initializers/active_record_lifecycle.rb index 8d4b6d61abe..92cc1d81617 100644 --- a/config/initializers/active_record_lifecycle.rb +++ b/config/initializers/active_record_lifecycle.rb @@ -5,7 +5,7 @@ if defined?(ActiveRecord::Base) && !Gitlab::Runtime.sidekiq? Gitlab::Cluster::LifecycleEvents.on_worker_start do ActiveSupport.on_load(:active_record) do - ActiveRecord::Base.establish_connection + ActiveRecord::Base.establish_connection # rubocop: disable Database/EstablishConnection Gitlab::AppLogger.debug("ActiveRecord connection established") end diff --git a/config/initializers/database_config.rb b/config/initializers/database_config.rb index a3172fae027..050ab1d9b3e 100644 --- a/config/initializers/database_config.rb +++ b/config/initializers/database_config.rb @@ -13,6 +13,6 @@ Gitlab.ee do # The Geo::TrackingBase model does not yet use connects_to. So, # this will not properly support geo: from config/databse.yml # file yet. This is ACK of the current state and will be fixed. - Geo::TrackingBase.establish_connection(Gitlab::Database.geo_db_config_with_default_pool_size) + Geo::TrackingBase.establish_connection(Gitlab::Database.geo_db_config_with_default_pool_size) # rubocop: disable Database/EstablishConnection end end diff --git a/lib/gitlab/merge_requests/commit_message_generator.rb b/lib/gitlab/merge_requests/commit_message_generator.rb index 39247a47bf5..2180de29e16 100644 --- a/lib/gitlab/merge_requests/commit_message_generator.rb +++ b/lib/gitlab/merge_requests/commit_message_generator.rb @@ -29,48 +29,41 @@ module Gitlab 'target_branch' => ->(merge_request, _) { merge_request.target_branch.to_s }, 'title' => ->(merge_request, _) { merge_request.title }, 'issues' => ->(merge_request, _) do - return "" if merge_request.visible_closing_issues_for.blank? + return if merge_request.visible_closing_issues_for.blank? closes_issues_references = merge_request.visible_closing_issues_for.map do |issue| issue.to_reference(merge_request.target_project) end "Closes #{closes_issues_references.to_sentence}" end, - 'description' => ->(merge_request, _) { merge_request.description.presence || '' }, + 'description' => ->(merge_request, _) { merge_request.description }, 'reference' => ->(merge_request, _) { merge_request.to_reference(full: true) }, - 'first_commit' => -> (merge_request, _) { merge_request.first_commit&.safe_message&.strip.presence || '' }, + 'first_commit' => -> (merge_request, _) { merge_request.first_commit&.safe_message&.strip }, 'first_multiline_commit' => -> (merge_request, _) { merge_request.first_multiline_commit&.safe_message&.strip.presence || merge_request.title }, 'url' => ->(merge_request, _) { Gitlab::UrlBuilder.build(merge_request) }, 'approved_by' => ->(merge_request, _) { merge_request.approved_by_users.map { |user| "Approved-by: #{user.name} <#{user.commit_email_or_default}>" }.join("\n") }, 'merged_by' => ->(_, user) { "#{user&.name} <#{user&.commit_email_or_default}>" } }.freeze - PLACEHOLDERS_REGEX = Regexp.union(PLACEHOLDERS.keys.map do |key| - Regexp.new(Regexp.escape(key)) - end).freeze - - BLANK_PLACEHOLDERS_REGEXES = (PLACEHOLDERS.map do |key, value| - [key, Regexp.new("[\n\r]+%{#{Regexp.escape(key)}}$")] - end).to_h.freeze + PLACEHOLDERS_COMBINED_REGEX = /%{(#{Regexp.union(PLACEHOLDERS.keys)})}/.freeze def replace_placeholders(message) - # convert CRLF to LF + # Convert CRLF to LF. message = message.delete("\r") - # Remove placeholders that correspond to empty values and are the last word in the line - # along with all whitespace characters preceding them. - # This allows us to recreate previous default merge commit message behaviour - we skipped new line character - # before empty description and before closed issues when none were present. - PLACEHOLDERS.each do |key, value| - unless value.call(merge_request, current_user).present? - message = message.gsub(BLANK_PLACEHOLDERS_REGEXES[key], '') - end + used_variables = message.scan(PLACEHOLDERS_COMBINED_REGEX).map { |value| value[0] }.uniq + values = used_variables.to_h do |variable_name| + ["%{#{variable_name}}", PLACEHOLDERS[variable_name].call(merge_request, current_user)] end + names_of_empty_variables = values.filter_map { |name, value| name if value.blank? } - Gitlab::StringPlaceholderReplacer - .replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key| - PLACEHOLDERS[key].call(merge_request, current_user) - end + # Remove placeholders that correspond to empty values and are the only word in a line + # along with all whitespace characters preceding them. + message = message.gsub(/[\n\r]+#{Regexp.union(names_of_empty_variables)}$/, '') if names_of_empty_variables.present? + # Substitute all variables with their values. + message = message.gsub(Regexp.union(values.keys), values) if values.present? + + message end end end diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 9e733fc3a0f..efb0e1ef1e1 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -170,7 +170,7 @@ namespace :gitlab do # the `ActiveRecord::Base.connection` might be switched to another one # This is due to `if should_reconnect`: # https://github.com/rails/rails/blob/a81aeb63a007ede2fe606c50539417dada9030c7/activerecord/lib/active_record/railties/databases.rake#L622 - ActiveRecord::Base.establish_connection :main + ActiveRecord::Base.establish_connection :main # rubocop: disable Database/EstablishConnection Rake::Task['gitlab:db:create_dynamic_partitions'].invoke end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ed0371f3f89..85fd0b37844 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10126,9 +10126,6 @@ msgstr "" msgid "CreateValueStreamForm|Code stage start" msgstr "" -msgid "CreateValueStreamForm|Create Value Stream" -msgstr "" - msgid "CreateValueStreamForm|Create from default template" msgstr "" @@ -10138,13 +10135,16 @@ msgstr "" msgid "CreateValueStreamForm|Create new Value Stream" msgstr "" +msgid "CreateValueStreamForm|Create value stream" +msgstr "" + msgid "CreateValueStreamForm|Default stages" msgstr "" msgid "CreateValueStreamForm|Default stages can only be hidden or re-ordered" msgstr "" -msgid "CreateValueStreamForm|Edit Value Stream" +msgid "CreateValueStreamForm|Edit value stream" msgstr "" msgid "CreateValueStreamForm|Editing stage" diff --git a/rubocop/cop/database/establish_connection.rb b/rubocop/cop/database/establish_connection.rb new file mode 100644 index 00000000000..20454887ce2 --- /dev/null +++ b/rubocop/cop/database/establish_connection.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Database + class EstablishConnection < RuboCop::Cop::Cop + MSG = "Don't establish new database connections, as this slows down " \ + 'tests and may result in new connections using an incorrect configuration' + + def_node_matcher :establish_connection?, <<~PATTERN + (send (const ...) :establish_connection ...) + PATTERN + + def on_send(node) + add_offense(node, location: :expression) if establish_connection?(node) + end + end + end + end +end diff --git a/scripts/insert-rspec-profiling-data b/scripts/insert-rspec-profiling-data index b2011858558..be25972644c 100755 --- a/scripts/insert-rspec-profiling-data +++ b/scripts/insert-rspec-profiling-data @@ -12,7 +12,7 @@ module RspecProfiling # This disables the automatic creation of the database and # table. In the future, we may want a way to specify the host of # the database to connect so that we can call #install. - Result.establish_connection(results_url) + Result.establish_connection(results_url) # rubocop: disable Database/EstablishConnection end def results_url diff --git a/spec/frontend/google_tag_manager/index_spec.js b/spec/frontend/google_tag_manager/index_spec.js new file mode 100644 index 00000000000..05094b40e59 --- /dev/null +++ b/spec/frontend/google_tag_manager/index_spec.js @@ -0,0 +1,250 @@ +import { merge } from 'lodash'; +import { + trackFreeTrialAccountSubmissions, + trackNewRegistrations, + trackSaasTrialSubmit, + trackSaasTrialSkip, + trackSaasTrialGroup, + trackSaasTrialProject, + trackSaasTrialProjectImport, + trackSaasTrialGetStarted, +} from '~/google_tag_manager'; +import { setHTMLFixture } from 'helpers/fixtures'; +import { logError } from '~/lib/logger'; + +jest.mock('~/lib/logger'); + +describe('~/google_tag_manager/index', () => { + let spy; + + beforeEach(() => { + spy = jest.fn(); + + window.dataLayer = { + push: spy, + }; + window.gon.features = { + gitlabGtmDatalayer: true, + }; + }); + + const createHTML = ({ links = [], forms = [] } = {}) => { + // .foo elements are used to test elements which shouldn't do anything + const allLinks = links.concat({ cls: 'foo' }); + const allForms = forms.concat({ cls: 'foo' }); + + const el = document.createElement('div'); + + allLinks.forEach(({ cls = '', id = '', href = '#', text = 'Hello', attributes = {} }) => { + const a = document.createElement('a'); + a.id = id; + a.href = href || '#'; + a.className = cls; + a.textContent = text; + + Object.entries(attributes).forEach(([key, value]) => { + a.setAttribute(key, value); + }); + + el.append(a); + }); + + allForms.forEach(({ cls = '', id = '' }) => { + const form = document.createElement('form'); + form.id = id; + form.className = cls; + + el.append(form); + }); + + return el.innerHTML; + }; + + const triggerEvent = (selector, eventType) => { + const el = document.querySelector(selector); + + el.dispatchEvent(new Event(eventType)); + }; + + const getSelector = ({ id, cls }) => (id ? `#${id}` : `.${cls}`); + + const createTestCase = (subject, { forms = [], links = [] }) => { + const expectedFormEvents = forms.map(({ expectation, ...form }) => ({ + selector: getSelector(form), + trigger: 'submit', + expectation, + })); + + const expectedLinkEvents = links.map(({ expectation, ...link }) => ({ + selector: getSelector(link), + trigger: 'click', + expectation, + })); + + return [ + subject, + { + forms, + links, + expectedEvents: [...expectedFormEvents, ...expectedLinkEvents], + }, + ]; + }; + + const createOmniAuthTestCase = (subject, accountType) => + createTestCase(subject, { + forms: [ + { + id: 'new_new_user', + expectation: { + event: 'accountSubmit', + accountMethod: 'form', + accountType, + }, + }, + ], + links: [ + { + // id is needed so that the test selects the right element to trigger + id: 'test-0', + cls: 'js-oauth-login', + attributes: { + 'data-provider': 'myspace', + }, + expectation: { + event: 'accountSubmit', + accountMethod: 'myspace', + accountType, + }, + }, + { + id: 'test-1', + cls: 'js-oauth-login', + attributes: { + 'data-provider': 'gitlab', + }, + expectation: { + event: 'accountSubmit', + accountMethod: 'gitlab', + accountType, + }, + }, + ], + }); + + describe.each([ + createOmniAuthTestCase(trackFreeTrialAccountSubmissions, 'freeThirtyDayTrial'), + createOmniAuthTestCase(trackNewRegistrations, 'standardSignUp'), + createTestCase(trackSaasTrialSubmit, { + forms: [{ id: 'new_trial', expectation: { event: 'saasTrialSubmit' } }], + }), + createTestCase(trackSaasTrialSkip, { + links: [{ cls: 'js-skip-trial', expectation: { event: 'saasTrialSkip' } }], + }), + createTestCase(trackSaasTrialGroup, { + forms: [{ id: 'new_group', expectation: { event: 'saasTrialGroup' } }], + }), + createTestCase(trackSaasTrialProject, { + forms: [{ id: 'new_project', expectation: { event: 'saasTrialProject' } }], + }), + createTestCase(trackSaasTrialProjectImport, { + links: [ + { + id: 'js-test-btn-0', + cls: 'js-import-project-btn', + attributes: { 'data-platform': 'bitbucket' }, + expectation: { event: 'saasTrialProjectImport', saasProjectImport: 'bitbucket' }, + }, + { + // id is neeeded so we trigger the right element in the test + id: 'js-test-btn-1', + cls: 'js-import-project-btn', + attributes: { 'data-platform': 'github' }, + expectation: { event: 'saasTrialProjectImport', saasProjectImport: 'github' }, + }, + ], + }), + createTestCase(trackSaasTrialGetStarted, { + links: [ + { + cls: 'js-get-started-btn', + expectation: { event: 'saasTrialGetStarted' }, + }, + ], + }), + ])('%p', (subject, { links = [], forms = [], expectedEvents }) => { + beforeEach(() => { + setHTMLFixture(createHTML({ links, forms })); + + subject(); + }); + + it.each(expectedEvents)('when %p', ({ selector, trigger, expectation }) => { + expect(spy).not.toHaveBeenCalled(); + + triggerEvent(selector, trigger); + + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith(expectation); + expect(logError).not.toHaveBeenCalled(); + }); + + it('when random link is clicked, does nothing', () => { + triggerEvent('a.foo', 'click'); + + expect(spy).not.toHaveBeenCalled(); + }); + + it('when random form is submitted, does nothing', () => { + triggerEvent('form.foo', 'submit'); + + expect(spy).not.toHaveBeenCalled(); + }); + }); + + describe.each([ + { dataLayer: null }, + { gon: { features: null } }, + { gon: { features: { gitlabGtmDatalayer: false } } }, + ])('when window %o', (windowAttrs) => { + beforeEach(() => { + merge(window, windowAttrs); + }); + + it('no ops', () => { + setHTMLFixture(createHTML({ forms: [{ id: 'new_project' }] })); + + trackSaasTrialProject(); + + triggerEvent('#new_project', 'submit'); + + expect(spy).not.toHaveBeenCalled(); + expect(logError).not.toHaveBeenCalled(); + }); + }); + + describe('when window.dataLayer throws error', () => { + const pushError = new Error('test'); + + beforeEach(() => { + window.dataLayer = { + push() { + throw pushError; + }, + }; + }); + + it('logs error', () => { + setHTMLFixture(createHTML({ forms: [{ id: 'new_project' }] })); + + trackSaasTrialProject(); + + triggerEvent('#new_project', 'submit'); + + expect(logError).toHaveBeenCalledWith( + 'Unexpected error while pushing to dataLayer', + pushError, + ); + }); + }); +}); diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index b481c214ca1..4bb09699db4 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -312,12 +312,6 @@ RSpec.describe AuthHelper do it { is_expected.to be_truthy } end - context 'when current user is set' do - let(:user) { instance_double('User') } - - it { is_expected.to eq(false) } - end - context 'when no key is set' do before do stub_config(extra: {}) diff --git a/spec/lib/gitlab/database/bulk_update_spec.rb b/spec/lib/gitlab/database/bulk_update_spec.rb index 9a6463c99fa..08b4d50f83b 100644 --- a/spec/lib/gitlab/database/bulk_update_spec.rb +++ b/spec/lib/gitlab/database/bulk_update_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Gitlab::Database::BulkUpdate do before do configuration_hash = ActiveRecord::Base.connection_db_config.configuration_hash - ActiveRecord::Base.establish_connection( + ActiveRecord::Base.establish_connection( # rubocop: disable Database/EstablishConnection configuration_hash.merge(prepared_statements: prepared_statements) ) end diff --git a/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb b/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb index 28318de12c9..ff10298f71e 100644 --- a/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb +++ b/spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb @@ -58,6 +58,19 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do end end + context 'when project has commit template with only the title' do + let(:merge_request) do + double(:merge_request, title: 'Fixes', target_project: project, to_reference: '!123', metrics: nil, merge_user: nil) + end + + let(message_template_name) { '%{title}' } + + it 'evaluates only necessary variables' do + expect(result_message).to eq 'Fixes' + expect(merge_request).not_to have_received(:to_reference) + end + end + context 'when project has commit template with closed issues' do let(message_template_name) { <<~MSG.rstrip } Merge branch '%{source_branch}' into '%{target_branch}' @@ -381,6 +394,57 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do end end end + + context 'when project has commit template with the same variable used twice' do + let(message_template_name) { '%{title} %{title}' } + + it 'uses custom template' do + expect(result_message).to eq 'Bugfix Bugfix' + end + end + + context 'when project has commit template without any variable' do + let(message_template_name) { 'static text' } + + it 'uses custom template' do + expect(result_message).to eq 'static text' + end + end + + context 'when project has template with all variables' do + let(message_template_name) { <<~MSG.rstrip } + source_branch:%{source_branch} + target_branch:%{target_branch} + title:%{title} + issues:%{issues} + description:%{description} + first_commit:%{first_commit} + first_multiline_commit:%{first_multiline_commit} + url:%{url} + approved_by:%{approved_by} + merged_by:%{merged_by} + MSG + + it 'uses custom template' do + expect(result_message).to eq <<~MSG.rstrip + source_branch:feature + target_branch:master + title:Bugfix + issues: + description:Merge Request Description + Next line + first_commit:Feature added + + Signed-off-by: Dmitriy Zaporozhets + first_multiline_commit:Feature added + + Signed-off-by: Dmitriy Zaporozhets + url:#{Gitlab::UrlBuilder.build(merge_request)} + approved_by: + merged_by:#{maintainer.name} <#{maintainer.commit_email_or_default}> + MSG + end + end end describe '#merge_message' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 79b8765b540..29f0810e660 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1647,13 +1647,10 @@ RSpec.describe MergeRequest, factory_default: :keep do end it 'uses template from target project' do - subject.title = 'Fix everything' - subject.compare_commits = [ - double(safe_message: 'Commit message', gitaly_commit?: true, merge_commit?: false, description?: false) - ] - subject.target_project.merge_commit_template = '%{title}' + request = build(:merge_request, title: 'Fix everything') + request.target_project.merge_commit_template = '%{title}' - expect(subject.default_merge_commit_message) + expect(request.default_merge_commit_message) .to eq('Fix everything') end diff --git a/spec/rubocop/cop/database/establish_connection_spec.rb b/spec/rubocop/cop/database/establish_connection_spec.rb new file mode 100644 index 00000000000..a3c27d33cb0 --- /dev/null +++ b/spec/rubocop/cop/database/establish_connection_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../../../rubocop/cop/database/establish_connection' + +RSpec.describe RuboCop::Cop::Database::EstablishConnection do + subject(:cop) { described_class.new } + + it 'flags the use of ActiveRecord::Base.establish_connection' do + expect_offense(<<~CODE) + ActiveRecord::Base.establish_connection + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't establish new database [...] + CODE + end + + it 'flags the use of ActiveRecord::Base.establish_connection with arguments' do + expect_offense(<<~CODE) + ActiveRecord::Base.establish_connection(:foo) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't establish new database [...] + CODE + end + + it 'flags the use of SomeModel.establish_connection' do + expect_offense(<<~CODE) + SomeModel.establish_connection + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't establish new database [...] + CODE + end +end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 316d645f99f..fb70f82ef87 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -67,7 +67,7 @@ module DbCleaner # Migrate each database individually with_reestablished_active_record_base do all_connection_classes.each do |connection_class| - ActiveRecord::Base.establish_connection(connection_class.connection_db_config) + ActiveRecord::Base.establish_connection(connection_class.connection_db_config) # rubocop: disable Database/EstablishConnection ActiveRecord::Tasks::DatabaseTasks.migrate end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 722d484609c..70b794f7d82 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -59,7 +59,7 @@ module CycleAnalyticsHelpers def save_value_stream(custom_value_stream_name) fill_in 'create-value-stream-name', with: custom_value_stream_name - page.find_button(s_('CreateValueStreamForm|Create Value Stream')).click + page.find_button(s_('CreateValueStreamForm|Create value stream')).click wait_for_requests end diff --git a/spec/support_specs/database/multiple_databases_spec.rb b/spec/support_specs/database/multiple_databases_spec.rb index 66056359458..b4cfa253813 100644 --- a/spec/support_specs/database/multiple_databases_spec.rb +++ b/spec/support_specs/database/multiple_databases_spec.rb @@ -7,13 +7,13 @@ RSpec.describe 'Database::MultipleDatabases' do context 'when doing establish_connection' do context 'on ActiveRecord::Base' do it 'raises exception' do - expect { ActiveRecord::Base.establish_connection(:main) }.to raise_error /Cannot re-establish/ + expect { ActiveRecord::Base.establish_connection(:main) }.to raise_error /Cannot re-establish/ # rubocop: disable Database/EstablishConnection end context 'when using with_reestablished_active_record_base' do it 'does not raise exception' do with_reestablished_active_record_base do - expect { ActiveRecord::Base.establish_connection(:main) }.not_to raise_error + expect { ActiveRecord::Base.establish_connection(:main) }.not_to raise_error # rubocop: disable Database/EstablishConnection end end end @@ -25,13 +25,13 @@ RSpec.describe 'Database::MultipleDatabases' do end it 'raises exception' do - expect { Ci::ApplicationRecord.establish_connection(:ci) }.to raise_error /Cannot re-establish/ + expect { Ci::ApplicationRecord.establish_connection(:ci) }.to raise_error /Cannot re-establish/ # rubocop: disable Database/EstablishConnection end context 'when using with_reestablished_active_record_base' do it 'does not raise exception' do with_reestablished_active_record_base do - expect { Ci::ApplicationRecord.establish_connection(:main) }.not_to raise_error + expect { Ci::ApplicationRecord.establish_connection(:main) }.not_to raise_error # rubocop: disable Database/EstablishConnection end end end