Merge branch '2256-add-issue-contact-quick-actions' into 'master'

Add customer relations contact quick actions

See merge request gitlab-org/gitlab!73413

app/graphql/mutations/issues/set_crm_contacts.rb

@@ -5,7 +5,7 @@ module Mutations
     class SetCrmContacts < Base
       graphql_name 'IssueSetCrmContacts'
 
-      argument :crm_contact_ids,
+      argument :contact_ids,
                [::Types::GlobalIDType[::CustomerRelations::Contact]],
                required: true,
                description: 'Customer relations contact IDs to set. Replaces existing contacts by default.'
@@ -15,27 +15,27 @@ module Mutations
                required: false,
                description: 'Changes the operation mode. Defaults to REPLACE.'
 
-      def resolve(project_path:, iid:, crm_contact_ids:, operation_mode: Types::MutationOperationModeEnum.enum[:replace])
+      def resolve(project_path:, iid:, contact_ids:, operation_mode: Types::MutationOperationModeEnum.enum[:replace])
         issue = authorized_find!(project_path: project_path, iid: iid)
         project = issue.project
         raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, project.group, default_enabled: :yaml)
 
-        crm_contact_ids = crm_contact_ids.compact.map do |crm_contact_id|
-          raise Gitlab::Graphql::Errors::ArgumentError, "Contact #{crm_contact_id} is invalid." unless crm_contact_id.respond_to?(:model_id)
+        contact_ids = contact_ids.compact.map do |contact_id|
+          raise Gitlab::Graphql::Errors::ArgumentError, "Contact #{contact_id} is invalid." unless contact_id.respond_to?(:model_id)
 
-          crm_contact_id.model_id.to_i
+          contact_id.model_id.to_i
         end
 
         attribute_name = case operation_mode
                          when Types::MutationOperationModeEnum.enum[:append]
-                           :add_crm_contact_ids
+                           :add_ids
                          when Types::MutationOperationModeEnum.enum[:remove]
-                           :remove_crm_contact_ids
+                           :remove_ids
                          else
-                           :crm_contact_ids
+                           :replace_ids
                          end
 
