Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42

5 files changed, 404 insertions, 18 deletions

diff --git a/spec/services/design_management/copy_design_collection/copy_service_spec.rb b/spec/services/design_management/copy_design_collection/copy_service_spec.rb
new file mode 100644
index 00000000000..e93e5f13fea
--- /dev/null
+++ b/spec/services/design_management/copy_design_collection/copy_service_spec.rb
@@ -0,0 +1,259 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe DesignManagement::CopyDesignCollection::CopyService, :clean_gitlab_redis_shared_state do
+  include DesignManagementTestHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:issue, refind: true) { create(:issue, project: project) }
+  let(:target_issue) { create(:issue) }
+
+  subject { described_class.new(project, user, issue: issue, target_issue: target_issue).execute }
+
+  before do
+    enable_design_management
+  end
+
+  shared_examples 'service error' do |message:|
+    it 'returns an error response', :aggregate_failures do
+      expect(subject).to be_kind_of(ServiceResponse)
+      expect(subject).to be_error
+      expect(subject.message).to eq(message)
+    end
+  end
+
+  shared_examples 'service success' do
+    it 'returns a success response', :aggregate_failures do
+      expect(subject).to be_kind_of(ServiceResponse)
+      expect(subject).to be_success
+    end
+  end
+
+  include_examples 'service error', message: 'User cannot copy design collection to issue'
+
+  context 'when user has permission to read the design collection' do
+    before_all do
+      project.add_reporter(user)
+    end
+
+    include_examples 'service error', message: 'User cannot copy design collection to issue'
+
+    context 'when the user also has permission to admin the target issue' do
+      let(:target_repository) { target_issue.project.design_repository }
+
+      before do
+        target_issue.project.add_reporter(user)
+      end
+
+      include_examples 'service error', message: 'Target design collection must first be queued'
+
+      context 'when the target design collection has been queued' do
+        before do
+          target_issue.design_collection.start_copy!
+        end
+
+        include_examples 'service error', message: 'Design collection has no designs'
+
+        context 'when design collection has designs' do
+          let_it_be(:designs) do
+            create_list(:design, 3, :with_lfs_file, :with_relative_position, issue: issue, project: project)
+          end
+
+          context 'when target issue already has designs' do
+            before do
+              create(:design, issue: target_issue, project: target_issue.project)
+            end
+
+            include_examples 'service error', message: 'Target design collection already has designs'
+          end
+
+          include_examples 'service success'
+
+          it 'creates a design repository for the target project' do
+            expect { subject }.to change { target_repository.exists? }.from(false).to(true)
+          end
+
+          context 'when the target project already has a design repository' do
+            before do
+              target_repository.create_if_not_exists
+            end
+
+            include_examples 'service success'
+          end
+
+          it 'copies the designs correctly', :aggregate_failures do
+            expect { subject }.to change { target_issue.designs.count }.by(3)
+
+            old_designs = issue.designs.ordered
+            new_designs = target_issue.designs.ordered
+
+            new_designs.zip(old_designs).each do |new_design, old_design|
+              expect(new_design).to have_attributes(
+                filename: old_design.filename,
+                relative_position: old_design.relative_position,
+                issue: target_issue,
+                project: target_issue.project
+              )
+            end
+          end
+
+          it 'copies the design versions correctly', :aggregate_failures do
+            expect { subject }.to change { target_issue.design_versions.count }.by(3)
+
+            old_versions = issue.design_versions.ordered
+            new_versions = target_issue.design_versions.ordered
+
+            new_versions.zip(old_versions).each do |new_version, old_version|
+              expect(new_version).to have_attributes(
+                created_at: old_version.created_at,
+                author_id: old_version.author_id
+              )
+              expect(new_version.designs.pluck(:filename)).to eq(old_version.designs.pluck(:filename))
+              expect(new_version.actions.pluck(:event)).to eq(old_version.actions.pluck(:event))
+            end
+          end
+
+          it 'copies the design actions correctly', :aggregate_failures do
+            expect { subject }.to change { DesignManagement::Action.count }.by(3)
+
+            old_actions = issue.design_versions.ordered.flat_map(&:actions)
+            new_actions = target_issue.design_versions.ordered.flat_map(&:actions)
+
+            new_actions.zip(old_actions).each do |new_action, old_action|
+              # This is a way to identify if the versions linked to the actions
+              # are correct is to compare design filenames, as the SHA changes.
+              new_design_filenames = new_action.version.designs.ordered.pluck(:filename)
+              old_design_filenames = old_action.version.designs.ordered.pluck(:filename)
+
+              expect(new_design_filenames).to eq(old_design_filenames)
+              expect(new_action.event).to eq(old_action.event)
+              expect(new_action.design.filename).to eq(old_action.design.filename)
+            end
+          end
+
+          it 'copies design notes correctly', :aggregate_failures, :sidekiq_inline do
+            old_notes = [
+              create(:diff_note_on_design, note: 'first note', noteable: designs.first, project: project, author: create(:user)),
+              create(:diff_note_on_design, note: 'second note', noteable: designs.first, project: project, author: create(:user))
+            ]
+            matchers = old_notes.map do |note|
+              have_attributes(
+                note.attributes.slice(
+                  :type,
+                  :author_id,
+                  :note,
+                  :position
+                )
+              )
+            end
+
+            expect { subject }.to change { Note.count }.by(2)
+
+            new_notes = target_issue.designs.first.notes.fresh
+
+            expect(new_notes).to match_array(matchers)
+          end
+
+          it 'links the LfsObjects' do
+            expect { subject }.to change { target_issue.project.lfs_objects.count }.by(3)
+          end
+
+          it 'copies the Git repository data', :aggregate_failures do
+            subject
+
+            commit_shas = target_repository.commits('master', limit: 99).map(&:id)
+
+            expect(commit_shas).to include(*target_issue.design_versions.ordered.pluck(:sha))
+          end
+
+          it 'creates a master branch if none previously existed' do
+            expect { subject }.to change { target_repository.branch_names }.from([]).to(['master'])
+          end
+
+          it 'leaves the design collection in the correct copy state' do
+            subject
+
+            expect(target_issue.design_collection).to be_copy_ready
+          end
+
+          describe 'rollback' do
+            before do
+              # Ensure the very last step throws an error
+              expect_next_instance_of(described_class) do |service|
+                expect(service).to receive(:finalize!).and_raise
+              end
+            end
+
+            include_examples 'service error', message: 'Designs were unable to be copied successfully'
+
+            it 'rollsback all PostgreSQL data created', :aggregate_failures do
+              expect { subject }.not_to change {
+                [
+                  DesignManagement::Design.count,
+                  DesignManagement::Action.count,
+                  DesignManagement::Version.count,
+                  Note.count
+                ]
+              }
+
+              collections = [
+                target_issue.design_collection,
+                target_issue.designs,
+                target_issue.design_versions
+              ]
+
+              expect(collections).to all(be_empty)
+            end
+
+            it 'does not alter master branch', :aggregate_failures do
+              # Add some Git data to the target_repository, so we are testing
+              # that any original data remains
+              issue_2 = create(:issue, project: target_issue.project)
+              create(:design, :with_file, issue: issue_2, project: target_issue.project)
+
+              expect { subject }.not_to change {
+                expect(target_repository.commits('master', limit: 10).size).to eq(1)
+              }
+            end
+
+            it 'sets the design collection copy state' do
+              subject
+
+              expect(target_issue.design_collection).to be_copy_error
+            end
+          end
+        end
+      end
+    end
+  end
+
+  describe 'Alert if schema changes', :aggregate_failures do
+    let_it_be(:config_file) { Rails.root.join('lib/gitlab/design_management/copy_design_collection_model_attributes.yml') }
+    let_it_be(:config) { YAML.load_file(config_file).symbolize_keys }
+
+    %w(Design Action Version).each do |model|
+      specify do
+        attributes = config["#{model.downcase}_attributes".to_sym] || []
+        ignored_attributes = config["ignore_#{model.downcase}_attributes".to_sym]
+
+        expect(attributes + ignored_attributes).to contain_exactly(
+          *DesignManagement.const_get(model, false).column_names
+        ), failure_message(model)
+      end
+    end
+
+    def failure_message(model)
+      <<-MSG
+      The schema of the `#{model}` model has changed.
+
+      `#{described_class.name}` refers to specific lists of attributes of `#{model}` to either
+      copy or ignore, so that we continue to copy designs correctly after schema changes.
+
+      Please update:
+      #{config_file}
+      to reflect the latest changes to `#{model}`. See that file for more information.
+      MSG
+    end
+  end
+end
diff --git a/spec/services/design_management/copy_design_collection/queue_service_spec.rb b/spec/services/design_management/copy_design_collection/queue_service_spec.rb
new file mode 100644
index 00000000000..2d9ea4633a0
--- /dev/null
+++ b/spec/services/design_management/copy_design_collection/queue_service_spec.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe DesignManagement::CopyDesignCollection::QueueService, :clean_gitlab_redis_shared_state do
+  include DesignManagementTestHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:issue) { create(:issue) }
+  let_it_be(:target_issue, refind: true) { create(:issue) }
+  let_it_be(:design) { create(:design, issue: issue, project: issue.project) }
+
+  subject { described_class.new(user, issue, target_issue).execute }
+
+  before do
+    enable_design_management
+  end
+
+  it 'returns an error if user does not have permission' do
+    expect(subject).to be_kind_of(ServiceResponse)
+    expect(subject).to be_error
+    expect(subject.message).to eq('User cannot copy designs to issue')
+  end
+
+  context 'when user has permission' do
+    before_all do
+      issue.project.add_reporter(user)
+      target_issue.project.add_reporter(user)
+    end
+
+    it 'returns an error if design collection copy_state is not queuable' do
+      target_issue.design_collection.start_copy!
+
+      expect(subject).to be_kind_of(ServiceResponse)
+      expect(subject).to be_error
+      expect(subject.message).to eq('Target design collection copy state must be `ready`')
+    end
+
+    it 'sets the design collection copy state' do
+      expect { subject }.to change { target_issue.design_collection.copy_state }.from('ready').to('in_progress')
+    end
+
+    it 'queues a DesignManagement::CopyDesignCollectionWorker' do
+      expect { subject }.to change(DesignManagement::CopyDesignCollectionWorker.jobs, :size).by(1)
+    end
+
+    it 'returns success' do
+      expect(subject).to be_kind_of(ServiceResponse)
+      expect(subject).to be_success
+    end
+  end
+end
diff --git a/spec/services/design_management/delete_designs_service_spec.rb b/spec/services/design_management/delete_designs_service_spec.rb
index ace63b6e59c..ed161b4c8ff 100644
--- a/spec/services/design_management/delete_designs_service_spec.rb
+++ b/spec/services/design_management/delete_designs_service_spec.rb
@@ -80,6 +80,16 @@ RSpec.describe DesignManagement::DeleteDesignsService do
           expect { run_service rescue nil }
             .not_to change { [counter.totals, Event.count] }
         end
