Merge branch 'ee-snowplow-backend-ee-to-ce' into 'master'

Migrates Snowplow backend from EE to CE

See merge request gitlab-org/gitlab-ee!14999

app/helpers/application_settings_helper.rb

@@ -270,7 +270,11 @@ module ApplicationSettingsHelper
       :diff_max_patch_bytes,
       :commit_email_hostname,
       :protected_ci_variables,
-      :local_markdown_version
+      :local_markdown_version,
+      :snowplow_collector_hostname,
+      :snowplow_cookie_domain,
+      :snowplow_enabled,
+      :snowplow_site_id
     ]
   end
 

app/helpers/tracking_helper.rb

@@ -2,8 +2,21 @@
 
 module TrackingHelper
   def tracking_attrs(label, event, property)
-    {} # CE has no tracking features
+    return {} unless tracking_enabled?
+
+    {
+      data: {
+        track_label: label,
+        track_event: event,
+        track_property: property
+      }
+    }
   end
-end
 
-TrackingHelper.prepend_if_ee('EE::TrackingHelper')
+  private
+
+  def tracking_enabled?
+    Rails.env.production? &&
+      ::Gitlab::CurrentSettings.snowplow_enabled?
+  end
+end

app/models/application_setting.rb

@@ -99,6 +99,11 @@ class ApplicationSetting < ApplicationRecord
             presence: true,
             if: :plantuml_enabled
 
+  validates :snowplow_collector_hostname,
+            presence: true,
+            hostname: true,
+            if: :snowplow_enabled
+
   validates :max_attachment_size,
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }

app/models/application_setting_implementation.rb

@@ -97,6 +97,10 @@ module ApplicationSettingImplementation
         usage_stats_set_by_user_id: nil,
         diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
         commit_email_hostname: default_commit_email_hostname,
+        snowplow_collector_hostname: nil,
+        snowplow_cookie_domain: nil,
+        snowplow_enabled: false,
+        snowplow_site_id: nil,
         protected_ci_variables: false,
         local_markdown_version: 0,
         outbound_local_requests_whitelist: [],

ee/app/views/admin/application_settings/_snowplow.html.haml → app/views/admin/application_settings/_snowplow.html.haml

+- expanded = true if !@application_setting.valid? && @application_setting.errors.any? { |k| k.to_s.start_with?('snowplow_') }
 %section.settings.as-snowplow.no-animate#js-snowplow-settings{ class: ('expanded' if expanded) }
   .settings-header
     %h4
-      Snowplow
+      = _('Snowplow')
     %button.btn.btn-default.js-settings-toggle{ type: 'button' }
       = expanded ? _('Collapse') : _('Expand')
     %p
       = _('Configure the %{link} integration.').html_safe % { link: link_to('Snowplow', 'https://snowplowanalytics.com/', target: '_blank') }
   .settings-content
 
-    = form_for @application_setting, url: admin_application_settings_path, html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: integrations_admin_application_settings_path, html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting)
 
       %fieldset
         .form-group
           .form-check
             = f.check_box :snowplow_enabled, class: 'form-check-input'
-            = f.label :snowplow_enabled, class: 'form-check-label' do
-              Enable Snowplow
+            = f.label :snowplow_enabled, _('Enable snowplow tracking'), class: 'form-check-label'
         .form-group
-          = f.label :snowplow_collector_hostname, 'Collector Hostname', class: 'label-light'
+          = f.label :snowplow_collector_hostname, _('Collector hostname'), class: 'label-light'
           = f.text_field :snowplow_collector_hostname, class: 'form-control', placeholder: 'snowplow.example.com'
         .form-group
-          = f.label :snowplow_site_id, class: 'label-light' do
-            Site ID
+          = f.label :snowplow_site_id, _('Site ID'), class: 'label-light'
           = f.text_field :snowplow_site_id, class: 'form-control'
         .form-group