-        response = ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { attribute_name => crm_contact_ids })
+        response = ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { attribute_name => contact_ids })
           .execute(issue)
 
         {

app/models/application_record.rb

@@ -7,6 +7,10 @@ class ApplicationRecord < ActiveRecord::Base
 
   self.abstract_class = true
 
+  # We should avoid using pluck https://docs.gitlab.com/ee/development/sql.html#plucking-ids
+  # but, if we are going to use it, let's try and limit the number of records
+  MAX_PLUCK = 1_000
+
   alias_method :reset, :reload
 
   def self.without_order

app/models/customer_relations/contact.rb

@@ -25,6 +25,13 @@ class CustomerRelations::Contact < ApplicationRecord
   validates :description, length: { maximum: 1024 }
   validate :validate_email_format
 
+  def self.find_ids_by_emails(group_id, emails)
+    raise ArgumentError, "Cannot lookup more than #{MAX_PLUCK} emails" if emails.length > MAX_PLUCK
+
+    where(group_id: group_id, email: emails)
+      .pluck(:id)
+  end
+
   private
 
   def validate_email_format

app/models/customer_relations/issue_contact.rb

@@ -8,6 +8,14 @@ class CustomerRelations::IssueContact < ApplicationRecord
 
   validate :contact_belongs_to_issue_group
 
+  def self.find_contact_ids_by_emails(issue_id, emails)
+    raise ArgumentError, "Cannot lookup more than #{MAX_PLUCK} emails" if emails.length > MAX_PLUCK
+
+    joins(:contact)
+      .where(issue_id: issue_id, customer_relations_contacts: { email: emails })
+      .pluck(:contact_id)
+  end
+
   private
 
   def contact_belongs_to_issue_group

app/services/issues/set_crm_contacts_service.rb

@@ -2,10 +2,9 @@
 
 module Issues
   class SetCrmContactsService < ::BaseProjectService
-    attr_accessor :issue, :errors
-
     MAX_ADDITIONAL_CONTACTS = 6
 
+    # Replacing contacts by email is not currently supported
     def execute(issue)
       @issue = issue
       @errors = []
@@ -13,12 +12,15 @@ module Issues
       return error_no_permissions unless allowed?
       return error_invalid_params unless valid_params?
 
-      determine_changes if params[:crm_contact_ids]
-
+      @existing_ids = issue.issue_customer_relations_contacts.map(&:contact_id)
+      determine_changes if params[:replace_ids].present?
       return error_too_many if too_many?
 
-      add_contacts if params[:add_crm_contact_ids]
-      remove_contacts if params[:remove_crm_contact_ids]
+      add if params[:add_ids].present?
+      remove if params[:remove_ids].present?
+
+      add_by_email if params[:add_emails].present?
+      remove_by_email if params[:remove_emails].present?
 
       if issue.valid?
         ServiceResponse.success(payload: issue)
@@ -26,20 +28,31 @@ module Issues
         # The default error isn't very helpful: "Issue customer relations contacts is invalid"
         issue.errors.delete(:issue_customer_relations_contacts)
         issue.errors.add(:issue_customer_relations_contacts, errors.to_sentence)
-        ServiceResponse.error(payload: issue, message: issue.errors.full_messages)
+        ServiceResponse.error(payload: issue, message: issue.errors.full_messages.to_sentence)
       end
     end
 
     private
 
+    attr_accessor :issue, :errors, :existing_ids
+
     def determine_changes
-      existing_contact_ids = issue.issue_customer_relations_contacts.map(&:contact_id)
-      params[:add_crm_contact_ids] = params[:crm_contact_ids] - existing_contact_ids
-      params[:remove_crm_contact_ids] = existing_contact_ids - params[:crm_contact_ids]
+      params[:add_ids] = params[:replace_ids] - existing_ids
+      params[:remove_ids] = existing_ids - params[:replace_ids]
+    end
+
+    def add
+      add_by_id(params[:add_ids])
+    end
+
+    def add_by_email
+      contact_ids = ::CustomerRelations::Contact.find_ids_by_emails(project_group.id, params[:add_emails])
+      add_by_id(contact_ids)
     end
 
-    def add_contacts
-      params[:add_crm_contact_ids].uniq.each do |contact_id|
+    def add_by_id(contact_ids)
+      contact_ids -= existing_ids
+      contact_ids.uniq.each do |contact_id|
         issue_contact = issue.issue_customer_relations_contacts.create(contact_id: contact_id)
 
         unless issue_contact.persisted?
@@ -49,9 +62,19 @@ module Issues
       end
     end
 
-    def remove_contacts
+    def remove
+      remove_by_id(params[:remove_ids])
+    end
+
+    def remove_by_email
+      contact_ids = ::CustomerRelations::IssueContact.find_contact_ids_by_emails(issue.id, params[:remove_emails])
+      remove_by_id(contact_ids)
+    end
+
+    def remove_by_id(contact_ids)
+      contact_ids &= existing_ids
       issue.issue_customer_relations_contacts
-        .where(contact_id: params[:remove_crm_contact_ids]) # rubocop: disable CodeReuse/ActiveRecord
+        .where(contact_id: contact_ids) # rubocop: disable CodeReuse/ActiveRecord
         .delete_all
     end
 
@@ -64,27 +87,43 @@ module Issues
     end
 
     def set_present?
-      params[:crm_contact_ids].present?
+      params[:replace_ids].present?
     end
 
     def add_or_remove_present?
-      params[:add_crm_contact_ids].present? || params[:remove_crm_contact_ids].present?
+      add_present? || remove_present?
+    end
+
+    def add_present?
+      params[:add_ids].present? || params[:add_emails].present?
+    end
+
+    def remove_present?
+      params[:remove_ids].present? || params[:remove_emails].present?
     end
 
     def too_many?
-      params[:add_crm_contact_ids] && params[:add_crm_contact_ids].length > MAX_ADDITIONAL_CONTACTS
+      too_many_ids? || too_many_emails?
+    end
+
+    def too_many_ids?
+      params[:add_ids] && params[:add_ids].length > MAX_ADDITIONAL_CONTACTS
+    end
+
+    def too_many_emails?
+      params[:add_emails] && params[:add_emails].length > MAX_ADDITIONAL_CONTACTS
     end
 
     def error_no_permissions
-      ServiceResponse.error(message: ['You have insufficient permissions to set customer relations contacts for this issue'])
+      ServiceResponse.error(message: _('You have insufficient permissions to set customer relations contacts for this issue'))
     end
 
     def error_invalid_params
-      ServiceResponse.error(message: ['You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids'])
+      ServiceResponse.error(message: _('You cannot combine replace_ids with add_ids or remove_ids'))
     end
 
     def error_too_many
-      ServiceResponse.error(payload: issue, message: ["You can only add up to #{MAX_ADDITIONAL_CONTACTS} contacts at one time"])
+      ServiceResponse.error(payload: issue, message: _("You can only add up to %{max_contacts} contacts at one time" % { max_contacts: MAX_ADDITIONAL_CONTACTS }))
     end
   end
 end

config/feature_flags/development/customer_relations.yml

 ---
 name: customer_relations
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69472
-rollout_issue_url: 
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346082
 milestone: '14.3'
 type: development
 group: group::product planning

config/metrics/counts_28d/20211109114953_i_quickactions_add_contacts_monthly.yml

+---
+key_path: redis_hll_counters.quickactions.i_quickactions_add_contacts_monthly
+description: Count of MAU using the `/add_contacts` quick action
+product_section: dev
+product_stage: plan
+product_group: group::product planning
+product_category: service_desk
+value_type: number
+status: active
+milestone: '14.5'
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73413
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_add_contacts
+distribution:
+  - ce
+  - ee
+tier:
+  - free
+  - premium
+  - ultimate

config/metrics/counts_28d/20211109120251_i_quickactions_remove_contacts_monthly.yml

+---
+key_path: redis_hll_counters.quickactions.i_quickactions_remove_contacts_monthly
+description: Count of MAU using the `/remove_contacts` quick action
+product_section: dev
+product_stage: plan
+product_group: group::product planning
+product_category: service_desk
+value_type: number
+status: active
+milestone: '14.5'
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73413
+time_frame: 28d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_remove_contacts
+distribution:
+  - ce
+  - ee
+tier:
+  - free
+  - premium
+  - ultimate

config/metrics/counts_7d/20211109114948_i_quickactions_add_contacts_weekly.yml

+---
+key_path: redis_hll_counters.quickactions.i_quickactions_add_contacts_weekly
+description: Count of WAU using the `/add_contacts` quick action
+product_section: dev
+product_stage: plan
+product_group: group::product planning
+product_category: service_desk
+value_type: number
+status: active
+milestone: '14.5'
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73413
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_add_contacts
+distribution:
+  - ce
+  - ee
+tier:
+  - free
+  - premium
+  - ultimate

config/metrics/counts_7d/20211109120245_i_quickactions_remove_contacts_weekly.yml

+---
+key_path: redis_hll_counters.quickactions.i_quickactions_remove_contacts_weekly
+description: Count of WAU using the `/remove_contacts` quick action
+product_section: dev
+product_stage: plan
+product_group: group::product planning
+product_category: service_desk
+value_type: number
+status: active
+milestone: '14.5'
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73413
+time_frame: 7d
+data_source: redis_hll
+data_category: optional
+instrumentation_class: RedisHLLMetric
+options:
+  events:
+    - i_quickactions_remove_contacts
+distribution:
+  - ce
+  - ee
+tier:
+  - free
+  - premium
+  - ultimate

doc/api/graphql/reference/index.md

@@ -2846,7 +2846,7 @@ Input type: `IssueSetCrmContactsInput`
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="mutationissuesetcrmcontactsclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationissuesetcrmcontactscrmcontactids"></a>`crmContactIds` | [`[CustomerRelationsContactID!]!`](#customerrelationscontactid) | Customer relations contact IDs to set. Replaces existing contacts by default. |
+| <a id="mutationissuesetcrmcontactscontactids"></a>`contactIds` | [`[CustomerRelationsContactID!]!`](#customerrelationscontactid) | Customer relations contact IDs to set. Replaces existing contacts by default. |
 | <a id="mutationissuesetcrmcontactsiid"></a>`iid` | [`String!`](#string) | IID of the issue to mutate. |
 | <a id="mutationissuesetcrmcontactsoperationmode"></a>`operationMode` | [`MutationOperationMode`](#mutationoperationmode) | Changes the operation mode. Defaults to REPLACE. |
 | <a id="mutationissuesetcrmcontactsprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the issue to mutate is in. |

lib/gitlab/quick_actions/issue_actions.rb

@@ -206,7 +206,7 @@ module Gitlab
         end
 
         desc _('Add Zoom meeting')
-        explanation _('Adds a Zoom meeting')
+        explanation _('Adds a Zoom meeting.')
         params '<Zoom URL>'
         types Issue
         condition do
@@ -223,7 +223,7 @@ module Gitlab
         end
 
         desc _('Remove Zoom meeting')
-        explanation _('Remove Zoom meeting')
+        explanation _('Remove Zoom meeting.')
         execution_message _('Zoom meeting removed')
         types Issue
         condition do
@@ -236,7 +236,7 @@ module Gitlab
         end
 
         desc _('Add email participant(s)')
-        explanation _('Adds email participant(s)')
+        explanation _('Adds email participant(s).')
         params 'email1@example.com email2@example.com (up to 6 emails)'
         types Issue
         condition do
@@ -285,6 +285,46 @@ module Gitlab
             end
         end
 
+        desc _('Add customer relation contacts')
+        explanation _('Add customer relation contact(s).')
+        params 'contact@example.com person@example.org'
+        types Issue
+        condition do
+          current_user.can?(:set_issue_crm_contacts, quick_action_target)
+        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
+        end
+
+        desc _('Remove customer relation contacts')
+        explanation _('Remove customer relation contact(s).')
+        params 'contact@example.com person@example.org'
+        types Issue
+        condition do
+          current_user.can?(:set_issue_crm_contacts, quick_action_target)
+        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
+        end
+
         private
 
         def zoom_link_service

lib/gitlab/usage_data_counters/known_events/quickactions.yml

@@ -279,3 +279,11 @@
   category: quickactions
   redis_slot: quickactions
   aggregation: weekly
+- name: i_quickactions_add_contacts
+  category: quickactions
+  redis_slot: quickactions
+  aggregation: weekly
+- name: i_quickactions_remove_contacts
+  category: quickactions
+  redis_slot: quickactions
+  aggregation: weekly

locale/gitlab.pot

@@ -2019,6 +2019,12 @@ msgstr ""
 msgid "Add commit messages as comments to Pivotal Tracker stories. %{docs_link}"
 msgstr ""
 
+msgid "Add customer relation contact(s)."
+msgstr ""
+
+msgid "Add customer relation contacts"
+msgstr ""
+
 msgid "Add deploy freeze"
 msgstr ""
 
@@ -2220,7 +2226,7 @@ msgstr ""
 msgid "Adds %{labels} %{label_text}."
 msgstr ""
 
-msgid "Adds a Zoom meeting"
+msgid "Adds a Zoom meeting."
 msgstr ""
 
 msgid "Adds a to do."
@@ -2229,7 +2235,7 @@ msgstr ""
 msgid "Adds an issue to an epic."
 msgstr ""
 
-msgid "Adds email participant(s)"
+msgid "Adds email participant(s)."
 msgstr ""
 
 msgid "Adjust how frequently the GitLab UI polls for updates."
@@ -24229,6 +24235,12 @@ msgid_plural "%d more items"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "One or more contacts were successfully added."
+msgstr ""
+
+msgid "One or more contacts were successfully removed."
+msgstr ""
+
 msgid "One or more groups that you don't have access to."
 msgstr ""
 
@@ -28753,6 +28765,9 @@ msgstr ""
 msgid "Remove Zoom meeting"
 msgstr ""
 
+msgid "Remove Zoom meeting."
+msgstr ""
+
 msgid "Remove access"
 msgstr ""
 
@@ -28792,6 +28807,12 @@ msgstr ""
 msgid "Remove child epic from an epic"
 msgstr ""
 
+msgid "Remove customer relation contact(s)."
+msgstr ""
+
+msgid "Remove customer relation contacts"
+msgstr ""
+
 msgid "Remove deploy key"
 msgstr ""
 
@@ -39569,6 +39590,9 @@ msgstr ""
 msgid "You can only %{action} files when you are on a branch"
 msgstr ""
 
+msgid "You can only add up to %{max_contacts} contacts at one time"
+msgstr ""
+
 msgid "You can only edit files when you are on a branch"
 msgstr ""
 
@@ -39611,6 +39635,9 @@ msgstr ""
 msgid "You cannot access the raw file. Please wait a minute."
 msgstr ""
 
+msgid "You cannot combine replace_ids with add_ids or remove_ids"
+msgstr ""
+
 msgid "You cannot impersonate a blocked user"
 msgstr ""
 
@@ -39749,6 +39776,9 @@ msgstr ""
 msgid "You have insufficient permissions to remove this HTTP integration"
 msgstr ""
 
+msgid "You have insufficient permissions to set customer relations contacts for this issue"
+msgstr ""
+
 msgid "You have insufficient permissions to update an on-call schedule for this project"
 msgstr ""
 

spec/factories/customer_relations/contacts.rb

@@ -6,6 +6,7 @@ FactoryBot.define do
 
     first_name { generate(:name) }
     last_name { generate(:name) }
+    email { generate(:email) }
 
     trait :with_organization do
       organization

spec/models/customer_relations/contact_spec.rb

@@ -36,4 +36,27 @@ RSpec.describe CustomerRelations::Contact, type: :model do
       expect(contact.phone).to eq('123456')
     end
   end
+
+  describe '#self.find_ids_by_emails' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:group_contacts) { create_list(:contact, 2, group: group) }
+    let_it_be(:other_contacts) { create_list(:contact, 2) }
+
+    it 'returns ids of contacts from group' do
+      contact_ids = described_class.find_ids_by_emails(group.id, group_contacts.pluck(:email))
+
+      expect(contact_ids).to match_array(group_contacts.pluck(:id))
+    end
+
+    it 'does not return ids of contacts from other groups' do
+      contact_ids = described_class.find_ids_by_emails(group.id, other_contacts.pluck(:email))
+
+      expect(contact_ids).to be_empty
+    end
+
+    it 'raises ArgumentError when called with too many emails' do
+      too_many_emails = described_class::MAX_PLUCK + 1
+      expect { described_class.find_ids_by_emails(group.id, Array(0..too_many_emails)) }.to raise_error(ArgumentError)
+    end
+  end
 end

spec/models/customer_relations/issue_contact_spec.rb

@@ -4,6 +4,9 @@ require 'spec_helper'
 
 RSpec.describe CustomerRelations::IssueContact do
   let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:issue) { create(:issue, project: project) }
 
   subject { issue_contact }
 
@@ -19,9 +22,6 @@ RSpec.describe CustomerRelations::IssueContact do
     let(:stubbed) { build_stubbed(:issue_customer_relations_contact) }
     let(:created) { create(:issue_customer_relations_contact) }
 
-    let(:group) { build(:group) }
-    let(:project) { build(:project, group: group) }
-    let(:issue) { build(:issue, project: project) }
     let(:contact) { build(:contact, group: group) }
     let(:for_issue) { build(:issue_customer_relations_contact, :for_issue, issue: issue) }
     let(:for_contact) { build(:issue_customer_relations_contact, :for_contact, contact: contact) }
@@ -45,4 +45,26 @@ RSpec.describe CustomerRelations::IssueContact do
       expect(built).not_to be_valid
     end
   end
+
+  describe '#self.find_contact_ids_by_emails' do
+    let_it_be(:for_issue) { create_list(:issue_customer_relations_contact, 2, :for_issue, issue: issue) }
+    let_it_be(:not_for_issue) { create_list(:issue_customer_relations_contact, 2) }
+
+    it 'returns ids of contacts from issue' do
+      contact_ids = described_class.find_contact_ids_by_emails(issue.id, for_issue.map(&:contact).pluck(:email))
+
+      expect(contact_ids).to match_array(for_issue.pluck(:contact_id))
+    end
+
+    it 'does not return ids of contacts from other issues' do
+      contact_ids = described_class.find_contact_ids_by_emails(issue.id, not_for_issue.map(&:contact).pluck(:email))
+
+      expect(contact_ids).to be_empty
+    end
+
+    it 'raises ArgumentError when called with too many emails' do
+      too_many_emails = described_class::MAX_PLUCK + 1
+      expect { described_class.find_contact_ids_by_emails(issue.id, Array(0..too_many_emails)) }.to raise_error(ArgumentError)
+    end
+  end
 end

spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe 'Setting issues crm contacts' do
 
   let(:issue) { create(:issue, project: project) }
   let(:operation_mode) { Types::MutationOperationModeEnum.default_mode }
-  let(:crm_contact_ids) { [global_id_of(contacts[1]), global_id_of(contacts[2])] }
+  let(:contact_ids) { [global_id_of(contacts[1]), global_id_of(contacts[2])] }
   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" }
 
   let(:mutation) do
@@ -20,7 +20,7 @@ RSpec.describe 'Setting issues crm contacts' do
       project_path: issue.project.full_path,
       iid: issue.iid.to_s,
       operation_mode: operation_mode,
-      crm_contact_ids: crm_contact_ids
+      contact_ids: contact_ids
     }
 
     graphql_mutation(:issue_set_crm_contacts, variables,
@@ -83,7 +83,7 @@ RSpec.describe 'Setting issues crm contacts' do
     end
 
     context 'append' do
-      let(:crm_contact_ids) { [global_id_of(contacts[3])] }
+      let(:contact_ids) { [global_id_of(contacts[3])] }
       let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] }
 
       it 'updates the issue with correct contacts' do
@@ -95,7 +95,7 @@ RSpec.describe 'Setting issues crm contacts' do
     end
 
     context 'remove' do
-      let(:crm_contact_ids) { [global_id_of(contacts[0])] }
+      let(:contact_ids) { [global_id_of(contacts[0])] }
       let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] }
 
       it 'updates the issue with correct contacts' do
