Add latest changes from gitlab-org/gitlab@14-8-stable-eev14.8.0-rc42

6 files changed, 147 insertions, 73 deletions

diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 158f9dec83e..1f6118e9fcc 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -118,7 +118,7 @@ RSpec.describe Issues::CloseService do
           expect { service.execute(issue) }.to change { issue.notes.count }.by(1)
 
           new_note = issue.notes.last
-          expect(new_note.note).to eq('changed the status to **Resolved** by closing the incident')
+          expect(new_note.note).to eq('changed the incident status to **Resolved** by closing the incident')
           expect(new_note.author).to eq(user)
         end
 
@@ -334,8 +334,12 @@ RSpec.describe Issues::CloseService do
           let!(:alert) { create(:alert_management_alert, issue: issue, project: project) }
 
           it 'resolves an alert and sends a system note' do
-            expect_next_instance_of(SystemNotes::AlertManagementService) do |notes_service|
-              expect(notes_service).to receive(:closed_alert_issue).with(issue)
+            expect_any_instance_of(SystemNoteService) do |notes_service|
+              expect(notes_service).to receive(:change_alert_status).with(
+                alert,
+                current_user,
+                " by closing issue #{issue.to_reference(project)}"
+              )
             end
 
             close_issue
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index b2dcfb5c6d3..f4bb1f0877b 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe Issues::CreateService do
 
       expect(described_class.rate_limiter_scoped_and_keyed.key).to eq(:issues_create)
       expect(described_class.rate_limiter_scoped_and_keyed.opts[:scope]).to eq(%i[project current_user external_author])
-      expect(described_class.rate_limiter_scoped_and_keyed.rate_limiter_klass).to eq(Gitlab::ApplicationRateLimiter)
+      expect(described_class.rate_limiter_scoped_and_keyed.rate_limiter).to eq(Gitlab::ApplicationRateLimiter)
     end
   end
 
@@ -92,6 +92,21 @@ RSpec.describe Issues::CreateService do
         end
       end
 
+      context 'when setting a position' do
+        let(:issue_before) { create(:issue, project: project, relative_position: 10) }
+        let(:issue_after) { create(:issue, project: project, relative_position: 50) }
+
+        before do
+          opts.merge!(move_between_ids: [issue_before.id, issue_after.id])
+        end
+
+        it 'sets the correct relative position' do
+          expect(issue).to be_persisted
+          expect(issue.relative_position).to be_present
+          expect(issue.relative_position).to be_between(issue_before.relative_position, issue_after.relative_position)
+        end
+      end
+
       it_behaves_like 'not an incident issue'
 
       context 'when issue is incident type' do
@@ -100,7 +115,6 @@ RSpec.describe Issues::CreateService do
         end
 
         let(:current_user) { user }
-        let(:incident_label_attributes) { attributes_for(:label, :incident) }
 
         subject { issue }
 
@@ -114,12 +128,6 @@ RSpec.describe Issues::CreateService do
           end
 
           it_behaves_like 'incident issue'
-          it_behaves_like 'does not have incident label'
-
-          it 'does not create an incident label' do
-            expect { subject }
-              .to not_change { Label.where(incident_label_attributes).count }
-          end
 
           it 'calls IncidentManagement::Incidents::CreateEscalationStatusService' do
             expect_next(::IncidentManagement::IssuableEscalationStatuses::CreateService, a_kind_of(Issue))
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index ef501f47f0d..35a380e01d0 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -168,6 +168,48 @@ RSpec.describe Issues::MoveService do
         end
       end
 
+      context 'issue with contacts' do
+        let_it_be(:contacts) { create_list(:contact, 2, group: group) }
+
+        before do
+          old_issue.customer_relations_contacts = contacts
+        end
+
+        it 'preserves contacts' do
+          new_issue = move_service.execute(old_issue, new_project)
+
+          expect(new_issue.customer_relations_contacts).to eq(contacts)
+        end
+
+        context 'when moving to another root group' do
+          let(:another_project) { create(:project, namespace: create(:group)) }
+
+          before do
+            another_project.add_reporter(user)
+          end
+
+          it 'does not preserve contacts' do
+            new_issue = move_service.execute(old_issue, another_project)
+
+            expect(new_issue.customer_relations_contacts).to be_empty
+          end
+        end
+
+        context 'when customer_relations feature is disabled' do
+          let(:another_project) { create(:project, namespace: create(:group)) }
+
+          before do
+            stub_feature_flags(customer_relations: false)
+          end
+
+          it 'does not preserve contacts' do
+            new_issue = move_service.execute(old_issue, new_project)
+
+            expect(new_issue.customer_relations_contacts).to be_empty
+          end
+        end
+      end
+
       context 'moving to same project' do
         let(:new_project) { old_project }
 
diff --git a/spec/services/issues/reorder_service_spec.rb b/spec/services/issues/reorder_service_spec.rb
index 15668a3aa23..392930c1b9f 100644
--- a/spec/services/issues/reorder_service_spec.rb
+++ b/spec/services/issues/reorder_service_spec.rb
@@ -67,20 +67,10 @@ RSpec.describe Issues::ReorderService do
       it_behaves_like 'issues reorder service'
 
       context 'when ordering in a group issue list' do
