Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/tracking.js

@@ -102,7 +102,6 @@ export function initUserTracking() {
   window.snowplow('enableActivityTracking', 30, 30);
   window.snowplow('trackPageView'); // must be after enableActivityTracking
-  if (opts.userId) window.snowplow('setUserId', opts.userId);
   if (opts.formTracking) window.snowplow('enableFormTracking');
   if (opts.linkClickTracking) window.snowplow('enableLinkClickTracking');

app/views/admin/application_settings/_pendo.html.haml

-- expanded = true if !@application_setting.valid? && @application_setting.errors.any? { |k| k.to_s.start_with?('pendo_') }
-%section.settings.as-pendo.no-animate#js-pendo-settings{ class: ('expanded' if expanded) }
-  .settings-header
-    %h4
-      = _('Pendo')
-    %button.btn.btn-default.js-settings-toggle{ type: 'button' }
-      = expanded ? _('Collapse') : _('Expand')
-    %p
-      = _('Configure the %{link} integration.').html_safe % { link: link_to('Pendo', 'https://www.pendo.io/', target: '_blank') }
-  .settings-content
-
-    = form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-pendo-settings'), html: { class: 'fieldset-form' } do |f|
-      = form_errors(@application_setting) if expanded
-
-      %fieldset
-        .form-group
-          .form-check
-            = f.check_box :pendo_enabled, class: 'form-check-input'
-            = f.label :pendo_enabled, _('Enable pendo tracking'), class: 'form-check-label'
-        .form-group
-          = f.label :pendo_url, _('Pendo endpoint'), class: 'label-light'
-          = f.text_field :pendo_url, class: 'form-control', placeholder: 'https://cdn.pendo.io/agent/static/your-api-key/pendo.js'
-
-      = f.submit _('Save changes'), class: 'btn btn-success'

app/views/admin/application_settings/integrations.html.haml

@@ -29,4 +29,4 @@
     = render 'third_party_offers', application_setting: @application_setting
 = render 'admin/application_settings/snowplow', expanded: expanded_by_default?
-= render 'admin/application_settings/pendo'
+= render_if_exists 'admin/application_settings/pendo'

app/views/layouts/_head.html.haml

@@ -90,4 +90,4 @@
   = render 'layouts/google_analytics' if extra_config.has_key?('google_analytics_id')
   = render 'layouts/piwik' if extra_config.has_key?('piwik_url') && extra_config.has_key?('piwik_site_id')
   = render 'layouts/snowplow'
-  = render 'layouts/pendo'
+  = render_if_exists 'layouts/pendo'

app/views/layouts/_pendo.html.haml

-- return unless Gitlab::CurrentSettings.pendo_enabled?
-
-= javascript_tag nonce: true do
-  :plain
-    ;var trackable = !['1', 'yes'].includes(window.doNotTrack || navigator.doNotTrack || navigator.msDoNotTrack);
-    if (trackable){
-    (function(p,e,n,d,o){var v,w,x,y,z;o=p[d]=p[d]||{};o._q=[];
-    v=['initialize','identify','updateOptions','pageLoad'];for(w=0,x=v.length;w<x;++w)(function(m){
-    o[m]=o[m]||function(){o._q[m===v[0]?'unshift':'push']([m].concat([].slice.call(arguments,0)));};})(v[w]);
-    y=e.createElement(n);y.async=!0;y.src='#{Gitlab::CurrentSettings.pendo_url}';
-    z=e.getElementsByTagName(n)[0];z.parentNode.insertBefore(y,z);})(window,document,'script','pendo');
-
-    pendo.initialize({
-      visitor: {
-        id: '#{current_user&.id}',
-      }
-    });};

app/views/layouts/_snowplow.html.haml

@@ -7,4 +7,4 @@
     };p[i].q=p[i].q||[];n=l.createElement(o);g=l.getElementsByTagName(o)[0];n.async=1;
     n.src=w;g.parentNode.insertBefore(n,g)}}(window,document,"script","#{asset_url('snowplow/sp.js')}","snowplow"));
-    window.snowplowOptions = #{Gitlab::Tracking.snowplow_options(@group, current_user).to_json}
+    window.snowplowOptions = #{Gitlab::Tracking.snowplow_options(@group).to_json}

changelogs/unreleased/sh-clean-duplicate-ci-trigger-request-indexes.yml

+---
+title: Clean up duplicate indexes on ci_trigger_requests
+merge_request: 19053
+author:
+type: fixed

db/migrate/20191016072826_replace_ci_trigger_requests_index.rb

@@ -10,7 +10,12 @@ class ReplaceCiTriggerRequestsIndex < ActiveRecord::Migration[5.2]
   def up
     add_concurrent_index :ci_trigger_requests, [:trigger_id, :id], order: { id: :desc }