@@ -107,7 +107,7 @@ RSpec.describe 'Setting issues crm contacts' do
     end
 
     context 'when the contact does not exist' do
-      let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
+      let(:contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
 
       it 'returns expected error' do
         post_graphql_mutation(mutation, current_user: user)
@@ -120,7 +120,7 @@ RSpec.describe 'Setting issues crm contacts' do
     context 'when the contact belongs to a different group' do
       let(:group2) { create(:group) }
       let(:contact) { create(:contact, group: group2) }
-      let(:crm_contact_ids) { [global_id_of(contact)] }
+      let(:contact_ids) { [global_id_of(contact)] }
 
       before do
         group2.add_reporter(user)
@@ -137,7 +137,7 @@ RSpec.describe 'Setting issues crm contacts' do
     context 'when attempting to add more than 6' do
       let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] }
       let(:gid) { global_id_of(contacts[0]) }
-      let(:crm_contact_ids) { [gid, gid, gid, gid, gid, gid, gid] }
+      let(:contact_ids) { [gid, gid, gid, gid, gid, gid, gid] }
 
       it 'returns expected error' do
         post_graphql_mutation(mutation, current_user: user)
@@ -149,7 +149,7 @@ RSpec.describe 'Setting issues crm contacts' do
 
     context 'when trying to remove non-existent contact' do
       let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] }