-        let(:params) { { move_after_id: issue2.id, move_before_id: issue3.id, group_full_path: group.full_path } }
+        let(:params) { { move_after_id: issue2.id, move_before_id: issue3.id } }
 
         subject { service(params) }
 
-        it 'sends the board_group_id parameter' do
-          match_params = { move_between_ids: [issue2.id, issue3.id], board_group_id: group.id }
-
-          expect(Issues::UpdateService)
-            .to receive(:new).with(project: project, current_user: user, params: match_params)
-            .and_return(double(execute: build(:issue)))
-
-          subject.execute(issue1)
-        end
-
         it 'sorts issues' do
           project2 = create(:project, namespace: group)
           issue4   = create(:issue, project: project2)
diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb
index 2418f317551..64011a7a003 100644
--- a/spec/services/issues/set_crm_contacts_service_spec.rb
+++ b/spec/services/issues/set_crm_contacts_service_spec.rb
@@ -7,20 +7,41 @@ RSpec.describe Issues::SetCrmContactsService do
   let_it_be(:group) { create(:group, :crm_enabled) }
   let_it_be(:project) { create(:project, group: group) }
   let_it_be(:contacts) { create_list(:contact, 4, group: group) }
+  let_it_be(:issue, reload: true) { create(:issue, project: project) }
+  let_it_be(:issue_contact_1) do
+    create(:issue_customer_relations_contact, issue: issue, contact: contacts[0]).contact
+  end
 
-  let(:issue) { create(:issue, project: project) }
-  let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" }
-
-  before do
-    create(:issue_customer_relations_contact, issue: issue, contact: contacts[0])
-    create(:issue_customer_relations_contact, issue: issue, contact: contacts[1])
+  let_it_be(:issue_contact_2) do
+    create(:issue_customer_relations_contact, issue: issue, contact: contacts[1]).contact
   end
 
+  let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" }
+
   subject(:set_crm_contacts) do
     described_class.new(project: project, current_user: user, params: params).execute(issue)
   end
 
   describe '#execute' do
+    shared_examples 'setting contacts' do
+      it 'updates the issue with correct contacts' do
+        response = set_crm_contacts
+
+        expect(response).to be_success
+        expect(issue.customer_relations_contacts).to match_array(expected_contacts)
+      end
+    end
+
+    shared_examples 'adds system note' do |added_count, removed_count|
+      it 'calls SystemNoteService.change_issuable_contacts with correct counts' do
+        expect(SystemNoteService)
+          .to receive(:change_issuable_contacts)
+          .with(issue, project, user, added_count, removed_count)
+
+        set_crm_contacts
+      end
+    end
+
     context 'when the user has no permission' do
       let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } }
 
@@ -67,56 +88,63 @@ RSpec.describe Issues::SetCrmContactsService do
 
       context 'replace' do
         let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } }
+        let(:expected_contacts) { [contacts[1], contacts[2]] }
 
-        it 'updates the issue with correct contacts' do
-          response = set_crm_contacts
-
-          expect(response).to be_success
-          expect(issue.customer_relations_contacts).to match_array([contacts[1], contacts[2]])
-        end
+        it_behaves_like 'setting contacts'
+        it_behaves_like 'adds system note', 1, 1
       end
 
       context 'add' do
-        let(:params) { { add_ids: [contacts[3].id] } }
-
-        it 'updates the issue with correct contacts' do
-          response = set_crm_contacts
+        let(:added_contact) { contacts[3] }
+        let(:params) { { add_ids: [added_contact.id] } }
+        let(:expected_contacts) { [issue_contact_1, issue_contact_2, added_contact] }
 
-          expect(response).to be_success
-          expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[1], contacts[3]])
-        end
+        it_behaves_like 'setting contacts'
+        it_behaves_like 'adds system note', 1, 0
       end
 
       context 'add by email' do
-        let(:params) { { add_emails: [contacts[3].email] } }
+        let(:added_contact) { contacts[3] }
+        let(:expected_contacts) { [issue_contact_1, issue_contact_2, added_contact] }
 
-        it 'updates the issue with correct contacts' do
-          response = set_crm_contacts
+        context 'with pure emails in params' do
+          let(:params) { { add_emails: [contacts[3].email] } }
 
-          expect(response).to be_success
-          expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[1], contacts[3]])
+          it_behaves_like 'setting contacts'
+          it_behaves_like 'adds system note', 1, 0
+        end
+
+        context 'with autocomplete prefix emails in params' do
+          let(:params) { { add_emails: ["[\"contact:\"#{contacts[3].email}\"]"] } }
+
+          it_behaves_like 'setting contacts'
+          it_behaves_like 'adds system note', 1, 0
         end
       end
 
       context 'remove' do
         let(:params) { { remove_ids: [contacts[0].id] } }
