Merge branch 'epic_issue_worker' into 'master'

Async job for note + event when creating/updating an EpicIssue link

See merge request gitlab-org/gitlab!64524

app/graphql/mutations/boards/issues/issue_move_list.rb

@@ -53,8 +53,6 @@ module Mutations
         end
 
         def resolve(board:, project_path:, iid:, **args)
-          Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/247861')
-
           issue = authorized_find!(project_path: project_path, iid: iid)
           move_params = { id: issue.id, board_id: board.id }.merge(move_arguments(args))
 

config/sidekiq_queues.yml

@@ -130,6 +130,8 @@
   - 1
 - - epics
   - 2
+- - epics_new_epic_issue
+  - 1
 - - error_tracking_issue_link
   - 1
 - - experiments_record_conversion_event

ee/app/models/epic_issue.rb

@@ -3,6 +3,7 @@
 class EpicIssue < ApplicationRecord
   include EpicTreeSorting
   include EachBatch
+  include AfterCommitQueue
 
   validates :epic, :issue, presence: true
   validates :issue, uniqueness: true

ee/app/services/epic_issues/create_service.rb

@@ -8,36 +8,23 @@ module EpicIssues
     def relate_issuables(referenced_issue)
       link = EpicIssue.find_or_initialize_by(issue: referenced_issue)
 
-      params = if link.persisted?
-                 { issue_moved: true, original_epic: link.epic }
-               else
-                 {}
-               end
+      params = { user_id: current_user.id }
+      params[:original_epic_id] = link.epic_id if link.persisted?
 
       link.epic = issuable
       link.move_to_start
 
-      if link.save
-        create_notes(referenced_issue, params)
-        usage_ping_record_epic_issue_added
+      link.run_after_commit do
+        params.merge!(epic_id: link.epic.id, issue_id: referenced_issue.id)
+        Epics::NewEpicIssueWorker.perform_async(params)
       end
 
+      link.save
+
       link
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    def create_notes(referenced_issue, params)
-      if params[:issue_moved]
-        SystemNoteService.epic_issue_moved(
-          params[:original_epic], referenced_issue, issuable, current_user
-        )
-        SystemNoteService.issue_epic_change(referenced_issue, issuable, current_user)
-      else
-        SystemNoteService.epic_issue(issuable, referenced_issue, current_user, :added)
-        SystemNoteService.issue_on_epic(referenced_issue, issuable, current_user, :added)
-      end
-    end
-
     def extractor_context
       { group: issuable.group }
     end
@@ -65,9 +52,5 @@ module EpicIssues
     def issuable_group_descendants
       @descendants ||= issuable.group.self_and_descendants
     end
-
-    def usage_ping_record_epic_issue_added
-      ::Gitlab::UsageDataCounters::EpicActivityUniqueCounter.track_epic_issue_added(author: current_user)
-    end
   end
 end

ee/app/workers/all_queues.yml

@@ -986,6 +986,15 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: epics_new_epic_issue
+  :worker_name: Epics::NewEpicIssueWorker
+  :feature_category: :epics
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent:
+  :tags: []
 - :name: group_saml_group_sync
   :worker_name: GroupSamlGroupSyncWorker
   :feature_category: :authentication_and_authorization

ee/app/workers/epics/new_epic_issue_worker.rb