-          = f.label :snowplow_cookie_domain, class: 'label-light' do
-            Cookie domain
+          = f.label :snowplow_cookie_domain, _('Cookie domain'), class: 'label-light'
           = f.text_field :snowplow_cookie_domain, class: 'form-control'
 
-      = f.submit 'Save changes', class: 'btn btn-success'
+      = f.submit _('Save changes'), class: 'btn btn-success'

ee/app/views/layouts/_snowplow.html.haml → app/views/layouts/_snowplow.html.haml

@@ -14,13 +14,16 @@
       respectDoNotTrack: true,
       forceSecureTracker: true,
       post: true,
-      contexts: {
-          webPage: true,
-      },
+      contexts: { webPage: true },
       stateStorageStrategy: "localStorage"
     });
 
     window.snowplow('enableActivityTracking', 30, 30);
     window.snowplow('trackPageView');
 
-= render 'layouts/snowplow_additional_tracking'
+- return unless Feature.enabled?(:additional_snowplow_tracking, @group)
+
+= javascript_tag nonce: true do
+  :plain
+    window.snowplow('enableFormTracking');
+    window.snowplow('enableLinkClickTracking');

doc/api/settings.md

@@ -321,4 +321,8 @@ are listed in the descriptions of the relevant settings.
 | `user_show_add_ssh_key_message`          | boolean          | no                                   | When set to `false` disable the "You won't be able to pull or push project code via SSH" warning shown to users with no uploaded SSH key. |
 | `version_check_enabled`                  | boolean          | no                                   | Let GitLab inform you when an update is available. |
 | `local_markdown_version`                 | integer          | no                                   | Increase this value when any cached markdown should be invalidated. |
+| `snowplow_enabled`                       | boolean          | no                                   | Enable snowplow tracking. |
+| `snowplow_collector_hostname`            | string           | required by: `snowplow_enabled`      | The Snowplow collector hostname. (e.g. `snowplow.trx.gitlab.net`) | 
+| `snowplow_site_id`                       | string           | no                                   | The Snowplow site name / application id. (e.g. `gitlab`) |
+| `snowplow_cookie_domain`                 | string           | no                                   | The Snowplow cookie domain. (e.g. `.gitlab.com`) |
 | `geo_node_allowed_ips`                   | string           | yes                                  | **(PREMIUM)** Comma-separated list of IPs and CIDRs of allowed secondary nodes. For example, `1.1.1.1, 2.2.2.0/24`. |

ee/app/helpers/ee/application_settings_helper.rb

@@ -44,11 +44,7 @@ module EE
         :slack_app_enabled,
         :slack_app_id,
         :slack_app_secret,
-        :slack_app_verification_token,
-        :snowplow_collector_hostname,
-        :snowplow_cookie_domain,
-        :snowplow_enabled,
-        :snowplow_site_id
+        :slack_app_verification_token
       ]
     end
 

ee/app/helpers/ee/tracking_helper.rb

-# frozen_string_literal: true
-
-module EE
-  module TrackingHelper
-    extend ::Gitlab::Utils::Override
-
-    override :tracking_attrs
-    def tracking_attrs(label, event, property)
-      return {} unless tracking_enabled?
-
-      {
-        data: {
-          track_label: label,
-          track_event: event,
-          track_property: property
-        }
-      }
-    end
-
-    private
-
-    def tracking_enabled?
-      Rails.env.production? &&
-        ::Gitlab::CurrentSettings.snowplow_enabled?
-    end
-  end
-end

ee/app/models/ee/application_setting.rb

@@ -59,11 +59,6 @@ module EE
                 allow_blank: true,
                 length: { maximum: EMAIL_ADDITIONAL_TEXT_CHARACTER_LIMIT }
 
-      validates :snowplow_collector_hostname,
-                presence: true,
-                hostname: true,
-                if: :snowplow_enabled
-
       validates :geo_node_allowed_ips, length: { maximum: 255 }, presence: true
 
       validates :required_instance_ci_template, presence: true, allow_nil: true
