Merge branch '2256-fix-crm-contact-quick-actions' into 'master'

Support contact quick actions in issue description

See merge request gitlab-org/gitlab!76218

app/services/issuable_base_service.rb

@@ -56,6 +56,8 @@ class IssuableBaseService < ::BaseProjectService
     # confidential attribute is a special type of metadata and needs to be allowed to be set
     # by non-members on issues in public projects so that security issues can be reported as confidential.
     params.delete(:confidential) unless can?(current_user, :set_confidentiality, issuable)
+    params.delete(:add_contacts) unless can?(current_user, :set_issue_crm_contacts, issuable)
+    params.delete(:remove_contacts) unless can?(current_user, :set_issue_crm_contacts, issuable)
 
     filter_assignees(issuable)
     filter_milestone
@@ -206,6 +208,9 @@ class IssuableBaseService < ::BaseProjectService
       params[:assignee_ids] = process_assignee_ids(params, extra_assignee_ids: issuable.assignee_ids.to_a)
     end
 
+    params.delete(:remove_contacts)
+    add_crm_contact_emails = params.delete(:add_contacts)
+
     issuable.assign_attributes(allowed_create_params(params))
 
     before_create(issuable)
@@ -219,6 +224,7 @@ class IssuableBaseService < ::BaseProjectService
       handle_changes(issuable, { params: params })
 
       after_create(issuable)
+      set_crm_contacts(issuable, add_crm_contact_emails)
       execute_hooks(issuable)
 
       users_to_invalidate = issuable.allows_reviewers? ? issuable.assignees | issuable.reviewers : issuable.assignees
@@ -229,6 +235,12 @@ class IssuableBaseService < ::BaseProjectService
     issuable
   end
 
+  def set_crm_contacts(issuable, add_crm_contact_emails, remove_crm_contact_emails = [])
+    return unless add_crm_contact_emails.present? || remove_crm_contact_emails.present?
+
+    ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { add_emails: add_crm_contact_emails, remove_emails: remove_crm_contact_emails }).execute(issuable)
+  end
+
   def before_create(issuable)
     # To be overridden by subclasses
   end
@@ -254,6 +266,7 @@ class IssuableBaseService < ::BaseProjectService
 
     assign_requested_labels(issuable)
     assign_requested_assignees(issuable)
+    assign_requested_crm_contacts(issuable)
 
     if issuable.changed? || params.present?
       issuable.assign_attributes(allowed_update_params(params))
@@ -414,6 +427,12 @@ class IssuableBaseService < ::BaseProjectService
     issuable.touch
   end
 
+  def assign_requested_crm_contacts(issuable)
+    add_crm_contact_emails = params.delete(:add_contacts)
+    remove_crm_contact_emails = params.delete(:remove_contacts)
+    set_crm_contacts(issuable, add_crm_contact_emails, remove_crm_contact_emails)
+  end
+
   def assign_requested_assignees(issuable)
     return if issuable.is_a?(Epic)
 

app/services/issues/set_crm_contacts_service.rb

@@ -12,7 +12,7 @@ module Issues
       return error_no_permissions unless allowed?
       return error_invalid_params unless valid_params?
 
-      @existing_ids = issue.issue_customer_relations_contacts.map(&:contact_id)
+      @existing_ids = issue.customer_relations_contact_ids
       determine_changes if params[:replace_ids].present?
       return error_too_many if too_many?
 
@@ -24,6 +24,7 @@ module Issues
 
       if issue.valid?
         GraphqlTriggers.issue_crm_contacts_updated(issue)
+        issue.touch
         ServiceResponse.success(payload: issue)
       else
         # The default error isn't very helpful: "Issue customer relations contacts is invalid"

lib/gitlab/quick_actions/issue_actions.rb

@@ -292,17 +292,11 @@ module Gitlab
         condition do
           current_user.can?(:set_issue_crm_contacts, quick_action_target)
         end
+        execution_message do
+          _('One or more contacts were successfully added.')
+        end
         command :add_contacts do |contact_emails|
-          result = ::Issues::SetCrmContactsService
-            .new(project: project, current_user: current_user, params: { add_emails: contact_emails.split(' ') })
-            .execute(quick_action_target)
-
-          @execution_message[:add_contacts] =
-            if result.success?
-              _('One or more contacts were successfully added.')
-            else
-              result.message
-            end
+          @updates[:add_contacts] = contact_emails.split(' ')
         end
 
         desc _('Remove customer relation contacts')
@@ -312,17 +306,11 @@ module Gitlab
         condition do
           current_user.can?(:set_issue_crm_contacts, quick_action_target)
         end
+        execution_message do
+          _('One or more contacts were successfully removed.')
+        end
         command :remove_contacts do |contact_emails|