-      let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
+      let(:contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
 
       it 'raises expected error' do
         post_graphql_mutation(mutation, current_user: user)

spec/services/issues/set_crm_contacts_service_spec.rb

@@ -22,13 +22,13 @@ RSpec.describe Issues::SetCrmContactsService do
 
   describe '#execute' do
     context 'when the user has no permission' do
-      let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } }
+      let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } }
 
       it 'returns expected error response' do
         response = set_crm_contacts
 
         expect(response).to be_error
-        expect(response.message).to match_array(['You have insufficient permissions to set customer relations contacts for this issue'])
+        expect(response.message).to eq('You have insufficient permissions to set customer relations contacts for this issue')
       end
     end
 
@@ -38,20 +38,20 @@ RSpec.describe Issues::SetCrmContactsService do
       end
 
       context 'when the contact does not exist' do
-        let(:params) { { crm_contact_ids: [non_existing_record_id] } }
+        let(:params) { { replace_ids: [non_existing_record_id] } }
 
         it 'returns expected error response' do
           response = set_crm_contacts
 
           expect(response).to be_error
-          expect(response.message).to match_array(["Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}"])
+          expect(response.message).to eq("Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}")
         end
       end
 
       context 'when the contact belongs to a different group' do
         let(:group2) { create(:group) }
         let(:contact) { create(:contact, group: group2) }