@@ -95,10 +90,6 @@ module EE
           slack_app_id: nil,
           slack_app_secret: nil,
           slack_app_verification_token: nil,
-          snowplow_collector_hostname: nil,
-          snowplow_cookie_domain: nil,
-          snowplow_enabled: false,
-          snowplow_site_id: nil,
           custom_project_templates_group_id: nil,
           geo_node_allowed_ips: '0.0.0.0/0, ::/0'
         )

ee/app/views/layouts/_snowplow_additional_tracking.html.haml

-- return unless Feature.enabled?(:additional_snowplow_tracking, @group)
-
-= javascript_tag nonce: true do
-  :plain
-    window.snowplow('enableFormTracking');
-    window.snowplow('enableLinkClickTracking');

ee/lib/ee/api/helpers/settings_helpers.rb

@@ -35,14 +35,6 @@ module EE
             optional :repository_size_limit, type: Integer, desc: 'Size limit per repository (MB)'
             optional :file_template_project_id, type: Integer, desc: 'ID of project where instance-level file templates are stored.'
             optional :repository_storages, type: Array[String], desc: 'A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random.'
-            optional :snowplow_enabled, type: Grape::API::Boolean, desc: 'Enable Snowplow'
-
-            given snowplow_enabled: ->(val) { val } do
-              requires :snowplow_collector_hostname, type: String, desc: 'Snowplow Collector Hostname'
-              optional :snowplow_cookie_domain, type: String, desc: 'Snowplow cookie domain'
-              optional :snowplow_site_id, type: String, desc: 'Snowplow Site/Application ID'
-            end
-
             optional :usage_ping_enabled, type: Grape::API::Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
           end
         end

ee/spec/helpers/ee/tracking_helper_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe EE::TrackingHelper do
-  describe '#tracking_attrs' do
-    using RSpec::Parameterized::TableSyntax
-
-    let(:input) { %w(a b c) }
-    let(:results) do
-      {
-        no_data: {},
-        with_data: { data: { track_label: 'a', track_event: 'b', track_property: 'c' } }
-      }
-    end
-
-    where(:snowplow_enabled, :environment, :result) do
-      true  | 'production'  | :with_data
-      false | 'production'  | :no_data
-      true  | 'development' | :no_data
-      false | 'development' | :no_data
-      true  | 'test'        | :no_data
-      false | 'test'        | :no_data
-    end
-
-    with_them do
-      it 'returns a hash' do
-        stub_application_setting(snowplow_enabled: snowplow_enabled)
-        allow(Rails).to receive(:env).and_return(environment.inquiry)
-
-        expect(helper.tracking_attrs(*input)).to eq(results[result])
-      end
-    end
-  end
-end

ee/spec/requests/api/settings_spec.rb

@@ -18,19 +18,11 @@ describe API::Settings, 'EE Settings' do
       put api("/application/settings", admin),
         params: {
           help_text: 'Help text',
-          snowplow_collector_hostname: 'snowplow.example.com',
-          snowplow_cookie_domain: '.example.com',
-          snowplow_enabled: true,
-          snowplow_site_id:  'site_id',
           file_template_project_id: project.id
         }
 
       expect(response).to have_gitlab_http_status(200)
       expect(json_response['help_text']).to eq('Help text')
-      expect(json_response['snowplow_collector_hostname']).to eq('snowplow.example.com')
-      expect(json_response['snowplow_cookie_domain']).to eq('.example.com')
-      expect(json_response['snowplow_enabled']).to be_truthy
-      expect(json_response['snowplow_site_id']).to eq('site_id')
       expect(json_response['file_template_project_id']).to eq(project.id)
     end
 
@@ -155,13 +147,4 @@ describe API::Settings, 'EE Settings' do
 
     it_behaves_like 'settings for licensed features'
   end
