Add latest changes from gitlab-org/gitlab@master

7 files changed, 241 insertions, 145 deletions

diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb
index 7a72a13febe..a2e9c68d255 100644
--- a/spec/controllers/profiles_controller_spec.rb
+++ b/spec/controllers/profiles_controller_spec.rb
@@ -110,7 +110,8 @@ RSpec.describe ProfilesController, :request_store do
 
       expect_snowplow_event(
         category: 'ProfilesController',
-        action: 'search_audit_event'
+        action: 'search_audit_event',
+        user: user
       )
     end
   end
diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb
index d2934ec4e97..7ef5371f2b5 100644
--- a/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -509,7 +509,10 @@ RSpec.describe Projects::Settings::OperationsController do
           it 'tracks an event' do
             expect_snowplow_event(
               category: 'project:operations:tracing',
-              action: 'external_url_populated'
+              action: 'external_url_populated',
+              user: user,
+              project: project,
+              namespace: project.namespace
             )
           end
         end
diff --git a/spec/migrations/20210601073400_fix_total_stage_in_vsa_spec.rb b/spec/migrations/20210601073400_fix_total_stage_in_vsa_spec.rb
new file mode 100644
index 00000000000..a4a33a0d030
--- /dev/null
+++ b/spec/migrations/20210601073400_fix_total_stage_in_vsa_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20210601073400_fix_total_stage_in_vsa.rb')
+
+RSpec.describe FixTotalStageInVsa, :migration, schema: 20210518001450 do
+  let(:namespaces) { table(:namespaces) }
+  let(:group_value_streams) { table(:analytics_cycle_analytics_group_value_streams) }
+  let(:group_stages) { table(:analytics_cycle_analytics_group_stages) }
+
+  let!(:group) { namespaces.create!(name: 'ns1', path: 'ns1', type: 'Group') }
+  let!(:group_vs_1) { group_value_streams.create!(name: 'default', group_id: group.id) }
+  let!(:group_vs_2) { group_value_streams.create!(name: 'other', group_id: group.id) }
+  let!(:group_vs_3) { group_value_streams.create!(name: 'another', group_id: group.id) }
+  let!(:group_stage_total) { group_stages.create!(name: 'Total', custom: false, group_id: group.id, group_value_stream_id: group_vs_1.id, start_event_identifier: 1, end_event_identifier: 2) }
+  let!(:group_stage_different_name) { group_stages.create!(name: 'Issue', custom: false, group_id: group.id, group_value_stream_id: group_vs_2.id, start_event_identifier: 1, end_event_identifier: 2) }
+  let!(:group_stage_total_custom) { group_stages.create!(name: 'Total', custom: true, group_id: group.id, group_value_stream_id: group_vs_3.id, start_event_identifier: 1, end_event_identifier: 2) }
+
+  it 'deduplicates issue_metrics table' do
+    migrate!
+
+    group_stage_total.reload
+    group_stage_different_name.reload
+    group_stage_total_custom.reload
+
+    expect(group_stage_total.custom).to eq(true)
+    expect(group_stage_different_name.custom).to eq(false)
+    expect(group_stage_total_custom.custom).to eq(true)
+  end
+end
diff --git a/spec/models/service_desk_setting_spec.rb b/spec/models/service_desk_setting_spec.rb
index ca57a5d4087..8ccbd983ba1 100644
--- a/spec/models/service_desk_setting_spec.rb
+++ b/spec/models/service_desk_setting_spec.rb
@@ -31,6 +31,37 @@ RSpec.describe ServiceDeskSetting do
     end
   end
 
+  describe '.valid_project_key' do
+    # Creates two projects with same full path slug
+    # group1/test/one and group1/test-one will both have 'group-test-one' slug
+    let_it_be(:group) { create(:group) }
+    let_it_be(:subgroup) { create(:group, parent: group, name: 'test') }
+    let_it_be(:project1) { create(:project, name: 'test-one', group: group) }
+    let_it_be(:project2) { create(:project, name: 'one', group: subgroup) }
+    let_it_be(:project_key) { 'key' }
+
+    before_all do
+      create(:service_desk_setting, project: project1, project_key: project_key)
+    end
+
+    context 'when project_key is unique for every project slug' do
+      it 'does not add error' do
+        settings = build(:service_desk_setting, project: project2, project_key: 'otherkey')
+
+        expect(settings).to be_valid
+      end
+    end
+
+    context 'when project with same slug and settings project_key exists' do
+      it 'adds error' do
+        settings = build(:service_desk_setting, project: project2, project_key: project_key)
+
+        expect(settings).to be_invalid
+        expect(settings.errors[:project_key].first).to eq('already in use for another service desk address.')
+      end
+    end
+  end
+
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
   end