-        let(:params) { { crm_contact_ids: [contact.id] } }
+        let(:params) { { replace_ids: [contact.id] } }
 
         before do
           group2.add_reporter(user)
@@ -61,12 +61,12 @@ RSpec.describe Issues::SetCrmContactsService do
           response = set_crm_contacts
 
           expect(response).to be_error
-          expect(response.message).to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"])
+          expect(response.message).to eq("Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}")
         end
       end
 
       context 'replace' do
-        let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } }
+        let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } }
 
         it 'updates the issue with correct contacts' do
           response = set_crm_contacts
@@ -77,7 +77,18 @@ RSpec.describe Issues::SetCrmContactsService do
       end
 
       context 'add' do
-        let(:params) { { add_crm_contact_ids: [contacts[3].id] } }
+        let(:params) { { add_ids: [contacts[3].id] } }
+
+        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[0], contacts[1], contacts[3]])
+        end
+      end
+
+      context 'add by email' do
+        let(:params) { { add_emails: [contacts[3].email] } }
 
         it 'updates the issue with correct contacts' do
           response = set_crm_contacts
@@ -88,7 +99,18 @@ RSpec.describe Issues::SetCrmContactsService do
       end
 
       context 'remove' do
-        let(:params) { { remove_crm_contact_ids: [contacts[0].id] } }
+        let(:params) { { remove_ids: [contacts[0].id] } }
+
+        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
+      end
+
+      context 'remove by email' do
+        let(:params) { { remove_emails: [contacts[0].email] } }
 
         it 'updates the issue with correct contacts' do
           response = set_crm_contacts