+# frozen_string_literal: true
+
+module Epics
+  class NewEpicIssueWorker # rubocop:disable Scalability/IdempotentWorker
+    include ApplicationWorker
+
+    feature_category :epics
+
+    def perform(params)
+      @params = params
+
+      prepare_params
+      return if missing_resources?
+
+      create_notes
+      usage_ping_record_epic_issue_added
+    end
+
+    private
+
+    attr_reader :params, :user, :epic, :issue, :original_epic
+
+    def prepare_params
+      @user = ::User.find_by_id(params['user_id'])
+      @epic = ::Epic.find_by_id(params['epic_id'])
+      @issue = ::Issue.find_by_id(params['issue_id'])
+
+      if params['original_epic_id']
+        @original_epic = ::Epic.find_by_id(params['original_epic_id'])
+      end
+    end
+
+    def missing_resources?
+      return true unless user && epic && issue
+      return true if params['original_epic_id'].present? && original_epic.nil?
+
+      false
+    end
+
+    def issue_moved?
+      original_epic.present?
+    end
+
+    def create_notes
+      if issue_moved?
+        SystemNoteService.epic_issue_moved(original_epic, issue, epic, user)
+        SystemNoteService.issue_epic_change(issue, epic, user)
+      else
+        SystemNoteService.epic_issue(epic, issue, user, :added)
+        SystemNoteService.issue_on_epic(issue, epic, user, :added)
+      end
+    end
+
+    def usage_ping_record_epic_issue_added
+      ::Gitlab::UsageDataCounters::EpicActivityUniqueCounter.track_epic_issue_added(author: user)
+    end
+  end
+end

ee/spec/services/ee/issues/create_service_spec.rb

@@ -90,7 +90,7 @@ RSpec.describe Issues::CreateService do
               expect(epic.due_date).to eq(milestone.due_date)
             end
 
-            it 'generates system notes for adding an epic and milestone' do
+            it 'generates system notes for adding an epic and milestone', :sidekiq_inline do
               expect { service.execute }.to change(Note, :count).by(3).and(change(ResourceMilestoneEvent, :count).by(1))
             end
 

ee/spec/services/ee/issues/update_service_spec.rb

@@ -322,7 +322,7 @@ RSpec.describe Issues::UpdateService do
             project.add_guest(assignee_user1)
           end
 
-          it 'assigns the issue passed to the provided epic' do
+          it 'assigns the issue passed to the provided epic', :sidekiq_inline do
             expect do
               subject
               issue.reload