diff --git a/spec/requests/groups/email_campaigns_controller_spec.rb b/spec/requests/groups/email_campaigns_controller_spec.rb
index 290e6009ecb..4d630ef6710 100644
--- a/spec/requests/groups/email_campaigns_controller_spec.rb
+++ b/spec/requests/groups/email_campaigns_controller_spec.rb
@@ -52,7 +52,9 @@ RSpec.describe Groups::EmailCampaignsController do
             context: [{
                         schema: described_class::EMAIL_CAMPAIGNS_SCHEMA_URL,
                         data: { namespace_id: group.id, series: series.to_i, subject_line: subject_line_text, track: track.to_s }
-                      }]
+                      }],
+            user: user,
+            namespace: group
           )
         end
 
diff --git a/spec/services/packages/nuget/metadata_extraction_service_spec.rb b/spec/services/packages/nuget/metadata_extraction_service_spec.rb
index 39fc0f9e6a1..b3980d3b7fa 100644
--- a/spec/services/packages/nuget/metadata_extraction_service_spec.rb
+++ b/spec/services/packages/nuget/metadata_extraction_service_spec.rb
@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Packages::Nuget::MetadataExtractionService do
-  let(:package_file) { create(:nuget_package).package_files.first }
+  let_it_be(:package_file) { create(:nuget_package).package_files.first }
+
   let(:service) { described_class.new(package_file.id) }
 
   describe '#execute' do
@@ -23,12 +24,27 @@ RSpec.describe Packages::Nuget::MetadataExtractionService do
         package_tags: []
       }
 
-      it { is_expected.to eq(expected_metadata) }
+      context 'with packages_nuget_archive_new_file_reader enabled' do
+        before do
+          expect(service).to receive(:with_new_file_reader).and_call_original
+        end
+
+        it { is_expected.to eq(expected_metadata) }
+      end
+
+      context 'with packages_nuget_archive_new_file_reader disabled' do
+        before do
+          stub_feature_flags(packages_nuget_archive_new_file_reader: false)
+          expect(service).to receive(:with_legacy_file_reader).and_call_original
+        end
+
+        it { is_expected.to eq(expected_metadata) }
+      end
     end
 
     context 'with nuspec file' do
       before do
-        allow(service).to receive(:nuspec_file).and_return(fixture_file(nuspec_filepath))
+        allow(service).to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
       end
 
       context 'with dependencies' do
@@ -57,7 +73,7 @@ RSpec.describe Packages::Nuget::MetadataExtractionService do
       let_it_be(:nuspec_filepath) { 'packages/nuget/with_metadata.nuspec' }
 
       before do
-        allow(service).to receive(:nuspec_file).and_return(fixture_file(nuspec_filepath))
+        allow(service).to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
       end
 
       it { expect(subject[:license_url]).to eq('https://opensource.org/licenses/MIT') }
diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb
index c1cce46a54c..e41475f786d 100644
--- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb
+++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb
@@ -12,7 +12,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
   let(:package_version) { '1.0.0' }
   let(:package_file_name) { 'dummyproject.dummypackage.1.0.0.nupkg' }
 
-  RSpec.shared_examples 'raising an' do |error_class|
+  shared_examples 'raising an' do |error_class|
     it "raises an #{error_class}" do
       expect { subject }.to raise_error(error_class)
     end
@@ -21,11 +21,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
   describe '#execute' do
     subject { service.execute }
 
-    before do
-      stub_package_file_object_storage(enabled: true, direct_upload: true)
-    end
-
-    RSpec.shared_examples 'taking the lease' do
+    shared_examples 'taking the lease' do
       before do
         allow(service).to receive(:lease_release?).and_return(false)
       end
@@ -39,7 +35,7 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
       end
     end
 
-    RSpec.shared_examples 'not updating the package if the lease is taken' do
+    shared_examples 'not updating the package if the lease is taken' do
       context 'without obtaining the exclusive lease' do
         let(:lease_key) { "packages:nuget:update_package_from_metadata_service:package:#{package_id}" }
         let(:metadata) { { package_name: package_name, package_version: package_version } }