-
-  context "missing snowplow_collector_hostname value when snowplow_enabled is true" do
-    it "returns a blank parameter error message" do
-      put api("/application/settings", admin), params: { snowplow_enabled: true }
-
-      expect(response).to have_gitlab_http_status(400)
-      expect(json_response['error']).to eq('snowplow_collector_hostname is missing')
-    end
-  end
 end

ee/spec/views/layouts/_head.html.haml_spec.rb

-require 'spec_helper'
-
-describe 'layouts/_head' do
-  before do
-    allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)
-  end
-
-  context 'when an asset_host is set and snowplow url is set' do
-    let(:asset_host) { 'http://test.host' }
-
-    before do
-      allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
-      allow(Gitlab::CurrentSettings).to receive(:snowplow_enabled?).and_return(true)
-      allow(Gitlab::CurrentSettings).to receive(:snowplow_collector_hostname).and_return('www.snow.plow')
-    end
-
-    it 'add a snowplow script tag with asset host' do
-      render
-      expect(rendered).to match('http://test.host/assets/snowplow/')
-      expect(rendered).to match('window.snowplow')
-      expect(rendered).to match('www.snow.plow')
-    end
-  end
-end

lib/api/settings.rb

@@ -125,6 +125,12 @@ module API
       optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
       optional :local_markdown_version, type: Integer, desc: "Local markdown version, increase this value when any cached markdown should be invalidated"
       optional :allow_local_requests_from_hooks_and_services, type: Boolean, desc: 'Deprecated: Use :allow_local_requests_from_web_hooks_and_services instead. Allow requests to the local network from hooks and services.' # support legacy names, can be removed in v5
+      optional :snowplow_enabled, type: Grape::API::Boolean, desc: 'Enable Snowplow tracking'
+      given snowplow_enabled: ->(val) { val } do
+        requires :snowplow_collector_hostname, type: String, desc: 'The Snowplow collector hostname'
+        optional :snowplow_cookie_domain, type: String, desc: 'The Snowplow cookie domain'
+        optional :snowplow_site_id, type: String, desc: 'The Snowplow site name / application ic'
+      end
 
       ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
         optional :"#{type}_key_restriction",

locale/gitlab.pot

@@ -3655,6 +3655,9 @@ msgstr ""
 msgid "Collapse sidebar"
 msgstr ""
 
+msgid "Collector hostname"
+msgstr ""
+
 msgid "ComboSearch is not defined"
 msgstr ""
 
@@ -4014,6 +4017,9 @@ msgstr ""
 msgid "ConvDev Index"
 msgstr ""
 
+msgid "Cookie domain"
+msgstr ""
+
 msgid "Copied"
 msgstr ""
 
@@ -5342,6 +5348,9 @@ msgstr ""
 msgid "Enable shared Runners"
 msgstr ""
 
+msgid "Enable snowplow tracking"
+msgstr ""
+
 msgid "Enable two-factor authentication"
 msgstr ""
 
@@ -13640,6 +13649,9 @@ msgstr ""
 msgid "Single or combined queries"
 msgstr ""
 
+msgid "Site ID"
+msgstr ""
+
 msgid "Size"
 msgstr ""
 
@@ -13691,6 +13703,9 @@ msgstr ""
 msgid "SnippetsEmptyState|They can be either public or private."
 msgstr ""
 
+msgid "Snowplow"
+msgstr ""
+
 msgid "Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead."
 msgstr ""
 

spec/helpers/tracking_helper_spec.rb

@@ -4,8 +4,32 @@ require 'spec_helper'
 
 describe TrackingHelper do
   describe '#tracking_attrs' do