@@ -100,18 +122,18 @@ RSpec.describe Issues::SetCrmContactsService do
 
       context 'when attempting to add more than 6' do
         let(:id) { contacts[0].id }
-        let(:params) { { add_crm_contact_ids: [id, id, id, id, id, id, id] } }
+        let(:params) { { add_ids: [id, id, id, id, id, id, id] } }
 
         it 'returns expected error message' do
           response = set_crm_contacts
 
           expect(response).to be_error
-          expect(response.message).to match_array(['You can only add up to 6 contacts at one time'])
+          expect(response.message).to eq('You can only add up to 6 contacts at one time')
         end
       end
 
       context 'when trying to remove non-existent contact' do
-        let(:params) { { remove_crm_contact_ids: [non_existing_record_id] } }
+        let(:params) { { remove_ids: [non_existing_record_id] } }
 
         it 'returns expected error message' do
           response = set_crm_contacts
@@ -122,10 +144,10 @@ RSpec.describe Issues::SetCrmContactsService do
       end
 
       context 'when combining params' do
-        let(:error_invalid_params) { 'You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids' }
+        let(:error_invalid_params) { 'You cannot combine replace_ids with add_ids or remove_ids' }
 
         context 'add and remove' do
-          let(:params) { { remove_crm_contact_ids: [contacts[1].id], add_crm_contact_ids: [contacts[3].id] } }
+          let(:params) { { remove_ids: [contacts[1].id], add_ids: [contacts[3].id] } }
 
           it 'updates the issue with correct contacts' do
             response = set_crm_contacts
