Ensure that new-version workers are run correctly

Workers need to be enqueued outside of transactions, so we use
`run_after_commit` to arrange for this.

ee/app/models/design_management/version.rb

@@ -4,6 +4,7 @@ module DesignManagement
   class Version < ApplicationRecord
     include Importable
     include ShaAttribute
+    include AfterCommitQueue
     include Gitlab::Utils::StrongMemoize
     extend Gitlab::ExclusiveLeaseHelpers
 

ee/app/services/design_management/delete_designs_service.rb

@@ -16,10 +16,6 @@ module DesignManagement
 
       version = delete_designs!
 
-      version.run_after_commit do
-        ::DesignManagement::NewVersionWorker.perform_async(version.id)
-      end
-
       # Create a Geo event so changes will be replicated to secondary node(s)
       repository.log_geo_updated_event
 

ee/app/services/design_management/runs_design_actions.rb

@@ -17,7 +17,17 @@ module DesignManagement
                                     message: commit_message,
                                     actions: actions.map(&:gitaly_action))
 
-      ::DesignManagement::Version.create_for_designs(actions, sha, current_user)
+      ::DesignManagement::Version
+        .create_for_designs(actions, sha, current_user)
+        .tap { |version| post_process(version) }
+    end
+
+    private
+
+    def post_process(version)
+      version.run_after_commit_or_now do
+        ::DesignManagement::NewVersionWorker.perform_async(id)
+      end
     end
   end
 end

ee/app/services/design_management/save_designs_service.rb

@@ -19,13 +19,13 @@ module DesignManagement
 
       repository.create_if_not_exists
 
-      uploaded_designs = upload_designs!
+      uploaded_designs, version = upload_designs!
       skipped_designs = designs - uploaded_designs
 
       # Create a Geo event so changes will be replicated to secondary node(s)
       repository.log_geo_updated_event
 
-      success({ designs: uploaded_designs, skipped_designs: skipped_designs })
+      success({ designs: uploaded_designs, version: version, skipped_designs: skipped_designs })
     rescue ::ActiveRecord::RecordInvalid => e
       error(e.message)
     end
@@ -35,25 +35,11 @@ module DesignManagement
     attr_reader :files
 
     def upload_designs!
-      # puts "Waiting [#{Thread.current.object_id}]"
-      actions = ::DesignManagement::Version.lock_for_creation(project.id) do
-        # puts ". Building [#{Thread.current.object_id}]"
+      ::DesignManagement::Version.lock_for_creation(project.id) do
         actions = build_actions
-        # if actions.empty?
-        #   puts ".. Skipping [#{Thread.current.object_id}]"
-        # else
-        if actions.present?
-          # puts ".. Running [#{Thread.current.object_id}]"
-          version = run_actions(actions)
-          version.run_after_commit do
-            ::DesignManagement::NewVersionWorker.perform_async(version.id)
-          end
-        end
 
-        actions
+        [actions.map(&:design), actions.presence && run_actions(actions)]
       end
-      # puts "Done [#{Thread.current.object_id}]"
-      actions.map(&:design)
     end
 
     # Returns `Design` instances that correspond with `files`.

ee/spec/services/design_management/delete_designs_service_spec.rb

@@ -91,6 +91,12 @@ describe DesignManagement::DeleteDesignsService do
           expect { run_service }.to change { counter.read(:delete) }.by(1)
         end
 
+        it 'informs the new-version-worker' do
+          expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer)
+
+          run_service
+        end
+
         it 'creates a new verison' do
           expect { run_service }.to change { DesignManagement::Version.where(issue: issue).count }.by(1)
         end

ee/spec/services/design_management/save_designs_service_spec.rb

@@ -20,18 +20,7 @@ describe DesignManagement::SaveDesignsService do
 
   before do
     project.add_developer(developer)
-
-    allow(::DesignManagement::NewVersionWorker).to receive(:perform_async)
-  end
-
-  RSpec::Matchers.define :enqueue_worker do
-    match do |action|
-      expect(::DesignManagement::NewVersionWorker)
-        .to receive(:perform_async).once.with(Integer)
-      action.call
-    end
-
-    supports_block_expectations
+    # allow(::DesignManagement::NewVersionWorker).to receive(:perform_async)
   end
 
   def run_service(files_to_upload = nil)
@@ -237,7 +226,10 @@ describe DesignManagement::SaveDesignsService do
         end
 
         it 'enqueues just one new version worker' do
-          expect { run_service }.to enqueue_worker
+          expect(::DesignManagement::NewVersionWorker)
+            .to receive(:perform_async).once.with(Integer)
+
+          run_service
         end
       end
 
@@ -260,7 +252,10 @@ describe DesignManagement::SaveDesignsService do
         end
 
         it 'enqueues a new version worker' do
-          expect { run_service }.to enqueue_worker
+          expect(::DesignManagement::NewVersionWorker)
+            .to receive(:perform_async).once.with(Integer)
+
+          run_service
         end
 
         it 'creates a single commit' do
@@ -272,7 +267,8 @@ describe DesignManagement::SaveDesignsService do
           expect { run_service }.to change { commit_count.call }.by(1)
         end
 
-        it 'only does 4 gitaly calls', :request_store, :sidekiq_might_not_need_inline do
+        it 'only does 5 gitaly calls', :request_store, :sidekiq_might_not_need_inline do
+          allow(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer)
           service = described_class.new(project, user, issue: issue, files: files)
           # Some unrelated calls that are usually cached or happen only once
           service.__send__(:repository).create_if_not_exists