-    it 'returns an empty hash' do
-      expect(helper.tracking_attrs('a', 'b', 'c')).to eq({})
+    using RSpec::Parameterized::TableSyntax
+
+    let(:input) { %w(a b c) }
+    let(:results) do
+      {
+        no_data: {},
+        with_data: { data: { track_label: 'a', track_event: 'b', track_property: 'c' } }
+      }
+    end
+
+    where(:snowplow_enabled, :environment, :result) do
+      true  | 'production'  | :with_data
+      false | 'production'  | :no_data
+      true  | 'development' | :no_data
+      false | 'development' | :no_data
+      true  | 'test'        | :no_data
+      false | 'test'        | :no_data
+    end
+
+    with_them do
+      it 'returns a hash' do
+        stub_application_setting(snowplow_enabled: snowplow_enabled)
+        allow(Rails).to receive(:env).and_return(environment.inquiry)
+
+        expect(helper.tracking_attrs(*input)).to eq(results[result])
+      end
     end
   end
 end

spec/requests/api/settings_spec.rb

@@ -144,6 +144,7 @@ describe API::Settings, 'Settings' do
           external_auth_client_key_pass: "5iveL!fe"
         }
       end
+
       let(:attribute_names) { settings.keys.map(&:to_s) }
 
       it 'includes the attributes in the API' do
@@ -165,6 +166,56 @@ describe API::Settings, 'Settings' do
       end
     end
 
+    context "snowplow tracking settings" do
+      let(:settings) do
+        {
+          snowplow_collector_hostname: "snowplow.example.com",
+          snowplow_cookie_domain: ".example.com",
+          snowplow_enabled: true,
+          snowplow_site_id: "site_id"
+        }
+      end
+
+      let(:attribute_names) { settings.keys.map(&:to_s) }
+
+      it "includes the attributes in the API" do
+        get api("/application/settings", admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        attribute_names.each do |attribute|
+          expect(json_response.keys).to include(attribute)
+        end
+      end
+
+      it "allows updating the settings" do
+        put api("/application/settings", admin), params: settings
+
+        expect(response).to have_gitlab_http_status(200)
+        settings.each do |attribute, value|
+          expect(ApplicationSetting.current.public_send(attribute)).to eq(value)
+        end
+      end
+
+      context "missing snowplow_collector_hostname value when snowplow_enabled is true" do
+        it "returns a blank parameter error message" do
+          put api("/application/settings", admin), params: { snowplow_enabled: true }
+
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response["error"]).to eq("snowplow_collector_hostname is missing")
+        end
+
+        it "handles validation errors" do
+          put api("/application/settings", admin), params: settings.merge({
+            snowplow_collector_hostname: nil
+          })
+
+          expect(response).to have_gitlab_http_status(400)
+          message = json_response["message"]
+          expect(message["snowplow_collector_hostname"]).to include("can't be blank")
+        end
+      end
+    end
+
     context "missing plantuml_url value when plantuml_enabled is true" do
       it "returns a blank parameter error message" do
         put api("/application/settings", admin), params: { plantuml_enabled: true }

spec/views/layouts/_head.html.haml_spec.rb

@@ -70,6 +70,23 @@ describe 'layouts/_head' do
     expect(rendered).to match('<link rel="stylesheet" media="all" href="/stylesheets/highlight/themes/solarised-light.css" />')
   end
 
+  context 'when an asset_host is set and snowplow url is set' do
+    let(:asset_host) { 'http://test.host' }
+
+    before do
+      allow(ActionController::Base).to receive(:asset_host).and_return(asset_host)
+      allow(Gitlab::CurrentSettings).to receive(:snowplow_enabled?).and_return(true)
+      allow(Gitlab::CurrentSettings).to receive(:snowplow_collector_hostname).and_return('www.snow.plow')
+    end
+
+    it 'add a snowplow script tag with asset host' do
+      render
+      expect(rendered).to match('http://test.host/assets/snowplow/')
+      expect(rendered).to match('window.snowplow')
+      expect(rendered).to match('www.snow.plow')
+    end
+  end
+
   def stub_helper_with_safe_string(method)
     allow_any_instance_of(PageLayoutHelper).to receive(method)
       .and_return(%q{foo" http-equiv="refresh}.html_safe)