@@ -136,25 +158,55 @@ RSpec.describe Issues::SetCrmContactsService do
         end
 
         context 'replace and remove' do
-          let(:params) { { crm_contact_ids: [contacts[3].id], remove_crm_contact_ids: [contacts[0].id] } }
+          let(:params) { { replace_ids: [contacts[3].id], remove_ids: [contacts[0].id] } }
 
           it 'returns expected error response' do
             response = set_crm_contacts
 
             expect(response).to be_error
-            expect(response.message).to match_array([error_invalid_params])
+            expect(response.message).to eq(error_invalid_params)
           end
         end
 
         context 'replace and add' do
-          let(:params) { { crm_contact_ids: [contacts[3].id], add_crm_contact_ids: [contacts[1].id] } }
+          let(:params) { { replace_ids: [contacts[3].id], add_ids: [contacts[1].id] } }
 
           it 'returns expected error response' do
             response = set_crm_contacts
 
             expect(response).to be_error
-            expect(response.message).to match_array([error_invalid_params])
+            expect(response.message).to eq(error_invalid_params)
+          end
+        end
       end
+
+      context 'when trying to add an existing issue contact' do
+        let(:params) { { add_ids: [contacts[0].id] } }
+
+        it 'does not return an error' do
+          response = set_crm_contacts
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'when trying to add the same contact twice' do
+        let(:params) { { add_ids: [contacts[3].id, contacts[3].id] } }
+
+        it 'does not return an error' do
+          response = set_crm_contacts
+
+          expect(response).to be_success
+        end
+      end
+
+      context 'when trying to remove a contact not attached to the issue' do
+        let(:params) { { remove_ids: [contacts[3].id] } }
+
+        it 'does not return an error' do
+          response = set_crm_contacts
+
+          expect(response).to be_success
         end
       end
     end