@@ -67,174 +63,191 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
       end
     end
 
-    context 'with no existing package' do
-      let(:package_id) { package.id }
-
-      it 'updates package and package file' do
-        expect { subject }
-          .to change { ::Packages::Package.count }.by(1)
-          .and change { Packages::Dependency.count }.by(1)
-          .and change { Packages::DependencyLink.count }.by(1)
-          .and change { ::Packages::Nuget::Metadatum.count }.by(0)
-
-        expect(package.reload.name).to eq(package_name)
-        expect(package.version).to eq(package_version)
-        expect(package).to be_default
-        expect(package_file.reload.file_name).to eq(package_file_name)
-        # hard reset needed to properly reload package_file.file
-        expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0
-      end
-
-      it_behaves_like 'taking the lease'
+    shared_examples 'handling all conditions' do
+      context 'with no existing package' do
+        let(:package_id) { package.id }
 
-      it_behaves_like 'not updating the package if the lease is taken'
-    end
-
-    context 'with existing package' do
-      let!(:existing_package) { create(:nuget_package, project: package.project, name: package_name, version: package_version) }
-      let(:package_id) { existing_package.id }
+        it 'updates package and package file' do
+          expect { subject }
+            .to change { ::Packages::Package.count }.by(1)
+            .and change { Packages::Dependency.count }.by(1)
+            .and change { Packages::DependencyLink.count }.by(1)
+            .and change { ::Packages::Nuget::Metadatum.count }.by(0)
+
+          expect(package.reload.name).to eq(package_name)
+          expect(package.version).to eq(package_version)
+          expect(package).to be_default
+          expect(package_file.reload.file_name).to eq(package_file_name)
+          # hard reset needed to properly reload package_file.file
+          expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0
+        end
 
-      it 'link existing package and updates package file' do
-        expect(service).to receive(:try_obtain_lease).and_call_original
+        it_behaves_like 'taking the lease'
 
-        expect { subject }
-          .to change { ::Packages::Package.count }.by(-1)
-          .and change { Packages::Dependency.count }.by(0)
-          .and change { Packages::DependencyLink.count }.by(0)
-          .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(0)
-          .and change { ::Packages::Nuget::Metadatum.count }.by(0)
-        expect(package_file.reload.file_name).to eq(package_file_name)
-        expect(package_file.package).to eq(existing_package)
+        it_behaves_like 'not updating the package if the lease is taken'
       end
 
-      it_behaves_like 'taking the lease'
+      context 'with existing package' do
+        let!(:existing_package) { create(:nuget_package, project: package.project, name: package_name, version: package_version) }
+        let(:package_id) { existing_package.id }
 
-      it_behaves_like 'not updating the package if the lease is taken'
-    end
+        it 'link existing package and updates package file' do
+          expect(service).to receive(:try_obtain_lease).and_call_original
 
-    context 'with a nuspec file with metadata' do
-      let(:nuspec_filepath) { 'packages/nuget/with_metadata.nuspec' }
-      let(:expected_tags) { %w(foo bar test tag1 tag2 tag3 tag4 tag5) }
+          expect { subject }
+            .to change { ::Packages::Package.count }.by(-1)
+            .and change { Packages::Dependency.count }.by(0)
+            .and change { Packages::DependencyLink.count }.by(0)
+            .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(0)
+            .and change { ::Packages::Nuget::Metadatum.count }.by(0)
+          expect(package_file.reload.file_name).to eq(package_file_name)
+          expect(package_file.package).to eq(existing_package)
+        end
 
-      before do
-        allow_any_instance_of(Packages::Nuget::MetadataExtractionService)
-          .to receive(:nuspec_file)
-          .and_return(fixture_file(nuspec_filepath))
-      end
+        it_behaves_like 'taking the lease'
 
-      it 'creates tags' do
-        expect(service).to receive(:try_obtain_lease).and_call_original
-        expect { subject }.to change { ::Packages::Tag.count }.by(8)
-        expect(package.reload.tags.map(&:name)).to contain_exactly(*expected_tags)
+        it_behaves_like 'not updating the package if the lease is taken'
       end
 