+
+        it 'does not log any UsageData metrics' do
+          redis_hll = ::Gitlab::UsageDataCounters::HLLRedisCounter
+          event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_DESIGNS_REMOVED
+
+          expect { run_service rescue nil }
+            .not_to change { redis_hll.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }
+
+          run_service rescue nil
+        end
       end
 
       context 'one design is passed' do
@@ -98,6 +108,12 @@ RSpec.describe DesignManagement::DeleteDesignsService do
           expect { run_service }.to change { counter.read(:delete) }.by(1)
         end
 
+        it 'updates UsageData for removed designs' do
+          expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_designs_removed_action).with(author: user)
+
+          run_service
+        end
+
         it 'creates an event in the activity stream' do
           expect { run_service }
             .to change { Event.count }.by(1)
@@ -105,7 +121,7 @@ RSpec.describe DesignManagement::DeleteDesignsService do
         end
 
         it 'informs the new-version-worker' do
-          expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer)
+          expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer, false)
 
           run_service
         end
diff --git a/spec/services/design_management/generate_image_versions_service_spec.rb b/spec/services/design_management/generate_image_versions_service_spec.rb
index 631eec97e5a..749030af97d 100644
--- a/spec/services/design_management/generate_image_versions_service_spec.rb
+++ b/spec/services/design_management/generate_image_versions_service_spec.rb
@@ -52,25 +52,50 @@ RSpec.describe DesignManagement::GenerateImageVersionsService do
     end
 
     context 'when an error is encountered when generating the image versions' do