spec/services/quick_actions/interpret_service_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe QuickActions::InterpretService do
-  let_it_be(:public_project) { create(:project, :public) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:public_project) { create(:project, :public, group: group) }
   let_it_be(:repository_project) { create(:project, :repository) }
   let_it_be(:project) { public_project }
   let_it_be(:developer) { create(:user) }
@@ -2233,6 +2234,57 @@ RSpec.describe QuickActions::InterpretService do
         end
       end
     end
+
+    context 'crm_contact commands' do
+      let_it_be(:new_contact) { create(:contact, group: group) }
+      let_it_be(:existing_contact) { create(:contact, group: group) }
+
+      let(:add_command) { service.execute("/add_contacts #{new_contact.email}", issue) }
+      let(:remove_command) { service.execute("/remove_contacts #{existing_contact.email}", issue) }
+
+      before do
+        issue.project.group.add_developer(developer)
+        create(:issue_customer_relations_contact, issue: issue, contact: existing_contact)
+      end
+
+      context 'with feature flag disabled' do
+        before do
+          stub_feature_flags(customer_relations: false)
+        end
+
+        it 'add_contacts command does not add the contact' do
+          add_command
+
+          expect(issue.reload.customer_relations_contacts).to match_array([existing_contact])
+        end
+
+        it 'remove_contacts command does not remove the contact' do
+          remove_command
+
+          expect(issue.reload.customer_relations_contacts).to match_array([existing_contact])
+        end
+      end
+
+      it 'add_contacts command adds the contact' do
+        _, _, message = add_command
+
+        expect(issue.reload.customer_relations_contacts).to match_array([existing_contact, new_contact])
+        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
+
+        expect(issue.reload.customer_relations_contacts).to be_empty
+        expect(message).to eq('One or more contacts were successfully removed.')
+      end
+    end
   end
 
   describe '#explain' do