Merge branch 'graphql-allow-update-issue-contacts' into 'master'

Add issues set crm contacts service and graphql mutation

See merge request gitlab-org/gitlab!72686

app/graphql/mutations/issues/set_crm_contacts.rb

+# frozen_string_literal: true
+
+module Mutations
+  module Issues
+    class SetCrmContacts < Base
+      graphql_name 'IssueSetCrmContacts'
+
+      argument :crm_contact_ids,
+               [::Types::GlobalIDType[::CustomerRelations::Contact]],
+               required: true,
+               description: 'Customer relations contact IDs to set. Replaces existing contacts by default.'
+
+      argument :operation_mode,
+               Types::MutationOperationModeEnum,
+               required: false,
+               description: 'Changes the operation mode. Defaults to REPLACE.'
+
+      def resolve(project_path:, iid:, crm_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)
+
+          crm_contact_id.model_id.to_i
+        end
+
+        attribute_name = case operation_mode
+                         when Types::MutationOperationModeEnum.enum[:append]
+                           :add_crm_contact_ids
+                         when Types::MutationOperationModeEnum.enum[:remove]
+                           :remove_crm_contact_ids
+                         else
+                           :crm_contact_ids
+                         end
+
+        response = ::Issues::SetCrmContactsService.new(project: project, current_user: current_user, params: { attribute_name => crm_contact_ids })
+          .execute(issue)
+
+        {
+          issue: issue,
+          errors: response.errors
+        }
+      end
+    end
+  end
+end

app/graphql/types/mutation_type.rb

@@ -49,6 +49,7 @@ module Types
     mount_mutation Mutations::Environments::CanaryIngress::Update
     mount_mutation Mutations::Issues::Create
     mount_mutation Mutations::Issues::SetAssignees
+    mount_mutation Mutations::Issues::SetCrmContacts
     mount_mutation Mutations::Issues::SetConfidential
     mount_mutation Mutations::Issues::SetLocked
     mount_mutation Mutations::Issues::SetDueDate

app/models/customer_relations/issue_contact.rb

@@ -15,6 +15,6 @@ class CustomerRelations::IssueContact < ApplicationRecord
     return unless issue&.project&.namespace_id
     return if contact.group_id == issue.project.namespace_id
 
-    errors.add(:base, _('The contact does not belong to the same group as the issue.'))
+    errors.add(:base, _('The contact does not belong to the same group as the issue'))
   end
 end

app/policies/issue_policy.rb

@@ -12,6 +12,9 @@ class IssuePolicy < IssuablePolicy
     @user && IssueCollection.new([@subject]).visible_to(@user).any?
   end
 
+  desc "User can read contacts belonging to the issue group"
+  condition(:can_read_crm_contacts, scope: :subject) { @user.can?(:read_crm_contact, @subject.project.group) }
+
   desc "Issue is confidential"
   condition(:confidential, scope: :subject) { @subject.confidential? }
 
@@ -77,6 +80,10 @@ class IssuePolicy < IssuablePolicy
   rule { ~persisted & can?(:create_issue) }.policy do
     enable :set_confidentiality
   end
+
+  rule { can?(:set_issue_metadata) & can_read_crm_contacts }.policy do
+    enable :set_issue_crm_contacts
+  end
 end
 
 IssuePolicy.prepend_mod_with('IssuePolicy')

app/services/issues/set_crm_contacts_service.rb