-          result = ::Issues::SetCrmContactsService
-            .new(project: project, current_user: current_user, params: { remove_emails: contact_emails.split(' ') })
-            .execute(quick_action_target)
-
-          @execution_message[:remove_contacts] =
-            if result.success?
-              _('One or more contacts were successfully removed.')
-            else
-              result.message
-            end
+          @updates[:remove_contacts] = contact_emails.split(' ')
         end
 
         private

spec/services/issues/create_service_spec.rb

@@ -5,7 +5,8 @@ require 'spec_helper'
 RSpec.describe Issues::CreateService do
   include AfterNextHelpers
 
-  let_it_be_with_reload(:project) { create(:project) }
+  let_it_be(:group) { create(:group) }
+  let_it_be_with_reload(:project) { create(:project, group: group) }
   let_it_be(:user) { create(:user) }
 
   let(:spam_params) { double }
@@ -430,25 +431,29 @@ RSpec.describe Issues::CreateService do
     end
 
     context 'Quick actions' do
-      context 'with assignee and milestone in params and command' do
+      context 'with assignee, milestone, and contact in params and command' do
+        let_it_be(:contact) { create(:contact, group: group) }
+
         let(:opts) do
           {
             assignee_ids: [create(:user).id],
             milestone_id: 1,
             title: 'Title',
-            description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}")
+            description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}"),
+            add_contacts: [contact.email]
           }
         end
 
         before_all do
-          project.add_maintainer(user)
+          group.add_maintainer(user)
           project.add_maintainer(assignee)
         end
 
-        it 'assigns and sets milestone to issuable from command' do
+        it 'assigns, sets milestone, and sets contact to issuable from command' do
           expect(issue).to be_persisted
           expect(issue.assignees).to eq([assignee])
           expect(issue.milestone).to eq(milestone)
+          expect(issue.issue_customer_relations_contacts.last.contact).to eq(contact)
         end
       end
     end

spec/services/issues/update_service_spec.rb

@@ -29,6 +29,8 @@ RSpec.describe Issues::UpdateService, :mailer do
   end
 
   describe 'execute' do
+    let_it_be(:contact) { create(:contact, group: group) }
+
     def find_note(starting_with)
       issue.notes.find do |note|
         note && note.note.start_with?(starting_with)
@@ -57,7 +59,8 @@ RSpec.describe Issues::UpdateService, :mailer do
           due_date: Date.tomorrow,
           discussion_locked: true,
           severity: 'low',
-          milestone_id: milestone.id
+          milestone_id: milestone.id,
+          add_contacts: [contact.email]
         }
       end
 
@@ -76,6 +79,7 @@ RSpec.describe Issues::UpdateService, :mailer do
         expect(issue.discussion_locked).to be_truthy
         expect(issue.confidential).to be_falsey
         expect(issue.milestone).to eq milestone
+        expect(issue.issue_customer_relations_contacts.last.contact).to eq contact
       end
 
       it 'updates issue milestone when passing `milestone` param' do

spec/services/quick_actions/interpret_service_spec.rb

@@ -2253,35 +2253,29 @@ RSpec.describe QuickActions::InterpretService do
         end
 
         it 'add_contacts command does not add the contact' do
-          add_command
+          _, updates, _ = add_command
 
-          expect(issue.reload.customer_relations_contacts).to match_array([existing_contact])
+          expect(updates).to be_empty
         end
 
         it 'remove_contacts command does not remove the contact' do
-          remove_command
+          _, updates, _ = remove_command
 
-          expect(issue.reload.customer_relations_contacts).to match_array([existing_contact])
+          expect(updates).to be_empty
         end
       end
 
       it 'add_contacts command adds the contact' do
-        _, _, message = add_command
+        _, updates, message = add_command
 
-        expect(issue.reload.customer_relations_contacts).to match_array([existing_contact, new_contact])
+        expect(updates).to eq(add_contacts: [new_contact.email])
         expect(message).to eq('One or more contacts were successfully added.')
       end
 
-      it 'add_contacts command returns the correct error when something goes wrong' do
-        _, _, message = service.execute("/add_contacts #{new_contact.email} #{new_contact.email} #{new_contact.email} #{new_contact.email} #{new_contact.email} #{new_contact.email} #{new_contact.email}", issue)
-
-        expect(message).to eq('You can only add up to 6 contacts at one time')
-      end
-
       it 'remove_contacts command removes the contact' do
-        _, _, message = remove_command
+        _, updates, message = remove_command
 
-        expect(issue.reload.customer_relations_contacts).to be_empty
+        expect(updates).to eq(remove_contacts: [existing_contact.email])
         expect(message).to eq('One or more contacts were successfully removed.')
       end
     end