@@ -335,7 +335,7 @@ RSpec.describe Issues::UpdateService do
           context 'when milestone and epic attributes are changed from description' do
             let(:params) { { description: %(/epic #{epic.to_reference}\n/milestone #{milestone.to_reference}\n/assign #{assignee_user1.to_reference}) } }
 
-            it 'assigns the issue passed to the provided epic' do
+            it 'assigns the issue passed to the provided epic', :sidekiq_inline do
               expect do
                 subject
                 issue.reload

ee/spec/services/ee/notes/quick_actions_service_spec.rb

@@ -48,7 +48,7 @@ RSpec.describe Notes::QuickActionsService do
           expect(execute(note)).to eq('')
         end
 
-        it 'creates a system note' do
+        it 'creates a system note', :sidekiq_inline do
           expect { execute(note) }.to change { Note.system.count }.from(0).to(2)
         end
       end

ee/spec/services/epic_issues/create_service_spec.rb

@@ -41,6 +41,7 @@ RSpec.describe EpicIssues::CreateService do
         expect(subject).to eq(status: :success)
       end
 
+      describe 'async actions', :sidekiq_inline do
         it 'creates 1 system note for epic and 1 system note for issue' do
           expect { subject }.to change { Note.count }.by(2)
         end
@@ -73,6 +74,7 @@ RSpec.describe EpicIssues::CreateService do
           subject
         end
       end
+    end
 
     shared_examples 'returns an error' do
       it 'returns an error' do
@@ -207,7 +209,7 @@ RSpec.describe EpicIssues::CreateService do
               expect(subject).to eq(status: :success)
             end
 
-            it 'creates 2 system notes for each issue' do
+            it 'creates 2 system notes for each issue', :sidekiq_inline do
               expect { subject }.to change { Note.count }.from(0).to(4)
             end
           end
@@ -284,7 +286,7 @@ RSpec.describe EpicIssues::CreateService do
         end
       end
 
-      context 'when an issue is already assigned to another epic' do
+      context 'when an issue is already assigned to another epic', :sidekiq_inline do
         before do
           group.add_developer(user)
           create(:epic_issue, epic: epic, issue: issue)
@@ -311,7 +313,7 @@ RSpec.describe EpicIssues::CreateService do
           is_expected.to eq(status: :success)
         end
 
-        it 'creates 3 system notes' do
+        it 'creates 3 system notes', :sidekiq_inline do
           expect { subject }.to change { Note.count }.by(3)
         end
 

ee/spec/services/epics/tree_reorder_service_spec.rb

@@ -103,7 +103,7 @@ RSpec.describe Epics::TreeReorderService do
               expect { subject }.to change { tree_object_2.reload.epic }.from(epic1).to(epic)
             end
 
-            it 'creates system notes' do
+            it 'creates system notes', :sidekiq_inline do
               expect { subject }.to change { Note.system.count }.by(3)
             end
           end
@@ -212,7 +212,7 @@ RSpec.describe Epics::TreeReorderService do
                 expect(tree_object_1.reload.relative_position).to be > tree_object_2.reload.relative_position
               end
 
-              it 'creates system notes' do
+              it 'creates system notes', :sidekiq_inline do
                 expect { subject }.to change { Note.system.count }.by(3)
               end
             end

ee/spec/workers/epics/new_epic_issue_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Epics::NewEpicIssueWorker do
+  describe '#perform' do
+    let_it_be(:epic) { create(:epic) }
+    let_it_be(:issue) { create(:issue) }
+    let_it_be(:user) { create(:user) }
+
+    let(:params) { { 'epic_id' => epic.id, 'issue_id' => issue.id, 'user_id' => user.id } }
+    let(:extra_params) { {} }
+
+    subject(:perform) { described_class.new.perform(params.merge(extra_params)) }
+
+    shared_examples 'performs successfully' do |action_type|
+      it 'creates system notes' do
+        if action_type == :moved
+          expect(SystemNoteService).to receive(:epic_issue_moved)
+          expect(SystemNoteService).to receive(:issue_epic_change)
+        else
+          expect(SystemNoteService).to receive(:epic_issue)
+          expect(SystemNoteService).to receive(:issue_on_epic)
+        end
+
+        subject
+      end
+
+      it 'updates usage data' do
+        expect(::Gitlab::UsageDataCounters::EpicActivityUniqueCounter).to receive(:track_epic_issue_added)
+
+        subject
+      end
+    end
+
+    shared_examples 'does nothing' do
+      it 'does not create system notes' do
+        expect(SystemNoteService).not_to receive(:epic_issue_moved)
+        expect(SystemNoteService).not_to receive(:issue_epic_change)
+        expect(SystemNoteService).not_to receive(:epic_issue)
+        expect(SystemNoteService).not_to receive(:issue_on_epic)
+      end
+
+      it 'does not update usage data' do
+        expect(::Gitlab::UsageDataCounters::EpicActivityUniqueCounter).not_to receive(:track_epic_issue_added)
+      end
+    end
+
+    it_behaves_like 'performs successfully'
+
+    context 'when reassinging an issue' do
+      let_it_be(:orig_epic) { create(:epic) }
+
+      let(:extra_params) { { 'original_epic_id' => orig_epic.id } }
+
+      it_behaves_like 'performs successfully', :moved
+
+      context 'when original epic does not exist' do
+        let(:extra_params) { { 'original_epic_id' => non_existing_record_id } }
+
+        it_behaves_like 'does nothing'
+      end
+    end
+
+    context 'when epic does not exist' do
+      let(:extra_params) { { 'epic_id' => non_existing_record_id } }
+
+      it_behaves_like 'does nothing'
+    end
+
+    context 'when issue does not exist' do
+      let(:extra_params) { { 'issue_id' => non_existing_record_id } }
+
+      it_behaves_like 'does nothing'
+    end
+
+    context 'when user does not exist' do
+      let(:extra_params) { { 'user_id' => non_existing_record_id } }
+
+      it_behaves_like 'does nothing'
+    end
+  end
+end