+# frozen_string_literal: true
+
+module Issues
+  class SetCrmContactsService < ::BaseProjectService
+    attr_accessor :issue, :errors
+
+    MAX_ADDITIONAL_CONTACTS = 6
+
+    def execute(issue)
+      @issue = issue
+      @errors = []
+
+      return error_no_permissions unless allowed?
+      return error_invalid_params unless valid_params?
+
+      determine_changes if params[:crm_contact_ids]
+
+      return error_too_many if too_many?
+
+      add_contacts if params[:add_crm_contact_ids]
+      remove_contacts if params[:remove_crm_contact_ids]
+
+      if issue.valid?
+        ServiceResponse.success(payload: issue)
+      else
+        # 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)
+      end
+    end
+
+    private
+
+    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]
+    end
+
+    def add_contacts
+      params[:add_crm_contact_ids].uniq.each do |contact_id|
+        issue_contact = issue.issue_customer_relations_contacts.create(contact_id: contact_id)
+
+        unless issue_contact.persisted?
+          # The validation ensures that the id exists and the user has permission
+          errors << "#{contact_id}: The resource that you are attempting to access does not exist or you don't have permission to perform this action"
+        end
+      end
+    end
+
+    def remove_contacts
+      issue.issue_customer_relations_contacts
+        .where(contact_id: params[:remove_crm_contact_ids]) # rubocop: disable CodeReuse/ActiveRecord
+        .delete_all
+    end
+
+    def allowed?
+      current_user&.can?(:set_issue_crm_contacts, issue)
+    end
+
+    def valid_params?
+      set_present? ^ add_or_remove_present?
+    end
+
+    def set_present?
+      params[:crm_contact_ids].present?
+    end
+
+    def add_or_remove_present?
+      params[:add_crm_contact_ids].present? || params[:remove_crm_contact_ids].present?
+    end
+
+    def too_many?
+      params[:add_crm_contact_ids] && params[:add_crm_contact_ids].length > MAX_ADDITIONAL_CONTACTS
+    end
+
+    def error_no_permissions
+      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'])
+    end
+
+    def error_too_many
+      ServiceResponse.error(payload: issue, message: ["You can only add up to #{MAX_ADDITIONAL_CONTACTS} contacts at one time"])
+    end
+  end
+end

doc/api/graphql/reference/index.md

@@ -2820,6 +2820,28 @@ Input type: `IssueSetConfidentialInput`
 | <a id="mutationissuesetconfidentialerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
 | <a id="mutationissuesetconfidentialissue"></a>`issue` | [`Issue`](#issue) | Issue after mutation. |
 
+### `Mutation.issueSetCrmContacts`
+
+Input type: `IssueSetCrmContactsInput`
+
+#### Arguments
+
+| 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="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. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationissuesetcrmcontactsclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationissuesetcrmcontactserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationissuesetcrmcontactsissue"></a>`issue` | [`Issue`](#issue) | Issue after mutation. |
+
 ### `Mutation.issueSetDueDate`
 
 Input type: `IssueSetDueDateInput`

locale/gitlab.pot

@@ -34163,7 +34163,7 @@ msgstr ""
 msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination."
 msgstr ""
 
-msgid "The contact does not belong to the same group as the issue."
+msgid "The contact does not belong to the same group as the issue"
 msgstr ""
 
 msgid "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository."

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Setting issues crm contacts' do
+  include GraphqlHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:contacts) { create_list(:contact, 4, group: group) }
+
+  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(: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
+    variables = {
+      project_path: issue.project.full_path,
+      iid: issue.iid.to_s,
+      operation_mode: operation_mode,
+      crm_contact_ids: crm_contact_ids
+    }
+
+    graphql_mutation(:issue_set_crm_contacts, variables,
+                     <<-QL.strip_heredoc
+                       clientMutationId
+                       errors
+                       issue {
+                         customerRelationsContacts {
+                           nodes {
+                             id
+                           }
+                         }
+                       }
+                     QL
+    )
+  end
+
+  def mutation_response
+    graphql_mutation_response(:issue_set_crm_contacts)
+  end
+
+  before do
+    create(:issue_customer_relations_contact, issue: issue, contact: contacts[0])
+    create(:issue_customer_relations_contact, issue: issue, contact: contacts[1])
+  end
+
+  context 'when the user has no permission' do
+    it 'returns expected error' do
+      error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR
+      post_graphql_mutation(mutation, current_user: user)
+
+      expect(graphql_errors).to include(a_hash_including('message' => error))
+    end
+  end
+
+  context 'when the user has permission' do
+    before do
+      group.add_reporter(user)
+    end
+
+    context 'when the feature is disabled' do
+      before do
+        stub_feature_flags(customer_relations: false)
+      end
+
+      it 'raises expected error' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_errors).to include(a_hash_including('message' => 'Feature disabled'))
+      end
+    end
+
+    context 'replace' do
+      it 'updates the issue with correct contacts' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id))
+          .to match_array([global_id_of(contacts[1]), global_id_of(contacts[2])])
+      end
+    end
+
+    context 'append' do
+      let(:crm_contact_ids) { [global_id_of(contacts[3])] }
+      let(:operation_mode) { Types::MutationOperationModeEnum.enum[:append] }
+
+      it 'updates the issue with correct contacts' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id))
+          .to match_array([global_id_of(contacts[0]), global_id_of(contacts[1]), global_id_of(contacts[3])])
+      end
+    end
+
+    context 'remove' do
+      let(:crm_contact_ids) { [global_id_of(contacts[0])] }
+      let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] }
+
+      it 'updates the issue with correct contacts' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id))
+          .to match_array([global_id_of(contacts[1])])
+      end
+    end
+
+    context 'when the contact does not exist' do
+      let(:crm_contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] }
+
+      it 'returns expected error' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :errors))
+          .to match_array(["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(:crm_contact_ids) { [global_id_of(contact)] }
+
+      before do
+        group2.add_reporter(user)
+      end
+
+      it 'returns expected error' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :errors))
+        .to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"])
+      end
+    end
+
+    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] }
+
+      it 'returns expected error' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :errors))
+          .to match_array(["You can only add up to 6 contacts at one time"])
+      end
+    end
+
+    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}"] }
+
+      it 'raises expected error' do
+        post_graphql_mutation(mutation, current_user: user)
+
+        expect(graphql_data_at(:issue_set_crm_contacts, :errors)).to be_empty
+      end
+    end
+  end
+end