-      before do
-        expect_next_instance_of(DesignManagement::DesignV432x230Uploader) do |uploader|
-          expect(uploader).to receive(:cache!).and_raise(CarrierWave::DownloadError, 'foo')
+      context "CarrierWave::IntegrityError" do
+        before do
+          expect_next_instance_of(DesignManagement::DesignV432x230Uploader) do |uploader|
+            expect(uploader).to receive(:cache!).and_raise(CarrierWave::IntegrityError, 'foo')
+          end
+        end
+
+        it 'logs the exception' do
+          expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+            instance_of(CarrierWave::IntegrityError),
+            project_id: project.id, version_id: version.id, design_id: version.designs.first.id
+          )
+
+          described_class.new(version).execute
         end
-      end
 
-      it 'logs the error' do
-        expect(Gitlab::AppLogger).to receive(:error).with('foo')
+        it 'logs the error' do
+          expect(Gitlab::AppLogger).to receive(:error).with('foo')
 
-        described_class.new(version).execute
+          described_class.new(version).execute
+        end
       end
 
-      it 'tracks the error' do
-        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
-          instance_of(CarrierWave::DownloadError),
-          project_id: project.id, version_id: version.id, design_id: version.designs.first.id
-        )
+      context "CarrierWave::UploadError" do
+        before do
+          expect_next_instance_of(DesignManagement::DesignV432x230Uploader) do |uploader|
+            expect(uploader).to receive(:cache!).and_raise(CarrierWave::UploadError, 'foo')
+          end
+        end
 