-      context 'with existing package and tags' do
-        let!(:existing_package) { create(:nuget_package, project: package.project, name: 'DummyProject.WithMetadata', version: '1.2.3') }
-        let!(:tag1) { create(:packages_tag, package: existing_package, name: 'tag1') }
-        let!(:tag2) { create(:packages_tag, package: existing_package, name: 'tag2') }
-        let!(:tag3) { create(:packages_tag, package: existing_package, name: 'tag_not_in_metadata') }
+      context 'with a nuspec file with metadata' do
+        let(:nuspec_filepath) { 'packages/nuget/with_metadata.nuspec' }
+        let(:expected_tags) { %w(foo bar test tag1 tag2 tag3 tag4 tag5) }
+
+        before do
+          allow_next_instance_of(Packages::Nuget::MetadataExtractionService) do |service|
+            allow(service)
+              .to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
+          end
+        end
 
-        it 'creates tags and deletes those not in metadata' do
+        it 'creates tags' do
           expect(service).to receive(:try_obtain_lease).and_call_original
-          expect { subject }.to change { ::Packages::Tag.count }.by(5)
-          expect(existing_package.tags.map(&:name)).to contain_exactly(*expected_tags)
+          expect { subject }.to change { ::Packages::Tag.count }.by(8)
+          expect(package.reload.tags.map(&:name)).to contain_exactly(*expected_tags)
+        end
+
+        context 'with existing package and tags' do
+          let!(:existing_package) { create(:nuget_package, project: package.project, name: 'DummyProject.WithMetadata', version: '1.2.3') }
+          let!(:tag1) { create(:packages_tag, package: existing_package, name: 'tag1') }
+          let!(:tag2) { create(:packages_tag, package: existing_package, name: 'tag2') }
+          let!(:tag3) { create(:packages_tag, package: existing_package, name: 'tag_not_in_metadata') }
+
+          it 'creates tags and deletes those not in metadata' do
+            expect(service).to receive(:try_obtain_lease).and_call_original
+            expect { subject }.to change { ::Packages::Tag.count }.by(5)
+            expect(existing_package.tags.map(&:name)).to contain_exactly(*expected_tags)
+          end
         end
-      end
 
-      it 'creates nuget metadatum' do
-        expect { subject }
-          .to change { ::Packages::Package.count }.by(1)
-          .and change { ::Packages::Nuget::Metadatum.count }.by(1)
+        it 'creates nuget metadatum' do
+          expect { subject }
+            .to change { ::Packages::Package.count }.by(1)
+            .and change { ::Packages::Nuget::Metadatum.count }.by(1)
 
-        metadatum = package_file.reload.package.nuget_metadatum
-        expect(metadatum.license_url).to eq('https://opensource.org/licenses/MIT')
-        expect(metadatum.project_url).to eq('https://gitlab.com/gitlab-org/gitlab')
-        expect(metadatum.icon_url).to eq('https://opensource.org/files/osi_keyhole_300X300_90ppi_0.png')
-      end
+          metadatum = package_file.reload.package.nuget_metadatum
+          expect(metadatum.license_url).to eq('https://opensource.org/licenses/MIT')
+          expect(metadatum.project_url).to eq('https://gitlab.com/gitlab-org/gitlab')
+          expect(metadatum.icon_url).to eq('https://opensource.org/files/osi_keyhole_300X300_90ppi_0.png')
+        end
 
-      context 'with too long url' do
-        let_it_be(:too_long_url) { "http://localhost/#{'bananas' * 50}" }
+        context 'with too long url' do
+          let_it_be(:too_long_url) { "http://localhost/#{'bananas' * 50}" }
 
-        let(:metadata) { { package_name: package_name, package_version: package_version, license_url: too_long_url } }
+          let(:metadata) { { package_name: package_name, package_version: package_version, license_url: too_long_url } }
 
-        before do
-          allow(service).to receive(:metadata).and_return(metadata)
-        end
+          before do
+            allow(service).to receive(:metadata).and_return(metadata)
+          end
 
-        it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
+          it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
+        end
       end
-    end
 
-    context 'with nuspec file with dependencies' do
-      let(:nuspec_filepath) { 'packages/nuget/with_dependencies.nuspec' }
-      let(:package_name) { 'Test.Package' }
-      let(:package_version) { '3.5.2' }
-      let(:package_file_name) { 'test.package.3.5.2.nupkg' }
+      context 'with nuspec file with dependencies' do
+        let(:nuspec_filepath) { 'packages/nuget/with_dependencies.nuspec' }
+        let(:package_name) { 'Test.Package' }
+        let(:package_version) { '3.5.2' }
+        let(:package_file_name) { 'test.package.3.5.2.nupkg' }
 