spec/services/issues/set_crm_contacts_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Issues::SetCrmContactsService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:contacts) { create_list(:contact, 4, group: group) }
+
+  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])
+  end
+
+  subject(:set_crm_contacts) do
+    described_class.new(project: project, current_user: user, params: params).execute(issue)
+  end
+
+  describe '#execute' do
+    context 'when the user has no permission' do
+      let(:params) { { crm_contact_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'])
+      end
+    end
+
+    context 'when user has permission' do
+      before do
+        group.add_reporter(user)
+      end
+
+      context 'when the contact does not exist' do
+        let(:params) { { crm_contact_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}"])
+        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] } }
+
+        before do
+          group2.add_reporter(user)
+        end
+
+        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 #{contact.id}: #{does_not_exist_or_no_permission}"])
+        end
+      end
+
+      context 'replace' do
+        let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].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], contacts[2]])
+        end
+      end
+
+      context 'add' do
+        let(:params) { { add_crm_contact_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 'remove' do
+        let(:params) { { remove_crm_contact_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 '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] } }
+
+        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'])
+        end
+      end
+
+      context 'when trying to remove non-existent contact' do
+        let(:params) { { remove_crm_contact_ids: [non_existing_record_id] } }
+
+        it 'returns expected error message' do
+          response = set_crm_contacts
+
+          expect(response).to be_success
+          expect(response.message).to be_nil
+        end
+      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' }
+
+        context 'add and remove' do
+          let(:params) { { remove_crm_contact_ids: [contacts[1].id], add_crm_contact_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[3]])
+          end
+        end
+
+        context 'replace and remove' do
+          let(:params) { { crm_contact_ids: [contacts[3].id], remove_crm_contact_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])
+          end
+        end
+
+        context 'replace and add' do
+          let(:params) { { crm_contact_ids: [contacts[3].id], add_crm_contact_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])
+          end
+        end
+      end
+    end
+  end
+end