-        described_class.new(version).execute
+        it 'logs the error' do
+          expect(Gitlab::AppLogger).to receive(:error).with('foo')
+
+          described_class.new(version).execute
+        end
+
+        it 'tracks the error' do
+          expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
+            instance_of(CarrierWave::UploadError),
+            project_id: project.id, version_id: version.id, design_id: version.designs.first.id
+          )
+
+          described_class.new(version).execute
+        end
       end
     end
   end
diff --git a/spec/services/design_management/save_designs_service_spec.rb b/spec/services/design_management/save_designs_service_spec.rb
index abba5de2c27..f36e68c8dbd 100644
--- a/spec/services/design_management/save_designs_service_spec.rb
+++ b/spec/services/design_management/save_designs_service_spec.rb
@@ -32,7 +32,7 @@ RSpec.describe DesignManagement::SaveDesignsService do
     end
 
     allow(::DesignManagement::NewVersionWorker)
-      .to receive(:perform_async).with(Integer).and_return(nil)
+      .to receive(:perform_async).with(Integer, false).and_return(nil)
   end
 
   def run_service(files_to_upload = nil)
@@ -102,9 +102,11 @@ RSpec.describe DesignManagement::SaveDesignsService do
         end
       end
 
-      it 'creates a commit, an event in the activity stream and updates the creation count' do
+      it 'creates a commit, an event in the activity stream and updates the creation count', :aggregate_failures do
         counter = Gitlab::UsageDataCounters::DesignsCounter
 
+        expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_designs_added_action).with(author: user)
+
         expect { run_service }
           .to change { Event.count }.by(1)
           .and change { Event.for_design.created_action.count }.by(1)
@@ -128,6 +130,25 @@ RSpec.describe DesignManagement::SaveDesignsService do
         expect { run_parallel(blocks) }.to change(DesignManagement::Version, :count).by(parellism)
       end
 
+      context 'when the design collection is in the process of being copied', :clean_gitlab_redis_shared_state do
+        before do
+          issue.design_collection.start_copy!
+        end
+
+        it_behaves_like 'a service error'
+      end
+
+      context 'when the design collection has a copy error', :clean_gitlab_redis_shared_state do
+        before do
+          issue.design_collection.copy_state = 'error'
+          issue.design_collection.send(:set_stored_copy_state!)
+        end
+
+        it 'resets the copy state' do
+          expect { run_service }.to change { issue.design_collection.copy_state }.from('error').to('ready')
+        end
+      end
+
       describe 'the response' do
         it 'includes designs with the expected properties' do
           updated_designs = response[:designs]
@@ -171,6 +192,12 @@ RSpec.describe DesignManagement::SaveDesignsService do
           expect(updated_designs.first.versions.size).to eq(2)
         end
 
+        it 'updates UsageData for changed designs' do
+          expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_designs_modified_action).with(author: user)
+
+          run_service
+        end
+
         it 'records the correct events' do
           counter = Gitlab::UsageDataCounters::DesignsCounter
           expect { run_service }
@@ -220,7 +247,7 @@ RSpec.describe DesignManagement::SaveDesignsService do
           counter = Gitlab::UsageDataCounters::DesignsCounter
 
           expect(::DesignManagement::NewVersionWorker)
-            .to receive(:perform_async).once.with(Integer).and_return(nil)
+            .to receive(:perform_async).once.with(Integer, false).and_return(nil)
 
           expect { run_service }
             .to change { Event.count }.by(2)
@@ -254,7 +281,7 @@ RSpec.describe DesignManagement::SaveDesignsService do
           design_repository.has_visible_content?
 
           expect(::DesignManagement::NewVersionWorker)
-            .to receive(:perform_async).once.with(Integer).and_return(nil)
+            .to receive(:perform_async).once.with(Integer, false).and_return(nil)
 
           expect { service.execute }
             .to change { issue.designs.count }.from(0).to(2)
@@ -271,6 +298,14 @@ RSpec.describe DesignManagement::SaveDesignsService do
             expect(response[:message]).to match(/only \d+ files are allowed simultaneously/i)
           end
         end
+
+        context 'when uploading duplicate files' do
+          let(:files) { [rails_sample, dk_png, rails_sample] }
+
+          it 'returns the correct error' do
+            expect(response[:message]).to match('Duplicate filenames are not allowed!')
+          end
+        end
       end
 
       context 'when the user is not allowed to upload designs' do