-    remove_concurrent_index :ci_trigger_requests, [:trigger_id]
+    # Some installations have legacy, duplicate indexes on
+    # ci_trigger_requests.trigger_id. Rails won't drop them without an
+    # explicit name: https://gitlab.com/gitlab-org/gitlab/issues/34818
+    old_index_names.each do |name|
+      remove_concurrent_index :ci_trigger_requests, [:trigger_id], name: name
+    end
   end
   def down
@@ -18,4 +23,10 @@ class ReplaceCiTriggerRequestsIndex < ActiveRecord::Migration[5.2]
     remove_concurrent_index :ci_trigger_requests, [:trigger_id, :id], order: { id: :desc }
   end
+
+  private
+
+  def old_index_names
+    indexes(:ci_trigger_requests).select { |i| i.columns == ['trigger_id'] }.map(&:name)
+  end
 end

doc/development/README.md

@@ -118,6 +118,7 @@ description: 'Learn how to contribute to GitLab.'
 - [Query Count Limits](query_count_limits.md)
 - [Database helper modules](database_helpers.md)
 - [Code comments](code_comments.md)
+- [Creating enums](creating_enums.md)
 ### Case studies

doc/development/creating_enums.md

+# Creating enums
+
+When creating a new enum, it should use the database type `SMALLINT`.
+The `SMALLINT` type size is 2 bytes, which is sufficient for an enum.
+This would help to save space in the database.
+
+To use this type, add `limit: 2` to the migration that creates the column.
+
+Example:
+
+```rb
+def change
+  add_column :ci_job_artifacts, :file_format, :integer, limit: 2
+end
+```

lib/gitlab/tracking.rb

@@ -39,7 +39,7 @@ module Gitlab
         snowplow.track_self_describing_event(event_json, context, Time.now.to_i)
       end
-      def snowplow_options(group, user)
+      def snowplow_options(group)
         additional_features = Feature.enabled?(:additional_snowplow_tracking, group)
         {
           namespace: SNOWPLOW_NAMESPACE,
@@ -47,8 +47,7 @@ module Gitlab
           cookie_domain: Gitlab::CurrentSettings.snowplow_cookie_domain,
           app_id: Gitlab::CurrentSettings.snowplow_site_id,
           form_tracking: additional_features,
-          link_click_tracking: additional_features,
-          user_id: user&.id
+          link_click_tracking: additional_features
         }.transform_keys! { |key| key.to_s.camelize(:lower).to_sym }
       end

spec/db/schema_spec.rb

@@ -120,9 +120,55 @@ describe 'Database schema' do
     end
   end
+  # These pre-existing enums have limits > 2 bytes
+  IGNORED_LIMIT_ENUMS = {
+    'Analytics::CycleAnalytics::GroupStage' => %w[start_event_identifier end_event_identifier],
+    'Analytics::CycleAnalytics::ProjectStage' => %w[start_event_identifier end_event_identifier],
+    'Ci::Bridge' => %w[failure_reason],
+    'Ci::Build' => %w[failure_reason],
+    'Ci::BuildMetadata' => %w[timeout_source],
+    'Ci::BuildTraceChunk' => %w[data_store],
+    'Ci::JobArtifact' => %w[file_type],
+    'Ci::Pipeline' => %w[source config_source failure_reason],
+    'Ci::Runner' => %w[access_level],
+    'Ci::Stage' => %w[status],
+    'Clusters::Applications::Ingress' => %w[ingress_type],
+    'Clusters::Cluster' => %w[platform_type provider_type],
+    'CommitStatus' => %w[failure_reason],
+    'GenericCommitStatus' => %w[failure_reason],
+    'Gitlab::DatabaseImporters::CommonMetrics::PrometheusMetric' => %w[group],
+    'InternalId' => %w[usage],
+    'List' => %w[list_type],
+    'NotificationSetting' => %w[level],
+    'Project' => %w[auto_cancel_pending_pipelines],
+    'ProjectAutoDevops' => %w[deploy_strategy],
+    'PrometheusMetric' => %w[group],
+    'ResourceLabelEvent' => %w[action],
+    'User' => %w[layout dashboard project_view],
+    'UserCallout' => %w[feature_name],
+    'PrometheusAlert' => %w[operator]
+  }.freeze
+
+  context 'for enums' do
+    ApplicationRecord.descendants.each do |model|
+      describe model do
+        let(:ignored_enums) { ignored_limit_enums(model.name) }
+        let(:enums) { model.defined_enums.keys - ignored_enums }
+
+        it 'uses smallint for enums' do
+          expect(model).to use_smallint_for_enums(enums)
+        end
+      end
+    end
+  end
+
   private
   def ignored_fk_columns(column)
     IGNORED_FK_COLUMNS.fetch(column, [])
   end