-      before do
-        allow_any_instance_of(Packages::Nuget::MetadataExtractionService)
-          .to receive(:nuspec_file)
-          .and_return(fixture_file(nuspec_filepath))
-      end
+        before do
+          allow_next_instance_of(Packages::Nuget::MetadataExtractionService) do |service|
+            allow(service)
+              .to receive(:nuspec_file_content).and_return(fixture_file(nuspec_filepath))
+          end
+        end
 
-      it 'updates package and package file' do
-        expect { subject }
-          .to change { ::Packages::Package.count }.by(1)
-          .and change { Packages::Dependency.count }.by(4)
-          .and change { Packages::DependencyLink.count }.by(4)
-          .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(2)
-
-        expect(package.reload.name).to eq(package_name)
-        expect(package.version).to eq(package_version)
-        expect(package).to be_default
-        expect(package_file.reload.file_name).to eq(package_file_name)
-        # hard reset needed to properly reload package_file.file
-        expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0
+        it 'updates package and package file' do
+          expect { subject }
+            .to change { ::Packages::Package.count }.by(1)
+            .and change { Packages::Dependency.count }.by(4)
+            .and change { Packages::DependencyLink.count }.by(4)
+            .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(2)
+
+          expect(package.reload.name).to eq(package_name)
+          expect(package.version).to eq(package_version)
+          expect(package).to be_default
+          expect(package_file.reload.file_name).to eq(package_file_name)
+          # hard reset needed to properly reload package_file.file
+          expect(Packages::PackageFile.find(package_file.id).file.size).not_to eq 0
+        end
       end
-    end
 
-    context 'with package file not containing a nuspec file' do
-      before do
-        allow_any_instance_of(Zip::File).to receive(:glob).and_return([])
+      context 'with package file not containing a nuspec file' do
+        before do
+          allow_next_instance_of(Zip::File) do |file|
+            allow(file).to receive(:glob).and_return([])
+          end
+        end
+
+        it_behaves_like 'raising an', ::Packages::Nuget::MetadataExtractionService::ExtractionError
       end
 
-      it_behaves_like 'raising an', ::Packages::Nuget::MetadataExtractionService::ExtractionError
-    end
+      context 'with an invalid package name' do
+        invalid_names = [
+          '',
+          'My/package',
+          '../../../my_package',
+          '%2e%2e%2fmy_package'
+        ]
 
-    context 'with an invalid package name' do
-      invalid_names = [
-        '',
-        'My/package',
-        '../../../my_package',
-        '%2e%2e%2fmy_package'
-      ]
+        invalid_names.each do |invalid_name|
+          before do
+            allow(service).to receive(:package_name).and_return(invalid_name)
+          end
 
-      invalid_names.each do |invalid_name|
-        before do
-          allow(service).to receive(:package_name).and_return(invalid_name)
+          it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
         end
+      end
 
-        it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
+      context 'with an invalid package version' do
+        invalid_versions = [
+          '',
+          '555',
+          '1.2',
+          '1./2.3',
+          '../../../../../1.2.3',
+          '%2e%2e%2f1.2.3'
+        ]
+
+        invalid_versions.each do |invalid_version|
+          before do
+            allow(service).to receive(:package_version).and_return(invalid_version)
+          end
+
+          it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
+        end
       end
     end
 
-    context 'with an invalid package version' do
-      invalid_versions = [
-        '',
-        '555',
-        '1.2',
-        '1./2.3',
-        '../../../../../1.2.3',
-        '%2e%2e%2f1.2.3'
-      ]
-
-      invalid_versions.each do |invalid_version|
-        before do
-          allow(service).to receive(:package_version).and_return(invalid_version)
-        end
+    it_behaves_like 'handling all conditions'
 
-        it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
+    context 'with packages_nuget_archive_new_file_reader disabled' do
+      before do
+        stub_feature_flags(packages_nuget_archive_new_file_reader: false)
+        stub_package_file_object_storage(enabled: true, direct_upload: true)
       end
+
+      it_behaves_like 'handling all conditions'
     end
   end
 end