+        let(:expected_contacts) { [contacts[1]] }
 
-        it 'updates the issue with correct contacts' do
-          response = set_crm_contacts
-
-          expect(response).to be_success
-          expect(issue.customer_relations_contacts).to match_array([contacts[1]])
-        end
+        it_behaves_like 'setting contacts'
+        it_behaves_like 'adds system note', 0, 1
       end
 
       context 'remove by email' do
-        let(:params) { { remove_emails: [contacts[0].email] } }
+        let(:expected_contacts) { [contacts[1]] }
 
-        it 'updates the issue with correct contacts' do
-          response = set_crm_contacts
+        context 'with pure email in params' do
+          let(:params) { { remove_emails: [contacts[0].email] } }
 
-          expect(response).to be_success
-          expect(issue.customer_relations_contacts).to match_array([contacts[1]])
+          it_behaves_like 'setting contacts'
+          it_behaves_like 'adds system note', 0, 1
+        end
+
+        context 'with autocomplete prefix and suffix email in params' do
+          let(:params) { { remove_emails: ["[contact:#{contacts[0].email}]"] } }
+
+          it_behaves_like 'setting contacts'
+          it_behaves_like 'adds system note', 0, 1
         end
       end
 
@@ -145,15 +173,19 @@ RSpec.describe Issues::SetCrmContactsService do
 
       context 'when combining params' do
         let(:error_invalid_params) { 'You cannot combine replace_ids with add_ids or remove_ids' }
+        let(:expected_contacts) { [contacts[0], contacts[3]] }
 
         context 'add and remove' do
-          let(:params) { { remove_ids: [contacts[1].id], add_ids: [contacts[3].id] } }
+          context 'with contact ids' do
+            let(:params) { { remove_ids: [contacts[1].id], add_ids: [contacts[3].id] } }
 
-          it 'updates the issue with correct contacts' do
-            response = set_crm_contacts
+            it_behaves_like 'setting contacts'
+          end
+
+          context 'with contact emails' do
+            let(:params) { { remove_emails: [contacts[1].email], add_emails: ["[\"contact:#{contacts[3].email}]"] } }
 
-            expect(response).to be_success
-            expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[3]])
+            it_behaves_like 'setting contacts'
           end
         end
 
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 11ed47b84d9..95394ba6597 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -385,7 +385,6 @@ RSpec.describe Issues::UpdateService, :mailer do
           [issue_1, issue_2, issue_3].map(&:save)
 
           opts[:move_between_ids] = [issue_1.id, issue_2.id]
-          opts[:board_group_id] = group.id
 
           described_class.new(project: issue_3.project, current_user: user, params: opts).execute(issue_3)
           expect(issue_2.relative_position).to be_between(issue_1.relative_position, issue_2.relative_position)
@@ -1147,11 +1146,11 @@ RSpec.describe Issues::UpdateService, :mailer do
       let(:opts) { { escalation_status: { status: 'acknowledged' } } }
       let(:escalation_update_class) { ::IncidentManagement::IssuableEscalationStatuses::AfterUpdateService }
 
-      shared_examples 'updates the escalation status record' do |expected_status|
+      shared_examples 'updates the escalation status record' do |expected_status, expected_reason = nil|
         let(:service_double) { instance_double(escalation_update_class) }
 
         it 'has correct value' do
-          expect(escalation_update_class).to receive(:new).with(issue, user).and_return(service_double)
+          expect(escalation_update_class).to receive(:new).with(issue, user, status_change_reason: expected_reason).and_return(service_double)
           expect(service_double).to receive(:execute)
 
           update_issue(opts)
@@ -1197,6 +1196,12 @@ RSpec.describe Issues::UpdateService, :mailer do
             end
           end
 
+          context 'with a status change reason provided' do
+            let(:opts) { { escalation_status: { status: 'acknowledged', status_change_reason: ' by changing the alert status' } } }
+
+            it_behaves_like 'updates the escalation status record', :acknowledged, ' by changing the alert status'
+          end
+
           context 'with unsupported status value' do
             let(:opts) { { escalation_status: { status: 'unsupported-status' } } }
 
@@ -1303,19 +1308,12 @@ RSpec.describe Issues::UpdateService, :mailer do
     end
 
     context 'when moving an issue ' do
-      it 'raises an error for invalid move ids within a project' do
+      it 'raises an error for invalid move ids' do
         opts = { move_between_ids: [9000, non_existing_record_id] }
 
         expect { described_class.new(project: issue.project, current_user: user, params: opts).execute(issue) }
             .to raise_error(ActiveRecord::RecordNotFound)
       end
-
-      it 'raises an error for invalid move ids within a group' do
-        opts = { move_between_ids: [9000, non_existing_record_id], board_group_id: create(:group).id }
-
-        expect { described_class.new(project: issue.project, current_user: user, params: opts).execute(issue) }
-            .to raise_error(ActiveRecord::RecordNotFound)
-      end
     end
 
     include_examples 'issuable update service' do