+
+  def ignored_limit_enums(model)
+    IGNORED_LIMIT_ENUMS.fetch(model, [])
+  end
 end

spec/frontend/tracking_spec.js

@@ -11,7 +11,6 @@ describe('Tracking', () => {
       namespace: '_namespace_',
       hostname: 'app.gitfoo.com',
       cookieDomain: '.gitfoo.com',
-      userId: null,
     };
     snowplowSpy = jest.spyOn(window, 'snowplow');
   });
@@ -35,7 +34,6 @@ describe('Tracking', () => {
         contexts: { webPage: true },
         formTracking: false,
         linkClickTracking: false,
-        userId: null,
       });
     });
@@ -43,18 +41,15 @@ describe('Tracking', () => {
       initUserTracking();
       expect(snowplowSpy).toHaveBeenCalledWith('enableActivityTracking', 30, 30);
       expect(snowplowSpy).toHaveBeenCalledWith('trackPageView');
-      expect(snowplowSpy).not.toHaveBeenCalledWith('setUserId');
       expect(snowplowSpy).not.toHaveBeenCalledWith('enableFormTracking');
       expect(snowplowSpy).not.toHaveBeenCalledWith('enableLinkClickTracking');
       window.snowplowOptions = Object.assign({}, window.snowplowOptions, {
         formTracking: true,
         linkClickTracking: true,
-        userId: '1',
       });
       initUserTracking();
-      expect(snowplowSpy).toHaveBeenCalledWith('setUserId', '1');
       expect(snowplowSpy).toHaveBeenCalledWith('enableFormTracking');
       expect(snowplowSpy).toHaveBeenCalledWith('enableLinkClickTracking');
     });

spec/lib/gitlab/tracking_spec.rb

@@ -19,11 +19,10 @@ describe Gitlab::Tracking do
         cookieDomain: '.gitfoo.com',
         appId: '_abc123_',
         formTracking: true,
-        linkClickTracking: true,
-        userId: nil
+        linkClickTracking: true
       }
-      expect(subject.snowplow_options(nil, nil)).to match(expected_fields)
+      expect(subject.snowplow_options(nil)).to match(expected_fields)
     end
     it 'enables features using feature flags' do
@@ -37,7 +36,7 @@ describe Gitlab::Tracking do
         linkClickTracking: false
       }
-      expect(subject.snowplow_options('_group_', nil)).to include(addition_feature_fields)
+      expect(subject.snowplow_options('_group_')).to include(addition_feature_fields)
     end
   end

spec/support/matchers/db_schema_matchers.rb

+# frozen_string_literal: true
+
+EXPECTED_SMALLINT_LIMIT = 2
+
+RSpec::Matchers.define :use_smallint_for_enums do |enums|
+  match do |actual|
+    @failing_enums = enums.select do |enum|
+      enum_type = actual.type_for_attribute(enum)
+      actual_limit = enum_type.send(:subtype).limit
+      actual_limit != EXPECTED_SMALLINT_LIMIT
+    end
+    @failing_enums.empty?
+  end
+
+  failure_message do
+    <<~FAILURE_MESSAGE
+      Expected #{actual.name} enums: #{failing_enums.join(', ')} to use the smallint type.
+
+      The smallint type is 2 bytes which is more than sufficient for an enum.
+      Using the smallint type would help us save space in the database.
+      To fix this, please add `limit: 2` in the migration file, for example:
+
+      def change
+        add_column :ci_job_artifacts, :file_format, :integer, limit: 2
+      end
+    FAILURE_MESSAGE
+  end
+
+  def failing_enums
+    @failing_enums ||= []
+  end
+end

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

@@ -92,28 +92,6 @@ describe 'layouts/_head' do
     end
   end
-  context 'when pendo is enabled' do
-    it 'adds a pendo initialization snippet with url', :aggregate_failures do
-      allow(Gitlab::CurrentSettings).to receive(:pendo_enabled?).and_return(true)
-      allow(Gitlab::CurrentSettings).to receive(:pendo_url).and_return('www.pen.do')
-
-      render
-
-      expect(rendered).to match('pendo.initialize')
-      expect(rendered).to match('www.pen.do')
-    end
-  end
-
-  context 'when pendo is not enabled' do
-    it 'do not add pendo snippet' do
-      allow(Gitlab::CurrentSettings).to receive(:pendo_enabled?).and_return(false)
-
-      render
-
-      expect(rendered).not_to match('pendo.initialize')
-    end
-  end
-
   context 'when a Piwik config is set' do
     let(:piwik_host) { 'piwik.example.com' }